linux-kernel - Re: [PATCH] hid: lenovo: Resend all settings on reset

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <e32494e6e797af74cc01be99465302b711140111.camel@posteo.de>
Date:   Tue, 24 Oct 2023 08:55:22 +0000
From:   Martin Kepplinger <martink@...teo.de>
To:     jikos@...nel.org, benjamin.tissoires@...hat.com, jm@...tin.co.uk,
        linux-kernel@...r.kernel.org
Cc:     linux-input@...r.kernel.org, stable@...r.kernel.org,
        mail@...nhard-seibold.de, hdegoede@...hat.com, iam@...dikss.org.ru
Subject: Re: [PATCH] hid: lenovo: Resend all settings on reset_resume for
 compact keyboards

Am Montag, dem 02.10.2023 um 15:09 +0000 schrieb Martin Kepplinger:
> From: Jamie Lentin <jm@...tin.co.uk>
> 
> The USB Compact Keyboard variant requires a reset_resume function to
> restore keyboard configuration after a suspend in some situations.
> Move
> configuration normally done on probe to lenovo_features_set_cptkbd(),
> then
> recycle this for use on reset_resume.
> 
> Without, the keyboard and driver would end up in an inconsistent
> state,
> breaking middle-button scrolling amongst other problems, and
> twiddling
> sysfs values wouldn't help as the middle-button mode won't be set
> until
> the driver is reloaded.
> 
> Tested on a USB and Bluetooth Thinkpad Compact Keyboard.
> 
> CC: stable@...r.kernel.org
> Fixes: 94eefa271323 ("HID: lenovo: Use native middle-button mode for
> compact keyboards")
> Signed-off-by: Jamie Lentin <jm@...tin.co.uk>
> Signed-off-by: Martin Kepplinger <martink@...teo.de>

This is sitting over 3 weeks and I simply add Bernhard and Hans who
wrote big parts of the driver. Maybe more review can help with queuing
this bugfix up? (scrolling and function keys are currently broken after
resuming)

I basically sent Jamie's patch because I have the hardware:
https://lore.kernel.org/all/20231002150914.22101-1-martink@posteo.de/

thanks,
                               martin


> ---
>  drivers/hid/hid-lenovo.c | 50 +++++++++++++++++++++++++++-----------
> --
>  1 file changed, 34 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
> index 44763c0da444..614320bff39f 100644
> --- a/drivers/hid/hid-lenovo.c
> +++ b/drivers/hid/hid-lenovo.c
> @@ -521,6 +521,19 @@ static void lenovo_features_set_cptkbd(struct
> hid_device *hdev)
>         int ret;
>         struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
>  
> +       /*
> +        * Tell the keyboard a driver understands it, and turn F7,
> F9, F11 into
> +        * regular keys
> +        */
> +       ret = lenovo_send_cmd_cptkbd(hdev, 0x01, 0x03);
> +       if (ret)
> +               hid_warn(hdev, "Failed to switch F7/9/11 mode: %d\n",
> ret);
> +
> +       /* Switch middle button to native mode */
> +       ret = lenovo_send_cmd_cptkbd(hdev, 0x09, 0x01);
> +       if (ret)
> +               hid_warn(hdev, "Failed to switch middle button:
> %d\n", ret);
> +
>         ret = lenovo_send_cmd_cptkbd(hdev, 0x05, cptkbd_data-
> >fn_lock);
>         if (ret)
>                 hid_err(hdev, "Fn-lock setting failed: %d\n", ret);
> @@ -1126,22 +1139,6 @@ static int lenovo_probe_cptkbd(struct
> hid_device *hdev)
>         }
>         hid_set_drvdata(hdev, cptkbd_data);
>  
> -       /*
> -        * Tell the keyboard a driver understands it, and turn F7,
> F9, F11 into
> -        * regular keys (Compact only)
> -        */
> -       if (hdev->product == USB_DEVICE_ID_LENOVO_CUSBKBD ||
> -           hdev->product == USB_DEVICE_ID_LENOVO_CBTKBD) {
> -               ret = lenovo_send_cmd_cptkbd(hdev, 0x01, 0x03);
> -               if (ret)
> -                       hid_warn(hdev, "Failed to switch F7/9/11
> mode: %d\n", ret);
> -       }
> -
> -       /* Switch middle button to native mode */
> -       ret = lenovo_send_cmd_cptkbd(hdev, 0x09, 0x01);
> -       if (ret)
> -               hid_warn(hdev, "Failed to switch middle button:
> %d\n", ret);
> -
>         /* Set keyboard settings to known state */
>         cptkbd_data->middlebutton_state = 0;
>         cptkbd_data->fn_lock = true;
> @@ -1264,6 +1261,24 @@ static int lenovo_probe(struct hid_device
> *hdev,
>         return ret;
>  }
>  
> +#ifdef CONFIG_PM
> +static int lenovo_reset_resume(struct hid_device *hdev)
> +{
> +       switch (hdev->product) {
> +       case USB_DEVICE_ID_LENOVO_CUSBKBD:
> +       case USB_DEVICE_ID_LENOVO_TPIIUSBKBD:
> +               if (hdev->type == HID_TYPE_USBMOUSE)
> +                       lenovo_features_set_cptkbd(hdev);
> +
> +               break;
> +       default:
> +               break;
> +       }
> +
> +       return 0;
> +}
> +#endif
> +
>  static void lenovo_remove_tpkbd(struct hid_device *hdev)
>  {
>         struct lenovo_drvdata *data_pointer = hid_get_drvdata(hdev);
> @@ -1380,6 +1395,9 @@ static struct hid_driver lenovo_driver = {
>         .raw_event = lenovo_raw_event,
>         .event = lenovo_event,
>         .report_fixup = lenovo_report_fixup,
> +#ifdef CONFIG_PM
> +       .reset_resume = lenovo_reset_resume,
> +#endif
>  };
>  module_hid_driver(lenovo_driver);
>