diff mbox series

platform/x86: intel: hid: Always call BTNL ACPI method

Message ID 20230715181516.5173-1-hdegoede@redhat.com (mailing list archive)
State Accepted, archived
Headers show
Series platform/x86: intel: hid: Always call BTNL ACPI method | expand

Commit Message

Hans de Goede July 15, 2023, 6:15 p.m. UTC
On a HP Elite Dragonfly G2 the 0xcc and 0xcd events for SW_TABLET_MODE
are only send after the BTNL ACPI method has been called.

Likely more devices need this, so make the BTNL ACPI method unconditional
instead of only doing it on devices with a 5 button array.

Note this also makes the intel_button_array_enable() call in probe()
unconditional, that function does its own priv->array check. This makes
the intel_button_array_enable() call in probe() consistent with the calls
done on suspend/resume which also rely on the priv->array check inside
the function.

Reported-by: Maxim Mikityanskiy <maxtram95@gmail.com>
Closes: https://lore.kernel.org/platform-driver-x86/20230712175023.31651-1-maxtram95@gmail.com/
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/platform/x86/intel/hid.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

Comments

Hans de Goede July 25, 2023, 1:29 p.m. UTC | #1
Hi,

On 7/15/23 20:15, Hans de Goede wrote:
> On a HP Elite Dragonfly G2 the 0xcc and 0xcd events for SW_TABLET_MODE
> are only send after the BTNL ACPI method has been called.
> 
> Likely more devices need this, so make the BTNL ACPI method unconditional
> instead of only doing it on devices with a 5 button array.
> 
> Note this also makes the intel_button_array_enable() call in probe()
> unconditional, that function does its own priv->array check. This makes
> the intel_button_array_enable() call in probe() consistent with the calls
> done on suspend/resume which also rely on the priv->array check inside
> the function.
> 
> Reported-by: Maxim Mikityanskiy <maxtram95@gmail.com>
> Closes: https://lore.kernel.org/platform-driver-x86/20230712175023.31651-1-maxtram95@gmail.com/
> Signed-off-by: Hans de Goede <hdegoede@redhat.com>

I've added this to the pdx86/fixes branch now.

Regards,

Hans

> ---
>  drivers/platform/x86/intel/hid.c | 21 +++++++++------------
>  1 file changed, 9 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
> index 5632bd3c534a..641f2797406e 100644
> --- a/drivers/platform/x86/intel/hid.c
> +++ b/drivers/platform/x86/intel/hid.c
> @@ -620,7 +620,7 @@ static bool button_array_present(struct platform_device *device)
>  static int intel_hid_probe(struct platform_device *device)
>  {
>  	acpi_handle handle = ACPI_HANDLE(&device->dev);
> -	unsigned long long mode;
> +	unsigned long long mode, dummy;
>  	struct intel_hid_priv *priv;
>  	acpi_status status;
>  	int err;
> @@ -692,18 +692,15 @@ static int intel_hid_probe(struct platform_device *device)
>  	if (err)
>  		goto err_remove_notify;
>  
> -	if (priv->array) {
> -		unsigned long long dummy;
> +	intel_button_array_enable(&device->dev, true);
>  
> -		intel_button_array_enable(&device->dev, true);
> -
> -		/* Call button load method to enable HID power button */
> -		if (!intel_hid_evaluate_method(handle, INTEL_HID_DSM_BTNL_FN,
> -					       &dummy)) {
> -			dev_warn(&device->dev,
> -				 "failed to enable HID power button\n");
> -		}
> -	}
> +	/*
> +	 * Call button load method to enable HID power button
> +	 * Always do this since it activates events on some devices without
> +	 * a button array too.
> +	 */
> +	if (!intel_hid_evaluate_method(handle, INTEL_HID_DSM_BTNL_FN, &dummy))
> +		dev_warn(&device->dev, "failed to enable HID power button\n");
>  
>  	device_init_wakeup(&device->dev, true);
>  	/*
diff mbox series

Patch

diff --git a/drivers/platform/x86/intel/hid.c b/drivers/platform/x86/intel/hid.c
index 5632bd3c534a..641f2797406e 100644
--- a/drivers/platform/x86/intel/hid.c
+++ b/drivers/platform/x86/intel/hid.c
@@ -620,7 +620,7 @@  static bool button_array_present(struct platform_device *device)
 static int intel_hid_probe(struct platform_device *device)
 {
 	acpi_handle handle = ACPI_HANDLE(&device->dev);
-	unsigned long long mode;
+	unsigned long long mode, dummy;
 	struct intel_hid_priv *priv;
 	acpi_status status;
 	int err;
@@ -692,18 +692,15 @@  static int intel_hid_probe(struct platform_device *device)
 	if (err)
 		goto err_remove_notify;
 
-	if (priv->array) {
-		unsigned long long dummy;
+	intel_button_array_enable(&device->dev, true);
 
-		intel_button_array_enable(&device->dev, true);
-
-		/* Call button load method to enable HID power button */
-		if (!intel_hid_evaluate_method(handle, INTEL_HID_DSM_BTNL_FN,
-					       &dummy)) {
-			dev_warn(&device->dev,
-				 "failed to enable HID power button\n");
-		}
-	}
+	/*
+	 * Call button load method to enable HID power button
+	 * Always do this since it activates events on some devices without
+	 * a button array too.
+	 */
+	if (!intel_hid_evaluate_method(handle, INTEL_HID_DSM_BTNL_FN, &dummy))
+		dev_warn(&device->dev, "failed to enable HID power button\n");
 
 	device_init_wakeup(&device->dev, true);
 	/*