Re: [PATCH] HID: logitech-hidpp: connect Bluetooth devices before HID++ handshake

[Archie Pusaka]

Hi Maintainers,

Please take a look. Thank you!


On Tue, 27 Oct 2026 at 14:13, Archie Pusaka <apusaka@xxxxxxxxxx> wrote:
>
> hidpp_probe() opens the device and runs the HID++ handshake before
> calling hid_connect(), so that the HID++ name can be stored in
> hdev->name before hid-input and hidraw expose it. Opening the device
> tells the transport that I/O may start (for uhid, UHID_OPEN), but any
> non-HID++ input report received before hid_connect() has no consumer
> and is dropped. Over BLE the handshake takes several hundred ms, thus a
> reconnecting keyboard might lose the first keystrokes.
>
> Bluetooth devices already get their name and serial from the transport,
> so connect hid-input and hidraw before opening the device, like
> hid-generic effectively does. hidpp_connect_event() still fetches the
> HID++ name into hidpp->name, so the battery model name is unchanged.
>
> As a consequence, the input and hidraw nodes of Bluetooth devices are
> named after the transport-provided name rather than the "Logitech "-
> prefixed HID++ name. Non-Bluetooth (USB, Unifying/DJ) and DELAYED_INIT
> devices keep the original ordering and naming.
>
> Signed-off-by: Archie Pusaka <apusaka@xxxxxxxxxx>
> ---
>  drivers/hid/hid-logitech-hidpp.c | 67 ++++++++++++++++++++++----------
>  1 file changed, 47 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
> index 27d83b8bb..b552f31f0 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4673,6 +4673,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         struct hidpp_device *hidpp;
>         int ret;
>         unsigned int connect_mask = HID_CONNECT_DEFAULT;
> +       bool early_connect;
>
>         /* report_fixup needs drvdata to be set before we call hid_parse */
>         hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
> @@ -4734,12 +4735,31 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>                          hdev->name);
>
>         /*
> -        * First call hid_hw_start(hdev, 0) to allow IO without connecting any
> -        * hid subdrivers (hid-input, hidraw). This allows retrieving the dev's
> -        * name and serial number and store these in hdev->name and hdev->uniq,
> -        * before the hid-input and hidraw drivers expose these to userspace.
> +        * hid_hw_open() below tells the transport that I/O may start. However,
> +        * Any non-HID++ input report (e.g. regular keystrokes) received before
> +        * hid-input is connected will be dropped, and over BLE the HID++
> +        * handshake below takes several hundred milliseconds.
> +        *
> +        * Bluetooth devices already get their name and serial number from the
> +        * transport, so for them there is no need to delay exposing the input
> +        * and hidraw nodes until the handshake is done: connect them right
> +        * away, like hid-generic does. hidpp_connect_event() still fetches the
> +        * HID++ name into hidpp->name (used e.g. for the battery model name).
> +        *
> +        * DELAYED_INIT devices create their input device from
> +        * hidpp_connect_event() and keep the original ordering.
>          */
> -       ret = hid_hw_start(hdev, 0);
> +       early_connect = hdev->bus == BUS_BLUETOOTH &&
> +                       !(hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT);
> +
> +       /*
> +        * Otherwise, first call hid_hw_start(hdev, 0) to allow IO without
> +        * connecting any hid subdrivers (hid-input, hidraw). This allows
> +        * retrieving the dev's name and serial number and store these in
> +        * hdev->name and hdev->uniq, before the hid-input and hidraw drivers
> +        * expose these to userspace.
> +        */
> +       ret = hid_hw_start(hdev, early_connect ? connect_mask : 0);
>         if (ret) {
>                 hid_err(hdev, "hw start failed\n");
>                 goto hid_hw_start_fail;
> @@ -4755,25 +4775,32 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
>         /* Allow incoming packets */
>         hid_device_io_start(hdev);
>
> -       /* Get name + serial, store in hdev->name + hdev->uniq */
> -       if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
> -               hidpp_unifying_init(hidpp);
> -       else
> -               hidpp_non_unifying_init(hidpp);
> +       if (!early_connect) {
> +               /* Get name + serial, store in hdev->name + hdev->uniq */
> +               if (id->group == HID_GROUP_LOGITECH_DJ_DEVICE)
> +                       hidpp_unifying_init(hidpp);
> +               else
> +                       hidpp_non_unifying_init(hidpp);
>
> -       if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
> -               connect_mask &= ~HID_CONNECT_HIDINPUT;
> +               if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
> +                       connect_mask &= ~HID_CONNECT_HIDINPUT;
>
> -       /* Now export the actual inputs and hidraw nodes to the world */
> -       hid_device_io_stop(hdev);
> -       ret = hid_connect(hdev, connect_mask);
> -       if (ret) {
> -               hid_err(hdev, "%s:hid_connect returned error %d\n", __func__, ret);
> -               goto hid_hw_init_fail;
> +               /* Now export the actual inputs and hidraw nodes to the world */
> +               hid_device_io_stop(hdev);
> +               ret = hid_connect(hdev, connect_mask);
> +               if (ret) {
> +                       hid_err(hdev, "%s:hid_connect returned error %d\n",
> +                               __func__, ret);
> +                       goto hid_hw_init_fail;
> +               }
> +
> +               /*
> +                * Check for connected devices now that incoming packets will
> +                * not be disabled again
> +                */
> +               hid_device_io_start(hdev);
>         }
>
> -       /* Check for connected devices now that incoming packets will not be disabled again */
> -       hid_device_io_start(hdev);
>         schedule_work(&hidpp->work);
>         flush_work(&hidpp->work);
>
>
> base-commit: 2c2fe95ab19b09bef7391e3e5cdf8146d09792b6
> --
> 2.56.0.307.g5fe7bd680c-goog
>