* [PATCH] HID: remove duplicate hid_warn_ratelimited definition
From: 刘 凯 @ 2026-05-07 8:41 UTC (permalink / raw)
To: jikos@kernel.org, bentiss@kernel.org
Cc: vi@endrift.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
From 565c81e1b2e1aaad675d9844428af7b35f4328c6 Mon Sep 17 00:00:00 2001
From: Liu Kai <lukace97@outlook.com>
Date: Thu, 7 May 2026 16:32:04 +0800
Subject: [PATCH] HID: remove duplicate hid_warn_ratelimited definition
The hid_warn_ratelimited macro is defined twice in include/linux/hid.h:
at line 1317 (added by commit 4051ead99888) and at line 1339 (added by
commit 1d64624243af).
The second definition is correctly grouped with other ratelimited macros.
Remove the duplicate definition at line 1317.
Fixes: 1d64624243af ("HID: core: Add printk_ratelimited variants to hid_warn() etc")
Signed-off-by: Liu Kai <lukace97@outlook.com>
---
include/linux/hid.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 442a80d79e89..e1877a9a58e2 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1314,8 +1314,6 @@ void hid_quirks_exit(__u16 bus);
dev_notice(&(hid)->dev, fmt, ##__VA_ARGS__)
#define hid_warn(hid, fmt, ...) \
dev_warn(&(hid)->dev, fmt, ##__VA_ARGS__)
-#define hid_warn_ratelimited(hid, fmt, ...) \
- dev_warn_ratelimited(&(hid)->dev, fmt, ##__VA_ARGS__)
#define hid_info(hid, fmt, ...) \
dev_info(&(hid)->dev, fmt, ##__VA_ARGS__)
#define hid_dbg(hid, fmt, ...) \
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 1/3] HID: asus: export asus_hid_fnlock_notify() for direct fn-lock control
From: Denis Benato @ 2026-05-06 22:17 UTC (permalink / raw)
To: Marcus Grenängen, platform-driver-x86
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary
In-Reply-To: <20260506193326.5862-2-marcus@grenangen.se>
On 5/6/26 21:33, Marcus Grenängen wrote:
> Some ASUS platforms cannot control fn-lock via WMI DEVS and must send a
> HID feature report directly to the N-Key keyboard device instead.
>
> Add a module-level fnlock_hdev pointer (protected by a mutex) that is set
> at probe time for devices with QUIRK_HID_FN_LOCK and cleared at remove.
> Export asus_hid_fnlock_notify(bool) so that asus-armoury can call into
> hid-asus without a circular dependency.
>
> Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
> ---
> drivers/hid/hid-asus.c | 43 +++++++++++++++++++++-
> include/linux/platform_data/x86/asus-wmi.h | 5 +++
> 2 files changed, 47 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
> index d34d74df3dc0..8a51dacf35eb 100644
> --- a/drivers/hid/hid-asus.c
> +++ b/drivers/hid/hid-asus.c
> @@ -584,6 +584,38 @@ static void asus_sync_fn_lock(struct work_struct *work)
> asus_kbd_set_fn_lock(drvdata->hdev, drvdata->fn_lock);
> }
>
> +/*
> + * Module-level reference to the HID device that handles fn-lock via feature
> + * report. Set at probe and cleared at remove for QUIRK_HID_FN_LOCK devices.
> + * Protected by fnlock_hdev_lock.
> + */
> +static DEFINE_MUTEX(fnlock_hdev_lock);
> +static struct hid_device *fnlock_hdev;
> +
> +/**
> + * asus_hid_fnlock_notify() - Set fn-lock state directly via HID feature report.
> + * @enabled: true to lock fn (F1-F12 primary), false to unlock.
> + *
> + * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
> + * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
> + *
> + * Returns 0 on success, -ENODEV if no fn-lock capable HID device is present.
> + */
> +int asus_hid_fnlock_notify(bool enabled)
Generally I see _notify naming being used for internal kernel
messaging or to notify the sysfs of a change, can you change
the name of this to something like asus_hid_fnlock_enable()
or asys_hid_fnlock_set()?
> +{
> + int ret = -ENODEV;
> +
> + guard(mutex)(&fnlock_hdev_lock);
> + if (fnlock_hdev) {
> + ret = asus_kbd_set_fn_lock(fnlock_hdev, enabled);
> + /* hid_hw_raw_request returns byte count on success; normalise to 0 */
The pattern I see most ofter used is
ret = operation()
if (ret < 0)
return ret;
return 0;
IMHO much easier to read and doesn't need a comment to explain.
> + if (ret > 0)
> + ret = 0;
> + }
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(asus_hid_fnlock_notify);
> +
> static void asus_schedule_work(struct asus_kbd_leds *led)
> {
> unsigned long flags;
> @@ -969,6 +1001,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> drvdata->fn_lock = true;
> INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> asus_kbd_set_fn_lock(hdev, true);
> + guard(mutex)(&fnlock_hdev_lock);
> + fnlock_hdev = hdev;
> }
>
> if (drvdata->tp) {
> @@ -1008,6 +1042,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
> drvdata->fn_lock = true;
> INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
> asus_kbd_set_fn_lock(hdev, true);
> + guard(mutex)(&fnlock_hdev_lock);
> + fnlock_hdev = hdev;
> }
>
> return 0;
> @@ -1362,8 +1398,13 @@ static void asus_remove(struct hid_device *hdev)
> cancel_work_sync(&drvdata->kbd_backlight->work);
> }
>
> - if (drvdata->quirks & QUIRK_HID_FN_LOCK)
> + if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
> + scoped_guard(mutex, &fnlock_hdev_lock) {
> + if (fnlock_hdev == hdev)
> + fnlock_hdev = NULL;
> + }
> cancel_work_sync(&drvdata->fn_lock_sync_work);
> + }
>
> hid_hw_stop(hdev);
> }
> diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
> index 554f41b827e1..20facd5da74e 100644
> --- a/include/linux/platform_data/x86/asus-wmi.h
> +++ b/include/linux/platform_data/x86/asus-wmi.h
> @@ -196,6 +196,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
> int asus_hid_register_listener(struct asus_hid_listener *cdev);
> void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
> int asus_hid_event(enum asus_hid_event event);
> +int asus_hid_fnlock_notify(bool enabled);
> #else
> static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
> {
> @@ -227,6 +228,10 @@ static inline int asus_hid_event(enum asus_hid_event event)
> {
> return -ENODEV;
> }
> +static inline int asus_hid_fnlock_notify(bool enabled)
> +{
> + return -ENODEV;
> +}
> #endif
>
> #endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
^ permalink raw reply
* Re: [PATCH v2 3/3] platform/x86: asus-armoury: add fn_lock firmware attribute
From: Denis Benato @ 2026-05-06 22:10 UTC (permalink / raw)
To: Marcus Grenängen, platform-driver-x86
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary
In-Reply-To: <20260506193326.5862-4-marcus@grenangen.se>
On 5/6/26 21:33, Marcus Grenängen wrote:
> Add a fn_lock attribute to the asus-armoury firmware-attributes interface,
> allowing userspace to read and set the Fn-lock state (whether F1-F12 keys
> are primary or media/system keys are primary).
>
> On most ASUS laptops fn-lock is backed by WMI DEVID 0x00100023 and the
> attribute uses armoury_get/set_devstate() as normal. On platforms where
> the WMI DEVS call is a no-op (fnlock_use_hid quirk, e.g. ProArt P16), the
> store path calls asus_hid_fnlock_notify() to send the feature report
> directly to the N-Key keyboard via hid-asus. The show path returns
> -EOPNOTSUPP on such platforms as the hardware provides no readback.
>
> The fnlock_use_hid flag is detected at init time via dmi_match() on
> DMI_PRODUCT_FAMILY. A direct DMI check is used rather than reading the
> asus-nb-wmi quirk flag because asus-armoury and asus-nb-wmi are both
> loadable modules at the same init level, so the asus_ref pointer set by
> asus-wmi may not yet be valid when asus-armoury initialises.
>
> Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
> ---
> drivers/platform/x86/asus-armoury.c | 69 +++++++++++++++++++++++++++++
> 1 file changed, 69 insertions(+)
>
> diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
> index 5b0987ccc270..9d7646eff944 100644
> --- a/drivers/platform/x86/asus-armoury.c
> +++ b/drivers/platform/x86/asus-armoury.c
> @@ -93,6 +93,7 @@ struct asus_armoury_priv {
>
> u32 mini_led_dev_id;
> u32 gpu_mux_dev_id;
> + bool fnlock_use_hid;
> };
>
> static struct asus_armoury_priv asus_armoury = {
> @@ -778,6 +779,58 @@ ASUS_ATTR_GROUP_ROG_TUNABLE(nv_tgp, "nv_tgp", ASUS_WMI_DEVID_DGPU_SET_TGP,
> ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_DGPU_BASE_TGP,
> "Read the base TGP value");
>
> +/*
> + * fn_lock: toggle whether Fn key is locked (F1-F12 primary) or unlocked
> + * (media/system keys primary).
> + *
> + * On most ASUS laptops this is backed by WMI DEVID 0x00100023. On some
> + * platforms (e.g. ProArt P16) that DEVS call is a no-op and the state must
> + * be sent as a HID feature report to the N-Key keyboard via hid-asus.
> + */
> +static ssize_t fn_lock_current_value_show(struct kobject *kobj,
> + struct kobj_attribute *attr, char *buf)
> +{
> + u32 result;
> + int err;
> +
> + if (asus_armoury.fnlock_use_hid)
> + return -EOPNOTSUPP;
> +
> + err = armoury_get_devstate(attr, &result, ASUS_WMI_DEVID_FNLOCK);
> + if (err)
> + return err;
> +
> + return sysfs_emit(buf, "%u\n", result & 1);
> +}
> +
> +static ssize_t fn_lock_current_value_store(struct kobject *kobj,
> + struct kobj_attribute *attr,
> + const char *buf, size_t count)
> +{
> + bool enable;
> + int err;
> +
> + err = kstrtobool(buf, &enable);
> + if (err)
> + return err;
> +
> + if (asus_armoury.fnlock_use_hid) {
> + err = asus_hid_fnlock_notify(enable);
Doing this would introduce a dependency from asus-armoury to hid-asus,
let's not do that.
Instead only show this attribute if it's actually doing something,
you can check it from asus-wmi: asus-armoury already depends on it.
Edit: you are actually already registering this only if fnlock_use_hid is
true, so the else looks dead code to me.
I think there is something not right here.
> + if (err)
> + return err;
> + } else {
> + err = armoury_set_devstate(attr, enable ? 1 : 0, NULL,
> + ASUS_WMI_DEVID_FNLOCK);
> + if (err)
> + return err;
> + }
> +
> + sysfs_notify(kobj, NULL, attr->attr.name);
> + return count;
> +}
> +
> +ASUS_ATTR_GROUP_BOOL(fn_lock, "fn_lock", "Set the Fn-lock state");
> +
> /* If an attribute does not require any special case handling add it here */
> static const struct asus_attr_group armoury_attr_groups[] = {
> { &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
> @@ -926,6 +979,16 @@ static int asus_fw_attr_add(void)
> }
> }
>
> + if (asus_armoury.fnlock_use_hid ||
> + armoury_has_devstate(ASUS_WMI_DEVID_FNLOCK)) {
> + err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
> + &fn_lock_attr_group);
> + if (err) {
> + pr_err("Failed to create sysfs-group for fn_lock\n");
> + goto err_remove_gpu_mux_group;
> + }
> + }
> +
> for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
> if (!armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
> continue;
> @@ -963,6 +1026,8 @@ static int asus_fw_attr_add(void)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
> armoury_attr_groups[i].attr_group);
> }
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
> +err_remove_gpu_mux_group:
> if (asus_armoury.gpu_mux_dev_id)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
> err_remove_mini_led_group:
> @@ -1121,6 +1186,8 @@ static int __init asus_fw_init(void)
>
> init_rog_tunables();
>
> + asus_armoury.fnlock_use_hid = dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16");
Perhaps you can reuse something from asus-wmi instead of re-doing the dmi_match again
in this driver.
> +
> /* Must always be last step to ensure data is available */
> return asus_fw_attr_add();
> }
> @@ -1138,6 +1205,8 @@ static void __exit asus_fw_exit(void)
> if (asus_armoury.gpu_mux_dev_id)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
>
> + sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
Not guarded the same way as the sysfs_create_group therefore will trigger
on hardware that doesn't need this.
> +
> if (asus_armoury.mini_led_dev_id)
> sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
>
^ permalink raw reply
* Re: [PATCH v2 1/3] HID: asus: export asus_hid_fnlock_notify() for direct fn-lock control
From: Randy Dunlap @ 2026-05-06 22:00 UTC (permalink / raw)
To: Marcus Grenängen, platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary
In-Reply-To: <20260506193326.5862-2-marcus@grenangen.se>
On 5/6/26 12:33 PM, Marcus Grenängen wrote:
> +/**
> + * asus_hid_fnlock_notify() - Set fn-lock state directly via HID feature report.
> + * @enabled: true to lock fn (F1-F12 primary), false to unlock.
> + *
> + * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
> + * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
> + *
> + * Returns 0 on success, -ENODEV if no fn-lock capable HID device is present.
* Returns: ...
> + */
> +int asus_hid_fnlock_notify(bool enabled)
> +{
--
~Randy
^ permalink raw reply
* [PATCH v2 3/3] platform/x86: asus-armoury: add fn_lock firmware attribute
From: Marcus Grenängen @ 2026-05-06 19:33 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <20260506193326.5862-1-marcus@grenangen.se>
Add a fn_lock attribute to the asus-armoury firmware-attributes interface,
allowing userspace to read and set the Fn-lock state (whether F1-F12 keys
are primary or media/system keys are primary).
On most ASUS laptops fn-lock is backed by WMI DEVID 0x00100023 and the
attribute uses armoury_get/set_devstate() as normal. On platforms where
the WMI DEVS call is a no-op (fnlock_use_hid quirk, e.g. ProArt P16), the
store path calls asus_hid_fnlock_notify() to send the feature report
directly to the N-Key keyboard via hid-asus. The show path returns
-EOPNOTSUPP on such platforms as the hardware provides no readback.
The fnlock_use_hid flag is detected at init time via dmi_match() on
DMI_PRODUCT_FAMILY. A direct DMI check is used rather than reading the
asus-nb-wmi quirk flag because asus-armoury and asus-nb-wmi are both
loadable modules at the same init level, so the asus_ref pointer set by
asus-wmi may not yet be valid when asus-armoury initialises.
Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
---
drivers/platform/x86/asus-armoury.c | 69 +++++++++++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/drivers/platform/x86/asus-armoury.c b/drivers/platform/x86/asus-armoury.c
index 5b0987ccc270..9d7646eff944 100644
--- a/drivers/platform/x86/asus-armoury.c
+++ b/drivers/platform/x86/asus-armoury.c
@@ -93,6 +93,7 @@ struct asus_armoury_priv {
u32 mini_led_dev_id;
u32 gpu_mux_dev_id;
+ bool fnlock_use_hid;
};
static struct asus_armoury_priv asus_armoury = {
@@ -778,6 +779,58 @@ ASUS_ATTR_GROUP_ROG_TUNABLE(nv_tgp, "nv_tgp", ASUS_WMI_DEVID_DGPU_SET_TGP,
ASUS_ATTR_GROUP_INT_VALUE_ONLY_RO(nv_base_tgp, ATTR_NV_BASE_TGP, ASUS_WMI_DEVID_DGPU_BASE_TGP,
"Read the base TGP value");
+/*
+ * fn_lock: toggle whether Fn key is locked (F1-F12 primary) or unlocked
+ * (media/system keys primary).
+ *
+ * On most ASUS laptops this is backed by WMI DEVID 0x00100023. On some
+ * platforms (e.g. ProArt P16) that DEVS call is a no-op and the state must
+ * be sent as a HID feature report to the N-Key keyboard via hid-asus.
+ */
+static ssize_t fn_lock_current_value_show(struct kobject *kobj,
+ struct kobj_attribute *attr, char *buf)
+{
+ u32 result;
+ int err;
+
+ if (asus_armoury.fnlock_use_hid)
+ return -EOPNOTSUPP;
+
+ err = armoury_get_devstate(attr, &result, ASUS_WMI_DEVID_FNLOCK);
+ if (err)
+ return err;
+
+ return sysfs_emit(buf, "%u\n", result & 1);
+}
+
+static ssize_t fn_lock_current_value_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf, size_t count)
+{
+ bool enable;
+ int err;
+
+ err = kstrtobool(buf, &enable);
+ if (err)
+ return err;
+
+ if (asus_armoury.fnlock_use_hid) {
+ err = asus_hid_fnlock_notify(enable);
+ if (err)
+ return err;
+ } else {
+ err = armoury_set_devstate(attr, enable ? 1 : 0, NULL,
+ ASUS_WMI_DEVID_FNLOCK);
+ if (err)
+ return err;
+ }
+
+ sysfs_notify(kobj, NULL, attr->attr.name);
+ return count;
+}
+
+ASUS_ATTR_GROUP_BOOL(fn_lock, "fn_lock", "Set the Fn-lock state");
+
/* If an attribute does not require any special case handling add it here */
static const struct asus_attr_group armoury_attr_groups[] = {
{ &egpu_connected_attr_group, ASUS_WMI_DEVID_EGPU_CONNECTED },
@@ -926,6 +979,16 @@ static int asus_fw_attr_add(void)
}
}
+ if (asus_armoury.fnlock_use_hid ||
+ armoury_has_devstate(ASUS_WMI_DEVID_FNLOCK)) {
+ err = sysfs_create_group(&asus_armoury.fw_attr_kset->kobj,
+ &fn_lock_attr_group);
+ if (err) {
+ pr_err("Failed to create sysfs-group for fn_lock\n");
+ goto err_remove_gpu_mux_group;
+ }
+ }
+
for (i = 0; i < ARRAY_SIZE(armoury_attr_groups); i++) {
if (!armoury_has_devstate(armoury_attr_groups[i].wmi_devid))
continue;
@@ -963,6 +1026,8 @@ static int asus_fw_attr_add(void)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj,
armoury_attr_groups[i].attr_group);
}
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
+err_remove_gpu_mux_group:
if (asus_armoury.gpu_mux_dev_id)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
err_remove_mini_led_group:
@@ -1121,6 +1186,8 @@ static int __init asus_fw_init(void)
init_rog_tunables();
+ asus_armoury.fnlock_use_hid = dmi_match(DMI_PRODUCT_FAMILY, "ProArt P16");
+
/* Must always be last step to ensure data is available */
return asus_fw_attr_add();
}
@@ -1138,6 +1205,8 @@ static void __exit asus_fw_exit(void)
if (asus_armoury.gpu_mux_dev_id)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &gpu_mux_mode_attr_group);
+ sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &fn_lock_attr_group);
+
if (asus_armoury.mini_led_dev_id)
sysfs_remove_group(&asus_armoury.fw_attr_kset->kobj, &mini_led_mode_attr_group);
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/3] HID: asus: export asus_hid_fnlock_notify() for direct fn-lock control
From: Marcus Grenängen @ 2026-05-06 19:33 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <20260506193326.5862-1-marcus@grenangen.se>
Some ASUS platforms cannot control fn-lock via WMI DEVS and must send a
HID feature report directly to the N-Key keyboard device instead.
Add a module-level fnlock_hdev pointer (protected by a mutex) that is set
at probe time for devices with QUIRK_HID_FN_LOCK and cleared at remove.
Export asus_hid_fnlock_notify(bool) so that asus-armoury can call into
hid-asus without a circular dependency.
Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
---
drivers/hid/hid-asus.c | 43 +++++++++++++++++++++-
include/linux/platform_data/x86/asus-wmi.h | 5 +++
2 files changed, 47 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index d34d74df3dc0..8a51dacf35eb 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -584,6 +584,38 @@ static void asus_sync_fn_lock(struct work_struct *work)
asus_kbd_set_fn_lock(drvdata->hdev, drvdata->fn_lock);
}
+/*
+ * Module-level reference to the HID device that handles fn-lock via feature
+ * report. Set at probe and cleared at remove for QUIRK_HID_FN_LOCK devices.
+ * Protected by fnlock_hdev_lock.
+ */
+static DEFINE_MUTEX(fnlock_hdev_lock);
+static struct hid_device *fnlock_hdev;
+
+/**
+ * asus_hid_fnlock_notify() - Set fn-lock state directly via HID feature report.
+ * @enabled: true to lock fn (F1-F12 primary), false to unlock.
+ *
+ * Called by asus-armoury on platforms where the WMI DEVS path for fn-lock is
+ * non-functional (e.g. ASUS ProArt P16, N-Key keyboard product ID 0x19B6).
+ *
+ * Returns 0 on success, -ENODEV if no fn-lock capable HID device is present.
+ */
+int asus_hid_fnlock_notify(bool enabled)
+{
+ int ret = -ENODEV;
+
+ guard(mutex)(&fnlock_hdev_lock);
+ if (fnlock_hdev) {
+ ret = asus_kbd_set_fn_lock(fnlock_hdev, enabled);
+ /* hid_hw_raw_request returns byte count on success; normalise to 0 */
+ if (ret > 0)
+ ret = 0;
+ }
+ return ret;
+}
+EXPORT_SYMBOL_GPL(asus_hid_fnlock_notify);
+
static void asus_schedule_work(struct asus_kbd_leds *led)
{
unsigned long flags;
@@ -969,6 +1001,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
drvdata->fn_lock = true;
INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
asus_kbd_set_fn_lock(hdev, true);
+ guard(mutex)(&fnlock_hdev_lock);
+ fnlock_hdev = hdev;
}
if (drvdata->tp) {
@@ -1008,6 +1042,8 @@ static int asus_input_configured(struct hid_device *hdev, struct hid_input *hi)
drvdata->fn_lock = true;
INIT_WORK(&drvdata->fn_lock_sync_work, asus_sync_fn_lock);
asus_kbd_set_fn_lock(hdev, true);
+ guard(mutex)(&fnlock_hdev_lock);
+ fnlock_hdev = hdev;
}
return 0;
@@ -1362,8 +1398,13 @@ static void asus_remove(struct hid_device *hdev)
cancel_work_sync(&drvdata->kbd_backlight->work);
}
- if (drvdata->quirks & QUIRK_HID_FN_LOCK)
+ if (drvdata->quirks & QUIRK_HID_FN_LOCK) {
+ scoped_guard(mutex, &fnlock_hdev_lock) {
+ if (fnlock_hdev == hdev)
+ fnlock_hdev = NULL;
+ }
cancel_work_sync(&drvdata->fn_lock_sync_work);
+ }
hid_hw_stop(hdev);
}
diff --git a/include/linux/platform_data/x86/asus-wmi.h b/include/linux/platform_data/x86/asus-wmi.h
index 554f41b827e1..20facd5da74e 100644
--- a/include/linux/platform_data/x86/asus-wmi.h
+++ b/include/linux/platform_data/x86/asus-wmi.h
@@ -196,6 +196,7 @@ int asus_wmi_evaluate_method(u32 method_id, u32 arg0, u32 arg1, u32 *retval);
int asus_hid_register_listener(struct asus_hid_listener *cdev);
void asus_hid_unregister_listener(struct asus_hid_listener *cdev);
int asus_hid_event(enum asus_hid_event event);
+int asus_hid_fnlock_notify(bool enabled);
#else
static inline void set_ally_mcu_hack(enum asus_ally_mcu_hack status)
{
@@ -227,6 +228,10 @@ static inline int asus_hid_event(enum asus_hid_event event)
{
return -ENODEV;
}
+static inline int asus_hid_fnlock_notify(bool enabled)
+{
+ return -ENODEV;
+}
#endif
#endif /* __PLATFORM_DATA_X86_ASUS_WMI_H */
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/3] platform/x86: asus-nb-wmi: add fnlock_use_hid quirk for ProArt P16
From: Marcus Grenängen @ 2026-05-06 19:33 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <20260506193326.5862-1-marcus@grenangen.se>
The ASUS ProArt P16 (N-Key keyboard 0B05:19B6) advertises the WMI fn-lock
DEVID (0x00100023) as present via DSTS, but the DEVS call has no effect.
Fn-lock must instead be toggled via a HID feature report sent to the N-Key
keyboard (handled by hid-asus).
Add a fnlock_use_hid flag to struct quirk_entry and set it for the ProArt
P16 via a DMI match on DMI_PRODUCT_FAMILY. This flag is consumed by
asus-armoury to select the HID path instead of WMI DEVS.
Signed-off-by: Marcus Grenängen <marcus@grenangen.se>
---
drivers/platform/x86/asus-nb-wmi.c | 13 +++++++++++++
drivers/platform/x86/asus-wmi.h | 5 +++++
2 files changed, 18 insertions(+)
diff --git a/drivers/platform/x86/asus-nb-wmi.c b/drivers/platform/x86/asus-nb-wmi.c
index b4677c5bba5b..44e4cf68ff70 100644
--- a/drivers/platform/x86/asus-nb-wmi.c
+++ b/drivers/platform/x86/asus-nb-wmi.c
@@ -155,6 +155,10 @@ static struct quirk_entry quirk_asus_z13 = {
.tablet_switch_mode = asus_wmi_kbd_dock_devid,
};
+static struct quirk_entry quirk_asus_proart_p16 = {
+ .fnlock_use_hid = true,
+};
+
static int dmi_matched(const struct dmi_system_id *dmi)
{
pr_info("Identified laptop model '%s'\n", dmi->ident);
@@ -553,6 +557,15 @@ static const struct dmi_system_id asus_quirks[] = {
},
.driver_data = &quirk_asus_z13,
},
+ {
+ .callback = dmi_matched,
+ .ident = "ASUS ProArt P16",
+ .matches = {
+ DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+ DMI_MATCH(DMI_PRODUCT_FAMILY, "ProArt P16"),
+ },
+ .driver_data = &quirk_asus_proart_p16,
+ },
{},
};
diff --git a/drivers/platform/x86/asus-wmi.h b/drivers/platform/x86/asus-wmi.h
index 5cd4392b964e..6c50b11860e8 100644
--- a/drivers/platform/x86/asus-wmi.h
+++ b/drivers/platform/x86/asus-wmi.h
@@ -52,6 +52,11 @@ struct quirk_entry {
*/
int no_display_toggle;
u32 xusb2pr;
+ /*
+ * Some platforms report WMI DEVID_FNLOCK as present but the DEVS call
+ * is a no-op. Force the HID feature report path via hid-asus instead.
+ */
+ bool fnlock_use_hid;
};
struct asus_wmi_driver {
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/3] platform/x86: fix fn-lock on ASUS ProArt P16 (WMI DEVS no-op)
From: Marcus Grenängen @ 2026-05-06 19:33 UTC (permalink / raw)
To: platform-driver-x86, denis.benato
Cc: linux-input, linux-kernel, luke, hansg, ilpo.jarvinen, jikos,
bentiss, corentin.chary, marcus
In-Reply-To: <458d9e6c-8702-4cbc-9c4f-33cbd1175e67@linux.dev>
This is v2 of the fn-lock fix for ASUS laptops where the WMI DEVS call
for DEVID 0x00100023 is silently non-functional.
Changes since v1:
- Split into three patches as requested: hid-asus export, nb-wmi quirk,
asus-armoury attribute (the NULL-ptr fix for brightness_set is folded
into patch 1 where the second listener type is introduced)
- Moved the sysfs attribute to asus-armoury's firmware-attributes
interface (fn_lock under /sys/class/firmware-attributes/asus-armoury/)
instead of asus-wmi, as suggested
- asus-armoury now calls asus_hid_fnlock_notify() directly rather than
routing through asus-wmi; this avoids touching asus-wmi.c entirely
- Dropped the asus_hid_listener::fnlock_set callback and the
asus_hid_set_fnlock() / asus_hid_has_fnlock_listener() machinery from
asus-wmi.c — the direct export from hid-asus is simpler and avoids
the module init ordering issue described below
Regarding Denis's question about auto-detection: unfortunately it is not
feasible. On the ProArt P16, DSTS reports DEVID 0x00100023 as present
(ASUS_WMI_DSTS_PRESENCE_BIT set), so the existing WMI probe path finds
it. There is no distinguishing bit in the DSTS result between "WMI works"
and "WMI is silently a no-op". Attempting a test write at probe time would
be unreliable (no readback available on HID-path platforms — the fn_lock
show function intentionally returns -EOPNOTSUPP). A DMI quirk is the
cleanest approach.
Note on module init ordering: asus-armoury and asus-nb-wmi are both
compiled as loadable modules at the same init level. When asus-armoury
initialises it cannot safely dereference asus_ref.asus (set by asus-wmi)
to read the quirk flags, because asus-nb-wmi may not have probed yet.
asus-armoury therefore uses dmi_match() directly rather than an exported
accessor through asus-wmi.
Tested on ASUS ProArt P16 (H7606WI, N-Key keyboard 0B05:19B6):
- fn_lock attribute appears under firmware-attributes/asus-armoury/
- Writing 0/1 to current_value correctly toggles fn-lock state via HID
- asusctl fn-lock -s true/false works end-to-end via asusd
Marcus Grenängen (3):
HID: asus: export asus_hid_fnlock_notify() for direct fn-lock control
platform/x86: asus-nb-wmi: add fnlock_use_hid quirk for ProArt P16
platform/x86: asus-armoury: add fn_lock firmware attribute
drivers/hid/hid-asus.c | 43 +++++++++++++-
drivers/platform/x86/asus-armoury.c | 69 ++++++++++++++++++++++
drivers/platform/x86/asus-nb-wmi.c | 13 ++++
drivers/platform/x86/asus-wmi.h | 5 ++
include/linux/platform_data/x86/asus-wmi.h | 5 ++
5 files changed, 134 insertions(+), 1 deletion(-)
--
2.54.0
^ permalink raw reply
* Re: [PATCH v4 1/3] HID: nintendo: Add preliminary Switch 2 controller driver
From: Silvan Jegen @ 2026-05-06 19:17 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20260415073142.1303505-2-vi@endrift.com>
Hi!
Just some more small things (that I unfortunately missed the first time
around) 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 designed, 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 7b277d5bf3d1..4d1a28df5fd2 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 c1d9f7c6a5f2..1a293a6c02c2 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 4ab7640b119a..a794dad7980f 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 29008c2cc530..ac84e32ed0bd 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);
According to the documentation of input_register_device, we have to call
input_free_device(input) here.
> + 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)
I assume we have to call input_unregister_device here as well, as we do
so in the error case below already.
With these comments addressed this is (for what it's worth)
Reviewed-by: Silvan Jegen <s.jegen@gmail.com>
I have bought the Nintendo Switch 2 Pro Controller and have tested
the current implementation of this particular controller using
evtest. Everything worked as expected so please free to add my Tested-by
tag below as well.
Tested-by: Silvan Jegen <s.jegen@gmail.com>
Cheers,
Silvan
> + 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)
> +{
> + 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_step);
> + 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_step);
> + 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_step);
> + 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_step);
> + 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_step);
> + 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)
> + 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;
> + }
> +
> + ns2 = switch2_get_controller(phys);
> + if (IS_ERR(ns2)) {
> + ret = PTR_ERR(ns2);
> + 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 000000000000..7aff22f30266
> --- /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 7755e5b454d2..868262c6ccd9 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 9976f596a920..8f92900ae885 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 000000000000..ebd89d852e21
> --- /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);
> +}
> +
> +#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");
^ permalink raw reply
* Re: [PATCH 4/9] dt-bindings: iio: adc: mt6359: Add MT6365 PMIC AuxADC
From: Rob Herring (Arm) @ 2026-05-06 15:32 UTC (permalink / raw)
To: Louis-Alexis Eyraud
Cc: AngeloGioacchino Del Regno, Dmitry Torokhov, Nuno Sá, kernel,
linux-kernel, devicetree, Chen Zhong, Krzysztof Kozlowski,
linux-arm-kernel, Sen Chu, David Lechner, Jonathan Cameron,
Matthias Brugger, linux-iio, linux-input, Lee Jones,
linux-mediatek, Sean Wang, Andy Shevchenko, linux-pm,
Conor Dooley, Macpaul Lin
In-Reply-To: <20260429-mediatek-genio-mt6365-cleanup-v1-4-6f43838be92f@collabora.com>
On Wed, 29 Apr 2026 11:44:17 +0200, Louis-Alexis Eyraud wrote:
> Add compatible string for the AuxADC block found on the MT6365 PMIC,
> that is compatible with the one found in MT6359.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
> .../bindings/iio/adc/mediatek,mt6359-auxadc.yaml | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 3/9] dt-bindings: input: mediatek,pmic-keys: Add MT6365 support
From: Rob Herring (Arm) @ 2026-05-06 15:32 UTC (permalink / raw)
To: Louis-Alexis Eyraud
Cc: linux-iio, Macpaul Lin, Sean Wang, Jonathan Cameron, kernel,
Lee Jones, Dmitry Torokhov, Conor Dooley, linux-kernel,
linux-input, devicetree, Krzysztof Kozlowski, linux-pm,
Nuno Sá, David Lechner, Matthias Brugger,
AngeloGioacchino Del Regno, linux-arm-kernel, Sen Chu,
linux-mediatek, Andy Shevchenko, Chen Zhong
In-Reply-To: <20260429-mediatek-genio-mt6365-cleanup-v1-3-6f43838be92f@collabora.com>
On Wed, 29 Apr 2026 11:44:16 +0200, Louis-Alexis Eyraud wrote:
> Add compatible string for the pmic keys block found on the MT6365 PMIC,
> that is compatible with the one found in MT6359.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
> .../bindings/input/mediatek,pmic-keys.yaml | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 2/9] dt-bindings: mfd: mediatek: mt6397: Add MT6365 PMIC support
From: Rob Herring (Arm) @ 2026-05-06 15:32 UTC (permalink / raw)
To: Louis-Alexis Eyraud
Cc: AngeloGioacchino Del Regno, Dmitry Torokhov, Krzysztof Kozlowski,
kernel, Andy Shevchenko, Conor Dooley, linux-kernel,
Matthias Brugger, Nuno Sá, David Lechner, Macpaul Lin,
linux-iio, linux-pm, Jonathan Cameron, Sean Wang, Sen Chu,
linux-mediatek, linux-arm-kernel, Lee Jones, linux-input,
Chen Zhong, devicetree
In-Reply-To: <20260429-mediatek-genio-mt6365-cleanup-v1-2-6f43838be92f@collabora.com>
On Wed, 29 Apr 2026 11:44:15 +0200, Louis-Alexis Eyraud wrote:
> MT6365 PMIC is compatible with MT6359, so add the compatible strings
> for the main and sub devices (regulator, rtc, audio codec).
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
> Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH 1/9] dt-bindings: mfd: mediatek: mt6397: Add rtc for MT6359
From: Rob Herring (Arm) @ 2026-05-06 15:31 UTC (permalink / raw)
To: Louis-Alexis Eyraud
Cc: Sen Chu, Macpaul Lin, Sean Wang, Lee Jones, linux-mediatek,
Jonathan Cameron, devicetree, kernel, Nuno Sá, linux-input,
Conor Dooley, David Lechner, AngeloGioacchino Del Regno,
linux-arm-kernel, Chen Zhong, linux-pm, Matthias Brugger,
Dmitry Torokhov, linux-iio, Krzysztof Kozlowski, Andy Shevchenko,
linux-kernel
In-Reply-To: <20260429-mediatek-genio-mt6365-cleanup-v1-1-6f43838be92f@collabora.com>
On Wed, 29 Apr 2026 11:44:14 +0200, Louis-Alexis Eyraud wrote:
> The rtc block of MT6359 PMIC is compatible with the one found in MT6358
> but this compatibility was never expressed in the dt-bindings, so add
> the missing compatible string for the rtc subnode.
>
> Signed-off-by: Louis-Alexis Eyraud <louisalexis.eyraud@collabora.com>
> ---
> Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml | 1 +
> 1 file changed, 1 insertion(+)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH] HID: mcp2221: Fix heap buffer overflow in mcp2221_raw_event()
From: Benoît Sevens @ 2026-05-06 14:40 UTC (permalink / raw)
To: Rishi Gupta, Jiri Kosina, Benjamin Tissoires
Cc: linux-i2c, linux-input, linux-kernel
In-Reply-To: <20260415114752.1181079-1-bsevens@google.com>
Hi,
Just checking in to see if anyone had a chance to look at this patch,
or if there is any feedback I can address.
Thanks!
^ permalink raw reply
* Re: [PATCH v3 0/4] HID: Proper fix for OOM in hid-core
From: Lee Jones @ 2026-05-06 9:16 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Filipe Laíns, Bastien Nocera, Ping Cheng,
Jason Gerecke, Viresh Kumar, Johan Hovold, Alex Elder,
Greg Kroah-Hartman, Icenowy Zheng, linux-input, linux-kernel,
greybus-dev, linux-staging, linux-usb, stable
In-Reply-To: <20260504-wip-fix-core-v3-0-ce1f11f4968f@kernel.org>
On Mon, 04 May 2026, Benjamin Tissoires wrote:
> Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> bogus memset()") enforced the provided data to be at least the size of
> the declared buffer in the report descriptor to prevent a buffer
> overflow.
>
> We only had corner cases of malicious devices exposing the OOM because
> in most cases, the buffer provided by the transport layer needs to be
> allocated at probe time and is large enough to handle all the possible
> reports.
>
> However, the patch from above, which enforces the spec a little bit more
> introduced both regressions for devices not following the spec (not
> necesserally malicious), but also a stream of errors for those devices.
>
> Let's revert to the old behavior by giving more information to HID core
> to be able to decide whether it can or not memset the rest of the buffer
> to 0 and continue the processing.
>
> Note that the first commit makes an API change, but the callers are
> relatively limited, so it should be fine on its own. The second patch
> can't really make the same kind of API change because we have too many
> callers in various subsystems. We can switch them one by one to the safe
> approach when needed.
>
> The last 2 patches are small cleanups I initially put together with the
> 2 first patches, but they can be applied on their own and don't need to
> be pulled in stable like the first 2.
>
> Cheers,
> Benjamin
>
> Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> ---
> Changes in v3:
> - fixed ghib -> ghid in greybus
> - fixed i386 size_t debug size reported by kernel-bot
> - Link to v2: https://lore.kernel.org/r/20260416-wip-fix-core-v2-0-be92570e5627@kernel.org
>
> Changes in v2:
> - added a small blurb explaining the difference between the safe and the
> non safe version of hid_safe_input_report
> - Link to v1: https://lore.kernel.org/r/20260415-wip-fix-core-v1-0-ed3c4c823175@kernel.org
>
> ---
> Benjamin Tissoires (4):
> HID: pass the buffer size to hid_report_raw_event
> HID: core: introduce hid_safe_input_report()
> HID: multitouch: use __free(kfree) to clean up temporary buffers
> HID: wacom: use __free(kfree) to clean up temporary buffers
>
> drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++--
> drivers/hid/hid-core.c | 67 ++++++++++++++++++++++++++++++--------
> drivers/hid/hid-gfrm.c | 4 +--
> drivers/hid/hid-logitech-hidpp.c | 2 +-
> drivers/hid/hid-multitouch.c | 18 ++++------
> drivers/hid/hid-primax.c | 2 +-
> drivers/hid/hid-vivaldi-common.c | 2 +-
> drivers/hid/i2c-hid/i2c-hid-core.c | 7 ++--
> drivers/hid/usbhid/hid-core.c | 11 ++++---
> drivers/hid/wacom_sys.c | 46 +++++++++-----------------
> drivers/staging/greybus/hid.c | 2 +-
> include/linux/hid.h | 6 ++--
> include/linux/hid_bpf.h | 14 +++++---
> 13 files changed, 109 insertions(+), 78 deletions(-)
What's the plan for this set Benjamin? -rcs or -next?
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: input: Document Imagis ISA1200 haptic motor driver
From: Linus Walleij @ 2026-05-06 8:01 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Svyatoslav Ryhel, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-input, devicetree,
linux-kernel
In-Reply-To: <20260506-nautilus-of-abstract-efficiency-eebe94@quoll>
On Wed, May 6, 2026 at 9:39 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sun, May 03, 2026 at 07:52:42PM +0300, Svyatoslav Ryhel wrote:
> > Document the Imagis ISA1200 haptic motor driver, used primarily in mobile
> > handheld devices and capable of supporting up to two motors.
> >
> > The exact datasheet for the ISA1200 is not available; all data was modeled
> > based on available downstream kernel sources for various devices and
> > fragments of information scattered across the internet.
> >
> > Tested-by: Linus Walleij <linusw@kernel.org> # Samsung GT-I9070 Janice
>
> Drop the tag here, you cannot test a binding. It's not possible or
> otherwise explain how YAML file was tested by this device.
Right, I was probably sloppy and replied Tested-by on the cover
letter.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: input: Document Imagis ISA1200 haptic motor driver
From: Krzysztof Kozlowski @ 2026-05-06 7:39 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Linus Walleij, linux-input, devicetree, linux-kernel
In-Reply-To: <20260503165243.215979-2-clamor95@gmail.com>
On Sun, May 03, 2026 at 07:52:42PM +0300, Svyatoslav Ryhel wrote:
> Document the Imagis ISA1200 haptic motor driver, used primarily in mobile
> handheld devices and capable of supporting up to two motors.
>
> The exact datasheet for the ISA1200 is not available; all data was modeled
> based on available downstream kernel sources for various devices and
> fragments of information scattered across the internet.
>
> Tested-by: Linus Walleij <linusw@kernel.org> # Samsung GT-I9070 Janice
Drop the tag here, you cannot test a binding. It's not possible or
otherwise explain how YAML file was tested by this device.
Best regards,
Krzysztof
^ permalink raw reply
* RE: [PATCH v2 0/4] iio: introduce devm_ API for hid sensro setup and cleanup
From: Zhang, Lixu @ 2026-05-06 1:51 UTC (permalink / raw)
To: Jonathan Cameron, srinivas pandruvada
Cc: Sanjay Chitroda, jikos@kernel.org, Lechner, David,
nuno.sa@analog.com, andy@kernel.org, sakari.ailus@linux.intel.com,
linux-input@vger.kernel.org, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260505173302.4284a077@jic23-huawei>
>-----Original Message-----
>From: Jonathan Cameron <jic23@kernel.org>
>Sent: Wednesday, May 6, 2026 12:33 AM
>To: srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
>Cc: Sanjay Chitroda <sanjayembeddedse@gmail.com>; jikos@kernel.org;
>Lechner, David <dlechner@baylibre.com>; nuno.sa@analog.com;
>andy@kernel.org; sakari.ailus@linux.intel.com; linux-input@vger.kernel.org;
>linux-iio@vger.kernel.org; linux-kernel@vger.kernel.org; Zhang, Lixu
><lixu.zhang@intel.com>
>Subject: Re: [PATCH v2 0/4] iio: introduce devm_ API for hid sensro setup and
>cleanup
>
>On Fri, 01 May 2026 04:53:38 -0700
>srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
>> + Lixu
>>
>> On Wed, 2026-04-29 at 23:29 +0530, Sanjay Chitroda wrote:
>> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>> >
>> > Key highlights:
>> > - Prepare change as pre-requisite for devm conversion for HID IIO
>> > drivers by removing redundant argument
>> > - Add devm API to setup trigger and clenaup resource using
>> > devm_add_action_or_reset()
>> > - few sample driver update using devm conversion to auto release
>> > resource
>> >
>>
>> devm_* calls are fine but needs tests particularly when forces ISH PCI
>> drivers unbind, while iio-sensor-proxy has open sessions.
>>
>> Lixu, Please check.
>Hold off perhaps until we have a v3 with fixes for the existing devm calls against
>wrong struct device.
Sure, I'll test the ISH unbind scenario once v3 is out.
Thanks,
Lixu
>
>Thanks,
>
>Jonathan
>
>>
>> Thanks,
>> Srinivas
>>
^ permalink raw reply
* Re: [PATCH v4 00/11] Input: support for STM FTS5
From: Dmitry Torokhov @ 2026-05-05 21:54 UTC (permalink / raw)
To: david
Cc: Maxime Coquelin, Alexandre Torgue, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Henrik Rydberg,
Bjorn Andersson, Konrad Dybcio, Petr Hodina, linux-input,
linux-stm32, linux-arm-kernel, linux-kernel, Krzysztof Kozlowski,
devicetree, linux-arm-msm, phone-devel, Konrad Dybcio
In-Reply-To: <20260409-stmfts5-v4-0-64fe62027db5@ixit.cz>
On Thu, Apr 09, 2026 at 12:15:43AM +0200, David Heidelberg via B4 Relay wrote:
> Used on various phones. Minimal viable driver.
>
> Includes device-tree enabling touchscreen on Pixel 3.
>
> What is missing:
> - switching between AP and SLPI mode (to be able to wake up phone by touch)
> - firmware loading
> - anything above basic touch
>
> Signed-off-by: David Heidelberg <david@ixit.cz>
Applied #1 through #8 (#5 with minor edits).
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5 4/9] dt-bindings: pinctrl: mediatek,mt65xx: Add MT6392 pinctrl
From: Rob Herring (Arm) @ 2026-05-05 19:41 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: Sen Chu, Matthias Brugger, Mark Brown, Gary Bisson,
AngeloGioacchino Del Regno, Lee Jones, linux-mediatek,
linux-kernel, linux-input, Louis-Alexis Eyraud, linux-arm-kernel,
linux-gpio, devicetree, Julien Massot, Fabien Parent, Macpaul Lin,
Val Packett, Liam Girdwood, Linus Walleij, linux-pm, Sean Wang,
Dmitry Torokhov, Conor Dooley, Akari Tsuyukusa, Chen Zhong,
Krzysztof Kozlowski
In-Reply-To: <20260420213529.1645560-5-l.scorcia@gmail.com>
On Mon, 20 Apr 2026 22:30:03 +0100, Luca Leonardo Scorcia wrote:
> Add a compatible for the pinctrl device of the MT6392 PMIC, a variant of
> the already supported MT6397.
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> .../pinctrl/mediatek,mt65xx-pinctrl.yaml | 1 +
> .../pinctrl/mediatek,mt6392-pinfunc.h | 39 +++++++++++++++++++
> 2 files changed, 40 insertions(+)
> create mode 100644 include/dt-bindings/pinctrl/mediatek,mt6392-pinfunc.h
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3] Input: ads7846 - don't use scratch for tx_buf when clearing register
From: Dmitry Torokhov @ 2026-05-05 18:10 UTC (permalink / raw)
To: Kris Bahnsen
Cc: Marek Vasut, stable, Mark Featherston, linux-input, linux-kernel
In-Reply-To: <c49600c3-a78d-4d74-82bd-7f95328388a5@embeddedTS.com>
On Tue, May 05, 2026 at 09:21:50AM -0700, Kris Bahnsen wrote:
> Dmitry,
>
> On 5/4/26 8:01 PM, Dmitry Torokhov wrote:
> > Hi Kris,
> >
> > On Thu, Apr 30, 2026 at 05:37:38PM +0000, Kris Bahnsen wrote:
> >> The workaround for XPT2046 clears the command register, giving the
> >> touchscreen controller a NOP. The change incorrectly re-uses the
> >> req->scratch variable which is used as rx_buf for xfer[5], so by
> >> the time xfer[6] occurs, the contents of req->scratch may not be
> >> 0. It was found that the touchscreen controller can end up in
> >> a completely unresponsive state due to it being given a command
> >> the driver does not expect.
> >>
> >> Instead, rely on the spi_transfer behavior of tx_buf being NULL to
> >> transmit all 0 bits and use the scratch variable for the rx_buf for
> >> both the 1 byte command to and 2 byte response from the controller.
> >>
> >> This change was tested on real TSC2046 and ADS7843 controllers,
> >> but not the XPT2046 the workaround was originally created for.
> >> Confirming that the original modification to clear the command
> >> register does not impact either real controller.
> >>
> >> Fixes: 781a07da9bb94 ("Input: ads7846 - add dummy command register clearing cycle")
> >> Cc: stable@vger.kernel.org
> >> Co-developed-by: Mark Featherston <mark@embeddedTS.com>
> >> Signed-off-by: Mark Featherston <mark@embeddedTS.com>
> >> Signed-off-by: Kris Bahnsen <kris@embeddedTS.com>
> >> ---
> >>
> >> V1 -> V2: Don't use rx_buf when clearing command reg
> >> V2 -> V3: Modify original 2 xfer command to eliminate dev_err()
> >> output on xfer with len and NULL buffers
> >>
> >> drivers/input/touchscreen/ads7846.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
> >> index 4b39f7212d35c..488bcc8393293 100644
> >> --- a/drivers/input/touchscreen/ads7846.c
> >> +++ b/drivers/input/touchscreen/ads7846.c
> >> @@ -403,8 +403,7 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
> >> spi_message_add_tail(&req->xfer[5], &req->msg);
> >>
> >> /* clear the command register */
> >> - req->scratch = 0;
> >> - req->xfer[6].tx_buf = &req->scratch;
> >> + req->xfer[6].rx_buf = &req->scratch;
> >
> > Sashiko (I believe correctly) pointed out that by doing this "scratch"
> > is now write only and this may cause DMA from the device stomp on
> > message status and other unrelated data that shares the same cacheline
> > with scracth. While it was already a problem before now it is even more
> > likely.
> >
> > Since scratch is now write-only I believe moving it below "sample"
> > forces it into separate cacheline and fixes this problem. Could you
> > please try making this change?
>
> Apologies, I'm not quite certain I understand what you mean by
> "moving it below sample." Do you mean relocating the xfer[6] block
> immediately below the xfer[3] block like so? If yes, I can get this
> tested and a v4 patch together. If not, can you please clarify?
I meant doing this:
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 093f4b56cc18..04ba98b62f70 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -328,7 +328,6 @@ struct ser_req {
u8 ref_on;
u8 command;
u8 ref_off;
- u16 scratch;
struct spi_message msg;
struct spi_transfer xfer[8];
/*
@@ -336,6 +335,7 @@ struct ser_req {
* transfer buffers to live in their own cache lines.
*/
__be16 sample ____cacheline_aligned;
+ u16 scratch;
};
struct ads7845_ser_req {
Thanks.
--
Dmitry
^ permalink raw reply related
* Re: [PATCH v2 0/4] iio: introduce devm_ API for hid sensro setup and cleanup
From: Jonathan Cameron @ 2026-05-05 16:33 UTC (permalink / raw)
To: srinivas pandruvada
Cc: Sanjay Chitroda, jikos, dlechner, nuno.sa, andy, sakari.ailus,
linux-input, linux-iio, linux-kernel, Zhang Lixu
In-Reply-To: <15509f5e5a6642d2a4b0ccc13bc7d40d3b79f72a.camel@linux.intel.com>
On Fri, 01 May 2026 04:53:38 -0700
srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
> + Lixu
>
> On Wed, 2026-04-29 at 23:29 +0530, Sanjay Chitroda wrote:
> > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >
> > Key highlights:
> > - Prepare change as pre-requisite for devm conversion for HID IIO
> > drivers by removing redundant argument
> > - Add devm API to setup trigger and clenaup resource using
> > devm_add_action_or_reset()
> > - few sample driver update using devm conversion to auto release
> > resource
> >
>
> devm_* calls are fine but needs tests particularly when forces ISH PCI
> drivers unbind, while iio-sensor-proxy has open sessions.
>
> Lixu, Please check.
Hold off perhaps until we have a v3 with fixes for the existing devm
calls against wrong struct device.
Thanks,
Jonathan
>
> Thanks,
> Srinivas
>
>
>
> > changes in v2:
> > - Following input from Jonathan and Andy, squash initial patch v1
> > series in single change as individual change should not break
> > anything
> > - Add devm API support and two driver using the same
> > - v1 series ->
> > https://lore.kernel.org/all/20260428071613.1134053-1-sanjayembedded@gmail.com/
> >
> > Testing:
> > - Compiled with W=1
> > - Build-tested on QEMU x86_64
> >
> > Based on further feedback and reviews, I would extend this series to
> > convert all HID IIO driver to use devm_* API.
> >
> > Thanks,
> > Sanjay Chitroda
> >
> > Sanjay Chitroda (4):
> > iio: hid-sensors: drop redundant iio_dev argument
> > iio: hid-sensors: introduce device managed API
> > iio: gyro: drop hid_sensor_remove_trigger() using devm API
> > iio: humidity: drop hid_sensor_remove_trigger() using devm API
> >
> > drivers/iio/accel/hid-sensor-accel-3d.c | 4 +--
> > .../common/hid-sensors/hid-sensor-trigger.c | 27
> > +++++++++++++++++--
> > .../common/hid-sensors/hid-sensor-trigger.h | 5 ++--
> > drivers/iio/gyro/hid-sensor-gyro-3d.c | 10 +++----
> > drivers/iio/humidity/hid-sensor-humidity.c | 10 +++----
> > drivers/iio/light/hid-sensor-als.c | 4 +--
> > drivers/iio/light/hid-sensor-prox.c | 4 +--
> > drivers/iio/magnetometer/hid-sensor-magn-3d.c | 4 +--
> > drivers/iio/orientation/hid-sensor-incl-3d.c | 4 +--
> > drivers/iio/orientation/hid-sensor-rotation.c | 4 +--
> > .../position/hid-sensor-custom-intel-hinge.c | 4 +--
> > drivers/iio/pressure/hid-sensor-press.c | 4 +--
> > .../iio/temperature/hid-sensor-temperature.c | 4 +--
> > 13 files changed, 52 insertions(+), 36 deletions(-)
> >
> >
> > base-commit: eade2b843d9b1f668fc1775f15611bb0a1999cd9
^ permalink raw reply
* Re: [PATCH v2 4/4] iio: humidity: drop hid_sensor_remove_trigger() using devm API
From: Jonathan Cameron @ 2026-05-05 16:32 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Sanjay Chitroda, Andy Shevchenko, jikos, srinivas.pandruvada,
dlechner, nuno.sa, andy, sakari.ailus, linux-input, linux-iio,
linux-kernel
In-Reply-To: <CAHp75VdzPr+SLCPT85zw6aK8pSueBHr-OFpJ1Vmsow61e8=p4w@mail.gmail.com>
On Fri, 1 May 2026 21:25:35 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Thu, Apr 30, 2026 at 10:21 PM Sanjay Chitroda
> <sanjayembeddedse@gmail.com> wrote:
> > On 30 April 2026 1:03:56 am IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> > >On Wed, Apr 29, 2026 at 11:29:18PM +0530, Sanjay Chitroda wrote:
> > >
> > >> Use devm_hid_sensor_setup_trigger() to automatically release resource
>
> resources
>
> > >> during fail, unbind or removal of driver using devres framework.
>
> failure
>
> > >>
> > >> This simplify the setup, remove goto, avoid manual resource cleanup in
>
> simplifies
> removes
> and avoids
>
> OR
>
> "This is done in a way to simplify..."
>
> > >> teardown path.
>
> ...
>
> > >> + ret = devm_hid_sensor_setup_trigger(&indio_dev->dev, indio_dev, name,
> > >> + &humid_st->common_attributes);
> > >
> > >I believe the first parameter is utterly wrong here.
> > >Or other way around, same issue but in the previous patch.
> > >
> > Thank you Andy for the review comment.
> >
> > It looks in same humidity probe of hid sensor with devm API two device pointer are used &pdev->dev and &indio_dev->dev; ideally all devm should have same parent device for devres resource framework and over here preferable and consistent device should be &pdev->dev;
> >
> > I would first update existing devm_* API to have consistent device and on top of that will use same device in devm conversation.
> >
> > While for gyro change device is consistent as &pdev->dev across all devm API.
>
> The idea is that you have to go deeply understanding the object
> lifetimes for the cases of different device instances along with
> userspace communication channels (all possible ABIs the driver uses).
> With only that the proper parameter may be chosen or even confirmed
> that device managed resources must not be used. Yeah, this is one of
> the downsides of devm_*() APIs.
>
FWIW it's always wrong from a design point of view to use iio_dev->dev
for devm. It works but leaves us open to getting the ordering wrong
as some other stuff (including things you can't necessarily see explicitly
in a driver) will be using the parent device.
Jonathan
^ permalink raw reply
* Re: [PATCH v3] Input: ads7846 - don't use scratch for tx_buf when clearing register
From: Kris Bahnsen @ 2026-05-05 16:21 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Marek Vasut, stable, Mark Featherston, linux-input, linux-kernel
In-Reply-To: <aflcL6y_ugHV5p8s@google.com>
Dmitry,
On 5/4/26 8:01 PM, Dmitry Torokhov wrote:
> Hi Kris,
>
> On Thu, Apr 30, 2026 at 05:37:38PM +0000, Kris Bahnsen wrote:
>> The workaround for XPT2046 clears the command register, giving the
>> touchscreen controller a NOP. The change incorrectly re-uses the
>> req->scratch variable which is used as rx_buf for xfer[5], so by
>> the time xfer[6] occurs, the contents of req->scratch may not be
>> 0. It was found that the touchscreen controller can end up in
>> a completely unresponsive state due to it being given a command
>> the driver does not expect.
>>
>> Instead, rely on the spi_transfer behavior of tx_buf being NULL to
>> transmit all 0 bits and use the scratch variable for the rx_buf for
>> both the 1 byte command to and 2 byte response from the controller.
>>
>> This change was tested on real TSC2046 and ADS7843 controllers,
>> but not the XPT2046 the workaround was originally created for.
>> Confirming that the original modification to clear the command
>> register does not impact either real controller.
>>
>> Fixes: 781a07da9bb94 ("Input: ads7846 - add dummy command register clearing cycle")
>> Cc: stable@vger.kernel.org
>> Co-developed-by: Mark Featherston <mark@embeddedTS.com>
>> Signed-off-by: Mark Featherston <mark@embeddedTS.com>
>> Signed-off-by: Kris Bahnsen <kris@embeddedTS.com>
>> ---
>>
>> V1 -> V2: Don't use rx_buf when clearing command reg
>> V2 -> V3: Modify original 2 xfer command to eliminate dev_err()
>> output on xfer with len and NULL buffers
>>
>> drivers/input/touchscreen/ads7846.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
>> index 4b39f7212d35c..488bcc8393293 100644
>> --- a/drivers/input/touchscreen/ads7846.c
>> +++ b/drivers/input/touchscreen/ads7846.c
>> @@ -403,8 +403,7 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
>> spi_message_add_tail(&req->xfer[5], &req->msg);
>>
>> /* clear the command register */
>> - req->scratch = 0;
>> - req->xfer[6].tx_buf = &req->scratch;
>> + req->xfer[6].rx_buf = &req->scratch;
>
> Sashiko (I believe correctly) pointed out that by doing this "scratch"
> is now write only and this may cause DMA from the device stomp on
> message status and other unrelated data that shares the same cacheline
> with scracth. While it was already a problem before now it is even more
> likely.
>
> Since scratch is now write-only I believe moving it below "sample"
> forces it into separate cacheline and fixes this problem. Could you
> please try making this change?
Apologies, I'm not quite certain I understand what you mean by
"moving it below sample." Do you mean relocating the xfer[6] block
immediately below the xfer[3] block like so? If yes, I can get this
tested and a v4 patch together. If not, can you please clarify?
diff --git a/drivers/input/touchscreen/ads7846.c b/drivers/input/touchscreen/ads7846.c
index 4b39f7212d35..6d57865ff505 100644
--- a/drivers/input/touchscreen/ads7846.c
+++ b/drivers/input/touchscreen/ads7846.c
@@ -390,6 +390,11 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
req->xfer[3].len = 2;
spi_message_add_tail(&req->xfer[3], &req->msg);
+ /* clear the command register */
+ req->xfer[6].rx_buf = &req->scratch;
+ req->xfer[6].len = 1;
+ spi_message_add_tail(&req->xfer[6], &req->msg);
+
/* REVISIT: take a few more samples, and compare ... */
/* converter in low power mode & enable PENIRQ */
@@ -402,12 +407,6 @@ static int ads7846_read12_ser(struct device *dev, unsigned command)
req->xfer[5].len = 2;
spi_message_add_tail(&req->xfer[5], &req->msg);
- /* clear the command register */
- req->scratch = 0;
- req->xfer[6].tx_buf = &req->scratch;
- req->xfer[6].len = 1;
- spi_message_add_tail(&req->xfer[6], &req->msg);
-
req->xfer[7].rx_buf = &req->scratch;
req->xfer[7].len = 2;
CS_CHANGE(req->xfer[7]);
>
> Thanks.
>
--
Kris Bahnsen
Software Engineer
embeddedTS
^ permalink raw reply related
* Re: [PATCH 1/3] Input: atmel_mxt_ts - fix boundary check in mxt_prepare_cfg_mem
From: Dmitry Torokhov @ 2026-05-05 15:03 UTC (permalink / raw)
To: Ricardo Ribalda; +Cc: Nick Dyer, linux-input, linux-kernel, stable
In-Reply-To: <CANiDSCv+h_ry7W1e1mFNLhont-1xigEZj6jL3m=FVgv2UC+KzQ@mail.gmail.com>
Hi Ricardo,
On Tue, May 05, 2026 at 11:08:15AM +0200, Ricardo Ribalda wrote:
> HI Dmitry
>
> FWIW this patch looks correct to me...
Thank you for looking this over.
>
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
>
> But there are a couple of things that look weird.
>
> 1) The patch line (1503) does not seem to match your tree
> https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/tree/drivers/input/touchscreen/atmel_mxt_ts.c#n1503
Yeah, I have an unrelated path in my queue that affects line offsets,
>
> 2) The sscanf just before this check has two conversions (val and
> offset), but you only check for ret != 1. Should't it be ret !=2? or I
> am missing something?
"%n" format specifier does not increment number of successfully parsed
elements returned by sscanf(). It kind of makes sense although may look
surprising.
Thanks.
--
Dmitry
^ 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