[PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Philip Elcan]

This allows setting an SDHC controller as non-removable
by using the _RMV method in the ACPI table. It doesn't
mark it as non-removable if GPIO card detection is
already setup.

Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
---
 drivers/mmc/host/sdhci-acpi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index f6047fc..8c06ba6 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -288,6 +288,20 @@ static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid,
 	return NULL;
 }
 
+static bool sdhci_acpi_is_removable(acpi_handle handle)
+{
+	acpi_status status;
+	unsigned long long removable = 1; /* default to removable */
+
+	if (acpi_has_method(handle, "_EJ0"))
+		return true;
+	status = acpi_evaluate_integer(handle, "_RMV", NULL, &removable);
+	if (ACPI_SUCCESS(status) && !removable)
+		return false;
+
+	return true;
+}
+
 static int sdhci_acpi_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -300,6 +314,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 	const char *hid;
 	const char *uid;
 	int err;
+	bool gpio_cd = false;
 
 	if (acpi_bus_get_device(handle, &device))
 		return -ENODEV;
@@ -373,9 +388,14 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
 		if (mmc_gpiod_request_cd(host->mmc, NULL, 0, v, 0, NULL)) {
 			dev_warn(dev, "failed to setup card detect gpio\n");
 			c->use_runtime_pm = false;
+		} else {
+			gpio_cd = true;
 		}
 	}
 
+	if (!gpio_cd && !sdhci_acpi_is_removable(handle))
+		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
+
 	err = sdhci_add_host(host);
 	if (err)
 		goto err_free;
-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Adrian Hunter]

On 03/12/15 15:48, Philip Elcan wrote:
> This allows setting an SDHC controller as non-removable
> by using the _RMV method in the ACPI table. It doesn't

Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
device node?

> mark it as non-removable if GPIO card detection is
> already setup.
> 
> Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
> ---
>  drivers/mmc/host/sdhci-acpi.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
> index f6047fc..8c06ba6 100644
> --- a/drivers/mmc/host/sdhci-acpi.c
> +++ b/drivers/mmc/host/sdhci-acpi.c
> @@ -288,6 +288,20 @@ static const struct sdhci_acpi_slot *sdhci_acpi_get_slot(const char *hid,
>  	return NULL;
>  }
>  
> +static bool sdhci_acpi_is_removable(acpi_handle handle)
> +{
> +	acpi_status status;
> +	unsigned long long removable = 1; /* default to removable */
> +
> +	if (acpi_has_method(handle, "_EJ0"))
> +		return true;
> +	status = acpi_evaluate_integer(handle, "_RMV", NULL, &removable);
> +	if (ACPI_SUCCESS(status) && !removable)
> +		return false;
> +
> +	return true;
> +}
> +
>  static int sdhci_acpi_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -300,6 +314,7 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>  	const char *hid;
>  	const char *uid;
>  	int err;
> +	bool gpio_cd = false;
>  
>  	if (acpi_bus_get_device(handle, &device))
>  		return -ENODEV;
> @@ -373,9 +388,14 @@ static int sdhci_acpi_probe(struct platform_device *pdev)
>  		if (mmc_gpiod_request_cd(host->mmc, NULL, 0, v, 0, NULL)) {
>  			dev_warn(dev, "failed to setup card detect gpio\n");
>  			c->use_runtime_pm = false;
> +		} else {
> +			gpio_cd = true;
>  		}
>  	}
>  
> +	if (!gpio_cd && !sdhci_acpi_is_removable(handle))
> +		host->mmc->caps |= MMC_CAP_NONREMOVABLE;
> +
>  	err = sdhci_add_host(host);
>  	if (err)
>  		goto err_free;
> 

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Philip Elcan]

On 12/03/2015 09:14 AM, Adrian Hunter wrote:
> On 03/12/15 15:48, Philip Elcan wrote:
>> This allows setting an SDHC controller as non-removable
>> by using the _RMV method in the ACPI table. It doesn't
> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
> device node?

Yes, this is on the host controller. The ACPI table only describes the
host controller, not the child nodes.

>
>> mark it as non-removable if GPIO card detection is
>> already setup.
>>
>> Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
>> ---
>>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Adrian Hunter]

On 04/12/15 17:40, Philip Elcan wrote:
> 
> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
>> On 03/12/15 15:48, Philip Elcan wrote:
>>> This allows setting an SDHC controller as non-removable
>>> by using the _RMV method in the ACPI table. It doesn't
>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
>> device node?
> 
> Yes, this is on the host controller. The ACPI table only describes the
> host controller, not the child nodes.
> 

If you look at Intel devices, the _RMV is on the child e.g.

        Device (SDHA)
        {
            Name (_HID, "80860F14")  // _HID: Hardware ID
            Name (_CID, "PNP0D40")  // _CID: Compatible ID
            Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS Device Name
	    ...
            Device (EMMD)
            {
		...
                Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
                {
                    Return (Zero)
                }
            }
        }

I am not an ACPI expert but that seems like the correct place for it.

>>
>>> mark it as non-removable if GPIO card detection is
>>> already setup.
>>>
>>> Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
>>> ---
>>>
> 

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Philip Elcan]

On 12/07/2015 03:30 AM, Adrian Hunter wrote:
> On 04/12/15 17:40, Philip Elcan wrote:
>> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
>>> On 03/12/15 15:48, Philip Elcan wrote:
>>>> This allows setting an SDHC controller as non-removable
>>>> by using the _RMV method in the ACPI table. It doesn't
>>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
>>> device node?
>> Yes, this is on the host controller. The ACPI table only describes the
>> host controller, not the child nodes.
>>
> If you look at Intel devices, the _RMV is on the child e.g.
>
>         Device (SDHA)
>         {
>             Name (_HID, "80860F14")  // _HID: Hardware ID
>             Name (_CID, "PNP0D40")  // _CID: Compatible ID
>             Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS Device Name
> 	    ...
>             Device (EMMD)
>             {
> 		...
>                 Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
>                 {
>                     Return (Zero)
>                 }
>             }
>         }
>
> I am not an ACPI expert but that seems like the correct place for it.
My understanding is that in ACPI you don't generally create child devices on buses that are discoverable.
>
>>>> mark it as non-removable if GPIO card detection is
>>>> already setup.
>>>>
>>>> Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
>>>> ---
>>>>

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Adrian Hunter]

On 10/12/15 22:57, Philip Elcan wrote:
> 
> On 12/07/2015 03:30 AM, Adrian Hunter wrote:
>> On 04/12/15 17:40, Philip Elcan wrote:
>>> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
>>>> On 03/12/15 15:48, Philip Elcan wrote:
>>>>> This allows setting an SDHC controller as non-removable
>>>>> by using the _RMV method in the ACPI table. It doesn't
>>>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
>>>> device node?
>>> Yes, this is on the host controller. The ACPI table only describes the
>>> host controller, not the child nodes.
>>>
>> If you look at Intel devices, the _RMV is on the child e.g.
>>
>>         Device (SDHA)
>>         {
>>             Name (_HID, "80860F14")  // _HID: Hardware ID
>>             Name (_CID, "PNP0D40")  // _CID: Compatible ID
>>             Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS Device Name
>> 	    ...
>>             Device (EMMD)
>>             {
>> 		...
>>                 Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
>>                 {
>>                     Return (Zero)
>>                 }
>>             }
>>         }
>>
>> I am not an ACPI expert but that seems like the correct place for it.
> My understanding is that in ACPI you don't generally create child devices on buses that are discoverable.

I've cc'ed Rafael and the linux-acpi mailing list.  Maybe someone there can
comment.

>>
>>>>> mark it as non-removable if GPIO card detection is
>>>>> already setup.
>>>>>
>>>>> Signed-off-by: Philip Elcan <pelcan@codeaurora.org>
>>>>> ---
>>>>>
> 

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Rafael J. Wysocki]

On Friday, December 11, 2015 10:17:18 AM Adrian Hunter wrote:
> On 10/12/15 22:57, Philip Elcan wrote:
> > 
> > On 12/07/2015 03:30 AM, Adrian Hunter wrote:
> >> On 04/12/15 17:40, Philip Elcan wrote:
> >>> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
> >>>> On 03/12/15 15:48, Philip Elcan wrote:
> >>>>> This allows setting an SDHC controller as non-removable
> >>>>> by using the _RMV method in the ACPI table. It doesn't
> >>>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
> >>>> device node?
> >>> Yes, this is on the host controller. The ACPI table only describes the
> >>> host controller, not the child nodes.
> >>>
> >> If you look at Intel devices, the _RMV is on the child e.g.
> >>
> >>         Device (SDHA)
> >>         {
> >>             Name (_HID, "80860F14")  // _HID: Hardware ID
> >>             Name (_CID, "PNP0D40")  // _CID: Compatible ID
> >>             Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS Device Name
> >> 	    ...
> >>             Device (EMMD)
> >>             {
> >> 		...
> >>                 Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
> >>                 {
> >>                     Return (Zero)
> >>                 }
> >>             }
> >>         }
> >>
> >> I am not an ACPI expert but that seems like the correct place for it.
> > My understanding is that in ACPI you don't generally create child devices on buses that are discoverable.
> 
> I've cc'ed Rafael and the linux-acpi mailing list.  Maybe someone there can
> comment.

The context here is a bit unclear to me.

Quite frankly, I don't see now _RMV above is useful for anything.  As per the
spec, _RMV is only necessary for devices that *can* be removed from the system
and where there's no eject mechanism controlled by software.  For those
devices _RMV is intended to indicate that it is safe to remove the device
at the time _RMV is evaluated.  Devices that can never be removed don't
need _RMV at all.

Thanks,
Rafael

Re: [PATCH] mmc: sdhci-acpi: set non-removable in ACPI table

[Rafael J. Wysocki]

On 12/15/2015 10:36 PM, Philip Elcan wrote:
> On 12/11/2015 05:53 PM, Rafael J. Wysocki wrote:
>> On Friday, December 11, 2015 10:17:18 AM Adrian Hunter wrote:
>>> On 10/12/15 22:57, Philip Elcan wrote:
>>>> On 12/07/2015 03:30 AM, Adrian Hunter wrote:
>>>>> On 04/12/15 17:40, Philip Elcan wrote:
>>>>>> On 12/03/2015 09:14 AM, Adrian Hunter wrote:
>>>>>>> On 03/12/15 15:48, Philip Elcan wrote:
>>>>>>>> This allows setting an SDHC controller as non-removable
>>>>>>>> by using the _RMV method in the ACPI table. It doesn't
>>>>>>> Is that _RMV on the host controller?  Shouldn't it be on the card i.e. child
>>>>>>> device node?
>>>>>> Yes, this is on the host controller. The ACPI table only describes the
>>>>>> host controller, not the child nodes.
>>>>>>
>>>>> If you look at Intel devices, the _RMV is on the child e.g.
>>>>>
>>>>>          Device (SDHA)
>>>>>          {
>>>>>              Name (_HID, "80860F14")  // _HID: Hardware ID
>>>>>              Name (_CID, "PNP0D40")  // _CID: Compatible ID
>>>>>              Name (_DDN, "Intel(R) eMMC Controller - 80860F14")  // _DDN: DOS Device Name
>>>>> 	    ...
>>>>>              Device (EMMD)
>>>>>              {
>>>>> 		...
>>>>>                  Method (_RMV, 0, NotSerialized)  // _RMV: Removal Status
>>>>>                  {
>>>>>                      Return (Zero)
>>>>>                  }
>>>>>              }
>>>>>          }
>>>>>
>>>>> I am not an ACPI expert but that seems like the correct place for it.
>>>> My understanding is that in ACPI you don't generally create child devices on buses that are discoverable.
>>> I've cc'ed Rafael and the linux-acpi mailing list.  Maybe someone there can
>>> comment.
>> The context here is a bit unclear to me.
>>
>> Quite frankly, I don't see now _RMV above is useful for anything.  As per the
>> spec, _RMV is only necessary for devices that *can* be removed from the system
>> and where there's no eject mechanism controlled by software.  For those
>> devices _RMV is intended to indicate that it is safe to remove the device
>> at the time _RMV is evaluated.  Devices that can never be removed don't
>> need _RMV at all.
>>
>> Thanks,
>> Rafael
>>
> Maybe I'm misinterpreting something, but the spec says that "_RMV object indicates
> to OSPM whether the device can be removed while the system is in the working state
> and does not require any ACPI system firmware actions to be performed for the device
> to be safely removed from the system." That sounds exactly like what I'm trying to
> do. And from Adrian's example, it sounds like others are using this to indicate if
> a devices is removeable.

It also says "Any such removable device that does not have _LCK or _EJx 
control methods must have and _RMV object."

Which clearly implies that _RMV is not required for devices that cannot 
be removed at all.  Indeed, it is redundant in those cases as stated above.

> However, if I follow your paradigm, I can just test if the _RMV object exists, and if
> it doesn't, I can assume the device is not removable?

That's correct as far as I can say.

> I'm concerned not all firmware will have this object implemented.

That's correct too.

It only is intended for devices that may be removed in a 
surprise-removal fashion and it is rarely used in general.

> Is there something else you suggest I use in the ACPI table to designate an SD/MMC
> device is removable?

If you want to say "this device is removable", then there should be 
either _LCK/_EJx or _RMV object present for it.  The latter indicates 
that the device can be removed at any time in principle, but it is only 
safe to remove it if/when 1 is returned by _RMV. That is, the OS can't 
prevent the user from removing the device, but it can indicate to the 
user (this way or another) when it is safe to remove it.  The decision 
whether or not to do that is still up to the user.

Thanks,
Rafael