| Message ID | 1519329522-16919-1-git-send-email-mario.limonciello@dell.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Darren Hart |
| Headers | show |
On Thu, Feb 22, 2018 at 01:58:42PM -0600, Mario Limonciello wrote: > Some laptops such as the XPS 9360 support the intel-vbtn INT33D6 > interface but don't initialize the bit that intel-vbtn uses to > represent switching tablet mode. > > By running this only on real 2-in-1's it shouldn't cause false > positives. > > Fixes: 30323fb6d5 ("Support tablet mode switch") > Reported-by: Jeremy Cline <jeremy@jcline.org> > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> > --- > Note: I also moved the code for detecting tablet into it's own > function. To keep things under 80 lines with if statements was > making the code very unreadable otherwise. > > drivers/platform/x86/intel-vbtn.c | 46 ++++++++++++++++++++++++--------------- > 1 file changed, 29 insertions(+), 17 deletions(-) > > diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c > index b703d6f..8173307 100644 > --- a/drivers/platform/x86/intel-vbtn.c > +++ b/drivers/platform/x86/intel-vbtn.c > @@ -7,6 +7,7 @@ > */ > > #include <linux/acpi.h> > +#include <linux/dmi.h> > #include <linux/input.h> > #include <linux/input/sparse-keymap.h> > #include <linux/kernel.h> > @@ -97,9 +98,35 @@ static void notify_handler(acpi_handle handle, u32 event, void *context) > dev_dbg(&device->dev, "unknown event index 0x%x\n", event); > } > > -static int intel_vbtn_probe(struct platform_device *device) > +static void detect_tablet_mode(struct platform_device *device) > { > + const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE); > + struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev); > + acpi_handle handle = ACPI_HANDLE(&device->dev); > struct acpi_buffer vgbs_output = { ACPI_ALLOCATE_BUFFER, NULL }; > + union acpi_object *obj; > + acpi_status status; > + int m; > + > + if (!(chassis_type && strcmp(chassis_type, "31") == 0)) > + goto out; > + > + status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output); > + if (ACPI_FAILURE(status)) > + goto out; > + > + obj = vgbs_output.pointer; > + if (!(obj && obj->type == ACPI_TYPE_INTEGER)) > + goto out; > + > + m = !(obj->integer.value & TABLET_MODE_FLAG); > + input_report_switch(priv->input_dev, SW_TABLET_MODE, m); > +out: > + kfree(vgbs_output.pointer); > +} > + > +static int intel_vbtn_probe(struct platform_device *device) > +{ > acpi_handle handle = ACPI_HANDLE(&device->dev); > struct intel_vbtn_priv *priv; > acpi_status status; > @@ -122,22 +149,7 @@ static int intel_vbtn_probe(struct platform_device *device) > return err; > } > > - /* > - * VGBS being present and returning something means we have > - * a tablet mode switch. > - */ > - status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output); > - if (ACPI_SUCCESS(status)) { > - union acpi_object *obj = vgbs_output.pointer; > - > - if (obj && obj->type == ACPI_TYPE_INTEGER) { > - int m = !(obj->integer.value & TABLET_MODE_FLAG); > - > - input_report_switch(priv->input_dev, SW_TABLET_MODE, m); > - } > - } > - > - kfree(vgbs_output.pointer); > + detect_tablet_mode(device); > > status = acpi_install_notify_handler(handle, > ACPI_DEVICE_NOTIFY, > -- > 2.7.4 > Tested-by: Jeremy Cline <jeremy@jcline.org> Regards, Jeremy
On Thu, Feb 22, 2018 at 08:15:51PM +0000, Jeremy Cline wrote: > On Thu, Feb 22, 2018 at 01:58:42PM -0600, Mario Limonciello wrote: > > Some laptops such as the XPS 9360 support the intel-vbtn INT33D6 > > interface but don't initialize the bit that intel-vbtn uses to > > represent switching tablet mode. > > > > By running this only on real 2-in-1's it shouldn't cause false > > positives. > > > > Fixes: 30323fb6d5 ("Support tablet mode switch") > > Reported-by: Jeremy Cline <jeremy@jcline.org> > > Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> > > --- > > Note: I also moved the code for detecting tablet into it's own > > function. To keep things under 80 lines with if statements was > > making the code very unreadable otherwise. > > ... > > Tested-by: Jeremy Cline <jeremy@jcline.org> Wow, incredible turn around time Mario and Jeremy. Doing some confirmation on my end, and will queue up to Fixes for 4.16 RC cycle.
On Thu, Feb 22, 2018 at 08:15:51PM +0000, Jeremy Cline wrote: > On Thu, Feb 22, 2018 at 01:58:42PM -0600, Mario Limonciello wrote: > > Some laptops such as the XPS 9360 support the intel-vbtn INT33D6 > > interface but don't initialize the bit that intel-vbtn uses to > > represent switching tablet mode. > > > > By running this only on real 2-in-1's it shouldn't cause false > > positives. > > > > Tested-by: Jeremy Cline <jeremy@jcline.org> Problem verified, solution tested and also verified. Queuing patch.
diff --git a/drivers/platform/x86/intel-vbtn.c b/drivers/platform/x86/intel-vbtn.c index b703d6f..8173307 100644 --- a/drivers/platform/x86/intel-vbtn.c +++ b/drivers/platform/x86/intel-vbtn.c @@ -7,6 +7,7 @@ */ #include <linux/acpi.h> +#include <linux/dmi.h> #include <linux/input.h> #include <linux/input/sparse-keymap.h> #include <linux/kernel.h> @@ -97,9 +98,35 @@ static void notify_handler(acpi_handle handle, u32 event, void *context) dev_dbg(&device->dev, "unknown event index 0x%x\n", event); } -static int intel_vbtn_probe(struct platform_device *device) +static void detect_tablet_mode(struct platform_device *device) { + const char *chassis_type = dmi_get_system_info(DMI_CHASSIS_TYPE); + struct intel_vbtn_priv *priv = dev_get_drvdata(&device->dev); + acpi_handle handle = ACPI_HANDLE(&device->dev); struct acpi_buffer vgbs_output = { ACPI_ALLOCATE_BUFFER, NULL }; + union acpi_object *obj; + acpi_status status; + int m; + + if (!(chassis_type && strcmp(chassis_type, "31") == 0)) + goto out; + + status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output); + if (ACPI_FAILURE(status)) + goto out; + + obj = vgbs_output.pointer; + if (!(obj && obj->type == ACPI_TYPE_INTEGER)) + goto out; + + m = !(obj->integer.value & TABLET_MODE_FLAG); + input_report_switch(priv->input_dev, SW_TABLET_MODE, m); +out: + kfree(vgbs_output.pointer); +} + +static int intel_vbtn_probe(struct platform_device *device) +{ acpi_handle handle = ACPI_HANDLE(&device->dev); struct intel_vbtn_priv *priv; acpi_status status; @@ -122,22 +149,7 @@ static int intel_vbtn_probe(struct platform_device *device) return err; } - /* - * VGBS being present and returning something means we have - * a tablet mode switch. - */ - status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output); - if (ACPI_SUCCESS(status)) { - union acpi_object *obj = vgbs_output.pointer; - - if (obj && obj->type == ACPI_TYPE_INTEGER) { - int m = !(obj->integer.value & TABLET_MODE_FLAG); - - input_report_switch(priv->input_dev, SW_TABLET_MODE, m); - } - } - - kfree(vgbs_output.pointer); + detect_tablet_mode(device); status = acpi_install_notify_handler(handle, ACPI_DEVICE_NOTIFY,
Some laptops such as the XPS 9360 support the intel-vbtn INT33D6 interface but don't initialize the bit that intel-vbtn uses to represent switching tablet mode. By running this only on real 2-in-1's it shouldn't cause false positives. Fixes: 30323fb6d5 ("Support tablet mode switch") Reported-by: Jeremy Cline <jeremy@jcline.org> Signed-off-by: Mario Limonciello <mario.limonciello@dell.com> --- Note: I also moved the code for detecting tablet into it's own function. To keep things under 80 lines with if statements was making the code very unreadable otherwise. drivers/platform/x86/intel-vbtn.c | 46 ++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 17 deletions(-)