* [PATCH 1/2] HID: nintendo: fix face button mappings @ 2022-05-12 0:14 Max Fletcher 2022-05-12 0:15 ` [PATCH 2/2] HID: nintendo: add parameter to swap face buttons Max Fletcher 2022-05-13 19:58 ` [PATCH 1/2] HID: nintendo: fix face button mappings Roderick Colenbrander 0 siblings, 2 replies; 8+ messages in thread From: Max Fletcher @ 2022-05-12 0:14 UTC (permalink / raw) To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel Cc: Max Fletcher Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. Signed-off-by: Max Fletcher <fletcher0max@gmail.com> --- drivers/hid/hid-nintendo.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c index 2204de889739..7735971ede3f 100644 --- a/drivers/hid/hid-nintendo.c +++ b/drivers/hid/hid-nintendo.c @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); + input_report_key(dev, BTN_X, btns & JC_BTN_Y); + input_report_key(dev, BTN_Y, btns & JC_BTN_X); + input_report_key(dev, BTN_B, btns & JC_BTN_A); + input_report_key(dev, BTN_A, btns & JC_BTN_B); } input_sync(dev); @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { static const unsigned int joycon_button_inputs_r[] = { BTN_START, BTN_MODE, BTN_THUMBR, - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, + BTN_A, BTN_B, BTN_Y, BTN_X, BTN_TR, BTN_TR2, 0 /* 0 signals end of array */ }; -- 2.35.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 2/2] HID: nintendo: add parameter to swap face buttons 2022-05-12 0:14 [PATCH 1/2] HID: nintendo: fix face button mappings Max Fletcher @ 2022-05-12 0:15 ` Max Fletcher 2022-05-13 19:58 ` [PATCH 1/2] HID: nintendo: fix face button mappings Roderick Colenbrander 1 sibling, 0 replies; 8+ messages in thread From: Max Fletcher @ 2022-05-12 0:15 UTC (permalink / raw) To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel Cc: Max Fletcher Add the module parameter "faceswap" that swaps the face (ABXY) buttons to match the printed layout. This will be helpful for people who prefer to use the Nintendo layout over the Xbox layout. Signed-off-by: Max Fletcher <fletcher0max@gmail.com> --- drivers/hid/hid-nintendo.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c index 7735971ede3f..925cdcd0ac77 100644 --- a/drivers/hid/hid-nintendo.c +++ b/drivers/hid/hid-nintendo.c @@ -32,6 +32,7 @@ #include <linux/jiffies.h> #include <linux/leds.h> #include <linux/module.h> +#include <linux/moduleparam.h> #include <linux/power_supply.h> #include <linux/spinlock.h> @@ -320,6 +321,8 @@ struct joycon_imu_cal { s16 scale[3]; }; +bool faceswap; + /* * All the controller's button values are stored in a u32. * They can be accessed with bitwise ANDs. @@ -1351,10 +1354,17 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); - input_report_key(dev, BTN_X, btns & JC_BTN_Y); - input_report_key(dev, BTN_Y, btns & JC_BTN_X); - input_report_key(dev, BTN_B, btns & JC_BTN_A); - input_report_key(dev, BTN_A, btns & JC_BTN_B); + if (!faceswap) { + input_report_key(dev, BTN_X, btns & JC_BTN_Y); + input_report_key(dev, BTN_Y, btns & JC_BTN_X); + input_report_key(dev, BTN_B, btns & JC_BTN_A); + input_report_key(dev, BTN_A, btns & JC_BTN_B); + } else { + input_report_key(dev, BTN_Y, btns & JC_BTN_Y); + input_report_key(dev, BTN_X, btns & JC_BTN_X); + input_report_key(dev, BTN_A, btns & JC_BTN_A); + input_report_key(dev, BTN_B, btns & JC_BTN_B); + } } input_sync(dev); @@ -2313,6 +2323,10 @@ static const struct hid_device_id nintendo_hid_devices[] = { }; MODULE_DEVICE_TABLE(hid, nintendo_hid_devices); +module_param(faceswap, bool, 0440); +MODULE_PARM_DESC(faceswap, + "Swaps the face buttons to match the printed layout"); + static struct hid_driver nintendo_hid_driver = { .name = "nintendo", .id_table = nintendo_hid_devices, -- 2.35.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: nintendo: fix face button mappings 2022-05-12 0:14 [PATCH 1/2] HID: nintendo: fix face button mappings Max Fletcher 2022-05-12 0:15 ` [PATCH 2/2] HID: nintendo: add parameter to swap face buttons Max Fletcher @ 2022-05-13 19:58 ` Roderick Colenbrander [not found] ` <CAKcX28WpKoP=HVq3zCvBh9knKFEdR0_+NmATpt9D6rFmprkFDA@mail.gmail.com> 2022-05-31 15:19 ` Martino Fontana 1 sibling, 2 replies; 8+ messages in thread From: Roderick Colenbrander @ 2022-05-13 19:58 UTC (permalink / raw) To: Max Fletcher Cc: Daniel J. Ogorchock, Jiri Kosina, Benjamin Tissoires, linux-input, lkml Hi Max, Thanks for your patch, however I must say the patch is not correct for 2 reasons. Over the years different controllers have different layouts. The standard which this driver (as well as others such as hid-sony/hid-playstation) follow is the Linux gamepad standard (see Documentation/input/gamepad.rst). It stays away of the debate what is A/B/X/Y. It talks about North/west/.., (yes they are macros which map to A/B/X/Y). In case of the Switch it does mean things are flipped, but it was not meant to represent an Xbox controller. (Technically one could argue that the Xbox controller should be flipped as it was the SNES controller back in the days which introduced X/Y and the Switch is still consistent with that.) Second, even if the patch was right it would be tricky to merge. The problem is that a changed mapping breaks user spaces and in general can't do this unless there is a really good reason. It just would break existing applications and libraries (often e.g. SDL) Thanks, Roderick On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@gmail.com> wrote: > > Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. > > Signed-off-by: Max Fletcher <fletcher0max@gmail.com> > --- > drivers/hid/hid-nintendo.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c > index 2204de889739..7735971ede3f 100644 > --- a/drivers/hid/hid-nintendo.c > +++ b/drivers/hid/hid-nintendo.c > @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, > input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); > input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); > input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); > - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); > - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); > - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); > - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); > + input_report_key(dev, BTN_X, btns & JC_BTN_Y); > + input_report_key(dev, BTN_Y, btns & JC_BTN_X); > + input_report_key(dev, BTN_B, btns & JC_BTN_A); > + input_report_key(dev, BTN_A, btns & JC_BTN_B); > } > > input_sync(dev); > @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { > > static const unsigned int joycon_button_inputs_r[] = { > BTN_START, BTN_MODE, BTN_THUMBR, > - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, > + BTN_A, BTN_B, BTN_Y, BTN_X, > BTN_TR, BTN_TR2, > 0 /* 0 signals end of array */ > }; > -- > 2.35.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
[parent not found: <CAKcX28WpKoP=HVq3zCvBh9knKFEdR0_+NmATpt9D6rFmprkFDA@mail.gmail.com>]
* Re: [PATCH 1/2] HID: nintendo: fix face button mappings [not found] ` <CAKcX28WpKoP=HVq3zCvBh9knKFEdR0_+NmATpt9D6rFmprkFDA@mail.gmail.com> @ 2022-05-15 3:33 ` Roderick Colenbrander 2022-05-26 20:07 ` Maxwell Fletcher 0 siblings, 1 reply; 8+ messages in thread From: Roderick Colenbrander @ 2022-05-15 3:33 UTC (permalink / raw) To: Maxwell Fletcher Cc: Daniel J. Ogorchock, Jiri Kosina, Benjamin Tissoires, linux-input, lkml Hi Maxwell, I don't think it is desired to have such kernel module parameters as it essentially adds new kernel APIs, which have to be maintained unless truly needed. However, you may have seen the work Benjamin is doing around eBPF for HID. I'm no expert on it yet, but it could probably allow you to do a similar kind of fixup without having to modify the kernel module. Thanks, Roderick On Sat, May 14, 2022 at 5:57 PM Maxwell Fletcher <fletcher0max@gmail.com> wrote: > > Hi Roderick, > > Thanks for the explanation. It makes sense that the mappings were never meant to mirror Xbox controllers. Would it still be possible to merge a patch that adds an opt-in module parameter that changes the mappings, similar to the one in the second part of this patch? > > Thanks, > Max > > On Fri, May 13, 2022 at 12:58 PM Roderick Colenbrander <thunderbird2k@gmail.com> wrote: >> >> Hi Max, >> >> Thanks for your patch, however I must say the patch is not correct for >> 2 reasons. >> >> Over the years different controllers have different layouts. The >> standard which this driver (as well as others such as >> hid-sony/hid-playstation) follow is the Linux gamepad standard (see >> Documentation/input/gamepad.rst). It stays away of the debate what is >> A/B/X/Y. It talks about North/west/.., (yes they are macros which map >> to A/B/X/Y). In case of the Switch it does mean things are flipped, >> but it was not meant to represent an Xbox controller. (Technically one >> could argue that the Xbox controller should be flipped as it was the >> SNES controller back in the days which introduced X/Y and the Switch >> is still consistent with that.) >> >> Second, even if the patch was right it would be tricky to merge. The >> problem is that a changed mapping breaks user spaces and in general >> can't do this unless there is a really good reason. It just would >> break existing applications and libraries (often e.g. SDL) >> >> Thanks, >> Roderick >> >> On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@gmail.com> wrote: >> > >> > Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. >> > >> > Signed-off-by: Max Fletcher <fletcher0max@gmail.com> >> > --- >> > drivers/hid/hid-nintendo.c | 10 +++++----- >> > 1 file changed, 5 insertions(+), 5 deletions(-) >> > >> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c >> > index 2204de889739..7735971ede3f 100644 >> > --- a/drivers/hid/hid-nintendo.c >> > +++ b/drivers/hid/hid-nintendo.c >> > @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, >> > input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); >> > input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); >> > input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); >> > - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); >> > - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); >> > - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); >> > - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); >> > + input_report_key(dev, BTN_X, btns & JC_BTN_Y); >> > + input_report_key(dev, BTN_Y, btns & JC_BTN_X); >> > + input_report_key(dev, BTN_B, btns & JC_BTN_A); >> > + input_report_key(dev, BTN_A, btns & JC_BTN_B); >> > } >> > >> > input_sync(dev); >> > @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { >> > >> > static const unsigned int joycon_button_inputs_r[] = { >> > BTN_START, BTN_MODE, BTN_THUMBR, >> > - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, >> > + BTN_A, BTN_B, BTN_Y, BTN_X, >> > BTN_TR, BTN_TR2, >> > 0 /* 0 signals end of array */ >> > }; >> > -- >> > 2.35.3 >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: nintendo: fix face button mappings 2022-05-15 3:33 ` Roderick Colenbrander @ 2022-05-26 20:07 ` Maxwell Fletcher 0 siblings, 0 replies; 8+ messages in thread From: Maxwell Fletcher @ 2022-05-26 20:07 UTC (permalink / raw) To: Roderick Colenbrander Cc: Daniel J. Ogorchock, Jiri Kosina, Benjamin Tissoires, linux-input, lkml Hi Roderick, Thank you so much for your time; this has all been very educational for me. I'll be following the work on eBPF. Best, Max On Sat, May 14, 2022 at 8:33 PM Roderick Colenbrander <thunderbird2k@gmail.com> wrote: > > Hi Maxwell, > > I don't think it is desired to have such kernel module parameters as > it essentially adds new kernel APIs, which have to be maintained > unless truly needed. > > However, you may have seen the work Benjamin is doing around eBPF for > HID. I'm no expert on it yet, but it could probably allow you to do a > similar kind of fixup without having to modify the kernel module. > > Thanks, > Roderick > > On Sat, May 14, 2022 at 5:57 PM Maxwell Fletcher <fletcher0max@gmail.com> wrote: > > > > Hi Roderick, > > > > Thanks for the explanation. It makes sense that the mappings were never meant to mirror Xbox controllers. Would it still be possible to merge a patch that adds an opt-in module parameter that changes the mappings, similar to the one in the second part of this patch? > > > > Thanks, > > Max > > > > On Fri, May 13, 2022 at 12:58 PM Roderick Colenbrander <thunderbird2k@gmail.com> wrote: > >> > >> Hi Max, > >> > >> Thanks for your patch, however I must say the patch is not correct for > >> 2 reasons. > >> > >> Over the years different controllers have different layouts. The > >> standard which this driver (as well as others such as > >> hid-sony/hid-playstation) follow is the Linux gamepad standard (see > >> Documentation/input/gamepad.rst). It stays away of the debate what is > >> A/B/X/Y. It talks about North/west/.., (yes they are macros which map > >> to A/B/X/Y). In case of the Switch it does mean things are flipped, > >> but it was not meant to represent an Xbox controller. (Technically one > >> could argue that the Xbox controller should be flipped as it was the > >> SNES controller back in the days which introduced X/Y and the Switch > >> is still consistent with that.) > >> > >> Second, even if the patch was right it would be tricky to merge. The > >> problem is that a changed mapping breaks user spaces and in general > >> can't do this unless there is a really good reason. It just would > >> break existing applications and libraries (often e.g. SDL) > >> > >> Thanks, > >> Roderick > >> > >> On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@gmail.com> wrote: > >> > > >> > Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. > >> > > >> > Signed-off-by: Max Fletcher <fletcher0max@gmail.com> > >> > --- > >> > drivers/hid/hid-nintendo.c | 10 +++++----- > >> > 1 file changed, 5 insertions(+), 5 deletions(-) > >> > > >> > diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c > >> > index 2204de889739..7735971ede3f 100644 > >> > --- a/drivers/hid/hid-nintendo.c > >> > +++ b/drivers/hid/hid-nintendo.c > >> > @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, > >> > input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); > >> > input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); > >> > input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); > >> > - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); > >> > - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); > >> > - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); > >> > - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); > >> > + input_report_key(dev, BTN_X, btns & JC_BTN_Y); > >> > + input_report_key(dev, BTN_Y, btns & JC_BTN_X); > >> > + input_report_key(dev, BTN_B, btns & JC_BTN_A); > >> > + input_report_key(dev, BTN_A, btns & JC_BTN_B); > >> > } > >> > > >> > input_sync(dev); > >> > @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { > >> > > >> > static const unsigned int joycon_button_inputs_r[] = { > >> > BTN_START, BTN_MODE, BTN_THUMBR, > >> > - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, > >> > + BTN_A, BTN_B, BTN_Y, BTN_X, > >> > BTN_TR, BTN_TR2, > >> > 0 /* 0 signals end of array */ > >> > }; > >> > -- > >> > 2.35.3 > >> > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: nintendo: fix face button mappings 2022-05-13 19:58 ` [PATCH 1/2] HID: nintendo: fix face button mappings Roderick Colenbrander [not found] ` <CAKcX28WpKoP=HVq3zCvBh9knKFEdR0_+NmATpt9D6rFmprkFDA@mail.gmail.com> @ 2022-05-31 15:19 ` Martino Fontana 2022-05-31 15:31 ` Roderick Colenbrander 1 sibling, 1 reply; 8+ messages in thread From: Martino Fontana @ 2022-05-31 15:19 UTC (permalink / raw) To: Roderick Colenbrander, Max Fletcher Cc: Daniel J. Ogorchock, Jiri Kosina, Benjamin Tissoires, linux-input, lkml > Second, even if the patch was right it would be tricky to merge. The > problem is that a changed mapping breaks user spaces and in general > can't do this unless there is a really good reason. It just would > break existing applications and libraries (often e.g. SDL) The problem is that the userspace is already broken. If, out of the box, you attempt to launch something that uses SDL (like Wine, or Super Tux Kart), the mapping you'll get will be wrong (and not visually, the buttons are literally swapped). Right now, this can be worked around (it makes the mapping correct) by setting an environment variable (which isn't a thing that a user is supposed to be doing, and the patch will remove the need of it): ``` SDL_GAMECONTROLLERCONFIG=050000007e0500000920000001800000,Nintendo Switch Pro Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,030000007e0500000920000011810000,Nintendo Switch Pro Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,060000007e0500000620000000000000,Nintendo Switch Combined Joy-Cons,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:b14,dpdown:b15,dpleft:b16,dpright:b17,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8, ``` The weird thing however is that SDL should already uses a controller database: https://github.com/libsdl-org/SDL/blob/main/src/joystick/SDL_gamecontrollerdb.h. If this problem is caused by an SDL bug (I don't know if it is), then it's SDL that will need a patch, and the problem will be solved with zero repercussions. I think that SDL should be investigated before continuing to discuss this. If, however, SDL is working as intended (making the patch necessary) and this patch is merged, it will make the mapping correct out of the box but it will also break a few things: - Those who were using this workaround will have to remove it (by the way, the page on the Arch Wiki should be updated too: https://wiki.archlinux.org/title/Gamepad#Using_hid-nintendo_with_SDL2_Games); - Applications that use raw joydev/evdev mappings (like Dolphin Emulator) will have to be reconfigured. - Also, some applications use SDL2 mappings in a different way (for example, PCSX2), so out of the box the mappings are applied and the controller is mapped as expected. Merging the patch will break applications like these (in PCSX2's case it will be temporary since its database is updated weekly, but it will force the user to use the new kernel version, otherwise they will be stuck with a wrong mapping). Whether it's a wise idea to merge this as soon as possible in order to cause the least amount amount of breakage, is not my call. On 13/05/22 21:58, Roderick Colenbrander wrote: > Hi Max, > > Thanks for your patch, however I must say the patch is not correct for > 2 reasons. > > Over the years different controllers have different layouts. The > standard which this driver (as well as others such as > hid-sony/hid-playstation) follow is the Linux gamepad standard (see > Documentation/input/gamepad.rst). It stays away of the debate what is > A/B/X/Y. It talks about North/west/.., (yes they are macros which map > to A/B/X/Y). In case of the Switch it does mean things are flipped, > but it was not meant to represent an Xbox controller. (Technically one > could argue that the Xbox controller should be flipped as it was the > SNES controller back in the days which introduced X/Y and the Switch > is still consistent with that.) > > Second, even if the patch was right it would be tricky to merge. The > problem is that a changed mapping breaks user spaces and in general > can't do this unless there is a really good reason. It just would > break existing applications and libraries (often e.g. SDL) > > Thanks, > Roderick > > On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@gmail.com> wrote: >> >> Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. >> >> Signed-off-by: Max Fletcher <fletcher0max@gmail.com> >> --- >> drivers/hid/hid-nintendo.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c >> index 2204de889739..7735971ede3f 100644 >> --- a/drivers/hid/hid-nintendo.c >> +++ b/drivers/hid/hid-nintendo.c >> @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, >> input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); >> input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); >> input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); >> - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); >> - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); >> - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); >> - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); >> + input_report_key(dev, BTN_X, btns & JC_BTN_Y); >> + input_report_key(dev, BTN_Y, btns & JC_BTN_X); >> + input_report_key(dev, BTN_B, btns & JC_BTN_A); >> + input_report_key(dev, BTN_A, btns & JC_BTN_B); >> } >> >> input_sync(dev); >> @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { >> >> static const unsigned int joycon_button_inputs_r[] = { >> BTN_START, BTN_MODE, BTN_THUMBR, >> - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, >> + BTN_A, BTN_B, BTN_Y, BTN_X, >> BTN_TR, BTN_TR2, >> 0 /* 0 signals end of array */ >> }; >> -- >> 2.35.3 >> > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: nintendo: fix face button mappings 2022-05-31 15:19 ` Martino Fontana @ 2022-05-31 15:31 ` Roderick Colenbrander 2022-06-04 13:11 ` Martino Fontana 0 siblings, 1 reply; 8+ messages in thread From: Roderick Colenbrander @ 2022-05-31 15:31 UTC (permalink / raw) To: Martino Fontana Cc: Max Fletcher, Daniel J. Ogorchock, Jiri Kosina, Benjamin Tissoires, linux-input, lkml On Tue, May 31, 2022 at 8:19 AM Martino Fontana <tinozzo123@gmail.com> wrote: > > > Second, even if the patch was right it would be tricky to merge. The > > problem is that a changed mapping breaks user spaces and in general > > can't do this unless there is a really good reason. It just would > > break existing applications and libraries (often e.g. SDL) > > The problem is that the userspace is already broken. > If, out of the box, you attempt to launch something that uses SDL (like > Wine, or Super Tux Kart), the mapping you'll get will be wrong (and not > visually, the buttons are literally swapped). > Right now, this can be worked around (it makes the mapping correct) by > setting an environment variable (which isn't a thing that a user is > supposed to be doing, and the patch will remove the need of it): > ``` > SDL_GAMECONTROLLERCONFIG=050000007e0500000920000001800000,Nintendo > Switch Pro > Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,030000007e0500000920000011810000,Nintendo > Switch Pro > Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,060000007e0500000620000000000000,Nintendo > Switch Combined > Joy-Cons,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:b14,dpdown:b15,dpleft:b16,dpright:b17,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8, > ``` > The weird thing however is that SDL should already uses a controller > database: > https://github.com/libsdl-org/SDL/blob/main/src/joystick/SDL_gamecontrollerdb.h. > If this problem is caused by an SDL bug (I don't know if it is), then > it's SDL that will need a patch, and the problem will be solved with > zero repercussions. I think that SDL should be investigated before > continuing to discuss this. It looks like SDL lacks the patch to deal with the upstream driver properly. The chicken and egg problem SDL there is between the kernel and SDL, is that SDL supports a device prior to there being a kernel driver (or there being enough penetration for one). Without a driver, many devices function with an often strange mapping. That's the mapping SDL has in its table (for hid-generic). When a kernel driver comes around with a different, but proper mapping there is an issue. At the time we first dealt with this for DualShock 3 / 4 devices. The trick used then was to patch the 'version' bit of the HID device with '0x8000'. This resulted in a different mapping line. To understand SDL uses a GUID as the index for the table, it is composed of vendor/product id, version id and other fiels. The patched version, resulted in a different GUID, which then allowed SDL2 to recognize the device properly as it would use a different mapping line. For the Switch controllers, it looks like no patched line was added. Someone would need to provide a patch. > If, however, SDL is working as intended (making the patch necessary) and > this patch is merged, it will make the mapping correct out of the box > but it will also break a few things: > - Those who were using this workaround will have to remove it (by the > way, the page on the Arch Wiki should be updated too: > https://wiki.archlinux.org/title/Gamepad#Using_hid-nintendo_with_SDL2_Games); > - Applications that use raw joydev/evdev mappings (like Dolphin > Emulator) will have to be reconfigured. > - Also, some applications use SDL2 mappings in a different way (for > example, PCSX2), so out of the box the mappings are applied and the > controller is mapped as expected. Merging the patch will break > applications like these (in PCSX2's case it will be temporary since its > database is updated weekly, but it will force the user to use the new > kernel version, otherwise they will be stuck with a wrong mapping). > > Whether it's a wise idea to merge this as soon as possible in order to > cause the least amount amount of breakage, is not my call. > > > On 13/05/22 21:58, Roderick Colenbrander wrote: > > Hi Max, > > > > Thanks for your patch, however I must say the patch is not correct for > > 2 reasons. > > > > Over the years different controllers have different layouts. The > > standard which this driver (as well as others such as > > hid-sony/hid-playstation) follow is the Linux gamepad standard (see > > Documentation/input/gamepad.rst). It stays away of the debate what is > > A/B/X/Y. It talks about North/west/.., (yes they are macros which map > > to A/B/X/Y). In case of the Switch it does mean things are flipped, > > but it was not meant to represent an Xbox controller. (Technically one > > could argue that the Xbox controller should be flipped as it was the > > SNES controller back in the days which introduced X/Y and the Switch > > is still consistent with that.) > > > > Second, even if the patch was right it would be tricky to merge. The > > problem is that a changed mapping breaks user spaces and in general > > can't do this unless there is a really good reason. It just would > > break existing applications and libraries (often e.g. SDL) > > > > Thanks, > > Roderick > > > > On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@gmail.com> wrote: > >> > >> Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. > >> > >> Signed-off-by: Max Fletcher <fletcher0max@gmail.com> > >> --- > >> drivers/hid/hid-nintendo.c | 10 +++++----- > >> 1 file changed, 5 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c > >> index 2204de889739..7735971ede3f 100644 > >> --- a/drivers/hid/hid-nintendo.c > >> +++ b/drivers/hid/hid-nintendo.c > >> @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, > >> input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); > >> input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); > >> input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); > >> - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); > >> - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); > >> - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); > >> - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); > >> + input_report_key(dev, BTN_X, btns & JC_BTN_Y); > >> + input_report_key(dev, BTN_Y, btns & JC_BTN_X); > >> + input_report_key(dev, BTN_B, btns & JC_BTN_A); > >> + input_report_key(dev, BTN_A, btns & JC_BTN_B); > >> } > >> > >> input_sync(dev); > >> @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { > >> > >> static const unsigned int joycon_button_inputs_r[] = { > >> BTN_START, BTN_MODE, BTN_THUMBR, > >> - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, > >> + BTN_A, BTN_B, BTN_Y, BTN_X, > >> BTN_TR, BTN_TR2, > >> 0 /* 0 signals end of array */ > >> }; > >> -- > >> 2.35.3 > >> > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: nintendo: fix face button mappings 2022-05-31 15:31 ` Roderick Colenbrander @ 2022-06-04 13:11 ` Martino Fontana 0 siblings, 0 replies; 8+ messages in thread From: Martino Fontana @ 2022-06-04 13:11 UTC (permalink / raw) To: Roderick Colenbrander Cc: Max Fletcher, Daniel J. Ogorchock, Jiri Kosina, Benjamin Tissoires, linux-input, lkml I made a PR: https://github.com/libsdl-org/SDL/pull/5755. On 31/05/22 17:31, Roderick Colenbrander wrote: > On Tue, May 31, 2022 at 8:19 AM Martino Fontana <tinozzo123@gmail.com> wrote: >> >> > Second, even if the patch was right it would be tricky to merge. The >> > problem is that a changed mapping breaks user spaces and in general >> > can't do this unless there is a really good reason. It just would >> > break existing applications and libraries (often e.g. SDL) >> >> The problem is that the userspace is already broken. >> If, out of the box, you attempt to launch something that uses SDL (like >> Wine, or Super Tux Kart), the mapping you'll get will be wrong (and not >> visually, the buttons are literally swapped). >> Right now, this can be worked around (it makes the mapping correct) by >> setting an environment variable (which isn't a thing that a user is >> supposed to be doing, and the patch will remove the need of it): >> ``` >> SDL_GAMECONTROLLERCONFIG=050000007e0500000920000001800000,Nintendo >> Switch Pro >> Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,030000007e0500000920000011810000,Nintendo >> Switch Pro >> Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,060000007e0500000620000000000000,Nintendo >> Switch Combined >> Joy-Cons,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:b14,dpdown:b15,dpleft:b16,dpright:b17,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8, >> ``` >> The weird thing however is that SDL should already uses a controller >> database: >> https://github.com/libsdl-org/SDL/blob/main/src/joystick/SDL_gamecontrollerdb.h. >> If this problem is caused by an SDL bug (I don't know if it is), then >> it's SDL that will need a patch, and the problem will be solved with >> zero repercussions. I think that SDL should be investigated before >> continuing to discuss this. > > It looks like SDL lacks the patch to deal with the upstream driver > properly. The chicken and egg problem SDL there is between the kernel > and SDL, is that SDL supports a device prior to there being a kernel > driver (or there being enough penetration for one). Without a driver, > many devices function with an often strange mapping. That's the > mapping SDL has in its table (for hid-generic). When a kernel driver > comes around with a different, but proper mapping there is an issue. > > At the time we first dealt with this for DualShock 3 / 4 devices. The > trick used then was to patch the 'version' bit of the HID device with > '0x8000'. This resulted in a different mapping line. To understand SDL > uses a GUID as the index for the table, it is composed of > vendor/product id, version id and other fiels. The patched version, > resulted in a different GUID, which then allowed SDL2 to recognize the > device properly as it would use a different mapping line. > > For the Switch controllers, it looks like no patched line was added. > Someone would need to provide a patch. > >> If, however, SDL is working as intended (making the patch necessary) and >> this patch is merged, it will make the mapping correct out of the box >> but it will also break a few things: >> - Those who were using this workaround will have to remove it (by the >> way, the page on the Arch Wiki should be updated too: >> https://wiki.archlinux.org/title/Gamepad#Using_hid-nintendo_with_SDL2_Games); >> - Applications that use raw joydev/evdev mappings (like Dolphin >> Emulator) will have to be reconfigured. >> - Also, some applications use SDL2 mappings in a different way (for >> example, PCSX2), so out of the box the mappings are applied and the >> controller is mapped as expected. Merging the patch will break >> applications like these (in PCSX2's case it will be temporary since its >> database is updated weekly, but it will force the user to use the new >> kernel version, otherwise they will be stuck with a wrong mapping). >> >> Whether it's a wise idea to merge this as soon as possible in order to >> cause the least amount amount of breakage, is not my call. >> >> >> On 13/05/22 21:58, Roderick Colenbrander wrote: >>> Hi Max, >>> >>> Thanks for your patch, however I must say the patch is not correct for >>> 2 reasons. >>> >>> Over the years different controllers have different layouts. The >>> standard which this driver (as well as others such as >>> hid-sony/hid-playstation) follow is the Linux gamepad standard (see >>> Documentation/input/gamepad.rst). It stays away of the debate what is >>> A/B/X/Y. It talks about North/west/.., (yes they are macros which map >>> to A/B/X/Y). In case of the Switch it does mean things are flipped, >>> but it was not meant to represent an Xbox controller. (Technically one >>> could argue that the Xbox controller should be flipped as it was the >>> SNES controller back in the days which introduced X/Y and the Switch >>> is still consistent with that.) >>> >>> Second, even if the patch was right it would be tricky to merge. The >>> problem is that a changed mapping breaks user spaces and in general >>> can't do this unless there is a really good reason. It just would >>> break existing applications and libraries (often e.g. SDL) >>> >>> Thanks, >>> Roderick >>> >>> On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@gmail.com> wrote: >>>> >>>> Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout. >>>> >>>> Signed-off-by: Max Fletcher <fletcher0max@gmail.com> >>>> --- >>>> drivers/hid/hid-nintendo.c | 10 +++++----- >>>> 1 file changed, 5 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c >>>> index 2204de889739..7735971ede3f 100644 >>>> --- a/drivers/hid/hid-nintendo.c >>>> +++ b/drivers/hid/hid-nintendo.c >>>> @@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr, >>>> input_report_key(dev, BTN_START, btns & JC_BTN_PLUS); >>>> input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK); >>>> input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME); >>>> - input_report_key(dev, BTN_WEST, btns & JC_BTN_Y); >>>> - input_report_key(dev, BTN_NORTH, btns & JC_BTN_X); >>>> - input_report_key(dev, BTN_EAST, btns & JC_BTN_A); >>>> - input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B); >>>> + input_report_key(dev, BTN_X, btns & JC_BTN_Y); >>>> + input_report_key(dev, BTN_Y, btns & JC_BTN_X); >>>> + input_report_key(dev, BTN_B, btns & JC_BTN_A); >>>> + input_report_key(dev, BTN_A, btns & JC_BTN_B); >>>> } >>>> >>>> input_sync(dev); >>>> @@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = { >>>> >>>> static const unsigned int joycon_button_inputs_r[] = { >>>> BTN_START, BTN_MODE, BTN_THUMBR, >>>> - BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST, >>>> + BTN_A, BTN_B, BTN_Y, BTN_X, >>>> BTN_TR, BTN_TR2, >>>> 0 /* 0 signals end of array */ >>>> }; >>>> -- >>>> 2.35.3 >>>> >>> >>> ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2022-06-04 13:11 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-12 0:14 [PATCH 1/2] HID: nintendo: fix face button mappings Max Fletcher
2022-05-12 0:15 ` [PATCH 2/2] HID: nintendo: add parameter to swap face buttons Max Fletcher
2022-05-13 19:58 ` [PATCH 1/2] HID: nintendo: fix face button mappings Roderick Colenbrander
[not found] ` <CAKcX28WpKoP=HVq3zCvBh9knKFEdR0_+NmATpt9D6rFmprkFDA@mail.gmail.com>
2022-05-15 3:33 ` Roderick Colenbrander
2022-05-26 20:07 ` Maxwell Fletcher
2022-05-31 15:19 ` Martino Fontana
2022-05-31 15:31 ` Roderick Colenbrander
2022-06-04 13:11 ` Martino Fontana
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.