linux-kernel - Re: [PATCH v2 1/2] PM: domains: Add devres variant for dev_pm_domain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <rmc7me6rvumr6pcekgp5lsrxtuge5houitn474lkljew2hzdcw@z7wh2vtntvs5>
Date: Wed, 28 May 2025 09:09:51 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Claudiu Beznea <claudiu.beznea@...on.dev>, gregkh@...uxfoundation.org, 
	rafael@...nel.org, dakr@...nel.org, len.brown@...el.com, pavel@...nel.org, 
	jic23@...nel.org, daniel.lezcano@...aro.org, linux-kernel@...r.kernel.org, 
	linux-pm@...r.kernel.org, linux-iio@...r.kernel.org, linux-renesas-soc@...r.kernel.org, 
	bhelgaas@...gle.com, geert@...ux-m68k.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH v2 1/2] PM: domains: Add devres variant for
 dev_pm_domain_attach()

On Wed, May 28, 2025 at 06:04:45PM +0200, Ulf Hansson wrote:
> [...]
> 
> > >> +/**
> > >> + * devm_pm_domain_attach - devres-enabled version of dev_pm_domain_attach()
> > >> + * @dev: Device to attach.
> > >> + * @attach_power_on: Use to indicate whether we should power on the device
> > >> + *                   when attaching (true indicates the device is powered on
> > >> + *                   when attaching).
> > >> + * @detach_power_off: Used to indicate whether we should power off the device
> > >> + *                    when detaching (true indicates the device is powered off
> > >> + *                    when detaching).
> > >> + *
> > >> + * 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 attach_power_on,
> > >> +                      bool detach_power_off)
> > >
> > > Do we have examples where we power on a device and leave it powered on
> > > (or do not power on device on attach but power off it on detach)? I
> >
> > I haven't found one yet.
> >
> > > believe devm release should strictly mirror the acquisition, so separate
> > > flag is not needed.
> >
> > I was in the middle whether I should do it with 2 flags or only to revert
> > the acquisition.
> >
> > >
> > >
> > >> +{
> > >> +    int ret;
> > >> +
> > >> +    ret = dev_pm_domain_attach(dev, attach_power_on);
> > >> +    if (ret)
> > >> +            return ret;
> > >> +
> > >> +    if (detach_power_off)
> > >> +            return devm_add_action_or_reset(dev, devm_pm_domain_detach_off,
> > >> +                                            dev);
> > >> +
> > >> +    return devm_add_action_or_reset(dev, devm_pm_domain_detach_on, dev);
> > >
> > > Instead of 2 separate cleanup methods maybe define dedicated devres:
> > >
> > > struct dev_pm_domain_devres {
> > >       struct device *dev;
> > >       bool power_off;
> > > }
> > >
> > > ?
> >
> > That was the other option I've thought about but I found the one with 2
> > cleanup methods to be simpler. What would you prefer here?
> >
> > Ulf: could you please let me know what would you prefer here?
> 
> As it looks like we agreed to use one cleanup method, the struct
> dev_pm_domain_devres seems superfluous to me.

I think we agreed that cleanup should mirror the acquisition, that is
true. But since attaching to the domain has an option to either turn the
device on or not we still need 2 cleanup branches. They can either be
implemented with 2 cleanup callbacks or with 1 callback and dedicated
devres structure.

Thanks.

-- 
Dmitry