Re: [RESEND PATCH 1/5] platform/x86: bitland-mifs-wmi: validate WMAA status, never abort suspend

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

[-- Attachment #1: Type: text/plain, Size: 4081 bytes --]

On Tue, 28 Jul 2026, 孙 誉铭 wrote:

> The WMAA firmware method signals "unknown/unsupported function" with
> the status word 0xe000 in the reply. The driver never checked it, so
> calls to functions a given firmware revision does not implement
> silently returned zero-filled data instead of failing.
> 
> Worse, the platform_profile suspend hook propagated such errors: on
> firmware whose perf-mode query returns values laptop_profile_get()
> cannot map (e.g. MIFS v2 firmware reporting raw QFAN codes, seen on
> Xiaomi Book Pro 14 2026 and REDMI Book Pro 14 2025), every suspend
> attempt died with:
> 
>   dpm_run_callback(): bitland_mifs_wmi_suspend returns -22
>   PM: Some devices failed to suspend, or early wake event detected
> 
> Return -EOPNOTSUPP when the firmware reports the error status word,
> and make the suspend/resume hooks fault-tolerant: skip saving and
> restoring the profile when the query fails instead of aborting the
> whole system sleep.
> 
> Status word semantics verified against the laptop's SSDT WMAA method
> (SGER=0x8000 on success, 0xe000 in the Default branch).
> 
> Signed-off-by: Yuming Sun <wolf109909@outlook.com>
> ---
>  drivers/platform/x86/bitland-mifs-wmi.c | 31 +++++++++++++++++++++----
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/platform/x86/bitland-mifs-wmi.c b/drivers/platform/x86/bitland-mifs-wmi.c
> index 3a373184..12426d11 100644
> --- a/drivers/platform/x86/bitland-mifs-wmi.c
> +++ b/drivers/platform/x86/bitland-mifs-wmi.c
> @@ -129,6 +129,18 @@ struct bitland_mifs_output {
>  	u8 data[28];
>  } __packed;
>  
> +/*
> + * The status word of a WMAA reply is formed by the first two bytes
> + * (reserved1 | operation << 8). MIFS firmware signals "unknown or
> + * unsupported function" with 0xe000 and success with 0x8000.
> + */
> +#define MIFS_STATUS_ERROR	0xe000
> +
> +static u16 mifs_status(const struct bitland_mifs_output *out)
> +{
> +	return out->reserved1 | (out->operation << 8);

How about using union inside the struct and a type that annotates the 
endianness + endianness accessor to read it here?

> +}
> +
>  struct bitland_mifs_event {
>  	u8 event_type;
>  	u8 event_id;
> @@ -159,6 +171,7 @@ struct bitland_mifs_wmi_data {
>  	struct device *hwmon_dev;
>  	struct device *pp_dev;
>  	enum platform_profile_option saved_profile;
> +	bool profile_valid;
>  };
>  
>  static int bitland_mifs_wmi_call(struct bitland_mifs_wmi_data *data,
> @@ -181,6 +194,9 @@ static int bitland_mifs_wmi_call(struct bitland_mifs_wmi_data *data,
>  	memcpy(output, out_buf.data, sizeof(*output));
>  	kfree(out_buf.data);
>  
> +	if (mifs_status(output) == MIFS_STATUS_ERROR)
> +		return -EOPNOTSUPP;
> +
>  	return 0;
>  }
>  
> @@ -298,17 +314,21 @@ static int bitland_mifs_wmi_suspend(struct device *dev)
>  {
>  	struct bitland_mifs_wmi_data *data = dev_get_drvdata(dev);
>  	enum platform_profile_option profile;
> -	int ret;
>  
>  	/* Skip event device */
>  	if (!data->pp_dev)
>  		return 0;
>  
> -	ret = laptop_profile_get(data->pp_dev, &profile);
> -	if (ret == 0)
> +	/*
> +	 * Never abort suspend: some firmware revisions answer the perf-mode
> +	 * query with values this driver cannot map, or fail the call
> +	 * entirely while the system is going down.
> +	 */
> +	data->profile_valid = laptop_profile_get(data->pp_dev, &profile) == 0;
> +	if (data->profile_valid)

This code makes no sense to me. laptop_profile_get() can return error 
codes so how come can profile be "valid" in that case?

>  		data->saved_profile = profile;
>  
> -	return ret;
> +	return 0;
>  }
>  
>  static int bitland_mifs_wmi_resume(struct device *dev)
> @@ -319,6 +339,9 @@ static int bitland_mifs_wmi_resume(struct device *dev)
>  	if (!data->pp_dev)
>  		return 0;
>  
> +	if (!data->profile_valid)
> +		return 0;
> +
>  	dev_dbg(dev, "Resuming, restoring profile %d\n", data->saved_profile);
>  	return laptop_profile_set(dev, data->saved_profile);
>  }
> 

-- 
 i.