* [PATCH v2 1/2] Input: snvs_pwrkey: Improve probe error handling
From: phucduc.bui @ 2026-07-20 9:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc
From: bui duc phuc <phucduc.bui@gmail.com>
Use dev_err_probe() for devm_clk_get_optional_enabled() to handle
deferred probe correctly.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
Drop the redundant error message after devm_request_irq(), as the
helper already reports request failures internally.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
drivers/input/keyboard/snvs_pwrkey.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/input/keyboard/snvs_pwrkey.c b/drivers/input/keyboard/snvs_pwrkey.c
index 954055aaf6e2..fa08ac561bf2 100644
--- a/drivers/input/keyboard/snvs_pwrkey.c
+++ b/drivers/input/keyboard/snvs_pwrkey.c
@@ -142,16 +142,15 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
}
clk = devm_clk_get_optional_enabled(&pdev->dev, NULL);
- if (IS_ERR(clk)) {
- dev_err(&pdev->dev, "Failed to get snvs clock (%pe)\n", clk);
- return PTR_ERR(clk);
- }
+ if (IS_ERR(clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(clk),
+ "Failed to get snvs clock\n");
pdata->wakeup = of_property_read_bool(np, "wakeup-source");
pdata->irq = platform_get_irq(pdev, 0);
if (pdata->irq < 0)
- return -EINVAL;
+ return pdata->irq;
error = of_property_read_u32(np, "power-off-time-sec", &val);
if (!error) {
@@ -209,10 +208,8 @@ static int imx_snvs_pwrkey_probe(struct platform_device *pdev)
error = devm_request_irq(&pdev->dev, pdata->irq,
imx_snvs_pwrkey_interrupt,
0, pdev->name, pdev);
- if (error) {
- dev_err(&pdev->dev, "interrupt not available.\n");
+ if (error)
return error;
- }
error = input_register_device(input);
if (error < 0) {
--
2.43.0
^ permalink raw reply related
* [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling
From: phucduc.bui @ 2026-07-20 9:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, bui duc phuc
In-Reply-To: <20260720090851.230244-1-phucduc.bui@gmail.com>
From: bui duc phuc <phucduc.bui@gmail.com>
Use dev_err_probe() for devm_clk_get() so deferred probe errors are
handled correctly.
Drop the redundant error message after devm_request_irq(), since the
helper already reports request failures internally.
Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes in v2 :
-Return the original error from platform_get_irq() instead of replacing
it with -EINVAL.
drivers/input/keyboard/st-keyscan.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
index e53ef4c670e4..8e9bac339dc4 100644
--- a/drivers/input/keyboard/st-keyscan.c
+++ b/drivers/input/keyboard/st-keyscan.c
@@ -173,10 +173,9 @@ static int keyscan_probe(struct platform_device *pdev)
return PTR_ERR(keypad_data->base);
keypad_data->clk = devm_clk_get(&pdev->dev, NULL);
- if (IS_ERR(keypad_data->clk)) {
- dev_err(&pdev->dev, "cannot get clock\n");
- return PTR_ERR(keypad_data->clk);
- }
+ if (IS_ERR(keypad_data->clk))
+ return dev_err_probe(&pdev->dev, PTR_ERR(keypad_data->clk),
+ "cannot get clock\n");
error = clk_enable(keypad_data->clk);
if (error) {
@@ -188,14 +187,12 @@ static int keyscan_probe(struct platform_device *pdev)
keypad_data->irq = platform_get_irq(pdev, 0);
if (keypad_data->irq < 0)
- return -EINVAL;
+ return keypad_data->irq;
error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
pdev->name, keypad_data);
- if (error) {
- dev_err(&pdev->dev, "failed to request IRQ\n");
+ if (error)
return error;
- }
error = input_register_device(input_dev);
if (error) {
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling
From: sashiko-bot @ 2026-07-20 9:17 UTC (permalink / raw)
To: phucduc.bui; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720090851.230244-2-phucduc.bui@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver requests and leaves the interrupt enabled while the hardware clock is explicitly disabled, leading to a potential crash (synchronous external abort) upon a spurious or pending interrupt.
--
--- Patch [2]: [PATCH v2 2/2] Input: st-keyscan: Improve probe error handling ---
commit 036864a311a2de2b62ec154e1ff242ede73fb89b
Author: bui duc phuc <phucduc.bui@gmail.com>
Input: st-keyscan: Improve probe error handling
This commit improves error handling in the probe function by using
dev_err_probe() for clock fetching, removing redundant error logs for
devm_request_irq(), and returning the original error code from
platform_get_irq().
> diff --git a/drivers/input/keyboard/st-keyscan.c b/drivers/input/keyboard/st-keyscan.c
> index e53ef4c670e4b..8e9bac339dc47 100644
> --- a/drivers/input/keyboard/st-keyscan.c
> +++ b/drivers/input/keyboard/st-keyscan.c
[ ... ]
> @@ -188,14 +187,12 @@ static int keyscan_probe(struct platform_device *pdev)
>
> keypad_data->irq = platform_get_irq(pdev, 0);
> if (keypad_data->irq < 0)
> - return -EINVAL;
> + return keypad_data->irq;
>
> error = devm_request_irq(&pdev->dev, keypad_data->irq, keyscan_isr, 0,
> pdev->name, keypad_data);
[Severity: High]
This is a pre-existing issue, but does requesting the IRQ here while the
hardware clock is explicitly disabled risk a bus fault?
Earlier in keyscan_probe(), keyscan_stop() is called to disable the clock.
When devm_request_irq() is called here, it unmasks and enables the interrupt
while the clock remains gated.
If a spurious interrupt or a bootloader-pending interrupt occurs at this
point, keyscan_isr() will execute and attempt to read the hardware register:
drivers/input/keyboard/st-keyscan.c:keyscan_isr() {
...
state = readl(keypad->base + KEYSCAN_MATRIX_STATE_OFF) & 0xffff;
...
}
Accessing the memory-mapped register while the IP block's clock is disabled
can trigger a synchronous external abort. The same condition appears to happen
during keyscan_close(), which disables the clock while the IRQ remains active.
Would it be safer to request the interrupt with IRQF_NO_AUTOEN or delay
requesting it until the device is actually opened and clocked?
> - if (error) {
> - dev_err(&pdev->dev, "failed to request IRQ\n");
> + if (error)
> return error;
> - }
>
> error = input_register_device(input_dev);
> if (error) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720090851.230244-1-phucduc.bui@gmail.com?part=2
^ permalink raw reply
* [PATCH 00/10] HID: iio: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
Hi all,
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Prepratory series to convert all HID IIO driver to devm.
Testing:
- Compiled with W=1 for each patch in series
---
Sanjay Chitroda (10):
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
drivers/iio/accel/hid-sensor-accel-3d.c | 22 +++++------
drivers/iio/gyro/hid-sensor-gyro-3d.c | 19 +++++-----
drivers/iio/humidity/hid-sensor-humidity.c | 7 ++--
drivers/iio/light/hid-sensor-als.c | 15 ++++----
drivers/iio/light/hid-sensor-prox.c | 15 ++++----
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 43 ++++++++++------------
drivers/iio/orientation/hid-sensor-incl-3d.c | 20 +++++-----
drivers/iio/orientation/hid-sensor-rotation.c | 24 ++++++------
.../iio/position/hid-sensor-custom-intel-hinge.c | 17 +++++----
drivers/iio/pressure/hid-sensor-press.c | 19 +++++-----
10 files changed, 100 insertions(+), 101 deletions(-)
---
base-commit: 36c12dbda81c284d72f3c64689461647497b643b
change-id: 20260720-hid-iio-local-struct-device-1d3af277658a
Best regards,
--
Sanjay Chitroda <sanjayembeddedse@gmail.com>
^ permalink raw reply
* [PATCH 01/10] iio: humidity: hid-sensor-humidity: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/humidity/hid-sensor-humidity.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index 7cec81ff5685..1b254d3aeee9 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -196,10 +196,11 @@ static int hid_humidity_probe(struct platform_device *pdev)
struct iio_dev *indio_dev;
struct hid_humidity_state *humid_st;
struct iio_chan_spec *humid_chans;
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret;
- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*humid_st));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*humid_st));
if (!indio_dev)
return -ENOMEM;
@@ -215,7 +216,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
if (ret)
return ret;
- humid_chans = devm_kmemdup(&pdev->dev, humidity_channels,
+ humid_chans = devm_kmemdup(dev, humidity_channels,
sizeof(humidity_channels), GFP_KERNEL);
if (!humid_chans)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH 02/10] iio: gyro: hid-sensor-gyro-3d: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index 58250a972567..8ff3ceda6dab 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -264,13 +264,14 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_gyro_3d_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret = 0;
static const char *name = "gyro_3d";
struct iio_dev *indio_dev;
struct gyro_3d_state *gyro_state;
- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*gyro_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*gyro_state));
if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
@@ -285,14 +286,14 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
gyro_3d_sensitivity_addresses,
ARRAY_SIZE(gyro_3d_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
- indio_dev->channels = devm_kmemdup(&pdev->dev, gyro_3d_channels,
+ indio_dev->channels = devm_kmemdup(dev, gyro_3d_channels,
sizeof(gyro_3d_channels), GFP_KERNEL);
if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ dev_err(dev, "failed to duplicate channels\n");
return -ENOMEM;
}
@@ -300,7 +301,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
(struct iio_chan_spec *)indio_dev->channels,
HID_USAGE_SENSOR_GYRO_3D, gyro_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -314,7 +315,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&gyro_state->common_attributes);
if (ret < 0) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -324,13 +325,13 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D,
&gyro_state->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 04/10] iio: light: hid-sensor-als: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/light/hid-sensor-als.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 6fb6ace26834..44088f9dff81 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -346,13 +346,14 @@ static int als_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_als_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret = 0;
static const char *name = "als";
struct iio_dev *indio_dev;
struct als_state *als_state;
- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct als_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct als_state));
if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
@@ -367,7 +368,7 @@ static int hid_als_probe(struct platform_device *pdev)
als_sensitivity_addresses,
ARRAY_SIZE(als_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
@@ -375,7 +376,7 @@ static int hid_als_probe(struct platform_device *pdev)
hsdev->usage,
als_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -397,7 +398,7 @@ static int hid_als_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&als_state->common_attributes);
if (ret < 0) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -406,13 +407,13 @@ static int hid_als_probe(struct platform_device *pdev)
als_state->callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, hsdev->usage, &als_state->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 03/10] iio: accel: hid-sensor-accel-3d: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index eb521125f4eb..5467ecc86b58 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -314,7 +314,8 @@ static int accel_3d_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_accel_3d_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret = 0;
const char *name;
struct iio_dev *indio_dev;
@@ -322,8 +323,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
const struct iio_chan_spec *channel_spec;
int channel_size;
- indio_dev = devm_iio_device_alloc(&pdev->dev,
- sizeof(struct accel_3d_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct accel_3d_state));
if (!indio_dev)
return -ENOMEM;
@@ -350,21 +350,21 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
accel_3d_sensitivity_addresses,
ARRAY_SIZE(accel_3d_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
- indio_dev->channels = devm_kmemdup(&pdev->dev, channel_spec,
- channel_size, GFP_KERNEL);
+ indio_dev->channels = devm_kmemdup(dev, channel_spec, channel_size, GFP_KERNEL);
if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ dev_err(dev, "failed to duplicate channels\n");
return -ENOMEM;
}
+
ret = accel_3d_parse_report(pdev, hsdev,
(struct iio_chan_spec *)indio_dev->channels,
hsdev->usage, accel_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -377,7 +377,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&accel_state->common_attributes);
if (ret < 0) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -387,13 +387,13 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
ret = sensor_hub_register_callback(hsdev, hsdev->usage,
&accel_state->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 08/10] iio: orientation: hid-sensor-rotation: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/orientation/hid-sensor-rotation.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index 9e13a0ecd584..50a684efeb45 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -238,6 +238,7 @@ static int dev_rot_parse_report(struct platform_device *pdev,
u32 usage_id,
struct dev_rot_state *st)
{
+ struct device *dev = &pdev->dev;
int ret;
ret = sensor_hub_input_get_attribute_info(hsdev,
@@ -248,11 +249,8 @@ static int dev_rot_parse_report(struct platform_device *pdev,
if (ret)
return ret;
- dev_dbg(&pdev->dev, "dev_rot %x:%x\n", st->quaternion.index,
- st->quaternion.report_id);
-
- dev_dbg(&pdev->dev, "dev_rot: attrib size %d\n",
- st->quaternion.size);
+ dev_dbg(dev, "dev_rot %x:%x\n", st->quaternion.index, st->quaternion.report_id);
+ dev_dbg(dev, "dev_rot: attrib size %d\n", st->quaternion.size);
st->scale_precision = hid_sensor_format_scale(
hsdev->usage,
@@ -265,14 +263,14 @@ static int dev_rot_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_dev_rot_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret;
char *name;
struct iio_dev *indio_dev;
struct dev_rot_state *rot_state;
- indio_dev = devm_iio_device_alloc(&pdev->dev,
- sizeof(struct dev_rot_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct dev_rot_state));
if (!indio_dev)
return -ENOMEM;
@@ -302,13 +300,13 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
rotation_sensitivity_addresses,
ARRAY_SIZE(rotation_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
ret = dev_rot_parse_report(pdev, hsdev, hsdev->usage, rot_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -323,7 +321,7 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&rot_state->common_attributes);
if (ret) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -333,13 +331,13 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
ret = sensor_hub_register_callback(hsdev, hsdev->usage,
&rot_state->callbacks);
if (ret) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 06/10] iio: magnetometer: hid-sensor-magn-3d: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 43 ++++++++++++---------------
1 file changed, 19 insertions(+), 24 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index ad18f233ee16..c9077adde882 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -337,6 +337,7 @@ static int magn_3d_parse_report(struct platform_device *pdev,
u32 usage_id,
struct magn_3d_state *st)
{
+ struct device *dev = &pdev->dev;
int i;
int attr_count = 0;
struct iio_chan_spec *_channels;
@@ -357,35 +358,30 @@ static int magn_3d_parse_report(struct platform_device *pdev,
}
if (attr_count <= 0) {
- dev_err(&pdev->dev,
- "failed to find any supported usage attributes in report\n");
+ dev_err(dev, "failed to find any supported usage attributes in report\n");
return -EINVAL;
}
- dev_dbg(&pdev->dev, "magn_3d Found %d usage attributes\n", attr_count);
- dev_dbg(&pdev->dev, "magn_3d X: %x:%x Y: %x:%x Z: %x:%x\n",
+ dev_dbg(dev, "magn_3d Found %d usage attributes\n", attr_count);
+ dev_dbg(dev, "magn_3d X: %x:%x Y: %x:%x Z: %x:%x\n",
st->magn[0].index,
st->magn[0].report_id,
st->magn[1].index, st->magn[1].report_id,
st->magn[2].index, st->magn[2].report_id);
/* Setup IIO channel array */
- _channels = devm_kcalloc(&pdev->dev, attr_count,
- sizeof(struct iio_chan_spec),
- GFP_KERNEL);
+ _channels = devm_kcalloc(dev, attr_count, sizeof(struct iio_chan_spec), GFP_KERNEL);
if (!_channels) {
- dev_err(&pdev->dev,
- "failed to allocate space for iio channels\n");
+ dev_err(dev, "failed to allocate space for iio channels\n");
return -ENOMEM;
}
/* attr_count include timestamp channel, and the iio_vals should be aligned to 8byte */
- st->iio_vals = devm_kcalloc(&pdev->dev,
+ st->iio_vals = devm_kcalloc(dev,
((attr_count + 1) % 2 + (attr_count + 1) / 2) * 2,
sizeof(u32), GFP_KERNEL);
if (!st->iio_vals) {
- dev_err(&pdev->dev,
- "failed to allocate space for iio values array\n");
+ dev_err(dev, "failed to allocate space for iio values array\n");
return -ENOMEM;
}
@@ -412,14 +408,13 @@ static int magn_3d_parse_report(struct platform_device *pdev,
}
if (*chan_count <= 0) {
- dev_err(&pdev->dev,
- "failed to find any magnetic channels setup\n");
+ dev_err(dev, "failed to find any magnetic channels setup\n");
return -EINVAL;
}
*channels = _channels;
- dev_dbg(&pdev->dev, "magn_3d Setup %d IIO channels\n", *chan_count);
+ dev_dbg(dev, "magn_3d Setup %d IIO channels\n", *chan_count);
st->magn_flux_attr.scale_precision = hid_sensor_format_scale(
HID_USAGE_SENSOR_COMPASS_3D,
@@ -439,7 +434,7 @@ static int magn_3d_parse_report(struct platform_device *pdev,
HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH,
&st->rot_attributes.sensitivity);
- dev_dbg(&pdev->dev, "Sensitivity index:report %d:%d\n",
+ dev_dbg(dev, "Sensitivity index:report %d:%d\n",
st->rot_attributes.sensitivity.index,
st->rot_attributes.sensitivity.report_id);
}
@@ -450,7 +445,8 @@ static int magn_3d_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_magn_3d_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret = 0;
static char *name = "magn_3d";
struct iio_dev *indio_dev;
@@ -458,8 +454,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
struct iio_chan_spec *channels;
int chan_count = 0;
- indio_dev = devm_iio_device_alloc(&pdev->dev,
- sizeof(struct magn_3d_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct magn_3d_state));
if (!indio_dev)
return -ENOMEM;
@@ -475,7 +470,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
magn_3d_sensitivity_addresses,
ARRAY_SIZE(magn_3d_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
magn_state->rot_attributes = magn_state->magn_flux_attributes;
@@ -486,7 +481,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
&channels, &chan_count,
HID_USAGE_SENSOR_COMPASS_3D, magn_state);
if (ret) {
- dev_err(&pdev->dev, "failed to parse report\n");
+ dev_err(dev, "failed to parse report\n");
return ret;
}
@@ -501,7 +496,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&magn_state->magn_flux_attributes);
if (ret < 0) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -511,13 +506,13 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D,
&magn_state->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 05/10] iio: light: hid-sensor-prox: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/light/hid-sensor-prox.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index 63fd3eff171c..e7fe40726385 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -260,13 +260,14 @@ static int prox_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_prox_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret = 0;
static const char *name = "prox";
struct iio_dev *indio_dev;
struct prox_state *prox_state;
- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct prox_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct prox_state));
if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
@@ -280,13 +281,13 @@ static int hid_prox_probe(struct platform_device *pdev)
prox_sensitivity_addresses,
ARRAY_SIZE(prox_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
ret = prox_parse_report(pdev, hsdev, prox_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -302,7 +303,7 @@ static int hid_prox_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&prox_state->common_attributes);
if (ret) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -312,13 +313,13 @@ static int hid_prox_probe(struct platform_device *pdev)
ret = sensor_hub_register_callback(hsdev, hsdev->usage,
&prox_state->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 10/10] iio: pressure: hid-sensor-press: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/pressure/hid-sensor-press.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index e688b0776547..fee170aa4379 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -230,13 +230,14 @@ static int press_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_press_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret = 0;
static const char *name = "press";
struct iio_dev *indio_dev;
struct press_state *press_state;
- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(struct press_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct press_state));
if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
@@ -251,14 +252,14 @@ static int hid_press_probe(struct platform_device *pdev)
press_sensitivity_addresses,
ARRAY_SIZE(press_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
- indio_dev->channels = devm_kmemdup(&pdev->dev, press_channels,
+ indio_dev->channels = devm_kmemdup(dev, press_channels,
sizeof(press_channels), GFP_KERNEL);
if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ dev_err(dev, "failed to duplicate channels\n");
return -ENOMEM;
}
@@ -266,7 +267,7 @@ static int hid_press_probe(struct platform_device *pdev)
(struct iio_chan_spec *)indio_dev->channels,
HID_USAGE_SENSOR_PRESSURE, press_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -281,7 +282,7 @@ static int hid_press_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&press_state->common_attributes);
if (ret) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -291,13 +292,13 @@ static int hid_press_probe(struct platform_device *pdev)
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_PRESSURE,
&press_state->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 09/10] iio: position: hid-sensor-custom-intel-hinge: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
index d275bc1413fe..febc32f5a332 100644
--- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
+++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
@@ -263,13 +263,14 @@ static int hinge_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_hinge_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
struct hinge_state *st;
struct iio_dev *indio_dev;
int ret;
int i;
- indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*st));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*st));
if (!indio_dev)
return -ENOMEM;
@@ -287,12 +288,12 @@ static int hid_hinge_probe(struct platform_device *pdev)
hinge_sensitivity_addresses,
ARRAY_SIZE(hinge_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
indio_dev->num_channels = ARRAY_SIZE(hinge_channels);
- indio_dev->channels = devm_kmemdup(&pdev->dev, hinge_channels,
+ indio_dev->channels = devm_kmemdup(dev, hinge_channels,
sizeof(hinge_channels), GFP_KERNEL);
if (!indio_dev->channels)
return -ENOMEM;
@@ -301,7 +302,7 @@ static int hid_hinge_probe(struct platform_device *pdev)
(struct iio_chan_spec *)indio_dev->channels,
hsdev->usage, st);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -313,7 +314,7 @@ static int hid_hinge_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, indio_dev->name,
&st->common_attributes);
if (ret < 0) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -322,13 +323,13 @@ static int hid_hinge_probe(struct platform_device *pdev)
st->callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, hsdev->usage, &st->callbacks);
if (ret < 0) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* [PATCH 07/10] iio: orientation: hid-sensor-incl-3d: use local struct device
From: Sanjay Chitroda via B4 Relay @ 2026-07-20 9:40 UTC (permalink / raw)
To: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel, Sanjay Chitroda
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Introduce a local struct device pointer derived from &pdev->dev.
This avoids repeated &pdev->dev usage and improves readability.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/orientation/hid-sensor-incl-3d.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index aea28321db95..ff28aed47c75 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -293,14 +293,14 @@ static int incl_3d_parse_report(struct platform_device *pdev,
/* Function to initialize the processing for usage id */
static int hid_incl_3d_probe(struct platform_device *pdev)
{
- struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
+ struct device *dev = &pdev->dev;
+ struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
int ret;
static char *name = "incli_3d";
struct iio_dev *indio_dev;
struct incl_3d_state *incl_state;
- indio_dev = devm_iio_device_alloc(&pdev->dev,
- sizeof(struct incl_3d_state));
+ indio_dev = devm_iio_device_alloc(dev, sizeof(struct incl_3d_state));
if (!indio_dev)
return -ENOMEM;
@@ -316,14 +316,14 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
incl_3d_sensitivity_addresses,
ARRAY_SIZE(incl_3d_sensitivity_addresses));
if (ret) {
- dev_err(&pdev->dev, "failed to setup common attributes\n");
+ dev_err(dev, "failed to setup common attributes\n");
return ret;
}
- indio_dev->channels = devm_kmemdup(&pdev->dev, incl_3d_channels,
+ indio_dev->channels = devm_kmemdup(dev, incl_3d_channels,
sizeof(incl_3d_channels), GFP_KERNEL);
if (!indio_dev->channels) {
- dev_err(&pdev->dev, "failed to duplicate channels\n");
+ dev_err(dev, "failed to duplicate channels\n");
return -ENOMEM;
}
@@ -332,7 +332,7 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
HID_USAGE_SENSOR_INCLINOMETER_3D,
incl_state);
if (ret) {
- dev_err(&pdev->dev, "failed to setup attributes\n");
+ dev_err(dev, "failed to setup attributes\n");
return ret;
}
@@ -346,7 +346,7 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
ret = hid_sensor_setup_trigger(indio_dev, name,
&incl_state->common_attributes);
if (ret) {
- dev_err(&pdev->dev, "trigger setup failed\n");
+ dev_err(dev, "trigger setup failed\n");
return ret;
}
@@ -357,13 +357,13 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
HID_USAGE_SENSOR_INCLINOMETER_3D,
&incl_state->callbacks);
if (ret) {
- dev_err(&pdev->dev, "callback reg failed\n");
+ dev_err(dev, "callback reg failed\n");
goto error_remove_trigger;
}
ret = iio_device_register(indio_dev);
if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
+ dev_err(dev, "device register failed\n");
goto error_remove_callback;
}
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 01/10] iio: humidity: hid-sensor-humidity: use local struct device
From: sashiko-bot @ 2026-07-20 9:49 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-1-74496fee6327@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver shares a single, global static `struct hid_sensor_hub_callbacks humidity_callbacks` across all device instances, leading to cross-device data corruption and potential Use-After-Free if multiple devices are probed.
--
--- Patch [1]: [PATCH 01/10] iio: humidity: hid-sensor-humidity: use local struct device ---
commit e9be49bc2265ade3aeb3c548b989cbea2029fdd4
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: humidity: hid-sensor-humidity: use local struct device
This commit introduces a local struct device pointer derived from pdev->dev
in hid_humidity_probe() to avoid repeated usage and improve readability.
> diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
> index 7cec81ff56856..1b254d3aeee90 100644
> --- a/drivers/iio/humidity/hid-sensor-humidity.c
> +++ b/drivers/iio/humidity/hid-sensor-humidity.c
> @@ -196,10 +196,11 @@ static int hid_humidity_probe(struct platform_device *pdev)
[Severity: High]
This is a pre-existing issue, but does sharing a single global static
humidity_callbacks structure across all device instances lead to cross-device
data corruption?
Looking at hid_humidity_probe(), it unconditionally overwrites the shared
static callback structure with the pdev of the most recently probed instance:
hid_humidity_probe(struct platform_device *pdev)
{
...
humidity_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
&humidity_callbacks);
...
}
If multiple HID humidity sensors are probed, the static humidity_callbacks
struct is overwritten. If an earlier probed device generates an event, the
callback functions will receive the wrong pdev.
Additionally, if the most recently probed device is disconnected, its pdev is
freed. If the remaining connected device generates an event, could the driver
dereference a dangling pointer?
Should the callbacks struct be allocated per-device inside the dynamically
allocated state structure instead of being defined globally as a static
variable?
> struct iio_dev *indio_dev;
> struct hid_humidity_state *humid_st;
> struct iio_chan_spec *humid_chans;
> - struct hid_sensor_hub_device *hsdev = dev_get_platdata(&pdev->dev);
> + struct device *dev = &pdev->dev;
> + struct hid_sensor_hub_device *hsdev = dev_get_platdata(dev);
> int ret;
>
> - indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*humid_st));
> + indio_dev = devm_iio_device_alloc(dev, sizeof(*humid_st));
> if (!indio_dev)
> return -ENOMEM;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 09/10] iio: position: hid-sensor-custom-intel-hinge: use local struct device
From: sashiko-bot @ 2026-07-20 9:59 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260720-hid-iio-local-struct-device-v1-9-74496fee6327@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Possible Use-After-Free of `hinge_state` and its embedded `callbacks` during driver unbind or probe error paths, due to lack of synchronization with concurrent HID sensor events.
--
--- Patch [9]: [PATCH 09/10] iio: position: hid-sensor-custom-intel-hinge: use local struct device ---
commit 7aa4981763773e6c94e52f6b3bcf4d8c0d85b050
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: position: hid-sensor-custom-intel-hinge: use local struct device
Introduce a local struct device pointer derived from pdev->dev to avoid
repeated pdev->dev usage and improve readability.
> diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
> index d275bc1413feb..febc32f5a3328 100644
> --- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
> +++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
[ ... ]
> @@ -322,13 +323,13 @@ static int hid_hinge_probe(struct platform_device *pdev)
> st->callbacks.pdev = pdev;
> ret = sensor_hub_register_callback(hsdev, hsdev->usage, &st->callbacks);
> if (ret < 0) {
> - dev_err(&pdev->dev, "callback reg failed\n");
> + dev_err(dev, "callback reg failed\n");
> goto error_remove_trigger;
> }
>
> ret = iio_device_register(indio_dev);
> if (ret) {
> - dev_err(&pdev->dev, "device register failed\n");
> + dev_err(dev, "device register failed\n");
> goto error_remove_callback;
> }
>
> return ret;
>
> error_remove_callback:
> sensor_hub_remove_callback(hsdev, hsdev->usage);
[Severity: High]
This is a pre-existing issue, but can this lead to a use-after-free during
the probe error path or device unbind?
If iio_device_register() fails, sensor_hub_remove_callback() removes the
callback from the list but does not appear to synchronize with
sensor_hub_raw_event() which might be concurrently executing the callback.
Since the hinge_state structure is allocated with devm_iio_device_alloc(),
returning from hid_hinge_probe() will cause the devres framework to
prematurely free the memory while the concurrent event handler is actively
writing to it.
Does there need to be some synchronization to ensure pending events are
flushed before the devres cleanup occurs?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-hid-iio-local-struct-device-v1-0-74496fee6327@gmail.com?part=9
^ permalink raw reply
* [PATCH v2 1/5] Input: applespi - use unified wait queue with timeouts for drain
From: Shih-Yuan Lee @ 2026-07-20 10:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mark Brown, linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260720101435.13612-1-fourdollars@debian.org>
The driver currently maintains a dedicated wait queue 'drain_complete'
specifically to wait for outstanding write requests to complete.
Consolidate this with other wait events by renaming it to 'wait_queue' to
make resource management cleaner.
Furthermore, using wait_event_lock_irq() without a timeout risks blocking
the thread indefinitely during driver unbinding (remove) or PM transition
phases if the hardware fails to respond or interrupts are missed.
Replace wait_event_lock_irq() with wait_event_lock_irq_timeout() in
applespi_drain_writes() and applespi_drain_reads() with a 3-second timeout.
This ensures the driver can gracefully recover and avoid lockups under
unresponsive hardware conditions.
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 32 +++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index b5ff71cd5a70..64bbeba85ea9 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -417,7 +417,7 @@ struct applespi_data {
bool suspended;
bool drain;
- wait_queue_head_t drain_complete;
+ wait_queue_head_t wait_queue;
bool read_active;
bool write_active;
@@ -677,7 +677,7 @@ static int applespi_setup_spi(struct applespi_data *applespi)
return sts;
spin_lock_init(&applespi->cmd_msg_lock);
- init_waitqueue_head(&applespi->drain_complete);
+ init_waitqueue_head(&applespi->wait_queue);
return 0;
}
@@ -725,7 +725,7 @@ static void applespi_msg_complete(struct applespi_data *applespi,
applespi->write_active = false;
if (applespi->drain && !applespi->write_active)
- wake_up_all(&applespi->drain_complete);
+ wake_up_all(&applespi->wait_queue);
if (is_write_msg) {
applespi->cmd_msg_queued = 0;
@@ -1415,7 +1415,7 @@ static void applespi_got_data(struct applespi_data *applespi)
applespi->read_active = false;
applespi->write_active = false;
- wake_up_all(&applespi->drain_complete);
+ wake_up_all(&applespi->wait_queue);
}
return;
@@ -1793,21 +1793,33 @@ static int applespi_probe(struct spi_device *spi)
static void applespi_drain_writes(struct applespi_data *applespi)
{
- guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+ unsigned long flags;
+
+ spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
applespi->drain = true;
- wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
- applespi->cmd_msg_lock);
+ wait_event_lock_irq_timeout(applespi->wait_queue,
+ !applespi->write_active,
+ applespi->cmd_msg_lock,
+ msecs_to_jiffies(3000));
+
+ spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
}
static void applespi_drain_reads(struct applespi_data *applespi)
{
- guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+ unsigned long flags;
+
+ spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
- wait_event_lock_irq(applespi->drain_complete, !applespi->read_active,
- applespi->cmd_msg_lock);
+ wait_event_lock_irq_timeout(applespi->wait_queue,
+ !applespi->read_active,
+ applespi->cmd_msg_lock,
+ msecs_to_jiffies(3000));
applespi->suspended = true;
+
+ spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
}
static void applespi_remove(struct spi_device *spi)
--
2.39.5
^ permalink raw reply related
* [PATCH v2 0/5] Input: applespi - Fix probe timeout and use-after-free bugs
From: Shih-Yuan Lee @ 2026-07-20 10:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mark Brown, linux-input, linux-kernel, Shih-Yuan Lee
This patch series simplifies the applespi driver by making touchpad
detection and registration synchronous during probe, and fixes a
use-after-free bug in applespi_remove().
Changes since v1:
- Split the large touchpad registration patch into 4 distinct, single-purpose
commits (wait queue consolidation & timeouts, async queue slots tracking,
synchronous registration, and async probe preference) for better readability.
- Fixed a self-deadlock in applespi_async() where it attempted to acquire
cmd_msg_lock while already held by applespi_notify() and other callers.
- Updated applespi_async() to assert the lock is held by the caller.
- Protected touchpad info flags under cmd_msg_lock in response handling.
- Simplified applespi_tp_dim_open() by removing redundant NULL checks.
- Overhauled GPE disabling and teardown order in applespi_remove() to prevent
GPE interrupt storms and unbind deadlocks.
- Addressed reviewer feedback from Sashiko.
Shih-Yuan Lee (5):
Input: applespi - use unified wait queue with timeouts for drain
Input: applespi - track asynchronous SPI transfers in flight
Input: applespi - register touchpad synchronously in probe
Input: applespi - prefer asynchronous driver probing
Input: applespi - fix use-after-free in applespi_remove()
drivers/input/keyboard/applespi.c | 175 ++++++++++++++++++++++++------
1 file changed, 140 insertions(+), 35 deletions(-)
--
2.39.5
^ permalink raw reply
* [PATCH v2 2/5] Input: applespi - track asynchronous SPI transfers in flight
From: Shih-Yuan Lee @ 2026-07-20 10:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mark Brown, linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260720101435.13612-1-fourdollars@debian.org>
The driver queues read and write packets asynchronously. When shutting
down, removing, or suspending, the driver must guarantee that no
asynchronous transfers remain in flight to prevent memory corruption or
use-after-free conditions.
Introduce a 'spi_complete' slot tracking array in struct applespi_data
to represent the two concurrent transfers (one for reads, one for
writes). Implement applespi_async_outstanding() and
applespi_async_complete() to track transfers under cmd_msg_lock.
Modify applespi_async() to allocate a completion slot and assert that the
caller holds the required cmd_msg_lock. This ensures robust and lock-safe
tracking of all asynchronous SPI transactions.
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 70 +++++++++++++++++++++++++++++--
1 file changed, 67 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index 64bbeba85ea9..a8f8d5370e95 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -421,6 +421,12 @@ struct applespi_data {
bool read_active;
bool write_active;
+ struct applespi_complete_info {
+ void (*complete)(void *context);
+ struct applespi_data *applespi;
+ } spi_complete[2];
+ bool cancel_spi;
+
struct work_struct work;
struct touchpad_info_protocol rcvd_tp_info;
@@ -607,13 +613,71 @@ static void applespi_setup_write_txfrs(struct applespi_data *applespi)
spi_message_add_tail(st_t, msg);
}
+static bool applespi_async_outstanding(struct applespi_data *applespi)
+{
+ return applespi->spi_complete[0].complete ||
+ applespi->spi_complete[1].complete;
+}
+
+static void applespi_async_complete(void *context)
+{
+ struct applespi_complete_info *info = context;
+ struct applespi_data *applespi = info->applespi;
+ void (*complete)(void *context);
+ unsigned long flags;
+
+ spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+ complete = info->complete;
+ info->complete = NULL;
+
+ if (applespi->cancel_spi && !applespi_async_outstanding(applespi))
+ wake_up_all(&applespi->wait_queue);
+
+ spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+ if (complete)
+ complete(applespi);
+}
+
static int applespi_async(struct applespi_data *applespi,
struct spi_message *message, void (*complete)(void *))
{
- message->complete = complete;
- message->context = applespi;
+ struct applespi_complete_info *info;
+ int sts;
+
+ assert_spin_locked(&applespi->cmd_msg_lock);
+
+ if (applespi->cancel_spi) {
+ if (!applespi_async_outstanding(applespi))
+ wake_up_all(&applespi->wait_queue);
+ return -ESHUTDOWN;
+ }
+
+ /*
+ * There can only be at most 2 spi requests in flight, one for "reads"
+ * and one for "writes".
+ */
+ if (!applespi->spi_complete[0].complete)
+ info = &applespi->spi_complete[0];
+ else if (!applespi->spi_complete[1].complete)
+ info = &applespi->spi_complete[1];
+ else {
+ dev_warn(&applespi->spi->dev, "Both SPI async slots in use\n");
+ return -EBUSY;
+ }
+
+ info->complete = complete;
+ info->applespi = applespi;
+
+ message->complete = applespi_async_complete;
+ message->context = info;
+
+ sts = spi_async(applespi->spi, message);
+ if (sts)
+ info->complete = NULL;
- return spi_async(applespi->spi, message);
+ return sts;
}
static inline bool applespi_check_write_status(struct applespi_data *applespi,
--
2.39.5
^ permalink raw reply related
* [PATCH v2 3/5] Input: applespi - register touchpad synchronously in probe
From: Shih-Yuan Lee @ 2026-07-20 10:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mark Brown, linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260720101435.13612-1-fourdollars@debian.org>
Touchpad registration is currently deferred to an asynchronous worker
applespi_worker(). This asynchronous registration introduces race
conditions if debugfs or other properties are accessed before the worker
completes, or if the driver is unbound while the worker is active.
Remove the workqueue and the asynchronous worker. Perform touchpad
registration synchronously during driver probe.
Wait up to 3 seconds for the touchpad information command packet response
using wait_event_timeout(). If the response times out, log a warning and
fallback to keyboard-only mode. If registration fails, gracefully unwind GPE
handlers and wait for outstanding SPI transactions to complete.
To prevent data races between the interrupt handler and the probe thread,
protect the 'have_tp_info' flag and 'rcvd_tp_info' structure under the
cmd_msg_lock in applespi_handle_cmd_response().
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 52 +++++++++++++++++++++----------
1 file changed, 36 insertions(+), 16 deletions(-)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index a8f8d5370e95..42b7f87ef2cd 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -427,7 +427,7 @@ struct applespi_data {
} spi_complete[2];
bool cancel_spi;
- struct work_struct work;
+ bool have_tp_info;
struct touchpad_info_protocol rcvd_tp_info;
struct dentry *debugfs_root;
@@ -1388,26 +1388,20 @@ applespi_register_touchpad_device(struct applespi_data *applespi,
return 0;
}
-static void applespi_worker(struct work_struct *work)
-{
- struct applespi_data *applespi =
- container_of(work, struct applespi_data, work);
-
- applespi_register_touchpad_device(applespi, &applespi->rcvd_tp_info);
-}
-
static void applespi_handle_cmd_response(struct applespi_data *applespi,
struct spi_packet *packet,
struct message *message)
{
+ unsigned long flags;
+
if (packet->device == PACKET_DEV_INFO &&
le16_to_cpu(message->type) == 0x1020) {
- /*
- * We're not allowed to sleep here, but registering an input
- * device can sleep.
- */
+ spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
applespi->rcvd_tp_info = message->tp_info;
- schedule_work(&applespi->work);
+ applespi->have_tp_info = true;
+ spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+ wake_up_all(&applespi->wait_queue);
return;
}
@@ -1675,6 +1669,7 @@ static int applespi_probe(struct spi_device *spi)
acpi_handle spi_handle = ACPI_HANDLE(&spi->dev);
acpi_status acpi_sts;
int sts, i;
+ unsigned long flags;
unsigned long long gpe, usb_status;
/* check if the USB interface is present and enabled already */
@@ -1692,8 +1687,6 @@ static int applespi_probe(struct spi_device *spi)
applespi->spi = spi;
- INIT_WORK(&applespi->work, applespi_worker);
-
/* store the driver data */
spi_set_drvdata(spi, applespi);
@@ -1821,6 +1814,20 @@ static int applespi_probe(struct spi_device *spi)
/* trigger touchpad setup */
applespi_init(applespi, false);
+ /* set up the touchpad as a separate input device if info is received */
+ sts = wait_event_timeout(applespi->wait_queue,
+ READ_ONCE(applespi->have_tp_info),
+ msecs_to_jiffies(3000));
+ if (!sts) {
+ dev_warn(&applespi->spi->dev,
+ "Timed out waiting for touchpad info, continuing keyboard-only\n");
+ } else {
+ sts = applespi_register_touchpad_device(applespi,
+ &applespi->rcvd_tp_info);
+ if (sts)
+ goto cancel_spi;
+ }
+
/*
* By default this device is not enabled for wakeup; but USB keyboards
* generally are, so the expectation is that by default the keyboard
@@ -1853,6 +1860,19 @@ static int applespi_probe(struct spi_device *spi)
&applespi_tp_dim_fops);
return 0;
+
+cancel_spi:
+ acpi_disable_gpe(NULL, applespi->gpe);
+ acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
+
+ spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+ applespi->cancel_spi = true;
+ wait_event_lock_irq(applespi->wait_queue,
+ !applespi_async_outstanding(applespi),
+ applespi->cmd_msg_lock);
+ spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+ return sts;
}
static void applespi_drain_writes(struct applespi_data *applespi)
--
2.39.5
^ permalink raw reply related
* [PATCH v2 4/5] Input: applespi - prefer asynchronous driver probing
From: Shih-Yuan Lee @ 2026-07-20 10:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mark Brown, linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260720101435.13612-1-fourdollars@debian.org>
Set probe_type to PROBE_PREFER_ASYNCHRONOUS to allow the driver core
to run applespi_probe() asynchronously. This improves system boot speeds by
avoiding blocking the main kernel thread during the 3-second touchpad
detection wait.
Additionally, clean up applespi_tp_dim_open() by simplifying product ID
retrieval and avoiding dereferencing touchpad_input_dev without a helper
variable.
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index 42b7f87ef2cd..95a8f790eaff 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1027,12 +1027,13 @@ static void applespi_debug_update_dimensions(struct applespi_data *applespi,
static int applespi_tp_dim_open(struct inode *inode, struct file *file)
{
struct applespi_data *applespi = inode->i_private;
+ struct input_dev *touchpad = applespi->touchpad_input_dev;
file->private_data = applespi;
snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
"0x%.4x %dx%d+%u+%u\n",
- applespi->touchpad_input_dev->id.product,
+ touchpad->id.product,
applespi->tp_dim_min_x, applespi->tp_dim_min_y,
applespi->tp_dim_max_x - applespi->tp_dim_min_x,
applespi->tp_dim_max_y - applespi->tp_dim_min_y);
@@ -2015,6 +2016,7 @@ static struct spi_driver applespi_driver = {
.name = "applespi",
.acpi_match_table = applespi_acpi_match,
.pm = pm_sleep_ptr(&applespi_pm_ops),
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
},
.probe = applespi_probe,
.remove = applespi_remove,
--
2.39.5
^ permalink raw reply related
* [PATCH v2 5/5] Input: applespi - fix use-after-free in applespi_remove()
From: Shih-Yuan Lee @ 2026-07-20 10:14 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mark Brown, linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260720101435.13612-1-fourdollars@debian.org>
applespi_remove() called applespi_drain_writes() to wait for in-flight
write transfers, then immediately called acpi_disable_gpe() and
acpi_remove_gpe_handler(). However it then called applespi_drain_reads()
*after* the GPE handler was removed, which races with any read SPI
completion callback that could still reference the applespi struct
already being torn down.
Moreover, the two drain helpers use separate wait paths that can miss
each other: a read completion arriving just after drain_writes() returns
but before drain_reads() is called will set read_active, and the
subsequent drain_reads() will then wait on a wait_queue that nobody will
ever wake because the GPE is already gone.
Fix by replacing the two separate drain calls with a single barrier
using the existing cancel_spi + wait_event_lock_irq mechanism:
- Set cancel_spi = true under the spinlock so that applespi_async()
immediately rejects new SPI submissions and wakes the wait queue
once all outstanding operations have drained.
- Wait for !applespi_async_outstanding() before proceeding with
teardown.
- Disable the GPE and remove its handler only after all in-flight SPI
transfers have completed, eliminating the use-after-free window.
Fixes: 0b7a8ac72fc1 ("Input: applespi - add driver for Apple SPI keyboard and touchpad")
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
drivers/input/keyboard/applespi.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index 95a8f790eaff..088337f060b2 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1910,15 +1910,22 @@ static void applespi_drain_reads(struct applespi_data *applespi)
static void applespi_remove(struct spi_device *spi)
{
struct applespi_data *applespi = spi_get_drvdata(spi);
+ unsigned long flags;
- applespi_drain_writes(applespi);
+ /* Prevent any new SPI transfers and wait for outstanding ones */
+ spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+ applespi->cancel_spi = true;
+ wait_event_lock_irq_timeout(applespi->wait_queue,
+ !applespi_async_outstanding(applespi),
+ applespi->cmd_msg_lock,
+ msecs_to_jiffies(3000));
+ spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+ /* Disable GPE and remove handler */
acpi_disable_gpe(NULL, applespi->gpe);
acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
device_wakeup_disable(&spi->dev);
- applespi_drain_reads(applespi);
-
debugfs_remove_recursive(applespi->debugfs_root);
}
--
2.39.5
^ permalink raw reply related
* Re: [PATCH v2 1/5] Input: applespi - use unified wait queue with timeouts for drain
From: sashiko-bot @ 2026-07-20 10:25 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720101435.13612-2-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Ignoring the return value of `wait_event_lock_irq_timeout()` allows driver unbinding to complete while asynchronous SPI transfers are still in flight, leading to Use-After-Free and DMA memory corruption.
--
--- Patch [1]: [PATCH v2 1/5] Input: applespi - use unified wait queue with timeouts for drain ---
commit 61741ce5d99a882b41af91bac9c6e65e14a201cf
Author: Shih-Yuan Lee <fourdollars@debian.org>
Input: applespi - use unified wait queue with timeouts for drain
This commit consolidates the wait queues by renaming drain_complete to
wait_queue and replaces wait_event_lock_irq() with
wait_event_lock_irq_timeout() using a 3-second timeout during read and
write drains. This aims to prevent indefinite lockups during driver unbind
or PM transitions if the hardware is unresponsive.
> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index b5ff71cd5a701..64bbeba85ea9e 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
[ ... ]
> @@ -1793,21 +1793,33 @@ static int applespi_probe(struct spi_device *spi)
>
> static void applespi_drain_writes(struct applespi_data *applespi)
> {
> - guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
>
> applespi->drain = true;
> - wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
> - applespi->cmd_msg_lock);
> + wait_event_lock_irq_timeout(applespi->wait_queue,
> + !applespi->write_active,
> + applespi->cmd_msg_lock,
> + msecs_to_jiffies(3000));
[Severity: High]
If this timeout expires (returning 0), the function will return while
applespi->write_active is still true.
During driver unbind, applespi_remove() calls applespi_drain_writes(). If
it returns early due to a timeout, applespi_remove() will complete, and
devres will free the devm-managed applespi_data structure and DMA buffers
like tx_buffer and rx_buffer.
Won't the SPI controller continue performing DMA into this freed memory?
Additionally, when the SPI transfer eventually completes, the SPI core
invokes the completion callback which dereferences the freed context:
applespi_async_write_complete(void *context) {
struct applespi_data *applespi = context;
enum applespi_evt_type evt_type = applespi->cmd_evt_type;
...
applespi_get_trace_fun(evt_type)(evt_type, PT_WRITE,
applespi->tx_buffer,
APPLESPI_PACKET_SIZE);
...
}
Could this lead to a use-after-free and DMA memory corruption? Should the
timeout return value be checked and handled, perhaps by canceling in-flight
SPI messages before returning?
> +
> + spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
> }
>
> static void applespi_drain_reads(struct applespi_data *applespi)
> {
> - guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
>
> - wait_event_lock_irq(applespi->drain_complete, !applespi->read_active,
> - applespi->cmd_msg_lock);
> + wait_event_lock_irq_timeout(applespi->wait_queue,
> + !applespi->read_active,
> + applespi->cmd_msg_lock,
> + msecs_to_jiffies(3000));
[Severity: High]
Similar to the drain_writes case above, if this wait times out, the function
returns while applespi->read_active is still true.
Does this expose the same use-after-free and memory corruption risks during
unbind if devm-managed structures are freed while reads are still in flight?
>
> applespi->suspended = true;
> +
> + spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720101435.13612-1-fourdollars@debian.org?part=1
^ permalink raw reply
* Re: [PATCH v2 3/5] Input: applespi - register touchpad synchronously in probe
From: sashiko-bot @ 2026-07-20 10:30 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720101435.13612-4-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Critical] A race condition in the SPI completion handler causes a Use-After-Free of `applespi` when the probe thread fails.
- [Critical] Ignoring the timeout in `applespi_drain_writes` and `applespi_drain_reads` allows the driver to unbind while SPI DMA transfers are active.
- [High] Reading the `tp_dim` debugfs file triggers a NULL pointer dereference if the touchpad info packet times out during probe.
- [Medium] Data Race when reading `rcvd_tp_info` without `cmd_msg_lock` in probe.
Pre-existing issues:
- [High] Failure to call `acpi_finish_gpe()` if `applespi_async()` fails in the GPE interrupt handler.
--
--- Patch [3]: [PATCH v2 3/5] Input: applespi - register touchpad synchronously in probe ---
commit d8377d333c1aa1ce257aedd6f29080dfb8f5320b
Author: Shih-Yuan Lee <fourdollars@debian.org>
Input: applespi - register touchpad synchronously in probe
This commit removes the asynchronous worker for touchpad registration.
It replaces it with synchronous registration during driver probe, waiting up
to 3 seconds for the device info packet and falling back to keyboard-only
mode on timeout.
> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
[ ... ]
> @@ -1821,6 +1814,20 @@ static int applespi_probe(struct spi_device *spi)
> /* trigger touchpad setup */
> applespi_init(applespi, false);
>
> + /* set up the touchpad as a separate input device if info is received */
> + sts = wait_event_timeout(applespi->wait_queue,
> + READ_ONCE(applespi->have_tp_info),
> + msecs_to_jiffies(3000));
> + if (!sts) {
> + dev_warn(&applespi->spi->dev,
> + "Timed out waiting for touchpad info, continuing keyboard-only\n");
[Severity: High]
If this timeout occurs, applespi->touchpad_input_dev remains null.
Will this cause a kernel panic later?
When a user reads the debugfs file, applespi_tp_dim_open() unconditionally
dereferences it:
snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
"0x%.4x %dx%d+%u+%u\n",
applespi->touchpad_input_dev->id.product,
...
> + } else {
> + sts = applespi_register_touchpad_device(applespi,
> + &applespi->rcvd_tp_info);
[Severity: Medium]
Is there a data race when reading rcvd_tp_info here?
The commit message mentions protecting rcvd_tp_info under cmd_msg_lock,
but applespi_register_touchpad_device() extracts the model info from it
without holding the lock. If the device sends another unexpected touchpad
info packet, could the SPI callback overwrite rcvd_tp_info concurrently?
> + if (sts)
> + goto cancel_spi;
> + }
> +
[ ... ]
> @@ -1853,6 +1860,19 @@ static int applespi_probe(struct spi_device *spi)
> &applespi_tp_dim_fops);
>
> return 0;
> +
> +cancel_spi:
> + acpi_disable_gpe(NULL, applespi->gpe);
> + acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
[Severity: High]
This is a pre-existing issue, but does applespi_notify() have a bug in its
error handling path that could cause a permanent GPE stall?
If applespi_async() fails in applespi_notify() (e.g. transient out of
memory), it returns ACPI_INTERRUPT_HANDLED but misses queuing the async
operation:
sts = applespi_async(applespi, &applespi->rd_m,
applespi_async_read_complete);
if (sts)
dev_warn(&applespi->spi->dev,
"Error queueing async read to device: %d\n", sts);
Because the async task was not queued, applespi_async_read_complete() is
never called. This means acpi_finish_gpe() is never invoked, permanently
leaving the level-triggered GPE unfinished and breaking all input.
> +
> + spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
> + applespi->cancel_spi = true;
> + wait_event_lock_irq(applespi->wait_queue,
> + !applespi_async_outstanding(applespi),
> + applespi->cmd_msg_lock);
> + spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
> +
> + return sts;
[Severity: Critical]
Does this error path introduce a use-after-free for applespi?
When wait_event_lock_irq completes, this probe thread returns an error
and devres frees the applespi structure. However, in
applespi_async_complete():
if (applespi->cancel_spi && !applespi_async_outstanding(applespi))
wake_up_all(&applespi->wait_queue);
spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
if (complete)
complete(applespi);
The completion thread wakes the wait queue and drops the lock. This allows
the probe thread to resume, return, and trigger the devres free operation
before the completion thread executes complete(applespi), leading to a
use-after-free.
> }
>
> static void applespi_drain_writes(struct applespi_data *applespi)
[Severity: Critical]
I noticed a regression here. Does applespi_drain_writes() allow the driver
to unbind while SPI DMA transfers are still active?
It uses a timeout when waiting for writes to finish:
wait_event_lock_irq_timeout(applespi->wait_queue,
!applespi->write_active,
applespi->cmd_msg_lock,
msecs_to_jiffies(3000));
spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
Since the return value of wait_event_lock_irq_timeout() is ignored, if the
SPI hardware is delayed and the timeout expires, this function silently
returns as if the drain succeeded. When applespi_remove() completes,
devres frees the memory. Once the hardware finally processes the transfer,
it will DMA to/from freed memory and execute the completion callback.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720101435.13612-1-fourdollars@debian.org?part=3
^ permalink raw reply
* Re: [PATCH v2 5/5] Input: applespi - fix use-after-free in applespi_remove()
From: sashiko-bot @ 2026-07-20 10:30 UTC (permalink / raw)
To: Shih-Yuan Lee; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260720101435.13612-6-fourdollars@debian.org>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] applespi_async_complete() triggers a use-after-free by signaling completion before actually executing the callback.
- [High] Ignored timeout in `wait_event_lock_irq_timeout` leads to a Use-After-Free on outstanding SPI transfers.
- [High] Setting `cancel_spi = true` before disabling the level-triggered GPE causes an interrupt storm.
--
--- Patch [5]: [PATCH v2 5/5] Input: applespi - fix use-after-free in applespi_remove() ---
commit 8828f1f25e357bbbc7a4945659f2ddf6d10cda5c
Author: Shih-Yuan Lee <fourdollars@debian.org>
Input: applespi - fix use-after-free in applespi_remove()
This commit replaces the separate read and write drain helpers with a single
barrier using the existing cancel_spi and wait_event_lock_irq mechanism. It
waits for outstanding asynchronous operations to complete before disabling
the GPE and tearing down the device.
> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index 95a8f790eaffc..088337f060b24 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
> @@ -1910,15 +1910,22 @@ static void applespi_drain_reads(struct applespi_data *applespi)
> static void applespi_remove(struct spi_device *spi)
> {
> struct applespi_data *applespi = spi_get_drvdata(spi);
> + unsigned long flags;
>
> - applespi_drain_writes(applespi);
> + /* Prevent any new SPI transfers and wait for outstanding ones */
> + spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
> + applespi->cancel_spi = true;
[Severity: High]
If we set cancel_spi to true while the level-triggered GPE is still enabled,
could this cause an interrupt storm?
When the GPE fires, applespi_notify() attempts to queue a read via
applespi_async(), which immediately rejects it with -ESHUTDOWN. Because the
read is rejected, the trackpad hardware never deasserts the interrupt line.
Would it be safer to disable the GPE before setting this flag?
> + wait_event_lock_irq_timeout(applespi->wait_queue,
> + !applespi_async_outstanding(applespi),
> + applespi->cmd_msg_lock,
> + msecs_to_jiffies(3000));
[Severity: High]
If the wait times out (returning 0), the driver continues tearing down and
returns, allowing devm to free the memory. If the delayed SPI transfer
completes shortly after, won't that result in a use-after-free in the SPI
core or applespi_async_complete() when it accesses the freed driver data?
[Severity: High]
Does this wait condition race with the inner completion callback?
Looking at applespi_async_complete(), it signals completion before executing
the inner callback:
drivers/input/keyboard/applespi.c:applespi_async_complete() {
...
info->complete = NULL;
if (applespi->cancel_spi && !applespi_async_outstanding(applespi))
wake_up_all(&applespi->wait_queue);
spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
if (complete)
complete(applespi);
...
}
If the teardown thread wakes up as soon as applespi_async_outstanding() is
false and frees the device structure, will complete(applespi) dereference
freed memory?
> + spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
>
> + /* Disable GPE and remove handler */
> acpi_disable_gpe(NULL, applespi->gpe);
> acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
> device_wakeup_disable(&spi->dev);
>
> - applespi_drain_reads(applespi);
> -
> debugfs_remove_recursive(applespi->debugfs_root);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720101435.13612-1-fourdollars@debian.org?part=5
^ 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