[0/3] Lenovo lap and palm sensor support

Message

Mark Pearson Oct. 15, 2020, 1:57 p.m. UTC

This patch series is to add support for the Lenovo lap and palm sensors. 
The original lap sensor implementation used a sysfs API but after
consultation with the kernel maintainers we agreed on using the input
subsystem instead.
The first patch just adds the new defines needed.
The second patch adds the implementation needed for the palm sensor.
The third patch adds the implementation needed for the lap sensor. 

This means currently thinkpad_acpi.c has both the sysfs and input dev
implementations. I will add a follow-on patch to remove the sysfs
interface once I've confirmed this is OK with the few people who are
using this in user space and given them some time to migrate to the
input dev implementation.

Mark Pearson (3):
  Adding event codes for Lenovo lap and palm sensors
  Add support for Lenovo palm sensor.
  Add support for Lenovo lap sensor

 drivers/platform/x86/thinkpad_acpi.c   | 144 ++++++++++++++++++++++++-
 include/linux/mod_devicetable.h        |   2 +-
 include/uapi/linux/input-event-codes.h |   4 +-
 3 files changed, 145 insertions(+), 5 deletions(-)

Comments

Hans de Goede Oct. 19, 2020, 6:49 p.m. UTC | #1

Hi,

On 10/15/20 3:57 PM, Mark Pearson wrote:
> This patch series is to add support for the Lenovo lap and palm sensors. 
> The original lap sensor implementation used a sysfs API but after
> consultation with the kernel maintainers we agreed on using the input
> subsystem instead.
> The first patch just adds the new defines needed.
> The second patch adds the implementation needed for the palm sensor.
> The third patch adds the implementation needed for the lap sensor. 
> 
> This means currently thinkpad_acpi.c has both the sysfs and input dev
> implementations. I will add a follow-on patch to remove the sysfs
> interface once I've confirmed this is OK with the few people who are
> using this in user space and given them some time to migrate to the
> input dev implementation.
> 
> Mark Pearson (3):
>   Adding event codes for Lenovo lap and palm sensors
>   Add support for Lenovo palm sensor.
>   Add support for Lenovo lap sensor

Not a full review, but one short remark, all your patch
subjects e.g. "Adding event codes for Lenovo lap and palm sensors"
are missing subsystem prefixes, if you do e.g.

git log include/uapi/linux/input-event-codes.h

You see subjects like "Input: allocate keycode for Fn + right shift",
etc. and for the the thinkpad_acpi.c code you get:
"platform/x86: thinkpad_acpi: Map Clipping tool hotkey to KEY_SELECTIVE_SCREENSHOT"

So your patch subjects should look something like this:

"Input: add event codes for lap and palmrest proximity switches"
(note I fixed more here then just the missing prefix)

"platform/x86: thinkpad_acpi: Add support for Lenovo palm sensor"
(note no . at the end)

"platform/x86: thinkpad_acpi: Add support for Lenovo lap sensor"

If you can send out a v2 with this fixed, that might help to
go Dmitry's attention for the first patch.

Regards,

Hans

Mark Pearson Oct. 20, 2020, 12:15 a.m. UTC | #2

On 19/10/2020 14:49, Hans de Goede wrote:
> Hi,
> 
> On 10/15/20 3:57 PM, Mark Pearson wrote:
>> This patch series is to add support for the Lenovo lap and palm sensors.
>> The original lap sensor implementation used a sysfs API but after
>> consultation with the kernel maintainers we agreed on using the input
>> subsystem instead.
>> The first patch just adds the new defines needed.
>> The second patch adds the implementation needed for the palm sensor.
>> The third patch adds the implementation needed for the lap sensor.
>>
>> This means currently thinkpad_acpi.c has both the sysfs and input dev
>> implementations. I will add a follow-on patch to remove the sysfs
>> interface once I've confirmed this is OK with the few people who are
>> using this in user space and given them some time to migrate to the
>> input dev implementation.
>>
>> Mark Pearson (3):
>>    Adding event codes for Lenovo lap and palm sensors
>>    Add support for Lenovo palm sensor.
>>    Add support for Lenovo lap sensor
> 
> Not a full review, but one short remark, all your patch
> subjects e.g. "Adding event codes for Lenovo lap and palm sensors"
> are missing subsystem prefixes, if you do e.g.
> 
> git log include/uapi/linux/input-event-codes.h
> 
> You see subjects like "Input: allocate keycode for Fn + right shift",
> etc. and for the the thinkpad_acpi.c code you get:
> "platform/x86: thinkpad_acpi: Map Clipping tool hotkey to KEY_SELECTIVE_SCREENSHOT"
> 
> So your patch subjects should look something like this:
> 
> "Input: add event codes for lap and palmrest proximity switches"
> (note I fixed more here then just the missing prefix)
> 
> "platform/x86: thinkpad_acpi: Add support for Lenovo palm sensor"
> (note no . at the end)
> 
> "platform/x86: thinkpad_acpi: Add support for Lenovo lap sensor"
> 
> If you can send out a v2 with this fixed, that might help to
> go Dmitry's attention for the first patch.
> 
> Regards,
> 
> Hans
> 
Thanks Hans,
I knew about that too detail, so slap on the wrist for me. I was too 
focused on doing a series of patches for the first time...

Updated version coming out shortly.

Mark
Mark