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: <i2b2hkfpwkwa6hla2f5ezrlu7ujbvkdfjxz2tuzajbhsyeoumy@yb3iq7m5du3k>
Date: Thu, 26 Dec 2024 23:50:21 -0500
From: Kurt Borja <kuurtb@...il.com>
To: Armin Wolf <W_Armin@....de>
Cc: platform-driver-x86@...r.kernel.org, ilpo.jarvinen@...ux.intel.com, 
	mario.limonciello@....com, hdegoede@...hat.com, linux-kernel@...r.kernel.org, 
	Dell.Client.Kernel@...l.com
Subject: Re: [PATCH 09/20] alienware-wmi: Add a state container for thermal
 control methods

On Fri, Dec 27, 2024 at 04:26:05AM +0100, Armin Wolf wrote:
> Am 21.12.24 um 06:59 schrieb Kurt Borja:
> 
> > Refactor all thermal control methods to use the newly defined awcc_priv
> > state container instead of global variables.
> > 
> > Signed-off-by: Kurt Borja <kuurtb@...il.com>
> > ---
> >   drivers/platform/x86/dell/alienware-wmi.c | 71 +++++++++++++++++------
> >   1 file changed, 52 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/dell/alienware-wmi.c b/drivers/platform/x86/dell/alienware-wmi.c
> > index f2f6842e27e6..c4ca141d628e 100644
> > --- a/drivers/platform/x86/dell/alienware-wmi.c
> > +++ b/drivers/platform/x86/dell/alienware-wmi.c
> > @@ -403,6 +403,12 @@ struct wmax_u32_args {
> >   	u8 arg3;
> >   };
> > 
> > +struct awcc_priv {
> > +	struct wmi_device *wdev;
> > +	struct platform_profile_handler pp_handler;
> > +	enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
> > +};
> > +
> >   struct alienfx_priv {
> >   	struct platform_device *pdev;
> >   	struct led_classdev global_led;
> > @@ -415,9 +421,6 @@ struct alienfx_platdata {
> >   	struct wmi_device *wdev;
> >   };
> > 
> > -static struct platform_profile_handler pp_handler;
> > -static enum wmax_thermal_mode supported_thermal_profiles[PLATFORM_PROFILE_LAST];
> > -
> >   static u8 interface;
> > 
> >   /*
> > @@ -1027,6 +1030,10 @@ static int thermal_profile_get(struct platform_profile_handler *pprof,
> >   static int thermal_profile_set(struct platform_profile_handler *pprof,
> >   			       enum platform_profile_option profile)
> >   {
> > +	struct awcc_priv *priv;
> > +
> > +	priv = container_of(pprof, struct awcc_priv, pp_handler);
> 
> Please merge those two statements.

Ack.

> 
> > +
> >   	if (quirks->gmode) {
> >   		u32 gmode_status;
> >   		int ret;
> > @@ -1047,18 +1054,21 @@ static int thermal_profile_set(struct platform_profile_handler *pprof,
> >   		}
> >   	}
> > 
> > -	return wmax_thermal_control(supported_thermal_profiles[profile]);
> > +	return wmax_thermal_control(priv->supported_thermal_profiles[profile]);
> >   }
> > 
> >   static int create_thermal_profile(struct wmi_device *wdev)
> >   {
> >   	enum platform_profile_option profile;
> >   	enum wmax_thermal_mode mode;
> > +	struct awcc_priv *priv;
> >   	u8 sys_desc[4];
> >   	u32 first_mode;
> >   	u32 out_data;
> >   	int ret;
> > 
> > +	priv = dev_get_drvdata(&wdev->dev);
> 
> Same as above.

Ack.

> 
> > +
> >   	ret = wmax_thermal_information(WMAX_OPERATION_SYS_DESCRIPTION,
> >   				       0, (u32 *) &sys_desc);
> >   	if (ret < 0)
> > @@ -1081,33 +1091,56 @@ static int create_thermal_profile(struct wmi_device *wdev)
> > 
> >   		mode = out_data & WMAX_THERMAL_MODE_MASK;
> >   		profile = wmax_mode_to_platform_profile[mode];
> > -		supported_thermal_profiles[profile] = out_data;
> > +		priv->supported_thermal_profiles[profile] = out_data;
> > 
> > -		set_bit(profile, pp_handler.choices);
> > +		set_bit(profile, priv->pp_handler.choices);
> >   	}
> > 
> > -	if (bitmap_empty(pp_handler.choices, PLATFORM_PROFILE_LAST))
> > +	if (bitmap_empty(priv->pp_handler.choices, PLATFORM_PROFILE_LAST))
> >   		return -ENODEV;
> > 
> >   	if (quirks->gmode) {
> > -		supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
> > +		priv->supported_thermal_profiles[PLATFORM_PROFILE_PERFORMANCE] =
> >   			WMAX_THERMAL_MODE_GMODE;
> > 
> > -		set_bit(PLATFORM_PROFILE_PERFORMANCE, pp_handler.choices);
> > +		set_bit(PLATFORM_PROFILE_PERFORMANCE, priv->pp_handler.choices);
> >   	}
> > 
> > -	pp_handler.profile_get = thermal_profile_get;
> > -	pp_handler.profile_set = thermal_profile_set;
> > -	pp_handler.name = "alienware-wmi";
> > -	pp_handler.dev = &wdev->dev;
> > +	priv->pp_handler.profile_get = thermal_profile_get;
> > +	priv->pp_handler.profile_set = thermal_profile_set;
> > +	priv->pp_handler.name = "alienware-wmi";
> > +	priv->pp_handler.dev = &wdev->dev;
> > 
> > -	return platform_profile_register(&pp_handler);
> > +	return platform_profile_register(&priv->pp_handler);
> >   }
> > 
> > -static void remove_thermal_profile(void)
> > +static int alienware_awcc_setup(struct wmi_device *wdev)
> >   {
> > -	if (quirks->thermal)
> > -		platform_profile_remove(&pp_handler);
> > +	struct awcc_priv *priv;
> > +	int ret;
> > +
> > +	priv = devm_kzalloc(&wdev->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return -ENOMEM;
> > +
> > +	dev_set_drvdata(&wdev->dev, priv);
> > +
> > +	priv->wdev = wdev;
> > +
> > +	ret = create_thermal_profile(wdev);
> > +	if (ret < 0)
> > +		return ret;
> > +
> > +	return 0;
> 
> Just directly return the result of create_thermal_profile() here.

Ack.

> 
> With those minor issues being fixed:
> 
> Reviewed-by: Armin Wolf <W_Armin@....de>

Thanks!

> 
> > +}
> > +
> > +static void alienware_awcc_exit(struct wmi_device *wdev)
> > +{
> > +	struct awcc_priv *priv;
> > +
> > +	priv = dev_get_drvdata(&wdev->dev);
> 
> Please merge both statements.
> 
> > +
> > +	platform_profile_remove(&priv->pp_handler);
> >   }
> > 
> >   /*
> > @@ -1242,7 +1275,7 @@ static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
> >   	int ret = 0;
> > 
> >   	if (quirks->thermal)
> > -		ret = create_thermal_profile(wdev);
> > +		ret = alienware_awcc_setup(wdev);
> >   	else
> >   		ret = alienware_alienfx_setup(&pdata);
> > 
> > @@ -1252,7 +1285,7 @@ static int wmax_wmi_probe(struct wmi_device *wdev, const void *context)
> >   static void wmax_wmi_remove(struct wmi_device *wdev)
> >   {
> >   	if (quirks->thermal)
> > -		remove_thermal_profile();
> > +		alienware_awcc_exit(wdev);
> >   	else
> >   		alienware_alienfx_exit(wdev);
> >   }

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ