Re: [PATCH v5 5/6] platform/x86: bitland-mifs-wmi: Add per-machine ops table

[Ilpo Järvinen]

On Sun, 16 Aug 2026, Mingyou Chen wrote:

> The MIFS WMI interface is shared by several Bitland ODM laptops which
> differ in the layout of the performance mode values of
> WMI_FN_SYSTEM_PER_MODE and in the capability checks.
> 
> Introduce a machine ops table to support multiple machine families
> without sprinkling model checks over the driver:
> 
> struct bitland_mifs_ops {
>         const char *name;
>         const struct bitland_profile_mode_map *mode_map;
>         int (*check_performance_capability)(
>                 struct bitland_mifs_wmi_data *data);
>         bool has_full_speed;
> };
> 
> The default ops instance keeps the current Bitland behavior, so this
> change is a pure refactor without any functional changes.
> 
> Signed-off-by: Mingyou Chen <qby140326@xxxxxxxxx>
> ---
>  drivers/platform/x86/bitland-mifs-wmi.c | 89 +++++++++++++++++++------
>  1 file changed, 67 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/platform/x86/bitland-mifs-wmi.c b/drivers/platform/x86/bitland-mifs-wmi.c
> index 81dad2b65f4f..17792d8b7c29 100644
> --- a/drivers/platform/x86/bitland-mifs-wmi.c
> +++ b/drivers/platform/x86/bitland-mifs-wmi.c
> @@ -78,6 +78,46 @@ enum bitland_mifs_power_profile {
>  	WMI_PP_FULL_SPEED	= 3,
>  };
>  
> +struct bitland_mifs_wmi_data;
> +
> +/*
> + * Value layout of WMI_FN_SYSTEM_PER_MODE.
> + */
> +struct bitland_profile_mode_map {
> +	u8 balanced;
> +	u8 performance;
> +	u8 quiet;
> +	u8 full_speed;
> +};
> +
> +static const struct bitland_profile_mode_map bitland_mode_map = {
> +	.balanced	= WMI_PP_BALANCED,
> +	.performance	= WMI_PP_PERFORMANCE,
> +	.quiet		= WMI_PP_QUIET,
> +	.full_speed	= WMI_PP_FULL_SPEED,
> +};

Hi,

This approach seems not generic enough to cover all the variations (see 
the recent submissions).

I suggest you take a look at those patches once they get the next version 
of the generalized framework out and review it to make sure it abstract 
on the correct level so we don't have to immediately broaded the scope.

I'd prefer to get the redmi merge done separately from the generalization 
of the performance profiles. If the merge depends on the generalization 
(my impression is that it does because of the patch 6), we should try to 
finish that first and them do the merge on top of it.


Also, dropping redmi-wmi first and adding support back in a later change 
is not acceptable as it creates a black hole where an unlucky bisect 
could land to.

> +static int bitland_pp_check_capability(struct bitland_mifs_wmi_data *data);
> +
> +/*
> + * Machine-specific operations. The MIFS WMI interface is shared by
> + * several Bitland ODM laptops which differ in the layout of the
> + * performance mode values and in the capability checks.
> + */
> +struct bitland_mifs_ops {
> +	const char *name;
> +	const struct bitland_profile_mode_map *mode_map;
> +	int (*check_performance_capability)(struct bitland_mifs_wmi_data *data);
> +	bool has_full_speed;
> +};
> +
> +static const struct bitland_mifs_ops bitland_ops = {
> +	.name = "Bitland",
> +	.mode_map = &bitland_mode_map,
> +	.check_performance_capability = bitland_pp_check_capability,
> +	.has_full_speed = true,
> +};
> +
>  enum bitland_mifs_event_id {
>  	WMI_EVENT_RESERVED_1		= 1,
>  	WMI_EVENT_RESERVED_2		= 2,
> @@ -163,6 +203,7 @@ struct bitland_mifs_wmi_data {
>  	struct input_dev *input_dev;
>  	struct device *hwmon_dev;
>  	struct device *pp_dev;
> +	const struct bitland_mifs_ops *ops;
>  	enum platform_profile_option saved_profile;
>  };
>  
> @@ -193,6 +234,7 @@ static int laptop_profile_get(struct device *dev,
>  			      enum platform_profile_option *profile)
>  {
>  	struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
> +	const struct bitland_profile_mode_map *map = data->ops->mode_map;
>  	struct bitland_mifs_input input = {
>  		.reserved1 = 0,
>  		.operation = WMI_METHOD_GET,
> @@ -200,32 +242,29 @@ static int laptop_profile_get(struct device *dev,
>  		.function = WMI_FN_SYSTEM_PER_MODE,
>  	};
>  	struct bitland_mifs_output result;
> +	u8 mode;
>  	int ret;
>  
>  	ret = bitland_mifs_wmi_call(data, &input, &result);
>  	if (ret)
>  		return ret;
>  
> -	switch (result.data[0]) {
> -	case WMI_PP_BALANCED:
> +	mode = result.data[0];
> +	if (mode == map->quiet)
> +		*profile = PLATFORM_PROFILE_LOW_POWER;
> +	else if (mode == map->balanced)
>  		*profile = PLATFORM_PROFILE_BALANCED;
> -		break;
> -	case WMI_PP_PERFORMANCE:
> +	else if (mode == map->performance)
>  		*profile = PLATFORM_PROFILE_BALANCED_PERFORMANCE;
> -		break;
> -	case WMI_PP_QUIET:
> -		*profile = PLATFORM_PROFILE_LOW_POWER;
> -		break;
> -	case WMI_PP_FULL_SPEED:
> +	else if (mode == map->full_speed)
>  		*profile = PLATFORM_PROFILE_PERFORMANCE;
> -		break;
> -	default:
> +	else
>  		return -EINVAL;
> -	}
> +
>  	return 0;
>  }
>  
> -static int bitland_check_performance_capability(struct bitland_mifs_wmi_data *data)
> +static int bitland_pp_check_capability(struct bitland_mifs_wmi_data *data)
>  {
>  	struct bitland_mifs_input input = {
>  		.operation = WMI_METHOD_GET,
> @@ -252,6 +291,7 @@ static int laptop_profile_set(struct device *dev,
>  			      enum platform_profile_option profile)
>  {
>  	struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
> +	const struct bitland_profile_mode_map *map = data->ops->mode_map;
>  	struct bitland_mifs_input input = {
>  		.reserved1 = 0,
>  		.operation = WMI_METHOD_SET,
> @@ -259,42 +299,46 @@ static int laptop_profile_set(struct device *dev,
>  		.function = WMI_FN_SYSTEM_PER_MODE,
>  	};
>  	int ret;
> -	u8 val;
> +	u8 mode;
>  
>  	switch (profile) {
>  	case PLATFORM_PROFILE_LOW_POWER:
> -		val = WMI_PP_QUIET;
> +		mode = map->quiet;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED:
> -		val = WMI_PP_BALANCED;
> +		mode = map->balanced;
>  		break;
>  	case PLATFORM_PROFILE_BALANCED_PERFORMANCE:
> -		ret = bitland_check_performance_capability(data);
> +		ret = data->ops->check_performance_capability(data);
>  		if (ret)
>  			return ret;
> -		val = WMI_PP_PERFORMANCE;
> +		mode = map->performance;
>  		break;
>  	case PLATFORM_PROFILE_PERFORMANCE:
> -		ret = bitland_check_performance_capability(data);
> +		ret = data->ops->check_performance_capability(data);
>  		if (ret)
>  			return ret;
> -		val = WMI_PP_FULL_SPEED;
> +		mode = map->full_speed;
>  		break;
>  	default:
>  		return -EOPNOTSUPP;
>  	}
>  
> -	input.payload[0] = val;
> +	input.payload[0] = mode;
>  
>  	return bitland_mifs_wmi_call(data, &input, NULL);
>  }
>  
>  static int platform_profile_probe(void *drvdata, unsigned long *choices)
>  {
> +	struct bitland_mifs_wmi_data *data = drvdata;
> +
>  	set_bit(PLATFORM_PROFILE_LOW_POWER, choices);
>  	set_bit(PLATFORM_PROFILE_BALANCED, choices);
>  	set_bit(PLATFORM_PROFILE_BALANCED_PERFORMANCE, choices);
> -	set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
> +
> +	if (data->ops->has_full_speed)
> +		set_bit(PLATFORM_PROFILE_PERFORMANCE, choices);
>  
>  	return 0;
>  }
> @@ -717,6 +761,7 @@ static int bitland_mifs_wmi_probe(struct wmi_device *wdev, const void *context)
>  		return -ENOMEM;
>  
>  	drv_data->wdev = wdev;
> +	drv_data->ops = &bitland_ops;
>  
>  	ret = devm_mutex_init(&wdev->dev, &drv_data->lock);
>  	if (ret)
> 


--
 i.