mbox series

[0/8] iio: accel: bmc150: Add support for yoga's with dual accelerometers with an ACPI HID of DUAL250E

Message ID 20210521171418.393871-1-hdegoede@redhat.com
Headers show
Series iio: accel: bmc150: Add support for yoga's with dual accelerometers with an ACPI HID of DUAL250E | expand

Message

Hans de Goede May 21, 2021, 5:14 p.m. UTC
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(-)

Comments

Jonathan Cameron May 22, 2021, 6:01 p.m. UTC | #1
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(-)
>
Hans de Goede May 23, 2021, 11:05 a.m. UTC | #2
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(-)
>>
>