Re: [PATCH v4 1/4] platform/x86: hp-wmi: Introduce board-specific feature data

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Wed, 20 May 2026, Radhey Kalra wrote:

> The victus_s_thermal_profile_boards DMI table is about to carry more than
> thermal-profile data. Replace the direct thermal_profile_params
> .driver_data pointers with hp_wmi_board_params and rename the table/setup
> helper accordingly.
> 
> No functional changes intended.
> 
> Signed-off-by: Radhey Kalra <radheykalra901@gmail.com>
> ---
>  drivers/platform/x86/hp/hp-wmi.c | 82 ++++++++++++++++++--------------
>  1 file changed, 47 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp/hp-wmi.c b/drivers/platform/x86/hp/hp-wmi.c
> index d1cc6e7..b226695 100644
> --- a/drivers/platform/x86/hp/hp-wmi.c
> +++ b/drivers/platform/x86/hp/hp-wmi.c
> @@ -133,11 +133,29 @@ static const struct thermal_profile_params omen_v1_no_ec_thermal_params = {
>  	.ec_tp_offset	= HP_NO_THERMAL_PROFILE_OFFSET,
>  };
>  
> -/*
> - * A generic pointer for the currently-active board's thermal profile
> - * parameters.
> - */
> -static struct thermal_profile_params *active_thermal_profile_params;
> +static const struct thermal_profile_params *active_thermal_profile_params;
> +
> +struct hp_wmi_board_params {
> +	const struct thermal_profile_params *thermal_profile;
> +};
> +
> +static const struct hp_wmi_board_params victus_s_board_params = {
> +	.thermal_profile	= &victus_s_thermal_params,
> +};
> +
> +static const struct hp_wmi_board_params omen_v1_board_params = {
> +	.thermal_profile	= &omen_v1_thermal_params,
> +};
> +
> +static const struct hp_wmi_board_params omen_v1_legacy_board_params = {
> +	.thermal_profile	= &omen_v1_legacy_thermal_params,
> +};
> +
> +static const struct hp_wmi_board_params omen_v1_no_ec_board_params = {
> +	.thermal_profile	= &omen_v1_no_ec_thermal_params,
> +};
> +
> +static const struct hp_wmi_board_params *active_board_params;
>  
>  /* DMI board names of devices that should use the omen specific path for
>   * thermal profiles.
> @@ -187,67 +205,67 @@ static const char * const victus_thermal_profile_boards[] = {
>  	"8A25",
>  };
>  
> -/* DMI Board names of Victus 16-r and Victus 16-s laptops */
> -static const struct dmi_system_id victus_s_thermal_profile_boards[] __initconst = {
> +/* DMI board-specific feature data for Omen and Victus laptops. */
> +static const struct dmi_system_id hp_wmi_feature_boards[] __initconst = {
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8A44") },
> -		.driver_data = (void *)&omen_v1_legacy_thermal_params,
> +		.driver_data = (void *)&omen_v1_legacy_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8A4D") },
> -		.driver_data = (void *)&omen_v1_legacy_thermal_params,
> +		.driver_data = (void *)&omen_v1_legacy_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BAB") },
> -		.driver_data = (void *)&omen_v1_thermal_params,
> +		.driver_data = (void *)&omen_v1_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BBE") },
> -		.driver_data = (void *)&victus_s_thermal_params,
> +		.driver_data = (void *)&victus_s_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BCA") },
> -		.driver_data = (void *)&omen_v1_thermal_params,
> +		.driver_data = (void *)&omen_v1_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BCD") },
> -		.driver_data = (void *)&omen_v1_thermal_params,
> +		.driver_data = (void *)&omen_v1_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD4") },
> -		.driver_data = (void *)&victus_s_thermal_params,
> +		.driver_data = (void *)&victus_s_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8BD5") },
> -		.driver_data = (void *)&victus_s_thermal_params,
> +		.driver_data = (void *)&victus_s_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C76") },
> -		.driver_data = (void *)&omen_v1_thermal_params,
> +		.driver_data = (void *)&omen_v1_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C77") },
> -		.driver_data = (void *)&omen_v1_thermal_params,
> +		.driver_data = (void *)&omen_v1_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C78") },
> -		.driver_data = (void *)&omen_v1_thermal_params,
> +		.driver_data = (void *)&omen_v1_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C99") },
> -		.driver_data = (void *)&victus_s_thermal_params,
> +		.driver_data = (void *)&victus_s_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8C9C") },
> -		.driver_data = (void *)&victus_s_thermal_params,
> +		.driver_data = (void *)&victus_s_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8D41") },
> -		.driver_data = (void *)&victus_s_thermal_params,
> +		.driver_data = (void *)&victus_s_board_params,
>  	},
>  	{
>  		.matches = { DMI_MATCH(DMI_BOARD_NAME, "8D87") },
> -		.driver_data = (void *)&omen_v1_no_ec_thermal_params,
> +		.driver_data = (void *)&omen_v1_no_ec_board_params,
>  	},
>  	{},
>  };
> @@ -1904,7 +1922,7 @@ static int platform_profile_victus_s_get_ec(enum platform_profile_option *profil
>  
>  static int platform_profile_victus_s_set_ec(enum platform_profile_option profile)
>  {
> -	struct thermal_profile_params *params;
> +	const struct thermal_profile_params *params;
>  	bool gpu_ctgp_enable, gpu_ppab_enable;
>  	u8 gpu_dstate; /* Test shows 1 = 100%, 2 = 50%, 3 = 25%, 4 = 12.5% */
>  	int err, tp;
> @@ -2673,23 +2691,20 @@ static int hp_wmi_hwmon_init(void)
>  	return 0;
>  }
>  
> -static void __init setup_active_thermal_profile_params(void)
> +static void __init setup_active_board_params(void)
>  {
>  	const struct dmi_system_id *id;
>  
> -	/*
> -	 * Currently only victus_s devices use the
> -	 * active_thermal_profile_params
> -	 */
> -	id = dmi_first_match(victus_s_thermal_profile_boards);
> +	id = dmi_first_match(hp_wmi_feature_boards);
>  	if (id) {
> +		active_board_params = id->driver_data;
> +		active_thermal_profile_params = active_board_params->thermal_profile;

Is the second variable necessary anymore? Can't we always deref it from
active_board_params?

Using drvdata, however, would be generally preferred over globals but 
that's a pre-existing problem in this driver (as with many other 
platform drivers).

>  		/*
>  		 * Marking this boolean is required to ensure that
>  		 * is_victus_s_thermal_profile() behaves like a valid
>  		 * wrapper.
>  		 */
>  		is_victus_s_board = true;
> -		active_thermal_profile_params = id->driver_data;
>  		if (active_thermal_profile_params->ec_tp_offset == HP_EC_OFFSET_UNKNOWN) {
>  			pr_warn("Unknown EC layout for board %s. Thermal profile readback will be disabled. Please report this to platform-driver-x86@vger.kernel.org\n",
>  				dmi_get_system_info(DMI_BOARD_NAME));
> @@ -2724,11 +2739,8 @@ static int __init hp_wmi_init(void)
>  			goto err_destroy_input;
>  		}
>  
> -		/*
> -		 * Setup active board's thermal profile parameters before
> -		 * starting platform driver probe.
> -		 */
> -		setup_active_thermal_profile_params();
> +		/* Setup active board feature data before starting platform driver probe. */

Keep comments limited to <= 80 cols (long lines are hard to read).

This only applies to comments, the for code the limit is 100 columns where 
it helps to make the code cleaner.

> +		setup_active_board_params();
>  		err = platform_driver_probe(&hp_wmi_driver, hp_wmi_bios_setup);
>  		if (err)
>  			goto err_unregister_device;
> 

-- 
 i.