Hi, On 11/25/20 11:34 AM, Andy Shevchenko wrote: > On Wed, Nov 25, 2020 at 10:56 AM Hans de Goede <hdegoede@xxxxxxxxxx> 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. > > ... > >> + 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). > >> + if (err) >> + return err; > > ... > >> + ret = devm_device_add_group(&data->client->dev, &tablet_mode_attrs_group); >> + if (ret < 0) >> + dev_warn(&data->client->dev, "Error creating tablet_mode sysfs attribute\n"); > > devm is a beast (sometimes). Just to make sure that on removal you > won't have situation when attribute is still there while data it's > accessing to are already gone or garbage. The main data struct is also devm managed and allocated earlier, so this is not an issue here. Regards, Hans
