[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <w2pu2epu4ly5s3rmvuh3vcnsm5vyonxbcvz3po57dwiikfpdsd@x3uldtjpzwmr>
Date: Mon, 23 Dec 2024 00:54:54 -0500
From: Kurt Borja <kuurtb@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: platform-driver-x86@...r.kernel.org,
"Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>, Mark Pearson <mpearson-lenovo@...ebb.ca>,
Mario Limonciello <mario.limonciello@....com>, Hans de Goede <hdegoede@...hat.com>,
Gergo Koteles <soyer@....hu>, linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ACPI: platform_profile: Add
devm_platform_profile_register()
On Sun, Dec 22, 2024 at 11:09:44PM +0100, Armin Wolf wrote:
> Am 21.12.24 um 08:08 schrieb Kurt Borja:
>
> > Platform profile's lifetime is usually tied to a device's lifetime,
> > therefore add a device managed version of platform_profile_register().
> >
> > Signed-off-by: Kurt Borja <kuurtb@...il.com>
> > ---
> > drivers/acpi/platform_profile.c | 27 +++++++++++++++++++++++++++
> > include/linux/platform_profile.h | 2 +-
> > 2 files changed, 28 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> > index 75a1415190ac..d0c88decef8b 100644
> > --- a/drivers/acpi/platform_profile.c
> > +++ b/drivers/acpi/platform_profile.c
> > @@ -519,6 +519,33 @@ int platform_profile_remove(struct platform_profile_handler *pprof)
> > }
> > EXPORT_SYMBOL_GPL(platform_profile_remove);
> >
> > +static void devm_platform_profile_release(struct device *dev, void *res)
> > +{
> > + platform_profile_remove(*(struct platform_profile_handler **) res);
>
> Please introduce a local variable instead of using a convoluted cast like this.
Sure!
>
> > +}
> > +
> > +int devm_platform_profile_register(struct platform_profile_handler *pprof)
> > +{
> > + struct platform_profile_handler **dr;
> > + int ret;
> > +
> > + dr = devres_alloc(devm_platform_profile_release, sizeof(*dr), GFP_KERNEL);
> > + if (!dr)
> > + return -ENOMEM;
>
> Maybe it would make sense to turn dr into a normal pointer? AFAIK there is no benefit in
> having another pointer if we can just use the original pointer.
Please, tell me if I'm wrong. devres_alloc returns a pointer to the data
buffer of the devres struct and the handler is already allocated by the
driver, so the only way to store *pprof is to have dr be **.
>
> Thanks,
> Armin Wolf
>
> > +
> > + ret = platform_profile_register(pprof);
> > + if (ret) {
> > + devres_free(dr);
> > + return ret;
> > + }
> > +
> > + *dr = pprof;
> > + devres_add(pprof->dev, dr);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(devm_platform_profile_register);
> > +
> > static int __init platform_profile_init(void)
> > {
> > int err;
> > diff --git a/include/linux/platform_profile.h b/include/linux/platform_profile.h
> > index 0682bb4c57e5..d8114435865b 100644
> > --- a/include/linux/platform_profile.h
> > +++ b/include/linux/platform_profile.h
> > @@ -41,7 +41,7 @@ struct platform_profile_handler {
> >
> > int platform_profile_register(struct platform_profile_handler *pprof);
> > int platform_profile_remove(struct platform_profile_handler *pprof);
> > +int devm_platform_profile_register(struct platform_profile_handler *pprof);
> > int platform_profile_cycle(void);
> > void platform_profile_notify(struct platform_profile_handler *pprof);
> > -
BTW, I just saw this -. I'll remove it on v2.
> > #endif /*_PLATFORM_PROFILE_H_*/
Powered by blists - more mailing lists