lkml.org 
[lkml]   [2023]   [Sep]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH 2/2] hid: lenovo: move type checks to lenovo_features_set_cptkbd()
On 2023-09-25 11:23, Martin Kepplinger wrote:
> These custom commands will be sent to both the USB keyboard & mouse
> devices but only the mouse will respond. Avoid sending known-useless
> messages by always prepending the filter before sending them.
>
> Suggested-by: Jamie Lentin <jm@lentin.co.uk>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
> ---
> drivers/hid/hid-lenovo.c | 27 +++++++++------------------
> 1 file changed, 9 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 29aa6d372bad..922f3e5462f4 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -521,6 +521,14 @@ static void lenovo_features_set_cptkbd(struct
> hid_device *hdev)
> int ret;
> struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
>
> + /* All the custom action happens on the USBMOUSE device for USB */
> + if (((hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD) ||
> + (hdev->product == USB_DEVICE_ID_LENOVO_TPIIUSBKBD)) &&
> + hdev->type != HID_TYPE_USBMOUSE) {
> + hid_dbg(hdev, "Ignoring keyboard half of device\n");
> + return;
> + }
> +
> /*
> * Tell the keyboard a driver understands it, and turn F7, F9, F11
> into
> * regular keys
> @@ -1122,14 +1130,6 @@ static int lenovo_probe_cptkbd(struct hid_device
> *hdev)
> int ret;
> struct lenovo_drvdata *cptkbd_data;
>
> - /* All the custom action happens on the USBMOUSE device for USB */
> - if (((hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD) ||
> - (hdev->product == USB_DEVICE_ID_LENOVO_TPIIUSBKBD)) &&
> - hdev->type != HID_TYPE_USBMOUSE) {
> - hid_dbg(hdev, "Ignoring keyboard half of device\n");
> - return 0;
> - }
> -

I like the idea of doing it once then forgetting about it, but removing
this will mean that the "keyboard half" will have it's own set of
non-functional sysfs parameters I think? Currently:-

# evtest
. . .
/dev/input/event10: ThinkPad Compact Bluetooth Keyboard with
TrackPoint
/dev/input/event11: Lenovo ThinkPad Compact USB Keyboard with
TrackPoint
/dev/input/event12: Lenovo ThinkPad Compact USB Keyboard with
TrackPoint

# ls -1 /sys/class/input/event*/device/device/fn_lock
/sys/class/input/event10/device/device/fn_lock
/sys/class/input/event12/device/device/fn_lock

(note 11 is missing.)

I think the easiest (but ugly) thing to do is to copy-paste this lump of
code to the top of lenovo_reset_resume.

Cheers,

> cptkbd_data = devm_kzalloc(&hdev->dev,
> sizeof(*cptkbd_data),
> GFP_KERNEL);
> @@ -1264,16 +1264,7 @@ static int lenovo_probe(struct hid_device *hdev,
> #ifdef CONFIG_PM
> static int lenovo_reset_resume(struct hid_device *hdev)
> {
> - switch (hdev->product) {
> - case USB_DEVICE_ID_LENOVO_CUSBKBD:
> - if (hdev->type == HID_TYPE_USBMOUSE) {
> - lenovo_features_set_cptkbd(hdev);
> - }
> -
> - break;
> - default:
> - break;
> - }
> + lenovo_features_set_cptkbd(hdev);
>
> return 0;
> }

\
 
 \ /
  Last update: 2023-09-27 18:51    [W:0.044 / U:0.268 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site