* Re: [PATCH 0/2] PCI: Remove no_pci_devices
From: Maciej W. Rozycki @ 2026-04-10 14:13 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Heiner Kallweit, Dmitry Torokhov, Bjorn Helgaas,
open list:HID CORE LAYER, linux-pci@vger.kernel.org
In-Reply-To: <20260407194812.GA250485@bhelgaas>
On Tue, 7 Apr 2026, Bjorn Helgaas wrote:
> [+cc Maciej]
>
> On Fri, Apr 03, 2026 at 12:15:34AM +0200, Heiner Kallweit wrote:
> > Remove the last user of no_pci_devices(), and then the function itself.
> >
> > Heiner Kallweit (2):
> > input: pc110pad: change PCI check to get rid of orphaned
> > no_pci_devices
> > PCI: Remove no_pci_devices
>
> Applied Dmitry's patch to remove pc110pad and your patch to remove
> no_pci_devices() to pci/enumeration for v7.1, thanks!
What can I say?
This:
static int pc110pad_io = 0x15e0;
seems to be a reasonably safe location in the PC/AT architecture to poke
at blindly, especially while guarded with request_region(). Ralf Brown's
"Ports List" doesn't list anything at location 0x1e0 modulo 0x400, and for
PCI/e we never assign locations 0x100-0x300 modulo 0x400 anyway. With PCI
systems featuring a southbridge ISA is subtractively decoded, so unclaimed
accesses will simply time out.
There could be an issue though with preventing the claimed IRQ from being
used for a legitimate device, and the call to no_pci_devices() only pushes
the problem to non-PCI systems. Poking at the handshake port and checking
for it being fixed at all-ones might be a way to figure out whether there
is anything actually wired there.
Overall this seems a case for a platform device, but these are hard to
wire in arch/x86 given how the platform has evolved. I imagine there's no
way to probe for a PC-110 system, as back in the day stuff used to rely on
manual configuration, though there could be a vendor BIOS call available
for guesswork. A kernel parameter to tell the various odd systems apart
would IMHO be a reasonable approach if this part of the kernel were being
implemented now.
Then I have nothing to say as to the removal of the driver itself; if I
do manage to find time to revive 486 support via emulation as I outlined
last year, then whoever wants the driver they can bring it back too; it's
not like the most complex piece of code I've ever seen.
Thanks for looping me in.
Maciej
^ permalink raw reply
* [PATCH] HID: logitech-dj: fix wrong detection of bad DJ_SHORT output report
From: Benjamin Tissoires @ 2026-04-10 14:03 UTC (permalink / raw)
To: Filipe Laíns, Jiri Kosina, Lee Jones
Cc: linux-input, linux-kernel, Benjamin Tissoires
commit b6a57912854e ("HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT
related user initiated OOB write") assumed that all HID devices attached
to the logitech-dj driver was having an output report of DJ_SHORT.
However, on the receiver itself, we have 2 other HID device we attach
here: the mouse emulation and the keyboard emulation. For those devices
the value of rep is NULL and we are triggered a segfault here.
This is doubly required because logitech-dj also handles non DJ devices
that might not have the DJ collection.
Fixes: b6a57912854e ("HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT related user initiated OOB write")
Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
---
drivers/hid/hid-logitech-dj.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 838c6de9a921..695d3031009e 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1907,7 +1907,8 @@ static int logi_dj_probe(struct hid_device *hdev,
output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
rep = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
- if (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1) {
+ if (rep && (rep->maxfield < 1 ||
+ rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1)) {
hid_err(hdev, "Expected size of DJ short report is %d, but got %d",
DJREPORT_SHORT_LENGTH - 1, rep->field[0]->report_count);
return -EINVAL;
---
base-commit: e2aaf2d3ad92ac4a8afa6b69ad4c38e7747d3d6e
change-id: 20260410-fix-logitech-dj-e9060b0b48bd
Best regards,
--
Benjamin Tissoires <bentiss@kernel.org>
^ permalink raw reply related
* Re: [PATCH v2 2/2] HID: logitech-dj: Prevent REPORT_ID_DJ_SHORT related user initiated OOB write
From: Benjamin Tissoires @ 2026-04-10 12:12 UTC (permalink / raw)
To: Lee Jones; +Cc: Filipe Laíns, Jiri Kosina, linux-input, linux-kernel
In-Reply-To: <20260324143651.342273-2-lee@kernel.org>
On Mar 24 2026, Lee Jones wrote:
> logi_dj_recv_send_report() assumes that all incoming REPORT_ID_DJ_SHORT
> reports are 14 Bytes (DJREPORT_SHORT_LENGTH - 1) long. It uses that
> assumption to load the associated field's 'value' array with 14 Bytes of
> data. However, if a malicious user only sends say 1 Byte of data,
> 'report_count' will be 1 and only 1 Byte of memory will be allocated to
> the 'value' Byte array. When we come to populate 'value[1-13]' we will
> experience an OOB write.
>
> Signed-off-by: Lee Jones <lee@kernel.org>
> ---
> v1 => v2: Move handling to .probe()
>
> drivers/hid/hid-logitech-dj.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
> index 32139b2561c0..a8082199d13d 100644
> --- a/drivers/hid/hid-logitech-dj.c
> +++ b/drivers/hid/hid-logitech-dj.c
> @@ -1859,6 +1859,7 @@ static int logi_dj_probe(struct hid_device *hdev,
> const struct hid_device_id *id)
> {
> struct hid_report_enum *input_report_enum;
> + struct hid_report_enum *output_report_enum;
> struct hid_report *rep;
> struct dj_receiver_dev *djrcv_dev;
> struct usb_interface *intf;
> @@ -1903,6 +1904,15 @@ static int logi_dj_probe(struct hid_device *hdev,
> }
> }
>
> + output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
> + rep = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
We've got an issue here: the driver binds on several HID devices that
can have no REPORT_ID_DJ_SHORT in the output reports. On those devices
(like the mouse/keyboard emulation on the receiver itself), rep is null.
And of course this segfaults in the test below.
A simple "if (rep &&)" should solve the issue:
diff --git a/drivers/hid/hid-logitech-dj.c b/drivers/hid/hid-logitech-dj.c
index 838c6de9a921..7c09faedefbd 100644
--- a/drivers/hid/hid-logitech-dj.c
+++ b/drivers/hid/hid-logitech-dj.c
@@ -1907,7 +1907,7 @@ static int logi_dj_probe(struct hid_device *hdev,
output_report_enum = &hdev->report_enum[HID_OUTPUT_REPORT];
rep = output_report_enum->report_id_hash[REPORT_ID_DJ_SHORT];
- if (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1) {
+ if (rep && (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1)) {
hid_err(hdev, "Expected size of DJ short report is %d, but got %d",
DJREPORT_SHORT_LENGTH - 1, rep->field[0]->report_count);
return -EINVAL;
Cheers,
Benjamin
> +
> + if (rep->maxfield < 1 || rep->field[0]->report_count != DJREPORT_SHORT_LENGTH - 1) {
> + hid_err(hdev, "Expected size of DJ short report is %d, but got %d",
> + DJREPORT_SHORT_LENGTH - 1, rep->field[0]->report_count);
> + return -EINVAL;
> + }
> +
> input_report_enum = &hdev->report_enum[HID_INPUT_REPORT];
>
> /* no input reports, bail out */
> --
> 2.53.0.983.g0bb29b3bc5-goog
>
>
^ permalink raw reply related
* [PATCH] HID: ft260: validate i2c input report length
From: Michael Zaidman @ 2026-04-10 9:41 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Sebastián Josué Alba Vives, linux-i2c, linux-input,
linux-kernel, Michael Zaidman
In-Reply-To: <CAPnwWgPhb+owa69-pTADpqk=KMWH71EUT6cxwCeT5KGnBWk+Xg@mail.gmail.com>
Validate xfer->length against the actual HID report size in
ft260_raw_event() before using it as the memcpy length. A malicious
or malfunctioning device could send a report with xfer->length
exceeding the data actually present in the HID report, causing an
out-of-bounds read.
Each I2C data report ID (0xD0 through 0xDE) defines a different
report size in the HID descriptor, so the available payload varies
per report. Validate against the actual received report size rather
than a fixed maximum to avoid breaking valid short transfers.
Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>
---
Tested on FT260 with I2C-attached EEPROM (24c02) behind PCA9548
mux switches. Verified short reads (1-4 bytes, report ID 0xD0)
and multi-report reads with debug tracing enabled, confirming
xfer->length is correctly validated against the HID report size
for each report ID.
---
drivers/hid/hid-ft260.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80b0e..b31c43353249 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1070,8 +1070,15 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
- ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
- xfer->length);
+ ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
+ xfer->report, xfer->length, size);
+
+ if (xfer->length > size -
+ offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
+ xfer->report, xfer->length);
+ return -1;
+ }
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
--
2.25.1
^ permalink raw reply related
* [PATCH] Input: touchscreen: tsc2007: Reduce I2C transactions for Z2 read
From: Yuki Horii @ 2026-04-10 7:41 UTC (permalink / raw)
To: dmitry.torokhov; +Cc: clamor95, johannes.kirchmair, linux-input, yuuki198708
From: Yuki Horii <yuuki198708@gmail.com>
The current implementation sends a separate power-down command
after reading the Z2 value, resulting in an extra I2C
transaction per measurement cycle.
The TSC2007 command byte contains a 2-bit power-down mode
selection field. By selecting the power-down state in the Z2
measurement command, the device powers down after the Z2 A/D
conversion completes, eliminating the subsequent power-down
transaction.
This reduces the number of I2C transactions by one per touch
measurement cycle, decreasing I2C bus overhead and improving
touch sampling performance.
Signed-off-by: Yuki Horii <yuuki198708@gmail.com>
---
drivers/input/touchscreen/tsc2007_core.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/tsc2007_core.c b/drivers/input/touchscreen/tsc2007_core.c
index 948935de894b..ff60245baa96 100644
--- a/drivers/input/touchscreen/tsc2007_core.c
+++ b/drivers/input/touchscreen/tsc2007_core.c
@@ -61,10 +61,8 @@ static void tsc2007_read_values(struct tsc2007 *tsc, struct ts_event *tc)
/* turn y+ off, x- on; we'll use formula #1 */
tc->z1 = tsc2007_xfer(tsc, READ_Z1);
- tc->z2 = tsc2007_xfer(tsc, READ_Z2);
-
- /* Prepare for next touch reading - power down ADC, enable PENIRQ */
- tsc2007_xfer(tsc, PWRDOWN);
+ /* Read Z2 and power down ADC after A/D conversion, enable PENIRQ */
+ tc->z2 = tsc2007_xfer(tsc, (TSC2007_POWER_OFF_IRQ_EN | TSC2007_MEASURE_Z2));
}
u32 tsc2007_calculate_resistance(struct tsc2007 *tsc, struct ts_event *tc)
--
2.51.0
^ permalink raw reply related
* [dtor-input:for-linus] BUILD SUCCESS 4cda78d6f8bf2b700529f2fbccb994c3e826d7c2
From: kernel test robot @ 2026-04-10 5:07 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: 4cda78d6f8bf2b700529f2fbccb994c3e826d7c2 Input: uinput - fix circular locking dependency with ff-core
elapsed time: 2870m
configs tested: 174
configs skipped: 18
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc-15.2.0
alpha allyesconfig gcc-15.2.0
alpha defconfig gcc-15.2.0
arc allmodconfig clang-16
arc allmodconfig gcc-15.2.0
arc allnoconfig gcc-15.2.0
arc allyesconfig clang-19
arc allyesconfig gcc-15.2.0
arc defconfig gcc-15.2.0
arc randconfig-001-20260408 clang-23
arc randconfig-001-20260408 gcc-8.5.0
arc randconfig-002-20260408 clang-23
arc randconfig-002-20260408 gcc-8.5.0
arm allnoconfig clang-23
arm allnoconfig gcc-15.2.0
arm allyesconfig clang-16
arm allyesconfig gcc-15.2.0
arm defconfig gcc-15.2.0
arm lpc18xx_defconfig clang-23
arm randconfig-001-20260408 clang-23
arm randconfig-001-20260408 gcc-8.5.0
arm randconfig-002-20260408 clang-23
arm randconfig-003-20260408 clang-23
arm randconfig-004-20260408 clang-23
arm64 allmodconfig clang-19
arm64 allnoconfig gcc-15.2.0
arm64 defconfig gcc-15.2.0
arm64 randconfig-001-20260408 gcc-14.3.0
arm64 randconfig-002-20260408 gcc-14.3.0
arm64 randconfig-003-20260408 gcc-14.3.0
arm64 randconfig-004-20260408 gcc-14.3.0
csky allmodconfig gcc-15.2.0
csky allnoconfig gcc-15.2.0
csky defconfig gcc-15.2.0
csky randconfig-001-20260408 gcc-14.3.0
csky randconfig-002-20260408 gcc-14.3.0
hexagon allmodconfig gcc-15.2.0
hexagon allnoconfig clang-23
hexagon allnoconfig gcc-15.2.0
hexagon defconfig gcc-15.2.0
hexagon randconfig-001-20260408 clang-23
hexagon randconfig-002-20260408 clang-23
i386 allmodconfig clang-20
i386 allnoconfig gcc-14
i386 allnoconfig gcc-15.2.0
i386 allyesconfig clang-20
i386 defconfig gcc-15.2.0
i386 randconfig-001-20260408 clang-20
i386 randconfig-002-20260408 clang-20
i386 randconfig-003-20260408 clang-20
i386 randconfig-004-20260408 clang-20
i386 randconfig-005-20260408 clang-20
i386 randconfig-006-20260408 clang-20
i386 randconfig-007-20260408 clang-20
i386 randconfig-011-20260408 gcc-14
i386 randconfig-012-20260408 gcc-14
i386 randconfig-013-20260408 gcc-14
i386 randconfig-014-20260408 gcc-14
i386 randconfig-015-20260408 gcc-14
i386 randconfig-016-20260408 gcc-14
i386 randconfig-017-20260408 gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-23
loongarch allnoconfig gcc-15.2.0
loongarch defconfig clang-19
loongarch randconfig-002-20260408 clang-23
m68k allmodconfig gcc-15.2.0
m68k allnoconfig gcc-15.2.0
m68k allyesconfig clang-16
m68k allyesconfig gcc-15.2.0
m68k defconfig clang-19
microblaze allnoconfig gcc-15.2.0
microblaze allyesconfig gcc-15.2.0
microblaze defconfig clang-19
mips allmodconfig gcc-15.2.0
mips allnoconfig gcc-15.2.0
mips allyesconfig gcc-15.2.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig clang-23
nios2 defconfig clang-19
nios2 randconfig-001-20260408 clang-23
nios2 randconfig-002-20260408 clang-23
openrisc allmodconfig gcc-11.5.0
openrisc allmodconfig gcc-15.2.0
openrisc allnoconfig clang-23
openrisc defconfig gcc-15.2.0
parisc allmodconfig gcc-15.2.0
parisc allnoconfig clang-23
parisc allyesconfig clang-19
parisc allyesconfig gcc-15.2.0
parisc defconfig gcc-15.2.0
parisc randconfig-001-20260408 gcc-8.5.0
parisc randconfig-002-20260408 gcc-8.5.0
parisc64 defconfig clang-19
powerpc allmodconfig gcc-15.2.0
powerpc allnoconfig clang-23
powerpc motionpro_defconfig clang-23
powerpc randconfig-001-20260408 gcc-8.5.0
powerpc randconfig-002-20260408 gcc-8.5.0
powerpc64 randconfig-001-20260408 gcc-8.5.0
powerpc64 randconfig-002-20260408 gcc-8.5.0
riscv allnoconfig clang-23
riscv allyesconfig clang-16
riscv defconfig gcc-15.2.0
riscv randconfig-001-20260408 gcc-15.2.0
riscv randconfig-002-20260408 gcc-15.2.0
s390 allmodconfig clang-18
s390 allmodconfig clang-19
s390 allnoconfig clang-23
s390 allyesconfig gcc-15.2.0
s390 defconfig gcc-15.2.0
sh allmodconfig gcc-15.2.0
sh allnoconfig clang-23
sh allyesconfig clang-19
sh allyesconfig gcc-15.2.0
sh defconfig gcc-14
sh randconfig-001-20260408 gcc-15.2.0
sh randconfig-002-20260408 gcc-15.2.0
sparc allnoconfig clang-23
sparc defconfig gcc-15.2.0
sparc randconfig-001-20260408 gcc-14
sparc randconfig-002-20260408 gcc-14
sparc64 allmodconfig clang-23
sparc64 defconfig gcc-14
sparc64 randconfig-001-20260408 gcc-14
sparc64 randconfig-002-20260408 gcc-14
um allmodconfig clang-19
um allnoconfig clang-23
um allyesconfig gcc-15.2.0
um defconfig gcc-14
um i386_defconfig gcc-14
um randconfig-001-20260408 gcc-14
um randconfig-002-20260408 gcc-14
x86_64 allmodconfig clang-20
x86_64 allnoconfig clang-23
x86_64 allyesconfig clang-20
x86_64 buildonly-randconfig-001-20260408 clang-20
x86_64 buildonly-randconfig-002-20260408 clang-20
x86_64 buildonly-randconfig-003-20260408 clang-20
x86_64 buildonly-randconfig-004-20260408 clang-20
x86_64 buildonly-randconfig-005-20260408 clang-20
x86_64 buildonly-randconfig-006-20260408 clang-20
x86_64 defconfig gcc-14
x86_64 kexec clang-20
x86_64 randconfig-001-20260408 gcc-12
x86_64 randconfig-002-20260408 gcc-12
x86_64 randconfig-003-20260408 gcc-12
x86_64 randconfig-004-20260408 gcc-12
x86_64 randconfig-005-20260408 gcc-12
x86_64 randconfig-006-20260408 gcc-12
x86_64 randconfig-011-20260408 clang-20
x86_64 randconfig-012-20260408 clang-20
x86_64 randconfig-013-20260408 clang-20
x86_64 randconfig-014-20260408 clang-20
x86_64 randconfig-015-20260408 clang-20
x86_64 randconfig-016-20260408 clang-20
x86_64 randconfig-071-20260408 clang-20
x86_64 randconfig-072-20260408 clang-20
x86_64 randconfig-073-20260408 clang-20
x86_64 randconfig-074-20260408 clang-20
x86_64 randconfig-075-20260408 clang-20
x86_64 randconfig-076-20260408 clang-20
x86_64 rhel-9.4 clang-20
x86_64 rhel-9.4-bpf gcc-14
x86_64 rhel-9.4-func clang-20
x86_64 rhel-9.4-kselftests clang-20
x86_64 rhel-9.4-kunit gcc-14
x86_64 rhel-9.4-ltp gcc-14
x86_64 rhel-9.4-rust clang-20
xtensa allnoconfig clang-23
xtensa allyesconfig gcc-11.5.0
xtensa allyesconfig gcc-15.2.0
xtensa randconfig-001-20260408 gcc-14
xtensa randconfig-002-20260408 gcc-14
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Vicki Pfau @ 2026-04-10 2:17 UTC (permalink / raw)
To: Silvan Jegen
Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <2Z2BPKWVI345D.3HJTS194G9TXN@homearch.localdomain>
Replies inline
On 4/8/26 12:51, Silvan Jegen wrote:
> Heyhey!
>
> Vicki Pfau <vi@endrift.com> wrote:
>> Hi,
>>
>> Replies inline
>>
>> On 4/2/26 12:09 PM, Silvan Jegen wrote:
>>> Hi
>>>
>>> Thanks for the patch!
>>>
>>> Just some comments and questions inline below.
>>>
>>> Vicki Pfau <vi@endrift.com> wrote:
>>>> This adds a new driver for the Switch 2 controllers. The Switch 2 uses an
>>>> unusual split-interface design such that input and rumble occur on the main
>>>> HID interface, but all other communication occurs over a "configuration"
>>>> interface. This is the case on both USB and Bluetooth, so this new driver
>>>> uses a split-driver design with the HID interface being the "main" driver
>>>> and the configuration interface is a secondary driver that looks up to the
>>>> HID interface, sharing resources on a common struct.
>>>>
>>>> Due to using a non-standard pairing interface as well as Bluetooth
>>>> communications being extremely limited in the kernel, a custom interface
>>>> between userspace and the kernel will need to be design, along with bringup
>>>> in BlueZ. That is beyond the scope of this initial patch, which only
>>>> contains the generic HID and USB configuration interface drivers.
>>>>
>>>> This initial work supports general input for the Joy-Con 2, Pro Controller
>>>> 2, and GameCube NSO controllers. IMU, rumble and battery support is not yet
>>>> present.
>>>>
>>>> Signed-off-by: Vicki Pfau <vi@endrift.com>
>>>> ---
>>>> MAINTAINERS | 1 +
>>>> drivers/hid/Kconfig | 11 +-
>>>> drivers/hid/hid-ids.h | 4 +
>>>> drivers/hid/hid-nintendo.c | 1194 ++++++++++++++++-
>>>> drivers/hid/hid-nintendo.h | 72 +
>>>> drivers/input/joystick/Kconfig | 11 +
>>>> drivers/input/joystick/Makefile | 1 +
>>>> drivers/input/joystick/nintendo-switch2-usb.c | 353 +++++
>>>> 8 files changed, 1637 insertions(+), 10 deletions(-)
>>>> create mode 100644 drivers/hid/hid-nintendo.h
>>>> create mode 100644 drivers/input/joystick/nintendo-switch2-usb.c
>>>>
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index 7b277d5bf3d12..4d1a28df5fd24 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -18743,6 +18743,7 @@ F: drivers/scsi/nsp32*
>>>>
>>>> NINTENDO HID DRIVER
>>>> M: Daniel J. Ogorchock <djogorchock@gmail.com>
>>>> +M: Vicki Pfau <vi@endrift.com>
>>>> L: linux-input@vger.kernel.org
>>>> S: Maintained
>>>> F: drivers/hid/hid-nintendo*
>>>> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
>>>> index c1d9f7c6a5f23..1a293a6c02c26 100644
>>>> --- a/drivers/hid/Kconfig
>>>> +++ b/drivers/hid/Kconfig
>>>> @@ -826,10 +826,13 @@ config HID_NINTENDO
>>>> depends on LEDS_CLASS
>>>> select POWER_SUPPLY
>>>> help
>>>> - Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller.
>>>> - All controllers support bluetooth, and the Pro Controller also supports
>>>> - its USB mode. This also includes support for the Nintendo Switch Online
>>>> - Controllers which include the NES, Genesis, SNES, and N64 controllers.
>>>> + Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller, as
>>>> + well as Nintendo Switch 2 Joy-Cons, Pro Controller, and NSO GameCube
>>>> + controllers. All Switch controllers support bluetooth, and the Pro
>>>> + Controller also supports its USB mode. This also includes support for
>>>> + the Nintendo Switch Online Controllers which include the NES, Genesis,
>>>> + SNES, and N64 controllers. Switch 2 controllers currently only support
>>>> + USB mode.
>>>>
>>>> To compile this driver as a module, choose M here: the
>>>> module will be called hid-nintendo.
>>>> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
>>>> index 4ab7640b119ac..a794dad7980f3 100644
>>>> --- a/drivers/hid/hid-ids.h
>>>> +++ b/drivers/hid/hid-ids.h
>>>> @@ -1073,6 +1073,10 @@
>>>> #define USB_DEVICE_ID_NINTENDO_SNESCON 0x2017
>>>> #define USB_DEVICE_ID_NINTENDO_GENCON 0x201e
>>>> #define USB_DEVICE_ID_NINTENDO_N64CON 0x2019
>>>> +#define USB_DEVICE_ID_NINTENDO_NS2_JOYCONR 0x2066
>>>> +#define USB_DEVICE_ID_NINTENDO_NS2_JOYCONL 0x2067
>>>> +#define USB_DEVICE_ID_NINTENDO_NS2_PROCON 0x2069
>>>> +#define USB_DEVICE_ID_NINTENDO_NS2_GCCON 0x2073
>>>>
>>>> #define USB_VENDOR_ID_NOVATEK 0x0603
>>>> #define USB_DEVICE_ID_NOVATEK_PCT 0x0600
>>>> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
>>>> index 29008c2cc5304..4ab8d4e7558a1 100644
>>>> --- a/drivers/hid/hid-nintendo.c
>>>> +++ b/drivers/hid/hid-nintendo.c
>>>> @@ -1,11 +1,13 @@
>>>> // SPDX-License-Identifier: GPL-2.0+
>>>> /*
>>>> - * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
>>>> + * HID driver for Nintendo Switch Joy-Cons and Pro Controllers, as well as
>>>> + * Nintendo Switch 2 Joy-Cons, Pro Controller, and GameCube Controller
>>>> *
>>>> * Copyright (c) 2019-2021 Daniel J. Ogorchock <djogorchock@gmail.com>
>>>> * Portions Copyright (c) 2020 Nadia Holmquist Pedersen <nadia@nhp.sh>
>>>> * Copyright (c) 2022 Emily Strickland <linux@emily.st>
>>>> * Copyright (c) 2023 Ryan McClelland <rymcclel@gmail.com>
>>>> + * Copyright (c) 2026 Valve Software
>>>> *
>>>> * The following resources/projects were referenced for this driver:
>>>> * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
>>>> @@ -13,6 +15,8 @@
>>>> * https://github.com/FrotBot/SwitchProConLinuxUSB
>>>> * https://github.com/MTCKC/ProconXInput
>>>> * https://github.com/Davidobot/BetterJoyForCemu
>>>> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
>>>> + * https://github.com/ndeadly/switch2_controller_research
>>>> * hid-wiimote kernel hid driver
>>>> * hid-logitech-hidpp driver
>>>> * hid-sony driver
>>>> @@ -29,6 +33,7 @@
>>>> */
>>>>
>>>> #include "hid-ids.h"
>>>> +#include "hid-nintendo.h"
>>>> #include <linux/unaligned.h>
>>>> #include <linux/delay.h>
>>>> #include <linux/device.h>
>>>> @@ -41,6 +46,8 @@
>>>> #include <linux/module.h>
>>>> #include <linux/power_supply.h>
>>>> #include <linux/spinlock.h>
>>>> +#include <linux/usb.h>
>>>> +#include "usbhid/usbhid.h"
>>>>
>>>> /*
>>>> * Reference the url below for the following HID report defines:
>>>> @@ -2614,7 +2621,7 @@ static int joycon_ctlr_handle_event(struct joycon_ctlr *ctlr, u8 *data,
>>>> return ret;
>>>> }
>>>>
>>>> -static int nintendo_hid_event(struct hid_device *hdev,
>>>> +static int joycon_event(struct hid_device *hdev,
>>>> struct hid_report *report, u8 *raw_data, int size)
>>>> {
>>>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>>>> @@ -2625,7 +2632,7 @@ static int nintendo_hid_event(struct hid_device *hdev,
>>>> return joycon_ctlr_handle_event(ctlr, raw_data, size);
>>>> }
>>>>
>>>> -static int nintendo_hid_probe(struct hid_device *hdev,
>>>> +static int joycon_probe(struct hid_device *hdev,
>>>> const struct hid_device_id *id)
>>>> {
>>>> int ret;
>>>> @@ -2729,7 +2736,7 @@ static int nintendo_hid_probe(struct hid_device *hdev,
>>>> return ret;
>>>> }
>>>>
>>>> -static void nintendo_hid_remove(struct hid_device *hdev)
>>>> +static void joycon_remove(struct hid_device *hdev)
>>>> {
>>>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>>>> unsigned long flags;
>>>> @@ -2748,7 +2755,9 @@ static void nintendo_hid_remove(struct hid_device *hdev)
>>>> hid_hw_stop(hdev);
>>>> }
>>>>
>>>> -static int nintendo_hid_resume(struct hid_device *hdev)
>>>> +#ifdef CONFIG_PM
>>>> +
>>>> +static int joycon_resume(struct hid_device *hdev)
>>>> {
>>>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>>>> int ret;
>>>> @@ -2771,7 +2780,7 @@ static int nintendo_hid_resume(struct hid_device *hdev)
>>>> return ret;
>>>> }
>>>>
>>>> -static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
>>>> +static int joycon_suspend(struct hid_device *hdev, pm_message_t message)
>>>> {
>>>> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
>>>>
>>>> @@ -2790,7 +2799,1120 @@ static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
>>>> return 0;
>>>> }
>>>>
>>>> +#endif
>>>> +
>>>> +/*
>>>> + * =============================================================================
>>>> + * Switch 2 support
>>>> + * =============================================================================
>>>> + */
>>>> +#define NS2_BTNR_B BIT(0)
>>>> +#define NS2_BTNR_A BIT(1)
>>>> +#define NS2_BTNR_Y BIT(2)
>>>> +#define NS2_BTNR_X BIT(3)
>>>> +#define NS2_BTNR_R BIT(4)
>>>> +#define NS2_BTNR_ZR BIT(5)
>>>> +#define NS2_BTNR_PLUS BIT(6)
>>>> +#define NS2_BTNR_RS BIT(7)
>>>> +
>>>> +#define NS2_BTNL_DOWN BIT(0)
>>>> +#define NS2_BTNL_RIGHT BIT(1)
>>>> +#define NS2_BTNL_LEFT BIT(2)
>>>> +#define NS2_BTNL_UP BIT(3)
>>>> +#define NS2_BTNL_L BIT(4)
>>>> +#define NS2_BTNL_ZL BIT(5)
>>>> +#define NS2_BTNL_MINUS BIT(6)
>>>> +#define NS2_BTNL_LS BIT(7)
>>>> +
>>>> +#define NS2_BTN3_C BIT(4)
>>>> +#define NS2_BTN3_SR BIT(6)
>>>> +#define NS2_BTN3_SL BIT(7)
>>>> +
>>>> +#define NS2_BTN_JCR_HOME BIT(0)
>>>> +#define NS2_BTN_JCR_GR BIT(2)
>>>> +#define NS2_BTN_JCR_C NS2_BTN3_C
>>>> +#define NS2_BTN_JCR_SR NS2_BTN3_SR
>>>> +#define NS2_BTN_JCR_SL NS2_BTN3_SL
>>>> +
>>>> +#define NS2_BTN_JCL_CAPTURE BIT(0)
>>>> +#define NS2_BTN_JCL_GL BIT(2)
>>>> +#define NS2_BTN_JCL_SR NS2_BTN3_SR
>>>> +#define NS2_BTN_JCL_SL NS2_BTN3_SL
>>>> +
>>>> +#define NS2_BTN_PRO_HOME BIT(0)
>>>> +#define NS2_BTN_PRO_CAPTURE BIT(1)
>>>> +#define NS2_BTN_PRO_GR BIT(2)
>>>> +#define NS2_BTN_PRO_GL BIT(3)
>>>> +#define NS2_BTN_PRO_C NS2_BTN3_C
>>>> +
>>>> +#define NS2_BTN_GC_HOME BIT(0)
>>>> +#define NS2_BTN_GC_CAPTURE BIT(1)
>>>> +#define NS2_BTN_GC_C NS2_BTN3_C
>>>> +
>>>> +#define NS2_TRIGGER_RANGE 4095
>>>> +#define NS2_AXIS_MIN -32768
>>>> +#define NS2_AXIS_MAX 32767
>>>> +
>>>> +#define NS2_MAX_PLAYER_ID 8
>>>> +
>>>> +#define NS2_MAX_INIT_RETRIES 4
>>>> +
>>>> +#define NS2_FLASH_ADDR_SERIAL 0x13002
>>>> +#define NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB 0x130a8
>>>> +#define NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB 0x130e8
>>>> +#define NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB 0x13140
>>>> +#define NS2_FLASH_ADDR_USER_PRIMARY_CALIB 0x1fc040
>>>> +#define NS2_FLASH_ADDR_USER_SECONDARY_CALIB 0x1fc080
>>>> +
>>>> +#define NS2_FLASH_SIZE_SERIAL 0x10
>>>> +#define NS2_FLASH_SIZE_FACTORY_AXIS_CALIB 9
>>>> +#define NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB 2
>>>> +#define NS2_FLASH_SIZE_USER_AXIS_CALIB 11
>>>> +
>>>> +#define NS2_USER_CALIB_MAGIC 0xa1b2
>>>> +
>>>> +#define NS2_FEATURE_BUTTONS BIT(0)
>>>> +#define NS2_FEATURE_ANALOG BIT(1)
>>>> +#define NS2_FEATURE_IMU BIT(2)
>>>> +#define NS2_FEATURE_MOUSE BIT(4)
>>>> +#define NS2_FEATURE_RUMBLE BIT(5)
>>>> +#define NS2_FEATURE_MAGNETO BIT(7)
>>>> +
>>>> +enum switch2_subcmd_flash {
>>>> + NS2_SUBCMD_FLASH_READ_BLOCK = 0x01,
>>>> + NS2_SUBCMD_FLASH_WRITE_BLOCK = 0x02,
>>>> + NS2_SUBCMD_FLASH_ERASE_BLOCK = 0x03,
>>>> + NS2_SUBCMD_FLASH_READ = 0x04,
>>>> + NS2_SUBCMD_FLASH_WRITE = 0x05,
>>>> +};
>>>> +
>>>> +enum switch2_subcmd_init {
>>>> + NS2_SUBCMD_INIT_SELECT_REPORT = 0xa,
>>>> + NS2_SUBCMD_INIT_USB = 0xd,
>>>> +};
>>>> +
>>>> +enum switch2_subcmd_feature_select {
>>>> + NS2_SUBCMD_FEATSEL_GET_INFO = 0x1,
>>>> + NS2_SUBCMD_FEATSEL_SET_MASK = 0x2,
>>>> + NS2_SUBCMD_FEATSEL_CLEAR_MASK = 0x3,
>>>> + NS2_SUBCMD_FEATSEL_ENABLE = 0x4,
>>>> + NS2_SUBCMD_FEATSEL_DISABLE = 0x5,
>>>> +};
>>>> +
>>>> +enum switch2_subcmd_grip {
>>>> + NS2_SUBCMD_GRIP_GET_INFO = 0x1,
>>>> + NS2_SUBCMD_GRIP_ENABLE_BUTTONS = 0x2,
>>>> + NS2_SUBCMD_GRIP_GET_INFO_EXT = 0x3,
>>>> +};
>>>> +
>>>> +enum switch2_subcmd_led {
>>>> + NS2_SUBCMD_LED_P1 = 0x1,
>>>> + NS2_SUBCMD_LED_P2 = 0x2,
>>>> + NS2_SUBCMD_LED_P3 = 0x3,
>>>> + NS2_SUBCMD_LED_P4 = 0x4,
>>>> + NS2_SUBCMD_LED_ALL_ON = 0x5,
>>>> + NS2_SUBCMD_LED_ALL_OFF = 0x6,
>>>> + NS2_SUBCMD_LED_PATTERN = 0x7,
>>>> + NS2_SUBCMD_LED_BLINK = 0x8,
>>>> +};
>>>> +
>>>> +enum switch2_subcmd_fw_info {
>>>> + NS2_SUBCMD_FW_INFO_GET = 0x1,
>>>> +};
>>>> +
>>>> +enum switch2_ctlr_type {
>>>> + NS2_CTLR_TYPE_JCL = 0x00,
>>>> + NS2_CTLR_TYPE_JCR = 0x01,
>>>> + NS2_CTLR_TYPE_PRO = 0x02,
>>>> + NS2_CTLR_TYPE_GC = 0x03,
>>>> +};
>>>> +
>>>> +enum switch2_report_id {
>>>> + NS2_REPORT_UNIFIED = 0x05,
>>>> + NS2_REPORT_JCL = 0x07,
>>>> + NS2_REPORT_JCR = 0x08,
>>>> + NS2_REPORT_PRO = 0x09,
>>>> + NS2_REPORT_GC = 0x0a,
>>>> +};
>>>> +
>>>> +enum switch2_init_step {
>>>> + NS2_INIT_READ_SERIAL,
>>>> + NS2_INIT_GET_FIRMWARE_INFO,
>>>> + NS2_INIT_READ_FACTORY_PRIMARY_CALIB,
>>>> + NS2_INIT_READ_FACTORY_SECONDARY_CALIB,
>>>> + NS2_INIT_READ_FACTORY_TRIGGER_CALIB,
>>>> + NS2_INIT_READ_USER_PRIMARY_CALIB,
>>>> + NS2_INIT_READ_USER_SECONDARY_CALIB,
>>>> + NS2_INIT_SET_FEATURE_MASK,
>>>> + NS2_INIT_ENABLE_FEATURES,
>>>> + NS2_INIT_GRIP_BUTTONS,
>>>> + NS2_INIT_REPORT_FORMAT,
>>>> + NS2_INIT_SET_PLAYER_LEDS,
>>>> + NS2_INIT_INPUT,
>>>> + NS2_INIT_FINISH,
>>>> + NS2_INIT_DONE,
>>>> +};
>>>> +
>>>> +struct switch2_version_info {
>>>> + uint8_t major;
>>>> + uint8_t minor;
>>>> + uint8_t patch;
>>>> + uint8_t ctlr_type;
>>>> + __le32 unk;
>>>> + int8_t dsp_major;
>>>> + int8_t dsp_minor;
>>>> + int8_t dsp_patch;
>>>> + int8_t dsp_type;
>>>> +};
>>>> +
>>>> +struct switch2_axis_calibration {
>>>> + uint16_t neutral;
>>>> + uint16_t negative;
>>>> + uint16_t positive;
>>>> +};
>>>> +
>>>> +struct switch2_stick_calibration {
>>>> + struct switch2_axis_calibration x;
>>>> + struct switch2_axis_calibration y;
>>>> +};
>>>> +
>>>> +struct switch2_controller {
>>>> + struct hid_device *hdev;
>>>> + struct switch2_cfg_intf *cfg;
>>>> +
>>>> + char name[64];
>>>> + char phys[64];
>>>> + struct list_head entry;
>>>> + struct mutex lock;
>>>> +
>>>> + enum switch2_ctlr_type ctlr_type;
>>>> + enum switch2_init_step init_step;
>>>> + struct input_dev __rcu *input;
>>>> + char serial[NS2_FLASH_SIZE_SERIAL + 1];
>>>> + struct switch2_version_info version;
>>>> +
>>>> + struct switch2_stick_calibration stick_calib[2];
>>>> + uint8_t lt_zero;
>>>> + uint8_t rt_zero;
>>>> +
>>>> + uint32_t player_id;
>>>> + struct led_classdev leds[4];
>>>> +};
>>>> +
>>>> +static DEFINE_MUTEX(switch2_controllers_lock);
>>>> +static LIST_HEAD(switch2_controllers);
>>>> +
>>>> +struct switch2_ctlr_button_mapping {
>>>> + uint32_t code;
>>>> + int byte;
>>>> + uint32_t bit;
>>>> +};
>>>> +
>>>> +static const struct switch2_ctlr_button_mapping ns2_left_joycon_button_mappings[] = {
>>>> + { BTN_DPAD_LEFT, 0, NS2_BTNL_LEFT, },
>>>> + { BTN_DPAD_UP, 0, NS2_BTNL_UP, },
>>>> + { BTN_DPAD_DOWN, 0, NS2_BTNL_DOWN, },
>>>> + { BTN_DPAD_RIGHT, 0, NS2_BTNL_RIGHT, },
>>>> + { BTN_TL, 0, NS2_BTNL_L, },
>>>> + { BTN_TL2, 0, NS2_BTNL_ZL, },
>>>> + { BTN_SELECT, 0, NS2_BTNL_MINUS, },
>>>> + { BTN_THUMBL, 0, NS2_BTNL_LS, },
>>>> + { KEY_RECORD, 1, NS2_BTN_JCL_CAPTURE, },
>>>> + { BTN_GRIPR, 1, NS2_BTN_JCL_SL, },
>>>> + { BTN_GRIPR2, 1, NS2_BTN_JCL_SR, },
>>>> + { BTN_GRIPL, 1, NS2_BTN_JCL_GL, },
>>>> + { /* sentinel */ },
>>>> +};
>>>> +
>>>> +static const struct switch2_ctlr_button_mapping ns2_right_joycon_button_mappings[] = {
>>>> + { BTN_SOUTH, 0, NS2_BTNR_A, },
>>>> + { BTN_EAST, 0, NS2_BTNR_B, },
>>>> + { BTN_NORTH, 0, NS2_BTNR_X, },
>>>> + { BTN_WEST, 0, NS2_BTNR_Y, },
>>>> + { BTN_TR, 0, NS2_BTNR_R, },
>>>> + { BTN_TR2, 0, NS2_BTNR_ZR, },
>>>> + { BTN_START, 0, NS2_BTNR_PLUS, },
>>>> + { BTN_THUMBR, 0, NS2_BTNR_RS, },
>>>> + { BTN_C, 1, NS2_BTN_JCR_C, },
>>>> + { BTN_MODE, 1, NS2_BTN_JCR_HOME, },
>>>> + { BTN_GRIPL2, 1, NS2_BTN_JCR_SL, },
>>>> + { BTN_GRIPL, 1, NS2_BTN_JCR_SR, },
>>>> + { BTN_GRIPR, 1, NS2_BTN_JCR_GR, },
>>>> + { /* sentinel */ },
>>>> +};
>>>> +
>>>> +static const struct switch2_ctlr_button_mapping ns2_procon_mappings[] = {
>>>> + { BTN_SOUTH, 0, NS2_BTNR_A, },
>>>> + { BTN_EAST, 0, NS2_BTNR_B, },
>>>> + { BTN_NORTH, 0, NS2_BTNR_X, },
>>>> + { BTN_WEST, 0, NS2_BTNR_Y, },
>>>> + { BTN_TL, 1, NS2_BTNL_L, },
>>>> + { BTN_TR, 0, NS2_BTNR_R, },
>>>> + { BTN_TL2, 1, NS2_BTNL_ZL, },
>>>> + { BTN_TR2, 0, NS2_BTNR_ZR, },
>>>> + { BTN_SELECT, 1, NS2_BTNL_MINUS, },
>>>> + { BTN_START, 0, NS2_BTNR_PLUS, },
>>>> + { BTN_THUMBL, 1, NS2_BTNL_LS, },
>>>> + { BTN_THUMBR, 0, NS2_BTNR_RS, },
>>>> + { BTN_MODE, 2, NS2_BTN_PRO_HOME },
>>>> + { KEY_RECORD, 2, NS2_BTN_PRO_CAPTURE },
>>>> + { BTN_GRIPR, 2, NS2_BTN_PRO_GR },
>>>> + { BTN_GRIPL, 2, NS2_BTN_PRO_GL },
>>>> + { BTN_C, 2, NS2_BTN_PRO_C },
>>>> + { /* sentinel */ },
>>>> +};
>>>> +
>>>> +static const struct switch2_ctlr_button_mapping ns2_gccon_mappings[] = {
>>>> + { BTN_SOUTH, 0, NS2_BTNR_A, },
>>>> + { BTN_EAST, 0, NS2_BTNR_B, },
>>>> + { BTN_NORTH, 0, NS2_BTNR_X, },
>>>> + { BTN_WEST, 0, NS2_BTNR_Y, },
>>>> + { BTN_TL2, 1, NS2_BTNL_L, },
>>>> + { BTN_TR2, 0, NS2_BTNR_R, },
>>>> + { BTN_TL, 1, NS2_BTNL_ZL, },
>>>> + { BTN_TR, 0, NS2_BTNR_ZR, },
>>>> + { BTN_SELECT, 1, NS2_BTNL_MINUS, },
>>>> + { BTN_START, 0, NS2_BTNR_PLUS, },
>>>> + { BTN_MODE, 2, NS2_BTN_GC_HOME },
>>>> + { KEY_RECORD, 2, NS2_BTN_GC_CAPTURE },
>>>> + { BTN_C, 2, NS2_BTN_GC_C },
>>>> + { /* sentinel */ },
>>>> +};
>>>> +
>>>> +static const uint8_t switch2_init_cmd_data[] = {
>>>> + /*
>>>> + * The last 6 bytes of this packet are the MAC address of
>>>> + * the console, but we don't need that for USB
>>>> + */
>>>> + 0x01, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
>>>> +};
>>>> +
>>>> +static const uint8_t switch2_one_data[] = { 0x01, 0x00, 0x00, 0x00 };
>>>> +
>>>> +static const uint8_t switch2_feature_mask[] = {
>>>> + NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU,
>>>> + 0x00, 0x00, 0x00
>>>> +};
>>>> +
>>>> +static void switch2_init_step_done(struct switch2_controller *ns2, enum switch2_init_step step)
>>>> +{
>>>> + if (ns2->init_step != step)
>>>> + return;
>>>> +
>>>> + ns2->init_step++;
>>>> +}
>>>> +
>>>> +static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type)
>>>> +{
>>>> + return type == NS2_CTLR_TYPE_JCL || type == NS2_CTLR_TYPE_JCR;
>>>> +}
>>>> +
>>>> +static int switch2_set_leds(struct switch2_controller *ns2)
>>>> +{
>>>> + int i;
>>>> + uint8_t message[8] = { 0 };
>>>> +
>>>> + for (i = 0; i < JC_NUM_LEDS; i++)
>>>> + message[0] |= (!!ns2->leds[i].brightness) << i;
>>>> +
>>>> + if (!ns2->cfg)
>>>> + return -ENOTCONN;
>>>> + return ns2->cfg->send_command(NS2_CMD_LED, NS2_SUBCMD_LED_PATTERN,
>>>> + &message, sizeof(message),
>>>> + ns2->cfg);
>>>> +}
>>>> +
>>>> +static int switch2_player_led_brightness_set(struct led_classdev *led,
>>>> + enum led_brightness brightness)
>>>> +{
>>>> + struct device *dev = led->dev->parent;
>>>> + struct hid_device *hdev = to_hid_device(dev);
>>>> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
>>>> +
>>>> + if (!ns2)
>>>> + return -ENODEV;
>>>> +
>>>> + guard(mutex)(&ns2->lock);
>>>> + return switch2_set_leds(ns2);
>>>> +}
>>>> +
>>>> +static void switch2_leds_create(struct switch2_controller *ns2)
>>>> +{
>>>> + struct hid_device *hdev = ns2->hdev;
>>>> + struct led_classdev *led;
>>>> + int i;
>>>> + int player_led_pattern;
>>>> +
>>>> + player_led_pattern = ns2->player_id % JC_NUM_LED_PATTERNS;
>>>> + hid_dbg(hdev, "assigned player %d led pattern", player_led_pattern + 1);
>>>> +
>>>> + for (i = 0; i < JC_NUM_LEDS; i++) {
>>>> + led = &ns2->leds[i];
>>>> + led->brightness = joycon_player_led_patterns[player_led_pattern][i];
>>>> + led->max_brightness = 1;
>>>> + led->brightness_set_blocking = switch2_player_led_brightness_set;
>>>> + led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
>>>> + }
>>>> +}
>>>> +
>>>> +static void switch2_config_buttons(struct input_dev *idev,
>>>> + const struct switch2_ctlr_button_mapping button_mappings[])
>>>> +{
>>>> + const struct switch2_ctlr_button_mapping *button;
>>>> +
>>>> + for (button = button_mappings; button->code; button++)
>>>> + input_set_capability(idev, EV_KEY, button->code);
>>>> +}
>>>> +
>>>> +static int switch2_init_input(struct switch2_controller *ns2)
>>>> +{
>>>> + struct input_dev *input;
>>>> + struct hid_device *hdev = ns2->hdev;
>>>> + int i;
>>>> + int ret;
>>>> +
>>>> + switch2_init_step_done(ns2, NS2_INIT_FINISH);
>>>> +
>>>> + rcu_read_lock();
>>>> + input = rcu_dereference(ns2->input);
>>>> + rcu_read_unlock();
>>>> +
>>>> + if (input)
>>>> + return 0;
>>>> +
>>>> + input = devm_input_allocate_device(&hdev->dev);
>>>> + if (!input)
>>>> + return -ENOMEM;
>>>> +
>>>> + input_set_drvdata(input, ns2);
>>>> + input->dev.parent = &hdev->dev;
>>>> + input->id.bustype = hdev->bus;
>>>> + input->id.vendor = hdev->vendor;
>>>> + input->id.product = hdev->product;
>>>> + input->id.version = hdev->version;
>>>> + input->uniq = ns2->serial;
>>>> + input->name = ns2->name;
>>>> + input->phys = hdev->phys;
>>>> +
>>>> + switch (ns2->ctlr_type) {
>>>> + case NS2_CTLR_TYPE_JCL:
>>>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + switch2_config_buttons(input, ns2_left_joycon_button_mappings);
>>>> + break;
>>>> + case NS2_CTLR_TYPE_JCR:
>>>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + switch2_config_buttons(input, ns2_right_joycon_button_mappings);
>>>> + break;
>>>> + case NS2_CTLR_TYPE_GC:
>>>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_RX, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_RY, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_Z, 0, NS2_TRIGGER_RANGE, 32, 128);
>>>> + input_set_abs_params(input, ABS_RZ, 0, NS2_TRIGGER_RANGE, 32, 128);
>>>> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
>>>> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
>>>> + switch2_config_buttons(input, ns2_gccon_mappings);
>>>> + break;
>>>> + case NS2_CTLR_TYPE_PRO:
>>>> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_RX, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_RY, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
>>>> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
>>>> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
>>>> + switch2_config_buttons(input, ns2_procon_mappings);
>>>> + break;
>>>> + default:
>>>> + input_free_device(input);
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + hid_info(ns2->hdev, "Firmware version %u.%u.%u (type %i)\n", ns2->version.major,
>>>> + ns2->version.minor, ns2->version.patch, ns2->version.ctlr_type);
>>>> + if (ns2->version.dsp_type >= 0)
>>>> + hid_info(ns2->hdev, "DSP version %u.%u.%u\n", ns2->version.dsp_major,
>>>> + ns2->version.dsp_minor, ns2->version.dsp_patch);
>>>> +
>>>> + ret = input_register_device(input);
>>>> + if (ret < 0) {
>>>> + hid_err(ns2->hdev, "Failed to register input; ret=%d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + for (i = 0; i < JC_NUM_LEDS; i++) {
>>>> + struct led_classdev *led = &ns2->leds[i];
>>>> + char *name = devm_kasprintf(&input->dev, GFP_KERNEL, "%s:%s:%s",
>>>> + dev_name(&input->dev),
>>>> + "green",
>>>> + joycon_player_led_names[i]);
>>>> +
>>>> + if (!name)
>>>> + return -ENOMEM;
>>>> +
>>>> + led->name = name;
>>>> + ret = devm_led_classdev_register(&input->dev, led);
>>>> + if (ret < 0) {
>>>> + dev_err(&input->dev, "Failed to register player %d LED; ret=%d\n",
>>>> + i + 1, ret);
>>>> + input_unregister_device(input);
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +
>>>> + rcu_assign_pointer(ns2->input, input);
>>>> + synchronize_rcu();
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static struct switch2_controller *switch2_get_controller(const char *phys)
>>>> +{
>>>> + struct switch2_controller *ns2;
>>>> +
>>>> + guard(mutex)(&switch2_controllers_lock);
>>>> + list_for_each_entry(ns2, &switch2_controllers, entry) {
>>>> + if (strncmp(ns2->phys, phys, sizeof(ns2->phys)) == 0)
>>>> + return ns2;
>>>> + }
>>>> + ns2 = kzalloc(sizeof(*ns2), GFP_KERNEL);
>>>> + if (!ns2)
>>>> + return ERR_PTR(-ENOMEM);
>>>> +
>>>> + mutex_init(&ns2->lock);
>>>> + INIT_LIST_HEAD(&ns2->entry);
>>>> + list_add(&ns2->entry, &switch2_controllers);
>>>> + strscpy(ns2->phys, phys, sizeof(ns2->phys));
>>>> + return ns2;
>>>> +}
>>>> +
>>>> +static void switch2_controller_put(struct switch2_controller *ns2)
>>>> +{
>>>> + struct input_dev *input;
>>>> + bool do_free;
>>>> +
>>>> + guard(mutex)(&switch2_controllers_lock);
>>>> + mutex_lock(&ns2->lock);
>>>> +
>>>> + rcu_read_lock();
>>>> + input = rcu_dereference(ns2->input);
>>>> + rcu_read_unlock();
>>>> +
>>>> + rcu_assign_pointer(ns2->input, NULL);
>>>> + synchronize_rcu();
>>>> +
>>>> + ns2->init_step = 0;
>>>> + do_free = !ns2->hdev && !ns2->cfg;
>>>> + mutex_unlock(&ns2->lock);
>>>> +
>>>> + if (input)
>>>> + input_unregister_device(input);
>>>> +
>>>> + if (do_free) {
>>>> + list_del_init(&ns2->entry);
>>>> + mutex_destroy(&ns2->lock);
>>>> + kfree(ns2);
>>>> + }
>>>> +}
>>>> +
>>>> +static bool switch2_parse_stick_calibration(struct switch2_stick_calibration *calib,
>>>> + const uint8_t *data)
>>>> +{
>>>> + static const uint8_t UNCALIBRATED[9] = {
>>>> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
>>>> + };
>>>> + if (memcmp(UNCALIBRATED, data, sizeof(UNCALIBRATED)) == 0)
>>>> + return false;
>>>> +
>>>> + calib->x.neutral = data[0];
>>>> + calib->x.neutral |= (data[1] & 0x0F) << 8;
>>>> +
>>>> + calib->y.neutral = data[1] >> 4;
>>>> + calib->y.neutral |= data[2] << 4;
>>>> +
>>>> + calib->x.positive = data[3];
>>>> + calib->x.positive |= (data[4] & 0x0F) << 8;
>>>> +
>>>> + calib->y.positive = data[4] >> 4;
>>>> + calib->y.positive |= data[5] << 4;
>>>> +
>>>> + calib->x.negative = data[6];
>>>> + calib->x.negative |= (data[7] & 0x0F) << 8;
>>>> +
>>>> + calib->y.negative = data[7] >> 4;
>>>> + calib->y.negative |= data[8] << 4;
>>>> +
>>>> + return true;
>>>> +}
>>>> +
>>>> +static void switch2_handle_flash_read(struct switch2_controller *ns2, uint8_t size,
>>>> + uint32_t address, const uint8_t *data)
>>>> +{
>>>> + bool ok;
>>>> +
>>>> + switch (address) {
>>>> + case NS2_FLASH_ADDR_SERIAL:
>>>> + if (size != NS2_FLASH_SIZE_SERIAL)
>>>> + return;
>>>> + memcpy(ns2->serial, data, size);
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_SERIAL);
>>>> + break;
>>>> + case NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB:
>>>> + if (size != NS2_FLASH_SIZE_FACTORY_AXIS_CALIB)
>>>> + return;
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_PRIMARY_CALIB);
>>>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[0], data);
>>>> + if (ok) {
>>>> + hid_dbg(ns2->hdev, "Got factory primary stick calibration:\n");
>>>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>>>> + ns2->stick_calib[0].x.negative,
>>>> + ns2->stick_calib[0].x.neutral,
>>>> + ns2->stick_calib[0].x.positive);
>>>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>>>> + ns2->stick_calib[0].y.negative,
>>>> + ns2->stick_calib[0].y.neutral,
>>>> + ns2->stick_calib[0].y.positive);
>>>> + } else {
>>>> + hid_dbg(ns2->hdev, "Factory primary stick calibration not present\n");
>>>> + }
>>>> + break;
>>>> + case NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB:
>>>> + if (size != NS2_FLASH_SIZE_FACTORY_AXIS_CALIB)
>>>> + return;
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_SECONDARY_CALIB);
>>>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[1], data);
>>>> + if (ok) {
>>>> + hid_dbg(ns2->hdev, "Got factory secondary stick calibration:\n");
>>>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>>>> + ns2->stick_calib[1].x.negative,
>>>> + ns2->stick_calib[1].x.neutral,
>>>> + ns2->stick_calib[1].x.positive);
>>>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>>>> + ns2->stick_calib[1].y.negative,
>>>> + ns2->stick_calib[1].y.neutral,
>>>> + ns2->stick_calib[1].y.positive);
>>>> + } else {
>>>> + hid_dbg(ns2->hdev, "Factory secondary stick calibration not present\n");
>>>> + }
>>>> + break;
>>>> + case NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB:
>>>> + if (size != NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB)
>>>> + return;
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_TRIGGER_CALIB);
>>>> + if (data[0] != 0xFF && data[1] != 0xFF) {
>>>> + ns2->lt_zero = data[0];
>>>> + ns2->rt_zero = data[1];
>>>> +
>>>> + hid_dbg(ns2->hdev, "Got factory trigger calibration:\n");
>>>> + hid_dbg(ns2->hdev, "Left zero point: %i\n", ns2->lt_zero);
>>>> + hid_dbg(ns2->hdev, "Right zero point: %i\n", ns2->rt_zero);
>>>> + } else {
>>>> + hid_dbg(ns2->hdev, "Factory trigger calibration not present\n");
>>>> + }
>>>> + break;
>>>> + case NS2_FLASH_ADDR_USER_PRIMARY_CALIB:
>>>> + if (size != NS2_FLASH_SIZE_USER_AXIS_CALIB)
>>>> + return;
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_PRIMARY_CALIB);
>>>> + if (__le16_to_cpu(*(__le16 *)data) != NS2_USER_CALIB_MAGIC) {
>>>> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");
>>>> + break;
>>>> + }
>>>> +
>>>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[0], &data[2]);
>>>> + if (ok) {
>>>> + hid_dbg(ns2->hdev, "Got user primary stick calibration:\n");
>>>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>>>> + ns2->stick_calib[0].x.negative,
>>>> + ns2->stick_calib[0].x.neutral,
>>>> + ns2->stick_calib[0].x.positive);
>>>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>>>> + ns2->stick_calib[0].y.negative,
>>>> + ns2->stick_calib[0].y.neutral,
>>>> + ns2->stick_calib[0].y.positive);
>>>> + } else {
>>>> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");
>>>> + }
>>>> + break;
>>>> + case NS2_FLASH_ADDR_USER_SECONDARY_CALIB:
>>>> + if (size != NS2_FLASH_SIZE_USER_AXIS_CALIB)
>>>> + return;
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_SECONDARY_CALIB);
>>>> + if (__le16_to_cpu(*(__le16 *)data) != NS2_USER_CALIB_MAGIC) {
>>>> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n");
>>>> + break;
>>>> + }
>>>> +
>>>> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[1], &data[2]);
>>>> + if (ok) {
>>>> + hid_dbg(ns2->hdev, "Got user secondary stick calibration:\n");
>>>> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
>>>> + ns2->stick_calib[1].x.negative,
>>>> + ns2->stick_calib[1].x.neutral,
>>>> + ns2->stick_calib[1].x.positive);
>>>> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
>>>> + ns2->stick_calib[1].y.negative,
>>>> + ns2->stick_calib[1].y.neutral,
>>>> + ns2->stick_calib[1].y.positive);
>>>> + } else {
>>>> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n");
>>>> + }
>>>> + break;
>>>> + }
>>>> +}
>>>> +
>>>> +static void switch2_report_buttons(struct input_dev *input, const uint8_t *bytes,
>>>> + const struct switch2_ctlr_button_mapping button_mappings[])
>>>> +{
>>>> + const struct switch2_ctlr_button_mapping *button;
>>>> +
>>>> + for (button = button_mappings; button->code; button++)
>>>> + input_report_key(input, button->code, bytes[button->byte] & button->bit);
>>>> +}
>>>> +
>>>> +static void switch2_report_axis(struct input_dev *input, struct switch2_axis_calibration *calib,
>>>> + int axis, bool invert, int value)
>>>> +{
>>>> + if (calib && calib->neutral && calib->negative && calib->positive) {
>>>> + value -= calib->neutral;
>>>> + value *= NS2_AXIS_MAX + 1;
>>>> + if (value < 0)
>>>> + value /= calib->negative;
>>>> + else
>>>> + value /= calib->positive;
>>>> + } else {
>>>> + value = (value - 2048) * 16;
>>>> + }
>>>> +
>>>> + if (invert)
>>>> + value = -value;
>>>> + input_report_abs(input, axis,
>>>> + clamp(value, NS2_AXIS_MIN, NS2_AXIS_MAX));
>>>> +}
>>>> +
>>>> +static void switch2_report_stick(struct input_dev *input, struct switch2_stick_calibration *calib,
>>>> + int x, bool invert_x, int y, bool invert_y, const uint8_t *data)
>>>> +{
>>>> + switch2_report_axis(input, &calib->x, x, invert_x, data[0] | ((data[1] & 0x0F) << 8));
>>>> + switch2_report_axis(input, &calib->y, y, invert_y, (data[1] >> 4) | (data[2] << 4));
>>>> +}
>>>> +
>>>> +static void switch2_report_trigger(struct input_dev *input, uint8_t zero, int abs, uint8_t data)
>>>> +{
>>>> + int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
>>>> +
>>>> + input_report_abs(input, abs, clamp(value, 0, NS2_TRIGGER_RANGE));
>>>> +}
>>>> +
>>>> +static int switch2_event(struct hid_device *hdev, struct hid_report *report, uint8_t *raw_data,
>>>> + int size)
>>>> +{
>>>> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
>>>> + struct input_dev *input;
>>>> +
>>>> + if (report->type != HID_INPUT_REPORT)
>>>> + return 0;
>>>> +
>>>> + if (size < 15)
>>>> + return -EINVAL;
>>>> +
>>>> + guard(rcu)();
>>>> + input = rcu_dereference(ns2->input);
>>>> +
>>>> + if (!input)
>>>> + return 0;
>>>> +
>>>> + switch (report->id) {
>>>> + case NS2_REPORT_UNIFIED:
>>>> + /*
>>>> + * TODO
>>>> + * This won't be sent unless the report type gets changed via command
>>>> + * 03-0A, but we should support it at some point regardless.
>>>> + */
>>>> + break;
>>>> + case NS2_REPORT_JCL:
>>>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>>>> + ABS_Y, true, &raw_data[6]);
>>>> + switch2_report_buttons(input, &raw_data[3], ns2_left_joycon_button_mappings);
>>>> + break;
>>>> + case NS2_REPORT_JCR:
>>>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>>>> + ABS_Y, true, &raw_data[6]);
>>>> + switch2_report_buttons(input, &raw_data[3], ns2_right_joycon_button_mappings);
>>>> + break;
>>>> + case NS2_REPORT_GC:
>>>> + input_report_abs(input, ABS_HAT0X,
>>>> + !!(raw_data[4] & NS2_BTNL_RIGHT) -
>>>> + !!(raw_data[4] & NS2_BTNL_LEFT));
>>>> + input_report_abs(input, ABS_HAT0Y,
>>>> + !!(raw_data[4] & NS2_BTNL_DOWN) -
>>>> + !!(raw_data[4] & NS2_BTNL_UP));
>>>> + switch2_report_buttons(input, &raw_data[3], ns2_gccon_mappings);
>>>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>>>> + ABS_Y, true, &raw_data[6]);
>>>> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, false,
>>>> + ABS_RY, true, &raw_data[9]);
>>>> + switch2_report_trigger(input, ns2->lt_zero, ABS_Z, raw_data[13]);
>>>> + switch2_report_trigger(input, ns2->rt_zero, ABS_RZ, raw_data[14]);
>>>> + break;
>>>> + case NS2_REPORT_PRO:
>>>> + input_report_abs(input, ABS_HAT0X,
>>>> + !!(raw_data[4] & NS2_BTNL_RIGHT) -
>>>> + !!(raw_data[4] & NS2_BTNL_LEFT));
>>>> + input_report_abs(input, ABS_HAT0Y,
>>>> + !!(raw_data[4] & NS2_BTNL_DOWN) -
>>>> + !!(raw_data[4] & NS2_BTNL_UP));
>>>> + switch2_report_buttons(input, &raw_data[3], ns2_procon_mappings);
>>>> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
>>>> + ABS_Y, true, &raw_data[6]);
>>>> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, false,
>>>> + ABS_RY, true, &raw_data[9]);
>>>> + break;
>>>> + default:
>>>> + return -EINVAL;
>>>> + }
>>>> +
>>>> + input_sync(input);
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int switch2_features_enable(struct switch2_controller *ns2, int features)
>>>> +{
>>>> + __le32 feature_bits = __cpu_to_le32(features);
>>>> +
>>>> + if (!ns2->cfg)
>>>> + return -ENOTCONN;
>>>> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_ENABLE,
>>>> + &feature_bits, sizeof(feature_bits),
>>>> + ns2->cfg);
>>>> +}
>>>> +
>>>> +static int switch2_read_flash(struct switch2_controller *ns2, uint32_t address,
>>>> + uint8_t size)
>>>> +{
>>>> + uint8_t message[8] = { size, 0x7e };
>>>> +
>>>> + if (!ns2->cfg)
>>>> + return -ENOTCONN;
>>>> + *(__le32 *)&message[4] = __cpu_to_le32(address);
>>>> + return ns2->cfg->send_command(NS2_CMD_FLASH, NS2_SUBCMD_FLASH_READ, message,
>>>> + sizeof(message), ns2->cfg);
>>>> +}
>>>> +
>>>> +static int switch2_set_player_id(struct switch2_controller *ns2, uint32_t player_id)
>>>> +{
>>>> + int i;
>>>> + int player_led_pattern = player_id % JC_NUM_LED_PATTERNS;
>>>> +
>>>> + for (i = 0; i < JC_NUM_LEDS; i++)
>>>> + ns2->leds[i].brightness = joycon_player_led_patterns[player_led_pattern][i];
>>>> +
>>>> + return switch2_set_leds(ns2);
>>>> +}
>>>> +
>>>> +static int switch2_set_report_format(struct switch2_controller *ns2, enum switch2_report_id fmt)
>>>> +{
>>>> + __le32 format_id = __cpu_to_le32(fmt);
>>>> +
>>>> + if (!ns2->cfg)
>>>> + return -ENOTCONN;
>>>> + return ns2->cfg->send_command(NS2_CMD_INIT, NS2_SUBCMD_INIT_SELECT_REPORT,
>>>> + &format_id, sizeof(format_id),
>>>> + ns2->cfg);
>>>> +}
>>>> +
>>>> +static int switch2_init_controller(struct switch2_controller *ns2)
>>>
>>> This is now a recursive call while in v1 it wasn't. I think I preferred
>>> the non-recursive version as there was one place where init_step
>>> state was changed while now I am not sure where it happens (and whether
>>> there is a code path where we end up in an infinite recursion)
>>>
>>> What is the advantage of the recursive version compared to the
>>> non-recursive one?
> >
>>
>> The old version incremented the step regardless of whether or not it
>> could confirm it had happened. Since the confirmation is now handled
>> with an external step, calling into switch2_init_step_done, the loop
>> condition would become somewhat complicated.
>> I replaced it with explicit tail calls since that make the
>> control flow simplier, and it is always matched with a call to
>> switch2_init_step_done to ensure that the state is always advanced. As
>
> From what I can tell switch2_init_step_done currently only advances
> the state if the current state is the expected one. This seems fine,
> but it also means that if the state is not the expected one, the
> state is not advanced and the recursive call continues anyway (in the
> NS2_INIT_READ_USER_SECONDARY_CALIB case, for example). I assume this
> should never happen but if we end up in this case for some reason we
> will recurse forever.
That's correct, and the only way it would happen forever is if there's a
bug. The same would be true in a loop version if it doesn't advance the
state properly either, fwiw, which happened during development of this
version. Regardless, I can reduce the chance of introducing such a bug
by passing ns2->init_step instead of a constant, so I'll make that
change in v4.
>
> The same case also potentially calls switch2_read_flash and it isn't
> clear to me if this means that the initialisation is done (as there
> is no switch2_init_step_done call and we are not in the FINISH state
> either). There is also the possibility of switch2_read_flash calling
> switch2_init_controller again, which one then has to check ... (note
> that this is not the case here though)
switch2_handle_flash_read will advance the state once it's verified that
the read actually happened. If the step failed for whatever reason, this
same codepath will retry the specific read, as the caller
(switch2_receive_command) will always call into switch2_init_controller
if setup isn't done. This is how the retry logic works.
>
> To me it seems like it would be clearer to do a `ns2->init_step++` and
> then `continue` to make the progress of the state more visible and to
> do an explicit `break` when we are supposed to stop the initialisation.
>
The problem with the loop approach, in my opinion is due to the fact
that the loop is the *exception*, not the rule. The loop idiom makes it
look like a loop is expected. Further the ns2->init_step++ in the
previous version means that the verification does not occur, so in the
case of any sort of failure it'll plow ahead anyway instead of retrying.
The point of this approach is to avoid that.
>
>> such, the number recursive calls is explicitly limited, and the fact
>> that they're tail calls should mean that it doesn't increase the stack
>> depth (unless there's something I don't know about how the kernel
>> is compiled, which is possible in the wake of things like retpoline
>> protections).
>
> I actually wasn't aware that the tail call optimisation means that no
> new stack frame will be added, neat!
>
> I don't know how or if retpoline has an impact on this optimisation
> either, I am afraid.
>
>
>> I suppose I could replace it with a loop, but the condition would be
>> the same so the only real difference would be a `while (ns2->init_step
>> < NS2_INIT_DONE)` instead of the tail calls; the tail calls themselves
>> would just become break statements. There would be no functional
>> difference.
>
> hm, wouldn't the tail calls become "continue" statements (as the loop
> version would just start a new iteration there)?
Yes, it'd be (roughly) equivalent. It'd just read differently.
>
> I agree that both versions can definitely be implemented in an equivalent
> manner with no functional differences, but I do have my preference :)
>
> This is not my call to make anyways though, so let's wait for others to
> chime in.
>
>
>>
>>>> +{
>>>> + if (ns2->init_step == NS2_INIT_DONE)
>>>> + return 0;
>>>> +
>>>> + if (!ns2->cfg)
>>>> + return -ENOTCONN;
>>>> +
>>>> + switch (ns2->init_step) {
>>>> + case NS2_INIT_READ_SERIAL:
>>>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_SERIAL,
>>>> + NS2_FLASH_SIZE_SERIAL);
>>>> + case NS2_INIT_GET_FIRMWARE_INFO:
>>>> + return ns2->cfg->send_command(NS2_CMD_FW_INFO, NS2_SUBCMD_FW_INFO_GET,
>>>> + NULL, 0, ns2->cfg);
>>>> + case NS2_INIT_READ_FACTORY_PRIMARY_CALIB:
>>>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB,
>>>> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB);
>>>> + case NS2_INIT_READ_FACTORY_SECONDARY_CALIB:
>>>> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_SECONDARY_CALIB);
>>>> + return switch2_init_controller(ns2);
>>>> + }
>>>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB,
>>>> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB);
>>>> + case NS2_INIT_READ_FACTORY_TRIGGER_CALIB:
>>>> + if (ns2->ctlr_type != NS2_CTLR_TYPE_GC) {
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_TRIGGER_CALIB);
>>>> + return switch2_init_controller(ns2);
>>>> + }
>>>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB,
>>>> + NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB);
>>>> + case NS2_INIT_READ_USER_PRIMARY_CALIB:
>>>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_USER_PRIMARY_CALIB,
>>>> + NS2_FLASH_SIZE_USER_AXIS_CALIB);
>>>> + case NS2_INIT_READ_USER_SECONDARY_CALIB:
>>>> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>>>> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_SECONDARY_CALIB);
>>>> + return switch2_init_controller(ns2);
>>>> + }
>>>> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_USER_SECONDARY_CALIB,
>>>> + NS2_FLASH_SIZE_USER_AXIS_CALIB);
>>>> + case NS2_INIT_SET_FEATURE_MASK:
>>>> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_SET_MASK,
>>>> + switch2_feature_mask, sizeof(switch2_feature_mask), ns2->cfg);
>>>> + case NS2_INIT_ENABLE_FEATURES:
>>>> + return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG);
>>>> + case NS2_INIT_GRIP_BUTTONS:
>>>> + if (!switch2_ctlr_is_joycon(ns2->ctlr_type)) {
>>>> + switch2_init_step_done(ns2, NS2_INIT_GRIP_BUTTONS);
>>>> + return switch2_init_controller(ns2);
>>>> + }
>>>> + return ns2->cfg->send_command(NS2_CMD_GRIP, NS2_SUBCMD_GRIP_ENABLE_BUTTONS,
>>>> + switch2_one_data, sizeof(switch2_one_data),
>>>> + ns2->cfg);
>>>> + case NS2_INIT_REPORT_FORMAT:
>>>> + switch (ns2->ctlr_type) {
>>>> + case NS2_CTLR_TYPE_JCL:
>>>> + return switch2_set_report_format(ns2, NS2_REPORT_JCL);
>>>> + case NS2_CTLR_TYPE_JCR:
>>>> + return switch2_set_report_format(ns2, NS2_REPORT_JCR);
>>>> + case NS2_CTLR_TYPE_PRO:
>>>> + return switch2_set_report_format(ns2, NS2_REPORT_PRO);
>>>> + case NS2_CTLR_TYPE_GC:
>>>> + return switch2_set_report_format(ns2, NS2_REPORT_GC);
>>>> + default:
>>>> + switch2_init_step_done(ns2, NS2_INIT_REPORT_FORMAT);
>>>> + return switch2_init_controller(ns2);
>>>> + }
>>>> + case NS2_INIT_SET_PLAYER_LEDS:
>>>> + return switch2_set_player_id(ns2, ns2->player_id);
>>>> + case NS2_INIT_INPUT:
>>>> + return ns2->cfg->send_command(NS2_CMD_INIT, NS2_SUBCMD_INIT_USB,
>>>> + switch2_init_cmd_data, sizeof(switch2_init_cmd_data), ns2->cfg);
>>>> + case NS2_INIT_FINISH:
>>>> + if (ns2->hdev)
>>>
>>> If this is not set we skip the switch2_init_input call but don't error
>>> out. Is this intentional (are we expecting ns2->hdev to be populated at
>>> a later time and this step retried, perhaps)?
>>
>> This is intentional, yes. There are a handful of places this can
>> get called, including the function that sets ns2->hdev in the first
>> place (switch2_probe). It's to ensure the steps start as soon as
>> possible (when the cfg pointer gets set) but it can't finish until the
>> hdev gets set, which can be either before or after, depending on the
>> ordering the interfaces get enumerated.
>
> I'm probably not correctly following the logic in this case, but if the
> steps can only finish once ns2->hdev is set and we are `break;`-ing here
> in case it's not, doesn't that mean we never retry (since there is no
> recursive call after)?
We retry in switch2_probe. It calls switch2_init_controller at the end
of it, so we hit that step again, and now since ns2->hdev is necessarily
filled it continues.
>
>>
>>>
>>>> + return switch2_init_input(ns2);
>>>> + break;
>>>> + default:
>>>> + WARN_ON_ONCE(1);
>>>> + break;
>>>> + }
>>>> + return 0;
>>>> +}
>>>> +
>>>> +int switch2_receive_command(struct switch2_controller *ns2,
>>>> + const uint8_t *message, size_t length)
>>>> +{
>>>> + const struct switch2_cmd_header *header;
>>>> + int ret = 0;
>>>> +
>>>> + if (length < 8)
>>>> + return -EINVAL;
>>>> +
>>>> + print_hex_dump_debug("got cmd: ", DUMP_PREFIX_OFFSET, 16, 1, message, length, false);
>>>> +
>>>> + guard(mutex)(&ns2->lock);
>>>> +
>>>> + header = (const struct switch2_cmd_header *)message;
>>>> + if (!(header->flags & NS2_FLAG_OK)) {
>>>> + ret = -EIO;
>>>> + goto exit;
>>>> + }
>>>> + message = &message[8];
>>>> + switch (header->command) {
>>>> + case NS2_CMD_FLASH:
>>>> + if (header->subcommand == NS2_SUBCMD_FLASH_READ) {
>>>> + uint8_t read_size;
>>>> + uint32_t read_address;
>>>> +
>>>> + if (length < 16) {
>>>> + ret = -EINVAL;
>>>> + goto exit;
>>>> + }
>>>> + read_size = message[0];
>>>> + read_address = __le32_to_cpu(*(__le32 *)&message[4]);
>>>> + if (length < read_size + 16) {
>>>> + ret = -EINVAL;
>>>> + goto exit;
>>>> + }
>>>> + switch2_handle_flash_read(ns2, read_size, read_address, &message[8]);
>>>> + }
>>>> + break;
>>>> + case NS2_CMD_INIT:
>>>> + if (header->subcommand == NS2_SUBCMD_INIT_USB)
>>>> + switch2_init_step_done(ns2, NS2_INIT_INPUT);
>>>> + else if (header->subcommand == NS2_SUBCMD_INIT_SELECT_REPORT)
>>>> + switch2_init_step_done(ns2, NS2_INIT_REPORT_FORMAT);
>>>> + break;
>>>> + case NS2_CMD_GRIP:
>>>> + if (header->subcommand == NS2_SUBCMD_GRIP_ENABLE_BUTTONS)
>>>> + switch2_init_step_done(ns2, NS2_INIT_GRIP_BUTTONS);
>>>> + break;
>>>> + case NS2_CMD_LED:
>>>> + if (header->subcommand == NS2_SUBCMD_LED_PATTERN)
>>>> + switch2_init_step_done(ns2, NS2_INIT_SET_PLAYER_LEDS);
>>>> + break;
>>>> + case NS2_CMD_FEATSEL:
>>>> + if (header->subcommand == NS2_SUBCMD_FEATSEL_SET_MASK)
>>>> + switch2_init_step_done(ns2, NS2_INIT_SET_FEATURE_MASK);
>>>> + else if (header->subcommand == NS2_SUBCMD_FEATSEL_ENABLE)
>>>> + switch2_init_step_done(ns2, NS2_INIT_ENABLE_FEATURES);
>>>> + break;
>>>> + case NS2_CMD_FW_INFO:
>>>> + if (header->subcommand == NS2_SUBCMD_FW_INFO_GET) {
>>>> + if (length < sizeof(ns2->version)) {
>>>> + ret = -EINVAL;
>>>> + goto exit;
>>>> + }
>>>> + memcpy(&ns2->version, message, sizeof(ns2->version));
>>>> + ns2->ctlr_type = ns2->version.ctlr_type;
>>>> + switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
>>>> + }
>>>> + break;
>>>> + default:
>>>> + break;
>>>> + }
>>>> +
>>>> +exit:
>>>> + if (ns2->init_step < NS2_INIT_DONE)
>>>> + switch2_init_controller(ns2);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(switch2_receive_command);
>>>> +
>>>> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg)
>>>> +{
>>>> + struct switch2_controller *ns2 = switch2_get_controller(phys);
>>>> +
>>>> + if (IS_ERR(ns2))
>>>> + return PTR_ERR(ns2);
>>>> +
>>>> + cfg->parent = ns2;
>>>> +
>>>> + guard(mutex)(&ns2->lock);
>>>> + WARN_ON(ns2->cfg);
>>>> + ns2->cfg = cfg;
>>>> +
>>>> + if (ns2->hdev)
>>>> + return switch2_init_controller(ns2);
>>>> + return 0;
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(switch2_controller_attach_cfg);
>>>> +
>>>> +void switch2_controller_detach_cfg(struct switch2_controller *ns2)
>>>> +{
>>>> + mutex_lock(&ns2->lock);
>>>> + WARN_ON(ns2 != ns2->cfg->parent);
>>>> + ns2->cfg = NULL;
>>>> + mutex_unlock(&ns2->lock);
>>>> + switch2_controller_put(ns2);
>>>> +}
>>>> +EXPORT_SYMBOL_GPL(switch2_controller_detach_cfg);
>>>> +
>>>> +static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>>> +{
>>>> + struct switch2_controller *ns2;
>>>> + struct usb_device *udev;
>>>> + char phys[64];
>>>> + int ret;
>>>> +
>>>> + if (!hid_is_usb(hdev))
>>>> + return -ENODEV;
>>>> +
>>>> + udev = hid_to_usb_dev(hdev);
>>>> + if (usb_make_path(udev, phys, sizeof(phys)) < 0)
>>>> + return -EINVAL;
>>>> +
>>>> + ret = hid_parse(hdev);
>>>> + if (ret) {
>>>> + hid_err(hdev, "parse failed %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>>>> + if (ret) {
>>>> + hid_err(hdev, "hw_start failed %d\n", ret);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = hid_hw_open(hdev);
>>>> + if (ret) {
>>>> + hid_err(hdev, "hw_open failed %d\n", ret);
>>>> + goto err_stop;
>>>> + }
>>>
>>> For the Switch 1 controllers we are calling hid_device_io_start after
>>> hid_hw_open. Is this not necessary in this case?
>>
>> Since we don't do any HID I/O during probe it's not needed; the
>> startup configuration is on the cfg interface instead.
>
> Ah, I can see that for the Switch 1 controllers we are sending USB
> HID packets during probe, while we don't do this for the Switch 2
> controllers. That explains why this call is not needed here.
>
>>
>>>
>>>> +
>>>> + ns2 = switch2_get_controller(phys);
>>>> + if (!ns2) {
>>>
>>> switch2_get_controller returns an err pointer in case of ENOMEM, not
>>> NULL so I think this check has to be changed.
>>
>> Fixed locally, thanks. I'll send that out with v4 (this was actually
>> v3 but I thought it was v2, oops).
>>
>>>
>>>> + ret = -ENOMEM;
>>>> + goto err_close;
>>>> + }
>>>> +
>>>> + guard(mutex)(&ns2->lock);
>>>> + WARN_ON(ns2->hdev);
>>>> + ns2->hdev = hdev;
>>>> + switch (hdev->product | (hdev->vendor << 16)) {
>>>> + default:
>>>> + strscpy(ns2->name, hdev->name, sizeof(ns2->name));
>>>> + break;
>>>> + /* Some controllers have slightly wrong names so we override them */
>>>> + case USB_DEVICE_ID_NINTENDO_NS2_JOYCONR | (USB_VENDOR_ID_NINTENDO << 16):
>>>> + /* Missing the "2" in the name */
>>>> + strscpy(ns2->name, "Nintendo Joy-Con 2 (R)", sizeof(ns2->name));
>>>> + break;
>>>> + case USB_DEVICE_ID_NINTENDO_NS2_GCCON | (USB_VENDOR_ID_NINTENDO << 16):
>>>> + /* Has "Nintendo" in the name twice */
>>>> + strscpy(ns2->name, "Nintendo GameCube Controller", sizeof(ns2->name));
>>>> + break;
>>>> + }
>>>> +
>>>> + ns2->player_id = U32_MAX;
>>>> + ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
>>>> + if (ret < 0)
>>>> + hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
>>>> + else
>>>> + ns2->player_id = ret;
>>>> +
>>>> + switch2_leds_create(ns2);
>>>> +
>>>> + hid_set_drvdata(hdev, ns2);
>>>> +
>>>> + if (ns2->cfg)
>>>> + return switch2_init_controller(ns2);
>>>> +
>>>> + return 0;
>>>> +
>>>> +err_close:
>>>> + hid_hw_close(hdev);
>>>> +err_stop:
>>>> + hid_hw_stop(hdev);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static void switch2_remove(struct hid_device *hdev)
>>>> +{
>>>> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
>>>> +
>>>> + hid_hw_close(hdev);
>>>> + mutex_lock(&ns2->lock);
>>>> + WARN_ON(ns2->hdev != hdev);
>>>> + ns2->hdev = NULL;
>>>> + mutex_unlock(&ns2->lock);
>>>> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
>>>> + switch2_controller_put(ns2);
>>>> + hid_hw_stop(hdev);
>>>> +}
>>>> +
>>>> static const struct hid_device_id nintendo_hid_devices[] = {
>>>> + /* Switch devices */
>>>> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> USB_DEVICE_ID_NINTENDO_PROCON) },
>>>> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> @@ -2813,10 +3935,69 @@ static const struct hid_device_id nintendo_hid_devices[] = {
>>>> USB_DEVICE_ID_NINTENDO_GENCON) },
>>>> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> USB_DEVICE_ID_NINTENDO_N64CON) },
>>>> + /* Switch 2 devices */
>>>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> + USB_DEVICE_ID_NINTENDO_NS2_JOYCONL) },
>>>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> + USB_DEVICE_ID_NINTENDO_NS2_JOYCONR) },
>>>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> + USB_DEVICE_ID_NINTENDO_NS2_PROCON) },
>>>> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
>>>> + USB_DEVICE_ID_NINTENDO_NS2_GCCON) },
>>>> { }
>>>> };
>>>> MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
>>>>
>>>> +static bool nintendo_is_switch2(struct hid_device *hdev)
>>>> +{
>>>> + return hdev->vendor == USB_VENDOR_ID_NINTENDO &&
>>>> + hdev->product >= USB_DEVICE_ID_NINTENDO_NS2_JOYCONR;
>>>> +}
>>>> +
>>>> +static void nintendo_hid_remove(struct hid_device *hdev)
>>>> +{
>>>> + if (nintendo_is_switch2(hdev))
>>>> + switch2_remove(hdev);
>>>> + else
>>>> + joycon_remove(hdev);
>>>> +}
>>>> +
>>>> +static int nintendo_hid_event(struct hid_device *hdev,
>>>> + struct hid_report *report, u8 *raw_data, int size)
>>>> +{
>>>> + if (nintendo_is_switch2(hdev))
>>>> + return switch2_event(hdev, report, raw_data, size);
>>>> + else
>>>> + return joycon_event(hdev, report, raw_data, size);
>>>> +}
>>>> +
>>>> +static int nintendo_hid_probe(struct hid_device *hdev,
>>>> + const struct hid_device_id *id)
>>>> +{
>>>> + if (nintendo_is_switch2(hdev))
>>>> + return switch2_probe(hdev, id);
>>>> + else
>>>> + return joycon_probe(hdev, id);
>>>> +}
>>>> +
>>>> +#ifdef CONFIG_PM
>>>> +static int nintendo_hid_resume(struct hid_device *hdev)
>>>> +{
>>>> + if (nintendo_is_switch2(hdev))
>>>> + return 0;
>>>> + else
>>>> + return joycon_resume(hdev);
>>>> +}
>>>> +
>>>> +static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
>>>> +{
>>>> + if (nintendo_is_switch2(hdev))
>>>> + return 0;
>>>> + else
>>>> + return joycon_suspend(hdev, message);
>>>> +}
>>>> +#endif
>>>> +
>>>> static struct hid_driver nintendo_hid_driver = {
>>>> .name = "nintendo",
>>>> .id_table = nintendo_hid_devices,
>>>> @@ -2844,4 +4025,5 @@ MODULE_LICENSE("GPL");
>>>> MODULE_AUTHOR("Ryan McClelland <rymcclel@gmail.com>");
>>>> MODULE_AUTHOR("Emily Strickland <linux@emily.st>");
>>>> MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
>>>> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
>>>> MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
>>>> diff --git a/drivers/hid/hid-nintendo.h b/drivers/hid/hid-nintendo.h
>>>> new file mode 100644
>>>> index 0000000000000..7aff22f302661
>>>> --- /dev/null
>>>> +++ b/drivers/hid/hid-nintendo.h
>>>> @@ -0,0 +1,72 @@
>>>> +/* SPDX-License-Identifier: GPL-2.0+ */
>>>> +/*
>>>> + * HID driver for Nintendo Switch 2 controllers
>>>> + *
>>>> + * Copyright (c) 2025 Valve Software
>>>> + *
>>>> + * This driver is based on the following work:
>>>> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
>>>> + * https://github.com/ndeadly/switch2_controller_research
>>>> + */
>>>> +
>>>> +#ifndef __HID_NINTENDO_H
>>>> +#define __HID_NINTENDO_H
>>>> +
>>>> +#include <linux/bits.h>
>>>> +
>>>> +#define NS2_FLAG_OK BIT(0)
>>>> +#define NS2_FLAG_NACK BIT(2)
>>>> +
>>>> +enum switch2_cmd {
>>>> + NS2_CMD_NFC = 0x01,
>>>> + NS2_CMD_FLASH = 0x02,
>>>> + NS2_CMD_INIT = 0x03,
>>>> + NS2_CMD_GRIP = 0x08,
>>>> + NS2_CMD_LED = 0x09,
>>>> + NS2_CMD_VIBRATE = 0x0a,
>>>> + NS2_CMD_BATTERY = 0x0b,
>>>> + NS2_CMD_FEATSEL = 0x0c,
>>>> + NS2_CMD_FW_UPD = 0x0d,
>>>> + NS2_CMD_FW_INFO = 0x10,
>>>> + NS2_CMD_BT_PAIR = 0x15,
>>>> +};
>>>> +
>>>> +enum switch2_direction {
>>>> + NS2_DIR_IN = 0x00,
>>>> + NS2_DIR_OUT = 0x90,
>>>> +};
>>>> +
>>>> +enum switch2_transport {
>>>> + NS2_TRANS_USB = 0x00,
>>>> + NS2_TRANS_BT = 0x01,
>>>> +};
>>>> +
>>>> +struct switch2_cmd_header {
>>>> + uint8_t command;
>>>> + uint8_t flags;
>>>> + uint8_t transport;
>>>> + uint8_t subcommand;
>>>> + uint8_t unk1;
>>>> + uint8_t length;
>>>> + uint16_t unk2;
>>>> +};
>>>> +static_assert(sizeof(struct switch2_cmd_header) == 8);
>>>> +
>>>> +struct device;
>>>> +struct switch2_controller;
>>>> +struct switch2_cfg_intf {
>>>> + struct switch2_controller *parent;
>>>> + struct device *dev;
>>>> +
>>>> + int (*send_command)(enum switch2_cmd command, uint8_t subcommand,
>>>> + const void *message, size_t length,
>>>> + struct switch2_cfg_intf *intf);
>>>> +};
>>>> +
>>>> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg);
>>>> +void switch2_controller_detach_cfg(struct switch2_controller *controller);
>>>> +
>>>> +int switch2_receive_command(struct switch2_controller *controller,
>>>> + const uint8_t *message, size_t length);
>>>> +
>>>> +#endif
>>>> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
>>>> index 7755e5b454d2c..868262c6ccd9a 100644
>>>> --- a/drivers/input/joystick/Kconfig
>>>> +++ b/drivers/input/joystick/Kconfig
>>>> @@ -422,4 +422,15 @@ config JOYSTICK_SEESAW
>>>> To compile this driver as a module, choose M here: the module will be
>>>> called adafruit-seesaw.
>>>>
>>>> +config JOYSTICK_NINTENDO_SWITCH2_USB
>>>> + tristate "Wired Nintendo Switch 2 controller support"
>>>> + depends on HID_NINTENDO
>>>> + depends on USB
>>>> + help
>>>> + Say Y here if you want to enable support for wired Nintendo Switch 2
>>>> + controllers.
>>>> +
>>>> + To compile this driver as a module, choose M here: the
>>>> + module will be called nintendo-switch2-usb.
>>>> +
>>>> endif
>>>> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
>>>> index 9976f596a9208..8f92900ae8856 100644
>>>> --- a/drivers/input/joystick/Makefile
>>>> +++ b/drivers/input/joystick/Makefile
>>>> @@ -34,6 +34,7 @@ obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
>>>> obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
>>>> obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o
>>>> obj-$(CONFIG_JOYSTICK_STINGER) += stinger.o
>>>> +obj-$(CONFIG_JOYSTICK_NINTENDO_SWITCH2_USB) += nintendo-switch2-usb.o
>>>> obj-$(CONFIG_JOYSTICK_TMDC) += tmdc.o
>>>> obj-$(CONFIG_JOYSTICK_TURBOGRAFX) += turbografx.o
>>>> obj-$(CONFIG_JOYSTICK_TWIDJOY) += twidjoy.o
>>>> diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
>>>> new file mode 100644
>>>> index 0000000000000..ebd89d852e21a
>>>> --- /dev/null
>>>> +++ b/drivers/input/joystick/nintendo-switch2-usb.c
>>>> @@ -0,0 +1,353 @@
>>>> +// SPDX-License-Identifier: GPL-2.0+
>>>> +/*
>>>> + * USB driver for Nintendo Switch 2 controllers configuration interface
>>>> + *
>>>> + * Copyright (c) 2025 Valve Software
>>>> + *
>>>> + * This driver is based on the following work:
>>>> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
>>>> + * https://github.com/ndeadly/switch2_controller_research
>>>> + */
>>>> +
>>>> +#include "../../hid/hid-ids.h"
>>>> +#include "../../hid/hid-nintendo.h"
>>>> +#include <linux/module.h>
>>>> +#include <linux/usb/input.h>
>>>> +
>>>> +#define NS2_BULK_SIZE 64
>>>> +#define NS2_IN_URBS 2
>>>> +#define NS2_OUT_URBS 4
>>>> +
>>>> +static struct usb_driver switch2_usb;
>>>> +
>>>> +struct switch2_urb {
>>>> + struct urb *urb;
>>>> + uint8_t *data;
>>>> + bool active;
>>>> +};
>>>> +
>>>> +struct switch2_usb {
>>>> + struct switch2_cfg_intf cfg;
>>>> + struct usb_device *udev;
>>>> +
>>>> + struct switch2_urb bulk_in[NS2_IN_URBS];
>>>> + struct usb_anchor bulk_in_anchor;
>>>> + spinlock_t bulk_in_lock;
>>>> +
>>>> + struct switch2_urb bulk_out[NS2_OUT_URBS];
>>>> + struct usb_anchor bulk_out_anchor;
>>>> + spinlock_t bulk_out_lock;
>>>> +
>>>> + int message_in;
>>>> + struct work_struct message_in_work;
>>>> +};
>>>> +
>>>> +static void switch2_bulk_in(struct urb *urb)
>>>> +{
>>>> + struct switch2_usb *ns2_usb = urb->context;
>>>> + int i;
>>>> + bool schedule = false;
>>>> + unsigned long flags;
>>>> +
>>>> + switch (urb->status) {
>>>> + case 0:
>>>> + schedule = true;
>>>> + break;
>>>> + case -ECONNRESET:
>>>> + case -ENOENT:
>>>> + case -ESHUTDOWN:
>>>> + dev_dbg(&ns2_usb->udev->dev, "shutting down input urb: %d\n", urb->status);
>>>> + return;
>>>> + default:
>>>> + dev_dbg(&ns2_usb->udev->dev, "unknown input urb status: %d\n", urb->status);
>>>> + break;
>>>> + }
>>>> +
>>>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>>>> + for (i = 0; i < NS2_IN_URBS; i++) {
>>>> + int err;
>>>> + struct switch2_urb *ns2_urb;
>>>> +
>>>> + if (ns2_usb->bulk_in[i].urb == urb) {
>>>> + ns2_usb->message_in = i;
>>>> + continue;
>>>> + }
>>>> +
>>>> + if (ns2_usb->bulk_in[i].active)
>>>> + continue;
>>>> +
>>>> + ns2_urb = &ns2_usb->bulk_in[i];
>>>> + usb_anchor_urb(ns2_urb->urb, &ns2_usb->bulk_in_anchor);
>>>> + err = usb_submit_urb(ns2_urb->urb, GFP_ATOMIC);
>>>> + if (err) {
>>>> + usb_unanchor_urb(ns2_urb->urb);
>>>> + dev_dbg(&ns2_usb->udev->dev, "failed to queue input urb: %d\n", err);
>>>> + } else {
>>>> + ns2_urb->active = true;
>>>> + }
>>>> + }
>>>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>>>> +
>>>> + if (schedule)
>>>> + schedule_work(&ns2_usb->message_in_work);
>>>> +}
>>>> +
>>>> +static void switch2_bulk_out(struct urb *urb)
>>>> +{
>>>> + struct switch2_usb *ns2_usb = urb->context;
>>>> + int i;
>>>> +
>>>> + guard(spinlock_irqsave)(&ns2_usb->bulk_out_lock);
>>>> +
>>>> + switch (urb->status) {
>>>> + case 0:
>>>> + break;
>>>> + case -ECONNRESET:
>>>> + case -ENOENT:
>>>> + case -ESHUTDOWN:
>>>> + dev_dbg(&ns2_usb->udev->dev, "shutting down output urb: %d\n", urb->status);
>>>> + return;
>>>> + default:
>>>> + dev_dbg(&ns2_usb->udev->dev, "unknown output urb status: %d\n", urb->status);
>>>> + return;
>>>> + }
>>>> +
>>>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>>>> + if (ns2_usb->bulk_out[i].urb != urb)
>>>> + continue;
>>>> +
>>>> + ns2_usb->bulk_out[i].active = false;
>>>> + break;
>>>> + }
>>>> +}
>>>> +
>>>> +static int switch2_usb_send_cmd(enum switch2_cmd command, uint8_t subcommand,
>>>> + const void *message, size_t size, struct switch2_cfg_intf *cfg)
>>>> +{
>>>> + struct switch2_usb *ns2_usb = (struct switch2_usb *)cfg;
>>>> + struct switch2_urb *urb = NULL;
>>>> + int i;
>>>> + int ret;
>>>> + unsigned long flags;
>>>> +
>>>> + struct switch2_cmd_header header = {
>>>> + command, NS2_DIR_OUT | NS2_FLAG_OK, NS2_TRANS_USB, subcommand, 0, size
>>>> + };
>>>> +
>>>> + if (WARN_ON(size > 56))
>>>> + return -EINVAL;
>>>> +
>>>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>>>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>>>> + if (ns2_usb->bulk_out[i].active)
>>>> + continue;
>>>> +
>>>> + urb = &ns2_usb->bulk_out[i];
>>>> + urb->active = true;
>>>> + break;
>>>> + }
>>>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>>>> +
>>>> + if (!urb) {
>>>> + dev_warn(&ns2_usb->udev->dev, "output queue full, dropping message\n");
>>>> + return -ENOBUFS;
>>>> + }
>>>> +
>>>> + memcpy(urb->data, &header, sizeof(header));
>>>> + if (message && size)
>>>> + memcpy(&urb->data[8], message, size);
>>>> + urb->urb->transfer_buffer_length = size + sizeof(header);
>>>> +
>>>> + print_hex_dump_debug("sending cmd: ", DUMP_PREFIX_OFFSET, 16, 1, urb->data,
>>>> + size + sizeof(header), false);
>>>> +
>>>> + usb_anchor_urb(urb->urb, &ns2_usb->bulk_out_anchor);
>>>> + ret = usb_submit_urb(urb->urb, GFP_ATOMIC);
>>>> + if (ret) {
>>>> + if (ret != -ENODEV)
>>>> + dev_warn(&ns2_usb->udev->dev, "failed to submit output urb: %i", ret);
>>>> + urb->active = false;
>>>> + usb_unanchor_urb(urb->urb);
>>>> + return ret;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static void switch2_usb_message_in_work(struct work_struct *work)
>>>> +{
>>>> + struct switch2_usb *ns2_usb = container_of(work, struct switch2_usb, message_in_work);
>>>> + struct switch2_urb *urb;
>>>> + int err;
>>>> + unsigned long flags;
>>>> +
>>>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>>>> + urb = &ns2_usb->bulk_in[ns2_usb->message_in];
>>>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>>>> +
>>>> + err = switch2_receive_command(ns2_usb->cfg.parent, urb->urb->transfer_buffer,
>>>> + urb->urb->actual_length);
>>>> + if (err)
>>>> + dev_dbg(&ns2_usb->udev->dev, "receive command failed: %d\n", err);
>>>> +
>>>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>>>> + urb->active = false;
>>>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>>>> +}
>>>> +
>>>> +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
>>>> +{
>>>> + struct switch2_usb *ns2_usb;
>>>> + struct usb_device *udev;
>>>> + struct usb_endpoint_descriptor *bulk_in, *bulk_out;
>>>> + char phys[64];
>>>> + int ret;
>>>> + int i;
>>>> +
>>>> + udev = interface_to_usbdev(intf);
>>>> + if (usb_make_path(udev, phys, sizeof(phys)) < 0)
>>>> + return -EINVAL;
>>>> +
>>>> + ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
>>>> + if (ret) {
>>>> + dev_err(&intf->dev, "failed to find bulk EPs\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ns2_usb = devm_kzalloc(&intf->dev, sizeof(*ns2_usb), GFP_KERNEL);
>>>> + if (!ns2_usb)
>>>> + return -ENOMEM;
>>>> +
>>>> + ns2_usb->udev = udev;
>>>> + for (i = 0; i < NS2_IN_URBS; i++) {
>>>> + ns2_usb->bulk_in[i].urb = usb_alloc_urb(0, GFP_KERNEL);
>>>> + if (!ns2_usb->bulk_in[i].urb) {
>>>> + ret = -ENOMEM;
>>>> + goto err_free_in;
>>>> + }
>>>> +
>>>> + ns2_usb->bulk_in[i].data = usb_alloc_coherent(udev, NS2_BULK_SIZE, GFP_KERNEL,
>>>> + &ns2_usb->bulk_in[i].urb->transfer_dma);
>>>> + if (!ns2_usb->bulk_in[i].data) {
>>>> + ret = -ENOMEM;
>>>> + goto err_free_in;
>>>> + }
>>>> +
>>>> + usb_fill_bulk_urb(ns2_usb->bulk_in[i].urb, udev,
>>>> + usb_rcvbulkpipe(udev, bulk_in->bEndpointAddress),
>>>> + ns2_usb->bulk_in[i].data, NS2_BULK_SIZE, switch2_bulk_in, ns2_usb);
>>>> + ns2_usb->bulk_in[i].urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>>>> + }
>>>> +
>>>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>>>> + ns2_usb->bulk_out[i].urb = usb_alloc_urb(0, GFP_KERNEL);
>>>> + if (!ns2_usb->bulk_out[i].urb) {
>>>> + ret = -ENOMEM;
>>>> + goto err_free_out;
>>>> + }
>>>> +
>>>> + ns2_usb->bulk_out[i].data = usb_alloc_coherent(udev, NS2_BULK_SIZE, GFP_KERNEL,
>>>> + &ns2_usb->bulk_out[i].urb->transfer_dma);
>>>> + if (!ns2_usb->bulk_out[i].data) {
>>>> + ret = -ENOMEM;
>>>> + goto err_free_out;
>>>> + }
>>>> +
>>>> + usb_fill_bulk_urb(ns2_usb->bulk_out[i].urb, udev,
>>>> + usb_sndbulkpipe(udev, bulk_out->bEndpointAddress),
>>>> + ns2_usb->bulk_out[i].data, NS2_BULK_SIZE, switch2_bulk_out, ns2_usb);
>>>> + ns2_usb->bulk_out[i].urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>>>> + }
>>>> +
>>>> + ns2_usb->bulk_in[0].active = true;
>>>> + ret = usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC);
>>>> + if (ret < 0)
>>>> + goto err_free_out;
>>>> +
>>>> + init_usb_anchor(&ns2_usb->bulk_out_anchor);
>>>> + spin_lock_init(&ns2_usb->bulk_out_lock);
>>>> + init_usb_anchor(&ns2_usb->bulk_in_anchor);
>>>> + spin_lock_init(&ns2_usb->bulk_in_lock);
>>>> + INIT_WORK(&ns2_usb->message_in_work, switch2_usb_message_in_work);
>>>> +
>>>> + usb_set_intfdata(intf, ns2_usb);
>>>> +
>>>> + ns2_usb->cfg.dev = &ns2_usb->udev->dev;
>>>> + ns2_usb->cfg.send_command = switch2_usb_send_cmd;
>>>> +
>>>> + ret = switch2_controller_attach_cfg(phys, &ns2_usb->cfg);
>>>> + if (ret < 0)
>>>> + goto err_kill_urb;
>>>> +
>>>> + return 0;
>>>> +
>>>> +err_kill_urb:
>>>> + usb_kill_urb(ns2_usb->bulk_in[0].urb);
>>>> +err_free_out:
>>>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>>>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
>>>> + ns2_usb->bulk_out[i].urb->transfer_dma);
>>>> + usb_free_urb(ns2_usb->bulk_out[i].urb);
>>>> + }
>>>> +err_free_in:
>>>> + for (i = 0; i < NS2_IN_URBS; i++) {
>>>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
>>>> + ns2_usb->bulk_in[i].urb->transfer_dma);
>>>> + usb_free_urb(ns2_usb->bulk_in[i].urb);
>>>> + }
>>>> + devm_kfree(&intf->dev, ns2_usb);
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> +static void switch2_usb_disconnect(struct usb_interface *intf)
>>>> +{
>>>> + struct switch2_usb *ns2_usb = usb_get_intfdata(intf);
>>>> + unsigned long flags;
>>>> + int i;
>>>> +
>>>> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
>>>> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
>>>> + for (i = 0; i < NS2_OUT_URBS; i++) {
>>>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
>>>> + ns2_usb->bulk_out[i].urb->transfer_dma);
>>>> + usb_free_urb(ns2_usb->bulk_out[i].urb);
>>>> + }
>>>> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
>>>> +
>>>> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
>>>> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
>>>> + cancel_work_sync(&ns2_usb->message_in_work);
>>>> + for (i = 0; i < NS2_IN_URBS; i++) {
>>>> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
>>>> + ns2_usb->bulk_in[i].urb->transfer_dma);
>>>> + usb_free_urb(ns2_usb->bulk_in[i].urb);
>>>> + }
>>>> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
>>>> +
>>>> + switch2_controller_detach_cfg(ns2_usb->cfg.parent);
>>>
>>> As we have allocated ns2_usb with devm_kzalloc, don't we need to free it
>>> with devm_kfree again?
>>
>> Devres will automatically free it when the device is freed. As this is
>> the callback for the device going away, it's freed directly after this
>> function exits. That's why I'm using devm_kzalloc instead of kzalloc.
>
> Makes sense!
>
>
> Thanks for helping me to understand the code better!
>
> Cheers,
> Silvan
>
>>
>>>
>>> Cheers,
>>> Silvan
>>>
>>>> +}
>>>> +
>>>> +#define SWITCH2_CONTROLLER(vend, prod) \
>>>> + USB_DEVICE_AND_INTERFACE_INFO(vend, prod, USB_CLASS_VENDOR_SPEC, 0, 0)
>>>> +
>>>> +static const struct usb_device_id switch2_usb_devices[] = {
>>>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_JOYCONL) },
>>>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_JOYCONR) },
>>>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_PROCON) },
>>>> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_GCCON) },
>>>> + { }
>>>> +};
>>>> +MODULE_DEVICE_TABLE(usb, switch2_usb_devices);
>>>> +
>>>> +static struct usb_driver switch2_usb = {
>>>> + .name = "switch2",
>>>> + .id_table = switch2_usb_devices,
>>>> + .probe = switch2_usb_probe,
>>>> + .disconnect = switch2_usb_disconnect,
>>>> +};
>>>> +module_usb_driver(switch2_usb);
>>>> +
>>>> +MODULE_LICENSE("GPL");
>>>> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
>>>> +MODULE_DESCRIPTION("Driver for Nintendo Switch 2 Controllers");
>>>
>>>
>>
>> Vicki
>
>
Unless anyone else chimes in with more suggestions I'll probably submit
v4 sometime next week with these two changes.
Vicki
^ permalink raw reply
* [PATCH] Input: edt-ft5x06 - Support label device property for input name
From: Aaron Kling via B4 Relay @ 2026-04-09 23:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Aaron Kling
From: Aaron Kling <webgeek1234@gmail.com>
The AYN Thor uses a ft5426 and a ft5452 for each screen respectively and
these currently get the same input name, making them indistinguishable
from userspace. Support setting a label in kernel dt to make these
report uniquely.
Signed-off-by: Aaron Kling <webgeek1234@gmail.com>
---
drivers/input/touchscreen/edt-ft5x06.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
index ba8ff65f7ea671..c36497571b1aa1 100644
--- a/drivers/input/touchscreen/edt-ft5x06.c
+++ b/drivers/input/touchscreen/edt-ft5x06.c
@@ -1285,7 +1285,9 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client)
"Model \"%s\", Rev. \"%s\", %dx%d sensors\n",
tsdata->name, tsdata->fw_version, tsdata->num_x, tsdata->num_y);
- input->name = tsdata->name;
+ if (device_property_read_string(&client->dev, "label", &input->name))
+ input->name = tsdata->name;
+
input->id.bustype = BUS_I2C;
input->dev.parent = &client->dev;
---
base-commit: 3fa7d958829eb9bc3b469ed07f11de3d2804ef71
change-id: 20260409-ft5x06-label-878de0e5f84c
Best regards,
--
Aaron Kling <webgeek1234@gmail.com>
^ permalink raw reply related
* Re: [PATCH v2 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Silvan Jegen @ 2026-04-08 19:51 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <ee55d9f8-506d-4b95-a63b-01844e4e1bdf@endrift.com>
Heyhey!
Vicki Pfau <vi@endrift.com> wrote:
> Hi,
>
> Replies inline
>
> On 4/2/26 12:09 PM, Silvan Jegen wrote:
> > Hi
> >
> > Thanks for the patch!
> >
> > Just some comments and questions inline below.
> >
> > Vicki Pfau <vi@endrift.com> wrote:
> >> This adds a new driver for the Switch 2 controllers. The Switch 2 uses an
> >> unusual split-interface design such that input and rumble occur on the main
> >> HID interface, but all other communication occurs over a "configuration"
> >> interface. This is the case on both USB and Bluetooth, so this new driver
> >> uses a split-driver design with the HID interface being the "main" driver
> >> and the configuration interface is a secondary driver that looks up to the
> >> HID interface, sharing resources on a common struct.
> >>
> >> Due to using a non-standard pairing interface as well as Bluetooth
> >> communications being extremely limited in the kernel, a custom interface
> >> between userspace and the kernel will need to be design, along with bringup
> >> in BlueZ. That is beyond the scope of this initial patch, which only
> >> contains the generic HID and USB configuration interface drivers.
> >>
> >> This initial work supports general input for the Joy-Con 2, Pro Controller
> >> 2, and GameCube NSO controllers. IMU, rumble and battery support is not yet
> >> present.
> >>
> >> Signed-off-by: Vicki Pfau <vi@endrift.com>
> >> ---
> >> MAINTAINERS | 1 +
> >> drivers/hid/Kconfig | 11 +-
> >> drivers/hid/hid-ids.h | 4 +
> >> drivers/hid/hid-nintendo.c | 1194 ++++++++++++++++-
> >> drivers/hid/hid-nintendo.h | 72 +
> >> drivers/input/joystick/Kconfig | 11 +
> >> drivers/input/joystick/Makefile | 1 +
> >> drivers/input/joystick/nintendo-switch2-usb.c | 353 +++++
> >> 8 files changed, 1637 insertions(+), 10 deletions(-)
> >> create mode 100644 drivers/hid/hid-nintendo.h
> >> create mode 100644 drivers/input/joystick/nintendo-switch2-usb.c
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index 7b277d5bf3d12..4d1a28df5fd24 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -18743,6 +18743,7 @@ F: drivers/scsi/nsp32*
> >>
> >> NINTENDO HID DRIVER
> >> M: Daniel J. Ogorchock <djogorchock@gmail.com>
> >> +M: Vicki Pfau <vi@endrift.com>
> >> L: linux-input@vger.kernel.org
> >> S: Maintained
> >> F: drivers/hid/hid-nintendo*
> >> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> >> index c1d9f7c6a5f23..1a293a6c02c26 100644
> >> --- a/drivers/hid/Kconfig
> >> +++ b/drivers/hid/Kconfig
> >> @@ -826,10 +826,13 @@ config HID_NINTENDO
> >> depends on LEDS_CLASS
> >> select POWER_SUPPLY
> >> help
> >> - Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller.
> >> - All controllers support bluetooth, and the Pro Controller also supports
> >> - its USB mode. This also includes support for the Nintendo Switch Online
> >> - Controllers which include the NES, Genesis, SNES, and N64 controllers.
> >> + Adds support for the Nintendo Switch Joy-Cons, NSO, Pro Controller, as
> >> + well as Nintendo Switch 2 Joy-Cons, Pro Controller, and NSO GameCube
> >> + controllers. All Switch controllers support bluetooth, and the Pro
> >> + Controller also supports its USB mode. This also includes support for
> >> + the Nintendo Switch Online Controllers which include the NES, Genesis,
> >> + SNES, and N64 controllers. Switch 2 controllers currently only support
> >> + USB mode.
> >>
> >> To compile this driver as a module, choose M here: the
> >> module will be called hid-nintendo.
> >> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> >> index 4ab7640b119ac..a794dad7980f3 100644
> >> --- a/drivers/hid/hid-ids.h
> >> +++ b/drivers/hid/hid-ids.h
> >> @@ -1073,6 +1073,10 @@
> >> #define USB_DEVICE_ID_NINTENDO_SNESCON 0x2017
> >> #define USB_DEVICE_ID_NINTENDO_GENCON 0x201e
> >> #define USB_DEVICE_ID_NINTENDO_N64CON 0x2019
> >> +#define USB_DEVICE_ID_NINTENDO_NS2_JOYCONR 0x2066
> >> +#define USB_DEVICE_ID_NINTENDO_NS2_JOYCONL 0x2067
> >> +#define USB_DEVICE_ID_NINTENDO_NS2_PROCON 0x2069
> >> +#define USB_DEVICE_ID_NINTENDO_NS2_GCCON 0x2073
> >>
> >> #define USB_VENDOR_ID_NOVATEK 0x0603
> >> #define USB_DEVICE_ID_NOVATEK_PCT 0x0600
> >> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> >> index 29008c2cc5304..4ab8d4e7558a1 100644
> >> --- a/drivers/hid/hid-nintendo.c
> >> +++ b/drivers/hid/hid-nintendo.c
> >> @@ -1,11 +1,13 @@
> >> // SPDX-License-Identifier: GPL-2.0+
> >> /*
> >> - * HID driver for Nintendo Switch Joy-Cons and Pro Controllers
> >> + * HID driver for Nintendo Switch Joy-Cons and Pro Controllers, as well as
> >> + * Nintendo Switch 2 Joy-Cons, Pro Controller, and GameCube Controller
> >> *
> >> * Copyright (c) 2019-2021 Daniel J. Ogorchock <djogorchock@gmail.com>
> >> * Portions Copyright (c) 2020 Nadia Holmquist Pedersen <nadia@nhp.sh>
> >> * Copyright (c) 2022 Emily Strickland <linux@emily.st>
> >> * Copyright (c) 2023 Ryan McClelland <rymcclel@gmail.com>
> >> + * Copyright (c) 2026 Valve Software
> >> *
> >> * The following resources/projects were referenced for this driver:
> >> * https://github.com/dekuNukem/Nintendo_Switch_Reverse_Engineering
> >> @@ -13,6 +15,8 @@
> >> * https://github.com/FrotBot/SwitchProConLinuxUSB
> >> * https://github.com/MTCKC/ProconXInput
> >> * https://github.com/Davidobot/BetterJoyForCemu
> >> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
> >> + * https://github.com/ndeadly/switch2_controller_research
> >> * hid-wiimote kernel hid driver
> >> * hid-logitech-hidpp driver
> >> * hid-sony driver
> >> @@ -29,6 +33,7 @@
> >> */
> >>
> >> #include "hid-ids.h"
> >> +#include "hid-nintendo.h"
> >> #include <linux/unaligned.h>
> >> #include <linux/delay.h>
> >> #include <linux/device.h>
> >> @@ -41,6 +46,8 @@
> >> #include <linux/module.h>
> >> #include <linux/power_supply.h>
> >> #include <linux/spinlock.h>
> >> +#include <linux/usb.h>
> >> +#include "usbhid/usbhid.h"
> >>
> >> /*
> >> * Reference the url below for the following HID report defines:
> >> @@ -2614,7 +2621,7 @@ static int joycon_ctlr_handle_event(struct joycon_ctlr *ctlr, u8 *data,
> >> return ret;
> >> }
> >>
> >> -static int nintendo_hid_event(struct hid_device *hdev,
> >> +static int joycon_event(struct hid_device *hdev,
> >> struct hid_report *report, u8 *raw_data, int size)
> >> {
> >> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> >> @@ -2625,7 +2632,7 @@ static int nintendo_hid_event(struct hid_device *hdev,
> >> return joycon_ctlr_handle_event(ctlr, raw_data, size);
> >> }
> >>
> >> -static int nintendo_hid_probe(struct hid_device *hdev,
> >> +static int joycon_probe(struct hid_device *hdev,
> >> const struct hid_device_id *id)
> >> {
> >> int ret;
> >> @@ -2729,7 +2736,7 @@ static int nintendo_hid_probe(struct hid_device *hdev,
> >> return ret;
> >> }
> >>
> >> -static void nintendo_hid_remove(struct hid_device *hdev)
> >> +static void joycon_remove(struct hid_device *hdev)
> >> {
> >> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> >> unsigned long flags;
> >> @@ -2748,7 +2755,9 @@ static void nintendo_hid_remove(struct hid_device *hdev)
> >> hid_hw_stop(hdev);
> >> }
> >>
> >> -static int nintendo_hid_resume(struct hid_device *hdev)
> >> +#ifdef CONFIG_PM
> >> +
> >> +static int joycon_resume(struct hid_device *hdev)
> >> {
> >> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> >> int ret;
> >> @@ -2771,7 +2780,7 @@ static int nintendo_hid_resume(struct hid_device *hdev)
> >> return ret;
> >> }
> >>
> >> -static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
> >> +static int joycon_suspend(struct hid_device *hdev, pm_message_t message)
> >> {
> >> struct joycon_ctlr *ctlr = hid_get_drvdata(hdev);
> >>
> >> @@ -2790,7 +2799,1120 @@ static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
> >> return 0;
> >> }
> >>
> >> +#endif
> >> +
> >> +/*
> >> + * =============================================================================
> >> + * Switch 2 support
> >> + * =============================================================================
> >> + */
> >> +#define NS2_BTNR_B BIT(0)
> >> +#define NS2_BTNR_A BIT(1)
> >> +#define NS2_BTNR_Y BIT(2)
> >> +#define NS2_BTNR_X BIT(3)
> >> +#define NS2_BTNR_R BIT(4)
> >> +#define NS2_BTNR_ZR BIT(5)
> >> +#define NS2_BTNR_PLUS BIT(6)
> >> +#define NS2_BTNR_RS BIT(7)
> >> +
> >> +#define NS2_BTNL_DOWN BIT(0)
> >> +#define NS2_BTNL_RIGHT BIT(1)
> >> +#define NS2_BTNL_LEFT BIT(2)
> >> +#define NS2_BTNL_UP BIT(3)
> >> +#define NS2_BTNL_L BIT(4)
> >> +#define NS2_BTNL_ZL BIT(5)
> >> +#define NS2_BTNL_MINUS BIT(6)
> >> +#define NS2_BTNL_LS BIT(7)
> >> +
> >> +#define NS2_BTN3_C BIT(4)
> >> +#define NS2_BTN3_SR BIT(6)
> >> +#define NS2_BTN3_SL BIT(7)
> >> +
> >> +#define NS2_BTN_JCR_HOME BIT(0)
> >> +#define NS2_BTN_JCR_GR BIT(2)
> >> +#define NS2_BTN_JCR_C NS2_BTN3_C
> >> +#define NS2_BTN_JCR_SR NS2_BTN3_SR
> >> +#define NS2_BTN_JCR_SL NS2_BTN3_SL
> >> +
> >> +#define NS2_BTN_JCL_CAPTURE BIT(0)
> >> +#define NS2_BTN_JCL_GL BIT(2)
> >> +#define NS2_BTN_JCL_SR NS2_BTN3_SR
> >> +#define NS2_BTN_JCL_SL NS2_BTN3_SL
> >> +
> >> +#define NS2_BTN_PRO_HOME BIT(0)
> >> +#define NS2_BTN_PRO_CAPTURE BIT(1)
> >> +#define NS2_BTN_PRO_GR BIT(2)
> >> +#define NS2_BTN_PRO_GL BIT(3)
> >> +#define NS2_BTN_PRO_C NS2_BTN3_C
> >> +
> >> +#define NS2_BTN_GC_HOME BIT(0)
> >> +#define NS2_BTN_GC_CAPTURE BIT(1)
> >> +#define NS2_BTN_GC_C NS2_BTN3_C
> >> +
> >> +#define NS2_TRIGGER_RANGE 4095
> >> +#define NS2_AXIS_MIN -32768
> >> +#define NS2_AXIS_MAX 32767
> >> +
> >> +#define NS2_MAX_PLAYER_ID 8
> >> +
> >> +#define NS2_MAX_INIT_RETRIES 4
> >> +
> >> +#define NS2_FLASH_ADDR_SERIAL 0x13002
> >> +#define NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB 0x130a8
> >> +#define NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB 0x130e8
> >> +#define NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB 0x13140
> >> +#define NS2_FLASH_ADDR_USER_PRIMARY_CALIB 0x1fc040
> >> +#define NS2_FLASH_ADDR_USER_SECONDARY_CALIB 0x1fc080
> >> +
> >> +#define NS2_FLASH_SIZE_SERIAL 0x10
> >> +#define NS2_FLASH_SIZE_FACTORY_AXIS_CALIB 9
> >> +#define NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB 2
> >> +#define NS2_FLASH_SIZE_USER_AXIS_CALIB 11
> >> +
> >> +#define NS2_USER_CALIB_MAGIC 0xa1b2
> >> +
> >> +#define NS2_FEATURE_BUTTONS BIT(0)
> >> +#define NS2_FEATURE_ANALOG BIT(1)
> >> +#define NS2_FEATURE_IMU BIT(2)
> >> +#define NS2_FEATURE_MOUSE BIT(4)
> >> +#define NS2_FEATURE_RUMBLE BIT(5)
> >> +#define NS2_FEATURE_MAGNETO BIT(7)
> >> +
> >> +enum switch2_subcmd_flash {
> >> + NS2_SUBCMD_FLASH_READ_BLOCK = 0x01,
> >> + NS2_SUBCMD_FLASH_WRITE_BLOCK = 0x02,
> >> + NS2_SUBCMD_FLASH_ERASE_BLOCK = 0x03,
> >> + NS2_SUBCMD_FLASH_READ = 0x04,
> >> + NS2_SUBCMD_FLASH_WRITE = 0x05,
> >> +};
> >> +
> >> +enum switch2_subcmd_init {
> >> + NS2_SUBCMD_INIT_SELECT_REPORT = 0xa,
> >> + NS2_SUBCMD_INIT_USB = 0xd,
> >> +};
> >> +
> >> +enum switch2_subcmd_feature_select {
> >> + NS2_SUBCMD_FEATSEL_GET_INFO = 0x1,
> >> + NS2_SUBCMD_FEATSEL_SET_MASK = 0x2,
> >> + NS2_SUBCMD_FEATSEL_CLEAR_MASK = 0x3,
> >> + NS2_SUBCMD_FEATSEL_ENABLE = 0x4,
> >> + NS2_SUBCMD_FEATSEL_DISABLE = 0x5,
> >> +};
> >> +
> >> +enum switch2_subcmd_grip {
> >> + NS2_SUBCMD_GRIP_GET_INFO = 0x1,
> >> + NS2_SUBCMD_GRIP_ENABLE_BUTTONS = 0x2,
> >> + NS2_SUBCMD_GRIP_GET_INFO_EXT = 0x3,
> >> +};
> >> +
> >> +enum switch2_subcmd_led {
> >> + NS2_SUBCMD_LED_P1 = 0x1,
> >> + NS2_SUBCMD_LED_P2 = 0x2,
> >> + NS2_SUBCMD_LED_P3 = 0x3,
> >> + NS2_SUBCMD_LED_P4 = 0x4,
> >> + NS2_SUBCMD_LED_ALL_ON = 0x5,
> >> + NS2_SUBCMD_LED_ALL_OFF = 0x6,
> >> + NS2_SUBCMD_LED_PATTERN = 0x7,
> >> + NS2_SUBCMD_LED_BLINK = 0x8,
> >> +};
> >> +
> >> +enum switch2_subcmd_fw_info {
> >> + NS2_SUBCMD_FW_INFO_GET = 0x1,
> >> +};
> >> +
> >> +enum switch2_ctlr_type {
> >> + NS2_CTLR_TYPE_JCL = 0x00,
> >> + NS2_CTLR_TYPE_JCR = 0x01,
> >> + NS2_CTLR_TYPE_PRO = 0x02,
> >> + NS2_CTLR_TYPE_GC = 0x03,
> >> +};
> >> +
> >> +enum switch2_report_id {
> >> + NS2_REPORT_UNIFIED = 0x05,
> >> + NS2_REPORT_JCL = 0x07,
> >> + NS2_REPORT_JCR = 0x08,
> >> + NS2_REPORT_PRO = 0x09,
> >> + NS2_REPORT_GC = 0x0a,
> >> +};
> >> +
> >> +enum switch2_init_step {
> >> + NS2_INIT_READ_SERIAL,
> >> + NS2_INIT_GET_FIRMWARE_INFO,
> >> + NS2_INIT_READ_FACTORY_PRIMARY_CALIB,
> >> + NS2_INIT_READ_FACTORY_SECONDARY_CALIB,
> >> + NS2_INIT_READ_FACTORY_TRIGGER_CALIB,
> >> + NS2_INIT_READ_USER_PRIMARY_CALIB,
> >> + NS2_INIT_READ_USER_SECONDARY_CALIB,
> >> + NS2_INIT_SET_FEATURE_MASK,
> >> + NS2_INIT_ENABLE_FEATURES,
> >> + NS2_INIT_GRIP_BUTTONS,
> >> + NS2_INIT_REPORT_FORMAT,
> >> + NS2_INIT_SET_PLAYER_LEDS,
> >> + NS2_INIT_INPUT,
> >> + NS2_INIT_FINISH,
> >> + NS2_INIT_DONE,
> >> +};
> >> +
> >> +struct switch2_version_info {
> >> + uint8_t major;
> >> + uint8_t minor;
> >> + uint8_t patch;
> >> + uint8_t ctlr_type;
> >> + __le32 unk;
> >> + int8_t dsp_major;
> >> + int8_t dsp_minor;
> >> + int8_t dsp_patch;
> >> + int8_t dsp_type;
> >> +};
> >> +
> >> +struct switch2_axis_calibration {
> >> + uint16_t neutral;
> >> + uint16_t negative;
> >> + uint16_t positive;
> >> +};
> >> +
> >> +struct switch2_stick_calibration {
> >> + struct switch2_axis_calibration x;
> >> + struct switch2_axis_calibration y;
> >> +};
> >> +
> >> +struct switch2_controller {
> >> + struct hid_device *hdev;
> >> + struct switch2_cfg_intf *cfg;
> >> +
> >> + char name[64];
> >> + char phys[64];
> >> + struct list_head entry;
> >> + struct mutex lock;
> >> +
> >> + enum switch2_ctlr_type ctlr_type;
> >> + enum switch2_init_step init_step;
> >> + struct input_dev __rcu *input;
> >> + char serial[NS2_FLASH_SIZE_SERIAL + 1];
> >> + struct switch2_version_info version;
> >> +
> >> + struct switch2_stick_calibration stick_calib[2];
> >> + uint8_t lt_zero;
> >> + uint8_t rt_zero;
> >> +
> >> + uint32_t player_id;
> >> + struct led_classdev leds[4];
> >> +};
> >> +
> >> +static DEFINE_MUTEX(switch2_controllers_lock);
> >> +static LIST_HEAD(switch2_controllers);
> >> +
> >> +struct switch2_ctlr_button_mapping {
> >> + uint32_t code;
> >> + int byte;
> >> + uint32_t bit;
> >> +};
> >> +
> >> +static const struct switch2_ctlr_button_mapping ns2_left_joycon_button_mappings[] = {
> >> + { BTN_DPAD_LEFT, 0, NS2_BTNL_LEFT, },
> >> + { BTN_DPAD_UP, 0, NS2_BTNL_UP, },
> >> + { BTN_DPAD_DOWN, 0, NS2_BTNL_DOWN, },
> >> + { BTN_DPAD_RIGHT, 0, NS2_BTNL_RIGHT, },
> >> + { BTN_TL, 0, NS2_BTNL_L, },
> >> + { BTN_TL2, 0, NS2_BTNL_ZL, },
> >> + { BTN_SELECT, 0, NS2_BTNL_MINUS, },
> >> + { BTN_THUMBL, 0, NS2_BTNL_LS, },
> >> + { KEY_RECORD, 1, NS2_BTN_JCL_CAPTURE, },
> >> + { BTN_GRIPR, 1, NS2_BTN_JCL_SL, },
> >> + { BTN_GRIPR2, 1, NS2_BTN_JCL_SR, },
> >> + { BTN_GRIPL, 1, NS2_BTN_JCL_GL, },
> >> + { /* sentinel */ },
> >> +};
> >> +
> >> +static const struct switch2_ctlr_button_mapping ns2_right_joycon_button_mappings[] = {
> >> + { BTN_SOUTH, 0, NS2_BTNR_A, },
> >> + { BTN_EAST, 0, NS2_BTNR_B, },
> >> + { BTN_NORTH, 0, NS2_BTNR_X, },
> >> + { BTN_WEST, 0, NS2_BTNR_Y, },
> >> + { BTN_TR, 0, NS2_BTNR_R, },
> >> + { BTN_TR2, 0, NS2_BTNR_ZR, },
> >> + { BTN_START, 0, NS2_BTNR_PLUS, },
> >> + { BTN_THUMBR, 0, NS2_BTNR_RS, },
> >> + { BTN_C, 1, NS2_BTN_JCR_C, },
> >> + { BTN_MODE, 1, NS2_BTN_JCR_HOME, },
> >> + { BTN_GRIPL2, 1, NS2_BTN_JCR_SL, },
> >> + { BTN_GRIPL, 1, NS2_BTN_JCR_SR, },
> >> + { BTN_GRIPR, 1, NS2_BTN_JCR_GR, },
> >> + { /* sentinel */ },
> >> +};
> >> +
> >> +static const struct switch2_ctlr_button_mapping ns2_procon_mappings[] = {
> >> + { BTN_SOUTH, 0, NS2_BTNR_A, },
> >> + { BTN_EAST, 0, NS2_BTNR_B, },
> >> + { BTN_NORTH, 0, NS2_BTNR_X, },
> >> + { BTN_WEST, 0, NS2_BTNR_Y, },
> >> + { BTN_TL, 1, NS2_BTNL_L, },
> >> + { BTN_TR, 0, NS2_BTNR_R, },
> >> + { BTN_TL2, 1, NS2_BTNL_ZL, },
> >> + { BTN_TR2, 0, NS2_BTNR_ZR, },
> >> + { BTN_SELECT, 1, NS2_BTNL_MINUS, },
> >> + { BTN_START, 0, NS2_BTNR_PLUS, },
> >> + { BTN_THUMBL, 1, NS2_BTNL_LS, },
> >> + { BTN_THUMBR, 0, NS2_BTNR_RS, },
> >> + { BTN_MODE, 2, NS2_BTN_PRO_HOME },
> >> + { KEY_RECORD, 2, NS2_BTN_PRO_CAPTURE },
> >> + { BTN_GRIPR, 2, NS2_BTN_PRO_GR },
> >> + { BTN_GRIPL, 2, NS2_BTN_PRO_GL },
> >> + { BTN_C, 2, NS2_BTN_PRO_C },
> >> + { /* sentinel */ },
> >> +};
> >> +
> >> +static const struct switch2_ctlr_button_mapping ns2_gccon_mappings[] = {
> >> + { BTN_SOUTH, 0, NS2_BTNR_A, },
> >> + { BTN_EAST, 0, NS2_BTNR_B, },
> >> + { BTN_NORTH, 0, NS2_BTNR_X, },
> >> + { BTN_WEST, 0, NS2_BTNR_Y, },
> >> + { BTN_TL2, 1, NS2_BTNL_L, },
> >> + { BTN_TR2, 0, NS2_BTNR_R, },
> >> + { BTN_TL, 1, NS2_BTNL_ZL, },
> >> + { BTN_TR, 0, NS2_BTNR_ZR, },
> >> + { BTN_SELECT, 1, NS2_BTNL_MINUS, },
> >> + { BTN_START, 0, NS2_BTNR_PLUS, },
> >> + { BTN_MODE, 2, NS2_BTN_GC_HOME },
> >> + { KEY_RECORD, 2, NS2_BTN_GC_CAPTURE },
> >> + { BTN_C, 2, NS2_BTN_GC_C },
> >> + { /* sentinel */ },
> >> +};
> >> +
> >> +static const uint8_t switch2_init_cmd_data[] = {
> >> + /*
> >> + * The last 6 bytes of this packet are the MAC address of
> >> + * the console, but we don't need that for USB
> >> + */
> >> + 0x01, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
> >> +};
> >> +
> >> +static const uint8_t switch2_one_data[] = { 0x01, 0x00, 0x00, 0x00 };
> >> +
> >> +static const uint8_t switch2_feature_mask[] = {
> >> + NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU,
> >> + 0x00, 0x00, 0x00
> >> +};
> >> +
> >> +static void switch2_init_step_done(struct switch2_controller *ns2, enum switch2_init_step step)
> >> +{
> >> + if (ns2->init_step != step)
> >> + return;
> >> +
> >> + ns2->init_step++;
> >> +}
> >> +
> >> +static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type)
> >> +{
> >> + return type == NS2_CTLR_TYPE_JCL || type == NS2_CTLR_TYPE_JCR;
> >> +}
> >> +
> >> +static int switch2_set_leds(struct switch2_controller *ns2)
> >> +{
> >> + int i;
> >> + uint8_t message[8] = { 0 };
> >> +
> >> + for (i = 0; i < JC_NUM_LEDS; i++)
> >> + message[0] |= (!!ns2->leds[i].brightness) << i;
> >> +
> >> + if (!ns2->cfg)
> >> + return -ENOTCONN;
> >> + return ns2->cfg->send_command(NS2_CMD_LED, NS2_SUBCMD_LED_PATTERN,
> >> + &message, sizeof(message),
> >> + ns2->cfg);
> >> +}
> >> +
> >> +static int switch2_player_led_brightness_set(struct led_classdev *led,
> >> + enum led_brightness brightness)
> >> +{
> >> + struct device *dev = led->dev->parent;
> >> + struct hid_device *hdev = to_hid_device(dev);
> >> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
> >> +
> >> + if (!ns2)
> >> + return -ENODEV;
> >> +
> >> + guard(mutex)(&ns2->lock);
> >> + return switch2_set_leds(ns2);
> >> +}
> >> +
> >> +static void switch2_leds_create(struct switch2_controller *ns2)
> >> +{
> >> + struct hid_device *hdev = ns2->hdev;
> >> + struct led_classdev *led;
> >> + int i;
> >> + int player_led_pattern;
> >> +
> >> + player_led_pattern = ns2->player_id % JC_NUM_LED_PATTERNS;
> >> + hid_dbg(hdev, "assigned player %d led pattern", player_led_pattern + 1);
> >> +
> >> + for (i = 0; i < JC_NUM_LEDS; i++) {
> >> + led = &ns2->leds[i];
> >> + led->brightness = joycon_player_led_patterns[player_led_pattern][i];
> >> + led->max_brightness = 1;
> >> + led->brightness_set_blocking = switch2_player_led_brightness_set;
> >> + led->flags = LED_CORE_SUSPENDRESUME | LED_HW_PLUGGABLE;
> >> + }
> >> +}
> >> +
> >> +static void switch2_config_buttons(struct input_dev *idev,
> >> + const struct switch2_ctlr_button_mapping button_mappings[])
> >> +{
> >> + const struct switch2_ctlr_button_mapping *button;
> >> +
> >> + for (button = button_mappings; button->code; button++)
> >> + input_set_capability(idev, EV_KEY, button->code);
> >> +}
> >> +
> >> +static int switch2_init_input(struct switch2_controller *ns2)
> >> +{
> >> + struct input_dev *input;
> >> + struct hid_device *hdev = ns2->hdev;
> >> + int i;
> >> + int ret;
> >> +
> >> + switch2_init_step_done(ns2, NS2_INIT_FINISH);
> >> +
> >> + rcu_read_lock();
> >> + input = rcu_dereference(ns2->input);
> >> + rcu_read_unlock();
> >> +
> >> + if (input)
> >> + return 0;
> >> +
> >> + input = devm_input_allocate_device(&hdev->dev);
> >> + if (!input)
> >> + return -ENOMEM;
> >> +
> >> + input_set_drvdata(input, ns2);
> >> + input->dev.parent = &hdev->dev;
> >> + input->id.bustype = hdev->bus;
> >> + input->id.vendor = hdev->vendor;
> >> + input->id.product = hdev->product;
> >> + input->id.version = hdev->version;
> >> + input->uniq = ns2->serial;
> >> + input->name = ns2->name;
> >> + input->phys = hdev->phys;
> >> +
> >> + switch (ns2->ctlr_type) {
> >> + case NS2_CTLR_TYPE_JCL:
> >> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + switch2_config_buttons(input, ns2_left_joycon_button_mappings);
> >> + break;
> >> + case NS2_CTLR_TYPE_JCR:
> >> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + switch2_config_buttons(input, ns2_right_joycon_button_mappings);
> >> + break;
> >> + case NS2_CTLR_TYPE_GC:
> >> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_RX, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_RY, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_Z, 0, NS2_TRIGGER_RANGE, 32, 128);
> >> + input_set_abs_params(input, ABS_RZ, 0, NS2_TRIGGER_RANGE, 32, 128);
> >> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
> >> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
> >> + switch2_config_buttons(input, ns2_gccon_mappings);
> >> + break;
> >> + case NS2_CTLR_TYPE_PRO:
> >> + input_set_abs_params(input, ABS_X, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_Y, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_RX, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_RY, NS2_AXIS_MIN, NS2_AXIS_MAX, 32, 128);
> >> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
> >> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
> >> + switch2_config_buttons(input, ns2_procon_mappings);
> >> + break;
> >> + default:
> >> + input_free_device(input);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + hid_info(ns2->hdev, "Firmware version %u.%u.%u (type %i)\n", ns2->version.major,
> >> + ns2->version.minor, ns2->version.patch, ns2->version.ctlr_type);
> >> + if (ns2->version.dsp_type >= 0)
> >> + hid_info(ns2->hdev, "DSP version %u.%u.%u\n", ns2->version.dsp_major,
> >> + ns2->version.dsp_minor, ns2->version.dsp_patch);
> >> +
> >> + ret = input_register_device(input);
> >> + if (ret < 0) {
> >> + hid_err(ns2->hdev, "Failed to register input; ret=%d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + for (i = 0; i < JC_NUM_LEDS; i++) {
> >> + struct led_classdev *led = &ns2->leds[i];
> >> + char *name = devm_kasprintf(&input->dev, GFP_KERNEL, "%s:%s:%s",
> >> + dev_name(&input->dev),
> >> + "green",
> >> + joycon_player_led_names[i]);
> >> +
> >> + if (!name)
> >> + return -ENOMEM;
> >> +
> >> + led->name = name;
> >> + ret = devm_led_classdev_register(&input->dev, led);
> >> + if (ret < 0) {
> >> + dev_err(&input->dev, "Failed to register player %d LED; ret=%d\n",
> >> + i + 1, ret);
> >> + input_unregister_device(input);
> >> + return ret;
> >> + }
> >> + }
> >> +
> >> + rcu_assign_pointer(ns2->input, input);
> >> + synchronize_rcu();
> >> + return 0;
> >> +}
> >> +
> >> +static struct switch2_controller *switch2_get_controller(const char *phys)
> >> +{
> >> + struct switch2_controller *ns2;
> >> +
> >> + guard(mutex)(&switch2_controllers_lock);
> >> + list_for_each_entry(ns2, &switch2_controllers, entry) {
> >> + if (strncmp(ns2->phys, phys, sizeof(ns2->phys)) == 0)
> >> + return ns2;
> >> + }
> >> + ns2 = kzalloc(sizeof(*ns2), GFP_KERNEL);
> >> + if (!ns2)
> >> + return ERR_PTR(-ENOMEM);
> >> +
> >> + mutex_init(&ns2->lock);
> >> + INIT_LIST_HEAD(&ns2->entry);
> >> + list_add(&ns2->entry, &switch2_controllers);
> >> + strscpy(ns2->phys, phys, sizeof(ns2->phys));
> >> + return ns2;
> >> +}
> >> +
> >> +static void switch2_controller_put(struct switch2_controller *ns2)
> >> +{
> >> + struct input_dev *input;
> >> + bool do_free;
> >> +
> >> + guard(mutex)(&switch2_controllers_lock);
> >> + mutex_lock(&ns2->lock);
> >> +
> >> + rcu_read_lock();
> >> + input = rcu_dereference(ns2->input);
> >> + rcu_read_unlock();
> >> +
> >> + rcu_assign_pointer(ns2->input, NULL);
> >> + synchronize_rcu();
> >> +
> >> + ns2->init_step = 0;
> >> + do_free = !ns2->hdev && !ns2->cfg;
> >> + mutex_unlock(&ns2->lock);
> >> +
> >> + if (input)
> >> + input_unregister_device(input);
> >> +
> >> + if (do_free) {
> >> + list_del_init(&ns2->entry);
> >> + mutex_destroy(&ns2->lock);
> >> + kfree(ns2);
> >> + }
> >> +}
> >> +
> >> +static bool switch2_parse_stick_calibration(struct switch2_stick_calibration *calib,
> >> + const uint8_t *data)
> >> +{
> >> + static const uint8_t UNCALIBRATED[9] = {
> >> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
> >> + };
> >> + if (memcmp(UNCALIBRATED, data, sizeof(UNCALIBRATED)) == 0)
> >> + return false;
> >> +
> >> + calib->x.neutral = data[0];
> >> + calib->x.neutral |= (data[1] & 0x0F) << 8;
> >> +
> >> + calib->y.neutral = data[1] >> 4;
> >> + calib->y.neutral |= data[2] << 4;
> >> +
> >> + calib->x.positive = data[3];
> >> + calib->x.positive |= (data[4] & 0x0F) << 8;
> >> +
> >> + calib->y.positive = data[4] >> 4;
> >> + calib->y.positive |= data[5] << 4;
> >> +
> >> + calib->x.negative = data[6];
> >> + calib->x.negative |= (data[7] & 0x0F) << 8;
> >> +
> >> + calib->y.negative = data[7] >> 4;
> >> + calib->y.negative |= data[8] << 4;
> >> +
> >> + return true;
> >> +}
> >> +
> >> +static void switch2_handle_flash_read(struct switch2_controller *ns2, uint8_t size,
> >> + uint32_t address, const uint8_t *data)
> >> +{
> >> + bool ok;
> >> +
> >> + switch (address) {
> >> + case NS2_FLASH_ADDR_SERIAL:
> >> + if (size != NS2_FLASH_SIZE_SERIAL)
> >> + return;
> >> + memcpy(ns2->serial, data, size);
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_SERIAL);
> >> + break;
> >> + case NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB:
> >> + if (size != NS2_FLASH_SIZE_FACTORY_AXIS_CALIB)
> >> + return;
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_PRIMARY_CALIB);
> >> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[0], data);
> >> + if (ok) {
> >> + hid_dbg(ns2->hdev, "Got factory primary stick calibration:\n");
> >> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
> >> + ns2->stick_calib[0].x.negative,
> >> + ns2->stick_calib[0].x.neutral,
> >> + ns2->stick_calib[0].x.positive);
> >> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
> >> + ns2->stick_calib[0].y.negative,
> >> + ns2->stick_calib[0].y.neutral,
> >> + ns2->stick_calib[0].y.positive);
> >> + } else {
> >> + hid_dbg(ns2->hdev, "Factory primary stick calibration not present\n");
> >> + }
> >> + break;
> >> + case NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB:
> >> + if (size != NS2_FLASH_SIZE_FACTORY_AXIS_CALIB)
> >> + return;
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_SECONDARY_CALIB);
> >> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[1], data);
> >> + if (ok) {
> >> + hid_dbg(ns2->hdev, "Got factory secondary stick calibration:\n");
> >> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
> >> + ns2->stick_calib[1].x.negative,
> >> + ns2->stick_calib[1].x.neutral,
> >> + ns2->stick_calib[1].x.positive);
> >> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
> >> + ns2->stick_calib[1].y.negative,
> >> + ns2->stick_calib[1].y.neutral,
> >> + ns2->stick_calib[1].y.positive);
> >> + } else {
> >> + hid_dbg(ns2->hdev, "Factory secondary stick calibration not present\n");
> >> + }
> >> + break;
> >> + case NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB:
> >> + if (size != NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB)
> >> + return;
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_TRIGGER_CALIB);
> >> + if (data[0] != 0xFF && data[1] != 0xFF) {
> >> + ns2->lt_zero = data[0];
> >> + ns2->rt_zero = data[1];
> >> +
> >> + hid_dbg(ns2->hdev, "Got factory trigger calibration:\n");
> >> + hid_dbg(ns2->hdev, "Left zero point: %i\n", ns2->lt_zero);
> >> + hid_dbg(ns2->hdev, "Right zero point: %i\n", ns2->rt_zero);
> >> + } else {
> >> + hid_dbg(ns2->hdev, "Factory trigger calibration not present\n");
> >> + }
> >> + break;
> >> + case NS2_FLASH_ADDR_USER_PRIMARY_CALIB:
> >> + if (size != NS2_FLASH_SIZE_USER_AXIS_CALIB)
> >> + return;
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_PRIMARY_CALIB);
> >> + if (__le16_to_cpu(*(__le16 *)data) != NS2_USER_CALIB_MAGIC) {
> >> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");
> >> + break;
> >> + }
> >> +
> >> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[0], &data[2]);
> >> + if (ok) {
> >> + hid_dbg(ns2->hdev, "Got user primary stick calibration:\n");
> >> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
> >> + ns2->stick_calib[0].x.negative,
> >> + ns2->stick_calib[0].x.neutral,
> >> + ns2->stick_calib[0].x.positive);
> >> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
> >> + ns2->stick_calib[0].y.negative,
> >> + ns2->stick_calib[0].y.neutral,
> >> + ns2->stick_calib[0].y.positive);
> >> + } else {
> >> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");
> >> + }
> >> + break;
> >> + case NS2_FLASH_ADDR_USER_SECONDARY_CALIB:
> >> + if (size != NS2_FLASH_SIZE_USER_AXIS_CALIB)
> >> + return;
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_SECONDARY_CALIB);
> >> + if (__le16_to_cpu(*(__le16 *)data) != NS2_USER_CALIB_MAGIC) {
> >> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n");
> >> + break;
> >> + }
> >> +
> >> + ok = switch2_parse_stick_calibration(&ns2->stick_calib[1], &data[2]);
> >> + if (ok) {
> >> + hid_dbg(ns2->hdev, "Got user secondary stick calibration:\n");
> >> + hid_dbg(ns2->hdev, "Left max: %i, neutral: %i, right max: %i\n",
> >> + ns2->stick_calib[1].x.negative,
> >> + ns2->stick_calib[1].x.neutral,
> >> + ns2->stick_calib[1].x.positive);
> >> + hid_dbg(ns2->hdev, "Down max: %i, neutral: %i, up max: %i\n",
> >> + ns2->stick_calib[1].y.negative,
> >> + ns2->stick_calib[1].y.neutral,
> >> + ns2->stick_calib[1].y.positive);
> >> + } else {
> >> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n");
> >> + }
> >> + break;
> >> + }
> >> +}
> >> +
> >> +static void switch2_report_buttons(struct input_dev *input, const uint8_t *bytes,
> >> + const struct switch2_ctlr_button_mapping button_mappings[])
> >> +{
> >> + const struct switch2_ctlr_button_mapping *button;
> >> +
> >> + for (button = button_mappings; button->code; button++)
> >> + input_report_key(input, button->code, bytes[button->byte] & button->bit);
> >> +}
> >> +
> >> +static void switch2_report_axis(struct input_dev *input, struct switch2_axis_calibration *calib,
> >> + int axis, bool invert, int value)
> >> +{
> >> + if (calib && calib->neutral && calib->negative && calib->positive) {
> >> + value -= calib->neutral;
> >> + value *= NS2_AXIS_MAX + 1;
> >> + if (value < 0)
> >> + value /= calib->negative;
> >> + else
> >> + value /= calib->positive;
> >> + } else {
> >> + value = (value - 2048) * 16;
> >> + }
> >> +
> >> + if (invert)
> >> + value = -value;
> >> + input_report_abs(input, axis,
> >> + clamp(value, NS2_AXIS_MIN, NS2_AXIS_MAX));
> >> +}
> >> +
> >> +static void switch2_report_stick(struct input_dev *input, struct switch2_stick_calibration *calib,
> >> + int x, bool invert_x, int y, bool invert_y, const uint8_t *data)
> >> +{
> >> + switch2_report_axis(input, &calib->x, x, invert_x, data[0] | ((data[1] & 0x0F) << 8));
> >> + switch2_report_axis(input, &calib->y, y, invert_y, (data[1] >> 4) | (data[2] << 4));
> >> +}
> >> +
> >> +static void switch2_report_trigger(struct input_dev *input, uint8_t zero, int abs, uint8_t data)
> >> +{
> >> + int value = (NS2_TRIGGER_RANGE + 1) * (data - zero) / (232 - zero);
> >> +
> >> + input_report_abs(input, abs, clamp(value, 0, NS2_TRIGGER_RANGE));
> >> +}
> >> +
> >> +static int switch2_event(struct hid_device *hdev, struct hid_report *report, uint8_t *raw_data,
> >> + int size)
> >> +{
> >> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
> >> + struct input_dev *input;
> >> +
> >> + if (report->type != HID_INPUT_REPORT)
> >> + return 0;
> >> +
> >> + if (size < 15)
> >> + return -EINVAL;
> >> +
> >> + guard(rcu)();
> >> + input = rcu_dereference(ns2->input);
> >> +
> >> + if (!input)
> >> + return 0;
> >> +
> >> + switch (report->id) {
> >> + case NS2_REPORT_UNIFIED:
> >> + /*
> >> + * TODO
> >> + * This won't be sent unless the report type gets changed via command
> >> + * 03-0A, but we should support it at some point regardless.
> >> + */
> >> + break;
> >> + case NS2_REPORT_JCL:
> >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
> >> + ABS_Y, true, &raw_data[6]);
> >> + switch2_report_buttons(input, &raw_data[3], ns2_left_joycon_button_mappings);
> >> + break;
> >> + case NS2_REPORT_JCR:
> >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
> >> + ABS_Y, true, &raw_data[6]);
> >> + switch2_report_buttons(input, &raw_data[3], ns2_right_joycon_button_mappings);
> >> + break;
> >> + case NS2_REPORT_GC:
> >> + input_report_abs(input, ABS_HAT0X,
> >> + !!(raw_data[4] & NS2_BTNL_RIGHT) -
> >> + !!(raw_data[4] & NS2_BTNL_LEFT));
> >> + input_report_abs(input, ABS_HAT0Y,
> >> + !!(raw_data[4] & NS2_BTNL_DOWN) -
> >> + !!(raw_data[4] & NS2_BTNL_UP));
> >> + switch2_report_buttons(input, &raw_data[3], ns2_gccon_mappings);
> >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
> >> + ABS_Y, true, &raw_data[6]);
> >> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, false,
> >> + ABS_RY, true, &raw_data[9]);
> >> + switch2_report_trigger(input, ns2->lt_zero, ABS_Z, raw_data[13]);
> >> + switch2_report_trigger(input, ns2->rt_zero, ABS_RZ, raw_data[14]);
> >> + break;
> >> + case NS2_REPORT_PRO:
> >> + input_report_abs(input, ABS_HAT0X,
> >> + !!(raw_data[4] & NS2_BTNL_RIGHT) -
> >> + !!(raw_data[4] & NS2_BTNL_LEFT));
> >> + input_report_abs(input, ABS_HAT0Y,
> >> + !!(raw_data[4] & NS2_BTNL_DOWN) -
> >> + !!(raw_data[4] & NS2_BTNL_UP));
> >> + switch2_report_buttons(input, &raw_data[3], ns2_procon_mappings);
> >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, false,
> >> + ABS_Y, true, &raw_data[6]);
> >> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, false,
> >> + ABS_RY, true, &raw_data[9]);
> >> + break;
> >> + default:
> >> + return -EINVAL;
> >> + }
> >> +
> >> + input_sync(input);
> >> + return 0;
> >> +}
> >> +
> >> +static int switch2_features_enable(struct switch2_controller *ns2, int features)
> >> +{
> >> + __le32 feature_bits = __cpu_to_le32(features);
> >> +
> >> + if (!ns2->cfg)
> >> + return -ENOTCONN;
> >> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_ENABLE,
> >> + &feature_bits, sizeof(feature_bits),
> >> + ns2->cfg);
> >> +}
> >> +
> >> +static int switch2_read_flash(struct switch2_controller *ns2, uint32_t address,
> >> + uint8_t size)
> >> +{
> >> + uint8_t message[8] = { size, 0x7e };
> >> +
> >> + if (!ns2->cfg)
> >> + return -ENOTCONN;
> >> + *(__le32 *)&message[4] = __cpu_to_le32(address);
> >> + return ns2->cfg->send_command(NS2_CMD_FLASH, NS2_SUBCMD_FLASH_READ, message,
> >> + sizeof(message), ns2->cfg);
> >> +}
> >> +
> >> +static int switch2_set_player_id(struct switch2_controller *ns2, uint32_t player_id)
> >> +{
> >> + int i;
> >> + int player_led_pattern = player_id % JC_NUM_LED_PATTERNS;
> >> +
> >> + for (i = 0; i < JC_NUM_LEDS; i++)
> >> + ns2->leds[i].brightness = joycon_player_led_patterns[player_led_pattern][i];
> >> +
> >> + return switch2_set_leds(ns2);
> >> +}
> >> +
> >> +static int switch2_set_report_format(struct switch2_controller *ns2, enum switch2_report_id fmt)
> >> +{
> >> + __le32 format_id = __cpu_to_le32(fmt);
> >> +
> >> + if (!ns2->cfg)
> >> + return -ENOTCONN;
> >> + return ns2->cfg->send_command(NS2_CMD_INIT, NS2_SUBCMD_INIT_SELECT_REPORT,
> >> + &format_id, sizeof(format_id),
> >> + ns2->cfg);
> >> +}
> >> +
> >> +static int switch2_init_controller(struct switch2_controller *ns2)
> >
> > This is now a recursive call while in v1 it wasn't. I think I preferred
> > the non-recursive version as there was one place where init_step
> > state was changed while now I am not sure where it happens (and whether
> > there is a code path where we end up in an infinite recursion)
> >
> > What is the advantage of the recursive version compared to the
> > non-recursive one?
>
>
> The old version incremented the step regardless of whether or not it
> could confirm it had happened. Since the confirmation is now handled
> with an external step, calling into switch2_init_step_done, the loop
> condition would become somewhat complicated.
> I replaced it with explicit tail calls since that make the
> control flow simplier, and it is always matched with a call to
> switch2_init_step_done to ensure that the state is always advanced. As
From what I can tell switch2_init_step_done currently only advances
the state if the current state is the expected one. This seems fine,
but it also means that if the state is not the expected one, the
state is not advanced and the recursive call continues anyway (in the
NS2_INIT_READ_USER_SECONDARY_CALIB case, for example). I assume this
should never happen but if we end up in this case for some reason we
will recurse forever.
The same case also potentially calls switch2_read_flash and it isn't
clear to me if this means that the initialisation is done (as there
is no switch2_init_step_done call and we are not in the FINISH state
either). There is also the possibility of switch2_read_flash calling
switch2_init_controller again, which one then has to check ... (note
that this is not the case here though)
To me it seems like it would be clearer to do a `ns2->init_step++` and
then `continue` to make the progress of the state more visible and to
do an explicit `break` when we are supposed to stop the initialisation.
> such, the number recursive calls is explicitly limited, and the fact
> that they're tail calls should mean that it doesn't increase the stack
> depth (unless there's something I don't know about how the kernel
> is compiled, which is possible in the wake of things like retpoline
> protections).
I actually wasn't aware that the tail call optimisation means that no
new stack frame will be added, neat!
I don't know how or if retpoline has an impact on this optimisation
either, I am afraid.
> I suppose I could replace it with a loop, but the condition would be
> the same so the only real difference would be a `while (ns2->init_step
> < NS2_INIT_DONE)` instead of the tail calls; the tail calls themselves
> would just become break statements. There would be no functional
> difference.
hm, wouldn't the tail calls become "continue" statements (as the loop
version would just start a new iteration there)?
I agree that both versions can definitely be implemented in an equivalent
manner with no functional differences, but I do have my preference :)
This is not my call to make anyways though, so let's wait for others to
chime in.
>
> >> +{
> >> + if (ns2->init_step == NS2_INIT_DONE)
> >> + return 0;
> >> +
> >> + if (!ns2->cfg)
> >> + return -ENOTCONN;
> >> +
> >> + switch (ns2->init_step) {
> >> + case NS2_INIT_READ_SERIAL:
> >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_SERIAL,
> >> + NS2_FLASH_SIZE_SERIAL);
> >> + case NS2_INIT_GET_FIRMWARE_INFO:
> >> + return ns2->cfg->send_command(NS2_CMD_FW_INFO, NS2_SUBCMD_FW_INFO_GET,
> >> + NULL, 0, ns2->cfg);
> >> + case NS2_INIT_READ_FACTORY_PRIMARY_CALIB:
> >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB,
> >> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB);
> >> + case NS2_INIT_READ_FACTORY_SECONDARY_CALIB:
> >> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) {
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_SECONDARY_CALIB);
> >> + return switch2_init_controller(ns2);
> >> + }
> >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_SECONDARY_CALIB,
> >> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB);
> >> + case NS2_INIT_READ_FACTORY_TRIGGER_CALIB:
> >> + if (ns2->ctlr_type != NS2_CTLR_TYPE_GC) {
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_FACTORY_TRIGGER_CALIB);
> >> + return switch2_init_controller(ns2);
> >> + }
> >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB,
> >> + NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB);
> >> + case NS2_INIT_READ_USER_PRIMARY_CALIB:
> >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_USER_PRIMARY_CALIB,
> >> + NS2_FLASH_SIZE_USER_AXIS_CALIB);
> >> + case NS2_INIT_READ_USER_SECONDARY_CALIB:
> >> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) {
> >> + switch2_init_step_done(ns2, NS2_INIT_READ_USER_SECONDARY_CALIB);
> >> + return switch2_init_controller(ns2);
> >> + }
> >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_USER_SECONDARY_CALIB,
> >> + NS2_FLASH_SIZE_USER_AXIS_CALIB);
> >> + case NS2_INIT_SET_FEATURE_MASK:
> >> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_SET_MASK,
> >> + switch2_feature_mask, sizeof(switch2_feature_mask), ns2->cfg);
> >> + case NS2_INIT_ENABLE_FEATURES:
> >> + return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG);
> >> + case NS2_INIT_GRIP_BUTTONS:
> >> + if (!switch2_ctlr_is_joycon(ns2->ctlr_type)) {
> >> + switch2_init_step_done(ns2, NS2_INIT_GRIP_BUTTONS);
> >> + return switch2_init_controller(ns2);
> >> + }
> >> + return ns2->cfg->send_command(NS2_CMD_GRIP, NS2_SUBCMD_GRIP_ENABLE_BUTTONS,
> >> + switch2_one_data, sizeof(switch2_one_data),
> >> + ns2->cfg);
> >> + case NS2_INIT_REPORT_FORMAT:
> >> + switch (ns2->ctlr_type) {
> >> + case NS2_CTLR_TYPE_JCL:
> >> + return switch2_set_report_format(ns2, NS2_REPORT_JCL);
> >> + case NS2_CTLR_TYPE_JCR:
> >> + return switch2_set_report_format(ns2, NS2_REPORT_JCR);
> >> + case NS2_CTLR_TYPE_PRO:
> >> + return switch2_set_report_format(ns2, NS2_REPORT_PRO);
> >> + case NS2_CTLR_TYPE_GC:
> >> + return switch2_set_report_format(ns2, NS2_REPORT_GC);
> >> + default:
> >> + switch2_init_step_done(ns2, NS2_INIT_REPORT_FORMAT);
> >> + return switch2_init_controller(ns2);
> >> + }
> >> + case NS2_INIT_SET_PLAYER_LEDS:
> >> + return switch2_set_player_id(ns2, ns2->player_id);
> >> + case NS2_INIT_INPUT:
> >> + return ns2->cfg->send_command(NS2_CMD_INIT, NS2_SUBCMD_INIT_USB,
> >> + switch2_init_cmd_data, sizeof(switch2_init_cmd_data), ns2->cfg);
> >> + case NS2_INIT_FINISH:
> >> + if (ns2->hdev)
> >
> > If this is not set we skip the switch2_init_input call but don't error
> > out. Is this intentional (are we expecting ns2->hdev to be populated at
> > a later time and this step retried, perhaps)?
>
> This is intentional, yes. There are a handful of places this can
> get called, including the function that sets ns2->hdev in the first
> place (switch2_probe). It's to ensure the steps start as soon as
> possible (when the cfg pointer gets set) but it can't finish until the
> hdev gets set, which can be either before or after, depending on the
> ordering the interfaces get enumerated.
I'm probably not correctly following the logic in this case, but if the
steps can only finish once ns2->hdev is set and we are `break;`-ing here
in case it's not, doesn't that mean we never retry (since there is no
recursive call after)?
>
> >
> >> + return switch2_init_input(ns2);
> >> + break;
> >> + default:
> >> + WARN_ON_ONCE(1);
> >> + break;
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +int switch2_receive_command(struct switch2_controller *ns2,
> >> + const uint8_t *message, size_t length)
> >> +{
> >> + const struct switch2_cmd_header *header;
> >> + int ret = 0;
> >> +
> >> + if (length < 8)
> >> + return -EINVAL;
> >> +
> >> + print_hex_dump_debug("got cmd: ", DUMP_PREFIX_OFFSET, 16, 1, message, length, false);
> >> +
> >> + guard(mutex)(&ns2->lock);
> >> +
> >> + header = (const struct switch2_cmd_header *)message;
> >> + if (!(header->flags & NS2_FLAG_OK)) {
> >> + ret = -EIO;
> >> + goto exit;
> >> + }
> >> + message = &message[8];
> >> + switch (header->command) {
> >> + case NS2_CMD_FLASH:
> >> + if (header->subcommand == NS2_SUBCMD_FLASH_READ) {
> >> + uint8_t read_size;
> >> + uint32_t read_address;
> >> +
> >> + if (length < 16) {
> >> + ret = -EINVAL;
> >> + goto exit;
> >> + }
> >> + read_size = message[0];
> >> + read_address = __le32_to_cpu(*(__le32 *)&message[4]);
> >> + if (length < read_size + 16) {
> >> + ret = -EINVAL;
> >> + goto exit;
> >> + }
> >> + switch2_handle_flash_read(ns2, read_size, read_address, &message[8]);
> >> + }
> >> + break;
> >> + case NS2_CMD_INIT:
> >> + if (header->subcommand == NS2_SUBCMD_INIT_USB)
> >> + switch2_init_step_done(ns2, NS2_INIT_INPUT);
> >> + else if (header->subcommand == NS2_SUBCMD_INIT_SELECT_REPORT)
> >> + switch2_init_step_done(ns2, NS2_INIT_REPORT_FORMAT);
> >> + break;
> >> + case NS2_CMD_GRIP:
> >> + if (header->subcommand == NS2_SUBCMD_GRIP_ENABLE_BUTTONS)
> >> + switch2_init_step_done(ns2, NS2_INIT_GRIP_BUTTONS);
> >> + break;
> >> + case NS2_CMD_LED:
> >> + if (header->subcommand == NS2_SUBCMD_LED_PATTERN)
> >> + switch2_init_step_done(ns2, NS2_INIT_SET_PLAYER_LEDS);
> >> + break;
> >> + case NS2_CMD_FEATSEL:
> >> + if (header->subcommand == NS2_SUBCMD_FEATSEL_SET_MASK)
> >> + switch2_init_step_done(ns2, NS2_INIT_SET_FEATURE_MASK);
> >> + else if (header->subcommand == NS2_SUBCMD_FEATSEL_ENABLE)
> >> + switch2_init_step_done(ns2, NS2_INIT_ENABLE_FEATURES);
> >> + break;
> >> + case NS2_CMD_FW_INFO:
> >> + if (header->subcommand == NS2_SUBCMD_FW_INFO_GET) {
> >> + if (length < sizeof(ns2->version)) {
> >> + ret = -EINVAL;
> >> + goto exit;
> >> + }
> >> + memcpy(&ns2->version, message, sizeof(ns2->version));
> >> + ns2->ctlr_type = ns2->version.ctlr_type;
> >> + switch2_init_step_done(ns2, NS2_INIT_GET_FIRMWARE_INFO);
> >> + }
> >> + break;
> >> + default:
> >> + break;
> >> + }
> >> +
> >> +exit:
> >> + if (ns2->init_step < NS2_INIT_DONE)
> >> + switch2_init_controller(ns2);
> >> +
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL_GPL(switch2_receive_command);
> >> +
> >> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg)
> >> +{
> >> + struct switch2_controller *ns2 = switch2_get_controller(phys);
> >> +
> >> + if (IS_ERR(ns2))
> >> + return PTR_ERR(ns2);
> >> +
> >> + cfg->parent = ns2;
> >> +
> >> + guard(mutex)(&ns2->lock);
> >> + WARN_ON(ns2->cfg);
> >> + ns2->cfg = cfg;
> >> +
> >> + if (ns2->hdev)
> >> + return switch2_init_controller(ns2);
> >> + return 0;
> >> +}
> >> +EXPORT_SYMBOL_GPL(switch2_controller_attach_cfg);
> >> +
> >> +void switch2_controller_detach_cfg(struct switch2_controller *ns2)
> >> +{
> >> + mutex_lock(&ns2->lock);
> >> + WARN_ON(ns2 != ns2->cfg->parent);
> >> + ns2->cfg = NULL;
> >> + mutex_unlock(&ns2->lock);
> >> + switch2_controller_put(ns2);
> >> +}
> >> +EXPORT_SYMBOL_GPL(switch2_controller_detach_cfg);
> >> +
> >> +static int switch2_probe(struct hid_device *hdev, const struct hid_device_id *id)
> >> +{
> >> + struct switch2_controller *ns2;
> >> + struct usb_device *udev;
> >> + char phys[64];
> >> + int ret;
> >> +
> >> + if (!hid_is_usb(hdev))
> >> + return -ENODEV;
> >> +
> >> + udev = hid_to_usb_dev(hdev);
> >> + if (usb_make_path(udev, phys, sizeof(phys)) < 0)
> >> + return -EINVAL;
> >> +
> >> + ret = hid_parse(hdev);
> >> + if (ret) {
> >> + hid_err(hdev, "parse failed %d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> >> + if (ret) {
> >> + hid_err(hdev, "hw_start failed %d\n", ret);
> >> + return ret;
> >> + }
> >> +
> >> + ret = hid_hw_open(hdev);
> >> + if (ret) {
> >> + hid_err(hdev, "hw_open failed %d\n", ret);
> >> + goto err_stop;
> >> + }
> >
> > For the Switch 1 controllers we are calling hid_device_io_start after
> > hid_hw_open. Is this not necessary in this case?
>
> Since we don't do any HID I/O during probe it's not needed; the
> startup configuration is on the cfg interface instead.
Ah, I can see that for the Switch 1 controllers we are sending USB
HID packets during probe, while we don't do this for the Switch 2
controllers. That explains why this call is not needed here.
>
> >
> >> +
> >> + ns2 = switch2_get_controller(phys);
> >> + if (!ns2) {
> >
> > switch2_get_controller returns an err pointer in case of ENOMEM, not
> > NULL so I think this check has to be changed.
>
> Fixed locally, thanks. I'll send that out with v4 (this was actually
> v3 but I thought it was v2, oops).
>
> >
> >> + ret = -ENOMEM;
> >> + goto err_close;
> >> + }
> >> +
> >> + guard(mutex)(&ns2->lock);
> >> + WARN_ON(ns2->hdev);
> >> + ns2->hdev = hdev;
> >> + switch (hdev->product | (hdev->vendor << 16)) {
> >> + default:
> >> + strscpy(ns2->name, hdev->name, sizeof(ns2->name));
> >> + break;
> >> + /* Some controllers have slightly wrong names so we override them */
> >> + case USB_DEVICE_ID_NINTENDO_NS2_JOYCONR | (USB_VENDOR_ID_NINTENDO << 16):
> >> + /* Missing the "2" in the name */
> >> + strscpy(ns2->name, "Nintendo Joy-Con 2 (R)", sizeof(ns2->name));
> >> + break;
> >> + case USB_DEVICE_ID_NINTENDO_NS2_GCCON | (USB_VENDOR_ID_NINTENDO << 16):
> >> + /* Has "Nintendo" in the name twice */
> >> + strscpy(ns2->name, "Nintendo GameCube Controller", sizeof(ns2->name));
> >> + break;
> >> + }
> >> +
> >> + ns2->player_id = U32_MAX;
> >> + ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
> >> + if (ret < 0)
> >> + hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
> >> + else
> >> + ns2->player_id = ret;
> >> +
> >> + switch2_leds_create(ns2);
> >> +
> >> + hid_set_drvdata(hdev, ns2);
> >> +
> >> + if (ns2->cfg)
> >> + return switch2_init_controller(ns2);
> >> +
> >> + return 0;
> >> +
> >> +err_close:
> >> + hid_hw_close(hdev);
> >> +err_stop:
> >> + hid_hw_stop(hdev);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void switch2_remove(struct hid_device *hdev)
> >> +{
> >> + struct switch2_controller *ns2 = hid_get_drvdata(hdev);
> >> +
> >> + hid_hw_close(hdev);
> >> + mutex_lock(&ns2->lock);
> >> + WARN_ON(ns2->hdev != hdev);
> >> + ns2->hdev = NULL;
> >> + mutex_unlock(&ns2->lock);
> >> + ida_free(&nintendo_player_id_allocator, ns2->player_id);
> >> + switch2_controller_put(ns2);
> >> + hid_hw_stop(hdev);
> >> +}
> >> +
> >> static const struct hid_device_id nintendo_hid_devices[] = {
> >> + /* Switch devices */
> >> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> USB_DEVICE_ID_NINTENDO_PROCON) },
> >> { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> @@ -2813,10 +3935,69 @@ static const struct hid_device_id nintendo_hid_devices[] = {
> >> USB_DEVICE_ID_NINTENDO_GENCON) },
> >> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> USB_DEVICE_ID_NINTENDO_N64CON) },
> >> + /* Switch 2 devices */
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> + USB_DEVICE_ID_NINTENDO_NS2_JOYCONL) },
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> + USB_DEVICE_ID_NINTENDO_NS2_JOYCONR) },
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> + USB_DEVICE_ID_NINTENDO_NS2_PROCON) },
> >> + { HID_USB_DEVICE(USB_VENDOR_ID_NINTENDO,
> >> + USB_DEVICE_ID_NINTENDO_NS2_GCCON) },
> >> { }
> >> };
> >> MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
> >>
> >> +static bool nintendo_is_switch2(struct hid_device *hdev)
> >> +{
> >> + return hdev->vendor == USB_VENDOR_ID_NINTENDO &&
> >> + hdev->product >= USB_DEVICE_ID_NINTENDO_NS2_JOYCONR;
> >> +}
> >> +
> >> +static void nintendo_hid_remove(struct hid_device *hdev)
> >> +{
> >> + if (nintendo_is_switch2(hdev))
> >> + switch2_remove(hdev);
> >> + else
> >> + joycon_remove(hdev);
> >> +}
> >> +
> >> +static int nintendo_hid_event(struct hid_device *hdev,
> >> + struct hid_report *report, u8 *raw_data, int size)
> >> +{
> >> + if (nintendo_is_switch2(hdev))
> >> + return switch2_event(hdev, report, raw_data, size);
> >> + else
> >> + return joycon_event(hdev, report, raw_data, size);
> >> +}
> >> +
> >> +static int nintendo_hid_probe(struct hid_device *hdev,
> >> + const struct hid_device_id *id)
> >> +{
> >> + if (nintendo_is_switch2(hdev))
> >> + return switch2_probe(hdev, id);
> >> + else
> >> + return joycon_probe(hdev, id);
> >> +}
> >> +
> >> +#ifdef CONFIG_PM
> >> +static int nintendo_hid_resume(struct hid_device *hdev)
> >> +{
> >> + if (nintendo_is_switch2(hdev))
> >> + return 0;
> >> + else
> >> + return joycon_resume(hdev);
> >> +}
> >> +
> >> +static int nintendo_hid_suspend(struct hid_device *hdev, pm_message_t message)
> >> +{
> >> + if (nintendo_is_switch2(hdev))
> >> + return 0;
> >> + else
> >> + return joycon_suspend(hdev, message);
> >> +}
> >> +#endif
> >> +
> >> static struct hid_driver nintendo_hid_driver = {
> >> .name = "nintendo",
> >> .id_table = nintendo_hid_devices,
> >> @@ -2844,4 +4025,5 @@ MODULE_LICENSE("GPL");
> >> MODULE_AUTHOR("Ryan McClelland <rymcclel@gmail.com>");
> >> MODULE_AUTHOR("Emily Strickland <linux@emily.st>");
> >> MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
> >> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
> >> MODULE_DESCRIPTION("Driver for Nintendo Switch Controllers");
> >> diff --git a/drivers/hid/hid-nintendo.h b/drivers/hid/hid-nintendo.h
> >> new file mode 100644
> >> index 0000000000000..7aff22f302661
> >> --- /dev/null
> >> +++ b/drivers/hid/hid-nintendo.h
> >> @@ -0,0 +1,72 @@
> >> +/* SPDX-License-Identifier: GPL-2.0+ */
> >> +/*
> >> + * HID driver for Nintendo Switch 2 controllers
> >> + *
> >> + * Copyright (c) 2025 Valve Software
> >> + *
> >> + * This driver is based on the following work:
> >> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
> >> + * https://github.com/ndeadly/switch2_controller_research
> >> + */
> >> +
> >> +#ifndef __HID_NINTENDO_H
> >> +#define __HID_NINTENDO_H
> >> +
> >> +#include <linux/bits.h>
> >> +
> >> +#define NS2_FLAG_OK BIT(0)
> >> +#define NS2_FLAG_NACK BIT(2)
> >> +
> >> +enum switch2_cmd {
> >> + NS2_CMD_NFC = 0x01,
> >> + NS2_CMD_FLASH = 0x02,
> >> + NS2_CMD_INIT = 0x03,
> >> + NS2_CMD_GRIP = 0x08,
> >> + NS2_CMD_LED = 0x09,
> >> + NS2_CMD_VIBRATE = 0x0a,
> >> + NS2_CMD_BATTERY = 0x0b,
> >> + NS2_CMD_FEATSEL = 0x0c,
> >> + NS2_CMD_FW_UPD = 0x0d,
> >> + NS2_CMD_FW_INFO = 0x10,
> >> + NS2_CMD_BT_PAIR = 0x15,
> >> +};
> >> +
> >> +enum switch2_direction {
> >> + NS2_DIR_IN = 0x00,
> >> + NS2_DIR_OUT = 0x90,
> >> +};
> >> +
> >> +enum switch2_transport {
> >> + NS2_TRANS_USB = 0x00,
> >> + NS2_TRANS_BT = 0x01,
> >> +};
> >> +
> >> +struct switch2_cmd_header {
> >> + uint8_t command;
> >> + uint8_t flags;
> >> + uint8_t transport;
> >> + uint8_t subcommand;
> >> + uint8_t unk1;
> >> + uint8_t length;
> >> + uint16_t unk2;
> >> +};
> >> +static_assert(sizeof(struct switch2_cmd_header) == 8);
> >> +
> >> +struct device;
> >> +struct switch2_controller;
> >> +struct switch2_cfg_intf {
> >> + struct switch2_controller *parent;
> >> + struct device *dev;
> >> +
> >> + int (*send_command)(enum switch2_cmd command, uint8_t subcommand,
> >> + const void *message, size_t length,
> >> + struct switch2_cfg_intf *intf);
> >> +};
> >> +
> >> +int switch2_controller_attach_cfg(const char *phys, struct switch2_cfg_intf *cfg);
> >> +void switch2_controller_detach_cfg(struct switch2_controller *controller);
> >> +
> >> +int switch2_receive_command(struct switch2_controller *controller,
> >> + const uint8_t *message, size_t length);
> >> +
> >> +#endif
> >> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> >> index 7755e5b454d2c..868262c6ccd9a 100644
> >> --- a/drivers/input/joystick/Kconfig
> >> +++ b/drivers/input/joystick/Kconfig
> >> @@ -422,4 +422,15 @@ config JOYSTICK_SEESAW
> >> To compile this driver as a module, choose M here: the module will be
> >> called adafruit-seesaw.
> >>
> >> +config JOYSTICK_NINTENDO_SWITCH2_USB
> >> + tristate "Wired Nintendo Switch 2 controller support"
> >> + depends on HID_NINTENDO
> >> + depends on USB
> >> + help
> >> + Say Y here if you want to enable support for wired Nintendo Switch 2
> >> + controllers.
> >> +
> >> + To compile this driver as a module, choose M here: the
> >> + module will be called nintendo-switch2-usb.
> >> +
> >> endif
> >> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> >> index 9976f596a9208..8f92900ae8856 100644
> >> --- a/drivers/input/joystick/Makefile
> >> +++ b/drivers/input/joystick/Makefile
> >> @@ -34,6 +34,7 @@ obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
> >> obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
> >> obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o
> >> obj-$(CONFIG_JOYSTICK_STINGER) += stinger.o
> >> +obj-$(CONFIG_JOYSTICK_NINTENDO_SWITCH2_USB) += nintendo-switch2-usb.o
> >> obj-$(CONFIG_JOYSTICK_TMDC) += tmdc.o
> >> obj-$(CONFIG_JOYSTICK_TURBOGRAFX) += turbografx.o
> >> obj-$(CONFIG_JOYSTICK_TWIDJOY) += twidjoy.o
> >> diff --git a/drivers/input/joystick/nintendo-switch2-usb.c b/drivers/input/joystick/nintendo-switch2-usb.c
> >> new file mode 100644
> >> index 0000000000000..ebd89d852e21a
> >> --- /dev/null
> >> +++ b/drivers/input/joystick/nintendo-switch2-usb.c
> >> @@ -0,0 +1,353 @@
> >> +// SPDX-License-Identifier: GPL-2.0+
> >> +/*
> >> + * USB driver for Nintendo Switch 2 controllers configuration interface
> >> + *
> >> + * Copyright (c) 2025 Valve Software
> >> + *
> >> + * This driver is based on the following work:
> >> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64
> >> + * https://github.com/ndeadly/switch2_controller_research
> >> + */
> >> +
> >> +#include "../../hid/hid-ids.h"
> >> +#include "../../hid/hid-nintendo.h"
> >> +#include <linux/module.h>
> >> +#include <linux/usb/input.h>
> >> +
> >> +#define NS2_BULK_SIZE 64
> >> +#define NS2_IN_URBS 2
> >> +#define NS2_OUT_URBS 4
> >> +
> >> +static struct usb_driver switch2_usb;
> >> +
> >> +struct switch2_urb {
> >> + struct urb *urb;
> >> + uint8_t *data;
> >> + bool active;
> >> +};
> >> +
> >> +struct switch2_usb {
> >> + struct switch2_cfg_intf cfg;
> >> + struct usb_device *udev;
> >> +
> >> + struct switch2_urb bulk_in[NS2_IN_URBS];
> >> + struct usb_anchor bulk_in_anchor;
> >> + spinlock_t bulk_in_lock;
> >> +
> >> + struct switch2_urb bulk_out[NS2_OUT_URBS];
> >> + struct usb_anchor bulk_out_anchor;
> >> + spinlock_t bulk_out_lock;
> >> +
> >> + int message_in;
> >> + struct work_struct message_in_work;
> >> +};
> >> +
> >> +static void switch2_bulk_in(struct urb *urb)
> >> +{
> >> + struct switch2_usb *ns2_usb = urb->context;
> >> + int i;
> >> + bool schedule = false;
> >> + unsigned long flags;
> >> +
> >> + switch (urb->status) {
> >> + case 0:
> >> + schedule = true;
> >> + break;
> >> + case -ECONNRESET:
> >> + case -ENOENT:
> >> + case -ESHUTDOWN:
> >> + dev_dbg(&ns2_usb->udev->dev, "shutting down input urb: %d\n", urb->status);
> >> + return;
> >> + default:
> >> + dev_dbg(&ns2_usb->udev->dev, "unknown input urb status: %d\n", urb->status);
> >> + break;
> >> + }
> >> +
> >> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> >> + for (i = 0; i < NS2_IN_URBS; i++) {
> >> + int err;
> >> + struct switch2_urb *ns2_urb;
> >> +
> >> + if (ns2_usb->bulk_in[i].urb == urb) {
> >> + ns2_usb->message_in = i;
> >> + continue;
> >> + }
> >> +
> >> + if (ns2_usb->bulk_in[i].active)
> >> + continue;
> >> +
> >> + ns2_urb = &ns2_usb->bulk_in[i];
> >> + usb_anchor_urb(ns2_urb->urb, &ns2_usb->bulk_in_anchor);
> >> + err = usb_submit_urb(ns2_urb->urb, GFP_ATOMIC);
> >> + if (err) {
> >> + usb_unanchor_urb(ns2_urb->urb);
> >> + dev_dbg(&ns2_usb->udev->dev, "failed to queue input urb: %d\n", err);
> >> + } else {
> >> + ns2_urb->active = true;
> >> + }
> >> + }
> >> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
> >> +
> >> + if (schedule)
> >> + schedule_work(&ns2_usb->message_in_work);
> >> +}
> >> +
> >> +static void switch2_bulk_out(struct urb *urb)
> >> +{
> >> + struct switch2_usb *ns2_usb = urb->context;
> >> + int i;
> >> +
> >> + guard(spinlock_irqsave)(&ns2_usb->bulk_out_lock);
> >> +
> >> + switch (urb->status) {
> >> + case 0:
> >> + break;
> >> + case -ECONNRESET:
> >> + case -ENOENT:
> >> + case -ESHUTDOWN:
> >> + dev_dbg(&ns2_usb->udev->dev, "shutting down output urb: %d\n", urb->status);
> >> + return;
> >> + default:
> >> + dev_dbg(&ns2_usb->udev->dev, "unknown output urb status: %d\n", urb->status);
> >> + return;
> >> + }
> >> +
> >> + for (i = 0; i < NS2_OUT_URBS; i++) {
> >> + if (ns2_usb->bulk_out[i].urb != urb)
> >> + continue;
> >> +
> >> + ns2_usb->bulk_out[i].active = false;
> >> + break;
> >> + }
> >> +}
> >> +
> >> +static int switch2_usb_send_cmd(enum switch2_cmd command, uint8_t subcommand,
> >> + const void *message, size_t size, struct switch2_cfg_intf *cfg)
> >> +{
> >> + struct switch2_usb *ns2_usb = (struct switch2_usb *)cfg;
> >> + struct switch2_urb *urb = NULL;
> >> + int i;
> >> + int ret;
> >> + unsigned long flags;
> >> +
> >> + struct switch2_cmd_header header = {
> >> + command, NS2_DIR_OUT | NS2_FLAG_OK, NS2_TRANS_USB, subcommand, 0, size
> >> + };
> >> +
> >> + if (WARN_ON(size > 56))
> >> + return -EINVAL;
> >> +
> >> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
> >> + for (i = 0; i < NS2_OUT_URBS; i++) {
> >> + if (ns2_usb->bulk_out[i].active)
> >> + continue;
> >> +
> >> + urb = &ns2_usb->bulk_out[i];
> >> + urb->active = true;
> >> + break;
> >> + }
> >> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
> >> +
> >> + if (!urb) {
> >> + dev_warn(&ns2_usb->udev->dev, "output queue full, dropping message\n");
> >> + return -ENOBUFS;
> >> + }
> >> +
> >> + memcpy(urb->data, &header, sizeof(header));
> >> + if (message && size)
> >> + memcpy(&urb->data[8], message, size);
> >> + urb->urb->transfer_buffer_length = size + sizeof(header);
> >> +
> >> + print_hex_dump_debug("sending cmd: ", DUMP_PREFIX_OFFSET, 16, 1, urb->data,
> >> + size + sizeof(header), false);
> >> +
> >> + usb_anchor_urb(urb->urb, &ns2_usb->bulk_out_anchor);
> >> + ret = usb_submit_urb(urb->urb, GFP_ATOMIC);
> >> + if (ret) {
> >> + if (ret != -ENODEV)
> >> + dev_warn(&ns2_usb->udev->dev, "failed to submit output urb: %i", ret);
> >> + urb->active = false;
> >> + usb_unanchor_urb(urb->urb);
> >> + return ret;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void switch2_usb_message_in_work(struct work_struct *work)
> >> +{
> >> + struct switch2_usb *ns2_usb = container_of(work, struct switch2_usb, message_in_work);
> >> + struct switch2_urb *urb;
> >> + int err;
> >> + unsigned long flags;
> >> +
> >> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> >> + urb = &ns2_usb->bulk_in[ns2_usb->message_in];
> >> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
> >> +
> >> + err = switch2_receive_command(ns2_usb->cfg.parent, urb->urb->transfer_buffer,
> >> + urb->urb->actual_length);
> >> + if (err)
> >> + dev_dbg(&ns2_usb->udev->dev, "receive command failed: %d\n", err);
> >> +
> >> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> >> + urb->active = false;
> >> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
> >> +}
> >> +
> >> +static int switch2_usb_probe(struct usb_interface *intf, const struct usb_device_id *id)
> >> +{
> >> + struct switch2_usb *ns2_usb;
> >> + struct usb_device *udev;
> >> + struct usb_endpoint_descriptor *bulk_in, *bulk_out;
> >> + char phys[64];
> >> + int ret;
> >> + int i;
> >> +
> >> + udev = interface_to_usbdev(intf);
> >> + if (usb_make_path(udev, phys, sizeof(phys)) < 0)
> >> + return -EINVAL;
> >> +
> >> + ret = usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &bulk_out, NULL, NULL);
> >> + if (ret) {
> >> + dev_err(&intf->dev, "failed to find bulk EPs\n");
> >> + return ret;
> >> + }
> >> +
> >> + ns2_usb = devm_kzalloc(&intf->dev, sizeof(*ns2_usb), GFP_KERNEL);
> >> + if (!ns2_usb)
> >> + return -ENOMEM;
> >> +
> >> + ns2_usb->udev = udev;
> >> + for (i = 0; i < NS2_IN_URBS; i++) {
> >> + ns2_usb->bulk_in[i].urb = usb_alloc_urb(0, GFP_KERNEL);
> >> + if (!ns2_usb->bulk_in[i].urb) {
> >> + ret = -ENOMEM;
> >> + goto err_free_in;
> >> + }
> >> +
> >> + ns2_usb->bulk_in[i].data = usb_alloc_coherent(udev, NS2_BULK_SIZE, GFP_KERNEL,
> >> + &ns2_usb->bulk_in[i].urb->transfer_dma);
> >> + if (!ns2_usb->bulk_in[i].data) {
> >> + ret = -ENOMEM;
> >> + goto err_free_in;
> >> + }
> >> +
> >> + usb_fill_bulk_urb(ns2_usb->bulk_in[i].urb, udev,
> >> + usb_rcvbulkpipe(udev, bulk_in->bEndpointAddress),
> >> + ns2_usb->bulk_in[i].data, NS2_BULK_SIZE, switch2_bulk_in, ns2_usb);
> >> + ns2_usb->bulk_in[i].urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >> + }
> >> +
> >> + for (i = 0; i < NS2_OUT_URBS; i++) {
> >> + ns2_usb->bulk_out[i].urb = usb_alloc_urb(0, GFP_KERNEL);
> >> + if (!ns2_usb->bulk_out[i].urb) {
> >> + ret = -ENOMEM;
> >> + goto err_free_out;
> >> + }
> >> +
> >> + ns2_usb->bulk_out[i].data = usb_alloc_coherent(udev, NS2_BULK_SIZE, GFP_KERNEL,
> >> + &ns2_usb->bulk_out[i].urb->transfer_dma);
> >> + if (!ns2_usb->bulk_out[i].data) {
> >> + ret = -ENOMEM;
> >> + goto err_free_out;
> >> + }
> >> +
> >> + usb_fill_bulk_urb(ns2_usb->bulk_out[i].urb, udev,
> >> + usb_sndbulkpipe(udev, bulk_out->bEndpointAddress),
> >> + ns2_usb->bulk_out[i].data, NS2_BULK_SIZE, switch2_bulk_out, ns2_usb);
> >> + ns2_usb->bulk_out[i].urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
> >> + }
> >> +
> >> + ns2_usb->bulk_in[0].active = true;
> >> + ret = usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC);
> >> + if (ret < 0)
> >> + goto err_free_out;
> >> +
> >> + init_usb_anchor(&ns2_usb->bulk_out_anchor);
> >> + spin_lock_init(&ns2_usb->bulk_out_lock);
> >> + init_usb_anchor(&ns2_usb->bulk_in_anchor);
> >> + spin_lock_init(&ns2_usb->bulk_in_lock);
> >> + INIT_WORK(&ns2_usb->message_in_work, switch2_usb_message_in_work);
> >> +
> >> + usb_set_intfdata(intf, ns2_usb);
> >> +
> >> + ns2_usb->cfg.dev = &ns2_usb->udev->dev;
> >> + ns2_usb->cfg.send_command = switch2_usb_send_cmd;
> >> +
> >> + ret = switch2_controller_attach_cfg(phys, &ns2_usb->cfg);
> >> + if (ret < 0)
> >> + goto err_kill_urb;
> >> +
> >> + return 0;
> >> +
> >> +err_kill_urb:
> >> + usb_kill_urb(ns2_usb->bulk_in[0].urb);
> >> +err_free_out:
> >> + for (i = 0; i < NS2_OUT_URBS; i++) {
> >> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> >> + ns2_usb->bulk_out[i].urb->transfer_dma);
> >> + usb_free_urb(ns2_usb->bulk_out[i].urb);
> >> + }
> >> +err_free_in:
> >> + for (i = 0; i < NS2_IN_URBS; i++) {
> >> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
> >> + ns2_usb->bulk_in[i].urb->transfer_dma);
> >> + usb_free_urb(ns2_usb->bulk_in[i].urb);
> >> + }
> >> + devm_kfree(&intf->dev, ns2_usb);
> >> +
> >> + return ret;
> >> +}
> >> +
> >> +static void switch2_usb_disconnect(struct usb_interface *intf)
> >> +{
> >> + struct switch2_usb *ns2_usb = usb_get_intfdata(intf);
> >> + unsigned long flags;
> >> + int i;
> >> +
> >> + spin_lock_irqsave(&ns2_usb->bulk_out_lock, flags);
> >> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor);
> >> + for (i = 0; i < NS2_OUT_URBS; i++) {
> >> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_out[i].data,
> >> + ns2_usb->bulk_out[i].urb->transfer_dma);
> >> + usb_free_urb(ns2_usb->bulk_out[i].urb);
> >> + }
> >> + spin_unlock_irqrestore(&ns2_usb->bulk_out_lock, flags);
> >> +
> >> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags);
> >> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor);
> >> + cancel_work_sync(&ns2_usb->message_in_work);
> >> + for (i = 0; i < NS2_IN_URBS; i++) {
> >> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i].data,
> >> + ns2_usb->bulk_in[i].urb->transfer_dma);
> >> + usb_free_urb(ns2_usb->bulk_in[i].urb);
> >> + }
> >> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags);
> >> +
> >> + switch2_controller_detach_cfg(ns2_usb->cfg.parent);
> >
> > As we have allocated ns2_usb with devm_kzalloc, don't we need to free it
> > with devm_kfree again?
>
> Devres will automatically free it when the device is freed. As this is
> the callback for the device going away, it's freed directly after this
> function exits. That's why I'm using devm_kzalloc instead of kzalloc.
Makes sense!
Thanks for helping me to understand the code better!
Cheers,
Silvan
>
> >
> > Cheers,
> > Silvan
> >
> >> +}
> >> +
> >> +#define SWITCH2_CONTROLLER(vend, prod) \
> >> + USB_DEVICE_AND_INTERFACE_INFO(vend, prod, USB_CLASS_VENDOR_SPEC, 0, 0)
> >> +
> >> +static const struct usb_device_id switch2_usb_devices[] = {
> >> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_JOYCONL) },
> >> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_JOYCONR) },
> >> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_PROCON) },
> >> + { SWITCH2_CONTROLLER(USB_VENDOR_ID_NINTENDO, USB_DEVICE_ID_NINTENDO_NS2_GCCON) },
> >> + { }
> >> +};
> >> +MODULE_DEVICE_TABLE(usb, switch2_usb_devices);
> >> +
> >> +static struct usb_driver switch2_usb = {
> >> + .name = "switch2",
> >> + .id_table = switch2_usb_devices,
> >> + .probe = switch2_usb_probe,
> >> + .disconnect = switch2_usb_disconnect,
> >> +};
> >> +module_usb_driver(switch2_usb);
> >> +
> >> +MODULE_LICENSE("GPL");
> >> +MODULE_AUTHOR("Vicki Pfau <vi@endrift.com>");
> >> +MODULE_DESCRIPTION("Driver for Nintendo Switch 2 Controllers");
> >
> >
>
> Vicki
^ permalink raw reply
* Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event
From: Michael Zaidman @ 2026-04-09 19:17 UTC (permalink / raw)
To: Jiri Kosina
Cc: Sebastian Josue Alba Vives, Benjamin Tissoires, linux-i2c,
linux-input, linux-kernel, stable
In-Reply-To: <7qr72215-4q40-qon4-808o-7o639qq90q3s@xreary.bet>
On Thu, Apr 9, 2026 at 9:29 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Thu, 9 Apr 2026, Michael Zaidman wrote:
>
> > The FT260 uses different report IDs (0xD0 through 0xDE) for different payload
> > lengths, with each report ID defining a different report size in the HID
> > descriptor. So yes, the device can legitimately send reports shorter than
> > FT260_REPORT_MAX_LENGTH, and a blanket size < 64 check would break valid
> > short transfers.
>
> Perfect, thanks a lot for the detailed writeup! I was rather suspicious
> about the bold statement in the changelog.
>
> Similarly to other Sebastián's fixes to various other drivers. This will
> need more thorough check.
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
Hi Jiri,
Indeed. The original patch would have been easily caught by testing on actual
FT260 hardware - short transfers using report IDs 0xD0 through 0xD3 carry well
under 64 bytes and are part of normal I2C operation. A blanket size < 64 check
would break them immediately.
I'll submit a proper fix with per-report-ID capacity validation based on the
HID descriptor.
^ permalink raw reply
* Re: [PATCH v2] HID: mcp2221: validate report size in raw_event handler
From: Jiri Kosina @ 2026-04-09 18:33 UTC (permalink / raw)
To: Sebastian Josue Alba Vives
Cc: gupt21, bentiss, linux-i2c, linux-input, linux-kernel, stable
In-Reply-To: <20260324170606.5407-1-sebasjosue84@gmail.com>
On Tue, 24 Mar 2026, Sebastian Josue Alba Vives wrote:
> MCP2221 devices use 64-byte HID reports.
> Add a check at the top of the handler to reject any report shorter than
> expected, and log a warning to aid debugging.
Similarly to ft260 -- where is the claim that the device can't send
shorter reports coming from, please?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event
From: Jiri Kosina @ 2026-04-09 18:28 UTC (permalink / raw)
To: Michael Zaidman
Cc: Sebastian Josue Alba Vives, Benjamin Tissoires, linux-i2c,
linux-input, linux-kernel, stable
In-Reply-To: <CAPnwWgPhb+owa69-pTADpqk=KMWH71EUT6cxwCeT5KGnBWk+Xg@mail.gmail.com>
On Thu, 9 Apr 2026, Michael Zaidman wrote:
> The FT260 uses different report IDs (0xD0 through 0xDE) for different payload
> lengths, with each report ID defining a different report size in the HID
> descriptor. So yes, the device can legitimately send reports shorter than
> FT260_REPORT_MAX_LENGTH, and a blanket size < 64 check would break valid
> short transfers.
Perfect, thanks a lot for the detailed writeup! I was rather suspicious
about the bold statement in the changelog.
Similarly to other Sebastián's fixes to various other drivers. This will
need more thorough check.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event
From: Michael Zaidman @ 2026-04-09 18:24 UTC (permalink / raw)
To: Jiri Kosina
Cc: Sebastian Josue Alba Vives, Benjamin Tissoires, linux-i2c,
linux-input, linux-kernel, stable
In-Reply-To: <2o8np813-n9n6-32sn-922p-6qnrq45s7rs7@xreary.bet>
On Thu, Apr 9, 2026 at 6:50 PM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Tue, 24 Mar 2026, Sebastian Josue Alba Vives wrote:
>
> > ft260_raw_event() casts the raw data buffer to a
> > ft260_i2c_input_report struct and accesses its fields without
> > validating the size parameter. Since __hid_input_report() invokes
> > the driver's raw_event callback before hid_report_raw_event()
> > performs its own report-size validation, a device sending a
> > truncated HID report can cause out-of-bounds heap reads.
> >
> > Additionally, even with a full-sized report, a corrupted
> > xfer->length field can cause memcpy to read beyond the report
> > buffer. The existing check only validates against the destination
> > buffer size, not the source data available in the report.
> >
> > Add two checks: reject reports shorter than FT260_REPORT_MAX_LENGTH,
> > and verify that xfer->length does not exceed the actual data
> > available in the report. Log warnings to aid debugging.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
> > ---
> > drivers/hid/hid-ft260.c | 11 +++++++++++
> > 1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> > index 333341e80..68008a423 100644
> > --- a/drivers/hid/hid-ft260.c
> > +++ b/drivers/hid/hid-ft260.c
> > @@ -1068,6 +1068,17 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> > struct ft260_device *dev = hid_get_drvdata(hdev);
> > struct ft260_i2c_input_report *xfer = (void *)data;
> >
> > + if (size < FT260_REPORT_MAX_LENGTH) {
> > + hid_warn(hdev, "short report: %d\n", size);
> > + return 0;
>
> Michael, can you please confirm whether the device can never legitimately
> send shorter than FT260_REPORT_MAX_LENGTH reports?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>
Hi Jiri,
The FT260 uses different report IDs (0xD0 through 0xDE) for different payload
lengths, with each report ID defining a different report size in the HID
descriptor. So yes, the device can legitimately send reports shorter than
FT260_REPORT_MAX_LENGTH, and a blanket size < 64 check would break valid
short transfers.
Looking at __hid_input_report(), the HID core could validate size against
hid_compute_report_size(report) before the raw_event call - essentially
moving the check that hid_report_raw_event() already does to happen earlier.
That would handle truncated reports generically for all HID drivers. However,
such a change would affect all HID drivers and require broad testing, so that
is your call.
What the HID core cannot validate is driver-specific payload semantics. In
ft260, the I2C input report has a length field at byte 1 that indicates the
payload size, and the driver uses it as the memcpy length without checking
it against the actual report size or against the expected data capacity for
the specific report ID.
I will submit a per-driver fix with two checks, both essential:
First, a minimum size check before accessing any header fields. Currently,
the HID core does not validate report size before calling raw_event, so a
1-byte report would cause an OOB read just from accessing the length field
at byte 1. This check is necessary regardless of the second check.
Second, a validation of xfer->length against the expected data capacity for
the given report ID. Each I2C input report ID defines a specific data capacity
(report 0xD0 holds up to 4 bytes, 0xDE up to 60 bytes). A corrupted length
field exceeding this capacity would cause an OOB read from the source buffer
during memcpy, even if the report itself is full-sized. Only the driver knows
these per-report-ID limits.
I have the hardware to test this change. I will credit Sebastian with
Reported-by for identifying the issue.
Thanks,
Michael
^ permalink raw reply
* Re: [PATCH 00/61] treewide: Use IS_ERR_OR_NULL over manual NULL check - refactor
From: Al Viro @ 2026-04-09 18:16 UTC (permalink / raw)
To: Philipp Hahn
Cc: amd-gfx, apparmor, bpf, ceph-devel, cocci, dm-devel, dri-devel,
gfs2, intel-gfx, intel-wired-lan, iommu, kvm, linux-arm-kernel,
linux-block, linux-bluetooth, linux-btrfs, linux-cifs, linux-clk,
linux-erofs, linux-ext4, linux-fsdevel, linux-gpio, linux-hyperv,
linux-input, linux-kernel, linux-leds, linux-media, linux-mips,
linux-mm, linux-modules, linux-mtd, linux-nfs, linux-omap,
linux-phy, linux-pm, linux-rockchip, linux-s390, linux-scsi,
linux-sctp, linux-security-module, linux-sh, linux-sound,
linux-stm32, linux-trace-kernel, linux-usb, linux-wireless,
netdev, ntfs3, samba-technical, sched-ext, target-devel,
tipc-discussion, v9fs, Julia Lawall, Nicolas Palix, Chris Mason,
David Sterba, Ilya Dryomov, Alex Markuze, Viacheslav Dubeyko,
Theodore Ts'o, Andreas Dilger, Steve French, Paulo Alcantara,
Ronnie Sahlberg, Shyam Prasad N, Tom Talpey, Bharath SM,
Eric Van Hensbergen, Latchesar Ionkov, Dominique Martinet,
Christian Schoenebeck, Gao Xiang, Chao Yu, Yue Hu, Jeffle Xu,
Sandeep Dhavale, Hongbo Li, Chunhai Guo, Miklos Szeredi,
Konstantin Komarov, Andreas Gruenbacher, Kees Cook, Tony Luck,
Guilherme G. Piccoli, Jan Kara, Phillip Lougher,
Christian Brauner, Jan Kara, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Tejun Heo, David Vernet, Andrea Righi,
Changwoo Min, Ingo Molnar, Peter Zijlstra, Juri Lelli,
Vincent Guittot, Dietmar Eggemann, Ben Segall, Mel Gorman,
Valentin Schneider, Luis Chamberlain, Petr Pavlu, Daniel Gomez,
Sami Tolvanen, Aaron Tomlin, Sylwester Nawrocki, Liam Girdwood,
Mark Brown, Jaroslav Kysela, Takashi Iwai, Max Filippov,
Paolo Bonzini, John Johansen, Paul Moore, James Morris,
Serge E. Hallyn, Andrew Morton, Alasdair Kergon, Mike Snitzer,
Mikulas Patocka, Benjamin Marzinski, David S. Miller, David Ahern,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Simon Horman,
Marcel Holtmann, Johan Hedberg, Luiz Augusto von Dentz,
Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, Jamal Hadi Salim, Jiri Pirko,
Marcelo Ricardo Leitner, Xin Long, Trond Myklebust,
Anna Schumaker, Chuck Lever, Jeff Layton, NeilBrown,
Olga Kornievskaia, Dai Ngo, Jon Maloy, Johannes Berg,
Catalin Marinas, Russell King, John Crispin, Thomas Bogendoerfer,
Yoshinori Sato, Rich Felker, John Paul Adrian Glaubitz,
Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart,
Jonas Karlman, Jernej Skrabec, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Zhenyu Wang,
Zhi Wang, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
Tvrtko Ursulin, Alex Deucher, Christian König, Sandy Huang,
Heiko Stübner, Andy Yan, Igor Russkikh, Andrew Lunn,
Pavan Chebbi, Michael Chan, Potnuri Bharat Teja, Tony Nguyen,
Przemek Kitszel, Taras Chornyi, Maxime Coquelin, Alexandre Torgue,
Iyappan Subramanian, Keyur Chudgar, Quan Nguyen, Heiner Kallweit,
Marc Zyngier, Thomas Gleixner, Andrew Lunn, Gregory Clement,
Sebastian Hesselbarth, Vinod Koul, Linus Walleij, Ulf Hansson,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger, Sven Schnelle, Martin K. Petersen,
Eduardo Valentin, Keerthy, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Alex Williamson, Mark Greer,
Miquel Raynal, Richard Weinberger, Vignesh Raghavendra,
Shuah Khan, Kieran Bingham, Mauro Carvalho Chehab, Joerg Roedel,
Will Deacon, Robin Murphy, Lee Jones, Pavel Machek, Dave Penkler,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Justin Sanders, Jens Axboe, Georgi Djakov, Michael Turquette,
Stephen Boyd, Philipp Zabel, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Pali Rohár, Dmitry Torokhov
In-Reply-To: <20260310-b4-is_err_or_null-v1-0-bd63b656022d@avm.de>
On Tue, Mar 10, 2026 at 12:48:26PM +0100, Philipp Hahn wrote:
> While doing some static code analysis I stumbled over a common pattern,
> where IS_ERR() is combined with a NULL check. For that there is
> IS_ERR_OR_NULL().
... and valid uses of IS_ERR_OR_NULL are rare as hen teeth.
Most of those are "I'm not sure how this function returns an
error, let's use that just in case".
Please, do not introduce more of that crap.
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: fix race condition when accessing stale stack pointer
From: Jiri Kosina @ 2026-04-09 17:26 UTC (permalink / raw)
To: Benoit Sevens
Cc: Filipe Laíns, Bastien Nocera, Benjamin Tissoires,
linux-input, linux-kernel
In-Reply-To: <20260401144811.1242722-1-bsevens@google.com>
On Wed, 1 Apr 2026, Benoit Sevens wrote:
> From: Beno=C3=AEt Sevens <bsevens@google.com>
>
> The driver uses hidpp->send_receive_buf to point to a stack-allocated
> buffer in the synchronous command path (__do_hidpp_send_message_sync).
> However, this pointer is not cleared when the function returns.
>
> If an event is processed (e.g. by a different thread) while the
> send_mutex is held by a new command, but before that command has
> updated send_receive_buf, the handler (hidpp_raw_hidpp_event) will
> observe that the mutex is locked and dereference the stale pointer.
>
> This results in an out-of-bounds access on a different thread's kernel
> stack (or a NULL pointer dereference on the very first command).
>
> Fix this by:
> 1. Clearing hidpp->send_receive_buf to NULL before releasing the mutex
> in the synchronous command path.
> 2. Moving the assignment of the local 'question' and 'answer' pointers
> inside the mutex_is_locked() block in the handler, and adding
> a NULL check before dereferencing.
Now applied.
Benjamin had some ideas on further cleanup (allocating with __free__
instead of using stack pointer), but that'd be a little bigger cleanup, so
let's keep that separate.
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 09/11] dt-bindings: input: Document hid-over-spi DT schema
From: Dmitry Torokhov @ 2026-04-09 17:16 UTC (permalink / raw)
To: Conor Dooley
Cc: Jingyuan Liang, Jiri Kosina, Benjamin Tissoires, Jonathan Corbet,
Mark Brown, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-input,
linux-doc, linux-kernel, linux-spi, linux-trace-kernel,
devicetree, hbarnor, tfiga, Dmitry Antipov, Jarrett Schultz
In-Reply-To: <20260409-defuse-thank-4b038128fac5@spud>
On Thu, Apr 09, 2026 at 05:02:11PM +0100, Conor Dooley wrote:
> On Thu, Apr 02, 2026 at 01:59:46AM +0000, Jingyuan Liang wrote:
> > Documentation describes the required and optional properties for
> > implementing Device Tree for a Microsoft G6 Touch Digitizer that
> > supports HID over SPI Protocol 1.0 specification.
> >
> > The properties are common to HID over SPI.
> >
> > Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> > Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> > Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> > ---
> > .../devicetree/bindings/input/hid-over-spi.yaml | 126 +++++++++++++++++++++
> > 1 file changed, 126 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> > new file mode 100644
> > index 000000000000..d1b0a2e26c32
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> > @@ -0,0 +1,126 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: HID over SPI Devices
> > +
> > +maintainers:
> > + - Benjamin Tissoires <benjamin.tissoires@redhat.com>
> > + - Jiri Kosina <jkosina@suse.cz>
>
> Why them and not you, the developers of the series?
>
> > +
> > +description: |+
> > + HID over SPI provides support for various Human Interface Devices over the
> > + SPI bus. These devices can be for example touchpads, keyboards, touch screens
> > + or sensors.
> > +
> > + The specification has been written by Microsoft and is currently available
> > + here: https://www.microsoft.com/en-us/download/details.aspx?id=103325
> > +
> > + If this binding is used, the kernel module spi-hid will handle the
> > + communication with the device and the generic hid core layer will handle the
> > + protocol.
>
> This is not relevant to the binding, please remove it.
>
> > +
> > +allOf:
> > + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - items:
> > + - enum:
> > + - microsoft,g6-touch-digitizer
> > + - const: hid-over-spi
> > + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> > + const: hid-over-spi
>
> Why is it allowed but not recommended? Seems to me like we should
> require device-specific compatibles.
Why would we want to change the driver code to add a new compatible each
time a vendor decides to create a chip that is fully hid-spi-protocol
compliant? Or is the plan to still allow "hid-over-spi" fallback but
require device-specific compatible that will be ignored unless there is
device-specific quirk needed?
>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + reset-gpios:
> > + maxItems: 1
> > + description:
> > + GPIO specifier for the digitizer's reset pin (active low). The line must
> > + be flagged with GPIO_ACTIVE_LOW.
> > +
> > + vdd-supply:
> > + description:
> > + Regulator for the VDD supply voltage.
> > +
> > + input-report-header-address:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 0xffffff
> > + description:
> > + A value to be included in the Read Approval packet, listing an address of
> > + the input report header to be put on the SPI bus. This address has 24
> > + bits.
> > +
> > + input-report-body-address:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 0xffffff
> > + description:
> > + A value to be included in the Read Approval packet, listing an address of
> > + the input report body to be put on the SPI bus. This address has 24 bits.
> > +
> > + output-report-address:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + minimum: 0
> > + maximum: 0xffffff
> > + description:
> > + A value to be included in the Output Report sent by the host, listing an
> > + address where the output report on the SPI bus is to be written to. This
> > + address has 24 bits.
> > +
> > + read-opcode:
> > + $ref: /schemas/types.yaml#/definitions/uint8
> > + description:
> > + Value to be used in Read Approval packets. 1 byte.
> > +
> > + write-opcode:
> > + $ref: /schemas/types.yaml#/definitions/uint8
> > + description:
> > + Value to be used in Write Approval packets. 1 byte.
>
> Why can none of these things be determined from the device's compatible?
> On the surface, they like the kinds of things that could/should be.
Why would we want to keep tables of these values in the kernel and again
have to update the driver for each new chip? It also probably
firmware-dependent.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: drop 'default !EXPERT' from HID_PICOLCD symbols
From: Thomas Weißschuh @ 2026-04-09 16:32 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Thomas Weißschuh
The 'EXPERT' configuration is meant to make advanced kconfig options
configurable. It is not meant to actively change these options.
In the case of the HID_PICOLCD suboptions the intention seems to be that
normal users always expect these options to be enabled for the driver.
So 'EXPERT' is enabled, these options should stay enabled and not
automatically and suddenly become disabled. Switch them to 'default y'
to match the intention of a normal user. EXPERT users then can disable
them if needed.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
drivers/hid/Kconfig | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index c1d9f7c6a5f2..1ef4fe3debf2 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -938,7 +938,7 @@ config HID_PICOLCD
config HID_PICOLCD_FB
bool "Framebuffer support" if EXPERT
- default !EXPERT
+ default y
depends on HID_PICOLCD
depends on HID_PICOLCD=FB || FB=y
select FB_SYSMEM_HELPERS_DEFERRED
@@ -948,7 +948,7 @@ config HID_PICOLCD_FB
config HID_PICOLCD_BACKLIGHT
bool "Backlight control" if EXPERT
- default !EXPERT
+ default y
depends on HID_PICOLCD
depends on HID_PICOLCD=BACKLIGHT_CLASS_DEVICE || BACKLIGHT_CLASS_DEVICE=y
help
@@ -957,7 +957,7 @@ config HID_PICOLCD_BACKLIGHT
config HID_PICOLCD_LCD
bool "Contrast control" if EXPERT
- default !EXPERT
+ default y
depends on HID_PICOLCD
depends on HID_PICOLCD=LCD_CLASS_DEVICE || LCD_CLASS_DEVICE=y
help
@@ -965,7 +965,7 @@ config HID_PICOLCD_LCD
config HID_PICOLCD_LEDS
bool "GPO via leds class" if EXPERT
- default !EXPERT
+ default y
depends on HID_PICOLCD
depends on HID_PICOLCD=LEDS_CLASS || LEDS_CLASS=y
help
@@ -973,7 +973,7 @@ config HID_PICOLCD_LEDS
config HID_PICOLCD_CIR
bool "CIR via RC class" if EXPERT
- default !EXPERT
+ default y
depends on HID_PICOLCD
depends on HID_PICOLCD=RC_CORE || RC_CORE=y
help
---
base-commit: 7f87a5ea75f011d2c9bc8ac0167e5e2d1adb1594
change-id: 20260409-hid-picolcd-1b6d7eac5795
Best regards,
--
Thomas Weißschuh <linux@weissschuh.net>
^ permalink raw reply related
* Re: [PATCH v3] HID: winwing: Enable rumble effects
From: Jiri Kosina @ 2026-04-09 16:10 UTC (permalink / raw)
To: Ivan Gorinov; +Cc: linux-input, linux-kernel
In-Reply-To: <20260307052246.GA21987@altimeter-info>
On Sat, 7 Mar 2026, Ivan Gorinov wrote:
> Enable rumble motor control on TGRIP-15E and TGRIP-15EX throttle grips
> by sending haptic feedback commands (EV_FF events) to the input device.
>
> Signed-off-by: Ivan Gorinov <linux-kernel@altimeter.info>
> ---
>
> Changes since v2:
> - Add comments about USB requests for LED and rumble control
>
> Changes since v1:
> - Fix possible NULL pointer dereference
Now applied to hid.git#for-7.1/winwing, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v3 09/11] dt-bindings: input: Document hid-over-spi DT schema
From: Conor Dooley @ 2026-04-09 16:02 UTC (permalink / raw)
To: Jingyuan Liang
Cc: Jiri Kosina, Benjamin Tissoires, Jonathan Corbet, Mark Brown,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-input, linux-doc, linux-kernel, linux-spi,
linux-trace-kernel, devicetree, hbarnor, tfiga, Dmitry Antipov,
Jarrett Schultz
In-Reply-To: <20260402-send-upstream-v3-9-6091c458d357@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 5441 bytes --]
On Thu, Apr 02, 2026 at 01:59:46AM +0000, Jingyuan Liang wrote:
> Documentation describes the required and optional properties for
> implementing Device Tree for a Microsoft G6 Touch Digitizer that
> supports HID over SPI Protocol 1.0 specification.
>
> The properties are common to HID over SPI.
>
> Signed-off-by: Dmitry Antipov <dmanti@microsoft.com>
> Signed-off-by: Jarrett Schultz <jaschultz@microsoft.com>
> Signed-off-by: Jingyuan Liang <jingyliang@chromium.org>
> ---
> .../devicetree/bindings/input/hid-over-spi.yaml | 126 +++++++++++++++++++++
> 1 file changed, 126 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-spi.yaml b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> new file mode 100644
> index 000000000000..d1b0a2e26c32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/hid-over-spi.yaml
> @@ -0,0 +1,126 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/hid-over-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: HID over SPI Devices
> +
> +maintainers:
> + - Benjamin Tissoires <benjamin.tissoires@redhat.com>
> + - Jiri Kosina <jkosina@suse.cz>
Why them and not you, the developers of the series?
> +
> +description: |+
> + HID over SPI provides support for various Human Interface Devices over the
> + SPI bus. These devices can be for example touchpads, keyboards, touch screens
> + or sensors.
> +
> + The specification has been written by Microsoft and is currently available
> + here: https://www.microsoft.com/en-us/download/details.aspx?id=103325
> +
> + If this binding is used, the kernel module spi-hid will handle the
> + communication with the device and the generic hid core layer will handle the
> + protocol.
This is not relevant to the binding, please remove it.
> +
> +allOf:
> + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> +
> +properties:
> + compatible:
> + oneOf:
> + - items:
> + - enum:
> + - microsoft,g6-touch-digitizer
> + - const: hid-over-spi
> + - description: Just "hid-over-spi" alone is allowed, but not recommended.
> + const: hid-over-spi
Why is it allowed but not recommended? Seems to me like we should
require device-specific compatibles.
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> + description:
> + GPIO specifier for the digitizer's reset pin (active low). The line must
> + be flagged with GPIO_ACTIVE_LOW.
> +
> + vdd-supply:
> + description:
> + Regulator for the VDD supply voltage.
> +
> + input-report-header-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 0xffffff
> + description:
> + A value to be included in the Read Approval packet, listing an address of
> + the input report header to be put on the SPI bus. This address has 24
> + bits.
> +
> + input-report-body-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 0xffffff
> + description:
> + A value to be included in the Read Approval packet, listing an address of
> + the input report body to be put on the SPI bus. This address has 24 bits.
> +
> + output-report-address:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 0xffffff
> + description:
> + A value to be included in the Output Report sent by the host, listing an
> + address where the output report on the SPI bus is to be written to. This
> + address has 24 bits.
> +
> + read-opcode:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description:
> + Value to be used in Read Approval packets. 1 byte.
> +
> + write-opcode:
> + $ref: /schemas/types.yaml#/definitions/uint8
> + description:
> + Value to be used in Write Approval packets. 1 byte.
Why can none of these things be determined from the device's compatible?
On the surface, they like the kinds of things that could/should be.
Cheers,
Conor.
> +
> +required:
> + - compatible
> + - interrupts
> + - reset-gpios
> + - vdd-supply
> + - input-report-header-address
> + - input-report-body-address
> + - output-report-address
> + - read-opcode
> + - write-opcode
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> + #include <dt-bindings/gpio/gpio.h>
> +
> + spi {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + hid@0 {
> + compatible = "microsoft,g6-touch-digitizer", "hid-over-spi";
> + reg = <0x0>;
> + interrupts-extended = <&gpio 42 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
> + vdd-supply = <&pm8350c_l3>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&ts_d6_int_bias>;
> + input-report-header-address = <0x1000>;
> + input-report-body-address = <0x1004>;
> + output-report-address = <0x2000>;
> + read-opcode = /bits/ 8 <0x0b>;
> + write-opcode = /bits/ 8 <0x02>;
> + };
> + };
>
> --
> 2.53.0.1185.g05d4b7b318-goog
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] HID: core: do not allow parsing 0-sized reports
From: Jiri Kosina @ 2026-04-09 16:00 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Peter Hutterer, Kees Cook, Alan Stern,
Charles Keepax, linux-input, linux-kernel
In-Reply-To: <acy1QWcu-yfd_kAu@google.com>
On Tue, 31 Mar 2026, Dmitry Torokhov wrote:
> Commit d7db259bd6df ("HID: core: factor out hid_parse_collections()")
> reworked collection parsing code and inadvertently allowed returning
> "success" when parsing 0-sized reports where old code returned -EINVAL.
Ah, good catch, I've missed that.
Applied on top, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: usbhid: refactor endpoint lookup
From: Jiri Kosina @ 2026-04-09 15:58 UTC (permalink / raw)
To: Johan Hovold; +Cc: Benjamin Tissoires, linux-usb, linux-input, linux-kernel
In-Reply-To: <20260330095034.1662231-1-johan@kernel.org>
On Mon, 30 Mar 2026, Johan Hovold wrote:
> Use the common USB helper for looking up interrupt-in endpoints instead
> of open coding.
Good catch, thanks Johan. Now applied to hid.git#for-7.1/core-v2.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: huawei: fix CD30 keyboard report descriptor issue
From: Jiri Kosina @ 2026-04-09 15:57 UTC (permalink / raw)
To: Miao Li; +Cc: bentiss, linux-input, linux-kernel, Miao Li
In-Reply-To: <20260318091249.217283-1-limiao870622@163.com>
On Wed, 18 Mar 2026, Miao Li wrote:
> From: Miao Li <limiao@kylinos.cn>
>
> When the Huawei CD30 USB keyboard undergoes 500 reboot cycles,
> initialization may fail due to a report descriptor problem.
Oh wow, that's a novelty I think.
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: playstation: validate num_touch_reports in DualShock 4 reports
From: Jiri Kosina @ 2026-04-09 15:55 UTC (permalink / raw)
To: FirstName LastName
Cc: Roderick Colenbrander, Benjamin Tissoires, linux-input,
linux-kernel
In-Reply-To: <20260323124737.3223129-1-bsevens@google.com>
Dear FirstName LastName,
there seems to be a way to fix in your mail setup configuration :)
On Mon, 23 Mar 2026, FirstName LastName wrote:
> From: Beno=C3=AEt Sevens <bsevens@google.com>
>
> The DualShock 4 HID driver fails to validate the num_touch_reports field
> received from the device in both USB and Bluetooth input reports.
> A malicious device could set this field to a value larger than the
> allocated size of the touch_reports array (3 for USB, 4 for Bluetooth),
> leading to an out-of-bounds read in dualshock4_parse_report().
>
> This can result in kernel memory disclosure when processing malicious
> HID reports.
>
> Validate num_touch_reports against the array size for the respective
> connection types before processing the touch data.
>
> Signed-off-by: Beno=C3=AEt Sevens <bsevens@google.com>
Applied now to hid.git#for-7.0/upstream-fixes, thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: drop 'default !EXPERT' from tristate symbols
From: Jiri Kosina @ 2026-04-09 15:53 UTC (permalink / raw)
To: Thomas Weißschuh; +Cc: Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <20260321-hid-expert-v1-1-4a6d81b4f3ac@weissschuh.net>
On Sat, 21 Mar 2026, Thomas Weißschuh wrote:
> There is no reason to build random drivers for obscure hardware into the
> core kernel by default.
This is from the time when invididual drivers have been extracted from
monolithing hid.o, and there was a general fear that it would cause
regressions for people, because the generic driver wouldn't work any more
and they'd have to look for fixing that by enabling particular driver.
That ship has sailed ages ago though, so I agree we could just remove it.
> The usages of 'default !EXPERT' for the HID_PICOLCD suboptions are kept,
> as these make some sense, although they probably should use 'default y'.
There are very few fans of 'default y' :)
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: ft260: validate report size and payload length in raw_event
From: Jiri Kosina @ 2026-04-09 15:50 UTC (permalink / raw)
To: Sebastian Josue Alba Vives
Cc: michael.zaidman, Benjamin Tissoires, linux-i2c, linux-input,
linux-kernel, stable
In-Reply-To: <20260324201858.46591-1-sebasjosue84@gmail.com>
On Tue, 24 Mar 2026, Sebastian Josue Alba Vives wrote:
> ft260_raw_event() casts the raw data buffer to a
> ft260_i2c_input_report struct and accesses its fields without
> validating the size parameter. Since __hid_input_report() invokes
> the driver's raw_event callback before hid_report_raw_event()
> performs its own report-size validation, a device sending a
> truncated HID report can cause out-of-bounds heap reads.
>
> Additionally, even with a full-sized report, a corrupted
> xfer->length field can cause memcpy to read beyond the report
> buffer. The existing check only validates against the destination
> buffer size, not the source data available in the report.
>
> Add two checks: reject reports shorter than FT260_REPORT_MAX_LENGTH,
> and verify that xfer->length does not exceed the actual data
> available in the report. Log warnings to aid debugging.
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Sebastian Josue Alba Vives <sebasjosue84@gmail.com>
> ---
> drivers/hid/hid-ft260.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 333341e80..68008a423 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -1068,6 +1068,17 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> struct ft260_device *dev = hid_get_drvdata(hdev);
> struct ft260_i2c_input_report *xfer = (void *)data;
>
> + if (size < FT260_REPORT_MAX_LENGTH) {
> + hid_warn(hdev, "short report: %d\n", size);
> + return 0;
Michael, can you please confirm whether the device can never legitimately
send shorter than FT260_REPORT_MAX_LENGTH reports?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox