diff mbox series

[v2,2/3] Fix 0x05 error code reported by several WMI calls

Message ID 20220218160907.3422-3-jorge.lopez2@hp.com (mailing list archive)
State Changes Requested, archived
Headers show
Series Fix SW_TABLET_MODE detection method | expand

Commit Message

Jorge Lopez Feb. 18, 2022, 4:09 p.m. UTC 
Several WMI queries leverage hp_wmi_read_int function to read their data.
hp_wmi_read_int function returns the appropiate value if the WMI command
requires an input and output buffer size values greater than zero.
WMI queries such HPWMI_HARDWARE_QUERY, HPWMI_WIRELESS2_QUERY,
HPWMI_FEATURE2_QUERY, HPWMI_DISPLAY_QUERY, HPWMI_ALS_QUERY, and
HPWMI_POSTCODEERROR_QUERY requires calling hp_wmi_perform_query function
with input buffer size value of zero.  Any input buffer size greater
than zero will cause error 0x05 to be returned.

All changes were validated on a ZBook Workstation notebook. Additional
validation was included to ensure no other commands were failing or
incorrectly handled.

Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>

---
Based on the latest platform-drivers-x86.git/for-next
---
 drivers/platform/x86/hp-wmi.c | 64 ++++++++++++++++++++++++-----------
 1 file changed, 44 insertions(+), 20 deletions(-)

Comments

Hans de Goede Feb. 22, 2022, 4:22 p.m. UTC | #1 
Hi,

On 2/18/22 17:09, Jorge Lopez wrote:
> Several WMI queries leverage hp_wmi_read_int function to read their data.
> hp_wmi_read_int function returns the appropiate value if the WMI command
> requires an input and output buffer size values greater than zero.
> WMI queries such HPWMI_HARDWARE_QUERY, HPWMI_WIRELESS2_QUERY,
> HPWMI_FEATURE2_QUERY, HPWMI_DISPLAY_QUERY, HPWMI_ALS_QUERY, and
> HPWMI_POSTCODEERROR_QUERY requires calling hp_wmi_perform_query function
> with input buffer size value of zero.  Any input buffer size greater
> than zero will cause error 0x05 to be returned.
> 
> All changes were validated on a ZBook Workstation notebook. Additional
> validation was included to ensure no other commands were failing or
> incorrectly handled.
> 
> Signed-off-by: Jorge Lopez <jorge.lopez2@hp.com>

So after this the only remaining callers of hp_wmi_read_int() are:

1. hp_wmi_notify() case HPWMI_BEZEL_BUTTON
2. hp_wmi_rfkill_setup()
3. thermal_profile_get()

Where 2. looks like you just forgot to convert it since it
does a hp_wmi_read_int(HPWMI_WIRELESS_QUERY) which you do
convert in the hp_wmi_get_sw_state() and hp_wmi_get_hw_state()
helpers, suggesting that it should be converted in
hp_wmi_rfkill_setup() too.

This leaves just 1 and 3. So IMHO it would be better to
fix hp_wmi_read_int() and if 1. and 3. indeed need the old
behavior convert them to call hp_wmi_perform_query() directly.

Regards,

Hans



> 
> ---
> Based on the latest platform-drivers-x86.git/for-next
> ---
>  drivers/platform/x86/hp-wmi.c | 64 ++++++++++++++++++++++++-----------
>  1 file changed, 44 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
> index 544fce906ce7..de715687021a 100644
> --- a/drivers/platform/x86/hp-wmi.c
> +++ b/drivers/platform/x86/hp-wmi.c
> @@ -442,7 +442,7 @@ static int __init hp_wmi_bios_2009_later(void)
>  {
>  	u8 state[128];
>  	int ret = hp_wmi_perform_query(HPWMI_FEATURE2_QUERY, HPWMI_READ, &state,
> -				       sizeof(state), sizeof(state));
> +				       0, sizeof(state));
>  	if (!ret)
>  		return 1;
>  
> @@ -477,25 +477,37 @@ static const struct rfkill_ops hp_wmi_rfkill_ops = {
>  static bool hp_wmi_get_sw_state(enum hp_wmi_radio r)
>  {
>  	int mask = 0x200 << (r * 8);
> +	int ret= 0;
> +	int wireless = 0;
> +
> +	ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless,
> +				   0, sizeof(wireless));
>  
> -	int wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY);
> +	if (ret < 0)
> +	  return -EINVAL;
>  
>  	/* TBD: Pass error */
>  	WARN_ONCE(wireless < 0, "error executing HPWMI_WIRELESS_QUERY");
>  
> -	return !(wireless & mask);
> +	return (wireless & mask);
>  }
>  
>  static bool hp_wmi_get_hw_state(enum hp_wmi_radio r)
>  {
>  	int mask = 0x800 << (r * 8);
> +	int ret= 0;
> +	int wireless = 0;
>  
> -	int wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY);
> +	ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless,
> +				   0, sizeof(wireless));
> +
> +	if (ret < 0)
> +	  return -EINVAL;
>  
>  	/* TBD: Pass error */
>  	WARN_ONCE(wireless < 0, "error executing HPWMI_WIRELESS_QUERY");
>  
> -	return !(wireless & mask);
> +	return (wireless & mask);
>  }
>  
>  static int hp_wmi_rfkill2_set_block(void *data, bool blocked)
> @@ -520,7 +532,7 @@ static int hp_wmi_rfkill2_refresh(void)
>  	int err, i;
>  
>  	err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state,
> -				   sizeof(state), sizeof(state));
> +				   0, sizeof(state));
>  	if (err)
>  		return err;
>  
> @@ -546,27 +558,36 @@ static int hp_wmi_rfkill2_refresh(void)
>  static ssize_t display_show(struct device *dev, struct device_attribute *attr,
>  			    char *buf)
>  {
> -	int value = hp_wmi_read_int(HPWMI_DISPLAY_QUERY);
> -	if (value < 0)
> -		return value;
> +	int value = 0;
> +	int ret  = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, HPWMI_READ, &value,
> +				   0, sizeof(value));
> +	if (ret)
> +		return ret < 0 ? ret : -EINVAL;
> +
>  	return sprintf(buf, "%d\n", value);
>  }
>  
>  static ssize_t hddtemp_show(struct device *dev, struct device_attribute *attr,
>  			    char *buf)
>  {
> -	int value = hp_wmi_read_int(HPWMI_HDDTEMP_QUERY);
> -	if (value < 0)
> -		return value;
> +	int value = 0;
> +	int ret  = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, HPWMI_READ, &value,
> +				   0, sizeof(value));
> +	if (ret)
> +		return ret < 0 ? ret : -EINVAL;
> +
>  	return sprintf(buf, "%d\n", value);
>  }
>  
>  static ssize_t als_show(struct device *dev, struct device_attribute *attr,
>  			char *buf)
>  {
> -	int value = hp_wmi_read_int(HPWMI_ALS_QUERY);
> -	if (value < 0)
> -		return value;
> +	int value = 0;
> +	int ret  = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_READ, &value,
> +				   0, sizeof(value));
> +	if (ret)
> +		return ret < 0 ? ret : -EINVAL;
> +
>  	return sprintf(buf, "%d\n", value);
>  }
>  
> @@ -592,9 +613,12 @@ static ssize_t postcode_show(struct device *dev, struct device_attribute *attr,
>  			     char *buf)
>  {
>  	/* Get the POST error code of previous boot failure. */
> -	int value = hp_wmi_read_int(HPWMI_POSTCODEERROR_QUERY);
> -	if (value < 0)
> -		return value;
> +	int value = 0;
> +	int ret  = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_READ, &value,
> +				   0, sizeof(value));
> +	if (ret)
> +		return ret < 0 ? ret : -EINVAL;
> +
>  	return sprintf(buf, "0x%x\n", value);
>  }
>  
> @@ -609,7 +633,7 @@ static ssize_t als_store(struct device *dev, struct device_attribute *attr,
>  		return ret;
>  
>  	ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_WRITE, &tmp,
> -				       sizeof(tmp), sizeof(tmp));
> +				   sizeof(tmp), 0);
>  	if (ret)
>  		return ret < 0 ? ret : -EINVAL;
>  
> @@ -922,7 +946,7 @@ static int __init hp_wmi_rfkill2_setup(struct platform_device *device)
>  	int err, i;
>  
>  	err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state,
> -				   sizeof(state), sizeof(state));
> +				   0, sizeof(state));
>  	if (err)
>  		return err < 0 ? err : -EINVAL;
>
diff mbox series

Patch

diff --git a/drivers/platform/x86/hp-wmi.c b/drivers/platform/x86/hp-wmi.c
index 544fce906ce7..de715687021a 100644
--- a/drivers/platform/x86/hp-wmi.c
+++ b/drivers/platform/x86/hp-wmi.c
@@ -442,7 +442,7 @@  static int __init hp_wmi_bios_2009_later(void)
 {
 	u8 state[128];
 	int ret = hp_wmi_perform_query(HPWMI_FEATURE2_QUERY, HPWMI_READ, &state,
-				       sizeof(state), sizeof(state));
+				       0, sizeof(state));
 	if (!ret)
 		return 1;
 
@@ -477,25 +477,37 @@  static const struct rfkill_ops hp_wmi_rfkill_ops = {
 static bool hp_wmi_get_sw_state(enum hp_wmi_radio r)
 {
 	int mask = 0x200 << (r * 8);
+	int ret= 0;
+	int wireless = 0;
+
+	ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless,
+				   0, sizeof(wireless));
 
-	int wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY);
+	if (ret < 0)
+	  return -EINVAL;
 
 	/* TBD: Pass error */
 	WARN_ONCE(wireless < 0, "error executing HPWMI_WIRELESS_QUERY");
 
-	return !(wireless & mask);
+	return (wireless & mask);
 }
 
 static bool hp_wmi_get_hw_state(enum hp_wmi_radio r)
 {
 	int mask = 0x800 << (r * 8);
+	int ret= 0;
+	int wireless = 0;
 
-	int wireless = hp_wmi_read_int(HPWMI_WIRELESS_QUERY);
+	ret = hp_wmi_perform_query(HPWMI_WIRELESS_QUERY, HPWMI_READ, &wireless,
+				   0, sizeof(wireless));
+
+	if (ret < 0)
+	  return -EINVAL;
 
 	/* TBD: Pass error */
 	WARN_ONCE(wireless < 0, "error executing HPWMI_WIRELESS_QUERY");
 
-	return !(wireless & mask);
+	return (wireless & mask);
 }
 
 static int hp_wmi_rfkill2_set_block(void *data, bool blocked)
@@ -520,7 +532,7 @@  static int hp_wmi_rfkill2_refresh(void)
 	int err, i;
 
 	err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state,
-				   sizeof(state), sizeof(state));
+				   0, sizeof(state));
 	if (err)
 		return err;
 
@@ -546,27 +558,36 @@  static int hp_wmi_rfkill2_refresh(void)
 static ssize_t display_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	int value = hp_wmi_read_int(HPWMI_DISPLAY_QUERY);
-	if (value < 0)
-		return value;
+	int value = 0;
+	int ret  = hp_wmi_perform_query(HPWMI_DISPLAY_QUERY, HPWMI_READ, &value,
+				   0, sizeof(value));
+	if (ret)
+		return ret < 0 ? ret : -EINVAL;
+
 	return sprintf(buf, "%d\n", value);
 }
 
 static ssize_t hddtemp_show(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
-	int value = hp_wmi_read_int(HPWMI_HDDTEMP_QUERY);
-	if (value < 0)
-		return value;
+	int value = 0;
+	int ret  = hp_wmi_perform_query(HPWMI_HDDTEMP_QUERY, HPWMI_READ, &value,
+				   0, sizeof(value));
+	if (ret)
+		return ret < 0 ? ret : -EINVAL;
+
 	return sprintf(buf, "%d\n", value);
 }
 
 static ssize_t als_show(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
-	int value = hp_wmi_read_int(HPWMI_ALS_QUERY);
-	if (value < 0)
-		return value;
+	int value = 0;
+	int ret  = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_READ, &value,
+				   0, sizeof(value));
+	if (ret)
+		return ret < 0 ? ret : -EINVAL;
+
 	return sprintf(buf, "%d\n", value);
 }
 
@@ -592,9 +613,12 @@  static ssize_t postcode_show(struct device *dev, struct device_attribute *attr,
 			     char *buf)
 {
 	/* Get the POST error code of previous boot failure. */
-	int value = hp_wmi_read_int(HPWMI_POSTCODEERROR_QUERY);
-	if (value < 0)
-		return value;
+	int value = 0;
+	int ret  = hp_wmi_perform_query(HPWMI_POSTCODEERROR_QUERY, HPWMI_READ, &value,
+				   0, sizeof(value));
+	if (ret)
+		return ret < 0 ? ret : -EINVAL;
+
 	return sprintf(buf, "0x%x\n", value);
 }
 
@@ -609,7 +633,7 @@  static ssize_t als_store(struct device *dev, struct device_attribute *attr,
 		return ret;
 
 	ret = hp_wmi_perform_query(HPWMI_ALS_QUERY, HPWMI_WRITE, &tmp,
-				       sizeof(tmp), sizeof(tmp));
+				   sizeof(tmp), 0);
 	if (ret)
 		return ret < 0 ? ret : -EINVAL;
 
@@ -922,7 +946,7 @@  static int __init hp_wmi_rfkill2_setup(struct platform_device *device)
 	int err, i;
 
 	err = hp_wmi_perform_query(HPWMI_WIRELESS2_QUERY, HPWMI_READ, &state,
-				   sizeof(state), sizeof(state));
+				   0, sizeof(state));
 	if (err)
 		return err < 0 ? err : -EINVAL;