mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Rosalie <rosalie@mailbox.org>
To: Brenton Simpson <appsforartists@google.com>
Cc: "Jiri Kosina" <jikos@kernel.org>,
	"Benjamin Tissoires" <bentiss@kernel.org>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	"Sanjay Govind" <sanjay.govind9@gmail.com>,
	"Antheas Kapenekakis" <lkml@antheas.dev>,
	"Vicki Pfau" <vi@endrift.com>,
	"Nícolas F . R . A . Prado" <nfraprado@collabora.com>
Subject: Re: [PATCH] HID: sony: add support for Rock Band 2 instruments
Date: Sat, 21 Feb 2026 06:26:07 +0100	[thread overview]
Message-ID: <6bf7ba2f-bfe1-4988-b15f-9a42f4546b91@mailbox.org> (raw)
In-Reply-To: <CAAL3-=-Cbi+xL2wXKG7vsjMgh8iX8chA+rzkXk_R80cpQF5vNw@mail.gmail.com>

On 2/21/26 06:04, Brenton Simpson wrote:
> On Fri, Feb 20, 2026 at 9:42 PM Rosalie <rosalie@mailbox.org> wrote:
> 
>> On 20/02/2026 17:50, Brenton Simpson <appsforartists@google.com> wrote:
>>
>>> Rock Band 2 for the Nintendo Wii and for the Sony PlayStation 3 use the
>>> same mapping as later games:
>>>
>>>       Green:              SOUTH   (A)
>>>       Red:                EAST    (B)
>>>       Yellow:             NORTH   (Y)
>>>       Blue:               WEST    (X)
>>>       Orange/pedal:       TL      (L1)
>>>       Solo flag:          TL2     (L2)
>>>       Tilt:               TR      (R1)
>>>       Pad flag:           THUMBL  (L3)
>>>       Instrument button:  MODE    (Steam/Xbox)
>>>       Whammy bar:         ABS_Z   (Axis 4)
>>>       Effects switch:     Z       (Axis 5)
>>>
>>> As documented at https://github.com/TheNathannator/PlasticBand/blob/main/Docs/.
>>>
>>> The guitar and drums both use the same mapping.  Tested using the Wii
>>> versions of the instruments.
>>>
>>> Signed-off-by: Brenton Simpson <appsforartists@google.com>
>>> ---
>>>    drivers/hid/hid-ids.h  |  8 +++++++-
>>>    drivers/hid/hid-sony.c | 45 +++++++++++++++++++++++++++++-------------
>>>    2 files changed, 38 insertions(+), 15 deletions(-)
>>>
> 
> The indentation also seemed inconsistent in these two hid- files.  I
> tried my best to follow the local style of the parts I edited.  Happy
> to do a style pass in a separate commit if desired.
> 

You can use the ./scripts/checkpatch.pl script to check for any issues 
with regards to style and indentation.

>>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>>> index 3e299a30dcde..644d3c4df144 100644
>>> --- a/drivers/hid/hid-ids.h
>>> +++ b/drivers/hid/hid-ids.h
>>> @@ -664,6 +664,10 @@
>>>    #define USB_DEVICE_ID_UGCI_FLYING   0x0020
>>>    #define USB_DEVICE_ID_UGCI_FIGHTING 0x0030
>>>
>>> +#define USB_VENDOR_ID_HARMONIX               0x1bad
>>> +#define USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE 0x3010
>>> +#define USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE  0x3110
>>> +
>>>    #define USB_VENDOR_ID_HP            0x03f0
>>>    #define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A                0x464a
>>>    #define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A       0x0a4a
>>> @@ -1299,7 +1303,9 @@
>>>
>>>    #define USB_VENDOR_ID_SONY_RHYTHM   0x12ba
>>>    #define USB_DEVICE_ID_SONY_PS3WIIU_GHLIVE_DONGLE    0x074b
>>> -#define USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE 0x0100
>>> +#define USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE              0x0100
>>> +#define USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE     0x0200
>>> +#define USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE              0x0210
>>>
>>>    #define USB_VENDOR_ID_SINO_LITE                     0x1345
>>>    #define USB_DEVICE_ID_SINO_LITE_CONTROLLER  0x3008
>>> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
>>> index a89af14e4acc..f6975f6ae882 100644
>>> --- a/drivers/hid/hid-sony.c
>>> +++ b/drivers/hid/hid-sony.c
>>> @@ -62,9 +62,10 @@
>>>    #define GH_GUITAR_CONTROLLER      BIT(14)
>>>    #define GHL_GUITAR_PS3WIIU        BIT(15)
>>>    #define GHL_GUITAR_PS4            BIT(16)
>>> -#define RB4_GUITAR_PS4_USB        BIT(17)
>>> -#define RB4_GUITAR_PS4_BT         BIT(18)
>>> -#define RB4_GUITAR_PS5            BIT(19)
>>> +#define RB2_INSTRUMENT            BIT(17)
>>> +#define RB4_GUITAR_PS4_USB        BIT(18)
>>> +#define RB4_GUITAR_PS4_BT         BIT(19)
>>> +#define RB4_GUITAR_PS5            BIT(20)
>>>
>>>    #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT)
>>>    #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT)
>>> @@ -422,12 +423,12 @@ static const unsigned int sixaxis_keymap[] = {
>>>        [0x11] = BTN_MODE, /* PS */
>>>    };
>>>
>>> -static const unsigned int rb4_absmap[] = {
>>> +static const unsigned int rb_absmap[] = {
>>>        [0x30] = ABS_X,
>>>        [0x31] = ABS_Y,
>>>    };
>>
>>
>> Is the Whammy bar mapped correctly to ABS_Z with this patch, or do the
>> instruments not use the rb_absmap array, because the array does not
>> contain ABS_Z?
>>
>> If it doesn't need the rb_absmap array, maybe it'd be better to make a
>> separate rb2_instrument_mapping function instead of re-using the
>> rb4_guitar_mapping function.
> 
> I renamed the absmap to be consistent, but unlike the CRKD guitars,
> the Harmonix guitars don't have a joystick, so they ignore the absmap.
> It's the keymap they need.
> 
>>> -static const unsigned int rb4_keymap[] = {
>>> +static const unsigned int rb_keymap[] = {
>>>        [0x1] = BTN_WEST, /* Square */
>>>        [0x2] = BTN_SOUTH, /* Cross */
>>>        [0x3] = BTN_EAST, /* Circle */
>>> @@ -625,17 +626,17 @@ static int gh_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
>>>        return 0;
>>>    }
>>>
>>> -static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
>>> +static int rb_instrument_mapping(struct hid_device *hdev, struct hid_input *hi,
>>>                          struct hid_field *field, struct hid_usage *usage,
>>>                          unsigned long **bit, int *max)
>>>    {
> 
> Without the patch, the red button is not usable without a VDF:
> 
> 0300037cad1b00001030000001010000,Licensed by Nintendo of America
> Harmonix Guitar Controller for Nintendo
> Wii,a:b1,b:b2,x:b0,y:b3,back:b8,start:b9,guide:b12,leftshoulder:b4,rightshoulder:b5,lefttrigger:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,rightx:a2,righty:a3,platform:Linux
> 

Is everything mapped correctly except the red button without this patch? 
If so, maybe a simplified rb_keymap would be more clear code wise, 
alongside my suggestions below.

> So all registered Rock Band instruments can use this
> rb_instrument_mapping function.  The ones that send the axes at weird
> locations are augmented by the existing rb4_parse functions; the older
> ones send them at the standard locations.
> 

I think you misunderstood my initial reply, if the rb_absmap array isn't 
required then it's rather confusing to use the same mapping function for 
the wii/ps3 instruments, because when reading the code it implies that 
it'll be used, I'd suggest creating a new mapping function named 
rb2_instrument_mapping which uses a (maybe simplified, see above) 
rb_keymap array but doesn't use the rb_absmap array, to clearly signal 
to code readers that it's unused for those instruments.

My suggestion would also remove the need to rename the rb4_absmap 
variable name.

>>>        if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON) {
>>>                unsigned int key = usage->hid & HID_USAGE;
>>>
>>> -             if (key >= ARRAY_SIZE(rb4_keymap))
>>> +             if (key >= ARRAY_SIZE(rb_keymap))
>>>                        return 0;
>>>
>>> -             key = rb4_keymap[key];
>>> +             key = rb_keymap[key];
>>>                hid_map_usage_clear(hi, usage, bit, max, EV_KEY, key);
>>>                return 1;
>>>        } else if ((usage->hid & HID_USAGE_PAGE) == HID_UP_GENDESK) {
>>> @@ -645,10 +646,10 @@ static int rb4_guitar_mapping(struct hid_device *hdev, struct hid_input *hi,
>>>                if (usage->hid == HID_GD_HATSWITCH)
>>>                        return 0;
>>>
> 
> Like the D-pad, the whammy and the effects switch are correctly mapped
> by default.  I tested all the buttons on both the guitar and the
> drums.  With the patch, they're correct.
> 
>>> -             if (abs >= ARRAY_SIZE(rb4_absmap))
>>> +             if (abs >= ARRAY_SIZE(rb_absmap))
>>>                        return 0;
>>>
>>> -             abs = rb4_absmap[abs];
>>> +             abs = rb_absmap[abs];
>>>                hid_map_usage_clear(hi, usage, bit, max, EV_ABS, abs);
>>>                return 1;
>>>        }
>>> @@ -1101,11 +1102,14 @@ static int sony_mapping(struct hid_device *hdev, struct hid_input *hi,
>>>        if (sc->quirks & GH_GUITAR_CONTROLLER)
>>>                return gh_guitar_mapping(hdev, hi, field, usage, bit, max);
>>>
>>> +     if (sc->quirks & RB2_INSTRUMENT)
>>> +             return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
>>> +
>>>        if (sc->quirks & (RB4_GUITAR_PS4_USB | RB4_GUITAR_PS4_BT))
>>> -             return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
>>> +             return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
>>>
>>>        if (sc->quirks & RB4_GUITAR_PS5)
>>> -             return rb4_guitar_mapping(hdev, hi, field, usage, bit, max);
>>> +             return rb_instrument_mapping(hdev, hi, field, usage, bit, max);
>>>
>>>        /* Let hid-core decide for the others */
>>>        return 0;
>>> @@ -2369,12 +2373,25 @@ static const struct hid_device_id sony_devices[] = {
>>>        /* Guitar Hero PC Guitar Dongle */
>>>        { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_GUITAR_DONGLE),
>>>                .driver_data = GH_GUITAR_CONTROLLER },
>>> -     /* Guitar Hero PS3 World Tour Guitar Dongle */
>>> -     { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GUITAR_DONGLE),
>>> +     /* Guitar Hero World Tour PS3 Guitar Dongle */
>>> +     { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_GH_GUITAR_DONGLE),
>>>                .driver_data = GH_GUITAR_CONTROLLER },
>>>        /* Guitar Hero Live PS4 guitar dongles */
>>>        { HID_USB_DEVICE(USB_VENDOR_ID_REDOCTANE, USB_DEVICE_ID_REDOCTANE_PS4_GHLIVE_DONGLE),
>>>                .driver_data = GHL_GUITAR_PS4 | GH_GUITAR_CONTROLLER },
>>> +     /* Rock Band 2 instruments
>>> +      * Nintendo Wii instruments are included in `hid-sony` because `hid-nintendo`
>>> +      * is for the newer Nintendo Switch, and the Wii instruments use the same
>>> +      * protocol as their Sony PlayStation 3 cousins.
>>> +      */
>>> +     { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_GUITAR_DONGLE),
>>> +             .driver_data = RB2_INSTRUMENT },
>>> +     { HID_USB_DEVICE(USB_VENDOR_ID_HARMONIX, USB_DEVICE_ID_HARMONIX_WII_RB2_DRUMS_DONGLE),
>>> +             .driver_data = RB2_INSTRUMENT },
>>> +     { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_GUITAR_DONGLE),
>>> +             .driver_data = RB2_INSTRUMENT },
>>> +     { HID_USB_DEVICE(USB_VENDOR_ID_SONY_RHYTHM, USB_DEVICE_ID_SONY_PS3_RB2_DRUMS_DONGLE),
>>> +             .driver_data = RB2_INSTRUMENT },
>>>        /* Rock Band 4 PS4 guitars */
>>>        { HID_USB_DEVICE(USB_VENDOR_ID_PDP, USB_DEVICE_ID_PDP_PS4_RIFFMASTER),
>>>                .driver_data = RB4_GUITAR_PS4_USB },
> 
> Some of the Rock Band 3 instruments are outliers that try to merge
> real stringed instruments with plastic ones, so I left those out of
> scope for this commit.  The instruments from other games (and the
> common ones from Rock Band 3) should be compatible with
> rb_instrument_mapping.


  reply	other threads:[~2026-02-21  5:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-20 16:50 appsforartists
2026-02-21  2:42 ` Rosalie
2026-02-21  5:04   ` Brenton Simpson
2026-02-21  5:26     ` Rosalie [this message]
2026-02-21  5:52       ` Brenton Simpson
2026-02-24 18:01 ` Antheas Kapenekakis
2026-02-24 18:47   ` Brenton Simpson
2026-02-24 19:19     ` Antheas Kapenekakis
2026-02-24 19:19       ` Antheas Kapenekakis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=6bf7ba2f-bfe1-4988-b15f-9a42f4546b91@mailbox.org \
    --to=rosalie@mailbox.org \
    --cc=appsforartists@google.com \
    --cc=bentiss@kernel.org \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lkml@antheas.dev \
    --cc=nfraprado@collabora.com \
    --cc=sanjay.govind9@gmail.com \
    --cc=vi@endrift.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®