From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751637AbeAZLRF (ORCPT ); Fri, 26 Jan 2018 06:17:05 -0500 Received: from mail-wm0-f68.google.com ([74.125.82.68]:53542 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751545AbeAZLRB (ORCPT ); Fri, 26 Jan 2018 06:17:01 -0500 X-Google-Smtp-Source: AH8x226JX/dXp0iE+lgCAbXeUsHrFnu0qKeWafG9lHv4OUO+XMj+0bYKIoS8Q4WSfwDafP0Ut05G+w== Date: Fri, 26 Jan 2018 12:16:52 +0100 From: Pali =?utf-8?B?Um9ow6Fy?= To: Marco Martin Cc: Linux Kernel Mailing List , Matthew Garrett , Darren Hart , Andy Shevchenko , Bhushan Shah , Platform Driver , Mario Limonciello , Mario Limonciello Subject: Re: [PATCH v4] Support intel-vbtn based tablet mode switch Message-ID: <20180126111652.oze3os3bsi3p6qeb@pali> References: <1516720704-19831-1-git-send-email-notmart@gmail.com> <1914818.Wtl4XKE3JN@phobos> <20180126105233.yhowbofqfcx6yvdw@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Friday 26 January 2018 12:09:01 Marco Martin wrote: > On Fri, Jan 26, 2018 at 11:52 AM, Pali Rohár wrote: > > On Friday 26 January 2018 10:45:55 Marco Martin wrote: > >> On martedì 23 gennaio 2018 16:18:24 CET Marco Martin wrote: > >> > Some laptops such as Dell Inspiron 7000 series have the > >> > tablet mode switch implemented in Intel ACPI, > >> > the events to enter and exit the tablet mode are 0xCC and 0xCD > >> > > >> > CC: platform-driver-x86@vger.kernel.org > >> > CC: Matthew Garrett > >> > CC: "Pali Rohár" > >> > CC: Darren Hart > >> > CC: Mario Limonciello > >> > CC: Andy Shevchenko > >> > > >> > Signed-off-by: Marco Martin > >> > --- > >> > drivers/platform/x86/intel-vbtn.c | 21 +++++++++++++++++++++ > >> > 1 file changed, 21 insertions(+) > >> > > >> > diff --git a/drivers/platform/x86/intel-vbtn.c > >> > b/drivers/platform/x86/intel-vbtn.c index 58c5ff3..64b4b34 100644 > >> > --- a/drivers/platform/x86/intel-vbtn.c > >> > +++ b/drivers/platform/x86/intel-vbtn.c > >> > @@ -26,6 +26,9 @@ > >> > #include > >> > #include > >> > > >> > +/* When NOT in tablet mode, VBDS has the flag 0x40 */ > >> > +#define TABLET_MODE_FLAG 0x40 > >> > + > >> > MODULE_LICENSE("GPL"); > >> > MODULE_AUTHOR("AceLan Kao"); > >> > > >> > @@ -42,6 +45,8 @@ static const struct key_entry intel_vbtn_keymap[] = { > >> > { KE_IGNORE, 0xC5, { KEY_VOLUMEUP } }, /* volume-up key release */ > >> > { KE_KEY, 0xC6, { KEY_VOLUMEDOWN } }, /* volume-down key press */ > >> > { KE_IGNORE, 0xC7, { KEY_VOLUMEDOWN } }, /* volume-down key release */ > >> > + { KE_SW, 0xCC, { .sw = { SW_TABLET_MODE, 1 } } }, /* Tablet mode in */ > >> > + { KE_SW, 0xCD, { .sw = { SW_TABLET_MODE, 0 } } }, /* Tablet mode out */ > >> > { KE_END }, > >> > }; > >> > > >> > @@ -88,6 +93,7 @@ static void notify_handler(acpi_handle handle, u32 event, > >> > void *context) > >> > > >> > static int intel_vbtn_probe(struct platform_device *device) > >> > { > >> > + struct acpi_buffer vgbs_output = { ACPI_ALLOCATE_BUFFER, NULL }; > >> > acpi_handle handle = ACPI_HANDLE(&device->dev); > >> > struct intel_vbtn_priv *priv; > >> > acpi_status status; > >> > @@ -110,6 +116,21 @@ static int intel_vbtn_probe(struct platform_device > >> > *device) return err; > >> > } > >> > > >> > + status = acpi_evaluate_object(handle, "VGBS", NULL, &vgbs_output); > >> > + /* VGBS being present and returning something means > >> > + * we have a tablet mode switch > >> > + */ > >> > + if (ACPI_SUCCESS(status)) { > >> > + union acpi_object *obj = vgbs_output.pointer; > >> > + > >> > + if (obj && obj->type == ACPI_TYPE_INTEGER) { > >> > + input_set_capability(priv->input_dev, EV_SW, SW_TABLET_MODE); > >> > + input_report_switch(priv->input_dev, > >> > + SW_TABLET_MODE, > >> > + !(obj->integer.value & TABLET_MODE_FLAG)); > >> > + } > >> > + } > >> > + > >> > status = acpi_install_notify_handler(handle, > >> > ACPI_DEVICE_NOTIFY, > >> > notify_handler, > >> > >> Is there still something to change in this version of the patch? > > > > Yes, I already wrote it in thread for older patch version. Calling > > input_set_capability() is not needed at all because all capabilities are > > already set by sparse_keymap_setup() function. > > ah, sorry, i somehow missed the answer. > Wouldn't this cause a problem tough? it was querying VGBS to add the > switch capability just conditionally > on models which actually have it, so now being in sparse_keymap, all > will have that switch available even if not there. > Now, if this is fine (after all that switch would never be triggered > on such hardware) i can go forward removing all of that so the patch > becomes a neat two-liner. Looking at other drivers and I see that they send initial status of SW_TABLET_MODE at probe time. I do not know if this is correct or not. If yes, then you need to call that input_report_switch() as you have. If not, then then really just 2 line patch is enough. But in both cases, call to input_set_capability() is not needed. -- Pali Rohár pali.rohar@gmail.com