Re: [PATCH v4 00/19] Hide platform_profile_handler from consumers

[Kurt Borja <kuurtb@gmail.com>]

On Thursday, January 16, 2025 11:09:49 AM Colombia Standard Time Ilpo Järvinen 
wrote:
> On Wed, 15 Jan 2025, Kurt Borja wrote:
> 
> > The merge window is about to open, so I rebased this patchset on top of
> > pdx86/review-ilpo-next to pick up acer-wmi latest commits, in case we
> > manage to squeeze this into v6.14.
> 
> Hi all,
> 
> I was hoping to finish this in this cycle since it has been so close and 
> has only required minor tweaks recently.
> 
> I went to review the changes once again and ended up applying this now 
> into the review-ilpo-next branch. Thank you all who worked on to improve 
> this series! :-)

Thank you Mario, Ilpo and Mark for reviewing this :)

> 
> There were minor nits, however, I hadto address while applying. I added 
> the justification into 5/19 commit message and changed a few trivial 
> things in the code. Please check that the diff compared with your series 
> w/o any modifications makes sense:
> 
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-platform-profile b/
Documentation/ABI/testing/sysfs-class-platform-profile
> index b5a3600080bc..dc72adfb830a 100644
> --- a/Documentation/ABI/testing/sysfs-class-platform-profile
> +++ b/Documentation/ABI/testing/sysfs-class-platform-profile
> @@ -1,12 +1,15 @@
>  What:		/sys/class/platform-profile/platform-profile-X/name
> -Date:		January 2025
> +Date:		March 2025
> +KernelVersion:	6.14
>  Description:	Name of the class device given by the driver.
>  
>  		RO
>  
>  What:		/sys/class/platform-profile/platform-profile-X/choices
> -Date:		January 2025
> -Description:	This file contains a space-separated list of profiles 
supported for this device.
> +Date:		March 2025
> +KernelVersion:	6.14
> +Description:	This file contains a space-separated list of profiles 
supported
> +		for this device.
>  
>  		Drivers must use the following standard profile-names:
>  
> @@ -26,7 +29,8 @@ Description:	This file contains a space-separated list 
of profiles supported for
>  		RO
>  
>  What:		/sys/class/platform-profile/platform-profile-X/profile
> -Date:		January 2025
> +Date:		March 2025
> +KernelVersion:	6.14
>  Description:	Reading this file gives the current selected profile 
for this
>  		device. Writing this file with one of the strings from
>  		platform_profile_choices changes the profile to the new 
value.
> diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c
> index c9e46b6e27da..fc92e43d0fe9 100644
> --- a/drivers/acpi/platform_profile.c
> +++ b/drivers/acpi/platform_profile.c
> @@ -540,7 +540,7 @@ struct device *platform_profile_register(struct device 
*dev, const char *name,
>  	pprof->dev.parent = dev;
>  	dev_set_drvdata(&pprof->dev, drvdata);
>  	dev_set_name(&pprof->dev, "platform-profile-%d", pprof->minor);
> -	/* device_register() takes ownership of ppdev */
> +	/* device_register() takes ownership of pprof/ppdev */
>  	ppdev = &no_free_ptr(pprof)->dev;
>  	err = device_register(ppdev);
>  	if (err) {
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-
wmi.c
> index 9a485f777591..1032c0e84e3d 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -3894,7 +3894,7 @@ static int platform_profile_setup(struct asus_wmi 
*asus)
>  	asus->ppdev = devm_platform_profile_register(dev, "asus-wmi", asus,
>  						     
&asus_wmi_platform_profile_ops);
>  	if (IS_ERR(asus->ppdev)) {
> -		dev_err(dev, "Failed to register a platform_profile 
class device");
> +		dev_err(dev, "Failed to register a platform_profile class 
device\n");
>  		return PTR_ERR(asus->ppdev);
>  	}
>  
> diff --git a/drivers/platform/x86/dell/dell-pc.c b/drivers/platform/x86/dell/
dell-pc.c
> index 1874d4adfb8a..483240bb36e7 100644
> --- a/drivers/platform/x86/dell/dell-pc.c
> +++ b/drivers/platform/x86/dell/dell-pc.c
> @@ -270,17 +270,16 @@ static int thermal_init(void)
>  	if (IS_ERR(platform_device))
>  		return PTR_ERR(platform_device);
>  
> -	/* Clean up if failed */
>  	ppdev = devm_platform_profile_register(&platform_device->dev, 
"dell-pc",
>  					       NULL, 
&dell_pc_platform_profile_ops);
>  	if (IS_ERR(ppdev)) {
>  		ret = PTR_ERR(ppdev);
> -		goto cleanup_thermal_handler;
> +		goto cleanup_platform_device;
>  	}
>  
>  	return 0;
>  
> -cleanup_thermal_handler:
> +cleanup_platform_device:
>  	platform_device_unregister(platform_device);
>  
>  	return ret;

Changes LGTM, thanks.

~ Kurt

> 
> 
> > 
> > ~ Kurt
> > ---
> > v3 -> v4:
> > 
> > [09/19]
> >   - Replace error message with a user-friendly one
> > 
> > v3: https://lore.kernel.org/platform-driver-x86/20250115071022.4815-1-kuurtb@gmail.com/
> > 
> > Kurt Borja (19):
> >   ACPI: platform_profile: Replace *class_dev member with class_dev
> >   ACPI: platform_profile: Let drivers set drvdata to the class device
> >   ACPI: platform_profile: Remove platform_profile_handler from callbacks
> >   ACPI: platform_profile: Add `ops` member to handlers
> >   ACPI: platform_profile: Add `probe` to platform_profile_ops
> >   platform/surface: surface_platform_profile: Use
> >     devm_platform_profile_register()
> >   platform/x86: acer-wmi: Use devm_platform_profile_register()
> >   platform/x86: amd: pmf: sps: Use devm_platform_profile_register()
> >   platform/x86: asus-wmi: Use devm_platform_profile_register()
> >   platform/x86: dell-pc: Use devm_platform_profile_register()
> >   platform/x86: ideapad-laptop: Use devm_platform_profile_register()
> >   platform/x86: hp-wmi: Use devm_platform_profile_register()
> >   platform/x86: inspur_platform_profile: Use
> >     devm_platform_profile_register()
> >   platform/x86: thinkpad_acpi: Use devm_platform_profile_register()
> >   ACPI: platform_profile: Remove platform_profile_handler from exported
> >     symbols
> >   ACPI: platform_profile: Move platform_profile_handler
> >   ACPI: platform_profile: Clean platform_profile_handler
> >   ACPI: platform_profile: Add documentation
> >   ACPI: platform_profile: Add a prefix to log messages
> > 
> >  .../ABI/testing/sysfs-class-platform-profile  |  44 +++++
> >  drivers/acpi/platform_profile.c               | 172 +++++++++++++-----
> >  .../surface/surface_platform_profile.c        |  48 ++---
> >  drivers/platform/x86/acer-wmi.c               | 114 ++++++------
> >  drivers/platform/x86/amd/pmf/core.c           |   1 -
> >  drivers/platform/x86/amd/pmf/pmf.h            |   3 +-
> >  drivers/platform/x86/amd/pmf/sps.c            |  51 +++---
> >  drivers/platform/x86/asus-wmi.c               |  55 +++---
> >  drivers/platform/x86/dell/alienware-wmi.c     |  34 ++--
> >  drivers/platform/x86/dell/dell-pc.c           |  60 +++---
> >  drivers/platform/x86/hp/hp-wmi.c              |  83 +++++----
> >  drivers/platform/x86/ideapad-laptop.c         |  45 +++--
> >  .../platform/x86/inspur_platform_profile.c    |  48 +++--
> >  drivers/platform/x86/thinkpad_acpi.c          |  37 ++--
> >  include/linux/platform_profile.h              |  37 ++--
> >  15 files changed, 495 insertions(+), 337 deletions(-)
> >  create mode 100644 Documentation/ABI/testing/sysfs-class-platform-profile
> > 
> > 
> > base-commit: d98bf6a6ed61a8047e199495b0887cce392f8e5b
> > 
> 
>