hid: lenovo: Resend all settings on reset_resume for compact keyboards

From: Jamie Lentin <jm@lentin.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@vger.kernel.org
Fixes: 94eefa271323 ("HID: lenovo: Use native middle-button mode for compact keyboards")
Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
Signed-off-by: Martin Kepplinger <martink@posteo.de>
---
 drivers/hid/hid-lenovo.c | 50 +++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 16 deletions(-)

Am Montag, dem 02.10.2023 um 15:09 +0000 schrieb Martin Kepplinger:
> From: Jamie Lentin <jm@lentin.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@vger.kernel.org
> Fixes: 94eefa271323 ("HID: lenovo: Use native middle-button mode for
> compact keyboards")
> Signed-off-by: Jamie Lentin <jm@lentin.co.uk>
> Signed-off-by: Martin Kepplinger <martink@posteo.de>
> ---

ok who could review and possibly queue this? This fixes a pretty
annoying bug and makes the Keyboard usable after resuming from system
suspend. Jiri or Benjamin? Should I add Dmitry?

thanks a lot,

                         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);
>

On Mon, 02 Oct 2023 15:09:14 +0000, Martin Kepplinger wrote:
> 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.
> 
> [...]

Applied to hid/hid.git (for-6.7/lenovo), thanks!

[1/1] hid: lenovo: Resend all settings on reset_resume for compact keyboards
      https://git.kernel.org/hid/hid/c/2f2bd7cbd1d1

Cheers,