| Message ID | 1449150480-1173-1-git-send-email-pelcan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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> >> --- >>
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> >>> --- >>> > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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> >>>> --- >>>>
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> >>>>> --- >>>>> > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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;
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(+)