On Fri, Jun 6, 2025 at 1:18 PM Claudiu <claudiu.beznea@xxxxxxxxx> wrote:
>
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> The dev_pm_domain_attach() function is typically used in bus code alongside
> dev_pm_domain_detach(), often following patterns like:
>
> static int bus_probe(struct device *_dev)
> {
> struct bus_driver *drv = to_bus_driver(dev->driver);
> struct bus_device *dev = to_bus_device(_dev);
> int ret;
>
> // ...
>
> ret = dev_pm_domain_attach(_dev, true);
> if (ret)
> return ret;
>
> if (drv->probe)
> ret = drv->probe(dev);
>
> // ...
> }
>
> static void bus_remove(struct device *_dev)
> {
> struct bus_driver *drv = to_bus_driver(dev->driver);
> struct bus_device *dev = to_bus_device(_dev);
>
> if (drv->remove)
> drv->remove(dev);
> dev_pm_domain_detach(_dev);
> }
>
> When the driver's probe function uses devres-managed resources that depend
> on the power domain state, those resources are released later during
> device_unbind_cleanup().
>
> Releasing devres-managed resources that depend on the power domain state
> after detaching the device from its PM domain can cause failures.
>
> For example, if the driver uses devm_pm_runtime_enable() in its probe
> function, and the device's clocks are managed by the PM domain, then
> during removal the runtime PM is disabled in device_unbind_cleanup() after
> the clocks have been removed from the PM domain. It may happen that the
> devm_pm_runtime_enable() action causes the device to be runtime-resumed.
Don't use devm_pm_runtime_enable() then.
> If the driver specific runtime PM APIs access registers directly, this
> will lead to accessing device registers without clocks being enabled.
> Similar issues may occur with other devres actions that access device
> registers.
>
> Add devm_pm_domain_attach(). When replacing the dev_pm_domain_attach() and
> dev_pm_domain_detach() in bus probe and bus remove, it ensures that the
> device is detached from its PM domain in device_unbind_cleanup(), only
> after all driver's devres-managed resources have been release.
>
> For flexibility, the implemented devm_pm_domain_attach() has 2 state
> arguments, one for the domain state on attach, one for the domain state on
> detach.
dev_pm_domain_attach() is not part driver API and I'm not convinced at
all by the arguments above.
Thanks!
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
> ---
>
> Changes in v3:
> - dropped devm_pm_domain_detach_off(), devm_pm_domain_detach_on()
> and use a single function devm_pm_domain_detach()
>
> Changes in v2:
> - none; this patch is new
>
> drivers/base/power/common.c | 50 +++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 6 +++++
> 2 files changed, 56 insertions(+)
>
> diff --git a/drivers/base/power/common.c b/drivers/base/power/common.c
> index 781968a128ff..82ea20b343f5 100644
> --- a/drivers/base/power/common.c
> +++ b/drivers/base/power/common.c
> @@ -115,6 +115,56 @@ int dev_pm_domain_attach(struct device *dev, bool power_on)
> }
> EXPORT_SYMBOL_GPL(dev_pm_domain_attach);
>
> +/**
> + * devm_pm_domain_detach - devres action for devm_pm_domain_attach() to
> + * detach a device from its domain.
> + * @dev: device to detach.
> + * @res: indicate if the device should be powered off
> + *
> + * This function reverse the actions from devm_pm_domain_attach().
> + * It will be invoked during the remove phase from drivers implicitly.
> + */
> +static void devm_pm_domain_detach(struct device *dev, void *res)
> +{
> + bool *power_off = res;
> +
> + dev_pm_domain_detach(dev, *power_off);
> +}
> +
> +/**
> + * devm_pm_domain_attach - devres-enabled version of dev_pm_domain_attach()
> + * @dev: Device to attach.
> + * @power_on: Use to indicate whether we should power on the device
> + * when attaching.
> + *
> + * NOTE: this will also handle calling dev_pm_domain_detach() for
> + * you during remove phase.
> + *
> + * Returns 0 on successfully attached PM domain, or a negative error code in
> + * case of a failure.
> + */
> +int devm_pm_domain_attach(struct device *dev, bool power_on)
> +{
> + bool *power_off;
> + int ret;
> +
> + power_off = devres_alloc(devm_pm_domain_detach, sizeof(*power_off), GFP_KERNEL);
> + if (!power_off)
> + return -ENOMEM;
> +
> + ret = dev_pm_domain_attach(dev, power_on);
> + if (ret) {
> + devres_free(power_off);
> + return ret;
> + }
> +
> + *power_off = power_on;
> + devres_add(dev, power_off);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(devm_pm_domain_attach);
> +
> /**
> * dev_pm_domain_attach_by_id - Associate a device with one of its PM domains.
> * @dev: The device used to lookup the PM domain.
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 0b18160901a2..f78b6b4dd734 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -509,6 +509,7 @@ struct device *dev_pm_domain_attach_by_name(struct device *dev,
> int dev_pm_domain_attach_list(struct device *dev,
> const struct dev_pm_domain_attach_data *data,
> struct dev_pm_domain_list **list);
> +int devm_pm_domain_attach(struct device *dev, bool power_on);
> int devm_pm_domain_attach_list(struct device *dev,
> const struct dev_pm_domain_attach_data *data,
> struct dev_pm_domain_list **list);
> @@ -539,6 +540,11 @@ static inline int dev_pm_domain_attach_list(struct device *dev,
> return 0;
> }
>
> +static inline int devm_pm_domain_attach(struct device *dev, bool power_on)
> +{
> + return 0;
> +}
> +
> static inline int devm_pm_domain_attach_list(struct device *dev,
> const struct dev_pm_domain_attach_data *data,
> struct dev_pm_domain_list **list)
> --
> 2.43.0
>
