From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from endrift.com (endrift.com [173.255.198.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3B49121A457 for ; Thu, 11 Dec 2025 03:08:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=173.255.198.10 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765422526; cv=none; b=pbiW5dapW2iJXzq3izgKKU4BW6iNy6auvlzIazwPtAC8MdRmjPT3z6wImNmqnmE/rmFJPY/YZgl9/GTv747RXYt9EuftR5B6H4IBffMkcjomNHpSJsQi44bzC+9l2dLglSDa+uV2zIXFCnTzQ8hXpCAjuAQePCF3/Gih6OaKzso= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1765422526; c=relaxed/simple; bh=GwROouDXH2OdUDZ5QNXbo0oaBpeXFMWWd8Y0mdtFtZY=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=C4vZru8vr/Gx2uHjQ3UCcNb85/Wr+jRDZ1yU1yBlUzsVopF9xNMV/nEI0oJ170cQHKjbICib99zIX1X0Vm2O8B23/40K8/MezXryQXI0k4wJKq7BhKBhuwwl3+dT8Usex6sXGAu0VSaETsgJkRSRyxHyiZVlWTNghR0E+qT/xKY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=endrift.com; spf=pass smtp.mailfrom=endrift.com; dkim=pass (2048-bit key) header.d=endrift.com header.i=@endrift.com header.b=kOtJA64R; arc=none smtp.client-ip=173.255.198.10 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=endrift.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=endrift.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=endrift.com header.i=@endrift.com header.b="kOtJA64R" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=endrift.com; s=2020; t=1765422519; bh=GwROouDXH2OdUDZ5QNXbo0oaBpeXFMWWd8Y0mdtFtZY=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=kOtJA64RDHnr+dcg5uObeI7y7qklPpfjMDBqidwQTfg3hW3RFRLJKEzBw35BOqSyp VpJmCZGTfHWgWTEcZxcwV3OQEEr0CBhjdmiXZGYsqCb4bSG8FydxXwUxuaMv2jIX62 CW6mY6VF4YfBViT1yigNSrIyYboh3PoYDGmY/wq6ZfIdn4Vwqqf4JQu3m0m2zFiUEf hpJzI8ObRiOnjYcb4+64rBLmuP5iIFC14PbET7VzPRSn/NgZ3x5kHMAa0TJNVctQsZ b355eocJXdezmA6rfm943cxzq1hEo9xOu23GjotfrXU0q4Cf+NCHjQ776sWw6IBhUz jLPqkmzCa8lwQ== Received: from [192.168.0.22] (71-212-14-89.tukw.qwest.net [71.212.14.89]) by endrift.com (Postfix) with ESMTPSA id 032B8A018; Wed, 10 Dec 2025 19:08:38 -0800 (PST) Message-ID: Date: Wed, 10 Dec 2025 19:08:38 -0800 Precedence: bulk X-Mailing-List: linux-input@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] HID: switch2: Add preliminary Switch 2 controller driver To: Lyude Paul , Dmitry Torokhov , Jiri Kosina , Benjamin Tissoires , linux-input@vger.kernel.org Cc: Luiz Augusto von Dentz , Bastien Nocera References: <20251120022457.469342-1-vi@endrift.com> Content-Language: en-US From: Vicki Pfau In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Lyude, On 12/10/25 3:24 PM, Lyude Paul wrote: > Hi there! For those following along - I've worked a bit in input (and a= cross > the kernel for at least a decade) and figured I'd help a bit with revie= w at > Vicki's request >=20 > On Wed, 2025-11-19 at 18:24 -0800, Vicki Pfau 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 "configuratio= n" >> interface. This is the case on both USB and Bluetooth, so this new dri= ver >> uses a split-driver design with the HID interface being the "main" dri= ver >> and the configuration interface is a secondary driver that looks up to= the >> configuration interface, sharing resources on a shared struct. >> >> Due to using a non-standard pairing interface as well as Bluetooth >> communications being extremely limited in the kernel, a custom interfa= ce >> between userspace and the kernel will need to be design, along with br= ingup >> 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 Contro= ller >> 2, and GameCube NSO controllers, as well as basic rumble support. IMU >> support is not yet present. >> >> Signed-off-by: Vicki Pfau >> --- >> MAINTAINERS | 7 + >> drivers/hid/Kconfig | 18 + >> drivers/hid/Makefile | 1 + >> drivers/hid/hid-ids.h | 4 + >> drivers/hid/hid-switch2.c | 971 ++++++++++++++++++++++++++= + >> drivers/hid/hid-switch2.h | 242 +++++++ >> drivers/input/joystick/Kconfig | 8 + >> drivers/input/joystick/Makefile | 1 + >> drivers/input/joystick/switch2-usb.c | 355 ++++++++++ >> 9 files changed, 1607 insertions(+) >> create mode 100644 drivers/hid/hid-switch2.c >> create mode 100644 drivers/hid/hid-switch2.h >> create mode 100644 drivers/input/joystick/switch2-usb.c >> >> diff --git a/MAINTAINERS b/MAINTAINERS >> index e64b94e6b5a97..a5bacbfc86b5a 100644 >> --- a/MAINTAINERS >> +++ b/MAINTAINERS >> @@ -18290,6 +18290,13 @@ L: linux-input@vger.kernel.org >> S: Maintained >> F: drivers/hid/hid-nintendo* >> =20 >> +NINTENDO SWITCH 2 CONTROLLER DRIVER >> +M: Vicki Pfau >> +L: linux-input@vger.kernel.org >> +S: Maintained >> +F: drivers/hid/hid-switch2* >> +F: drivers/input/joystick/switch2-usb.c >> + >> NIOS2 ARCHITECTURE >> M: Dinh Nguyen >> S: Maintained >> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig >> index 04420a713be08..7486212ff5d28 100644 >> --- a/drivers/hid/Kconfig >> +++ b/drivers/hid/Kconfig >> @@ -1141,6 +1141,24 @@ config HID_SUNPLUS >> help >> Support for Sunplus wireless desktop. >> =20 >> +config HID_SWITCH2 >> + tristate "Nintendo Switch 2 controller support" >> + depends on HID >> + select USB_HID >> + help >> + Adds support for the Nintendo Switch 2 Controllers. >> + >> + To compile this driver as a module, choose M here: the >> + module will be called hid-switch2. >> + >> +config SWITCH2_FF >> + bool "Nintendo Switch 2 force feedback support" >> + depends on HID_SWITCH2 >> + select INPUT_FF_MEMLESS >> + help >> + Say Y here if you want to enable force feedback support for Nintendo= >> + Switch 2 game controllers. >=20 > I mentioned this on telegram but for the record: it might be worth cons= idering > if we need this as an actual build option or not For the record, I'm only following the convention set by other drivers wi= th force feedback. I'm A-OK dropping this flag if desired by subsystem ma= intainers, and I have a weak preference for removing it as well. But I'd = defer to the subsystem maintainers here. >=20 >> + >> config HID_RMI >> tristate "Synaptics RMI4 device support" >> select RMI4_CORE >> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile >> index 361a7daedeb85..74a3f82ed82f1 100644 >> --- a/drivers/hid/Makefile >> +++ b/drivers/hid/Makefile >> @@ -131,6 +131,7 @@ obj-$(CONFIG_HID_SPEEDLINK) +=3D hid-speedlink.o >> obj-$(CONFIG_HID_STEAM) +=3D hid-steam.o >> obj-$(CONFIG_HID_STEELSERIES) +=3D hid-steelseries.o >> obj-$(CONFIG_HID_SUNPLUS) +=3D hid-sunplus.o >> +obj-$(CONFIG_HID_SWITCH2) +=3D hid-switch2.o >> obj-$(CONFIG_HID_GREENASIA) +=3D hid-gaff.o >> obj-$(CONFIG_HID_THRUSTMASTER) +=3D hid-tmff.o hid-thrustmaster.o >> obj-$(CONFIG_HID_TIVO) +=3D hid-tivo.o >> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h >> index c4589075a5ed6..eef152f7b55f7 100644 >> --- a/drivers/hid/hid-ids.h >> +++ b/drivers/hid/hid-ids.h >> @@ -1052,6 +1052,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 >> =20 >> #define USB_VENDOR_ID_NOVATEK 0x0603 >> #define USB_DEVICE_ID_NOVATEK_PCT 0x0600 >> diff --git a/drivers/hid/hid-switch2.c b/drivers/hid/hid-switch2.c >> new file mode 100644 >> index 0000000000000..a06d181e76cc7 >> --- /dev/null >> +++ b/drivers/hid/hid-switch2.c >> @@ -0,0 +1,971 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * HID driver for Nintendo Switch controllers >> + * >> + * Copyright (c) 2025 Valve Software >> + * >> + * This driver is based on the following work: >> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1= e2279b64 >> + * https://github.com/ndeadly/switch2_controller_research >> + * hid-nintendo driver >> + */ >> + >> +#include "hid-switch2.h" >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "usbhid/usbhid.h" >> + >> +#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 >> + >> +enum gc_rumble { >> + GC_RUMBLE_OFF =3D 0, >> + GC_RUMBLE_ON =3D 1, >> + GC_RUMBLE_STOP =3D 2, >> +}; >> + >> +/* >> + * The highest rumble level for "HD Rumble" is strong enough to poten= tially damage the controller, >> + * and also leaves your hands feeling like melted jelly, so we set a = semi-abitrary scaling factor >> + * to artificially limit the maximum for safety and comfort. It is cu= rrently unknown if the Switch >> + * 2 itself does something similar, but it's quite likely. >> + * >> + * This value must be between 0 and 1024, otherwise the math below wi= ll overflow. >> + */ >> +#define RUMBLE_MAX 450u >> + >> +/* >> + * Semi-arbitrary values used to simulate the "rumble" sensation of a= n eccentric rotating >> + * mass type haptic motor on the Switch 2 controllers' linear resonan= t actuator type haptics. >> + * >> + * The units used are unknown, but the values must be between 0 and 1= 023. >> + */ >> +#define RUMBLE_HI_FREQ 0x187 >> +#define RUMBLE_LO_FREQ 0x112 >> + >> +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 left_joycon_button_ma= ppings[] =3D { >> + { BTN_TL, 0, NS2_BTNL_L, }, >> + { BTN_TL2, 0, NS2_BTNL_ZL, }, >> + { BTN_SELECT, 0, NS2_BTNL_MINUS, }, >> + { BTN_THUMBL, 0, NS2_BTNL_LS, }, >> + { BTN_GRIPL, 1, NS2_BTN_JCL_GL, }, >> + { KEY_RECORD, 1, NS2_BTN_JCL_CAPTURE, }, >> + { /* sentinel */ }, >> +}; >> + >> +static const struct switch2_ctlr_button_mapping right_joycon_button_m= appings[] =3D { >> + { 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_GRIPR, 1, NS2_BTN_JCR_GR, }, >> + { BTN_MODE, 1, NS2_BTN_JCR_HOME, }, >> + { /* sentinel */ }, >> +}; >> + >> +static const struct switch2_ctlr_button_mapping procon_mappings[] =3D= { >> + { 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 gccon_mappings[] =3D = { >> + { 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_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[] =3D { >> + /* >> + * 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[] =3D { 0x01, 0x00, 0x00, 0x00 = }; >> +#ifdef CONFIG_SWITCH2_FF >> +static const uint8_t switch2_zero_data[] =3D { 0x00, 0x00, 0x00, 0x00= }; >> +#endif >> + >> +static const uint8_t switch2_player_pattern[] =3D { 0x1, 0x3, 0x7, 0x= f, 0x9, 0x5, 0xd, 0x6 }; >> + >> +static const uint8_t switch2_feature_mask[] =3D { >> + NS2_FEATURE_BUTTONS | NS2_FEATURE_ANALOG | NS2_FEATURE_IMU | NS2_FEA= TURE_RUMBLE, >> + 0x00, 0x00, 0x00 >> +}; >> + >> +static DEFINE_IDA(switch2_player_id_allocator); >> + >> +#ifdef CONFIG_SWITCH2_FF >> +static void switch2_encode_rumble(struct switch2_hd_rumble *rumble, u= int8_t buffer[5]) >> +{ >> + buffer[0] =3D rumble->hi_freq; >> + buffer[1] =3D (rumble->hi_freq >> 8) | (rumble->hi_amp << 2); >> + buffer[2] =3D (rumble->hi_amp >> 6) | (rumble->lo_freq << 4); >> + buffer[3] =3D (rumble->lo_freq >> 4) | (rumble->lo_amp << 6); >> + buffer[4] =3D rumble->lo_amp >> 2; >> +} >> + >> +static int switch2_play_effect(struct input_dev *dev, void *data, str= uct ff_effect *effect) >> +{ >> + struct switch2_controller *ns2 =3D input_get_drvdata(dev); >> + >> + if (effect->type !=3D FF_RUMBLE) >> + return 0; >> + >> + guard(spinlock_irqsave)(&ns2->rumble_lock); >> + if (ns2->ctlr_type =3D=3D NS2_CTLR_TYPE_GC) { >> + ns2->rumble.sd.amplitude =3D max(effect->u.rumble.strong_magnitude,= >> + effect->u.rumble.weak_magnitude >> 1); >> + } else { >> + ns2->rumble.hd.hi_amp =3D effect->u.rumble.weak_magnitude * RUMBLE_= MAX >> 16; >> + ns2->rumble.hd.lo_amp =3D effect->u.rumble.strong_magnitude * RUMBL= E_MAX >> 16; >> + } >> + >> + schedule_delayed_work(&ns2->rumble_work, 0); >> + >> + return 0; >> +} >> + >> +static void switch2_rumble_work(struct work_struct *work) >> +{ >> + struct switch2_controller *ns2 =3D container_of(to_delayed_work(work= ), >> + struct switch2_controller, rumble_work); >> + unsigned long current_ms =3D jiffies_to_msecs(get_jiffies_64()); >> + uint8_t *buffer =3D kzalloc(64, GFP_KERNEL); >=20 > While it's technically impossible for this allocation to fail (since it= 's > certainly < PAGE_SIZE) we probably should still error check here anyhow= in > case that changes in the future. Fixed locally and will be fixed in v2. >=20 >> + unsigned long flags; >> + bool active; >> + int ret; >> + >> + spin_lock_irqsave(&ns2->rumble_lock, flags); >> + buffer[0x1] =3D 0x50 | ns2->rumble_seq; >> + if (ns2->ctlr_type =3D=3D NS2_CTLR_TYPE_GC) { >> + buffer[0] =3D 3; >> + if (ns2->rumble.sd.amplitude =3D=3D 0) { >> + buffer[2] =3D GC_RUMBLE_STOP; >> + ns2->rumble.sd.error =3D 0; >> + active =3D false; >> + } else { >> + if (ns2->rumble.sd.error < ns2->rumble.sd.amplitude) { >> + buffer[2] =3D GC_RUMBLE_ON; >> + ns2->rumble.sd.error +=3D U16_MAX - ns2->rumble.sd.amplitude; >> + } else { >> + buffer[2] =3D GC_RUMBLE_OFF; >> + ns2->rumble.sd.error -=3D ns2->rumble.sd.amplitude; >> + } >> + active =3D true; >> + } >> + } else { >> + buffer[0] =3D 1; >> + switch2_encode_rumble(&ns2->rumble.hd, &buffer[0x2]); >> + active =3D ns2->rumble.hd.hi_amp || ns2->rumble.hd.lo_amp; >> + if (ns2->ctlr_type =3D=3D NS2_CTLR_TYPE_PRO) { >> + /* >> + * The Pro Controller contains separate LRAs on each >> + * side that can be controlled individually. >> + */ >> + buffer[0] =3D 2; >> + buffer[0x11] =3D 0x50 | ns2->rumble_seq; >> + switch2_encode_rumble(&ns2->rumble.hd, &buffer[0x12]); >> + } >> + } >> + ns2->rumble_seq =3D (ns2->rumble_seq + 1) & 0xF; >> + >> + if (active) { >> + unsigned long interval =3D msecs_to_jiffies(4); >> + >> + if (!ns2->last_rumble_work) >> + ns2->last_rumble_work =3D current_ms; >> + else >> + ns2->last_rumble_work +=3D interval; >> + schedule_delayed_work(&ns2->rumble_work, >> + ns2->last_rumble_work + interval - current_ms); >> + } else { >> + ns2->last_rumble_work =3D 0; >> + } >> + spin_unlock_irqrestore(&ns2->rumble_lock, flags); >> + >> + if (!ns2->hdev) { >> + cancel_delayed_work(&ns2->rumble_work); >> + ret =3D -ENODEV; >=20 > I think this is backwards - usually with workqueues tied to a specific = device > in the kernel you don't want the workqueue in charge of checking whethe= r the > device is present or not. You just want the device to cancel the workqu= eue > unconditionally during driver shutdown, before doing any teardown that = could > put the device into a state where portions of its data aren't initializ= ed. I actually do cancel the work when tearing down the hdev. I'm not sure if= I added this for extra safety or if I added this due to debugging that I= didn't remove, but either way while this might be redundant, I guess it'= s also theoretically safer in case there's a bug where it gets reschedule= d somewhere. I'll take a closer look to see if I can figure out if it's e= ven doing anything. >=20 >> + } else { >> + ret =3D hid_hw_output_report(ns2->hdev, buffer, 64); >> + } >> + >> + kfree(buffer); >> + if (ret < 0) >> + hid_dbg(ns2->hdev, "Failed to send output report ret=3D%d\n", ret);= >> +} >> +#endif >> + >> +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)) =3D=3D 0) >> + return ns2; >> + } >> + ns2 =3D 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; >> +} >> +EXPORT_SYMBOL_GPL(switch2_get_controller); >> + >> +void switch2_controller_put(struct switch2_controller *ns2) >> +{ >> + mutex_lock(&ns2->lock); >> + if (ns2->input) { >> + input_unregister_device(ns2->input); >> + ns2->input =3D NULL; >> + } >> + ns2->init_step =3D 0; >> + if (ns2->hdev || ns2->cfg) { >> + mutex_unlock(&ns2->lock); >> + return; >> + } >> + mutex_unlock(&ns2->lock); >> + >> + mutex_lock(&switch2_controllers_lock); >> + list_del_init(&ns2->entry); >> + mutex_unlock(&switch2_controllers_lock); >> + kfree(ns2); >=20 > You're missing a teardown of the mutex here (mutex_destroy). >=20 > BUT - you should also consider maybe using devm_mutex_init() (and the m= any > other fun devm function variants, which make resources tied to devices)= for > this and other mutexes mutex instead. I mentioned this on Telegram, but the primary motivation I have for not u= sing devres more widely in these drivers is the discovery that if a drive= r gets rmmod-ed without the device itself being removed, that memory "lea= ks". While it's not a "real" leak, as it will be cleaned up if/when the d= evice itself gets removed, it does mean that there's unused memory that c= ould be freed more eagerly. I don't know if maintainers care about this s= emi-leak in general, but for e.g. hid drivers where the device they're bo= und to isn't necessarily going to be freed immediately following the hid = driver shutdown, I prefer intentional cleanup. This seems like an oversig= ht in how devres is designed and/or used, but since drivers being detache= d and reattached repeatedly is a pretty rare scenario I'm not sure how mu= ch it matters. USB is more common, however, as e.g. SDL now has code to d= etach the USB device from the kernel driver if it wants to use it directl= y. If that happens repeatedly, unused memory will pile up until the devic= e is actually removed. >=20 >> +} >> +EXPORT_SYMBOL_GPL(switch2_controller_put); >> + >> +static bool switch2_parse_stick_calibration(struct switch2_stick_cali= bration *calib, >> + const uint8_t *data) >> +{ >> + static const uint8_t UNCALIBRATED[9] =3D { >> + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF >> + }; >> + if (memcmp(UNCALIBRATED, data, sizeof(UNCALIBRATED)) =3D=3D 0) >> + return false; >> + >> + calib->x.neutral =3D data[0]; >> + calib->x.neutral |=3D (data[1] & 0x0F) << 8; >> + >> + calib->y.neutral =3D data[1] >> 4; >> + calib->y.neutral |=3D data[2] << 4; >> + >> + calib->x.positive =3D data[3]; >> + calib->x.positive |=3D (data[4] & 0x0F) << 8; >> + >> + calib->y.positive =3D data[4] >> 4; >> + calib->y.positive |=3D data[5] << 4; >> + >> + calib->x.negative =3D data[6]; >> + calib->x.negative |=3D (data[7] & 0x0F) << 8; >> + >> + calib->y.negative =3D data[7] >> 4; >> + calib->y.negative |=3D 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 !=3D NS2_FLASH_SIZE_SERIAL) >> + return; >=20 > What's the intent with these conditionals? If it's just checking to mak= e sure > that *data is large enough to hold this, I would assume we probably wan= t this > to be: >=20 > if (WARN_ON(size !=3D NS2_FLASH_SIZE_SERIAL)) > return; >=20 > Since a mismatched size seems like it would be a pretty exceptional (e.= g. > something went very wrong, or a programmer mistake) error we'd want to = know > about. (same for the other conditionals in this function). >=20 > (If this is actually something we expect to happen regularly, feel free= to > ignore this comment) We do not expect this to happen, generally speaking, but due to the afore= mentioned USB device detaching from userspace, I don't really want to mak= e any implicit assumptions about what we get being the same as what we'd = request. If it's something that doesn't match what we expect, just reject= it and carry on. >=20 >> + memcpy(ns2->serial, data, size); >> + break; >> + case NS2_FLASH_ADDR_FACTORY_PRIMARY_CALIB: >> + if (size !=3D NS2_FLASH_SIZE_FACTORY_AXIS_CALIB) >> + return; >> + ok =3D 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 !=3D NS2_FLASH_SIZE_FACTORY_AXIS_CALIB) >> + return; >> + ok =3D 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 presen= t\n"); >> + } >> + break; >> + case NS2_FLASH_ADDR_FACTORY_TRIGGER_CALIB: >> + if (size !=3D NS2_FLASH_SIZE_FACTORY_TRIGGER_CALIB) >> + return; >> + if (data[0] !=3D 0xFF && data[1] !=3D 0xFF) { >> + ns2->lt_zero =3D data[0]; >> + ns2->rt_zero =3D 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 !=3D NS2_FLASH_SIZE_USER_AXIS_CALIB) >> + return; >> + if (__le16_to_cpu(*(__le16 *)data) !=3D NS2_USER_CALIB_MAGIC) { >> + hid_dbg(ns2->hdev, "No user primary stick calibration present\n");= >> + break; >> + } >> + >> + ok =3D 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 !=3D NS2_FLASH_SIZE_USER_AXIS_CALIB) >> + return; >> + if (__le16_to_cpu(*(__le16 *)data) !=3D NS2_USER_CALIB_MAGIC) { >> + hid_dbg(ns2->hdev, "No user secondary stick calibration present\n"= ); >> + break; >> + } >> + >> + ok =3D 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; >> + } >> +} >> + >> +int switch2_receive_command(struct switch2_controller *ns2, >> + const uint8_t *message, size_t length) >> +{ >> + const struct switch2_cmd_header *header; >> + >> + if (length < 8) >> + return -EINVAL; >> + >> + print_hex_dump_debug("got cmd: ", DUMP_PREFIX_OFFSET, 16, 1, message= , length, false); >> + >> + guard(mutex)(&ns2->lock); >> + if (ns2->init_step < NS2_INIT_DONE) >> + switch2_init_controller(ns2); >> + >> + header =3D (const struct switch2_cmd_header *)message; >> + message =3D &message[8]; >> + switch (header->command) { >> + case NS2_CMD_FLASH: >> + if (header->subcommand =3D=3D NS2_SUBCMD_FLASH_READ) { >> + uint8_t read_size; >> + uint32_t read_address; >> + >> + if (length < 16) >> + return -EINVAL; >> + read_size =3D message[0]; >> + read_address =3D __le32_to_cpu(*(__le32 *)&message[4]); >> + if (length < read_size + 16) >> + return -EINVAL; >> + switch2_handle_flash_read(ns2, read_size, read_address, &message[8= ]); >> + } >> + break; >> + case NS2_CMD_FW_INFO: >> + if (header->subcommand =3D=3D NS2_SUBCMD_FW_INFO_GET) { >> + if (length < sizeof(ns2->version)) >> + return -EINVAL; >> + memcpy(&ns2->version, message, sizeof(ns2->version)); >> + ns2->ctlr_type =3D ns2->version.ctlr_type; >> + } >> + break; >> + default: >> + break; >> + } >> + >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(switch2_receive_command); >> + >> +static int switch2_features_enable(struct switch2_controller *ns2, in= t features) >> +{ >> + __le32 feature_bits =3D __cpu_to_le32(features); >> + >> + if (!ns2->cfg) >> + return -ENOTCONN; >> + return ns2->cfg->send_command(NS2_CMD_FEATSEL, NS2_SUBCMD_FEATSEL_EN= ABLE, >> + &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] =3D { size, 0x7e }; >> + >> + if (!ns2->cfg) >> + return -ENOTCONN; >> + *(__le32 *)&message[4] =3D __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, uint= 32_t player_id) >> +{ >> + player_id %=3D NS2_MAX_PLAYER_ID; >> + uint8_t message[8] =3D { switch2_player_pattern[player_id] }; >> + >> + if (!ns2->cfg) >> + return -ENOTCONN; >> + return ns2->cfg->send_command(NS2_CMD_LED, NS2_SUBCMD_LED_PATTERN, >> + &message, sizeof(message), >> + ns2->cfg); >> +} >> + >> +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 =3D 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 =3D ns2->hdev; >> +#ifdef CONFIG_SWITCH2_FF >> + int ret; >> +#endif >> + >> + rcu_read_lock(); >> + input =3D rcu_dereference(ns2->input); >> + rcu_read_unlock(); >> + >> + if (input) >> + return 0; >> + >> + input =3D devm_input_allocate_device(&hdev->dev); >> + if (!input) >> + return -ENOMEM; >=20 > oh hey you are using devm in some places already, nice :) >=20 >> + >> + input_set_drvdata(input, ns2); >> + input->dev.parent =3D &hdev->dev; >> + input->id.bustype =3D hdev->bus; >> + input->id.vendor =3D hdev->vendor; >> + input->id.product =3D hdev->product; >> + input->id.version =3D hdev->version; >> + input->uniq =3D ns2->serial; >> + input->name =3D hdev->name; >> + input->phys =3D 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); >> + 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, left_joycon_button_mappings); >> + break; >> + case NS2_CTLR_TYPE_JCR: >> + 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); >> + switch2_config_buttons(input, 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, 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, procon_mappings); >> + break; >> + default: >> + input_free_device(input); >> + return -EINVAL; >> + } >> + >> +#ifdef CONFIG_SWITCH2_FF >> + input_set_capability(input, EV_FF, FF_RUMBLE); >> + ret =3D input_ff_create_memless(input, NULL, switch2_play_effect); >> + if (ret) { >> + input_free_device(input); >> + return ret; >> + } >> +#endif >> + >> + rcu_assign_pointer(ns2->input, input); >> + >> + hid_info(ns2->hdev, "Firmware version %u.%u.%u (%i)\n", ns2->version= =2Emajor, >> + ns2->version.minor, ns2->version.patch, ns2->version.ctlr_type); >> + if (ns2->version.dsp_type >=3D 0) >> + hid_info(ns2->hdev, "DSP version %u.%u.%u\n", ns2->version.dsp_majo= r, >> + ns2->version.dsp_minor, ns2->version.dsp_patch); >> + return input_register_device(input); >> +} >> + >> +int switch2_init_controller(struct switch2_controller *ns2) >> +{ >> + if (ns2->init_step =3D=3D NS2_INIT_DONE) >> + return 0; >> + >> + if (!ns2->cfg) >> + return -ENOTCONN; >> + >> + while (ns2->init_step < NS2_INIT_DONE) { >> + ns2->init_step++; >> + /* TODO verify that the step completed successfully */ >=20 > Are we sure we need this TODO comment? it seems like you do check (or p= ass > that responsibility off to the caller of this function, which is fine) This comment is again here just in case the driver gets detached by users= pace and then reattached later. Within a normal, constrained flow it shou= ldn't go wrong, but again I don't want to make any assumptions that nothi= ng goes wrong. It's left as a TODO for now just as a reminder to shake ou= t any assumptions later. As-is, if any singular step fails we'll be left = with a weirdly partially-initialized device too, and we'd presumably want= to retry the step instead of just continuing on. >=20 >> + 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); >> + break; >> + case NS2_INIT_READ_FACTORY_PRIMARY_CALIB: >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_PRIMARY_CALI= B, >> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB); >> + case NS2_INIT_READ_FACTORY_SECONDARY_CALIB: >> + if (switch2_ctlr_is_joycon(ns2->ctlr_type)) >> + break; >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_SECONDARY_CA= LIB, >> + NS2_FLASH_SIZE_FACTORY_AXIS_CALIB); >> + case NS2_INIT_READ_FACTORY_TRIGGER_CALIB: >> + if (ns2->ctlr_type !=3D NS2_CTLR_TYPE_GC) >> + break; >> + return switch2_read_flash(ns2, NS2_FLASH_ADDR_FACTORY_TRIGGER_CALI= B, >> + 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)) >> + break; >> + 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); >> + break; >> + case NS2_INIT_ENABLE_FEATURES: >> + return switch2_features_enable(ns2, NS2_FEATURE_BUTTONS | >> + NS2_FEATURE_ANALOG | NS2_FEATURE_RUMBLE); >> + break; >> +#ifdef CONFIG_SWITCH2_FF >> + case NS2_INIT_ENABLE_RUMBLE: >> + return ns2->cfg->send_command(NS2_CMD_NFC, 1, >> + switch2_zero_data, sizeof(switch2_zero_data), >> + ns2->cfg); >> + break; >> +#endif >> + case NS2_INIT_GRIP_BUTTONS: >> + if (!switch2_ctlr_is_joycon(ns2->ctlr_type)) >> + break; >> + return ns2->cfg->send_command(NS2_CMD_GRIP, NS2_SUBCMD_GRIP_ENABLE= _BUTTONS, >> + switch2_one_data, sizeof(switch2_one_data), >> + ns2->cfg); >> + break; >> + 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_DONE: >> + if (ns2->hdev) >> + return switch2_init_input(ns2); >> + break; >> + default: >> + WARN_ON_ONCE(1); >> + } >> + } >> + return 0; >> +} >> +EXPORT_SYMBOL_GPL(switch2_init_controller); >> + >> +static void switch2_report_buttons(struct input_dev *input, const uin= t8_t *bytes, >> + const struct switch2_ctlr_button_mapping button_mappings[]) >> +{ >> + const struct switch2_ctlr_button_mapping *button; >> + >> + for (button =3D 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 switc= h2_axis_calibration *calib, >> + int axis, int value, bool negate) >> +{ >> + if (calib && calib->neutral && calib->negative && calib->positive) {= >> + value -=3D calib->neutral; >> + value *=3D NS2_AXIS_MAX + 1; >> + if (value < 0) >> + value /=3D calib->negative; >> + else >> + value /=3D calib->positive; >> + } else { >> + value =3D (value - 2048) * 16; >> + } >> + >> + if (negate) >> + value =3D -value; >> + input_report_abs(input, axis, >> + clamp(value, NS2_AXIS_MIN, NS2_AXIS_MAX)); >> +} >> + >> +static void switch2_report_stick(struct input_dev *input, struct swit= ch2_stick_calibration *calib, >> + int x, int y, const uint8_t *data) >> +{ >> + switch2_report_axis(input, &calib->x, x, data[0] | ((data[1] & 0x0F)= << 8), false); >> + switch2_report_axis(input, &calib->y, y, (data[1] >> 4) | (data[2] <= < 4), true); >> +} >> + >> +static void switch2_report_trigger(struct input_dev *input, uint8_t z= ero, int abs, uint8_t data) >> +{ >> + int value =3D (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 =3D hid_get_drvdata(hdev); >> + struct input_dev *input; >> + >> + if (report->type !=3D HID_INPUT_REPORT) >> + return 0; >> + >> + if (size < 15) >> + return -EINVAL; >> + >> + rcu_read_lock(); >> + input =3D rcu_dereference(ns2->input); >> + rcu_read_unlock(); >> + >> + if (!input) >> + return 0; >> + >> + switch (report->id) { >> + case NS2_REPORT_UNIFIED: >> + /* >> + * TODO >> + * This won't be sent unless the report type gets changed via comma= nd >> + * 03-0A, but we should support it at some point regardless. >> + */ >> + break; >> + case NS2_REPORT_JCL: >> + input_report_abs(input, ABS_HAT0X, >> + !!(raw_data[3] & NS2_BTNL_RIGHT) - >> + !!(raw_data[3] & NS2_BTNL_LEFT)); >> + input_report_abs(input, ABS_HAT0Y, >> + !!(raw_data[3] & NS2_BTNL_DOWN) - >> + !!(raw_data[3] & NS2_BTNL_UP)); >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, ABS_Y, &ra= w_data[6]); >> + switch2_report_buttons(input, &raw_data[3], left_joycon_button_mapp= ings); >> + break; >> + case NS2_REPORT_JCR: >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_RX, ABS_RY, &= raw_data[6]); >> + switch2_report_buttons(input, &raw_data[3], right_joycon_button_map= pings); >> + 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], gccon_mappings); >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, ABS_Y, &ra= w_data[6]); >> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, ABS_RY, &= 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], procon_mappings); >> + switch2_report_stick(input, &ns2->stick_calib[0], ABS_X, ABS_Y, &ra= w_data[6]); >> + switch2_report_stick(input, &ns2->stick_calib[1], ABS_RX, ABS_RY, &= raw_data[9]); >> + break; >> + default: >> + return -EINVAL; >> + } >> + >> + input_sync(input); >> + return 0; >> +} >> + >> +static int switch2_probe(struct hid_device *hdev, const struct hid_de= vice_id *id) >> +{ >> + struct switch2_controller *ns2; >> + struct usb_device *udev; >> + char phys[64]; >> + int ret; >> + >> + if (!hid_is_usb(hdev)) >> + return -ENODEV; >> + >> + udev =3D hid_to_usb_dev(hdev); >> + if (usb_make_path(udev, phys, sizeof(phys)) < 0) >> + return -EINVAL; >> + >> + ret =3D hid_parse(hdev); >> + if (ret) { >> + hid_err(hdev, "parse failed %d\n", ret); >> + return ret; >> + } >> + >> + ret =3D hid_hw_start(hdev, HID_CONNECT_HIDRAW); >> + if (ret) { >> + hid_err(hdev, "hw_start failed %d\n", ret); >> + return ret; >> + } >> + >> + ret =3D hid_hw_open(hdev); >> + if (ret) { >> + hid_err(hdev, "hw_open failed %d\n", ret); >> + goto err_stop; >> + } >> + >> + ns2 =3D switch2_get_controller(phys); >> + if (!ns2) { >> + ret =3D -ENOMEM; >> + goto err_close; >> + } >> + >> + guard(mutex)(&ns2->lock); >> + ns2->hdev =3D hdev; >> + >> + ns2->player_id =3D U32_MAX; >> + ret =3D ida_alloc(&switch2_player_id_allocator, GFP_KERNEL); >> + if (ret < 0) >> + hid_warn(hdev, "Failed to allocate player ID, skipping; ret=3D%d\n"= , ret); >> + else >> + ns2->player_id =3D ret; >> +#ifdef CONFIG_SWITCH2_FF >> + if (ns2->ctlr_type !=3D NS2_CTLR_TYPE_GC) { >> + ns2->rumble.hd.hi_freq =3D RUMBLE_HI_FREQ; >> + ns2->rumble.hd.lo_freq =3D RUMBLE_LO_FREQ; >> + } >> + spin_lock_init(&ns2->rumble_lock); >> + INIT_DELAYED_WORK(&ns2->rumble_work, switch2_rumble_work); >> +#endif >> + 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 =3D hid_get_drvdata(hdev); >> +#ifdef CONFIG_SWITCH2_FF >> + unsigned long flags; >> +#endif >> + >> +#ifdef CONFIG_SWITCH2_FF >> + spin_lock_irqsave(&ns2->rumble_lock, flags); >> + cancel_delayed_work_sync(&ns2->rumble_work); >> + spin_unlock_irqrestore(&ns2->rumble_lock, flags); >> +#endif >> + mutex_lock(&ns2->lock); >> + ns2->hdev =3D NULL; >> + mutex_unlock(&ns2->lock); >> + hid_hw_close(hdev); >> + ida_free(&switch2_player_id_allocator, ns2->player_id); >> + switch2_controller_put(ns2); >> + hid_hw_stop(hdev); >> +} >> + >> +static const struct hid_device_id switch2_devices[] =3D { >> + { 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, switch2_devices); >> + >> +static struct hid_driver switch2_hid_driver =3D { >> + .name =3D "switch2", >> + .id_table =3D switch2_devices, >> + .probe =3D switch2_probe, >> + .remove =3D switch2_remove, >> + .raw_event =3D switch2_event, >> +}; >> + >> +static int __init switch2_init(void) >> +{ >> + return hid_register_driver(&switch2_hid_driver); >> +} >> + >> +static void __exit switch2_exit(void) >> +{ >> + hid_unregister_driver(&switch2_hid_driver); >> + ida_destroy(&switch2_player_id_allocator); >> +} >> + >> +module_init(switch2_init); >> +module_exit(switch2_exit); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Vicki Pfau "); >> +MODULE_DESCRIPTION("Driver for Nintendo Switch 2 Controllers"); >> diff --git a/drivers/hid/hid-switch2.h b/drivers/hid/hid-switch2.h >> new file mode 100644 >> index 0000000000000..4b7b82da0c37f >> --- /dev/null >> +++ b/drivers/hid/hid-switch2.h >> @@ -0,0 +1,242 @@ >> +/* SPDX-License-Identifier: GPL-2.0+ */ >> +/* >> + * HID driver for Nintendo Switch controllers >> + * >> + * Copyright (c) 2025 Valve Software >> + * >> + * This driver is based on the following work: >> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1= e2279b64 >> + * https://github.com/ndeadly/switch2_controller_research >> + */ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include "hid-ids.h" >> + >> +#define NS2_FLAG_OK BIT(0) >> +#define NS2_FLAG_NACK BIT(2) >> + >> +#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_cmd { >> + NS2_CMD_NFC =3D 0x01, >> + NS2_CMD_FLASH =3D 0x02, >> + NS2_CMD_INIT =3D 0x03, >> + NS2_CMD_GRIP =3D 0x08, >> + NS2_CMD_LED =3D 0x09, >> + NS2_CMD_VIBRATE =3D 0x0a, >> + NS2_CMD_BATTERY =3D 0x0b, >> + NS2_CMD_FEATSEL =3D 0x0c, >> + NS2_CMD_FW_UPD =3D 0x0d, >> + NS2_CMD_FW_INFO =3D 0x10, >> + NS2_CMD_BT_PAIR =3D 0x15, >> +}; >> + >> +enum switch2_direction { >> + NS2_DIR_IN =3D 0x00, >> + NS2_DIR_OUT =3D 0x90, >> +}; >> + >> +enum switch2_transport { >> + NS2_TRANS_USB =3D 0x00, >> + NS2_TRANS_BT =3D 0x01, >> +}; >> + >> +enum switch2_subcmd_flash { >> + NS2_SUBCMD_FLASH_READ_BLOCK =3D 0x01, >> + NS2_SUBCMD_FLASH_WRITE_BLOCK =3D 0x02, >> + NS2_SUBCMD_FLASH_ERASE_BLOCK =3D 0x03, >> + NS2_SUBCMD_FLASH_READ =3D 0x04, >> + NS2_SUBCMD_FLASH_WRITE =3D 0x05, >> +}; >> + >> +enum switch2_subcmd_init { >> + NS2_SUBCMD_INIT_SELECT_REPORT =3D 0xa, >> + NS2_SUBCMD_INIT_USB =3D 0xd, >> +}; >> + >> +enum switch2_subcmd_feature_select { >> + NS2_SUBCMD_FEATSEL_GET_INFO =3D 0x1, >> + NS2_SUBCMD_FEATSEL_SET_MASK =3D 0x2, >> + NS2_SUBCMD_FEATSEL_CLEAR_MASK =3D 0x3, >> + NS2_SUBCMD_FEATSEL_ENABLE =3D 0x4, >> + NS2_SUBCMD_FEATSEL_DISABLE =3D 0x5, >> +}; >> + >> +enum switch2_subcmd_grip { >> + NS2_SUBCMD_GRIP_GET_INFO =3D 0x1, >> + NS2_SUBCMD_GRIP_ENABLE_BUTTONS =3D 0x2, >> + NS2_SUBCMD_GRIP_GET_INFO_EXT =3D 0x3, >> +}; >> + >> +enum switch2_subcmd_led { >> + NS2_SUBCMD_LED_P1 =3D 0x1, >> + NS2_SUBCMD_LED_P2 =3D 0x2, >> + NS2_SUBCMD_LED_P3 =3D 0x3, >> + NS2_SUBCMD_LED_P4 =3D 0x4, >> + NS2_SUBCMD_LED_ALL_ON =3D 0x5, >> + NS2_SUBCMD_LED_ALL_OFF =3D 0x6, >> + NS2_SUBCMD_LED_PATTERN =3D 0x7, >> + NS2_SUBCMD_LED_BLINK =3D 0x8, >> +}; >> + >> +enum switch2_subcmd_fw_info { >> + NS2_SUBCMD_FW_INFO_GET =3D 0x1, >> +}; >> + >> +enum switch2_ctlr_type { >> + NS2_CTLR_TYPE_JCL =3D 0x00, >> + NS2_CTLR_TYPE_JCR =3D 0x01, >> + NS2_CTLR_TYPE_PRO =3D 0x02, >> + NS2_CTLR_TYPE_GC =3D 0x03, >> +}; >> + >> +enum switch2_report_id { >> + NS2_REPORT_UNIFIED =3D 0x05, >> + NS2_REPORT_JCL =3D 0x07, >> + NS2_REPORT_JCR =3D 0x08, >> + NS2_REPORT_PRO =3D 0x09, >> + NS2_REPORT_GC =3D 0x0a, >> +}; >> + >> +enum switch2_init_step { >> + NS2_INIT_STARTING, >> + NS2_INIT_READ_SERIAL, >> + 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_GET_FIRMWARE_INFO, >> +#ifdef CONFIG_SWITCH2_FF >> + NS2_INIT_ENABLE_RUMBLE, >> +#endif >> + NS2_INIT_GRIP_BUTTONS, >> + NS2_INIT_SET_PLAYER_LEDS, >> + NS2_INIT_INPUT, >> + NS2_INIT_DONE, >> +}; >> + >> +struct switch2_cmd_header { >> + uint8_t command; >> + uint8_t direciton; >> + uint8_t transport; >> + uint8_t subcommand; >> + uint8_t unk1; >> + uint8_t length; >> + uint16_t unk2; >> +}; >> +static_assert(sizeof(struct switch2_cmd_header) =3D=3D 8); >> + >> +struct switch2_controller; >> +struct switch2_cfg_intf { >> + struct switch2_controller *parent; >> + >> + int (*send_command)(enum switch2_cmd command, uint8_t subcommand, >> + const void *message, size_t length, >> + struct switch2_cfg_intf *intf); >> +}; >> + >> +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_hd_rumble { >> + uint16_t hi_freq : 10; >> + uint16_t hi_amp : 10; >> + uint16_t lo_freq : 10; >> + uint16_t lo_amp : 10; >> +}; >=20 > I may or may not be wrong here, but I think this (and any other structu= res > you're passing as-is to or from the device) should be __packed to make = sure > the compiler doesn't try to be clever with the data layout here. >=20 >> + >> +struct switch2_erm_rumble { >> + uint16_t error; >> + uint16_t amplitude; >> +}; >> + >> +struct switch2_controller { >> + struct hid_device *hdev; >> + struct switch2_cfg_intf *cfg; >> + >> + 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; >> + >> +#ifdef CONFIG_SWITCH2_FF >> + spinlock_t rumble_lock; >> + uint8_t rumble_seq; >> + union { >> + struct switch2_hd_rumble hd; >> + struct switch2_erm_rumble sd; >> + } rumble; >> + unsigned long last_rumble_work; >> + struct delayed_work rumble_work; >> +#endif >> +}; >> + >> +struct switch2_controller *switch2_get_controller(const char *phys); >> +void switch2_controller_put(struct switch2_controller *controller); >> + >> +int switch2_receive_command(struct switch2_controller *controller, >> + const uint8_t *message, size_t length); >> + >> +int switch2_init_controller(struct switch2_controller *controller); >> + >> +static inline bool switch2_ctlr_is_joycon(enum switch2_ctlr_type type= ) >> +{ >> + return type =3D=3D NS2_CTLR_TYPE_JCL || type =3D=3D NS2_CTLR_TYPE_JC= R; >> +} >> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/K= config >> index 7755e5b454d2c..73b0e15205aee 100644 >> --- a/drivers/input/joystick/Kconfig >> +++ b/drivers/input/joystick/Kconfig >> @@ -422,4 +422,12 @@ config JOYSTICK_SEESAW >> To compile this driver as a module, choose M here: the module will= be >> called adafruit-seesaw. >> =20 >> +config JOYSTICK_SWITCH2_USB >> + tristate "Wired Nintendo Switch 2 controller support" >> + depends on HID_SWITCH2 >> + select USB >> + help >> + To compile this driver as a module, choose M here: the >> + module will be called switch2-usb. >> + >> endif >> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/= Makefile >> index 9976f596a9208..46feb7ef60ba0 100644 >> --- a/drivers/input/joystick/Makefile >> +++ b/drivers/input/joystick/Makefile >> @@ -34,6 +34,7 @@ obj-$(CONFIG_JOYSTICK_SIDEWINDER) +=3D sidewinder.o >> obj-$(CONFIG_JOYSTICK_SPACEBALL) +=3D spaceball.o >> obj-$(CONFIG_JOYSTICK_SPACEORB) +=3D spaceorb.o >> obj-$(CONFIG_JOYSTICK_STINGER) +=3D stinger.o >> +obj-$(CONFIG_JOYSTICK_SWITCH2_USB) +=3D switch2-usb.o >> obj-$(CONFIG_JOYSTICK_TMDC) +=3D tmdc.o >> obj-$(CONFIG_JOYSTICK_TURBOGRAFX) +=3D turbografx.o >> obj-$(CONFIG_JOYSTICK_TWIDJOY) +=3D twidjoy.o >> diff --git a/drivers/input/joystick/switch2-usb.c b/drivers/input/joys= tick/switch2-usb.c >> new file mode 100644 >> index 0000000000000..2c8b09f4b98e9 >> --- /dev/null >> +++ b/drivers/input/joystick/switch2-usb.c >> @@ -0,0 +1,355 @@ >> +// SPDX-License-Identifier: GPL-2.0+ >> +/* >> + * HID driver for Nintendo Switch controllers >> + * >> + * Copyright (c) 2025 Valve Software >> + * >> + * This driver is based on the following work: >> + * https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1= e2279b64 >> + * https://github.com/ndeadly/switch2_controller_research >> + */ >> + >> +#include "../../hid/hid-switch2.h" >> +#include >> +#include >> + >> +#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 =3D urb->context; >> + int i; >> + bool schedule =3D false; >> + unsigned long flags; >> + >> + switch (urb->status) { >> + case 0: >> + schedule =3D true; >> + break; >> + case -ECONNRESET: >> + case -ENOENT: >> + case -ESHUTDOWN: >> + return; >> + default: >> + dev_dbg(&ns2_usb->udev->dev, "unknown urb status: %d\n", >> + urb->status); >> + break; >> + } >> + >> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags); >> + for (i =3D 0; i < NS2_IN_URBS; i++) { >> + int err; >> + struct switch2_urb *ns2_urb; >> + >> + if (ns2_usb->bulk_in[i].urb =3D=3D urb) { >> + ns2_usb->message_in =3D i; >> + continue; >> + } >> + >> + if (ns2_usb->bulk_in[i].active) >> + continue; >> + >> + ns2_urb =3D &ns2_usb->bulk_in[i]; >> + usb_anchor_urb(ns2_urb->urb, &ns2_usb->bulk_out_anchor); >> + err =3D 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", er= r); >> + } else { >> + ns2_urb->active =3D 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 =3D urb->context; >> + int i; >> + >> + guard(spinlock_irqsave)(&ns2_usb->bulk_out_lock); >> + >> + switch (urb->status) { >> + case 0: >> + break; >> + case -ECONNRESET: >> + case -ENOENT: >> + case -ESHUTDOWN: >> + return; >> + default: >> + dev_dbg(&ns2_usb->udev->dev, "unknown urb status: %d\n", urb->statu= s); >> + return; >> + } >> + >> + for (i =3D 0; i < NS2_OUT_URBS; i++) { >> + if (ns2_usb->bulk_out[i].urb !=3D urb) >> + continue; >> + >> + ns2_usb->bulk_out[i].active =3D false; >> + break; >> + } >> +} >> + >> +static int switch2_usb_send_cmd(enum switch2_cmd command, uint8_t sub= command, >> + const void *message, size_t size, struct switch2_cfg_intf *cfg) >> +{ >> + struct switch2_usb *ns2_usb =3D (struct switch2_usb *)cfg; >> + struct switch2_urb *urb =3D NULL; >> + int i; >> + int ret; >> + >> + struct switch2_cmd_header header =3D { >> + command, NS2_DIR_OUT | NS2_FLAG_OK, NS2_TRANS_USB, subcommand, 0, s= ize >> + }; >> + >> + if (size > 56) { >> + WARN_ON(1); >=20 > you can just do >=20 > if (WARN_ON(size > 56)) > /* etc. */ >=20 > here Fixed locally, will be in v2. >=20 >> + return -EINVAL; >> + } >> + >> + for (i =3D 0; i < NS2_OUT_URBS; i++) { >> + if (ns2_usb->bulk_out[i].active) >> + continue; >> + >> + urb =3D &ns2_usb->bulk_out[i]; >> + urb->active =3D true; >> + break; >> + } >> + 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 =3D 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 =3D usb_submit_urb(urb->urb, GFP_ATOMIC); >> + if (ret) { >> + dev_warn(&ns2_usb->udev->dev, "failed to submit urb: %i", ret); >> + urb->active =3D 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 =3D container_of(work, struct switch2_us= b, message_in_work); >> + struct switch2_urb *urb; >> + int err; >> + unsigned long flags; >> + >> + spin_lock_irqsave(&ns2_usb->bulk_in_lock, flags); >> + urb =3D &ns2_usb->bulk_in[ns2_usb->message_in]; >> + spin_unlock_irqrestore(&ns2_usb->bulk_in_lock, flags); >> + >> + err =3D switch2_receive_command(ns2_usb->cfg.parent, urb->urb->trans= fer_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 =3D 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_controller *ns2; >> + 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 =3D interface_to_usbdev(intf); >> + if (usb_make_path(udev, phys, sizeof(phys)) < 0) >> + return -EINVAL; >> + >> + ret =3D usb_find_common_endpoints(intf->cur_altsetting, &bulk_in, &b= ulk_out, NULL, NULL); >> + if (ret) { >> + dev_err(&intf->dev, "failed to find bulk EPs\n"); >> + return ret; >> + } >> + >> + ns2_usb =3D devm_kzalloc(&intf->dev, sizeof(*ns2_usb), GFP_KERNEL); >> + if (!ns2_usb) >> + return -ENOMEM; >> + >> + ns2_usb->udev =3D udev; >> + for (i =3D 0; i < NS2_IN_URBS; i++) { >> + ns2_usb->bulk_in[i].urb =3D usb_alloc_urb(0, GFP_KERNEL); >> + if (!ns2_usb->bulk_in[i].urb) { >> + ret =3D -ENOMEM; >> + goto err_free_in; >> + } >> + >> + ns2_usb->bulk_in[i].data =3D 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 =3D -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 |=3D URB_NO_TRANSFER_DMA_MA= P; >> + } >> + >> + for (i =3D 0; i < NS2_OUT_URBS; i++) { >> + ns2_usb->bulk_out[i].urb =3D usb_alloc_urb(0, GFP_KERNEL); >> + if (!ns2_usb->bulk_out[i].urb) { >> + ret =3D -ENOMEM; >> + goto err_free_out; >> + } >> + >> + ns2_usb->bulk_out[i].data =3D usb_alloc_coherent(udev, NS2_BULK_SIZ= E, GFP_KERNEL, >> + &ns2_usb->bulk_out[i].urb->transfer_dma); >> + if (!ns2_usb->bulk_out[i].data) { >> + ret =3D -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_us= b); >> + ns2_usb->bulk_out[i].urb->transfer_flags |=3D URB_NO_TRANSFER_DMA_M= AP; >> + } >> + >> + ns2_usb->bulk_in[0].active =3D true; >> + ret =3D usb_submit_urb(ns2_usb->bulk_in[0].urb, GFP_ATOMIC); >> + if (ret < 0) >> + goto err_free_out; >> + >> + ns2 =3D switch2_get_controller(phys); >> + if (IS_ERR(ns2)) { >> + ret =3D PTR_ERR(ns2); >> + goto err_kill_urb; >> + } >> + >> + ns2_usb->cfg.parent =3D ns2; >> + ns2_usb->cfg.send_command =3D switch2_usb_send_cmd; >> + >> + 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); >> + >> + guard(mutex)(&ns2->lock); >> + ns2->cfg =3D (struct switch2_cfg_intf *) ns2_usb; >> + >> + if (ns2->hdev) >> + return switch2_init_controller(ns2); >> + >> + return 0; >> + >> +err_kill_urb: >> + usb_kill_urb(ns2_usb->bulk_in[0].urb); >> +err_free_out: >> + for (i =3D 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 =3D 0; i < NS2_IN_URBS; i++) { >> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i]= =2Edata, >> + 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 =3D usb_get_intfdata(intf); >> + struct switch2_controller *ns2 =3D ns2_usb->cfg.parent; >> + int i; >> + >> + usb_kill_anchored_urbs(&ns2_usb->bulk_out_anchor); >> + for (i =3D 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); >> + } >> + >> + usb_kill_anchored_urbs(&ns2_usb->bulk_in_anchor); >> + for (i =3D 0; i < NS2_IN_URBS; i++) { >> + usb_free_coherent(ns2_usb->udev, NS2_BULK_SIZE, ns2_usb->bulk_in[i]= =2Edata, >> + ns2_usb->bulk_in[i].urb->transfer_dma); >> + usb_free_urb(ns2_usb->bulk_in[i].urb); >> + } >> + >> + mutex_lock(&ns2->lock); >> + ns2->cfg =3D NULL; >> + mutex_unlock(&ns2->lock); >> + switch2_controller_put(ns2); >> +} >> + >> +#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[] =3D { >> + { 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 =3D { >> + .name =3D "switch2", >> + .id_table =3D switch2_usb_devices, >> + .probe =3D switch2_usb_probe, >> + .disconnect =3D switch2_usb_disconnect, >> +}; >> +module_usb_driver(switch2_usb); >> + >> +MODULE_LICENSE("GPL"); >> +MODULE_AUTHOR("Vicki Pfau "); >> +MODULE_DESCRIPTION("Driver for Nintendo Switch 2 Controllers"); >=20 Thanks for taking a look at this, Vicki