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: <4fzotopz57igmiyssgkogfbup6uu7qgza3t53t5qsouegmj7ii@wfiz4g3eiffs>
Date: Thu, 22 May 2025 11:47:29 -0700
From: Dmitry Torokhov <dmitry.torokhov@...il.com>
To: Ulf Hansson <ulf.hansson@...aro.org>
Cc: Claudiu Beznea <claudiu.beznea@...on.dev>, 
	Jonathan Cameron <jic23@...nel.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, dakr@...nel.org, linux-kernel@...r.kernel.org, 
	linux-pm@...r.kernel.org, linux-renesas-soc@...r.kernel.org, geert@...ux-m68k.org, 
	Claudiu Beznea <claudiu.beznea.uj@...renesas.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	linux-iio@...r.kernel.org, bhelgaas@...gle.com
Subject: Re: [PATCH] driver core: platform: Use devres group to free driver
 probe resources

On Thu, May 22, 2025 at 06:28:44PM +0200, Ulf Hansson wrote:
> On Thu, 22 May 2025 at 16:08, Claudiu Beznea <claudiu.beznea@...on.dev> wrote:
> >
> > Hi, Ulf,
> >
> > On 22.05.2025 14:53, Ulf Hansson wrote:
> > >
> > > That said, I think adding a devm_pm_domain_attach() interface would
> > > make perfect sense. Then we can try to replace
> > > dev_pm_domain_attach|detach() in bus level code, with just a call to
> > > devm_pm_domain_attach(). In this way, we should preserve the
> > > expectation for drivers around devres for PM domains. Even if it would
> > > change the behaviour for some drivers, it still sounds like the
> > > correct thing to do in my opinion.
> >
> > This looks good to me, as well. I did prototype it on my side and tested on
> > all my failure cases and it works.
> 
> That's great! I am happy to help review, if/when you decide to post it.

So you are saying you'd be OK with essentially the following (with
devm_pm_domain_attach() actually being elsewhere in a real patch and not
necessarily mimicked by devm_add_action_or_reset()):

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index cfccf3ff36e7..1e017bfa5caf 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -1376,6 +1376,27 @@ static int platform_uevent(const struct device *dev, struct kobj_uevent_env *env
 	return 0;
 }
 
+
+static void platform_pm_domain_detach(void *d)
+{
+	dev_pm_domain_detach(d, true);
+}
+
+static int devm_pm_domain_attach(struct device *dev)
+{
+	int error;
+
+	error = dev_pm_domain_attach(dev, true);
+	if (error)
+		return error;
+
+	error = devm_add_action_or_reset(dev, platform_pm_domain_detach, dev);
+	if (error)
+		return error;
+
+	return 0;
+}
+
 static int platform_probe(struct device *_dev)
 {
 	struct platform_driver *drv = to_platform_driver(_dev->driver);
@@ -1396,15 +1417,12 @@ static int platform_probe(struct device *_dev)
 	if (ret < 0)
 		return ret;
 
-	ret = dev_pm_domain_attach(_dev, true);
+	ret = devm_pm_domain_attach(_dev);
 	if (ret)
 		goto out;
 
-	if (drv->probe) {
+	if (drv->probe)
 		ret = drv->probe(dev);
-		if (ret)
-			dev_pm_domain_detach(_dev, true);
-	}
 
 out:
 	if (drv->prevent_deferred_probe && ret == -EPROBE_DEFER) {
@@ -1422,7 +1440,6 @@ static void platform_remove(struct device *_dev)
 
 	if (drv->remove)
 		drv->remove(dev);
-	dev_pm_domain_detach(_dev, true);
 }
 
 static void platform_shutdown(struct device *_dev)


If so, then OK, it will work for me as well. This achieves the
same behavior as with using devres group. The only difference is that if
we ever need to extend the platform bus to acquire/release more
resources they will also have to use devm API and not the regular one.

Thanks.

-- 
Dmitry

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ