[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <95e34926-4f06-4443-9798-9089f6073786@gmx.de>
Date: Fri, 27 Dec 2024 04:26:05 +0100
From: Armin Wolf <W_Armin@....de>
To: Kurt Borja <kuurtb@...il.com>, platform-driver-x86@...r.kernel.org
Cc: 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
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.
> +
> 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.
> +
> 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.
With those minor issues being fixed:
Reviewed-by: Armin Wolf <W_Armin@....de>
> +}
> +
> +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