[PATCH 1/3] Revert "HID: logitech-hidpp: add a module parameter to keep firmware gestures"

[PATCH 1/3] Revert "HID: logitech-hidpp: add a module parameter to keep firmware gestures"

[Bastien Nocera]

Now that we're in 2022, and the majority of desktop environments can and
should support touchpad gestures through libinput, remove the legacy
module parameter that made it possible to use gestures implemented in
firmware.

This will eventually allow simplifying the driver's initialisation code.

This reverts commit 9188dbaed68a4b23dc96eba165265c08caa7dc2a.
---
 drivers/hid/hid-logitech-hidpp.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 08ad19097e9e..7f9187201913 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -32,11 +32,6 @@ MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
 MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
 
-static bool disable_raw_mode;
-module_param(disable_raw_mode, bool, 0644);
-MODULE_PARM_DESC(disable_raw_mode,
-	"Disable Raw mode reporting for touchpads and keep firmware gestures.");
-
 static bool disable_tap_to_click;
 module_param(disable_tap_to_click, bool, 0644);
 MODULE_PARM_DESC(disable_tap_to_click,
@@ -4355,11 +4350,6 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	    hidpp_application_equals(hdev, HID_GD_KEYBOARD))
 		hidpp->quirks |= HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS;
 
-	if (disable_raw_mode) {
-		hidpp->quirks &= ~HIDPP_QUIRK_CLASS_WTP;
-		hidpp->quirks &= ~HIDPP_QUIRK_NO_HIDINPUT;
-	}
-
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
 		ret = wtp_allocate(hdev, id);
 		if (ret)
-- 
2.38.1

[PATCH 2/3] HID: logitech-hidpp: Don't restart communication if not necessary

[Bastien Nocera]

Don't stop and restart communication with the device unless we need to
modify the connect flags used because of a device quirk.
---
 drivers/hid/hid-logitech-hidpp.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 7f9187201913..b4e4a8c79c75 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4310,6 +4310,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	bool connected;
 	unsigned int connect_mask = HID_CONNECT_DEFAULT;
 	struct hidpp_ff_private_data data;
+	bool will_restart = false;
 
 	/* report_fixup needs drvdata to be set before we call hid_parse */
 	hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
@@ -4360,6 +4361,9 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			return ret;
 	}
 
+	if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
+		will_restart = true;
+
 	INIT_WORK(&hidpp->work, delayed_work_cb);
 	mutex_init(&hidpp->send_mutex);
 	init_waitqueue_head(&hidpp->wait);
@@ -4374,7 +4378,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	 * Plain USB connections need to actually call start and open
 	 * on the transport driver to allow incoming data.
 	 */
-	ret = hid_hw_start(hdev, 0);
+	ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
 	if (ret) {
 		hid_err(hdev, "hw start failed\n");
 		goto hid_hw_start_fail;
@@ -4411,6 +4415,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 			hidpp->wireless_feature_index = 0;
 		else if (ret)
 			goto hid_hw_init_fail;
+		ret = 0;
 	}
 
 	if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
@@ -4425,19 +4430,21 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	hidpp_connect_event(hidpp);
 
-	/* Reset the HID node state */
-	hid_device_io_stop(hdev);
-	hid_hw_close(hdev);
-	hid_hw_stop(hdev);
+	if (will_restart) {
+		/* Reset the HID node state */
+		hid_device_io_stop(hdev);
+		hid_hw_close(hdev);
+		hid_hw_stop(hdev);
 
-	if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
-		connect_mask &= ~HID_CONNECT_HIDINPUT;
+		if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
+			connect_mask &= ~HID_CONNECT_HIDINPUT;
 
-	/* Now export the actual inputs and hidraw nodes to the world */
-	ret = hid_hw_start(hdev, connect_mask);
-	if (ret) {
-		hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
-		goto hid_hw_start_fail;
+		/* Now export the actual inputs and hidraw nodes to the world */
+		ret = hid_hw_start(hdev, connect_mask);
+		if (ret) {
+			hid_err(hdev, "%s:hid_hw_start returned error\n", __func__);
+			goto hid_hw_start_fail;
+		}
 	}
 
 	if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
-- 
2.38.1

Re: [PATCH 2/3] HID: logitech-hidpp: Don't restart communication if not necessary

[Bastien Nocera]

On Tue, 2022-12-20 at 10:22 +0100, Bastien Nocera wrote:
> Don't stop and restart communication with the device unless we need
> to
> modify the connect flags used because of a device quirk.

FIWW, Andreas Bergmeier told me off-list that this fixed their problem
with the Litra Glow not connecting properly.

Would be great to have reviews on this and my other HID++ patches.

Cheers

> ---
>  drivers/hid/hid-logitech-hidpp.c | 31 +++++++++++++++++++-----------
> -
>  1 file changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 7f9187201913..b4e4a8c79c75 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4310,6 +4310,7 @@ static int hidpp_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
>         bool connected;
>         unsigned int connect_mask = HID_CONNECT_DEFAULT;
>         struct hidpp_ff_private_data data;
> +       bool will_restart = false;
>  
>         /* report_fixup needs drvdata to be set before we call
> hid_parse */
>         hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
> @@ -4360,6 +4361,9 @@ static int hidpp_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
>                         return ret;
>         }
>  
> +       if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
> +               will_restart = true;
> +
>         INIT_WORK(&hidpp->work, delayed_work_cb);
>         mutex_init(&hidpp->send_mutex);
>         init_waitqueue_head(&hidpp->wait);
> @@ -4374,7 +4378,7 @@ static int hidpp_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
>          * Plain USB connections need to actually call start and open
>          * on the transport driver to allow incoming data.
>          */
> -       ret = hid_hw_start(hdev, 0);
> +       ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
>         if (ret) {
>                 hid_err(hdev, "hw start failed\n");
>                 goto hid_hw_start_fail;
> @@ -4411,6 +4415,7 @@ static int hidpp_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
>                         hidpp->wireless_feature_index = 0;
>                 else if (ret)
>                         goto hid_hw_init_fail;
> +               ret = 0;
>         }
>  
>         if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
> @@ -4425,19 +4430,21 @@ static int hidpp_probe(struct hid_device
> *hdev, const struct hid_device_id *id)
>  
>         hidpp_connect_event(hidpp);
>  
> -       /* Reset the HID node state */
> -       hid_device_io_stop(hdev);
> -       hid_hw_close(hdev);
> -       hid_hw_stop(hdev);
> +       if (will_restart) {
> +               /* Reset the HID node state */
> +               hid_device_io_stop(hdev);
> +               hid_hw_close(hdev);
> +               hid_hw_stop(hdev);
>  
> -       if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> -               connect_mask &= ~HID_CONNECT_HIDINPUT;
> +               if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> +                       connect_mask &= ~HID_CONNECT_HIDINPUT;
>  
> -       /* Now export the actual inputs and hidraw nodes to the world
> */
> -       ret = hid_hw_start(hdev, connect_mask);
> -       if (ret) {
> -               hid_err(hdev, "%s:hid_hw_start returned error\n",
> __func__);
> -               goto hid_hw_start_fail;
> +               /* Now export the actual inputs and hidraw nodes to
> the world */
> +               ret = hid_hw_start(hdev, connect_mask);
> +               if (ret) {
> +                       hid_err(hdev, "%s:hid_hw_start returned
> error\n", __func__);
> +                       goto hid_hw_start_fail;
> +               }
>         }
>  
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {

Re: [PATCH 2/3] HID: logitech-hidpp: Don't restart communication if not necessary

[Benjamin Tissoires]

On Tue, Jan 24, 2023 at 6:20 PM Bastien Nocera <hadess@hadess.net> wrote:
>
> On Tue, 2022-12-20 at 10:22 +0100, Bastien Nocera wrote:
> > Don't stop and restart communication with the device unless we need
> > to
> > modify the connect flags used because of a device quirk.
>
> FIWW, Andreas Bergmeier told me off-list that this fixed their problem
> with the Litra Glow not connecting properly.
>
> Would be great to have reviews on this and my other HID++ patches.

Sigh. I reviewed the patches just now (well, v2 at least), and thought
I better give a shot at it before merging, and it turns out that this
patch breaks the Unifying receivers.

Without it, each device presented to the user space has a proper name:

logitech-hidpp-device 0003:046D:4041.001C: input,hidraw15: USB HID
v1.11 Keyboard [Logitech MX Master] on usb-0000:01:00.0-4/input2:5

But with it, I get:

logitech-hidpp-device 0003:046D:4041.0024: input,hidraw8: USB HID
v1.11 Keyboard [Logitech Wireless Device PID:4041] on
usb-0000:00:14.0-8.2.4/input2:5

This is because we present the device to the userspace before being
able to fetch the name from the receiver.

I think we should make that connect/disconnect a special case of the
receivers too. Or maybe if the bus is not Bluetooth or USB, do the
disconnect/reconnect.

Cheers,
Benjamin

>
> Cheers
>
> > ---
> >  drivers/hid/hid-logitech-hidpp.c | 31 +++++++++++++++++++-----------
> > -
> >  1 file changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> > logitech-hidpp.c
> > index 7f9187201913..b4e4a8c79c75 100644
> > --- a/drivers/hid/hid-logitech-hidpp.c
> > +++ b/drivers/hid/hid-logitech-hidpp.c
> > @@ -4310,6 +4310,7 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> >         bool connected;
> >         unsigned int connect_mask = HID_CONNECT_DEFAULT;
> >         struct hidpp_ff_private_data data;
> > +       bool will_restart = false;
> >
> >         /* report_fixup needs drvdata to be set before we call
> > hid_parse */
> >         hidpp = devm_kzalloc(&hdev->dev, sizeof(*hidpp), GFP_KERNEL);
> > @@ -4360,6 +4361,9 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> >                         return ret;
> >         }
> >
> > +       if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
> > +               will_restart = true;
> > +
> >         INIT_WORK(&hidpp->work, delayed_work_cb);
> >         mutex_init(&hidpp->send_mutex);
> >         init_waitqueue_head(&hidpp->wait);
> > @@ -4374,7 +4378,7 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> >          * Plain USB connections need to actually call start and open
> >          * on the transport driver to allow incoming data.
> >          */
> > -       ret = hid_hw_start(hdev, 0);
> > +       ret = hid_hw_start(hdev, will_restart ? 0 : connect_mask);
> >         if (ret) {
> >                 hid_err(hdev, "hw start failed\n");
> >                 goto hid_hw_start_fail;
> > @@ -4411,6 +4415,7 @@ static int hidpp_probe(struct hid_device *hdev,
> > const struct hid_device_id *id)
> >                         hidpp->wireless_feature_index = 0;
> >                 else if (ret)
> >                         goto hid_hw_init_fail;
> > +               ret = 0;
> >         }
> >
> >         if (connected && (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP)) {
> > @@ -4425,19 +4430,21 @@ static int hidpp_probe(struct hid_device
> > *hdev, const struct hid_device_id *id)
> >
> >         hidpp_connect_event(hidpp);
> >
> > -       /* Reset the HID node state */
> > -       hid_device_io_stop(hdev);
> > -       hid_hw_close(hdev);
> > -       hid_hw_stop(hdev);
> > +       if (will_restart) {
> > +               /* Reset the HID node state */
> > +               hid_device_io_stop(hdev);
> > +               hid_hw_close(hdev);
> > +               hid_hw_stop(hdev);
> >
> > -       if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> > -               connect_mask &= ~HID_CONNECT_HIDINPUT;
> > +               if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
> > +                       connect_mask &= ~HID_CONNECT_HIDINPUT;
> >
> > -       /* Now export the actual inputs and hidraw nodes to the world
> > */
> > -       ret = hid_hw_start(hdev, connect_mask);
> > -       if (ret) {
> > -               hid_err(hdev, "%s:hid_hw_start returned error\n",
> > __func__);
> > -               goto hid_hw_start_fail;
> > +               /* Now export the actual inputs and hidraw nodes to
> > the world */
> > +               ret = hid_hw_start(hdev, connect_mask);
> > +               if (ret) {
> > +                       hid_err(hdev, "%s:hid_hw_start returned
> > error\n", __func__);
> > +                       goto hid_hw_start_fail;
> > +               }
> >         }
> >
> >         if (hidpp->quirks & HIDPP_QUIRK_CLASS_G920) {
>

Re: [PATCH 2/3] HID: logitech-hidpp: Don't restart communication if not necessary

[Bastien Nocera]

On Wed, 2023-01-25 at 11:18 +0100, Benjamin Tissoires wrote:
> On Tue, Jan 24, 2023 at 6:20 PM Bastien Nocera <hadess@hadess.net>
> wrote:
> > 
> > On Tue, 2022-12-20 at 10:22 +0100, Bastien Nocera wrote:
> > > Don't stop and restart communication with the device unless we
> > > need
> > > to
> > > modify the connect flags used because of a device quirk.
> > 
> > FIWW, Andreas Bergmeier told me off-list that this fixed their
> > problem
> > with the Litra Glow not connecting properly.
> > 
> > Would be great to have reviews on this and my other HID++ patches.
> 
> Sigh. I reviewed the patches just now (well, v2 at least), and
> thought
> I better give a shot at it before merging, and it turns out that this
> patch breaks the Unifying receivers.
> 
> Without it, each device presented to the user space has a proper
> name:
> 
> logitech-hidpp-device 0003:046D:4041.001C: input,hidraw15: USB HID
> v1.11 Keyboard [Logitech MX Master] on usb-0000:01:00.0-4/input2:5
> 
> But with it, I get:
> 
> logitech-hidpp-device 0003:046D:4041.0024: input,hidraw8: USB HID
> v1.11 Keyboard [Logitech Wireless Device PID:4041] on
> usb-0000:00:14.0-8.2.4/input2:5
> 
> This is because we present the device to the userspace before being
> able to fetch the name from the receiver.
> 
> I think we should make that connect/disconnect a special case of the
> receivers too. Or maybe if the bus is not Bluetooth or USB, do the
> disconnect/reconnect.

From what I can tell, this would mean restarting the connection in case
hidpp_unifying_init() did anything, right?

I'll test this out and update the patch.

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 4547e9580101..e0c28257f598 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4392,8 +4392,10 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
        /* Allow incoming packets */
        hid_device_io_start(hdev);
 
-       if (hidpp->quirks & HIDPP_QUIRK_UNIFYING)
-               hidpp_unifying_init(hidpp);
+       if (hidpp->quirks & HIDPP_QUIRK_UNIFYING) {
+               if (hidpp_unifying_init(hidpp) == 0)
+                       will_restart = true;
+       }
 
        connected = hidpp_root_get_protocol_version(hidpp) == 0;
        atomic_set(&hidpp->connected, connected);

[PATCH 3/3] HID: logitech-hidpp: Remove HIDPP_QUIRK_NO_HIDINPUT quirk

[Bastien Nocera]

HIDPP_QUIRK_NO_HIDINPUT isn't used by any devices but still happens to
work as HIDPP_QUIRK_DELAYED_INIT is defined to the same value. Remove
HIDPP_QUIRK_NO_HIDINPUT and use HIDPP_QUIRK_DELAYED_INIT everywhere
instead.

Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
 drivers/hid/hid-logitech-hidpp.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index b4e4a8c79c75..2092fb1be627 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -67,7 +67,7 @@ MODULE_PARM_DESC(disable_tap_to_click,
 /* bits 2..20 are reserved for classes */
 /* #define HIDPP_QUIRK_CONNECT_EVENTS		BIT(21) disabled */
 #define HIDPP_QUIRK_WTP_PHYSICAL_BUTTONS	BIT(22)
-#define HIDPP_QUIRK_NO_HIDINPUT			BIT(23)
+#define HIDPP_QUIRK_DELAYED_INIT		BIT(23)
 #define HIDPP_QUIRK_FORCE_OUTPUT_REPORTS	BIT(24)
 #define HIDPP_QUIRK_UNIFYING			BIT(25)
 #define HIDPP_QUIRK_HIDPP_WHEELS		BIT(26)
@@ -83,8 +83,6 @@ MODULE_PARM_DESC(disable_tap_to_click,
 					 HIDPP_CAPABILITY_HIDPP20_HI_RES_SCROLL | \
 					 HIDPP_CAPABILITY_HIDPP20_HI_RES_WHEEL)
 
-#define HIDPP_QUIRK_DELAYED_INIT		HIDPP_QUIRK_NO_HIDINPUT
-
 #define HIDPP_CAPABILITY_HIDPP10_BATTERY	BIT(0)
 #define HIDPP_CAPABILITY_HIDPP20_BATTERY	BIT(1)
 #define HIDPP_CAPABILITY_BATTERY_MILEAGE	BIT(2)
@@ -4205,7 +4203,7 @@ static void hidpp_connect_event(struct hidpp_device *hidpp)
 	if (hidpp->capabilities & HIDPP_CAPABILITY_HI_RES_SCROLL)
 		hi_res_scroll_enable(hidpp);
 
-	if (!(hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT) || hidpp->delayed_input)
+	if (!(hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT) || hidpp->delayed_input)
 		/* if the input nodes are already created, we can stop now */
 		return;
 
@@ -4436,7 +4434,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hid_hw_close(hdev);
 		hid_hw_stop(hdev);
 
-		if (hidpp->quirks & HIDPP_QUIRK_NO_HIDINPUT)
+		if (hidpp->quirks & HIDPP_QUIRK_DELAYED_INIT)
 			connect_mask &= ~HID_CONNECT_HIDINPUT;
 
 		/* Now export the actual inputs and hidraw nodes to the world */
-- 
2.38.1

Re: [PATCH 1/3] Revert "HID: logitech-hidpp: add a module parameter to keep firmware gestures"

[Bastien Nocera]

On Tue, 2022-12-20 at 10:22 +0100, Bastien Nocera wrote:
> Now that we're in 2022, and the majority of desktop environments can
> and
> should support touchpad gestures through libinput, remove the legacy
> module parameter that made it possible to use gestures implemented in
> firmware.
> 
> This will eventually allow simplifying the driver's initialisation
> code.
> 
> This reverts commit 9188dbaed68a4b23dc96eba165265c08caa7dc2a.

Forgot the signed-off-by, resent as v2.

> ---
>  drivers/hid/hid-logitech-hidpp.c | 10 ----------
>  1 file changed, 10 deletions(-)
> 
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 08ad19097e9e..7f9187201913 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -32,11 +32,6 @@ MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Benjamin Tissoires <benjamin.tissoires@gmail.com>");
>  MODULE_AUTHOR("Nestor Lopez Casado <nlopezcasad@logitech.com>");
>  
> -static bool disable_raw_mode;
> -module_param(disable_raw_mode, bool, 0644);
> -MODULE_PARM_DESC(disable_raw_mode,
> -       "Disable Raw mode reporting for touchpads and keep firmware
> gestures.");
> -
>  static bool disable_tap_to_click;
>  module_param(disable_tap_to_click, bool, 0644);
>  MODULE_PARM_DESC(disable_tap_to_click,
> @@ -4355,11 +4350,6 @@ static int hidpp_probe(struct hid_device
> *hdev, const struct hid_device_id *id)
>             hidpp_application_equals(hdev, HID_GD_KEYBOARD))
>                 hidpp->quirks |=
> HIDPP_QUIRK_HIDPP_CONSUMER_VENDOR_KEYS;
>  
> -       if (disable_raw_mode) {
> -               hidpp->quirks &= ~HIDPP_QUIRK_CLASS_WTP;
> -               hidpp->quirks &= ~HIDPP_QUIRK_NO_HIDINPUT;
> -       }
> -
>         if (hidpp->quirks & HIDPP_QUIRK_CLASS_WTP) {
>                 ret = wtp_allocate(hdev, id);
>                 if (ret)