| Message ID | 20210521171418.393871-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | iio: accel: bmc150: Add support for yoga's with dual accelerometers with an ACPI HID of DUAL250E | expand |
On Fri, 21 May 2021 19:14:10 +0200 Hans de Goede <hdegoede@redhat.com> wrote: > Hi All, > > Some 360 degree hinges (yoga) style 2-in-1 devices use 2 bmc150 accels > 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. > > We already support this setup on devices using a single ACPI node > with a HID of "BOSC0200" to describe both accelerometers. This patch > set extends this support to also support the same setup but then > using a HID of "DUAL250E". > > While testing this I found some crashes on rmmod, patches 1-2 > fix those patches, patch 3 does some refactoring and patch 4 > adds support for the "DUAL250E" HID. > > Unfortunately we need some more special handling though, which the > rest of the patches are for. > > On Windows both accelerometers are read (polled) by a special service > and this service calls a DSM (Device Specific Method), which in turn > translates the angles to one of laptop/tablet/tent/stand mode and then > notifies the EC about the new mode and the EC then enables or disables > the builtin keyboard and touchpad based in the mode. > > When the 2-in-1 is powered-on or resumed folded in tablet mode the > EC senses this independent of the DSM by using a HALL effect sensor > which senses that the keyboard has been folded away behind the display. > > At power-on or resume the EC disables the keyboard based on this and > the only way to get the keyboard to work after this is to call the > DSM to re-enable it (similar to how we also need to call a special > DSM in the kxcjk-1013.c accel driver to re-enable the keyboard). > > Patches 5-7 deal with the DSM mess and patch 8 adds labels to the > 2 accelerometers specifying which one is which. Given only thing I'm planning to do is tweak the line wrapping, I'm happy to pick this series up. The two fixes will slow things down a bit though as we should probably get those upstream this cycle. I'm going to leave this on list for a few days before I take anything though, to give others time to take a look. One side note, cc list includes a few random choices... Seems you've accidentally included alsa people as well as IIO ones. Thanks Jonathan > > Regards, > > Hans > > > Hans de Goede (8): > iio: accel: bmc150: Fix dereferencing the wrong pointer in > bmc150_get/set_second_device > iio: accel: bmc150: Don't make the remove function of the second > accelerometer unregister itself > iio: accel: bmc150: Move check for second ACPI device into a separate > function > iio: accel: bmc150: Add support for dual-accelerometers with a > DUAL250E HID > iio: accel: bmc150: Move struct bmc150_accel_data definition to > bmc150-accel.h > iio: accel: bmc150: Remove bmc150_set/get_second_device() accessor > functions > iio: accel: bmc150: Add support for DUAL250E ACPI DSM for setting the > hinge angle > iio: accel: bmc150: Set label based on accel-location for ACPI > DUAL250E fwnodes > > drivers/iio/accel/bmc150-accel-core.c | 87 ++---------- > drivers/iio/accel/bmc150-accel-i2c.c | 192 +++++++++++++++++++++----- > drivers/iio/accel/bmc150-accel.h | 66 ++++++++- > 3 files changed, 239 insertions(+), 106 deletions(-) >
On Sat, May 22, 2021 at 9:00 PM Jonathan Cameron <jic23@kernel.org> wrote: > On Fri, 21 May 2021 19:14:10 +0200 > Hans de Goede <hdegoede@redhat.com> wrote: > > Some 360 degree hinges (yoga) style 2-in-1 devices use 2 bmc150 accels > > 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. > > > > We already support this setup on devices using a single ACPI node > > with a HID of "BOSC0200" to describe both accelerometers. This patch > > set extends this support to also support the same setup but then > > using a HID of "DUAL250E". > > > > While testing this I found some crashes on rmmod, patches 1-2 > > fix those patches, patch 3 does some refactoring and patch 4 > > adds support for the "DUAL250E" HID. > > > > Unfortunately we need some more special handling though, which the > > rest of the patches are for. > > > > On Windows both accelerometers are read (polled) by a special service > > and this service calls a DSM (Device Specific Method), which in turn > > translates the angles to one of laptop/tablet/tent/stand mode and then > > notifies the EC about the new mode and the EC then enables or disables > > the builtin keyboard and touchpad based in the mode. > > > > When the 2-in-1 is powered-on or resumed folded in tablet mode the > > EC senses this independent of the DSM by using a HALL effect sensor > > which senses that the keyboard has been folded away behind the display. > > > > At power-on or resume the EC disables the keyboard based on this and > > the only way to get the keyboard to work after this is to call the > > DSM to re-enable it (similar to how we also need to call a special > > DSM in the kxcjk-1013.c accel driver to re-enable the keyboard). > > > > Patches 5-7 deal with the DSM mess and patch 8 adds labels to the > > 2 accelerometers specifying which one is which. > > Given only thing I'm planning to do is tweak the line wrapping, I'm > happy to pick this series up. > > The two fixes will slow things down a bit though as we should probably > get those upstream this cycle. > > I'm going to leave this on list for a few days before I take anything > though, to give others time to take a look. You are, guys, too fast :-) I have few (minor) comments on a few patches, in general they are okay! So, after settling on the comments, Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> for the entire series. Thanks, Hans, for doing this! > One side note, cc list includes a few random choices... Seems you've > accidentally included alsa people as well as IIO ones.
On Sat, 22 May 2021 21:03:02 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Sat, May 22, 2021 at 9:00 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > On Fri, 21 May 2021 19:14:10 +0200 > > Hans de Goede <hdegoede@redhat.com> wrote: > > > > Some 360 degree hinges (yoga) style 2-in-1 devices use 2 bmc150 accels > > > 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. > > > > > > We already support this setup on devices using a single ACPI node > > > with a HID of "BOSC0200" to describe both accelerometers. This patch > > > set extends this support to also support the same setup but then > > > using a HID of "DUAL250E". > > > > > > While testing this I found some crashes on rmmod, patches 1-2 > > > fix those patches, patch 3 does some refactoring and patch 4 > > > adds support for the "DUAL250E" HID. > > > > > > Unfortunately we need some more special handling though, which the > > > rest of the patches are for. > > > > > > On Windows both accelerometers are read (polled) by a special service > > > and this service calls a DSM (Device Specific Method), which in turn > > > translates the angles to one of laptop/tablet/tent/stand mode and then > > > notifies the EC about the new mode and the EC then enables or disables > > > the builtin keyboard and touchpad based in the mode. > > > > > > When the 2-in-1 is powered-on or resumed folded in tablet mode the > > > EC senses this independent of the DSM by using a HALL effect sensor > > > which senses that the keyboard has been folded away behind the display. > > > > > > At power-on or resume the EC disables the keyboard based on this and > > > the only way to get the keyboard to work after this is to call the > > > DSM to re-enable it (similar to how we also need to call a special > > > DSM in the kxcjk-1013.c accel driver to re-enable the keyboard). > > > > > > Patches 5-7 deal with the DSM mess and patch 8 adds labels to the > > > 2 accelerometers specifying which one is which. > > > > Given only thing I'm planning to do is tweak the line wrapping, I'm > > happy to pick this series up. > > > > The two fixes will slow things down a bit though as we should probably > > get those upstream this cycle. > > > > I'm going to leave this on list for a few days before I take anything > > though, to give others time to take a look. > > You are, guys, too fast :-) One day did seem a bit short for a series doing as much as this one :) > > I have few (minor) comments on a few patches, in general they are okay! > So, after settling on the comments, > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > for the entire series. > > Thanks, Hans, for doing this! > > > One side note, cc list includes a few random choices... Seems you've > > accidentally included alsa people as well as IIO ones. > >
Hi, On 5/22/21 8:01 PM, Jonathan Cameron wrote: > On Fri, 21 May 2021 19:14:10 +0200 > Hans de Goede <hdegoede@redhat.com> wrote: > >> Hi All, >> >> Some 360 degree hinges (yoga) style 2-in-1 devices use 2 bmc150 accels >> 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. >> >> We already support this setup on devices using a single ACPI node >> with a HID of "BOSC0200" to describe both accelerometers. This patch >> set extends this support to also support the same setup but then >> using a HID of "DUAL250E". >> >> While testing this I found some crashes on rmmod, patches 1-2 >> fix those patches, patch 3 does some refactoring and patch 4 >> adds support for the "DUAL250E" HID. >> >> Unfortunately we need some more special handling though, which the >> rest of the patches are for. >> >> On Windows both accelerometers are read (polled) by a special service >> and this service calls a DSM (Device Specific Method), which in turn >> translates the angles to one of laptop/tablet/tent/stand mode and then >> notifies the EC about the new mode and the EC then enables or disables >> the builtin keyboard and touchpad based in the mode. >> >> When the 2-in-1 is powered-on or resumed folded in tablet mode the >> EC senses this independent of the DSM by using a HALL effect sensor >> which senses that the keyboard has been folded away behind the display. >> >> At power-on or resume the EC disables the keyboard based on this and >> the only way to get the keyboard to work after this is to call the >> DSM to re-enable it (similar to how we also need to call a special >> DSM in the kxcjk-1013.c accel driver to re-enable the keyboard). >> >> Patches 5-7 deal with the DSM mess and patch 8 adds labels to the >> 2 accelerometers specifying which one is which. > > Given only thing I'm planning to do is tweak the line wrapping, I'm > happy to pick this series up. > > The two fixes will slow things down a bit though as we should probably > get those upstream this cycle. > > I'm going to leave this on list for a few days before I take anything > though, to give others time to take a look. I'll do a v2 addressing a few minor things which Andy pointed out, I'll also take care of the comment re-wrap in the v2. > One side note, cc list includes a few random choices... Seems you've > accidentally included alsa people as well as IIO ones. You're right, I accidentally included the address-list which I use for ASoC patches. ASoc folks, sorry for the noise. Regards, Hans >> Hans de Goede (8): >> iio: accel: bmc150: Fix dereferencing the wrong pointer in >> bmc150_get/set_second_device >> iio: accel: bmc150: Don't make the remove function of the second >> accelerometer unregister itself >> iio: accel: bmc150: Move check for second ACPI device into a separate >> function >> iio: accel: bmc150: Add support for dual-accelerometers with a >> DUAL250E HID >> iio: accel: bmc150: Move struct bmc150_accel_data definition to >> bmc150-accel.h >> iio: accel: bmc150: Remove bmc150_set/get_second_device() accessor >> functions >> iio: accel: bmc150: Add support for DUAL250E ACPI DSM for setting the >> hinge angle >> iio: accel: bmc150: Set label based on accel-location for ACPI >> DUAL250E fwnodes >> >> drivers/iio/accel/bmc150-accel-core.c | 87 ++---------- >> drivers/iio/accel/bmc150-accel-i2c.c | 192 +++++++++++++++++++++----- >> drivers/iio/accel/bmc150-accel.h | 66 ++++++++- >> 3 files changed, 239 insertions(+), 106 deletions(-) >> >
Hi All, Some 360 degree hinges (yoga) style 2-in-1 devices use 2 bmc150 accels 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. We already support this setup on devices using a single ACPI node with a HID of "BOSC0200" to describe both accelerometers. This patch set extends this support to also support the same setup but then using a HID of "DUAL250E". While testing this I found some crashes on rmmod, patches 1-2 fix those patches, patch 3 does some refactoring and patch 4 adds support for the "DUAL250E" HID. Unfortunately we need some more special handling though, which the rest of the patches are for. On Windows both accelerometers are read (polled) by a special service and this service calls a DSM (Device Specific Method), which in turn translates the angles to one of laptop/tablet/tent/stand mode and then notifies the EC about the new mode and the EC then enables or disables the builtin keyboard and touchpad based in the mode. When the 2-in-1 is powered-on or resumed folded in tablet mode the EC senses this independent of the DSM by using a HALL effect sensor which senses that the keyboard has been folded away behind the display. At power-on or resume the EC disables the keyboard based on this and the only way to get the keyboard to work after this is to call the DSM to re-enable it (similar to how we also need to call a special DSM in the kxcjk-1013.c accel driver to re-enable the keyboard). Patches 5-7 deal with the DSM mess and patch 8 adds labels to the 2 accelerometers specifying which one is which. Regards, Hans Hans de Goede (8): iio: accel: bmc150: Fix dereferencing the wrong pointer in bmc150_get/set_second_device iio: accel: bmc150: Don't make the remove function of the second accelerometer unregister itself iio: accel: bmc150: Move check for second ACPI device into a separate function iio: accel: bmc150: Add support for dual-accelerometers with a DUAL250E HID iio: accel: bmc150: Move struct bmc150_accel_data definition to bmc150-accel.h iio: accel: bmc150: Remove bmc150_set/get_second_device() accessor functions iio: accel: bmc150: Add support for DUAL250E ACPI DSM for setting the hinge angle iio: accel: bmc150: Set label based on accel-location for ACPI DUAL250E fwnodes drivers/iio/accel/bmc150-accel-core.c | 87 ++---------- drivers/iio/accel/bmc150-accel-i2c.c | 192 +++++++++++++++++++++----- drivers/iio/accel/bmc150-accel.h | 66 ++++++++- 3 files changed, 239 insertions(+), 106 deletions(-)