diff mbox series

HID: quirks: fix RetroUSB.com devices

Message ID 20181206165925.GA12842@jasnah (mailing list archive)
State Mainlined
Delegated to: Jiri Kosina
Headers show
Series HID: quirks: fix RetroUSB.com devices | expand

Commit Message

Nic Soudée Dec. 6, 2018, 4:59 p.m. UTC 
SNES RetroPort and RetroPad register only 4 gamepad buttons
when they should register all 8 buttons. This is described here:

https://ask.fedoraproject.org/en/question/128102

This is happening because of:

Commit 190d7f02ce8e ("HID: input: do not increment usages when
    duplicate is found")

Here, I add the quirk HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE
(created for backward compatibility with the change in 190d7f02ce8e)
for the two products.

Tested with both RetroPort and RetroPad.

Signed-off-by: Nic Soudée <nsoudee@gmail.com>
---
 drivers/hid/hid-ids.h    | 4 ++++
 drivers/hid/hid-quirks.c | 2 ++
 2 files changed, 6 insertions(+)

Comments

Benjamin Tissoires Dec. 6, 2018, 6:09 p.m. UTC | #1 
On Thu, Dec 6, 2018 at 5:59 PM Nic Soudée <nsoudee@gmail.com> wrote:
>
> SNES RetroPort and RetroPad register only 4 gamepad buttons
> when they should register all 8 buttons. This is described here:
>
> https://ask.fedoraproject.org/en/question/128102
>
> This is happening because of:
>
> Commit 190d7f02ce8e ("HID: input: do not increment usages when
>     duplicate is found")
>
> Here, I add the quirk HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE
> (created for backward compatibility with the change in 190d7f02ce8e)
> for the two products.
>
> Tested with both RetroPort and RetroPad.

Hi Nic,

Thanks for the patch. It looks good but I might need a couple of days
before I am able to apply it.

While I try to put my system back on its feet, can you provide me the
hid-recorder[1] outputs of these 2 devices?

I just want to be sure that there is not something I overlooked in the
mentioned patch and that a generic solution should not be used.

Cheers,
Benjamin

[1] http://bentiss.github.io/hid-replay-docs/

>
> Signed-off-by: Nic Soudée <nsoudee@gmail.com>
> ---
>  drivers/hid/hid-ids.h    | 4 ++++
>  drivers/hid/hid-quirks.c | 2 ++
>  2 files changed, 6 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index f63489c882bb..ba2f97fd54e5 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -933,6 +933,10 @@
>  #define USB_VENDOR_ID_REALTEK          0x0bda
>  #define USB_DEVICE_ID_REALTEK_READER   0x0152
>
> +#define USB_VENDOR_ID_RETROUSB         0xf000
> +#define USB_DEVICE_ID_RETROUSB_SNES_RETROPAD   0x0003
> +#define USB_DEVICE_ID_RETROUSB_SNES_RETROPORT  0x00f1
> +
>  #define USB_VENDOR_ID_ROCCAT           0x1e7d
>  #define USB_DEVICE_ID_ROCCAT_ARVO      0x30d4
>  #define USB_DEVICE_ID_ROCCAT_ISKU      0x319c
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 52c3b01917e7..a38f3e08efed 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -135,6 +135,8 @@ static const struct hid_device_id hid_quirks[] = {
>         { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3003), HID_QUIRK_NOGET },
>         { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008), HID_QUIRK_NOGET },
>         { HID_USB_DEVICE(USB_VENDOR_ID_REALTEK, USB_DEVICE_ID_REALTEK_READER), HID_QUIRK_NO_INIT_REPORTS },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPAD), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
> +       { HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPORT), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
>         { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD), HID_QUIRK_BADPAD },
>         { HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD2), HID_QUIRK_NO_INIT_REPORTS },
>         { HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD), HID_QUIRK_NO_INIT_REPORTS },
> --
> 2.19.2
>
>
Nic Soudée Dec. 6, 2018, 7:07 p.m. UTC | #2 
On Thu, Dec 06, 2018 at 07:09:06PM +0100, Benjamin Tissoires wrote:
> On Thu, Dec 6, 2018 at 5:59 PM Nic Soudée <nsoudee@gmail.com> wrote:
> >
> > SNES RetroPort and RetroPad register only 4 gamepad buttons
> > when they should register all 8 buttons. This is described here:
> >
> > https://ask.fedoraproject.org/en/question/128102
> >
> > This is happening because of:
> >
> > Commit 190d7f02ce8e ("HID: input: do not increment usages when
> >     duplicate is found")
> >
> > Here, I add the quirk HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE
> > (created for backward compatibility with the change in 190d7f02ce8e)
> > for the two products.
> >
> > Tested with both RetroPort and RetroPad.
> 
> Hi Nic,
> 
> Thanks for the patch. It looks good but I might need a couple of days
> before I am able to apply it.
> 
> While I try to put my system back on its feet, can you provide me the
> hid-recorder[1] outputs of these 2 devices?
> 
> I just want to be sure that there is not something I overlooked in the
> mentioned patch and that a generic solution should not be used.
> 
> Cheers,
> Benjamin
> 
> [1] http://bentiss.github.io/hid-replay-docs/
> 

Hi Benjamin,

Here are the outputs from hid-recorder:

RetroPad:

D: 0
R: 74 05 01 09 05 a1 01 09 01 a1 00 09 30 09 31 15 00 26 ff 00 35 00 46 ff 00 75 08 95 02 81 02 c0 05 09 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 c0
N: RetroUSB.com RetroPad
P: usb-0000:00:1d.0-1.1/input0
I: 3 f000 0003

RetroPort:

D: 0
R: 74 05 01 09 05 a1 01 09 01 a1 00 09 30 09 31 15 00 26 ff 00 35 00 46 ff 00 75 08 95 02 81 02 c0 05 09 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 c0
N: RetroUSB.com SNES RetroPort
P: usb-0000:00:1d.0-1.4.1/input0
I: 3 f000 00f1

Thanks,
Nic

> >
> > Signed-off-by: Nic Soudée <nsoudee@gmail.com>
> > ---
> >  drivers/hid/hid-ids.h    | 4 ++++
> >  drivers/hid/hid-quirks.c | 2 ++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > index f63489c882bb..ba2f97fd54e5 100644
> > --- a/drivers/hid/hid-ids.h
> > +++ b/drivers/hid/hid-ids.h
> > @@ -933,6 +933,10 @@
> >  #define USB_VENDOR_ID_REALTEK          0x0bda
> >  #define USB_DEVICE_ID_REALTEK_READER   0x0152
> >
> > +#define USB_VENDOR_ID_RETROUSB         0xf000
> > +#define USB_DEVICE_ID_RETROUSB_SNES_RETROPAD   0x0003
> > +#define USB_DEVICE_ID_RETROUSB_SNES_RETROPORT  0x00f1
> > +
> >  #define USB_VENDOR_ID_ROCCAT           0x1e7d
> >  #define USB_DEVICE_ID_ROCCAT_ARVO      0x30d4
> >  #define USB_DEVICE_ID_ROCCAT_ISKU      0x319c
> > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> > index 52c3b01917e7..a38f3e08efed 100644
> > --- a/drivers/hid/hid-quirks.c
> > +++ b/drivers/hid/hid-quirks.c
> > @@ -135,6 +135,8 @@ static const struct hid_device_id hid_quirks[] = {
> >         { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3003), HID_QUIRK_NOGET },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008), HID_QUIRK_NOGET },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_REALTEK, USB_DEVICE_ID_REALTEK_READER), HID_QUIRK_NO_INIT_REPORTS },
> > +       { HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPAD), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
> > +       { HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPORT), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD), HID_QUIRK_BADPAD },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD2), HID_QUIRK_NO_INIT_REPORTS },
> >         { HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD), HID_QUIRK_NO_INIT_REPORTS },
> > --
> > 2.19.2
> >
> >
Benjamin Tissoires Dec. 7, 2018, 1:06 p.m. UTC | #3 
On Thu, Dec 6, 2018 at 8:07 PM Nic Soudée <nsoudee@gmail.com> wrote:
>
> On Thu, Dec 06, 2018 at 07:09:06PM +0100, Benjamin Tissoires wrote:
> > On Thu, Dec 6, 2018 at 5:59 PM Nic Soudée <nsoudee@gmail.com> wrote:
> > >
> > > SNES RetroPort and RetroPad register only 4 gamepad buttons
> > > when they should register all 8 buttons. This is described here:
> > >
> > > https://ask.fedoraproject.org/en/question/128102
> > >
> > > This is happening because of:
> > >
> > > Commit 190d7f02ce8e ("HID: input: do not increment usages when
> > >     duplicate is found")
> > >
> > > Here, I add the quirk HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE
> > > (created for backward compatibility with the change in 190d7f02ce8e)
> > > for the two products.
> > >
> > > Tested with both RetroPort and RetroPad.
> >
> > Hi Nic,
> >
> > Thanks for the patch. It looks good but I might need a couple of days
> > before I am able to apply it.
> >
> > While I try to put my system back on its feet, can you provide me the
> > hid-recorder[1] outputs of these 2 devices?
> >
> > I just want to be sure that there is not something I overlooked in the
> > mentioned patch and that a generic solution should not be used.
> >
> > Cheers,
> > Benjamin
> >
> > [1] http://bentiss.github.io/hid-replay-docs/
> >
>
> Hi Benjamin,
>
> Here are the outputs from hid-recorder:
>
> RetroPad:
>
> D: 0
> R: 74 05 01 09 05 a1 01 09 01 a1 00 09 30 09 31 15 00 26 ff 00 35 00 46 ff 00 75 08 95 02 81 02 c0 05 09 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 c0
> N: RetroUSB.com RetroPad
> P: usb-0000:00:1d.0-1.1/input0
> I: 3 f000 0003
>
> RetroPort:
>
> D: 0
> R: 74 05 01 09 05 a1 01 09 01 a1 00 09 30 09 31 15 00 26 ff 00 35 00 46 ff 00 75 08 95 02 81 02 c0 05 09 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 19 01 29 04 15 00 25 01 75 01 95 04 81 02 75 01 95 04 81 03 c0
> N: RetroUSB.com SNES RetroPort
> P: usb-0000:00:1d.0-1.4.1/input0
> I: 3 f000 00f1
>

Thanks.
So for the record, this is indeed a situation where we need the quirk.
The report descriptors looks like:
0x05, 0x01,                    // Usage Page (Generic Desktop)        0
0x09, 0x05,                    // Usage (Game Pad)                    2
0xa1, 0x01,                    // Collection (Application)            4
0x09, 0x01,                    //  Usage (Pointer)                    6
0xa1, 0x00,                    //  Collection (Physical)              8
0x09, 0x30,                    //   Usage (X)                         10
0x09, 0x31,                    //   Usage (Y)                         12
0x15, 0x00,                    //   Logical Minimum (0)               14
0x26, 0xff, 0x00,              //   Logical Maximum (255)             16
0x35, 0x00,                    //   Physical Minimum (0)              19
0x46, 0xff, 0x00,              //   Physical Maximum (255)            21
0x75, 0x08,                    //   Report Size (8)                   24
0x95, 0x02,                    //   Report Count (2)                  26
0x81, 0x02,                    //   Input (Data,Var,Abs)              28
0xc0,                          //  End Collection                     30
0x05, 0x09,                    //  Usage Page (Button)                31
0x19, 0x01,                    //  Usage Minimum (1)                  33
0x29, 0x04,                    //  Usage Maximum (4)                  35
0x15, 0x00,                    //  Logical Minimum (0)                37
0x25, 0x01,                    //  Logical Maximum (1)                39
0x75, 0x01,                    //  Report Size (1)                    41
0x95, 0x04,                    //  Report Count (4)                   43
0x81, 0x02,                    //  Input (Data,Var,Abs)               45
0x75, 0x01,                    //  Report Size (1)                    47
0x95, 0x04,                    //  Report Count (4)                   49
0x81, 0x03,                    //  Input (Cnst,Var,Abs)               51
0x19, 0x01,                    //  Usage Minimum (1)                  53
0x29, 0x04,                    //  Usage Maximum (4)                  55
0x15, 0x00,                    //  Logical Minimum (0)                57
0x25, 0x01,                    //  Logical Maximum (1)                59
0x75, 0x01,                    //  Report Size (1)                    61
0x95, 0x04,                    //  Report Count (4)                   63
0x81, 0x02,                    //  Input (Data,Var,Abs)               65
0x75, 0x01,                    //  Report Size (1)                    67
0x95, 0x04,                    //  Report Count (4)                   69
0x81, 0x03,                    //  Input (Cnst,Var,Abs)               71
0xc0,                          // End Collection                      73

And the buttons are in the same collection, and both series are
reusing the same usage min/max.

I'll schedule the patch for 4.20 and Cc stable as soon as I finished
some sanity testing.

Cheers,
Benjamin

> Thanks,
> Nic
>
> > >
> > > Signed-off-by: Nic Soudée <nsoudee@gmail.com>
> > > ---
> > >  drivers/hid/hid-ids.h    | 4 ++++
> > >  drivers/hid/hid-quirks.c | 2 ++
> > >  2 files changed, 6 insertions(+)
> > >
> > > diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> > > index f63489c882bb..ba2f97fd54e5 100644
> > > --- a/drivers/hid/hid-ids.h
> > > +++ b/drivers/hid/hid-ids.h
> > > @@ -933,6 +933,10 @@
> > >  #define USB_VENDOR_ID_REALTEK          0x0bda
> > >  #define USB_DEVICE_ID_REALTEK_READER   0x0152
> > >
> > > +#define USB_VENDOR_ID_RETROUSB         0xf000
> > > +#define USB_DEVICE_ID_RETROUSB_SNES_RETROPAD   0x0003
> > > +#define USB_DEVICE_ID_RETROUSB_SNES_RETROPORT  0x00f1
> > > +
> > >  #define USB_VENDOR_ID_ROCCAT           0x1e7d
> > >  #define USB_DEVICE_ID_ROCCAT_ARVO      0x30d4
> > >  #define USB_DEVICE_ID_ROCCAT_ISKU      0x319c
> > > diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> > > index 52c3b01917e7..a38f3e08efed 100644
> > > --- a/drivers/hid/hid-quirks.c
> > > +++ b/drivers/hid/hid-quirks.c
> > > @@ -135,6 +135,8 @@ static const struct hid_device_id hid_quirks[] = {
> > >         { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3003), HID_QUIRK_NOGET },
> > >         { HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008), HID_QUIRK_NOGET },
> > >         { HID_USB_DEVICE(USB_VENDOR_ID_REALTEK, USB_DEVICE_ID_REALTEK_READER), HID_QUIRK_NO_INIT_REPORTS },
> > > +       { HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPAD), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
> > > +       { HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPORT), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
> > >         { HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD), HID_QUIRK_BADPAD },
> > >         { HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD2), HID_QUIRK_NO_INIT_REPORTS },
> > >         { HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD), HID_QUIRK_NO_INIT_REPORTS },
> > > --
> > > 2.19.2
> > >
> > >
diff mbox series

Patch

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f63489c882bb..ba2f97fd54e5 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -933,6 +933,10 @@ 
 #define USB_VENDOR_ID_REALTEK		0x0bda
 #define USB_DEVICE_ID_REALTEK_READER	0x0152
 
+#define USB_VENDOR_ID_RETROUSB		0xf000
+#define USB_DEVICE_ID_RETROUSB_SNES_RETROPAD	0x0003
+#define USB_DEVICE_ID_RETROUSB_SNES_RETROPORT	0x00f1
+
 #define USB_VENDOR_ID_ROCCAT		0x1e7d
 #define USB_DEVICE_ID_ROCCAT_ARVO	0x30d4
 #define USB_DEVICE_ID_ROCCAT_ISKU	0x319c
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 52c3b01917e7..a38f3e08efed 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -135,6 +135,8 @@  static const struct hid_device_id hid_quirks[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3003), HID_QUIRK_NOGET },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_QUANTA, USB_DEVICE_ID_QUANTA_OPTICAL_TOUCH_3008), HID_QUIRK_NOGET },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_REALTEK, USB_DEVICE_ID_REALTEK_READER), HID_QUIRK_NO_INIT_REPORTS },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPAD), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_RETROUSB, USB_DEVICE_ID_RETROUSB_SNES_RETROPORT), HID_QUIRK_INCREMENT_USAGE_ON_DUPLICATE },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SAITEK, USB_DEVICE_ID_SAITEK_RUMBLEPAD), HID_QUIRK_BADPAD },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD2), HID_QUIRK_NO_INIT_REPORTS },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_SEMICO, USB_DEVICE_ID_SEMICO_USB_KEYKOARD), HID_QUIRK_NO_INIT_REPORTS },