[PATCH v2 0/2] Device managed platform_profile_register()

[PATCH v2 0/2] Device managed platform_profile_register()

[Kurt Borja]

Hi :)

This is meant to be merged on the pdx86 tree.

~ Kurt

v2:
 - Replaced convoluted cast with intermediate variable (1/2)
 - Restored dropped empty line (1/2)
 - Couldn't incorporate Armin's second comment. I probably didn't
   understand it (1/2)
v1: 
 - https://lore.kernel.org/platform-driver-x86/20241221070817.3764-2-kuurtb@gmail.com

Kurt Borja (2):
  ACPI: platform_profile: Add devm_platform_profile_register()
  alienware-wmi: Use devm_platform_profile_register()

 drivers/acpi/platform_profile.c           | 29 +++++++++++++++++++++++
 drivers/platform/x86/dell/alienware-wmi.c | 10 +-------
 include/linux/platform_profile.h          |  1 +
 3 files changed, 31 insertions(+), 9 deletions(-)


base-commit: 03f8e0e05510dad6377cd5ef029594d30e6c096d
-- 
2.47.1

[PATCH v2 1/2] ACPI: platform_profile: Add devm_platform_profile_register()

[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@gmail.com>
---
 drivers/acpi/platform_profile.c  | 29 +++++++++++++++++++++++++++++
 include/linux/platform_profile.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
index 75a1415190ac..4c4200a0b1a6 100644
--- a/drivers/acpi/platform_profile.c
+++ b/drivers/acpi/platform_profile.c
@@ -519,6 +519,35 @@ 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)
+{
+	struct platform_profile_handler **pprof = res;
+
+	platform_profile_remove(*pprof);
+}
+
+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;
+
+	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..f1cd4b65e351 100644
--- a/include/linux/platform_profile.h
+++ b/include/linux/platform_profile.h
@@ -41,6 +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);
 
-- 
2.47.1

Re: [PATCH v2 1/2] ACPI: platform_profile: Add devm_platform_profile_register()

[Rafael J. Wysocki]

On Tue, Dec 24, 2024 at 3:02 PM Kurt Borja <kuurtb@gmail.com> wrote:
>
> 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@gmail.com>

This is fine by me, so

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

and I think that it would be better to route it via platform/x86 along
with the second patch.

> ---
>  drivers/acpi/platform_profile.c  | 29 +++++++++++++++++++++++++++++
>  include/linux/platform_profile.h |  1 +
>  2 files changed, 30 insertions(+)
>
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index 75a1415190ac..4c4200a0b1a6 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -519,6 +519,35 @@ 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)
> +{
> +       struct platform_profile_handler **pprof = res;
> +
> +       platform_profile_remove(*pprof);
> +}
> +
> +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;
> +
> +       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..f1cd4b65e351 100644
> --- a/include/linux/platform_profile.h
> +++ b/include/linux/platform_profile.h
> @@ -41,6 +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);
>
> --
> 2.47.1
>

Re: [PATCH v2 1/2] ACPI: platform_profile: Add devm_platform_profile_register()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2994 bytes --]

On Tue, 14 Jan 2025, Rafael J. Wysocki wrote:

> On Tue, Dec 24, 2024 at 3:02 PM Kurt Borja <kuurtb@gmail.com> wrote:
> >
> > 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@gmail.com>
> 
> This is fine by me, so
> 
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> and I think that it would be better to route it via platform/x86 along
> with the second patch.

That's were it already is among the other platform profile changes.

We still have one big platform profile series pending (though Kurt needs 
to do small tweaks still into it before it can be applied):

https://lore.kernel.org/all/20250109150731.110799-1-kuurtb@gmail.com/

-- 
 i.

> > ---
> >  drivers/acpi/platform_profile.c  | 29 +++++++++++++++++++++++++++++
> >  include/linux/platform_profile.h |  1 +
> >  2 files changed, 30 insertions(+)
> >
> > diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> > index 75a1415190ac..4c4200a0b1a6 100644
> > --- a/drivers/acpi/platform_profile.c
> > +++ b/drivers/acpi/platform_profile.c
> > @@ -519,6 +519,35 @@ 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)
> > +{
> > +       struct platform_profile_handler **pprof = res;
> > +
> > +       platform_profile_remove(*pprof);
> > +}
> > +
> > +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;
> > +
> > +       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..f1cd4b65e351 100644
> > --- a/include/linux/platform_profile.h
> > +++ b/include/linux/platform_profile.h
> > @@ -41,6 +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);
> >
> > --
> > 2.47.1
> >
> 

[PATCH v2 2/2] alienware-wmi: Use devm_platform_profile_register()

[Kurt Borja]

Replace platform_profile_register() with it's device managed version.
Drop remove_thermal_profile() because it's no longer needed.

Reviewed-by: Armin Wolf <W_Armin@gmx.de>
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/alienware-wmi.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
index e95d22c7b60c..7b3ee2d6a23d 100644
--- a/drivers/platform/x86/dell/alienware-wmi.c
+++ b/drivers/platform/x86/dell/alienware-wmi.c
@@ -1159,13 +1159,7 @@ static int create_thermal_profile(struct platform_device *platform_device)
 	pp_handler.name = "alienware-wmi";
 	pp_handler.dev = &platform_device->dev;
 
-	return platform_profile_register(&pp_handler);
-}
-
-static void remove_thermal_profile(void)
-{
-	if (quirks->thermal)
-		platform_profile_remove(&pp_handler);
+	return devm_platform_profile_register(&pp_handler);
 }
 
 static int __init alienware_wmi_init(void)
@@ -1239,7 +1233,6 @@ static int __init alienware_wmi_init(void)
 
 fail_prep_zones:
 	alienware_zone_exit(platform_device);
-	remove_thermal_profile();
 fail_prep_thermal_profile:
 fail_prep_deepsleep:
 fail_prep_amplifier:
@@ -1260,7 +1253,6 @@ static void __exit alienware_wmi_exit(void)
 	if (platform_device) {
 		alienware_zone_exit(platform_device);
 		remove_hdmi(platform_device);
-		remove_thermal_profile();
 		platform_device_unregister(platform_device);
 		platform_driver_unregister(&platform_driver);
 	}
-- 
2.47.1

Re: [PATCH v2 0/2] Device managed platform_profile_register()

[Armin Wolf]

Am 24.12.24 um 15:01 schrieb Kurt Borja:

> Hi :)
>
> This is meant to be merged on the pdx86 tree.
>
> ~ Kurt

After thinking about it my second commit regarding the first patch is invalid, so for the
whole series:

Reviewed-by: Armin Wolf <W_Armin@gmx.de>

> v2:
>   - Replaced convoluted cast with intermediate variable (1/2)
>   - Restored dropped empty line (1/2)
>   - Couldn't incorporate Armin's second comment. I probably didn't
>     understand it (1/2)
> v1:
>   - https://lore.kernel.org/platform-driver-x86/20241221070817.3764-2-kuurtb@gmail.com
>
> Kurt Borja (2):
>    ACPI: platform_profile: Add devm_platform_profile_register()
>    alienware-wmi: Use devm_platform_profile_register()
>
>   drivers/acpi/platform_profile.c           | 29 +++++++++++++++++++++++
>   drivers/platform/x86/dell/alienware-wmi.c | 10 +-------
>   include/linux/platform_profile.h          |  1 +
>   3 files changed, 31 insertions(+), 9 deletions(-)
>
>
> base-commit: 03f8e0e05510dad6377cd5ef029594d30e6c096d

Re: [PATCH v2 0/2] Device managed platform_profile_register()

[Ilpo Järvinen]

On Tue, 24 Dec 2024, Kurt Borja wrote:

> Hi :)
> 
> This is meant to be merged on the pdx86 tree.
> 
> ~ Kurt
> 
> v2:
>  - Replaced convoluted cast with intermediate variable (1/2)
>  - Restored dropped empty line (1/2)
>  - Couldn't incorporate Armin's second comment. I probably didn't
>    understand it (1/2)
> v1: 
>  - https://lore.kernel.org/platform-driver-x86/20241221070817.3764-2-kuurtb@gmail.com
> 
> Kurt Borja (2):
>   ACPI: platform_profile: Add devm_platform_profile_register()
>   alienware-wmi: Use devm_platform_profile_register()
> 
>  drivers/acpi/platform_profile.c           | 29 +++++++++++++++++++++++
>  drivers/platform/x86/dell/alienware-wmi.c | 10 +-------
>  include/linux/platform_profile.h          |  1 +
>  3 files changed, 31 insertions(+), 9 deletions(-)

Thanks. I've now applied these.

The first patch is already in the for-next branch and the second is 
currently in the review-ilpo-next branch (as I wanted to retain ability to 
easily separate changes into platform_profile.c from the rest, they go to 
their own branch first).

-- 
 i.

Re: [PATCH v2 0/2] Device managed platform_profile_register()

[Kurt Borja]

On Mon, Dec 30, 2024 at 08:31:28PM +0200, Ilpo Järvinen wrote:
> On Tue, 24 Dec 2024, Kurt Borja wrote:
> 
> > Hi :)
> > 
> > This is meant to be merged on the pdx86 tree.
> > 
> > ~ Kurt
> > 
> > v2:
> >  - Replaced convoluted cast with intermediate variable (1/2)
> >  - Restored dropped empty line (1/2)
> >  - Couldn't incorporate Armin's second comment. I probably didn't
> >    understand it (1/2)
> > v1: 
> >  - https://lore.kernel.org/platform-driver-x86/20241221070817.3764-2-kuurtb@gmail.com
> > 
> > Kurt Borja (2):
> >   ACPI: platform_profile: Add devm_platform_profile_register()
> >   alienware-wmi: Use devm_platform_profile_register()
> > 
> >  drivers/acpi/platform_profile.c           | 29 +++++++++++++++++++++++
> >  drivers/platform/x86/dell/alienware-wmi.c | 10 +-------
> >  include/linux/platform_profile.h          |  1 +
> >  3 files changed, 31 insertions(+), 9 deletions(-)
> 
> Thanks. I've now applied these.
> 
> The first patch is already in the for-next branch and the second is 
> currently in the review-ilpo-next branch (as I wanted to retain ability to 
> easily separate changes into platform_profile.c from the rest, they go to 
> their own branch first).

Thanks Ilpo!

Should I rebase the alienware-wmi rework patch series on top of
review-ilpo-next in v3? Currently it's on top of for-next branch.

~ Kurt

> 
> -- 
>  i.
> 

Re: [PATCH v2 0/2] Device managed platform_profile_register()

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2238 bytes --]

On Tue, 31 Dec 2024, Kurt Borja wrote:
> On Mon, Dec 30, 2024 at 08:31:28PM +0200, Ilpo Järvinen wrote:
> > On Tue, 24 Dec 2024, Kurt Borja wrote:
> > 
> > > Hi :)
> > > 
> > > This is meant to be merged on the pdx86 tree.
> > > 
> > > ~ Kurt
> > > 
> > > v2:
> > >  - Replaced convoluted cast with intermediate variable (1/2)
> > >  - Restored dropped empty line (1/2)
> > >  - Couldn't incorporate Armin's second comment. I probably didn't
> > >    understand it (1/2)
> > > v1: 
> > >  - https://lore.kernel.org/platform-driver-x86/20241221070817.3764-2-kuurtb@gmail.com
> > > 
> > > Kurt Borja (2):
> > >   ACPI: platform_profile: Add devm_platform_profile_register()
> > >   alienware-wmi: Use devm_platform_profile_register()
> > > 
> > >  drivers/acpi/platform_profile.c           | 29 +++++++++++++++++++++++
> > >  drivers/platform/x86/dell/alienware-wmi.c | 10 +-------
> > >  include/linux/platform_profile.h          |  1 +
> > >  3 files changed, 31 insertions(+), 9 deletions(-)
> > 
> > Thanks. I've now applied these.
> > 
> > The first patch is already in the for-next branch and the second is 
> > currently in the review-ilpo-next branch (as I wanted to retain ability to 
> > easily separate changes into platform_profile.c from the rest, they go to 
> > their own branch first).
> 
> Thanks Ilpo!
> 
> Should I rebase the alienware-wmi rework patch series on top of
> review-ilpo-next in v3? Currently it's on top of for-next branch.

Hi Kurt,

You've probably seen it by now but in general, the content in those 
review-ilpo-* branches is just a staging area to what will become the 
for-next or fixes branch once LKP has been able to build test the changes. 
If no issues appear, they'll become content of the for-next or fixes 
branch as is.

In case of problems, I will rebase/edit/drop patches in the 
review-ilpo-* branches with a relatively low bar so it might not always 
be fast-forwardable but other than that minor annoyance, I see no issue in 
basing your work on top of those branches (in particular, if you know 
there are going to be conflicts).

I might rebase also fixes and for-next at times, but I try to avoid 
having to do that.

-- 
 i.