| Message ID | 20201110133835.129080-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: accel: kxcjk1013: Fix kbd/touchpad not working on some 2-in-1s | expand |
On Tue, 10 Nov 2020 14:38:35 +0100 Hans de Goede <hdegoede@redhat.com> wrote: > Some 360 degree hinges (yoga) style 2-in-1 devices use 2 KXCJ91008-s > to allow the OS to determine the angle between the display and the base > of the device, so that the OS can determine if the 2-in-1 is in laptop > or in tablet-mode. > > On Windows both accelerometers are read by a special HingeAngleService > process; and this process calls a DSM (Device Specific Method) on the > ACPI KIOX010A device node for the sensor in the display, to let the > embedded-controller (EC) know about the mode so that it can disable the > kbd and touchpad to avoid spurious input while folded into tablet-mode. > > This notifying of the EC is problematic because sometimes the EC comes up > thinking that device is in tablet-mode and the kbd and touchpad do not > work. This happens for example on Irbis NB111 devices after a suspend / > resume cycle (after a complete battery drain / hard reset without having > booted Windows at least once). Other 2-in-1s which are likely affected > too are e.g. the Teclast F5 and F6 series. > > The kxcjk-1013 driver may seem like a strange place to deal with this, > but since it is *the* driver for the ACPI KIOX010A device, it is also > the driver which has access to the ACPI handle needed by the DSM. > > Add support for calling the DSM and on probe unconditionally tell the > EC that the device is laptop mode, fixing the kbd and touchpad sometimes > not working. > > Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Hi Hans, *Mutters darkly about crazy firmware hacks* I'm fine taking this but I assume we want to backport and for that I'm after a fixes tag. Thanks, Jonathan > --- > drivers/iio/accel/kxcjk-1013.c | 36 ++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c > index abeb0d254046..560a3373ff20 100644 > --- a/drivers/iio/accel/kxcjk-1013.c > +++ b/drivers/iio/accel/kxcjk-1013.c > @@ -129,6 +129,7 @@ enum kx_chipset { > enum kx_acpi_type { > ACPI_GENERIC, > ACPI_SMO8500, > + ACPI_KIOX010A, > }; > > struct kxcjk1013_data { > @@ -275,6 +276,32 @@ static const struct { > {19163, 1, 0}, > {38326, 0, 1} }; > > +#ifdef CONFIG_ACPI > +enum kiox010a_fn_index { > + KIOX010A_SET_LAPTOP_MODE = 1, > + KIOX010A_SET_TABLET_MODE = 2, > +}; > + > +static int kiox010a_dsm(struct device *dev, int fn_index) > +{ > + acpi_handle handle = ACPI_HANDLE(dev); > + guid_t kiox010a_dsm_guid; > + union acpi_object *obj; > + > + if (!handle) > + return -ENODEV; > + > + guid_parse("1f339696-d475-4e26-8cad-2e9f8e6d7a91", &kiox010a_dsm_guid); > + > + obj = acpi_evaluate_dsm(handle, &kiox010a_dsm_guid, 1, fn_index, NULL); > + if (!obj) > + return -EIO; > + > + ACPI_FREE(obj); > + return 0; > +} > +#endif > + > static int kxcjk1013_set_mode(struct kxcjk1013_data *data, > enum kxcjk1013_mode mode) > { > @@ -352,6 +379,13 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data) > { > int ret; > > +#ifdef CONFIG_ACPI > + if (data->acpi_type == ACPI_KIOX010A) { > + /* Make sure the kbd and touchpad on 2-in-1s using 2 KXCJ91008-s work */ > + kiox010a_dsm(&data->client->dev, KIOX010A_SET_LAPTOP_MODE); > + } > +#endif > + > ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_WHO_AM_I); > if (ret < 0) { > dev_err(&data->client->dev, "Error reading who_am_i\n"); > @@ -1262,6 +1296,8 @@ static const char *kxcjk1013_match_acpi_device(struct device *dev, > > if (strcmp(id->id, "SMO8500") == 0) > *acpi_type = ACPI_SMO8500; > + else if (strcmp(id->id, "KIOX010A") == 0) > + *acpi_type = ACPI_KIOX010A; > > *chipset = (enum kx_chipset)id->driver_data; >
Hi, On 11/14/20 5:01 PM, Jonathan Cameron wrote: > On Tue, 10 Nov 2020 14:38:35 +0100 > Hans de Goede <hdegoede@redhat.com> wrote: > >> Some 360 degree hinges (yoga) style 2-in-1 devices use 2 KXCJ91008-s >> to allow the OS to determine the angle between the display and the base >> of the device, so that the OS can determine if the 2-in-1 is in laptop >> or in tablet-mode. >> >> On Windows both accelerometers are read by a special HingeAngleService >> process; and this process calls a DSM (Device Specific Method) on the >> ACPI KIOX010A device node for the sensor in the display, to let the >> embedded-controller (EC) know about the mode so that it can disable the >> kbd and touchpad to avoid spurious input while folded into tablet-mode. >> >> This notifying of the EC is problematic because sometimes the EC comes up >> thinking that device is in tablet-mode and the kbd and touchpad do not >> work. This happens for example on Irbis NB111 devices after a suspend / >> resume cycle (after a complete battery drain / hard reset without having >> booted Windows at least once). Other 2-in-1s which are likely affected >> too are e.g. the Teclast F5 and F6 series. >> >> The kxcjk-1013 driver may seem like a strange place to deal with this, >> but since it is *the* driver for the ACPI KIOX010A device, it is also >> the driver which has access to the ACPI handle needed by the DSM. >> >> Add support for calling the DSM and on probe unconditionally tell the >> EC that the device is laptop mode, fixing the kbd and touchpad sometimes >> not working. >> >> Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > Hi Hans, > > *Mutters darkly about crazy firmware hacks* > > I'm fine taking this but I assume we want to backport and for that I'm > after a fixes tag. Good point, I guess taking the commit which originally added the KIOX010A ACPI Hardware-ID makes the most sense: Fixes: 7f6232e69539 ("iio: accel: kxcjk1013: Add KIOX010A ACPI Hardware-ID") Regards, Hans >> --- >> drivers/iio/accel/kxcjk-1013.c | 36 ++++++++++++++++++++++++++++++++++ >> 1 file changed, 36 insertions(+) >> >> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c >> index abeb0d254046..560a3373ff20 100644 >> --- a/drivers/iio/accel/kxcjk-1013.c >> +++ b/drivers/iio/accel/kxcjk-1013.c >> @@ -129,6 +129,7 @@ enum kx_chipset { >> enum kx_acpi_type { >> ACPI_GENERIC, >> ACPI_SMO8500, >> + ACPI_KIOX010A, >> }; >> >> struct kxcjk1013_data { >> @@ -275,6 +276,32 @@ static const struct { >> {19163, 1, 0}, >> {38326, 0, 1} }; >> >> +#ifdef CONFIG_ACPI >> +enum kiox010a_fn_index { >> + KIOX010A_SET_LAPTOP_MODE = 1, >> + KIOX010A_SET_TABLET_MODE = 2, >> +}; >> + >> +static int kiox010a_dsm(struct device *dev, int fn_index) >> +{ >> + acpi_handle handle = ACPI_HANDLE(dev); >> + guid_t kiox010a_dsm_guid; >> + union acpi_object *obj; >> + >> + if (!handle) >> + return -ENODEV; >> + >> + guid_parse("1f339696-d475-4e26-8cad-2e9f8e6d7a91", &kiox010a_dsm_guid); >> + >> + obj = acpi_evaluate_dsm(handle, &kiox010a_dsm_guid, 1, fn_index, NULL); >> + if (!obj) >> + return -EIO; >> + >> + ACPI_FREE(obj); >> + return 0; >> +} >> +#endif >> + >> static int kxcjk1013_set_mode(struct kxcjk1013_data *data, >> enum kxcjk1013_mode mode) >> { >> @@ -352,6 +379,13 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data) >> { >> int ret; >> >> +#ifdef CONFIG_ACPI >> + if (data->acpi_type == ACPI_KIOX010A) { >> + /* Make sure the kbd and touchpad on 2-in-1s using 2 KXCJ91008-s work */ >> + kiox010a_dsm(&data->client->dev, KIOX010A_SET_LAPTOP_MODE); >> + } >> +#endif >> + >> ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_WHO_AM_I); >> if (ret < 0) { >> dev_err(&data->client->dev, "Error reading who_am_i\n"); >> @@ -1262,6 +1296,8 @@ static const char *kxcjk1013_match_acpi_device(struct device *dev, >> >> if (strcmp(id->id, "SMO8500") == 0) >> *acpi_type = ACPI_SMO8500; >> + else if (strcmp(id->id, "KIOX010A") == 0) >> + *acpi_type = ACPI_KIOX010A; >> >> *chipset = (enum kx_chipset)id->driver_data; >> >
On Sat, 14 Nov 2020 18:07:38 +0100 Hans de Goede <hdegoede@redhat.com> wrote: > Hi, > > On 11/14/20 5:01 PM, Jonathan Cameron wrote: > > On Tue, 10 Nov 2020 14:38:35 +0100 > > Hans de Goede <hdegoede@redhat.com> wrote: > > > >> Some 360 degree hinges (yoga) style 2-in-1 devices use 2 KXCJ91008-s > >> to allow the OS to determine the angle between the display and the base > >> of the device, so that the OS can determine if the 2-in-1 is in laptop > >> or in tablet-mode. > >> > >> On Windows both accelerometers are read by a special HingeAngleService > >> process; and this process calls a DSM (Device Specific Method) on the > >> ACPI KIOX010A device node for the sensor in the display, to let the > >> embedded-controller (EC) know about the mode so that it can disable the > >> kbd and touchpad to avoid spurious input while folded into tablet-mode. > >> > >> This notifying of the EC is problematic because sometimes the EC comes up > >> thinking that device is in tablet-mode and the kbd and touchpad do not > >> work. This happens for example on Irbis NB111 devices after a suspend / > >> resume cycle (after a complete battery drain / hard reset without having > >> booted Windows at least once). Other 2-in-1s which are likely affected > >> too are e.g. the Teclast F5 and F6 series. > >> > >> The kxcjk-1013 driver may seem like a strange place to deal with this, > >> but since it is *the* driver for the ACPI KIOX010A device, it is also > >> the driver which has access to the ACPI handle needed by the DSM. > >> > >> Add support for calling the DSM and on probe unconditionally tell the > >> EC that the device is laptop mode, fixing the kbd and touchpad sometimes > >> not working. > >> > >> Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru> > >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > Hi Hans, > > > > *Mutters darkly about crazy firmware hacks* > > > > I'm fine taking this but I assume we want to backport and for that I'm > > after a fixes tag. > > Good point, I guess taking the commit which originally added the > KIOX010A ACPI Hardware-ID makes the most sense: > > Fixes: 7f6232e69539 ("iio: accel: kxcjk1013: Add KIOX010A ACPI Hardware-ID") Agreed. Applied to the fixes-togreg branch of iio.git and marked for stable. Thanks, Jonathan > > Regards, > > Hans > > > > >> --- > >> drivers/iio/accel/kxcjk-1013.c | 36 ++++++++++++++++++++++++++++++++++ > >> 1 file changed, 36 insertions(+) > >> > >> diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c > >> index abeb0d254046..560a3373ff20 100644 > >> --- a/drivers/iio/accel/kxcjk-1013.c > >> +++ b/drivers/iio/accel/kxcjk-1013.c > >> @@ -129,6 +129,7 @@ enum kx_chipset { > >> enum kx_acpi_type { > >> ACPI_GENERIC, > >> ACPI_SMO8500, > >> + ACPI_KIOX010A, > >> }; > >> > >> struct kxcjk1013_data { > >> @@ -275,6 +276,32 @@ static const struct { > >> {19163, 1, 0}, > >> {38326, 0, 1} }; > >> > >> +#ifdef CONFIG_ACPI > >> +enum kiox010a_fn_index { > >> + KIOX010A_SET_LAPTOP_MODE = 1, > >> + KIOX010A_SET_TABLET_MODE = 2, > >> +}; > >> + > >> +static int kiox010a_dsm(struct device *dev, int fn_index) > >> +{ > >> + acpi_handle handle = ACPI_HANDLE(dev); > >> + guid_t kiox010a_dsm_guid; > >> + union acpi_object *obj; > >> + > >> + if (!handle) > >> + return -ENODEV; > >> + > >> + guid_parse("1f339696-d475-4e26-8cad-2e9f8e6d7a91", &kiox010a_dsm_guid); > >> + > >> + obj = acpi_evaluate_dsm(handle, &kiox010a_dsm_guid, 1, fn_index, NULL); > >> + if (!obj) > >> + return -EIO; > >> + > >> + ACPI_FREE(obj); > >> + return 0; > >> +} > >> +#endif > >> + > >> static int kxcjk1013_set_mode(struct kxcjk1013_data *data, > >> enum kxcjk1013_mode mode) > >> { > >> @@ -352,6 +379,13 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data) > >> { > >> int ret; > >> > >> +#ifdef CONFIG_ACPI > >> + if (data->acpi_type == ACPI_KIOX010A) { > >> + /* Make sure the kbd and touchpad on 2-in-1s using 2 KXCJ91008-s work */ > >> + kiox010a_dsm(&data->client->dev, KIOX010A_SET_LAPTOP_MODE); > >> + } > >> +#endif > >> + > >> ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_WHO_AM_I); > >> if (ret < 0) { > >> dev_err(&data->client->dev, "Error reading who_am_i\n"); > >> @@ -1262,6 +1296,8 @@ static const char *kxcjk1013_match_acpi_device(struct device *dev, > >> > >> if (strcmp(id->id, "SMO8500") == 0) > >> *acpi_type = ACPI_SMO8500; > >> + else if (strcmp(id->id, "KIOX010A") == 0) > >> + *acpi_type = ACPI_KIOX010A; > >> > >> *chipset = (enum kx_chipset)id->driver_data; > >> > > >
diff --git a/drivers/iio/accel/kxcjk-1013.c b/drivers/iio/accel/kxcjk-1013.c index abeb0d254046..560a3373ff20 100644 --- a/drivers/iio/accel/kxcjk-1013.c +++ b/drivers/iio/accel/kxcjk-1013.c @@ -129,6 +129,7 @@ enum kx_chipset { enum kx_acpi_type { ACPI_GENERIC, ACPI_SMO8500, + ACPI_KIOX010A, }; struct kxcjk1013_data { @@ -275,6 +276,32 @@ static const struct { {19163, 1, 0}, {38326, 0, 1} }; +#ifdef CONFIG_ACPI +enum kiox010a_fn_index { + KIOX010A_SET_LAPTOP_MODE = 1, + KIOX010A_SET_TABLET_MODE = 2, +}; + +static int kiox010a_dsm(struct device *dev, int fn_index) +{ + acpi_handle handle = ACPI_HANDLE(dev); + guid_t kiox010a_dsm_guid; + union acpi_object *obj; + + if (!handle) + return -ENODEV; + + guid_parse("1f339696-d475-4e26-8cad-2e9f8e6d7a91", &kiox010a_dsm_guid); + + obj = acpi_evaluate_dsm(handle, &kiox010a_dsm_guid, 1, fn_index, NULL); + if (!obj) + return -EIO; + + ACPI_FREE(obj); + return 0; +} +#endif + static int kxcjk1013_set_mode(struct kxcjk1013_data *data, enum kxcjk1013_mode mode) { @@ -352,6 +379,13 @@ static int kxcjk1013_chip_init(struct kxcjk1013_data *data) { int ret; +#ifdef CONFIG_ACPI + if (data->acpi_type == ACPI_KIOX010A) { + /* Make sure the kbd and touchpad on 2-in-1s using 2 KXCJ91008-s work */ + kiox010a_dsm(&data->client->dev, KIOX010A_SET_LAPTOP_MODE); + } +#endif + ret = i2c_smbus_read_byte_data(data->client, KXCJK1013_REG_WHO_AM_I); if (ret < 0) { dev_err(&data->client->dev, "Error reading who_am_i\n"); @@ -1262,6 +1296,8 @@ static const char *kxcjk1013_match_acpi_device(struct device *dev, if (strcmp(id->id, "SMO8500") == 0) *acpi_type = ACPI_SMO8500; + else if (strcmp(id->id, "KIOX010A") == 0) + *acpi_type = ACPI_KIOX010A; *chipset = (enum kx_chipset)id->driver_data;
Some 360 degree hinges (yoga) style 2-in-1 devices use 2 KXCJ91008-s to allow the OS to determine the angle between the display and the base of the device, so that the OS can determine if the 2-in-1 is in laptop or in tablet-mode. On Windows both accelerometers are read by a special HingeAngleService process; and this process calls a DSM (Device Specific Method) on the ACPI KIOX010A device node for the sensor in the display, to let the embedded-controller (EC) know about the mode so that it can disable the kbd and touchpad to avoid spurious input while folded into tablet-mode. This notifying of the EC is problematic because sometimes the EC comes up thinking that device is in tablet-mode and the kbd and touchpad do not work. This happens for example on Irbis NB111 devices after a suspend / resume cycle (after a complete battery drain / hard reset without having booted Windows at least once). Other 2-in-1s which are likely affected too are e.g. the Teclast F5 and F6 series. The kxcjk-1013 driver may seem like a strange place to deal with this, but since it is *the* driver for the ACPI KIOX010A device, it is also the driver which has access to the ACPI handle needed by the DSM. Add support for calling the DSM and on probe unconditionally tell the EC that the device is laptop mode, fixing the kbd and touchpad sometimes not working. Reported-and-tested-by: russianneuromancer <russianneuromancer@ya.ru> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/iio/accel/kxcjk-1013.c | 36 ++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)