Re: [PATCH] iio: accel: kxcjk1013: Add tablet_mode sysfs file for exercising the KIOX010A ACPI DSM

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 25 Nov 2020 11:55:45 +0100
Hans de Goede <hdegoede@redhat.com> wrote:

> Hi,
> 
> On 11/25/20 11:34 AM, Andy Shevchenko wrote:
> > On Wed, Nov 25, 2020 at 10:56 AM 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.
> >>
> >> Currently the kxcjk1013 driver calls the DSM for this once at probe time
> >> to ensure that the builtin kbd and touchpad work.
> >>
> >> But some users have expressed interest in using this functionality to
> >> disable the kbd and touchpad when folded into tablet-mode as done under
> >> Windows.
> >>
> >> Add a tablet_mode sysfs file so that users can control the kbd/touchpad
> >> enable/disable functionality from user-space.  

Biggest thing missing here is documentation.

Documentation/ABI/testing/sysfs-bus-iio-kxcjk1013

Unless this is documented somewhere else?  I've no idea if there is any
precedence for this/

> > 
> > ...
> >   
> >> +       err = kiox010a_dsm(&data->client->dev,
> >> +                          tablet_mode ? KIOX010A_SET_TABLET_MODE :
> >> +                                        KIOX010A_SET_LAPTOP_MODE);  
> > 
> > A nit. With temporary variable it may be slightly better to read, like:
> > 
> >   int value;
> >   ...
> >   value = tablet_mode ? KIOX010A_SET_TABLET_MODE : KIOX010A_SET_LAPTOP_MODE);
> >   err = kiox010a_dsm(&data->client->dev, value);  
> 
> I'm fine with either solution, Jonathan let me know if you want a v2 with
> Andy's suggestion implemented (assuming you are willing to take this at all).

Prefer Andy's suggestion slightly seeing as you are going around again to
include some docs :)

Thanks,

Jonathan