Linux ACPI
 help / color / mirror / Atom feed
* [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion
@ 2026-10-17 15:59 Hans de Goede
  0 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2026-10-17 15:59 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Hans de Goede, Aaron Lu, linux-mmc, linux-acpi

Many MMC host drivers call pm_suspend_ignore_children() which allows
to keep MMC cards and SDIO-functions on while the host is suspended.

For SDIO functions which are in the ACPI power-domain this causes a race
where the host may end up runtime-suspended before the driver for a SDIO
function loads and probes it. When the probe() looses this race this causes
the pm_runtime_get_sync() in sdio_bus_probe() to fail in the ACPI pm-domain
code with e.g. the following error:

 ACPI: \_SB_.SDHB.WLAN: Cannot transition to D0 for parent in D3hot

Busses using pm_suspend_ignore_children() for their children should set
acpi_device.power.flags.ignore_parent on the ACPI companion device of
their children to avoid this.

This is safe todo because drivers for sdio_func devices should always
call sdio_claim_host() before doing any IO and that runtime-resumes
the host.

Add the missing setting of the acpi_device.power.flags.ignore_parent flag
to fix probing of SDIO functions sometimes failing on ACPI systems with
the error mentioned above.

Also add a sdio_acpi_clear_handle() function to undo this, since the ACPI
companion device wills stay around after removing the sdio_func device. To
make sdio_acpi_clear_handle() the proper inverse of sdio_acpi_set_handle()
also make it actually clear the companion pointer on the sdio_func device.
Technically this clearing is not really necessary since the sdio_func
device will go away shortly afterwards anyways.

Closes: https://github.com/MocLG/rtw/issues/2/
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/mmc/core/sdio_bus.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 4b07098c33c3..fa1d300211d3 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -373,11 +373,27 @@ static void sdio_acpi_set_handle(struct sdio_func *func)
 {
 	struct mmc_host *host = func->card->host;
 	u64 addr = ((u64)host->slotno << 16) | func->num;
+	struct acpi_device *adev;
 
 	acpi_preset_companion(&func->dev, ACPI_COMPANION(host->parent), addr);
+	adev = ACPI_COMPANION(&func->dev);
+	if (adev)
+		adev->power.flags.ignore_parent = true;
+}
+
+static void sdio_acpi_clear_handle(struct sdio_func *func)
+{
+	struct acpi_device *adev;
+
+	adev = ACPI_COMPANION(&func->dev);
+	if (adev) {
+		adev->power.flags.ignore_parent = false;
+		ACPI_COMPANION_SET(&func->dev, NULL);
+	}
 }
 #else
 static inline void sdio_acpi_set_handle(struct sdio_func *func) {}
+static inline void sdio_acpi_clear_handle(struct sdio_func *func) {}
 #endif
 
 static void sdio_set_of_node(struct sdio_func *func)
@@ -402,6 +418,8 @@ int sdio_add_func(struct sdio_func *func)
 	ret = device_add(&func->dev);
 	if (ret == 0)
 		sdio_func_set_present(func);
+	else
+		sdio_acpi_clear_handle(func);
 
 	return ret;
 }
@@ -414,6 +432,8 @@ int sdio_add_func(struct sdio_func *func)
  */
 void sdio_remove_func(struct sdio_func *func)
 {
+	sdio_acpi_clear_handle(func);
+
 	if (sdio_func_present(func))
 		device_del(&func->dev);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion
@ 2026-10-17 16:04 Hans de Goede
  2026-10-17 16:06 ` Hans de Goede
  2026-10-18 12:00 ` Luka Gejak
  0 siblings, 2 replies; 4+ messages in thread
From: Hans de Goede @ 2026-10-17 16:04 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Hans de Goede, Aaron Lu, linux-mmc, linux-acpi

Many MMC host drivers call pm_suspend_ignore_children() which allows
to keep MMC cards and SDIO-functions on while the host is suspended.

For SDIO functions which are in the ACPI power-domain this causes a race
where the host may end up runtime-suspended before the driver for a SDIO
function loads and probes it. When the probe() looses this race this causes
the pm_runtime_get_sync() in sdio_bus_probe() to fail in the ACPI pm-domain
code with e.g. the following error:

 ACPI: \_SB_.SDHB.WLAN: Cannot transition to D0 for parent in D3hot

Busses using pm_suspend_ignore_children() for their children should set
acpi_device.power.flags.ignore_parent on the ACPI companion device of
their children to avoid this.

This is safe todo because drivers for sdio_func devices should always
call sdio_claim_host() before doing any IO and that runtime-resumes
the host.

Add the missing setting of the acpi_device.power.flags.ignore_parent flag
to fix probing of SDIO functions sometimes failing on ACPI systems with
the error mentioned above.

Also add a sdio_acpi_clear_handle() function to undo this, since the ACPI
companion device wills stay around after removing the sdio_func device. To
make sdio_acpi_clear_handle() the proper inverse of sdio_acpi_set_handle()
also make it actually clear the companion pointer on the sdio_func device.
Technically this clearing is not really necessary since the sdio_func
device will go away shortly afterwards anyways.

Closes: https://github.com/MocLG/rtw/issues/2/
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 drivers/mmc/core/sdio_bus.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
index 4b07098c33c3..fa1d300211d3 100644
--- a/drivers/mmc/core/sdio_bus.c
+++ b/drivers/mmc/core/sdio_bus.c
@@ -373,11 +373,27 @@ static void sdio_acpi_set_handle(struct sdio_func *func)
 {
 	struct mmc_host *host = func->card->host;
 	u64 addr = ((u64)host->slotno << 16) | func->num;
+	struct acpi_device *adev;
 
 	acpi_preset_companion(&func->dev, ACPI_COMPANION(host->parent), addr);
+	adev = ACPI_COMPANION(&func->dev);
+	if (adev)
+		adev->power.flags.ignore_parent = true;
+}
+
+static void sdio_acpi_clear_handle(struct sdio_func *func)
+{
+	struct acpi_device *adev;
+
+	adev = ACPI_COMPANION(&func->dev);
+	if (adev) {
+		adev->power.flags.ignore_parent = false;
+		ACPI_COMPANION_SET(&func->dev, NULL);
+	}
 }
 #else
 static inline void sdio_acpi_set_handle(struct sdio_func *func) {}
+static inline void sdio_acpi_clear_handle(struct sdio_func *func) {}
 #endif
 
 static void sdio_set_of_node(struct sdio_func *func)
@@ -402,6 +418,8 @@ int sdio_add_func(struct sdio_func *func)
 	ret = device_add(&func->dev);
 	if (ret == 0)
 		sdio_func_set_present(func);
+	else
+		sdio_acpi_clear_handle(func);
 
 	return ret;
 }
@@ -414,6 +432,8 @@ int sdio_add_func(struct sdio_func *func)
  */
 void sdio_remove_func(struct sdio_func *func)
 {
+	sdio_acpi_clear_handle(func);
+
 	if (sdio_func_present(func))
 		device_del(&func->dev);
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion
  2026-10-17 16:04 [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion Hans de Goede
@ 2026-10-17 16:06 ` Hans de Goede
  2026-10-18 12:00 ` Luka Gejak
  1 sibling, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2026-10-17 16:06 UTC (permalink / raw)
  To: Ulf Hansson; +Cc: Aaron Lu, linux-mmc, linux-acpi

Hi All,

On 17-Oct-26 18:04, Hans de Goede wrote:
> Many MMC host drivers call pm_suspend_ignore_children() which allows
> to keep MMC cards and SDIO-functions on while the host is suspended.
> 
> For SDIO functions which are in the ACPI power-domain this causes a race
> where the host may end up runtime-suspended before the driver for a SDIO
> function loads and probes it. When the probe() looses this race this causes
> the pm_runtime_get_sync() in sdio_bus_probe() to fail in the ACPI pm-domain
> code with e.g. the following error:
> 
>  ACPI: \_SB_.SDHB.WLAN: Cannot transition to D0 for parent in D3hot
> 
> Busses using pm_suspend_ignore_children() for their children should set
> acpi_device.power.flags.ignore_parent on the ACPI companion device of
> their children to avoid this.
> 
> This is safe todo because drivers for sdio_func devices should always
> call sdio_claim_host() before doing any IO and that runtime-resumes
> the host.
> 
> Add the missing setting of the acpi_device.power.flags.ignore_parent flag
> to fix probing of SDIO functions sometimes failing on ACPI systems with
> the error mentioned above.
> 
> Also add a sdio_acpi_clear_handle() function to undo this, since the ACPI
> companion device wills stay around after removing the sdio_func device. To
> make sdio_acpi_clear_handle() the proper inverse of sdio_acpi_set_handle()
> also make it actually clear the companion pointer on the sdio_func device.
> Technically this clearing is not really necessary since the sdio_func
> device will go away shortly afterwards anyways.
> 
> Closes: https://github.com/MocLG/rtw/issues/2/
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>

2 remarks:

1. This is a resend with Ulf's email address fixed
2. This patch makes the ignore_children / ignore_parent handling
   identical to how the SPI and I2C busses are handling this.

Regards,

Hans



> ---
>  drivers/mmc/core/sdio_bus.c | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/drivers/mmc/core/sdio_bus.c b/drivers/mmc/core/sdio_bus.c
> index 4b07098c33c3..fa1d300211d3 100644
> --- a/drivers/mmc/core/sdio_bus.c
> +++ b/drivers/mmc/core/sdio_bus.c
> @@ -373,11 +373,27 @@ static void sdio_acpi_set_handle(struct sdio_func *func)
>  {
>  	struct mmc_host *host = func->card->host;
>  	u64 addr = ((u64)host->slotno << 16) | func->num;
> +	struct acpi_device *adev;
>  
>  	acpi_preset_companion(&func->dev, ACPI_COMPANION(host->parent), addr);
> +	adev = ACPI_COMPANION(&func->dev);
> +	if (adev)
> +		adev->power.flags.ignore_parent = true;
> +}
> +
> +static void sdio_acpi_clear_handle(struct sdio_func *func)
> +{
> +	struct acpi_device *adev;
> +
> +	adev = ACPI_COMPANION(&func->dev);
> +	if (adev) {
> +		adev->power.flags.ignore_parent = false;
> +		ACPI_COMPANION_SET(&func->dev, NULL);
> +	}
>  }
>  #else
>  static inline void sdio_acpi_set_handle(struct sdio_func *func) {}
> +static inline void sdio_acpi_clear_handle(struct sdio_func *func) {}
>  #endif
>  
>  static void sdio_set_of_node(struct sdio_func *func)
> @@ -402,6 +418,8 @@ int sdio_add_func(struct sdio_func *func)
>  	ret = device_add(&func->dev);
>  	if (ret == 0)
>  		sdio_func_set_present(func);
> +	else
> +		sdio_acpi_clear_handle(func);
>  
>  	return ret;
>  }
> @@ -414,6 +432,8 @@ int sdio_add_func(struct sdio_func *func)
>   */
>  void sdio_remove_func(struct sdio_func *func)
>  {
> +	sdio_acpi_clear_handle(func);
> +
>  	if (sdio_func_present(func))
>  		device_del(&func->dev);
>  


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion
  2026-10-17 16:04 [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion Hans de Goede
  2026-10-17 16:06 ` Hans de Goede
@ 2026-10-18 12:00 ` Luka Gejak
  1 sibling, 0 replies; 4+ messages in thread
From: Luka Gejak @ 2026-10-18 12:00 UTC (permalink / raw)
  To: Hans de Goede, Ulf Hansson; +Cc: Hans de Goede, Aaron Lu, linux-mmc, linux-acpi

Hi Hans,

October 17, 2026 at 6:04 PM, "Hans de Goede" <johannes.goede@oss.qualcomm.com> wrote:

> 
> Many MMC host drivers call pm_suspend_ignore_children() which allows
> to keep MMC cards and SDIO-functions on while the host is suspended.
> 
> For SDIO functions which are in the ACPI power-domain this causes a race
> where the host may end up runtime-suspended before the driver for a SDIO
> function loads and probes it. When the probe() looses this race this causes
> the pm_runtime_get_sync() in sdio_bus_probe() to fail in the ACPI pm-domain
> code with e.g. the following error:
> 
>  ACPI: \_SB_.SDHB.WLAN: Cannot transition to D0 for parent in D3hot
> 
> Busses using pm_suspend_ignore_children() for their children should set
> acpi_device.power.flags.ignore_parent on the ACPI companion device of
> their children to avoid this.
> 
> This is safe todo because drivers for sdio_func devices should always
> call sdio_claim_host() before doing any IO and that runtime-resumes
> the host.
> 
> Add the missing setting of the acpi_device.power.flags.ignore_parent flag
> to fix probing of SDIO functions sometimes failing on ACPI systems with
> the error mentioned above.
> 
> Also add a sdio_acpi_clear_handle() function to undo this, since the ACPI
> companion device wills stay around after removing the sdio_func device. To
> make sdio_acpi_clear_handle() the proper inverse of sdio_acpi_set_handle()
> also make it actually clear the companion pointer on the sdio_func device.
> Technically this clearing is not really necessary since the sdio_func
> device will go away shortly afterwards anyways.
> 
> Closes: https://github.com/MocLG/rtw/issues/2/
> Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
> ---
[...]
+
> +static void sdio_acpi_clear_handle(struct sdio_func *func)
> +{
> +     struct acpi_device *adev;
> +
> +     adev = ACPI_COMPANION(&func->dev);
> +     if (adev) {
> +             adev->power.flags.ignore_parent = false;
> +             ACPI_COMPANION_SET(&func->dev, NULL);
> +     }
>  }
>  #else
>  static inline void sdio_acpi_set_handle(struct sdio_func *func) {}
> +static inline void sdio_acpi_clear_handle(struct sdio_func *func) {}
>  #endif
>
>  static void sdio_set_of_node(struct sdio_func *func)
[...]
> @@ -414,6 +432,8 @@ int sdio_add_func(struct sdio_func *func)
>   */
>  void sdio_remove_func(struct sdio_func *func)
>  {
> +     sdio_acpi_clear_handle(func);
> +
>       if (sdio_func_present(func))
>               device_del(&func->dev);
>

I am not an expert on this topic, but clearing the companion here
before device_del(), looks wrong to me. As far as I understand
device_add() binds this device with acpi_bind_one() and that takes
a reference on both the companion and the device. The unbinding is
done in device_del() via acpi_device_notify_remove(), which returns
early when the companion is NULL, so does acpi_unbind_one()
never run and the physical_node entry, the sysfs links and the
references from acpi_bind_one() all stay behind and does that also
mean that sdio_release_func() never runs, causing the sdio_func and
its card reference to leak on every removal?

Also it seems to me that the stale entry stays first in the physical
node list, so the next card init would fail
acpi_device_is_first_physical_node() and would gets no ACPI PM domain
at all. Therefore the flag set here would then do nothing.

Would it be enough to only clear ignore_parent in sdio_acpi_clear_handle()
and let device_del() clear the companion?

Take this review with a grain of salt as I am not an expert on this
topic as I previously said.

Best regards,
Luka Gejak

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-18 12:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-17 16:04 [PATCH] mmc: sdio: Set acpi_device.power.flags.ignore_parent flag on sdio_func companion Hans de Goede
2026-10-17 16:06 ` Hans de Goede
2026-10-18 12:00 ` Luka Gejak
  -- strict thread matches above, loose matches on Subject: below --
2026-10-17 15:59 Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox