All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
@ 2020-10-20 22:09 Samuel Čavoj
  2020-10-20 22:14 ` Samuel Čavoj
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Samuel Čavoj @ 2020-10-20 22:09 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mark Gross, Corentin Chary, platform-driver-x86, linux-kernel,
	Samuel Čavoj

The UX360CA has a WMI device id 0x00060062, which reports whether the
lid is flipped in tablet mode (1) or in normal laptop mode (0).

Add a quirk (quirk_asus_use_lid_flip_devid) for devices on which this
WMI device should be used to figure out the SW_TABLET_MODE state, as
opposed to the quirk_asus_use_kbd_dock_devid.

Additionally, the device needs to be queried on resume and restore
because the firmware does not generate an event if the laptop is put to
sleep while in tablet mode, flipped to normal mode, and later awoken.

It is assumed other UX360* models have the same WMI device. As such, the
quirk is applied to devices with DMI_MATCH(DMI_PRODUCT_NAME, "UX360").
More devices with this feature need to be tested and added accordingly.

The reason for using an allowlist via the quirk mechanism is that the new
WMI device (0x00060062) is also present on some models which do not have
a 360 degree hinge (at least FX503VD and GL503VD from Hans' DSTS
collection) and therefore its presence cannot be relied on.

Signed-off-by: Samuel Čavoj <samuel@cavoj.net>
Cc: Hans de Goede <hdegoede@redhat.com>
---
changed v2 -> v3:
    - added check on resume from sleep and restore
---
 drivers/platform/x86/asus-nb-wmi.c         | 15 +++++++++
 drivers/platform/x86/asus-wmi.c            | 38 ++++++++++++++++++++++
 drivers/platform/x86/asus-wmi.h            |  1 +
 include/linux/platform_data/x86/asus-wmi.h |  1 +
 4 files changed, 55 insertions(+)

diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index 1d9fbabd02fb..d41d7ad14be0 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -119,6 +119,11 @@ static struct quirk_entry quirk_asus_use_kbd_dock_devid = {
 	.use_kbd_dock_devid = true,
 };
 
+static struct quirk_entry quirk_asus_use_lid_flip_devid = {
+	.wmi_backlight_set_devstate = true,
+	.use_lid_flip_devid = true,
+};
+
 static int dmi_matched(const struct dmi_system_id *dmi)
 {
 	pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -520,6 +525,16 @@ static const struct dmi_system_id asus_quirks[] = {
 		},
 		.driver_data = &quirk_asus_use_kbd_dock_devid,
 	},
+	{
+		.callback = dmi_matched,
+		.ident = "ASUS ZenBook Flip UX360",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			/* Match UX360* */
+			DMI_MATCH(DMI_PRODUCT_NAME, "UX360"),
+		},
+		.driver_data = &quirk_asus_use_lid_flip_devid,
+	},
 	{},
 };
 
diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
index 39e1a6396e08..864c608ad569 100644
--- a/drivers/platform/x86/asus-wmi.c
+++ b/drivers/platform/x86/asus-wmi.c
@@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
 #define NOTIFY_KBD_BRTTOGGLE		0xc7
 #define NOTIFY_KBD_FBM			0x99
 #define NOTIFY_KBD_TTP			0xae
+#define NOTIFY_LID_FLIP			0xfa
 
 #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
 
@@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
 		}
 	}
 
+	if (asus->driver->quirks->use_lid_flip_devid) {
+		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
+        if (result < 0)
+			asus->driver->quirks->use_lid_flip_devid = 0;
+		if (result >= 0) {
+			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
+			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
+		} else if (result == -ENODEV) {
+			pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
+		} else {
+			pr_err("Error checking for lid-flip: %d\n", result);
+		}
+	}
+
 	err = input_register_device(asus->inputdev);
 	if (err)
 		goto err_free_dev;
@@ -394,6 +409,16 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
 	asus->inputdev = NULL;
 }
 
+/* Tablet mode ****************************************************************/
+
+static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus) {
+	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
+	if (result >= 0) {
+		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
+		input_sync(asus->inputdev);
+	}
+}
+
 /* Battery ********************************************************************/
 
 /* The battery maximum charging percentage */
@@ -2128,6 +2153,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
 		return;
 	}
 
+	if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
+		lid_flip_tablet_mode_get_state(asus);
+		return;
+	}
+
 	if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
 		fan_boost_mode_switch_next(asus);
 		return;
@@ -2719,6 +2749,10 @@ static int asus_hotk_resume(struct device *device)
 
 	if (asus_wmi_has_fnlock_key(asus))
 		asus_wmi_fnlock_update(asus);
+
+	if (asus->driver->quirks->use_lid_flip_devid)
+		lid_flip_tablet_mode_get_state(asus);
+
 	return 0;
 }
 
@@ -2757,6 +2791,10 @@ static int asus_hotk_restore(struct device *device)
 
 	if (asus_wmi_has_fnlock_key(asus))
 		asus_wmi_fnlock_update(asus);
+
+	if (asus->driver->quirks->use_lid_flip_devid)
+		lid_flip_tablet_mode_get_state(asus);
+
 	return 0;
 }
 
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 1a95c172f94b..b302415bf1d9 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -34,6 +34,7 @@ struct quirk_entry {
 	bool wmi_backlight_set_devstate;
 	bool wmi_force_als_set;
 	bool use_kbd_dock_devid;
+	bool use_lid_flip_devid;
 	int wapf;
 	/*
 	 * For machines with AMD graphic chips, it will send out WMI event
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 897b8332a39f..2f274cf52805 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -62,6 +62,7 @@
 
 /* Misc */
 #define ASUS_WMI_DEVID_CAMERA		0x00060013
+#define ASUS_WMI_DEVID_LID_FLIP		0x00060062
 
 /* Storage */
 #define ASUS_WMI_DEVID_CARDREADER	0x00080013
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
  2020-10-20 22:09 [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360 Samuel Čavoj
@ 2020-10-20 22:14 ` Samuel Čavoj
  2020-10-20 22:29 ` Samuel Čavoj
  2020-10-28 11:50 ` Hans de Goede
  2 siblings, 0 replies; 6+ messages in thread
From: Samuel Čavoj @ 2020-10-20 22:14 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mark Gross, Corentin Chary, platform-driver-x86, linux-kernel

Hi,

On 21.10.2020 00:09, Samuel Čavoj wrote:
> The UX360CA has a WMI device id 0x00060062, which reports whether the
> lid is flipped in tablet mode (1) or in normal laptop mode (0).
> 
> Add a quirk (quirk_asus_use_lid_flip_devid) for devices on which this
> WMI device should be used to figure out the SW_TABLET_MODE state, as
> opposed to the quirk_asus_use_kbd_dock_devid.
> 
> Additionally, the device needs to be queried on resume and restore
> because the firmware does not generate an event if the laptop is put to
> sleep while in tablet mode, flipped to normal mode, and later awoken.
> 
> It is assumed other UX360* models have the same WMI device. As such, the
> quirk is applied to devices with DMI_MATCH(DMI_PRODUCT_NAME, "UX360").
> More devices with this feature need to be tested and added accordingly.
> 
> The reason for using an allowlist via the quirk mechanism is that the new
> WMI device (0x00060062) is also present on some models which do not have
> a 360 degree hinge (at least FX503VD and GL503VD from Hans' DSTS
> collection) and therefore its presence cannot be relied on.
> 
> Signed-off-by: Samuel Čavoj <samuel@cavoj.net>
> Cc: Hans de Goede <hdegoede@redhat.com>
> ---
> changed v2 -> v3:
>     - added check on resume from sleep and restore
> ---
>  drivers/platform/x86/asus-nb-wmi.c         | 15 +++++++++
>  drivers/platform/x86/asus-wmi.c            | 38 ++++++++++++++++++++++
>  drivers/platform/x86/asus-wmi.h            |  1 +
>  include/linux/platform_data/x86/asus-wmi.h |  1 +
>  4 files changed, 55 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index 1d9fbabd02fb..d41d7ad14be0 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -119,6 +119,11 @@ static struct quirk_entry quirk_asus_use_kbd_dock_devid = {
>  	.use_kbd_dock_devid = true,
>  };
>  
> +static struct quirk_entry quirk_asus_use_lid_flip_devid = {
> +	.wmi_backlight_set_devstate = true,
> +	.use_lid_flip_devid = true,
> +};
> +
>  static int dmi_matched(const struct dmi_system_id *dmi)
>  {
>  	pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -520,6 +525,16 @@ static const struct dmi_system_id asus_quirks[] = {
>  		},
>  		.driver_data = &quirk_asus_use_kbd_dock_devid,
>  	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ZenBook Flip UX360",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			/* Match UX360* */
> +			DMI_MATCH(DMI_PRODUCT_NAME, "UX360"),
> +		},
> +		.driver_data = &quirk_asus_use_lid_flip_devid,
> +	},
>  	{},
>  };
>  
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 39e1a6396e08..864c608ad569 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
>  #define NOTIFY_KBD_BRTTOGGLE		0xc7
>  #define NOTIFY_KBD_FBM			0x99
>  #define NOTIFY_KBD_TTP			0xae
> +#define NOTIFY_LID_FLIP			0xfa
>  
>  #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
>  
> @@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
>  		}
>  	}
>  
> +	if (asus->driver->quirks->use_lid_flip_devid) {
> +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +        if (result < 0)
> +			asus->driver->quirks->use_lid_flip_devid = 0;

I had the idea of doing this, not sure if it is actually a good idea
though. The idea is that it would prevent querying the device later
during runtime, if the first get_devstate fails. Also I assume doing a
input_report_switch without a corresponding input_set_capability is not
great either, this would prevent that, if for some reason the
get_devstate fails in the beginning but later fixes itself.

However, I can also imagine that writing to the quirks structure is
frowned upon, please tell if that is the case. Thanks.

> +		if (result >= 0) {
> +			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
> +			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> +		} else if (result == -ENODEV) {
> +			pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
> +		} else {
> +			pr_err("Error checking for lid-flip: %d\n", result);
> +		}
> +	}
> +
>  	err = input_register_device(asus->inputdev);
>  	if (err)
>  		goto err_free_dev;
> @@ -394,6 +409,16 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
>  	asus->inputdev = NULL;
>  }
>  
> +/* Tablet mode ****************************************************************/
> +
> +static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus) {

Is the function name reasonable?

Regards, 
Sam

> +	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +	if (result >= 0) {
> +		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> +		input_sync(asus->inputdev);
> +	}
> +}
> +
>  /* Battery ********************************************************************/
>  
>  /* The battery maximum charging percentage */
> @@ -2128,6 +2153,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
>  		return;
>  	}
>  
> +	if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
> +		lid_flip_tablet_mode_get_state(asus);
> +		return;
> +	}
> +
>  	if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
>  		fan_boost_mode_switch_next(asus);
>  		return;
> @@ -2719,6 +2749,10 @@ static int asus_hotk_resume(struct device *device)
>  
>  	if (asus_wmi_has_fnlock_key(asus))
>  		asus_wmi_fnlock_update(asus);
> +
> +	if (asus->driver->quirks->use_lid_flip_devid)
> +		lid_flip_tablet_mode_get_state(asus);
> +
>  	return 0;
>  }
>  
> @@ -2757,6 +2791,10 @@ static int asus_hotk_restore(struct device *device)
>  
>  	if (asus_wmi_has_fnlock_key(asus))
>  		asus_wmi_fnlock_update(asus);
> +
> +	if (asus->driver->quirks->use_lid_flip_devid)
> +		lid_flip_tablet_mode_get_state(asus);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 1a95c172f94b..b302415bf1d9 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -34,6 +34,7 @@ struct quirk_entry {
>  	bool wmi_backlight_set_devstate;
>  	bool wmi_force_als_set;
>  	bool use_kbd_dock_devid;
> +	bool use_lid_flip_devid;
>  	int wapf;
>  	/*
>  	 * For machines with AMD graphic chips, it will send out WMI event
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 897b8332a39f..2f274cf52805 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -62,6 +62,7 @@
>  
>  /* Misc */
>  #define ASUS_WMI_DEVID_CAMERA		0x00060013
> +#define ASUS_WMI_DEVID_LID_FLIP		0x00060062
>  
>  /* Storage */
>  #define ASUS_WMI_DEVID_CARDREADER	0x00080013
> -- 
> 2.29.0
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
  2020-10-20 22:09 [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360 Samuel Čavoj
  2020-10-20 22:14 ` Samuel Čavoj
@ 2020-10-20 22:29 ` Samuel Čavoj
  2020-10-28 11:44   ` Hans de Goede
  2020-10-28 11:50 ` Hans de Goede
  2 siblings, 1 reply; 6+ messages in thread
From: Samuel Čavoj @ 2020-10-20 22:29 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mark Gross, Corentin Chary, platform-driver-x86, linux-kernel

Hi,

naturally I notice this right after I send the patch, but my whitespace
is wrong. Time to set a pre-commit hook up. I guess that means a v4,
unless you would fix it on your end? It's just a single line.

Sorry about all the noise,
Sam

On 21.10.2020 00:09, Samuel Čavoj wrote:
> @@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
>  		}
>  	}
>  
> +	if (asus->driver->quirks->use_lid_flip_devid) {
> +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +        if (result < 0)

Right ^here.

> +			asus->driver->quirks->use_lid_flip_devid = 0;
> +		if (result >= 0) {
> +			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
> +			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> +		} else if (result == -ENODEV) {
> +			pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
> +		} else {
> +			pr_err("Error checking for lid-flip: %d\n", result);
> +		}
> +	}

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
  2020-10-20 22:29 ` Samuel Čavoj
@ 2020-10-28 11:44   ` Hans de Goede
  0 siblings, 0 replies; 6+ messages in thread
From: Hans de Goede @ 2020-10-28 11:44 UTC (permalink / raw)
  To: Samuel Čavoj
  Cc: Mark Gross, Corentin Chary, platform-driver-x86, linux-kernel

Hi,

On 10/21/20 12:29 AM, Samuel Čavoj wrote:
> Hi,
> 
> naturally I notice this right after I send the patch, but my whitespace
> is wrong. Time to set a pre-commit hook up. I guess that means a v4,
> unless you would fix it on your end? It's just a single line.
> 
> Sorry about all the noise,
> Sam
> 
> On 21.10.2020 00:09, Samuel Čavoj wrote:
>> @@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
>>  		}
>>  	}
>>  
>> +	if (asus->driver->quirks->use_lid_flip_devid) {
>> +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
>> +        if (result < 0)
> 
> Right ^here.

No worries I will fix this when merging it.

> 
>> +			asus->driver->quirks->use_lid_flip_devid = 0;
>> +		if (result >= 0) {
>> +			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
>> +			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
>> +		} else if (result == -ENODEV) {
>> +			pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
>> +		} else {
>> +			pr_err("Error checking for lid-flip: %d\n", result);
>> +		}
>> +	}
> 

>> +	if (asus->driver->quirks->use_lid_flip_devid) {
>> +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
>> +        if (result < 0)
>> +			asus->driver->quirks->use_lid_flip_devid = 0;
> 
> I had the idea of doing this, not sure if it is actually a good idea
> though. The idea is that it would prevent querying the device later
> during runtime, if the first get_devstate fails. Also I assume doing a
> input_report_switch without a corresponding input_set_capability is not
> great either, this would prevent that, if for some reason the
> get_devstate fails in the beginning but later fixes itself.
> 
> However, I can also imagine that writing to the quirks structure is
> frowned upon, please tell if that is the case. Thanks.

The quirk_entry structs are not defined const; and they are already written
to in other places (e.g. the wapf module param handling). So this is fine.

Note FWIW, that calling input_report_switch() with a switch which
has not been declared before through input_set_capability() before
is allowed and is simply treated as a no-op. But just skipping the
handling of the event alltogether is a bit cleaner, so your approach is
fine.

Regards,

Hans


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
  2020-10-20 22:09 [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360 Samuel Čavoj
  2020-10-20 22:14 ` Samuel Čavoj
  2020-10-20 22:29 ` Samuel Čavoj
@ 2020-10-28 11:50 ` Hans de Goede
  2020-10-28 12:22   ` Samuel Čavoj
  2 siblings, 1 reply; 6+ messages in thread
From: Hans de Goede @ 2020-10-28 11:50 UTC (permalink / raw)
  To: Samuel Čavoj
  Cc: Mark Gross, Corentin Chary, platform-driver-x86, linux-kernel

Hi,

On 10/21/20 12:09 AM, Samuel Čavoj wrote:
> The UX360CA has a WMI device id 0x00060062, which reports whether the
> lid is flipped in tablet mode (1) or in normal laptop mode (0).
> 
> Add a quirk (quirk_asus_use_lid_flip_devid) for devices on which this
> WMI device should be used to figure out the SW_TABLET_MODE state, as
> opposed to the quirk_asus_use_kbd_dock_devid.
> 
> Additionally, the device needs to be queried on resume and restore
> because the firmware does not generate an event if the laptop is put to
> sleep while in tablet mode, flipped to normal mode, and later awoken.
> 
> It is assumed other UX360* models have the same WMI device. As such, the
> quirk is applied to devices with DMI_MATCH(DMI_PRODUCT_NAME, "UX360").
> More devices with this feature need to be tested and added accordingly.
> 
> The reason for using an allowlist via the quirk mechanism is that the new
> WMI device (0x00060062) is also present on some models which do not have
> a 360 degree hinge (at least FX503VD and GL503VD from Hans' DSTS
> collection) and therefore its presence cannot be relied on.
> 
> Signed-off-by: Samuel Čavoj <samuel@cavoj.net>
> Cc: Hans de Goede <hdegoede@redhat.com>

Thank you for your patch, I've applied this patch to my review-hans 
branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

As discussed I've fixed the whitespace issue; and I've also fixed
the following 2 checkpatch warnings:

ERROR: open brace '{' following function definitions go on the next line
#114: FILE: drivers/platform/x86/asus-wmi.c:414:
+static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus) {

WARNING: Missing a blank line after declarations
#116: FILE: drivers/platform/x86/asus-wmi.c:416:
+	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
+	if (result >= 0) {

Next time please consider doing:

git format-patch HEAD~
scripts/checkpatch.pl 0001-*.patch

Before sending out your patch.

Note it will show up in my review-hans branch once I've pushed my local
branch there, which might take a while.

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans


> ---
> changed v2 -> v3:
>     - added check on resume from sleep and restore
> ---
>  drivers/platform/x86/asus-nb-wmi.c         | 15 +++++++++
>  drivers/platform/x86/asus-wmi.c            | 38 ++++++++++++++++++++++
>  drivers/platform/x86/asus-wmi.h            |  1 +
>  include/linux/platform_data/x86/asus-wmi.h |  1 +
>  4 files changed, 55 insertions(+)
> 
> diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> index 1d9fbabd02fb..d41d7ad14be0 100644
> --- a/drivers/platform/x86/asus-nb-wmi.c
> +++ b/drivers/platform/x86/asus-nb-wmi.c
> @@ -119,6 +119,11 @@ static struct quirk_entry quirk_asus_use_kbd_dock_devid = {
>  	.use_kbd_dock_devid = true,
>  };
>  
> +static struct quirk_entry quirk_asus_use_lid_flip_devid = {
> +	.wmi_backlight_set_devstate = true,
> +	.use_lid_flip_devid = true,
> +};
> +
>  static int dmi_matched(const struct dmi_system_id *dmi)
>  {
>  	pr_info("Identified laptop model '%s'\n", dmi->ident);
> @@ -520,6 +525,16 @@ static const struct dmi_system_id asus_quirks[] = {
>  		},
>  		.driver_data = &quirk_asus_use_kbd_dock_devid,
>  	},
> +	{
> +		.callback = dmi_matched,
> +		.ident = "ASUS ZenBook Flip UX360",
> +		.matches = {
> +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> +			/* Match UX360* */
> +			DMI_MATCH(DMI_PRODUCT_NAME, "UX360"),
> +		},
> +		.driver_data = &quirk_asus_use_lid_flip_devid,
> +	},
>  	{},
>  };
>  
> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> index 39e1a6396e08..864c608ad569 100644
> --- a/drivers/platform/x86/asus-wmi.c
> +++ b/drivers/platform/x86/asus-wmi.c
> @@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
>  #define NOTIFY_KBD_BRTTOGGLE		0xc7
>  #define NOTIFY_KBD_FBM			0x99
>  #define NOTIFY_KBD_TTP			0xae
> +#define NOTIFY_LID_FLIP			0xfa
>  
>  #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
>  
> @@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
>  		}
>  	}
>  
> +	if (asus->driver->quirks->use_lid_flip_devid) {
> +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +        if (result < 0)
> +			asus->driver->quirks->use_lid_flip_devid = 0;
> +		if (result >= 0) {
> +			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
> +			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> +		} else if (result == -ENODEV) {
> +			pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
> +		} else {
> +			pr_err("Error checking for lid-flip: %d\n", result);
> +		}
> +	}
> +
>  	err = input_register_device(asus->inputdev);
>  	if (err)
>  		goto err_free_dev;
> @@ -394,6 +409,16 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
>  	asus->inputdev = NULL;
>  }
>  
> +/* Tablet mode ****************************************************************/
> +
> +static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus) {
> +	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +	if (result >= 0) {
> +		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> +		input_sync(asus->inputdev);
> +	}
> +}
> +
>  /* Battery ********************************************************************/
>  
>  /* The battery maximum charging percentage */
> @@ -2128,6 +2153,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
>  		return;
>  	}
>  
> +	if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
> +		lid_flip_tablet_mode_get_state(asus);
> +		return;
> +	}
> +
>  	if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
>  		fan_boost_mode_switch_next(asus);
>  		return;
> @@ -2719,6 +2749,10 @@ static int asus_hotk_resume(struct device *device)
>  
>  	if (asus_wmi_has_fnlock_key(asus))
>  		asus_wmi_fnlock_update(asus);
> +
> +	if (asus->driver->quirks->use_lid_flip_devid)
> +		lid_flip_tablet_mode_get_state(asus);
> +
>  	return 0;
>  }
>  
> @@ -2757,6 +2791,10 @@ static int asus_hotk_restore(struct device *device)
>  
>  	if (asus_wmi_has_fnlock_key(asus))
>  		asus_wmi_fnlock_update(asus);
> +
> +	if (asus->driver->quirks->use_lid_flip_devid)
> +		lid_flip_tablet_mode_get_state(asus);
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> index 1a95c172f94b..b302415bf1d9 100644
> --- a/drivers/platform/x86/asus-wmi.h
> +++ b/drivers/platform/x86/asus-wmi.h
> @@ -34,6 +34,7 @@ struct quirk_entry {
>  	bool wmi_backlight_set_devstate;
>  	bool wmi_force_als_set;
>  	bool use_kbd_dock_devid;
> +	bool use_lid_flip_devid;
>  	int wapf;
>  	/*
>  	 * For machines with AMD graphic chips, it will send out WMI event
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 897b8332a39f..2f274cf52805 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -62,6 +62,7 @@
>  
>  /* Misc */
>  #define ASUS_WMI_DEVID_CAMERA		0x00060013
> +#define ASUS_WMI_DEVID_LID_FLIP		0x00060062
>  
>  /* Storage */
>  #define ASUS_WMI_DEVID_CARDREADER	0x00080013
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360
  2020-10-28 11:50 ` Hans de Goede
@ 2020-10-28 12:22   ` Samuel Čavoj
  0 siblings, 0 replies; 6+ messages in thread
From: Samuel Čavoj @ 2020-10-28 12:22 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Mark Gross, Corentin Chary, platform-driver-x86, linux-kernel

Hi, 

On 28.10.2020 12:50, Hans de Goede wrote:
> Hi,
> 
> On 10/21/20 12:09 AM, Samuel Čavoj wrote:
> > The UX360CA has a WMI device id 0x00060062, which reports whether the
> > lid is flipped in tablet mode (1) or in normal laptop mode (0).
> > 
> > Add a quirk (quirk_asus_use_lid_flip_devid) for devices on which this
> > WMI device should be used to figure out the SW_TABLET_MODE state, as
> > opposed to the quirk_asus_use_kbd_dock_devid.
> > 
> > Additionally, the device needs to be queried on resume and restore
> > because the firmware does not generate an event if the laptop is put to
> > sleep while in tablet mode, flipped to normal mode, and later awoken.
> > 
> > It is assumed other UX360* models have the same WMI device. As such, the
> > quirk is applied to devices with DMI_MATCH(DMI_PRODUCT_NAME, "UX360").
> > More devices with this feature need to be tested and added accordingly.
> > 
> > The reason for using an allowlist via the quirk mechanism is that the new
> > WMI device (0x00060062) is also present on some models which do not have
> > a 360 degree hinge (at least FX503VD and GL503VD from Hans' DSTS
> > collection) and therefore its presence cannot be relied on.
> > 
> > Signed-off-by: Samuel Čavoj <samuel@cavoj.net>
> > Cc: Hans de Goede <hdegoede@redhat.com>
> 
> Thank you for your patch, I've applied this patch to my review-hans 
> branch:
> https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Great, thanks.

> As discussed I've fixed the whitespace issue; and I've also fixed
> the following 2 checkpatch warnings:
> 
> ERROR: open brace '{' following function definitions go on the next line
> #114: FILE: drivers/platform/x86/asus-wmi.c:414:
> +static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus) {
> 
> WARNING: Missing a blank line after declarations
> #116: FILE: drivers/platform/x86/asus-wmi.c:416:
> +	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> +	if (result >= 0) {
> 
> Next time please consider doing:
> 
> git format-patch HEAD~
> scripts/checkpatch.pl 0001-*.patch

Yes, will do. Sorry about the trouble.

> Before sending out your patch.
> 
> Note it will show up in my review-hans branch once I've pushed my local
> branch there, which might take a while.
> 
> Once I've run some tests on this branch the patches there will be
> added to the platform-drivers-x86/for-next branch and eventually
> will be included in the pdx86 pull-request to Linus for the next
> merge-window.
> 
> Regards,
> 
> Hans
> 

Regards,
Sam

> 
> > ---
> > changed v2 -> v3:
> >     - added check on resume from sleep and restore
> > ---
> >  drivers/platform/x86/asus-nb-wmi.c         | 15 +++++++++
> >  drivers/platform/x86/asus-wmi.c            | 38 ++++++++++++++++++++++
> >  drivers/platform/x86/asus-wmi.h            |  1 +
> >  include/linux/platform_data/x86/asus-wmi.h |  1 +
> >  4 files changed, 55 insertions(+)
> > 
> > diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
> > index 1d9fbabd02fb..d41d7ad14be0 100644
> > --- a/drivers/platform/x86/asus-nb-wmi.c
> > +++ b/drivers/platform/x86/asus-nb-wmi.c
> > @@ -119,6 +119,11 @@ static struct quirk_entry quirk_asus_use_kbd_dock_devid = {
> >  	.use_kbd_dock_devid = true,
> >  };
> >  
> > +static struct quirk_entry quirk_asus_use_lid_flip_devid = {
> > +	.wmi_backlight_set_devstate = true,
> > +	.use_lid_flip_devid = true,
> > +};
> > +
> >  static int dmi_matched(const struct dmi_system_id *dmi)
> >  {
> >  	pr_info("Identified laptop model '%s'\n", dmi->ident);
> > @@ -520,6 +525,16 @@ static const struct dmi_system_id asus_quirks[] = {
> >  		},
> >  		.driver_data = &quirk_asus_use_kbd_dock_devid,
> >  	},
> > +	{
> > +		.callback = dmi_matched,
> > +		.ident = "ASUS ZenBook Flip UX360",
> > +		.matches = {
> > +			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
> > +			/* Match UX360* */
> > +			DMI_MATCH(DMI_PRODUCT_NAME, "UX360"),
> > +		},
> > +		.driver_data = &quirk_asus_use_lid_flip_devid,
> > +	},
> >  	{},
> >  };
> >  
> > diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c
> > index 39e1a6396e08..864c608ad569 100644
> > --- a/drivers/platform/x86/asus-wmi.c
> > +++ b/drivers/platform/x86/asus-wmi.c
> > @@ -63,6 +63,7 @@ MODULE_LICENSE("GPL");
> >  #define NOTIFY_KBD_BRTTOGGLE		0xc7
> >  #define NOTIFY_KBD_FBM			0x99
> >  #define NOTIFY_KBD_TTP			0xae
> > +#define NOTIFY_LID_FLIP			0xfa
> >  
> >  #define ASUS_WMI_FNLOCK_BIOS_DISABLED	BIT(0)
> >  
> > @@ -375,6 +376,20 @@ static int asus_wmi_input_init(struct asus_wmi *asus)
> >  		}
> >  	}
> >  
> > +	if (asus->driver->quirks->use_lid_flip_devid) {
> > +		result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> > +        if (result < 0)
> > +			asus->driver->quirks->use_lid_flip_devid = 0;
> > +		if (result >= 0) {
> > +			input_set_capability(asus->inputdev, EV_SW, SW_TABLET_MODE);
> > +			input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> > +		} else if (result == -ENODEV) {
> > +			pr_err("This device has lid_flip quirk but got ENODEV checking it. This is a bug.");
> > +		} else {
> > +			pr_err("Error checking for lid-flip: %d\n", result);
> > +		}
> > +	}
> > +
> >  	err = input_register_device(asus->inputdev);
> >  	if (err)
> >  		goto err_free_dev;
> > @@ -394,6 +409,16 @@ static void asus_wmi_input_exit(struct asus_wmi *asus)
> >  	asus->inputdev = NULL;
> >  }
> >  
> > +/* Tablet mode ****************************************************************/
> > +
> > +static void lid_flip_tablet_mode_get_state(struct asus_wmi *asus) {
> > +	int result = asus_wmi_get_devstate_simple(asus, ASUS_WMI_DEVID_LID_FLIP);
> > +	if (result >= 0) {
> > +		input_report_switch(asus->inputdev, SW_TABLET_MODE, result);
> > +		input_sync(asus->inputdev);
> > +	}
> > +}
> > +
> >  /* Battery ********************************************************************/
> >  
> >  /* The battery maximum charging percentage */
> > @@ -2128,6 +2153,11 @@ static void asus_wmi_handle_event_code(int code, struct asus_wmi *asus)
> >  		return;
> >  	}
> >  
> > +	if (asus->driver->quirks->use_lid_flip_devid && code == NOTIFY_LID_FLIP) {
> > +		lid_flip_tablet_mode_get_state(asus);
> > +		return;
> > +	}
> > +
> >  	if (asus->fan_boost_mode_available && code == NOTIFY_KBD_FBM) {
> >  		fan_boost_mode_switch_next(asus);
> >  		return;
> > @@ -2719,6 +2749,10 @@ static int asus_hotk_resume(struct device *device)
> >  
> >  	if (asus_wmi_has_fnlock_key(asus))
> >  		asus_wmi_fnlock_update(asus);
> > +
> > +	if (asus->driver->quirks->use_lid_flip_devid)
> > +		lid_flip_tablet_mode_get_state(asus);
> > +
> >  	return 0;
> >  }
> >  
> > @@ -2757,6 +2791,10 @@ static int asus_hotk_restore(struct device *device)
> >  
> >  	if (asus_wmi_has_fnlock_key(asus))
> >  		asus_wmi_fnlock_update(asus);
> > +
> > +	if (asus->driver->quirks->use_lid_flip_devid)
> > +		lid_flip_tablet_mode_get_state(asus);
> > +
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
> > index 1a95c172f94b..b302415bf1d9 100644
> > --- a/drivers/platform/x86/asus-wmi.h
> > +++ b/drivers/platform/x86/asus-wmi.h
> > @@ -34,6 +34,7 @@ struct quirk_entry {
> >  	bool wmi_backlight_set_devstate;
> >  	bool wmi_force_als_set;
> >  	bool use_kbd_dock_devid;
> > +	bool use_lid_flip_devid;
> >  	int wapf;
> >  	/*
> >  	 * For machines with AMD graphic chips, it will send out WMI event
> > diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> > index 897b8332a39f..2f274cf52805 100644
> > --- a/include/linux/platform_data/x86/asus-wmi.h
> > +++ b/include/linux/platform_data/x86/asus-wmi.h
> > @@ -62,6 +62,7 @@
> >  
> >  /* Misc */
> >  #define ASUS_WMI_DEVID_CAMERA		0x00060013
> > +#define ASUS_WMI_DEVID_LID_FLIP		0x00060062
> >  
> >  /* Storage */
> >  #define ASUS_WMI_DEVID_CARDREADER	0x00080013
> > 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-10-28 21:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-20 22:09 [PATCH v3] platform/x86: asus-wmi: Add support for SW_TABLET_MODE on UX360 Samuel Čavoj
2020-10-20 22:14 ` Samuel Čavoj
2020-10-20 22:29 ` Samuel Čavoj
2020-10-28 11:44   ` Hans de Goede
2020-10-28 11:50 ` Hans de Goede
2020-10-28 12:22   ` Samuel Čavoj

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.