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] [day] [month] [year] [list]

Message-ID: <CAPDyKFpsk-o0KvaJK+dgNDvW30piHKgvtyOxF7URaUEvrPZmZA@mail.gmail.com>
Date: Fri, 30 May 2025 11:17:42 +0200
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Dmitry Torokhov <dmitry.torokhov@...il.com>
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, 28 May 2025 at 18:09, Dmitry Torokhov <dmitry.torokhov@...il.com> wrote:
>
> 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.

Yes, you are right. Better with one callback and using struct
dev_pm_domain_devres to manage the power_off parameter.

Kind regards
Uffe