mmc: sdhci-acpi: set non-removable in ACPI table

PATCH In review Philip Elcan 4 participants 8 messages Dec 3, 2015 – Dec 16, 2015 1 message not in the archive

Subsystems (4)

Arm Qualcomm mailing list ACPI MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver

lore

Share & tools

lore

Philip Elcan

author

2015-12-03 13:48 UTC #0 lore raw

PATCH 1/1 mmc: sdhci-acpi: set non-removable in ACPI table

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 <[email protected]>

---
 drivers/mmc/host/sdhci-acpi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

drivers/mmc/host/sdhci-acpi.c +20 -0

index f6047fc..8c06ba6 100644
@@ -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;

Signature · 3 lines

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

Adrian Hunter

maintainer

2015-12-03 14:14 UTC #1 lore raw

Quoted 2 lines · Philip Elcan

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?

Quoted 57 lines

mark it as non-removable if GPIO card detection is already setup.

Signed-off-by Philip Elcan <[email protected]>

---
 drivers/mmc/host/sdhci-acpi.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

drivers/mmc/host/sdhci-acpi.c +20 -0

index f6047fc..8c06ba6 100644
@@ -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;

Philip Elcan

author

2015-12-04 15:40 UTC #2 lore raw

Quoted 5 lines · Adrian Hunter

Quoted 2 lines · Philip Elcan
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.

Quoted 7 lines

Quoted 7 lines
mark it as non-removable if GPIO card detection is already setup.
Signed-off-by Philip Elcan <[email protected]>
---

Signature · 3 lines

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

Adrian Hunter

maintainer

2015-12-07 08:30 UTC #3 lore raw

Quoted 11 lines · Philip Elcan

Quoted 5 lines · Adrian Hunter
Quoted 2 lines · Philip Elcan
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.

Quoted 8 lines

Quoted 7 lines
Quoted 7 lines
mark it as non-removable if GPIO card detection is already setup.
Signed-off-by Philip Elcan <[email protected]>
---

Philip Elcan

author

2015-12-10 20:57 UTC #4 lore raw

Quoted 28 lines · Adrian Hunter

Quoted 9 lines · Philip Elcan
Quoted 5 lines · Adrian Hunter
Quoted 2 lines · Philip Elcan
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)
                }
            }
        }

Quoted 1 line

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.

Quoted 7 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
mark it as non-removable if GPIO card detection is already setup.
Signed-off-by Philip Elcan <[email protected]>
---

Signature · 3 lines

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

Adrian Hunter

maintainer

2015-12-11 08:17 UTC #5 lore raw

Quoted 30 lines · Philip Elcan

Quoted 28 lines · Adrian Hunter
Quoted 9 lines · Philip Elcan
Quoted 5 lines · Adrian Hunter
Quoted 2 lines · Philip Elcan
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)
                }
            }
        }

Quoted 2 lines

Quoted 1 line
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.

Quoted 8 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
mark it as non-removable if GPIO card detection is already setup.
Signed-off-by Philip Elcan <[email protected]>
---

Rafael J. Wysocki

2015-12-11 22:23 UTC #6 lore raw

Quoted 31 lines · Adrian Hunter

Quoted 30 lines · Philip Elcan
Quoted 28 lines · Adrian Hunter
Quoted 9 lines · Philip Elcan
Quoted 5 lines · Adrian Hunter
Quoted 2 lines · Philip Elcan
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)
                }
            }
        }

Quoted 5 lines

Quoted 2 lines
Quoted 1 line
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

A message that is not in the archive (sent off-list or not yet received): [email protected]

Rafael J. Wysocki

2015-12-16 01:17 UTC #8 lore raw

Quoted 48 lines · Philip Elcan

Quoted 44 lines · Rafael J. Wysocki
Quoted 30 lines · Adrian Hunter
Quoted 29 lines · Philip Elcan
Quoted 28 lines · Adrian Hunter
Quoted 9 lines · Philip Elcan
Quoted 5 lines · Adrian Hunter
Quoted 2 lines · Philip Elcan
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)
                 }
             }
         }
Quoted 4 lines
Quoted 2 lines
Quoted 1 line
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.
Quoted 3 lines
Thanks,
Rafael

Quoted 6 lines

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.

Quoted 2 lines

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.

Quoted 1 line

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.

Quoted 2 lines

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