* Re: [PATCH] HID: nintendo: add support for HORI Wireless Switch Pad
From: Joshua Peisach @ 2026-05-27 1:12 UTC (permalink / raw)
To: Hector Zelaya, Jiri Kosina, Benjamin Tissoires,
Daniel J. Ogorchock
Cc: linux-input, linux-kernel
In-Reply-To: <20260526-hori-support-v1-1-1861c0abc2e0@hectorzelaya.dev>
On Tue May 26, 2026 at 1:08 PM EDT, Hector Zelaya wrote:
> Add support for the HORI Wireless Switch Pad (vendor 0x0f0d, product
> 0x00f6), a licensed third-party Nintendo Switch Pro Controller.
>
> The controller reports controller type 0x06 (vs 0x03 for first-party
> Pro Controllers) and has the following quirks:
>
> - SPI flash calibration data is incompatible; use default stick
> calibration values instead.
> - X and Y button bits are swapped compared to first-party controllers;
> add a dedicated button mapping table.
> - Rumble and IMU enable may timeout (no vibration motor in hardware);
> treat as non-fatal for licensed controllers.
>
> Tested over Bluetooth on NixOS with kernel 7.0.5 and 7.0.10:
> - All 14 buttons map correctly
> - Player LED sets on connect
> - Sticks report correctly with default calibration
> - IMU/gyro data streams at 60Hz
> - D-pad reports on ABS_HAT0X/HAT0Y
>
> Device information:
> Bluetooth name: Lic Pro Controller
> Bluetooth HID: 0005:0F0D:00F6
>
> Assisted-by: Kiro:Auto [Amazon Kiro IDE]
> Signed-off-by: Hector Zelaya <hector@hectorzelaya.dev>
> ---
Looks fine.
Reviewed-by: Joshua Peisach <jpeisach@ubuntu.com>
^ permalink raw reply
* [PATCH] Input: mtk-pmic-keys - match loop with count
From: Rosen Penev @ 2026-05-27 0:45 UTC (permalink / raw)
To: linux-input
Cc: Dmitry Torokhov, Matthias Brugger, AngeloGioacchino Del Regno,
open list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support
of_get_available_child_count is used along with
for_each_child_of_node_scoped, which can cause a mismatch when keys have
a disabled status.
If a disabled child node exists in the device tree alongside available ones,
the loop could execute more times than the initial validation accounted for.
This might increment the index variable past the allocated array bounds,
leading to out-of-bounds accesses on irqnames[] and keys->keys[].
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
drivers/input/keyboard/mtk-pmic-keys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/mtk-pmic-keys.c b/drivers/input/keyboard/mtk-pmic-keys.c
index c78d9f6d97c4..5d4ebbafd276 100644
--- a/drivers/input/keyboard/mtk-pmic-keys.c
+++ b/drivers/input/keyboard/mtk-pmic-keys.c
@@ -363,7 +363,7 @@ static int mtk_pmic_keys_probe(struct platform_device *pdev)
return -EINVAL;
}
- for_each_child_of_node_scoped(node, child) {
+ for_each_available_child_of_node_scoped(node, child) {
keys->keys[index].regs = &mtk_pmic_regs->keys_regs[index];
keys->keys[index].irq =
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v4 35/36] iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger() using devm API
From: Sanjay Chitroda @ 2026-05-26 18:16 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Christophe JAILLET, jikos, srinivas.pandruvada, dlechner, nuno.sa,
andy, sakari.ailus, linux-input, linux-iio, linux-kernel
In-Reply-To: <20260526165451.6a587221@jic23-huawei>
On 26 May 2026 9:24:51 pm IST, Jonathan Cameron <jic23@kernel.org> wrote:
>On Mon, 25 May 2026 22:35:50 +0530
>Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
>
>> On 25 May 2026 3:39:11 pm IST, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
>> >Le 24/05/2026 à 21:29, Sanjay Chitroda a écrit :
>> >> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>> >>
>> >> Use devm_hid_sensor_setup_trigger() to automatically release resource
>> >> during fail, unbind or removal of driver using devres framework.
>> >>
>> >> This simplify the setup, remove goto, avoid manual resource cleanup in
>> >> teardown path.
>> >>
>> >> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>> >> ---
>> >> drivers/iio/pressure/hid-sensor-press.c | 10 +++-------
>> >> 1 file changed, 3 insertions(+), 7 deletions(-)
>> >>
>> >> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
>> >> index fee7dcb86801..8438498c9aeb 100644
>> >> --- a/drivers/iio/pressure/hid-sensor-press.c
>> >> +++ b/drivers/iio/pressure/hid-sensor-press.c
>> >> @@ -291,8 +291,8 @@ static int hid_press_probe(struct platform_device *pdev)
>> >> atomic_set(&press_state->common_attributes.data_ready, 0);
>> >> - ret = hid_sensor_setup_trigger(indio_dev, name,
>> >> - &press_state->common_attributes);
>> >> + ret = devm_hid_sensor_setup_trigger(dev, indio_dev, name,
>> >> + &press_state->common_attributes);
>> >> if (ret) {
>> >> dev_err(dev, "trigger setup failed\n");
>> >> return ret;
>> >> @@ -301,7 +301,7 @@ static int hid_press_probe(struct platform_device *pdev)
>> >> ret = iio_device_register(indio_dev);
>> >
>> >Hi,
>> >
>> >Looks like devm_iio_device_register() would now make sense and simplify things even further.
>> >
>> >CJ
>>
>> Hi CJ,
>>
>> Yes, as mentioned in cover letter most of HID IIO drivers have potential and will converted to devm based driver gradually.
>
>This is putting a lot of churn and many patches on a critical driver
>(given this device is in a lot of laptops). As such I think it's fine
>to do prep for devm in separate changes, but the final per driver change
>should be one per driver to go fully devm rather than doing it in smaller
>chunks. I obviously want to see this well tested, and don't want to have
>to keep asking people to test 'the next' set of changes.
>
>Jonathan
>
Hi Jonathan,
I understood and agreed with you. To add devm API support we have common separate change and on top of while updating driver instead of updating a single devm, I shall update all possible driver to devm. I think we can keep this series for few time for the review and next series would incorporate fully devm change and also attempted to use b4 to handle long series.
Later in that we can plan to proceed for testing instead of partial devm driver series validation.
Thanks, Sanjay
>>
>> Welcoming all suggestions and especially if someone can validate and have Tested-by tag.
>>
>> Thank, Sanjay
>>
>> >
>> >> if (ret) {
>> >> dev_err(dev, "device register failed\n");
>> >> - goto error_remove_trigger;
>> >> + return ret;
>> >> }
>> >> press_state->callbacks.send_event = press_proc_event;
>> >> @@ -318,8 +318,6 @@ static int hid_press_probe(struct platform_device *pdev)
>> >> error_iio_unreg:
>> >> iio_device_unregister(indio_dev);
>> >> -error_remove_trigger:
>> >> - hid_sensor_remove_trigger(&press_state->common_attributes);
>> >> return ret;
>> >> }
>> >> @@ -328,11 +326,9 @@ static void hid_press_remove(struct platform_device *pdev)
>> >> {
>> >> struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
>> >> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> >> - struct press_state *press_state = iio_priv(indio_dev);
>> >> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
>> >> iio_device_unregister(indio_dev);
>> >> - hid_sensor_remove_trigger(&press_state->common_attributes);
>> >> }
>> >> static const struct platform_device_id hid_press_ids[] = {
>> >
>
^ permalink raw reply
* Re: [PATCH] HID: nintendo: add support for HORI Wireless Switch Pad
From: sashiko-bot @ 2026-05-26 17:52 UTC (permalink / raw)
To: Hector Zelaya; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260526-hori-support-v1-1-1861c0abc2e0@hectorzelaya.dev>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
New issues:
- [High] Unintentional probe failure due to uncleared error codes during non-fatal IMU and Rumble timeouts for licensed controllers.
- [High] Incompatible SPI flash data is erroneously read and parsed as IMU calibration.
- [Low] Confusing kernel warning printed when intentionally using default joystick calibration.
- [Low] `JOYCON_CTLR_TYPE_LIC_PRO` is missing from the `jc_type_has_right()` capability macro.
- [Low] Falsely advertising hardware capabilities (Rumble and IMU) that fail to initialize.
Pre-existing issues:
- [High] The input device is registered before its capabilities and force-feedback interfaces are configured.
- [High] Race condition in `joycon_ctlr_handle_event` reading shared state locklessly.
--
commit 6f0bfb377cdb5252c860b0efda09360159b62479
Author: Hector Zelaya <hector@hectorzelaya.dev>
HID: nintendo: add support for HORI Wireless Switch Pad
This commit adds support for the HORI Wireless Switch Pad by introducing
the new JOYCON_CTLR_TYPE_LIC_PRO controller type. It provides a dedicated
button mapping and handles initialization quirks for the device.
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index 29008c2cc5304..b5e799ace2492 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -316,6 +316,7 @@ enum joycon_ctlr_type {
> JOYCON_CTLR_TYPE_JCL = 0x01,
> JOYCON_CTLR_TYPE_JCR = 0x02,
> JOYCON_CTLR_TYPE_PRO = 0x03,
> + JOYCON_CTLR_TYPE_LIC_PRO = 0x06,
[Severity: Low]
Since JOYCON_CTLR_TYPE_LIC_PRO is a new controller type, does it need to be
added to the jc_type_has_right() macro?
Without it, the macro might evaluate to false, skipping the creation and
registration of the HOME LED device in joycon_leds_create().
[ ... ]
> @@ -2156,7 +2180,10 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
> joycon_config_left_stick(ctlr->input);
> joycon_config_right_stick(ctlr->input);
> joycon_config_dpad(ctlr->input);
> - joycon_config_buttons(ctlr->input, procon_button_mappings);
> + if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO)
> + joycon_config_buttons(ctlr->input, lic_procon_button_mappings);
> + else
> + joycon_config_buttons(ctlr->input, procon_button_mappings);
[Severity: High]
This is a pre-existing issue, but is the input device registered before its
capabilities and force-feedback interfaces are configured?
Looking at joycon_input_create(), input_register_device() is called
immediately after the input device is allocated:
joycon_input_create() {
...
ret = input_register_device(ctlr->input);
if (ret)
return ret;
if (joycon_type_is_right_joycon(ctlr)) {
...
}
After this registration, the code proceeds to configure axis limits, set up
button capability bits, and add the force-feedback rumble interface.
Modifying an input device's capabilities after it has been registered can
introduce race conditions where userspace reads an incomplete capability mask
before the driver finishes initializing it.
[ ... ]
> @@ -2503,13 +2530,30 @@ static int joycon_init(struct hid_device *hdev)
>
> if (joycon_has_joysticks(ctlr)) {
> /* get controller calibration data, and parse it */
> - ret = joycon_request_calibration(ctlr);
> - if (ret) {
> + if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO) {
> /*
> - * We can function with default calibration, but it may be
> - * inaccurate. Provide a warning, and continue on.
> + * Licensed controllers may have incompatible SPI flash
> + * layouts. Use default calibration values.
> */
> - hid_warn(hdev, "Analog stick positions may be inaccurate\n");
> + hid_info(hdev, "using default cal for licensed controller\n");
> + joycon_use_default_calibration(hdev,
> + &ctlr->left_stick_cal_x,
> + &ctlr->left_stick_cal_y,
> + "left", 0);
[Severity: Low]
Does calling joycon_use_default_calibration() with an error code of 0 cause a
confusing kernel warning to be logged on every successful connection?
joycon_use_default_calibration() unconditionally prints a warning:
joycon_use_default_calibration() {
...
hid_warn(hdev,
"Failed to read %s stick cal, using defaults; e=%d\n",
stick, ret);
...
}
This might falsely signal a hardware failure to users when the driver is
functioning exactly as intended.
> + joycon_use_default_calibration(hdev,
> + &ctlr->right_stick_cal_x,
> + &ctlr->right_stick_cal_y,
> + "right", 0);
> + } else {
[ ... ]
> @@ -2527,8 +2571,12 @@ static int joycon_init(struct hid_device *hdev)
> /* Enable the IMU */
> ret = joycon_enable_imu(ctlr);
> if (ret) {
> - hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> - goto out_unlock;
> + if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO) {
> + hid_dbg(hdev, "IMU enable failed for licensed controller, continuing\n");
[Severity: High]
The commit message states that SPI flash calibration data is incompatible,
but does this bypass joycon_request_imu_calibration() which is called right
before this block?
Because the controller type is treated as a Pro Controller, joycon_has_imu()
returns true, and the driver might issue an SPI read to the incompatible
flash layout, parsing garbage data into ctlr->accel_cal and ctlr->gyro_cal.
This could cause incorrect IMU divisor calculations and corrupted telemetry.
[Severity: Low]
While the initialization timeouts are treated as non-fatal, does the driver
still register the IMU device and force-feedback capabilities?
If joycon_has_imu() and joycon_has_rumble() still return true, userspace
applications might be falsely informed that the controller supports rumble
and motion tracking.
> + } else {
> + hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
> + goto out_unlock;
> + }
> }
> }
>
> @@ -2543,8 +2591,12 @@ static int joycon_init(struct hid_device *hdev)
> /* Enable rumble */
> ret = joycon_enable_rumble(ctlr);
> if (ret) {
> - hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> - goto out_unlock;
> + if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO) {
> + hid_dbg(hdev, "rumble enable failed for licensed controller, continuing\n");
[Severity: High]
Is it possible that ret remains set to -ETIMEDOUT after skipping the
goto out_unlock?
Since this is the last step in joycon_init(), the function might erroneously
return the error code when it reaches the end of the function, causing the
entire probe to fail for the newly added controller.
> + } else {
> + hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
> + goto out_unlock;
> + }
> }
> }
[Severity: High]
This is a pre-existing issue, but is there a race condition in
joycon_ctlr_handle_event() when reading shared state locklessly?
The interrupt handler joycon_ctlr_handle_event() checks
mutex_is_locked(&ctlr->output_mutex) and then locklessly reads ctlr->msg_type
and ctlr->subcmd_ack_match.
If a delayed response arrives from an old timed-out subcommand while a thread
is preparing to send a new subcommand, the IRQ handler might accept the old
packet if the subcommand IDs match. It then writes the old data to the buffer
and signals completion, prematurely waking the waitqueue and causing the new
subcommand attempt to consume stale data.
Could proper synchronization via ctlr->lock prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260526-hori-support-v1-1-1861c0abc2e0@hectorzelaya.dev?part=1
^ permalink raw reply
* Re: [PATCH] Input: rmi4 - release F54 queue on video registration failure
From: Dmitry Torokhov @ 2026-05-26 17:36 UTC (permalink / raw)
To: Myeonghun Pak, Hans Verkuil; +Cc: linux-input, linux-kernel, stable, Ijae Kim
In-Reply-To: <20260524182351.27658-1-mhun512@gmail.com>
On Mon, May 25, 2026 at 03:23:45AM +0900, Myeonghun Pak wrote:
> rmi_f54_probe() initializes the videobuf2 queue before registering the
> video device. If video_register_device() fails, probe only unregisters
> the V4L2 device and leaves the initialized queue unwound by neither
> remove nor file release paths.
>
> Release the queue before continuing through the existing probe error
> path.
>
> This issue was identified during our ongoing static-analysis research while
> reviewing kernel code.
>
> Fixes: 3a762dbd5347 ("[media] Input: synaptics-rmi4 - add support for F54 diagnostics")
> Cc: stable@vger.kernel.org
> Co-developed-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Ijae Kim <ae878000@gmail.com>
> Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
> ---
> drivers/input/rmi4/rmi_f54.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/input/rmi4/rmi_f54.c b/drivers/input/rmi4/rmi_f54.c
> index 61909e1a39..fca7b9fec5 100644
> --- a/drivers/input/rmi4/rmi_f54.c
> +++ b/drivers/input/rmi4/rmi_f54.c
> @@ -722,6 +722,7 @@ static int rmi_f54_probe(struct rmi_function *fn)
> ret = video_register_device(&f54->vdev, VFL_TYPE_TOUCH, -1);
> if (ret) {
> dev_err(&fn->dev, "Unable to register video subdevice.");
> + vb2_queue_release(&f54->queue);
> goto remove_v4l2;
> }
>
Hans, could you please Ack or Nak it? It is unclear to me if this
cleanup is mandatory and whether it is also needed in rmi_f54_remove().
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH] HID: nintendo: add support for HORI Wireless Switch Pad
From: Hector Zelaya @ 2026-05-26 17:08 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Daniel J. Ogorchock
Cc: linux-input, linux-kernel, Hector Zelaya
Add support for the HORI Wireless Switch Pad (vendor 0x0f0d, product
0x00f6), a licensed third-party Nintendo Switch Pro Controller.
The controller reports controller type 0x06 (vs 0x03 for first-party
Pro Controllers) and has the following quirks:
- SPI flash calibration data is incompatible; use default stick
calibration values instead.
- X and Y button bits are swapped compared to first-party controllers;
add a dedicated button mapping table.
- Rumble and IMU enable may timeout (no vibration motor in hardware);
treat as non-fatal for licensed controllers.
Tested over Bluetooth on NixOS with kernel 7.0.5 and 7.0.10:
- All 14 buttons map correctly
- Player LED sets on connect
- Sticks report correctly with default calibration
- IMU/gyro data streams at 60Hz
- D-pad reports on ABS_HAT0X/HAT0Y
Device information:
Bluetooth name: Lic Pro Controller
Bluetooth HID: 0005:0F0D:00F6
Assisted-by: Kiro:Auto [Amazon Kiro IDE]
Signed-off-by: Hector Zelaya <hector@hectorzelaya.dev>
---
drivers/hid/hid-ids.h | 3 ++
drivers/hid/hid-nintendo.c | 78 +++++++++++++++++++++++++++++++++++++++-------
2 files changed, 69 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index a1cfa436344a..3b0767cc47fd 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -683,6 +683,9 @@
#define USB_DEVICE_ID_HARMONIX_WII_RB3_KEYBOARD 0x3330
#define USB_DEVICE_ID_HARMONIX_WII_RB3_MPA_KEYBOARD_MODE 0x3338
+#define USB_VENDOR_ID_HORI 0x0f0d
+#define USB_DEVICE_ID_HORI_WIRELESS_SWITCH_PAD 0x00f6
+
#define USB_VENDOR_ID_HP 0x03f0
#define USB_PRODUCT_ID_HP_ELITE_PRESENTER_MOUSE_464A 0x464a
#define USB_PRODUCT_ID_HP_LOGITECH_OEM_USB_OPTICAL_MOUSE_0A4A 0x0a4a
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 29008c2cc530..b5e799ace249 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -316,6 +316,7 @@ enum joycon_ctlr_type {
JOYCON_CTLR_TYPE_JCL = 0x01,
JOYCON_CTLR_TYPE_JCR = 0x02,
JOYCON_CTLR_TYPE_PRO = 0x03,
+ JOYCON_CTLR_TYPE_LIC_PRO = 0x06,
JOYCON_CTLR_TYPE_NESL = 0x09,
JOYCON_CTLR_TYPE_NESR = 0x0A,
JOYCON_CTLR_TYPE_SNES = 0x0B,
@@ -433,6 +434,25 @@ static const struct joycon_ctlr_button_mapping procon_button_mappings[] = {
{ /* sentinel */ },
};
+/* Licensed Pro Controllers (e.g. HORI) swap X/Y bits in the report */
+static const struct joycon_ctlr_button_mapping lic_procon_button_mappings[] = {
+ { BTN_EAST, JC_BTN_A, },
+ { BTN_SOUTH, JC_BTN_B, },
+ { BTN_NORTH, JC_BTN_Y, },
+ { BTN_WEST, JC_BTN_X, },
+ { BTN_TL, JC_BTN_L, },
+ { BTN_TR, JC_BTN_R, },
+ { BTN_TL2, JC_BTN_ZL, },
+ { BTN_TR2, JC_BTN_ZR, },
+ { BTN_SELECT, JC_BTN_MINUS, },
+ { BTN_START, JC_BTN_PLUS, },
+ { BTN_THUMBL, JC_BTN_LSTICK, },
+ { BTN_THUMBR, JC_BTN_RSTICK, },
+ { BTN_MODE, JC_BTN_HOME, },
+ { BTN_Z, JC_BTN_CAP, },
+ { /* sentinel */ },
+};
+
static const struct joycon_ctlr_button_mapping nescon_button_mappings[] = {
{ BTN_SOUTH, JC_BTN_A, },
{ BTN_EAST, JC_BTN_B, },
@@ -695,7 +715,8 @@ static inline bool joycon_type_is_right_joycon(struct joycon_ctlr *ctlr)
static inline bool joycon_type_is_procon(struct joycon_ctlr *ctlr)
{
- return ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO;
+ return ctlr->ctlr_type == JOYCON_CTLR_TYPE_PRO ||
+ ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO;
}
static inline bool joycon_type_is_snescon(struct joycon_ctlr *ctlr)
@@ -1710,7 +1731,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
joycon_report_left_stick(ctlr, rep);
joycon_report_right_stick(ctlr, rep);
joycon_report_dpad(ctlr, rep);
- joycon_report_buttons(ctlr, rep, procon_button_mappings);
+ if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO)
+ joycon_report_buttons(ctlr, rep, lic_procon_button_mappings);
+ else
+ joycon_report_buttons(ctlr, rep, procon_button_mappings);
} else if (joycon_type_is_any_nescon(ctlr)) {
joycon_report_dpad(ctlr, rep);
joycon_report_buttons(ctlr, rep, nescon_button_mappings);
@@ -2156,7 +2180,10 @@ static int joycon_input_create(struct joycon_ctlr *ctlr)
joycon_config_left_stick(ctlr->input);
joycon_config_right_stick(ctlr->input);
joycon_config_dpad(ctlr->input);
- joycon_config_buttons(ctlr->input, procon_button_mappings);
+ if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO)
+ joycon_config_buttons(ctlr->input, lic_procon_button_mappings);
+ else
+ joycon_config_buttons(ctlr->input, procon_button_mappings);
} else if (joycon_type_is_any_nescon(ctlr)) {
joycon_config_dpad(ctlr->input);
joycon_config_buttons(ctlr->input, nescon_button_mappings);
@@ -2503,13 +2530,30 @@ static int joycon_init(struct hid_device *hdev)
if (joycon_has_joysticks(ctlr)) {
/* get controller calibration data, and parse it */
- ret = joycon_request_calibration(ctlr);
- if (ret) {
+ if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO) {
/*
- * We can function with default calibration, but it may be
- * inaccurate. Provide a warning, and continue on.
+ * Licensed controllers may have incompatible SPI flash
+ * layouts. Use default calibration values.
*/
- hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+ hid_info(hdev, "using default cal for licensed controller\n");
+ joycon_use_default_calibration(hdev,
+ &ctlr->left_stick_cal_x,
+ &ctlr->left_stick_cal_y,
+ "left", 0);
+ joycon_use_default_calibration(hdev,
+ &ctlr->right_stick_cal_x,
+ &ctlr->right_stick_cal_y,
+ "right", 0);
+ } else {
+ ret = joycon_request_calibration(ctlr);
+ if (ret) {
+ /*
+ * We can function with default calibration, but
+ * it may be inaccurate. Provide a warning, and
+ * continue on.
+ */
+ hid_warn(hdev, "Analog stick positions may be inaccurate\n");
+ }
}
}
@@ -2527,8 +2571,12 @@ static int joycon_init(struct hid_device *hdev)
/* Enable the IMU */
ret = joycon_enable_imu(ctlr);
if (ret) {
- hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
- goto out_unlock;
+ if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO) {
+ hid_dbg(hdev, "IMU enable failed for licensed controller, continuing\n");
+ } else {
+ hid_err(hdev, "Failed to enable the IMU; ret=%d\n", ret);
+ goto out_unlock;
+ }
}
}
@@ -2543,8 +2591,12 @@ static int joycon_init(struct hid_device *hdev)
/* Enable rumble */
ret = joycon_enable_rumble(ctlr);
if (ret) {
- hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
- goto out_unlock;
+ if (ctlr->ctlr_type == JOYCON_CTLR_TYPE_LIC_PRO) {
+ hid_dbg(hdev, "rumble enable failed for licensed controller, continuing\n");
+ } else {
+ hid_err(hdev, "Failed to enable rumble; ret=%d\n", ret);
+ goto out_unlock;
+ }
}
}
@@ -2813,6 +2865,8 @@ 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) },
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_HORI,
+ USB_DEVICE_ID_HORI_WIRELESS_SWITCH_PAD) },
{ }
};
MODULE_DEVICE_TABLE(hid, nintendo_hid_devices);
---
base-commit: e71bac24ec1f517f399a9eb471255b8f1c330b93
change-id: 20260526-hori-support-08b08bca40d8
Best regards,
--
Hector Zelaya <hector@hectorzelaya.dev>
^ permalink raw reply related
* Re: [PATCH v2] HID: appletb-kbd: add option to switch default layer on double pressing fn key
From: sashiko-bot @ 2026-05-26 16:51 UTC (permalink / raw)
To: Aditya Garg; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260526154954.29002-1-gargaditya08@proton.me>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Medium] Dynamic mutation of a read-only global module parameter from a per-device atomic event handler without synchronization.
- [Medium] Incorrect double-press detection during early system uptime on 32-bit systems due to jiffies wrap-around with a 0-initialized timestamp.
- [Low] Intervening non-Fn key presses do not reset the double-press detection timer.
Pre-existing issues:
- [High] Calling sleeping functions (`hid_hw_power` and `hid_hw_request`) from an atomic context causes a kernel panic.
- [High] A race condition during `probe()` allows incoming HID events to trigger a NULL pointer dereference.
--
commit 9461d5707b93702971195b71e23ddea06392c720
Author: Aditya Garg <gargaditya08@proton.me>
HID: appletb-kbd: add option to switch default layer on double pressing fn key
This patch enables a user to switch the default layer from media to fn
keys and vice-versa upon double pressing the fn key. This behaviour can
be configured using the double_press_switch_time module parameter.
> diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
> index 462010a758993..34bfc595949dc 100644
> --- a/drivers/hid/hid-appletb-kbd.c
> +++ b/drivers/hid/hid-appletb-kbd.c
[ ... ]
> @@ -243,6 +250,18 @@ static int appletb_kbd_hid_event(struct hid_device *hdev, struct hid_field *fiel
> return kbd->current_mode == APPLETB_KBD_MODE_OFF;
> }
>
[Severity: High]
This is a pre-existing issue, but does a race condition during probe() allow
incoming HID events to trigger a NULL pointer dereference?
If hid_hw_start() and hid_hw_open() are called before hid_set_drvdata() is
executed, an incoming keyboard event might invoke appletb_kbd_hid_event()
before probe() finishes. This would call hid_get_drvdata(hdev) returning
NULL, which is then passed to reset_inactivity_timer() and unconditionally
dereferenced.
> +static u8 appletb_switch_mode(u8 mode)
> +{
[ ... ]
> @@ -250,15 +269,42 @@ static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type
>
> reset_inactivity_timer(kbd);
>
> - if (type == EV_KEY && code == KEY_FN && appletb_tb_fn_toggle &&
> - (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
> - kbd->current_mode == APPLETB_KBD_MODE_FN)) {
> + if (type == EV_KEY && code == KEY_FN &&
> + (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
> + kbd->current_mode == APPLETB_KBD_MODE_FN)) {
> +
[Severity: Low]
Is it possible that intervening non-Fn key presses should reset the
double-press detection timer?
If a user presses Fn, types another key, and then presses Fn again within
the delay timeout, the driver might erroneously interpret this as a
double-press and switch the layer.
> if (value == 1) {
> - kbd->saved_mode = kbd->current_mode;
> - appletb_kbd_set_mode(kbd, kbd->current_mode == APPLETB_KBD_MODE_SPCL
> - ? APPLETB_KBD_MODE_FN : APPLETB_KBD_MODE_SPCL);
> + if (appletb_tb_double_press_switch_layers > 0) {
> + unsigned long now = jiffies;
> +
> + if (time_before(now, kbd->last_fn_press +
> + msecs_to_jiffies(appletb_tb_double_press_switch_layers))) {
[Severity: Medium]
Could this double-press detection trigger incorrectly during early system
uptime on 32-bit systems?
Since last_fn_press is zero-initialized and jiffies starts at a large
negative value, time_before() might evaluate to true for the first 5
minutes of uptime.
This would cause any single press of the Fn key to erroneously trigger a
double-press layer switch.
> +
> + appletb_tb_def_mode =
> + appletb_switch_mode(
> + appletb_tb_def_mode);
[Severity: Medium]
Is it safe to dynamically mutate the appletb_tb_def_mode module parameter
from a per-device atomic event handler without synchronization?
Since appletb_tb_def_mode is a global variable exported as read-only,
modifying it dynamically could violate its sysfs contract and introduce a
data race if multiple keyboards are used concurrently.
This would also improperly alter the default mode for all current and future
connected devices.
> +
> + appletb_kbd_set_mode(kbd,
> + appletb_tb_def_mode);
[Severity: High]
This is a pre-existing issue, but does calling appletb_kbd_set_mode() from
this atomic context cause a kernel panic?
The appletb_kbd_inp_event handler is invoked by the input subsystem with
the dev->event_lock spinlock held and interrupts disabled.
appletb_kbd_set_mode() calls hid_hw_power() and hid_hw_request(). For USB
HID devices, hid_hw_power() may sleep and will warn in atomic context,
potentially leading to a scheduling while atomic panic.
> +
> + kbd->saved_mode = appletb_tb_def_mode;
> + kbd->last_fn_press = 0;
> +
> + return;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260526154954.29002-1-gargaditya08@proton.me?part=1
^ permalink raw reply
* Re: [PATCH v4 00/36] HID: iio: basic clean up and introduce devm_ API for HID sensors
From: Jonathan Cameron @ 2026-05-26 15:56 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, sakari.ailus,
linux-input, linux-iio, linux-kernel
In-Reply-To: <20260524192059.1193716-1-sanjayembedded@gmail.com>
On Mon, 25 May 2026 00:50:23 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Key highlights:
> - 0000-0024: General cleanup and kernel few coding style fixes across HID IIO drivers
> - 0025: Remove unused iio_dev argument from HID IIO sensor helper
> - 0026: Introduce devm_hid_sensor_setup_trigger() device-managed API
> - 0027-0036: Convert HID IIO sensor drivers to use the new devm-based trigger setup
>
> changes in v4:
> - Extend the series to cover remaining HID IIO drivers with devm API usage
> - Reorder patches to place cleanup and warning fix at beginning and,
> devm-related changes toward the end based on feedback from David
> - v3 series -> https://lore.kernel.org/all/20260509101040.791404-1-sanjayembedded@gmail.com/
> changes in v3:
> - Added cleanup and prepratory changes before adding devm_ API
> conversion based on self review: 0002, 0004, 0006, 0007 and 0008
> - Address andy's review comment on commit message and coding style
> - v2 series -> https://lore.kernel.org/all/20260429175918.2541914-1-sanjayembedded@gmail.com/
> 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 for each patch in series
> - Build-tested on QEMU x86_64
>
> P.S.
> - Sashiko reported an issue in a different driver and noted that it is not
> introduced by this series. I have taken this feedback into account and
> will address the actual issue in a separate series focus on that driver.
> - Once this series is merged into the IIO tree, a number of HID IIO
> drivers will become available to fully converted to devm API usage.
> - The changes are organized across drivers to keep similar modifications
> grouped together for consistency, making the series easier to review,
> rather than grouping all changes per driver.
>
The series is mostly fine, though not sure what happened with how you sent
it as I have it in seperate threads of 10 patches at a time. If you are having
email troubles with big series, use b4 to send them instead (instructions on kernel.org)
My main concern is this is a lot of churn on a critical driver. I'd like therefore
to be able to see the full benefit of those devm patches rather than get us
part way as this does.
Jonathan
> Thanks,
> Sanjay Chitroda
>
> Sanjay Chitroda (36):
> iio: hid-sensors: add missing blank line after declarations
> iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
> iio: accel: hid-sensor-accel-3d: use u32 instead of unsigned
> iio: light: hid-sensor-als: use u32 instead of unsigned
> iio: light: hid-sensor-prox: use u32 instead of unsigned
> iio: orientation: hid-sensor-incl-3d: use u32 instead of unsigned
> iio: orientation: hid-sensor-rotation: use u32 instead of unsigned
> iio: pressure: hid-sensor-press: use u32 instead of unsigned
> iio: humidity: hid-sensor-humidity: align parenthesis for readability
> iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
> iio: magnetometer: hid-sensor-magn-3d: align parenthesis for
> readability
> iio: humidity: hid-sensor-humidity: use common device for devres
> iio: position: hid-sensor-custom-intel-hinge: use common device for
> devres
> iio: temperature: hid-sensor-temperature: use common device for devres
> iio: humidity: hid-sensor-humidity: use local struct device
> iio: gyro: hid-sensor-gyro-3d: use local struct device
> iio: accel: hid-sensor-accel-3d: use local struct device
> iio: light: hid-sensor-als: use local struct device
> iio: light: hid-sensor-prox: use local struct device
> iio: magnetometer: hid-sensor-magn-3d: use local struct device
> iio: orientation: hid-sensor-incl-3d: use local struct device
> iio: orientation: hid-sensor-rotation: use local struct device
> iio: position: hid-sensor-custom-intel-hinge: use local struct device
> iio: pressure: hid-sensor-press: use local struct device
> iio: hid-sensors: remove unused iio_dev argument
> iio: hid-sensors: introduce device managed API
> iio: gyro: hid-sensor-gyro-3d: drop hid_sensor_remove_trigger() using
> devm API
> iio: humidity: hid-sensor-humidity: drop hid_sensor_remove_trigger()
> using devm API
> iio: light: hid-sensor-prox: drop hid_sensor_remove_trigger() using
> devm API
> iio: light: hid-sensor-als: drop hid_sensor_remove_trigger() using
> devm API
> iio: magnetometer: hid-sensor-magn-3d: drop
> hid_sensor_remove_trigger() using devm API
> iio: orientation: hid-sensor-incl-3d: drop hid_sensor_remove_trigger()
> using devm API
> iio: orientation: hid-sensor-rotation: drop
> hid_sensor_remove_trigger() using devm API
> iio: position: hid-sensor-custom-intel-hinge: drop
> hid_sensor_remove_trigger() using devm API
> iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger()
> using devm API
> iio: temperature: hid-sensor-temperature: drop
> hid_sensor_remove_trigger() using devm API
>
> drivers/iio/accel/hid-sensor-accel-3d.c | 30 ++---
> .../common/hid-sensors/hid-sensor-trigger.c | 24 +++-
> .../common/hid-sensors/hid-sensor-trigger.h | 5 +-
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 96 ++++++++-------
> drivers/iio/humidity/hid-sensor-humidity.c | 61 +++++-----
> drivers/iio/light/hid-sensor-als.c | 31 +++--
> drivers/iio/light/hid-sensor-prox.c | 30 ++---
> drivers/iio/magnetometer/hid-sensor-magn-3d.c | 112 +++++++++---------
> drivers/iio/orientation/hid-sensor-incl-3d.c | 36 +++---
> drivers/iio/orientation/hid-sensor-rotation.c | 38 +++---
> .../position/hid-sensor-custom-intel-hinge.c | 27 ++---
> drivers/iio/pressure/hid-sensor-press.c | 36 +++---
> .../iio/temperature/hid-sensor-temperature.c | 15 +--
> 13 files changed, 264 insertions(+), 277 deletions(-)
>
>
> base-commit: 08297ca8422541dde6c8b7e6b1d68bd4aa4568ef
^ permalink raw reply
* Re: [PATCH v4 35/36] iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger() using devm API
From: Jonathan Cameron @ 2026-05-26 15:54 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Christophe JAILLET, jikos, srinivas.pandruvada, dlechner, nuno.sa,
andy, sakari.ailus, linux-input, linux-iio, linux-kernel
In-Reply-To: <39A229F0-436E-4AAC-82A4-9B04733E9EE3@gmail.com>
On Mon, 25 May 2026 22:35:50 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> On 25 May 2026 3:39:11 pm IST, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
> >Le 24/05/2026 à 21:29, Sanjay Chitroda a écrit :
> >> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >>
> >> Use devm_hid_sensor_setup_trigger() to automatically release resource
> >> during fail, unbind or removal of driver using devres framework.
> >>
> >> This simplify the setup, remove goto, avoid manual resource cleanup in
> >> teardown path.
> >>
> >> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> >> ---
> >> drivers/iio/pressure/hid-sensor-press.c | 10 +++-------
> >> 1 file changed, 3 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> >> index fee7dcb86801..8438498c9aeb 100644
> >> --- a/drivers/iio/pressure/hid-sensor-press.c
> >> +++ b/drivers/iio/pressure/hid-sensor-press.c
> >> @@ -291,8 +291,8 @@ static int hid_press_probe(struct platform_device *pdev)
> >> atomic_set(&press_state->common_attributes.data_ready, 0);
> >> - ret = hid_sensor_setup_trigger(indio_dev, name,
> >> - &press_state->common_attributes);
> >> + ret = devm_hid_sensor_setup_trigger(dev, indio_dev, name,
> >> + &press_state->common_attributes);
> >> if (ret) {
> >> dev_err(dev, "trigger setup failed\n");
> >> return ret;
> >> @@ -301,7 +301,7 @@ static int hid_press_probe(struct platform_device *pdev)
> >> ret = iio_device_register(indio_dev);
> >
> >Hi,
> >
> >Looks like devm_iio_device_register() would now make sense and simplify things even further.
> >
> >CJ
>
> Hi CJ,
>
> Yes, as mentioned in cover letter most of HID IIO drivers have potential and will converted to devm based driver gradually.
This is putting a lot of churn and many patches on a critical driver
(given this device is in a lot of laptops). As such I think it's fine
to do prep for devm in separate changes, but the final per driver change
should be one per driver to go fully devm rather than doing it in smaller
chunks. I obviously want to see this well tested, and don't want to have
to keep asking people to test 'the next' set of changes.
Jonathan
>
> Welcoming all suggestions and especially if someone can validate and have Tested-by tag.
>
> Thank, Sanjay
>
> >
> >> if (ret) {
> >> dev_err(dev, "device register failed\n");
> >> - goto error_remove_trigger;
> >> + return ret;
> >> }
> >> press_state->callbacks.send_event = press_proc_event;
> >> @@ -318,8 +318,6 @@ static int hid_press_probe(struct platform_device *pdev)
> >> error_iio_unreg:
> >> iio_device_unregister(indio_dev);
> >> -error_remove_trigger:
> >> - hid_sensor_remove_trigger(&press_state->common_attributes);
> >> return ret;
> >> }
> >> @@ -328,11 +326,9 @@ static void hid_press_remove(struct platform_device *pdev)
> >> {
> >> struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
> >> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> >> - struct press_state *press_state = iio_priv(indio_dev);
> >> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
> >> iio_device_unregister(indio_dev);
> >> - hid_sensor_remove_trigger(&press_state->common_attributes);
> >> }
> >> static const struct platform_device_id hid_press_ids[] = {
> >
^ permalink raw reply
* [PATCH v2] HID: appletb-kbd: add option to switch default layer on double pressing fn key
From: Aditya Garg @ 2026-05-26 15:50 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Linux Input Mailing List, Linux Kernel Mailing List, Aditya Garg
In-Reply-To: <20260521132447.19241-1-gargaditya08@proton.me>
From: Aditya Garg <gargaditya08@proton.me>
This patch enables a user to switch the default layer from media to fn
keys and vice-versa upon double pressing the fn key. This behaviour can
be configured using the double_press_switch_time module parameter whose
value depicts the time in milliseconds within which the fn key must be
pressed again to switch the default layer. If set to 0, it simply
disables this behaviour.
Signed-off-by: Aditya Garg <gargaditya08@proton.me>
---
v2: added a check to ensure negative fn switch times are ignored.
drivers/hid/hid-appletb-kbd.c | 60 +++++++++++++++++++++++++++++++----
1 file changed, 53 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-appletb-kbd.c b/drivers/hid/hid-appletb-kbd.c
index 462010a75899..34bfc595949d 100644
--- a/drivers/hid/hid-appletb-kbd.c
+++ b/drivers/hid/hid-appletb-kbd.c
@@ -56,6 +56,12 @@ static int appletb_tb_idle_timeout = 15;
module_param_named(idle_timeout, appletb_tb_idle_timeout, int, 0644);
MODULE_PARM_DESC(idle_timeout, "Idle timeout in sec");
+static int appletb_tb_double_press_switch_layers;
+module_param_named(double_press_switch_time,
+ appletb_tb_double_press_switch_layers, int, 0644);
+MODULE_PARM_DESC(double_press_switch_time, "Time in ms within which if fn key is double "
+ "pressed will switch layers");
+
struct appletb_kbd {
struct hid_field *mode_field;
struct input_handler inp_handler;
@@ -68,6 +74,7 @@ struct appletb_kbd {
bool has_turned_off;
u8 saved_mode;
u8 current_mode;
+ unsigned long last_fn_press;
};
static const struct key_entry appletb_kbd_keymap[] = {
@@ -243,6 +250,18 @@ static int appletb_kbd_hid_event(struct hid_device *hdev, struct hid_field *fiel
return kbd->current_mode == APPLETB_KBD_MODE_OFF;
}
+static u8 appletb_switch_mode(u8 mode)
+{
+ switch (mode) {
+ case APPLETB_KBD_MODE_SPCL:
+ return APPLETB_KBD_MODE_FN;
+ case APPLETB_KBD_MODE_FN:
+ return APPLETB_KBD_MODE_SPCL;
+ default:
+ return mode;
+ }
+}
+
static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type,
unsigned int code, int value)
{
@@ -250,15 +269,42 @@ static void appletb_kbd_inp_event(struct input_handle *handle, unsigned int type
reset_inactivity_timer(kbd);
- if (type == EV_KEY && code == KEY_FN && appletb_tb_fn_toggle &&
- (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
- kbd->current_mode == APPLETB_KBD_MODE_FN)) {
+ if (type == EV_KEY && code == KEY_FN &&
+ (kbd->current_mode == APPLETB_KBD_MODE_SPCL ||
+ kbd->current_mode == APPLETB_KBD_MODE_FN)) {
+
if (value == 1) {
- kbd->saved_mode = kbd->current_mode;
- appletb_kbd_set_mode(kbd, kbd->current_mode == APPLETB_KBD_MODE_SPCL
- ? APPLETB_KBD_MODE_FN : APPLETB_KBD_MODE_SPCL);
+ if (appletb_tb_double_press_switch_layers > 0) {
+ unsigned long now = jiffies;
+
+ if (time_before(now, kbd->last_fn_press +
+ msecs_to_jiffies(appletb_tb_double_press_switch_layers))) {
+
+ appletb_tb_def_mode =
+ appletb_switch_mode(
+ appletb_tb_def_mode);
+
+ appletb_kbd_set_mode(kbd,
+ appletb_tb_def_mode);
+
+ kbd->saved_mode = appletb_tb_def_mode;
+ kbd->last_fn_press = 0;
+
+ return;
+ }
+
+ kbd->last_fn_press = now;
+ }
+ if (appletb_tb_fn_toggle) {
+ kbd->saved_mode = kbd->current_mode;
+
+ appletb_kbd_set_mode(kbd,
+ appletb_switch_mode(kbd->current_mode));
+ }
+
} else if (value == 0) {
- if (kbd->saved_mode != kbd->current_mode)
+ if (appletb_tb_fn_toggle &&
+ kbd->saved_mode != kbd->current_mode)
appletb_kbd_set_mode(kbd, kbd->saved_mode);
}
}
Range-diff against v1:
1: 5c9d8c8ba8c0 ! 1: 4f6a818b98b9 HID: appletb-kbd: add option to switch default layer on double pressing fn key
@@ drivers/hid/hid-appletb-kbd.c: static void appletb_kbd_inp_event(struct input_ha
- kbd->saved_mode = kbd->current_mode;
- appletb_kbd_set_mode(kbd, kbd->current_mode == APPLETB_KBD_MODE_SPCL
- ? APPLETB_KBD_MODE_FN : APPLETB_KBD_MODE_SPCL);
-+ if (appletb_tb_double_press_switch_layers) {
++ if (appletb_tb_double_press_switch_layers > 0) {
+ unsigned long now = jiffies;
+
+ if (time_before(now, kbd->last_fn_press +
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v4 02/36] iio: gyro: hid-sensor-gyro-3d: use u32 instead of unsigned
From: Jonathan Cameron @ 2026-05-26 15:45 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, sakari.ailus,
linux-input, linux-iio, linux-kernel
In-Reply-To: <20260524192059.1193716-3-sanjayembedded@gmail.com>
On Mon, 25 May 2026 00:50:25 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Prefer 'u32' instead of bare 'unsigned' variable to improve code
> clarity and consistency with kernel style.
I think it is relevant that the definition of the callbacks is:
struct hid_sensor_hub_callbacks {
...
int (*capture_sample)(struct hid_sensor_hub_device *hsdev,
u32 usage_id, size_t raw_len, char *raw_data,
void *priv);
int (*send_event)(struct hid_sensor_hub_device *hsdev, u32 usage_id,
void *priv);
};
So in all cases we have a u32. Same applies to other calls the usage_id is passed to.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
> Changes in v4:
> - Use 'u32' instead of 'unsigned int' as driver is already uses that and respective prototype has same data type with input from David and Andy
> - Link to v3 -> https://lore.kernel.org/all/20260509101040.791404-5-sanjayembedded@gmail.com/
> ---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index c340cc899a7c..020812639947 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> @@ -187,7 +187,7 @@ static const struct iio_info gyro_3d_info = {
>
> /* Callback handler to send event after all samples are received and captured */
> static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> void *priv)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(priv);
> @@ -209,7 +209,7 @@ static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int gyro_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> - unsigned usage_id,
> + u32 usage_id,
> size_t raw_len, char *raw_data,
> void *priv)
> {
> @@ -244,7 +244,7 @@ static int gyro_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> static int gyro_3d_parse_report(struct platform_device *pdev,
> struct hid_sensor_hub_device *hsdev,
> struct iio_chan_spec *channels,
> - unsigned usage_id,
> + u32 usage_id,
> struct gyro_3d_state *st)
> {
> int ret;
^ permalink raw reply
* [old REGRESSION] brightness keys on HP Elitebook 840 G3
From: Marijn Schouten @ 2026-05-26 12:29 UTC (permalink / raw)
To: linux-input
LS (not subscribed please CC)
The last kernel version that the brightness keys on my HP Elitebook
840 G3 worked on was 4.19.208, and they did not work from 5.4.150 on.
I was not able to test any intermediate versions.
I'm currently on 5.15.208. I've been using sysfs as a workaround:
"echo 700 > /sys/class/backlight/intel_backlight/brightness".
I get this failing dmesg output when pressing brightness down and brightness up:
$ dmesg |tail -n 8
[ 682.502592] atkbd serio0: Unknown key pressed (translated set 2,
code 0x92 on isa0060/serio0).
[ 682.502614] atkbd serio0: Use 'setkeycodes e012 <keycode>' to make it known.
[ 682.584414] atkbd serio0: Unknown key released (translated set 2,
code 0x92 on isa0060/serio0).
[ 682.584435] atkbd serio0: Use 'setkeycodes e012 <keycode>' to make it known.
[ 683.334480] atkbd serio0: Unknown key pressed (translated set 2,
code 0x97 on isa0060/serio0).
[ 683.334501] atkbd serio0: Use 'setkeycodes e017 <keycode>' to make it known.
[ 683.449715] atkbd serio0: Unknown key released (translated set 2,
code 0x97 on isa0060/serio0).
[ 683.449737] atkbd serio0: Use 'setkeycodes e017 <keycode>' to make it known.
Should this be fixed in drivers/input/keyboard/atkbd.c? I tried:
--- drivers/input/keyboard/atkbd.c.orig 2026-05-26 13:34:12.275821222 +0200
+++ drivers/input/keyboard/atkbd.c 2026-05-26 13:40:52.309840298 +0200
@@ -1162,6 +1162,12 @@
atkbd->keycode[scancode] = KEY_HANJA;
__set_bit(scancode, atkbd->force_release_mask);
+ /* HP brightness keys mapping for translated mode */
+ if (atkbd->translated) {
+ atkbd->keycode[0x112] = KEY_BRIGHTNESSDOWN;
+ atkbd->keycode[0x117] = KEY_BRIGHTNESSUP;
+ }
+
/*
* Perform additional fixups
*/
but it did not work...
With kind regards,
Marijn (not subscribed please CC)
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: hid-over-i2c: Add reset-gpios property
From: Hans de Goede @ 2026-05-26 8:55 UTC (permalink / raw)
To: Pradyot Kumar Nayak, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Abel Vesa, Dmitry Baryshkov,
Benjamin Tissoires, Jiri Kosina
Cc: linux-arm-msm, linux-input, Konrad Dybcio, devicetree,
linux-kernel,
20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398
In-Reply-To: <20260523-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-v2-1-5c333051e5bb@oss.qualcomm.com>
Hi,
On 22-May-26 21:55, Pradyot Kumar Nayak wrote:
> Document reset-gpios property for devices that explicitly require
> the reset line to be de-asserted during power up.
>
> Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> index 138caad96a29..fe0ec4fce673 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> @@ -50,6 +50,10 @@ properties:
> description: Time required by the device after enabling its regulators
> or powering it on, before it is ready for communication.
>
> + reset-gpios:
> + maxItems: 1
> + description: GPIO line used to reset the device.
> +
For I2C HID devices which need a reset, we typically also
need reset timings, trying to do I2C transfers to the device directly
after the reset has been de-asserted typically will not work and
some touchscreen-model specific delay is necessary.
So far in the DT world this has been handled by touchscreen
vendor specific bindings:
Documentation/devicetree/bindings/input/elan,ekth6915.yaml
Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
Which are all 3 handled by:
drivers/hid/i2c-hid/i2c-hid-of-elan.c
Which has per compatible match-data with the reset timings.
You should probably extend drivers/hid/i2c-hid/i2c-hid-of-elan.c
with support for the CRD touchscreen and either add a new
compatible to one of the existing 3 bindings; or if the touchscreen
controller IC is from a vendor without a binding yet maybe add
a new bindings file?
There also is:
Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
which is handled by:
drivers/hid/i2c-hid/i2c-hid-of-goodix.c
and is special in its own way.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: hid-over-i2c: Add reset-gpios property
From: Dmitry Baryshkov @ 2026-05-25 19:22 UTC (permalink / raw)
To: Pradyot Kumar Nayak
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Abel Vesa, Benjamin Tissoires, Jiri Kosina,
linux-arm-msm, linux-input, Konrad Dybcio, devicetree,
linux-kernel,
20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398
In-Reply-To: <20260523-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-v2-1-5c333051e5bb@oss.qualcomm.com>
On Sat, May 23, 2026 at 01:25:37AM +0530, Pradyot Kumar Nayak wrote:
> Document reset-gpios property for devices that explicitly require
> the reset line to be de-asserted during power up.
>
> Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> index 138caad96a29..fe0ec4fce673 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> @@ -50,6 +50,10 @@ properties:
> description: Time required by the device after enabling its regulators
> or powering it on, before it is ready for communication.
>
> + reset-gpios:
> + maxItems: 1
> + description: GPIO line used to reset the device.
> +
If you are extending the schema, where are the driver changes?
> touchscreen-inverted-x: true
>
> touchscreen-inverted-y: true
>
> --
> 2.43.0
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: sync wheel multiplier on wheel mode changes
From: Lauri Saurus @ 2026-05-25 18:29 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260518201000.BBD71C2BCB7@smtp.kernel.org>
> > +static int hidpp20_hires_wheel_raw_event(struct hidpp_device *hidpp,
> > + u8 *data, int size)
> > +{
> > + if (hidpp->hires_wheel_feature_index == 0xff)
> > + return 0;
> > +
> > + if (size < 5)
> > + return 0;
> > +
> > + if (data[0] != REPORT_ID_HIDPP_LONG ||
> > + data[2] != hidpp->hires_wheel_feature_index)
> > + return 0;
>
> Does this code need to handle VERY_LONG reports as well?
>
> Userspace tools or newer devices might transmit FAP commands using
> REPORT_ID_HIDPP_VERY_LONG (0x12). If the request is a VERY_LONG report, the
> response will also be VERY_LONG, and this check would drop it, failing to
> update the multiplier.
I agree, it could be possible some hosts could send the command as VERY_LONG report for newer devices which support that. I will change this to support VERY_LONG reports also.
>
> > +
> > + if ((data[3] & 0xf0) == CMD_HIRES_WHEEL_SET_WHEEL_MODE) {
> > + u8 mode = data[4];
> > + bool hires = (mode & 0x02) != 0;
>
> Does the HID++ 2.0 protocol guarantee that SET command responses will echo
> the requested mode in the payload?
>
> In the FAP protocol, responses to SET commands often return an empty
> (zero-filled) payload. If the device returns a zero payload, data[4] would
> be 0x00. The code would then evaluate hires = false and forcefully set the
> multiplier to 1, even if high-resolution mode was successfully requested.
As per the "Protocol HID++2.0 essential features" section in https://lekensteyn.nl/files/logitech/logitech_hidpp_2.0_specification_draft_2012-06-04.pdf:
"
All functions must respect the following rule:
All parameters in a request must always be repeated in the response:
Any parameter that is fully supported must be repeated "as is".
Any parameter that is only "partially supported", must be returned as supported.
The following examples should help to better understand these requirement:
A read command where an address (i.e., memory address, register address, etc.) is given as parameter must return the address and the data in the response (and not just the address).
A write command which sets a collection of bits or bit fields, should return the same value where all unsupported bits and bit fields have been masked to their default values (usually 0).
A command such as open-lock, erase-memory, etc. should return its parameters unchanged.
Note that there is no requirement to implement partial support. Each feature designer is free to decide if "partially correct" parameters
should return and error or be "partially supported." However, if partial support is implemented, then the parameters must be returned
as supported.
"
Based on the aforementioned specification, all parameters must be echoed in the response. If the SetWheelMode would not support setting hires parameter (partial support), then hires parameter would be returned as default value 0 (low res) in the response. In that case, it is correct to set the multiplier to 1, since setting hires was not supported.
Unless there are verified cases of HID++ capable mouses behaving as described in the review comment, or knowledge of newer version of the specification to have changed aforementioned rule, I would assume all devices to work according to the specification.
>
> > + int new_multiplier = (hires && hidpp->hires_wheel_multiplier > 0)
> > + ? hidpp->hires_wheel_multiplier : 1;
> > + hidpp->vertical_wheel_counter.wheel_multiplier = new_multiplier;
>
> Is there a potential data race on wheel_multiplier and
> hires_wheel_multiplier here?
>
> Looking at hi_res_scroll_enable(), it writes to these variables from
> process context via the workqueue. Concurrently, this event handler reads
> and writes to them from the HID raw event delivery path in interrupt context.
> There does not appear to be any synchronization protecting these shared
> variables across contexts.
>
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
>
I agree that in theory there is a possible race condition. However, wheel_multiplier was already written to from hi_res_scroll_enable, and read from the HID event handler before this patch. Not sure if this patch should address this issue or not.
^ permalink raw reply
* Re: [PATCH v4 35/36] iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger() using devm API
From: Sanjay Chitroda @ 2026-05-25 17:05 UTC (permalink / raw)
To: Christophe JAILLET, jikos, jic23, srinivas.pandruvada
Cc: dlechner, nuno.sa, andy, sakari.ailus, linux-input, linux-iio,
linux-kernel
In-Reply-To: <0a8ebbd2-5de6-48d2-9218-f7b816eb3455@wanadoo.fr>
On 25 May 2026 3:39:11 pm IST, Christophe JAILLET <christophe.jaillet@wanadoo.fr> wrote:
>Le 24/05/2026 à 21:29, Sanjay Chitroda a écrit :
>> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>>
>> Use devm_hid_sensor_setup_trigger() to automatically release resource
>> during fail, unbind or removal of driver using devres framework.
>>
>> This simplify the setup, remove goto, avoid manual resource cleanup in
>> teardown path.
>>
>> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>> ---
>> drivers/iio/pressure/hid-sensor-press.c | 10 +++-------
>> 1 file changed, 3 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
>> index fee7dcb86801..8438498c9aeb 100644
>> --- a/drivers/iio/pressure/hid-sensor-press.c
>> +++ b/drivers/iio/pressure/hid-sensor-press.c
>> @@ -291,8 +291,8 @@ static int hid_press_probe(struct platform_device *pdev)
>> atomic_set(&press_state->common_attributes.data_ready, 0);
>> - ret = hid_sensor_setup_trigger(indio_dev, name,
>> - &press_state->common_attributes);
>> + ret = devm_hid_sensor_setup_trigger(dev, indio_dev, name,
>> + &press_state->common_attributes);
>> if (ret) {
>> dev_err(dev, "trigger setup failed\n");
>> return ret;
>> @@ -301,7 +301,7 @@ static int hid_press_probe(struct platform_device *pdev)
>> ret = iio_device_register(indio_dev);
>
>Hi,
>
>Looks like devm_iio_device_register() would now make sense and simplify things even further.
>
>CJ
Hi CJ,
Yes, as mentioned in cover letter most of HID IIO drivers have potential and will converted to devm based driver gradually.
Welcoming all suggestions and especially if someone can validate and have Tested-by tag.
Thank, Sanjay
>
>> if (ret) {
>> dev_err(dev, "device register failed\n");
>> - goto error_remove_trigger;
>> + return ret;
>> }
>> press_state->callbacks.send_event = press_proc_event;
>> @@ -318,8 +318,6 @@ static int hid_press_probe(struct platform_device *pdev)
>> error_iio_unreg:
>> iio_device_unregister(indio_dev);
>> -error_remove_trigger:
>> - hid_sensor_remove_trigger(&press_state->common_attributes);
>> return ret;
>> }
>> @@ -328,11 +326,9 @@ static void hid_press_remove(struct platform_device *pdev)
>> {
>> struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
>> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
>> - struct press_state *press_state = iio_priv(indio_dev);
>> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
>> iio_device_unregister(indio_dev);
>> - hid_sensor_remove_trigger(&press_state->common_attributes);
>> }
>> static const struct platform_device_id hid_press_ids[] = {
>
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: input: hid-over-i2c: Add reset-gpios property
From: Conor Dooley @ 2026-05-25 16:24 UTC (permalink / raw)
To: Pradyot Kumar Nayak
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Abel Vesa, Dmitry Baryshkov, Benjamin Tissoires,
Jiri Kosina, linux-arm-msm, linux-input, Konrad Dybcio,
devicetree, linux-kernel,
20260519-glymur-mahua-common-nodes-v2-1-9f1b89a3f398
In-Reply-To: <20260523-arm64-dts-glymur-crd-add-reset-gpio-to-touchscreen-v2-1-5c333051e5bb@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 1289 bytes --]
On Sat, May 23, 2026 at 01:25:37AM +0530, Pradyot Kumar Nayak wrote:
> Document reset-gpios property for devices that explicitly require
> the reset line to be de-asserted during power up.
If there are devices that require this, it seems short-sighted to not
add their compatibles to hid-over-i2c.yaml and make the property
required for them.
pw-bot: changes-requested
Cheers,
Conor.
>
> Signed-off-by: Pradyot Kumar Nayak <pradyot.nayak@oss.qualcomm.com>
> ---
> Documentation/devicetree/bindings/input/hid-over-i2c.yaml | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> index 138caad96a29..fe0ec4fce673 100644
> --- a/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> +++ b/Documentation/devicetree/bindings/input/hid-over-i2c.yaml
> @@ -50,6 +50,10 @@ properties:
> description: Time required by the device after enabling its regulators
> or powering it on, before it is ready for communication.
>
> + reset-gpios:
> + maxItems: 1
> + description: GPIO line used to reset the device.
> +
> touchscreen-inverted-x: true
>
> touchscreen-inverted-y: true
>
> --
> 2.43.0
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH] Input: ims-pcu - fix use-after-free in probe error path
From: sashiko-bot @ 2026-05-25 16:06 UTC (permalink / raw)
To: Zhian Liang; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260525151410.42750-1-liangzhan5dev@gmail.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [Low] The patch introduces a useless `goto` statement immediately preceding its target label.
Pre-existing issues:
- [High] A malicious USB device can trigger a Double Free and Use-After-Free in the probe error path by spoofing the data interface class.
- [High] The driver blindly trusts the CDC Union descriptor's `bMasterInterface0` field, allowing a malicious device to corrupt other drivers' drvdata.
- [High] An infinite loop can occur in `ims_pcu_get_cdc_union_desc` if a USB descriptor has a `bLength` of 0.
- [High] The driver frees the `urb_in_buf` DMA coherent buffer using an incorrect size parameter.
- [High] The driver assumes the first endpoint of the control interface is an Interrupt IN endpoint without verifying its type or direction.
--
commit a298fe95c1b14555659ce23349aaf55e3bb035b5
Author: Zhian Liang <liangzhan5dev@gmail.com>
Input: ims-pcu - fix use-after-free in probe error path
This commit addresses a use-after-free in the probe error path by clearing
the control interface data to NULL before freeing the pcu structure. This
prevents the disconnect handler from accessing the freed pointer if the
device is removed.
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> index 4c022a36dbe84..fce3232ebf072 100644
> --- a/drivers/input/misc/ims-pcu.c
> +++ b/drivers/input/misc/ims-pcu.c
> @@ -2063,6 +2063,10 @@ static int ims_pcu_probe(struct usb_interface *intf,
> ims_pcu_buffers_free(pcu);
[Severity: High]
This is a pre-existing issue, but does this function properly free the
urb_in_buf?
Looking at ims_pcu_buffers_free(), it seems to use max_out_size instead of
max_in_size:
ims_pcu_buffers_free() {
...
usb_free_coherent(pcu->udev, pcu->max_out_size,
pcu->urb_in_buf, pcu->read_dma);
...
}
Could a device with mismatched IN and OUT endpoint descriptors cause an
improper free size and corrupt the DMA allocator?
> err_unclaim_intf:
> usb_driver_release_interface(&ims_pcu_driver, pcu->data_intf);
[Severity: High]
This is a pre-existing issue, but can a device spoofing the data interface
class trigger a double free here?
If a device presents a data interface with bInterfaceClass == USB_CLASS_COMM,
ims_pcu_disconnect() might fail to skip it. When
usb_driver_release_interface() is called on the data interface during a
probe failure, it synchronously unbinds the interface, triggering
ims_pcu_disconnect().
Could ims_pcu_disconnect() then incorrectly assume it is handling the
control interface and free the pcu structure, leading to a use-after-free
read on pcu->ctrl_intf and a double free on pcu in this error path?
> + goto err_clear_intfdata;
[Severity: Low]
Is this goto statement necessary? Execution would naturally fall through to
the err_clear_intfdata label on the very next line, making this instruction
redundant.
> +err_clear_intfdata:
> + if (pcu->ctrl_intf)
> + usb_set_intfdata(pcu->ctrl_intf, NULL);
[Severity: High]
This is a pre-existing issue, but does the driver safely assign
pcu->ctrl_intf?
In ims_pcu_parse_cdc_data(), pcu->ctrl_intf is assigned using the
bMasterInterface0 field from the CDC Union descriptor. If a device sets
bMasterInterface0 to point to a different, already-bound interface, could
the probe function unconditionally overwrite that victim interface's drvdata?
If the probe subsequently fails, this error path would clear the victim's
drvdata to NULL, which might cause the victim driver to crash when
disconnected.
> err_free_mem:
> kfree(pcu);
> return error;
[Severity: High]
This is a pre-existing issue, but could there be an infinite loop during
descriptor parsing?
If a device provides a CDC union descriptor with a bLength of 0, the size
check in ims_pcu_get_cdc_union_desc() would pass since 0 is not greater
than the remaining length. The loop would then subtract 0 from buflen and
add 0 to buf, causing it to never advance or terminate. Would this hang
the USB enumeration thread?
[Severity: High]
This is a pre-existing issue, but does ims_pcu_parse_cdc_data() verify the
endpoint direction and type before using it?
The code assigns pcu->ep_ctrl to &alt->endpoint[0].desc and passes its
address to usb_fill_int_urb(). If a device exposes a Bulk or OUT endpoint
at this index instead, could submitting it as an Interrupt IN URB lead to
unhandled behavior or warnings in host controller drivers?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525151410.42750-1-liangzhan5dev@gmail.com?part=1
^ permalink raw reply
* [PATCH] Input: ims-pcu - fix use-after-free in probe error path
From: Zhian Liang @ 2026-05-25 15:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, stable, Zhian Liang
If the driver fails during init (e.g. in ims_pcu_init_application_mode),the error path frees the pcu struct without clearing the interface data.
If the device is disconnected while in this state, the disconnect handler will retrieve the stale pointer from
usb_get_intfdata() and trigger a use-after-free
Fix this by setting the interface data to NULL in the probe before freeing the pcu struct.
Fixes: 628329d52474 ("Input: add IMS Passenger Control Unit driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Zhian Liang <liangzhan5dev@gmail.com>
---
drivers/input/misc/ims-pcu.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index 4c022a36dbe8..fce3232ebf07 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -2063,6 +2063,10 @@ static int ims_pcu_probe(struct usb_interface *intf,
ims_pcu_buffers_free(pcu);
err_unclaim_intf:
usb_driver_release_interface(&ims_pcu_driver, pcu->data_intf);
+ goto err_clear_intfdata;
+err_clear_intfdata:
+ if (pcu->ctrl_intf)
+ usb_set_intfdata(pcu->ctrl_intf, NULL);
err_free_mem:
kfree(pcu);
return error;
--
2.34.1
^ permalink raw reply related
* Re: [syzbot] [fs?] [usb?] [input?] INFO: rcu detected stall in __fsnotify_parent (3)
From: Jan Kara @ 2026-05-25 14:47 UTC (permalink / raw)
To: syzbot
Cc: amir73il, jack, linux-fsdevel, linux-input, linux-kernel,
linux-usb, syzkaller-bugs
In-Reply-To: <6a0cce0d.170a0220.3cbd30.0138.GAE@google.com>
Hello!
On Tue 19-05-26 13:54:37, syzbot wrote:
> syzbot found the following issue on:
>
> HEAD commit: 6916d5703ddf Merge tag 'drm-fixes-2026-05-16' of https://g..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=13437cc8580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=59da38148f3a3d24
> dashboard link: https://syzkaller.appspot.com/bug?extid=27ce6cc06c1311c152f5
> compiler: gcc (Debian 14.2.0-19) 14.2.0, GNU ld (GNU Binutils for Debian) 2.44
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=123e8596580000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/637181391a5c/disk-6916d570.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/bfbefc8a3671/vmlinux-6916d570.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/a419831b32a6/bzImage-6916d570.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+27ce6cc06c1311c152f5@syzkaller.appspotmail.com
>
> rcu: INFO: rcu_preempt detected stalls on CPUs/tasks:
> rcu: Tasks blocked on level-0 rcu_node (CPUs 0-1): P4963/1:b..l
>
> rcu: (detected by 1, t=10506 jiffies, g=15565, q=1900 ncpus=2)
> task:acpid state:R
> running task stack:22856 pid:4963 tgid:4963 ppid:1 task_flags:0x400040 flags:0x00080001
FWIW all the reproducer does is some games with USB, no signs of fsnotify.
Bisection has failed, the problem was hit only once more than a week ago.
I'm going to sign this off to some random fluctuation.
Honza
> Call Trace:
> <TASK>
> context_switch kernel/sched/core.c:5388 [inline]
> __schedule+0x1295/0x67a0 kernel/sched/core.c:7189
> preempt_schedule_irq+0x50/0x90 kernel/sched/core.c:7513
> irqentry_exit_to_kernel_mode_preempt include/linux/irq-entry-common.h:476 [inline]
> irqentry_exit_to_kernel_mode include/linux/irq-entry-common.h:547 [inline]
> irqentry_exit+0x205/0x7e0 kernel/entry/common.c:164
> asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:697
> RIP: 0010:lock_acquire+0x5e/0x370 kernel/locking/lockdep.c:5872
> Code: 05 3b a2 26 12 83 f8 07 0f 87 d9 02 00 00 48 0f a3 05 46 41 f3 0e 0f 82 a4 02 00 00 8b 35 ce 74 f3 0e 85 f6 0f 85 bf 00 00 00 <48> 8b 44 24 30 65 48 2b 05 dd a1 26 12 0f 85 ed 02 00 00 48 83 c4
> RSP: 0018:ffffc9000342fb30 EFLAGS: 00000206
>
> RAX: 0000000000000046 RBX: 0000000000000000 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff8defcf06 RDI: ffffffff8c1c3a00
> RBP: ffffffff8e7e57a0 R08: 0000000086db7919 R09: 0000000000000007
> R10: 0000000000000200 R11: 0000000000000000 R12: 0000000000000002
> R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
> rcu_lock_acquire include/linux/rcupdate.h:300 [inline]
> rcu_read_lock include/linux/rcupdate.h:838 [inline]
> dput.part.0+0x4c/0x570 fs/dcache.c:971
> dput+0x1f/0x30 fs/dcache.c:968
> __fsnotify_parent+0x694/0xca0 fs/notify/fsnotify.c:242
> fsnotify_parent include/linux/fsnotify.h:96 [inline]
> fsnotify_path include/linux/fsnotify.h:113 [inline]
> fsnotify_file include/linux/fsnotify.h:127 [inline]
> fsnotify_file include/linux/fsnotify.h:116 [inline]
> fsnotify_access include/linux/fsnotify.h:425 [inline]
> vfs_read+0x486/0xb30 fs/read_write.c:578
> ksys_read+0x1f8/0x250 fs/read_write.c:717
> do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
> do_syscall_64+0x10b/0xf80 arch/x86/entry/syscall_64.c:94
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7feac7fb3407
> RSP: 002b:00007ffc6dd51e00 EFLAGS: 00000202
> ORIG_RAX: 0000000000000000
> RAX: ffffffffffffffda RBX: 00007feac7f29780 RCX: 00007feac7fb3407
> RDX: 0000000000000018 RSI: 00007ffc6dd51e50 RDI: 000000000000000b
> RBP: 00007ffc6dd51e50 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000202 R12: 0000556e2f8031e4
> R13: 0000556e4a9f2380 R14: 0000000000000007 R15: 000000000000000b
> </TASK>
> rcu: rcu_preempt kthread starved for 4298 jiffies! g15565 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x0 ->cpu=1
> rcu: Unless rcu_preempt kthread gets sufficient CPU time, OOM is now expected behavior.
> rcu: RCU grace-period kthread stack dump:
> task:rcu_preempt state:R
> running task stack:27992 pid:16 tgid:16 ppid:2 task_flags:0x208040 flags:0x00080000
> Call Trace:
> <TASK>
> context_switch kernel/sched/core.c:5388 [inline]
> __schedule+0x1295/0x67a0 kernel/sched/core.c:7189
> __schedule_loop kernel/sched/core.c:7268 [inline]
> schedule+0xdd/0x390 kernel/sched/core.c:7283
> schedule_timeout+0x127/0x280 kernel/time/sleep_timeout.c:99
> rcu_gp_fqs_loop+0x1a9/0x900 kernel/rcu/tree.c:2095
> rcu_gp_kthread+0x179/0x230 kernel/rcu/tree.c:2297
> kthread+0x370/0x450 kernel/kthread.c:436
> ret_from_fork+0x72b/0xd50 arch/x86/kernel/process.c:158
> ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
> </TASK>
> rcu: Stack dump where RCU GP kthread last ran:
> CPU: 1 UID: 0 PID: 5978 Comm: syz.1.18 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
> RIP: 0010:write_comp_data+0x40/0x90 kernel/kcov.c:246
> Code: 02 12 a9 00 01 ff 00 74 1b f6 c4 01 74 07 a9 00 00 ff 00 74 05 c3 cc cc cc cc 8b 87 d4 16 00 00 85 c0 74 f1 8b 87 b0 16 00 00 <83> f8 03 75 e6 48 8b 87 b8 16 00 00 8b bf b4 16 00 00 48 8b 30 48
> RSP: 0018:ffffc900024af648 EFLAGS: 00000246
>
> RAX: 0000000000000000 RBX: 0000000000000000 RCX: ffffffff81fc2bcd
> RDX: 0000000000000001 RSI: 0000000000000000 RDI: ffff88805a5e0000
> RBP: ffff8880b8443320 R08: 0000000000000005 R09: 0000000000000000
> R10: 0000000000000001 R11: 0000000000000000 R12: 0000000000000003
> R13: ffffed1017088665 R14: 0000000000000001 R15: ffff8880b853c5c0
> FS: 0000000000000000(0000) GS:ffff88812446d000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f8c6e1b56e8 CR3: 0000000059a89000 CR4: 00000000003526f0
> Call Trace:
> <TASK>
> csd_lock_wait kernel/smp.c:342 [inline]
> smp_call_function_many_cond+0x5ad/0x1700 kernel/smp.c:892
> on_each_cpu_cond_mask+0x40/0x90 kernel/smp.c:1057
> __flush_tlb_multi arch/x86/include/asm/paravirt.h:46 [inline]
> flush_tlb_multi arch/x86/mm/tlb.c:1361 [inline]
> flush_tlb_mm_range+0x45f/0x16f0 arch/x86/mm/tlb.c:1451
> tlb_flush arch/x86/include/asm/tlb.h:23 [inline]
> tlb_flush_mmu_tlbonly include/asm-generic/tlb.h:509 [inline]
> tlb_flush_mmu_tlbonly include/asm-generic/tlb.h:499 [inline]
> tlb_flush_mmu mm/mmu_gather.c:423 [inline]
> tlb_finish_mmu+0x3fe/0x810 mm/mmu_gather.c:549
> exit_mmap+0x454/0xa10 mm/mmap.c:1313
> __mmput+0x12a/0x410 kernel/fork.c:1178
> mmput+0x67/0x80 kernel/fork.c:1201
> exit_mm kernel/exit.c:582 [inline]
> do_exit+0x8b2/0x2af0 kernel/exit.c:964
> do_group_exit+0xd5/0x2a0 kernel/exit.c:1119
> get_signal+0x20ff/0x2210 kernel/signal.c:3037
> arch_do_signal_or_restart+0x91/0x7e0 arch/x86/kernel/signal.c:337
> __exit_to_user_mode_loop kernel/entry/common.c:64 [inline]
> exit_to_user_mode_loop+0x8b/0x4f0 kernel/entry/common.c:98
> __exit_to_user_mode_prepare include/linux/irq-entry-common.h:207 [inline]
> syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:238 [inline]
> syscall_exit_to_user_mode include/linux/entry-common.h:318 [inline]
> do_syscall_64+0x706/0xf80 arch/x86/entry/syscall_64.c:100
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f7a9eb5d68e
> Code: Unable to access opcode bytes at 0x7f7a9eb5d664.
> RSP: 002b:00007fffdcda5708 EFLAGS: 00000246
> ORIG_RAX: 00000000000000e6
> RAX: 0000000000000000 RBX: 000055558fc36500 RCX: 00007f7a9eb5d68e
> RDX: 00007fffdcda5760 RSI: 0000000000000000 RDI: 0000000000000000
> RBP: 0000000000000001 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 00007fffdcda58b0
> R13: 00007f7a9ee15fac R14: 000000000002bbe6 R15: 00007f7a9ee15fa0
> </TASK>
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
^ permalink raw reply
* Re: [PATCH v5 02/11] dt-bindings: hwmon: Add Apple System Management Controller hwmon schema
From: Guenter Roeck @ 2026-05-25 13:33 UTC (permalink / raw)
To: James Calligeros
Cc: Sven Peter, Janne Grunau, Alyssa Rosenzweig, Neal Gompa,
Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Alexandre Belloni, Jean Delvare, Dmitry Torokhov, Jonathan Corbet,
asahi, linux-arm-kernel, devicetree, linux-kernel, linux-rtc,
linux-hwmon, linux-input, linux-doc
In-Reply-To: <20251112-macsmc-subdevs-v5-2-728e4b91fe81@gmail.com>
On Wed, Nov 12, 2025 at 09:16:48PM +1000, James Calligeros wrote:
> Apple Silicon devices integrate a vast array of sensors, monitoring
> current, power, temperature, and voltage across almost every part of
> the system. The sensors themselves are all connected to the System
> Management Controller (SMC). The SMC firmware exposes the data
> reported by these sensors via its standard FourCC-based key-value
> API. The SMC is also responsible for monitoring and controlling any
> fans connected to the system, exposing them in the same way.
>
> For reasons known only to Apple, each device exposes its sensors with
> an almost totally unique set of keys. This is true even for devices
> which share an SoC. An M1 Mac mini, for example, will report its core
> temperatures on different keys to an M1 MacBook Pro. Worse still, the
> SMC does not provide a way to enumerate the available keys at runtime,
> nor do the keys follow any sort of reasonable or consistent naming
> rules that could be used to deduce their purpose. We must therefore
> know which keys are present on any given device, and which function
> they serve, ahead of time.
>
> Add a schema so that we can describe the available sensors for a given
> Apple Silicon device in the Devicetree.
>
> Reviewed-by: Neal Gompa <neal@gompa.dev>
> Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
> Signed-off-by: James Calligeros <jcalligeros99@gmail.com>
> ---
> .../bindings/hwmon/apple,smc-hwmon.yaml | 86 +++++++++++++++++++++++++
> .../bindings/mfd/apple,smc.yaml | 36 +++++++++++
I see that the rest of this series is going to be applied. This patch
touches bindings in mfd, which I can not apply.
With the assumption that the patch will be applied through some other tree,
presumably mfd:
Acked-by: Guenter Roeck <linux@roeck-us.net>
Guenter
> MAINTAINERS | 1 +
> 3 files changed, 123 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> new file mode 100644
> index 000000000000..2eec317bc4b3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/hwmon/apple,smc-hwmon.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Apple SMC Hardware Monitoring
> +
> +description:
> + Apple's System Management Controller (SMC) exposes a vast array of
> + hardware monitoring sensors, including temperature probes, current and
> + voltage sense, power meters, and fan speeds. It also provides endpoints
> + to manually control the speed of each fan individually. Each Apple
> + Silicon device exposes a different set of endpoints via SMC keys. This
> + is true even when two machines share an SoC. The CPU core temperature
> + sensor keys on an M1 Mac mini are different to those on an M1 MacBook
> + Pro, for example.
> +
> +maintainers:
> + - James Calligeros <jcalligeros99@gmail.com>
> +
> +$defs:
> + sensor:
> + type: object
> +
> + properties:
> + apple,key-id:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: The SMC FourCC key of the desired sensor.
> + Must match the node's suffix.
> +
> + label:
> + description: Human-readable name for the sensor
> +
> + required:
> + - apple,key-id
> +
> +properties:
> + compatible:
> + const: apple,smc-hwmon
> +
> +patternProperties:
> + "^current-[A-Za-z0-9]{4}$":
> + $ref: "#/$defs/sensor"
> + unevaluatedProperties: false
> +
> + "^fan-[A-Za-z0-9]{4}$":
> + $ref: "#/$defs/sensor"
> + unevaluatedProperties: false
> +
> + properties:
> + apple,fan-minimum:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: SMC key containing the fan's minimum speed
> +
> + apple,fan-maximum:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: SMC key containing the fan's maximum speed
> +
> + apple,fan-target:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: Writeable endpoint for setting desired fan speed
> +
> + apple,fan-mode:
> + $ref: /schemas/types.yaml#/definitions/string
> + pattern: "^[A-Za-z0-9]{4}$"
> + description: Writeable key to enable/disable manual fan control
> +
> +
> + "^power-[A-Za-z0-9]{4}$":
> + $ref: "#/$defs/sensor"
> + unevaluatedProperties: false
> +
> + "^temperature-[A-Za-z0-9]{4}$":
> + $ref: "#/$defs/sensor"
> + unevaluatedProperties: false
> +
> + "^voltage-[A-Za-z0-9]{4}$":
> + $ref: "#/$defs/sensor"
> + unevaluatedProperties: false
> +
> +additionalProperties: false
> diff --git a/Documentation/devicetree/bindings/mfd/apple,smc.yaml b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> index 0410e712c900..34ce048619f5 100644
> --- a/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> +++ b/Documentation/devicetree/bindings/mfd/apple,smc.yaml
> @@ -49,6 +49,9 @@ properties:
> rtc:
> $ref: /schemas/rtc/apple,smc-rtc.yaml
>
> + hwmon:
> + $ref: /schemas/hwmon/apple,smc-hwmon.yaml
> +
> additionalProperties: false
>
> required:
> @@ -89,5 +92,38 @@ examples:
> nvmem-cells = <&rtc_offset>;
> nvmem-cell-names = "rtc_offset";
> };
> +
> + hwmon {
> + compatible = "apple,smc-hwmon";
> +
> + current-ID0R {
> + apple,key-id = "ID0R";
> + label = "AC Input Current";
> + };
> +
> + fan-F0Ac {
> + apple,key-id = "F0Ac";
> + apple,fan-minimum = "F0Mn";
> + apple,fan-maximum = "F0Mx";
> + apple,fan-target = "F0Tg";
> + apple,fan-mode = "F0Md";
> + label = "Fan 1";
> + };
> +
> + power-PSTR {
> + apple,key-id = "PSTR";
> + label = "Total System Power";
> + };
> +
> + temperature-TW0P {
> + apple,key-id = "TW0P";
> + label = "WiFi/BT Module Temperature";
> + };
> +
> + voltage-VD0R {
> + apple,key-id = "VD0R";
> + label = "AC Input Voltage";
> + };
> + };
> };
> };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 51942a9a9b43..6e5e219c5fe6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2422,6 +2422,7 @@ F: Documentation/devicetree/bindings/cpufreq/apple,cluster-cpufreq.yaml
> F: Documentation/devicetree/bindings/dma/apple,admac.yaml
> F: Documentation/devicetree/bindings/gpio/apple,smc-gpio.yaml
> F: Documentation/devicetree/bindings/gpu/apple,agx.yaml
> +F: Documentation/devicetree/bindings/hwmon/apple,smc-hwmon.yaml
> F: Documentation/devicetree/bindings/i2c/apple,i2c.yaml
> F: Documentation/devicetree/bindings/input/touchscreen/apple,z2-multitouch.yaml
> F: Documentation/devicetree/bindings/interrupt-controller/apple,*
^ permalink raw reply
* Re: [PATCH v4 35/36] iio: pressure: hid-sensor-press: drop hid_sensor_remove_trigger() using devm API
From: Christophe JAILLET @ 2026-05-25 10:09 UTC (permalink / raw)
To: Sanjay Chitroda, jikos, jic23, srinivas.pandruvada
Cc: dlechner, nuno.sa, andy, sakari.ailus, linux-input, linux-iio,
linux-kernel
In-Reply-To: <20260524192943.1196773-6-sanjayembedded@gmail.com>
Le 24/05/2026 à 21:29, Sanjay Chitroda a écrit :
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Use devm_hid_sensor_setup_trigger() to automatically release resource
> during fail, unbind or removal of driver using devres framework.
>
> This simplify the setup, remove goto, avoid manual resource cleanup in
> teardown path.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
> drivers/iio/pressure/hid-sensor-press.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index fee7dcb86801..8438498c9aeb 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c
> @@ -291,8 +291,8 @@ static int hid_press_probe(struct platform_device *pdev)
>
> atomic_set(&press_state->common_attributes.data_ready, 0);
>
> - ret = hid_sensor_setup_trigger(indio_dev, name,
> - &press_state->common_attributes);
> + ret = devm_hid_sensor_setup_trigger(dev, indio_dev, name,
> + &press_state->common_attributes);
> if (ret) {
> dev_err(dev, "trigger setup failed\n");
> return ret;
> @@ -301,7 +301,7 @@ static int hid_press_probe(struct platform_device *pdev)
> ret = iio_device_register(indio_dev);
Hi,
Looks like devm_iio_device_register() would now make sense and simplify
things even further.
CJ
> if (ret) {
> dev_err(dev, "device register failed\n");
> - goto error_remove_trigger;
> + return ret;
> }
>
> press_state->callbacks.send_event = press_proc_event;
> @@ -318,8 +318,6 @@ static int hid_press_probe(struct platform_device *pdev)
>
> error_iio_unreg:
> iio_device_unregister(indio_dev);
> -error_remove_trigger:
> - hid_sensor_remove_trigger(&press_state->common_attributes);
> return ret;
> }
>
> @@ -328,11 +326,9 @@ static void hid_press_remove(struct platform_device *pdev)
> {
> struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> - struct press_state *press_state = iio_priv(indio_dev);
>
> sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_PRESSURE);
> iio_device_unregister(indio_dev);
> - hid_sensor_remove_trigger(&press_state->common_attributes);
> }
>
> static const struct platform_device_id hid_press_ids[] = {
^ permalink raw reply
* Re: [PATCH 02/10] [v3] input: gpio-keys: make legacy gpiolib optional
From: Linus Walleij @ 2026-05-25 8:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-gpio, linux-kernel, Arnd Bergmann, Christian Lamparter,
Johannes Berg, Aaro Koskinen, Andreas Kemnade, Kevin Hilman,
Roger Quadros, Tony Lindgren, Thomas Bogendoerfer,
John Paul Adrian Glaubitz, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Bartosz Golaszewski, Dmitry Torokhov, Lee Jones, Pavel Machek,
Matti Vaittinen, Florian Fainelli, Jonas Gorski, Andrew Lunn,
Vladimir Oltean, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, linux-wireless, linux-omap, linux-arm-kernel,
linux-mips, linux-sh, linux-input, linux-leds, netdev
In-Reply-To: <20260520183815.2510387-3-arnd@kernel.org>
On Wed, May 20, 2026 at 8:38 PM Arnd Bergmann <arnd@kernel.org> wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Most users of gpio-keys and gpio-keys-polled use modern gpiolib
> interfaces, but there are still number of ancient sh, arm32 and x86
> machines that have never been converted.
>
> Add an #ifdef block for the parts of the driver that are only used on
> those legacy machines.
>
> The two Rohm PMIC drivers use a gpio-keys device without an actual GPIO,
> passing an IRQ number instead. In order to keep this working both with
> and with CONFIG_GPIOLIB_LEGACY, change the gpio-keys driver to ignore
> the gpio number if an IRQ is passed.
>
> Link: https://lore.kernel.org/all/b3c94552-c104-42e3-be15-7e8362e8039e@gmail.com/
> Link: https://lore.kernel.org/all/afJXG4_rtaj3l2Dk@google.com/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v9 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: sashiko-bot @ 2026-05-25 4:17 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260525020543.519082-5-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Mixing goto-based error handling with scope-based cleanup (__free/guard) in claw_hw_output_report().
--
commit a152767539ab279fa2750588503cc5a77a8522da
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add Rumble Intensity Attributes
Adds intensity adjustment for the left and right rumble motors. It introduces
new sysfs attributes for reading and writing rumble intensity.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 96710d80543d4..f5c54cd62f3bb 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -930,6 +962,162 @@ static ssize_t button_mapping_options_show(struct device *dev,
[ ... ]
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
[ ... ]
> + guard(mutex)(&drvdata->rom_mutex);
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> + (u8 *)&report, sizeof(report), 25);
[Severity: High]
This is a pre-existing issue, but does claw_hw_output_report() mix goto-based
error handling with scope-based cleanup?
Looking at claw_hw_output_report(), it defines dmabuf with __free(kfree)
and uses guard(mutex), but also relies on explicit goto err_clear_state
jumps for error handling:
unsigned char *dmabuf __free(kfree) = NULL;
...
if (ret < 0)
goto err_clear_state;
...
err_clear_state:
According to the cleanup guidelines, functions should either convert all
resources to scope-based cleanup, or convert none of them to avoid confusing
ownership semantics. Could this function be updated to strictly use one
approach?
> + if (ret)
> + return ret;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525020543.519082-1-derekjohn.clark@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH v9 3/4] HID: hid-msi: Add RGB control interface
From: sashiko-bot @ 2026-05-25 3:41 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260525020543.519082-4-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Tearing down the `rgb_queue` delayed work before unregistering the sysfs attributes in `claw_remove()` allows the work to be re-armed, leading to a Use-After-Free.
--
commit e775f842567caa50d4c5a9bd87b14e6d6c8ffff6
Author: Derek J. Clark <derekjohn.clark@gmail.com>
Subject: HID: hid-msi: Add RGB control interface
This commit adds an RGB control interface for MSI Claw devices. It supports
configuring up to 8 sequential frames of RGB zone data across 9 zones,
mirroring effects available in the official Windows driver.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 5f20102eae69d..96710d80543d4 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -1043,11 +1675,21 @@ static void claw_remove(struct hid_device *hdev)
> gp_registered = smp_load_acquire(&drvdata->gp_registered);
> /* Pairs with smp_load_acquire in attribute show/store functions */
> smp_store_release(&drvdata->gp_registered, false);
> + /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
> + rgb_registered = smp_load_acquire(&drvdata->rgb_registered);
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->rgb_registered, false);
> }
>
> if (gp_registered)
> device_remove_group(&hdev->dev, &claw_gamepad_attr_group);
>
> + cancel_delayed_work_sync(&drvdata->rgb_queue);
[Severity: High]
Could this lead to a use-after-free if the rgb_queue delayed work is re-armed
while claw_remove() is running?
If a sysfs writer (like effect_store()) reads rgb_registered as true under
registration_lock and gets preempted, claw_remove() can clear rgb_registered,
call cancel_delayed_work_sync(), and then block in device_remove_group()
waiting for the sysfs writer to drop its active kernfs reference.
The sysfs writer could then resume, call mod_delayed_work() to re-arm the
workqueue, and finish:
sysfs writer
scoped_guard(spinlock_irqsave, &drvdata->registration_lock) {
if (!smp_load_acquire(&drvdata->rgb_registered))
return -ENODEV;
}
// Preempted here
claw_remove()
smp_store_release(&drvdata->rgb_registered, false);
cancel_delayed_work_sync(&drvdata->rgb_queue);
device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
// Blocks waiting for sysfs writer
sysfs writer (resumes)
mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
// Completes
claw_remove()
// Unblocks and completes
After claw_remove() finishes, drvdata is freed by devres. When the re-armed
delayed work executes 50ms later, it would dereference the freed drvdata,
causing a use-after-free crash.
Should cancel_delayed_work_sync() be called after the sysfs attributes and LED
class device are fully unregistered?
> + if (rgb_registered) {
> + device_remove_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
> + led_classdev_multicolor_unregister(&drvdata->led_mc);
> + }
> +
> hid_hw_close(hdev);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260525020543.519082-1-derekjohn.clark@gmail.com?part=3
^ 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