[PATCH] platform/x86: hp-wmi: Fix implementation of the platform_profile_omen_get function

[PATCH] platform/x86: hp-wmi: Fix implementation of the platform_profile_omen_get function

[Alexis Belmonte]

Fix ill-formed implementation of the platform_profile_omen_get function
introduced by patch "platform/x86: hp-wmi: Fix platform profile option
switch"

Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
---
 drivers/platform/x86/hp/hp-wmi.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
index c8bcb3e2b344..876e0a97cee1 100644
--- a/drivers/platform/x86/hp/hp-wmi.c
+++ b/drivers/platform/x86/hp/hp-wmi.c
@@ -1238,8 +1238,6 @@ static int platform_profile_omen_get_ec(enum platform_profile_option *profile)
 static int platform_profile_omen_get(struct platform_profile_handler *pprof,
 				     enum platform_profile_option *profile)
 {
-	enum platform_profile_option selected_platform_profile;
-
 	/*
 	 * We directly return the stored platform profile, as the embedded
 	 * controller will not accept switching to the performance option when
@@ -1253,9 +1251,9 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
 	 * See also omen_powersource_event.
 	 */
 	guard(mutex)(&active_platform_profile_lock);
-	selected_platform_profile = active_platform_profile;
+	*profile = active_platform_profile;
 
-	return selected_platform_profile;
+	return 0;
 }
 
 static bool has_omen_thermal_profile_ec_timer(void)
-- 
2.45.2

Re: [PATCH] platform/x86: hp-wmi: Fix implementation of the platform_profile_omen_get function

[Armin Wolf]

Am 12.07.24 um 19:26 schrieb Alexis Belmonte:

> Fix ill-formed implementation of the platform_profile_omen_get function
> introduced by patch "platform/x86: hp-wmi: Fix platform profile option
> switch"
>
> Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>
> ---
>   drivers/platform/x86/hp/hp-wmi.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index c8bcb3e2b344..876e0a97cee1 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -1238,8 +1238,6 @@ static int platform_profile_omen_get_ec(enum platform_profile_option *profile)
>   static int platform_profile_omen_get(struct platform_profile_handler *pprof,
>   				     enum platform_profile_option *profile)
>   {
> -	enum platform_profile_option selected_platform_profile;
> -
>   	/*
>   	 * We directly return the stored platform profile, as the embedded
>   	 * controller will not accept switching to the performance option when
> @@ -1253,9 +1251,9 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
>   	 * See also omen_powersource_event.
>   	 */
>   	guard(mutex)(&active_platform_profile_lock);
> -	selected_platform_profile = active_platform_profile;
> +	*profile = active_platform_profile;
>
> -	return selected_platform_profile;
> +	return 0;
>   }
>
>   static bool has_omen_thermal_profile_ec_timer(void)

Looks good to me.

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

Re: [PATCH] platform/x86: hp-wmi: Fix implementation of the platform_profile_omen_get function

[Ilpo Järvinen]

On Fri, 12 Jul 2024, Alexis Belmonte wrote:

> Fix ill-formed implementation of the platform_profile_omen_get function
> introduced by patch "platform/x86: hp-wmi: Fix platform profile option
> switch"
> 
> Signed-off-by: Alexis Belmonte <alexbelm48@gmail.com>

I took this patch but there were a number of issues with it I had to fix:

- Reported-by tag missing
- Fixes tag missing
- You didn't tell in the commit message what was wrong.

-- 
 i.

> ---
>  drivers/platform/x86/hp/hp-wmi.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index c8bcb3e2b344..876e0a97cee1 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -1238,8 +1238,6 @@ static int platform_profile_omen_get_ec(enum platform_profile_option *profile)
>  static int platform_profile_omen_get(struct platform_profile_handler *pprof,
>  				     enum platform_profile_option *profile)
>  {
> -	enum platform_profile_option selected_platform_profile;
> -
>  	/*
>  	 * We directly return the stored platform profile, as the embedded
>  	 * controller will not accept switching to the performance option when
> @@ -1253,9 +1251,9 @@ static int platform_profile_omen_get(struct platform_profile_handler *pprof,
>  	 * See also omen_powersource_event.
>  	 */
>  	guard(mutex)(&active_platform_profile_lock);
> -	selected_platform_profile = active_platform_profile;
> +	*profile = active_platform_profile;
>  
> -	return selected_platform_profile;
> +	return 0;
>  }
>  
>  static bool has_omen_thermal_profile_ec_timer(void)
> 

Re: [PATCH] platform/x86: hp-wmi: Fix implementation of the platform_profile_omen_get function

[Alexis Belmonte]

Hi Ilpo,

On Mon, Jul 15, 2024 at 12:39:22PM +0300, Ilpo Järvinen wrote:
> 
> I took this patch but there were a number of issues with it I had to fix:
> 

Thanks a lot for your attention to detail. As you can probably tell,
it's my first time doing this kind of bug-fixing submission, so I apologize for
that. I'll try to be more careful in the future. :]

Don't hesitate to let me do the fixes myself next time -- you certainly have
better things to do than fixing this kind of stuff, and this will help me
remember how things are supposed to be done. :]

Still, I'm very thankful for your help.

Hope you have a great afternoon!

Alexis