Re: [PATCH 2/2] HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [Low] The patch unconditionally adds the KEY_PERFORMANCE capability to all input nodes exposed by the device, causing capability pollution on unrelated interfaces.

Pre-existing issues:
- [High] The raw event handler emits keys like KEY_RFKILL, KEY_BOOKMARKS, and KEY_KBDILLUMTOGGLE, but their capabilities are never declared, causing the input subsystem to silently drop these events.
- [High] The raw event handler contains a loop that unconditionally fires key events on the first input node it encounters, ignoring capability routing and causing events to be dropped.
--

commit 1b40b44e417a26a21650fec78e6a2ea55f535004
Author: Vishnu Sankar <vishnuocv@gmail.com>

HID: lenovo: Use KEY_PERFORMANCE capability for ThinkPad X12 Tab Gen 2

This commit adds the KEY_PERFORMANCE capability for the ThinkPad X12 Tab Gen 2
and X13 Tab devices during input configuration. This allows userspace tools to
discover the key emitted via the raw event handler.

> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 35b00619f0456..3492ba8d0fb02 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -1525,6 +1525,7 @@ static int lenovo_input_configured(struct hid_device *hdev,
>  						hi->input->propbit);
>  			}
>  			break;
> +		case USB_DEVICE_ID_LENOVO_X12_TAB2:
>  		case USB_DEVICE_ID_LENOVO_X13_TAB:
>  			input_set_capability(hi->input, EV_KEY, KEY_PERFORMANCE);

[Severity: Low]
Does this unconditionally add the KEY_PERFORMANCE capability to all input
nodes exposed by the device?

Since lenovo_input_configured() is invoked for every hid_input interface,
this regression might advertise the capability on unrelated interfaces.

Could we filter this by checking the application usage or verifying it is
the primary keyboard node?

[Severity: High]
This isn't a bug introduced by this patch, but I noticed the raw event
handler also emits keys like KEY_RFKILL, KEY_PICKUP_PHONE, KEY_BOOKMARKS,
and KEY_KBDILLUMTOGGLE.

Should we also declare the capabilities for these keys here?

Since they are generated from raw reports rather than standard HID
descriptors, they appear to be missing from the capability bits, which would
cause the input subsystem to silently drop these events.

[Severity: High]
This is a pre-existing issue, but looking at how the raw events are handled
in lenovo_raw_event_TP_X12_tab(), is it possible the events are firing on
the wrong input node?

The handler loops over hdev->inputs but returns immediately after reporting
the key event on the first node it encounters. If the first device lacks the
specific key capabilities, the input core might discard the event.

Should the loop instead locate the specific input node that possesses the
corresponding key capability before dispatching?

>  			break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522050632.8079-1-vishnuocv@gmail.com?part=2