* Re: [PATCH v2 1/4] HID: multitouch: set INPUT_PROP_PRESSUREPAD based on Digitizer/Button Type
From: Rong Zhang @ 2026-06-16 11:16 UTC (permalink / raw)
To: Peter Hutterer
Cc: devnull+peter.hutterer.who-t.net, bentiss, dmitry.torokhov, jikos,
linux-input, linux-kernel, linux-kselftest, shuah, vadim
In-Reply-To: <ajDO531wxzVZlL4g@quokka>
Hi Peter,
On Tue, 2026-06-16 at 14:24 +1000, Peter Hutterer wrote:
> Hi Rong,
>
> On Tue, Jun 02, 2026 at 01:25:57AM +0800, Rong Zhang wrote:
> >
> > Hi all,
> >
> > Hopefully I'm not too late to show up here.
> >
> > > From: Peter Hutterer <peter.hutterer@who-t.net>
> > >
> > > A Digitizer/Button Type value of 1 indicates the device is a
> > > pressurepad, see
> > > https://learn.microsoft.com/en-us/windows-hardware/design/component-guidelines/touchpad-windows-precision-touchpad-collection#device-capabilities-feature-report
> > >
> > > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> > > ---
> > > drivers/hid/hid-multitouch.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> > > index 179dc316b4b518d78bdc900d9fd15756c5eba83e..382e6f50c4f7e663af7d028abb8be7cb2e6e7b8e 100644
> > > --- a/drivers/hid/hid-multitouch.c
> > > +++ b/drivers/hid/hid-multitouch.c
> > > @@ -81,6 +81,7 @@ MODULE_LICENSE("GPL");
> > > #define MT_INPUTMODE_TOUCHPAD 0x03
> > >
> > > #define MT_BUTTONTYPE_CLICKPAD 0
> > > +#define MT_BUTTONTYPE_PRESSUREPAD 1
> > >
> > > enum latency_mode {
> > > HID_LATENCY_NORMAL = 0,
> > > @@ -179,6 +180,7 @@ struct mt_device {
> > > __u8 inputmode_value; /* InputMode HID feature value */
> > > __u8 maxcontacts;
> > > bool is_buttonpad; /* is this device a button pad? */
> > > + bool is_pressurepad; /* is this device a pressurepad? */
> > > bool is_haptic_touchpad; /* is this device a haptic touchpad? */
> > > bool serial_maybe; /* need to check for serial protocol */
> > >
> > > @@ -530,8 +532,14 @@ static void mt_feature_mapping(struct hid_device *hdev,
> > > }
> > >
> > > mt_get_feature(hdev, field->report);
> > > - if (field->value[usage->usage_index] == MT_BUTTONTYPE_CLICKPAD)
> > > + switch (field->value[usage->usage_index]) {
> > > + case MT_BUTTONTYPE_CLICKPAD:
> > > td->is_buttonpad = true;
> > > + break;
> > > + case MT_BUTTONTYPE_PRESSUREPAD:
> > > + td->is_pressurepad = true;
> > > + break;
> > > + }
> > >
> > > break;
> > > case 0xff0000c5:
> > > @@ -1393,6 +1401,8 @@ static int mt_touch_input_configured(struct hid_device *hdev,
> > >
> > > if (td->is_buttonpad)
> > > __set_bit(INPUT_PROP_BUTTONPAD, input->propbit);
> > > + if (td->is_pressurepad)
> > > + __set_bit(INPUT_PROP_PRESSUREPAD, input->propbit);
> >
> > I noticed that this leads to dual reporting on my device.
> >
> > Consider previous checks:
> >
> > if (application == HID_DG_TOUCHPAD) {
> > mt_application->mt_flags |= INPUT_MT_POINTER;
> > td->inputmode_value = MT_INPUTMODE_TOUCHPAD;
> > }
> >
> > ...
> >
> > /* check for clickpads */
> > if ((app->mt_flags & INPUT_MT_POINTER) &&
> > (app->buttons_count == 1))
> > td->is_buttonpad = true;
> >
> > ... where `td->is_buttonpad' is set to true when a pressure pad has only
> > one button, i.e., the "touchpad button integrated with digitizer" [1].
> > Most (if not all) pressure pads fall into this category. As a result,
> > the presence of INPUT_PROP_PRESSUREPAD is always accompanied by the
> > presence of INPUT_PROP_BUTTONPAD.
>
> Yes, this is intended, see commit
> ae8966b7b5bd69b86209cc34bcca1ba9f18b68e6 which lists this in the commit
> message:
>
> ```
> This means:
> - clickpad: INPUT_PROP_BUTTONPAD
> - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD
> - pressurepad with configurable haptics:
> INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD + FF_HAPTIC
> ```
>
> We have to keep setting BUTTONPAD on all pressurepads because otherwise
> we'd break existing userspace which relies on this.
Thanks a lot for your explanation. It makes sense to me.
Thanks,
Rong
>
> Cheers,
> Peter
>
^ permalink raw reply
* Re: [PATCH 01/11] iio: hid-sensors: add missing blank line after declarations
From: sashiko-bot @ 2026-06-16 11:22 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-1-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but pending asynchronous suspend work is cancelled during device removal, leaving hardware powered on indefinitely.
--
commit fb90a2bc06515600f67795b51872db81c06e23a2
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: hid-sensors: add missing blank line after declarations
Add a blank line after variable declarations to improve
readability and align with kernel coding style.
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 417c4ab8c1b27..c8ccf96f3d035 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -314,7 +314,9 @@ static int __maybe_unused hid_sensor_resume(struct device *dev)
> {
> struct iio_dev *indio_dev = dev_get_drvdata(dev);
> struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
> +
> schedule_work(&attrb->work);
> +
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem where
pending asynchronous suspend work might be cancelled during device removal,
leaving the hardware powered on indefinitely.
During driver removal, iio_device_unregister() disables active iio buffers,
triggering buffer_predisable(). This calls hid_sensor_power_state() with
state false, which queues an asynchronous suspend:
hid_sensor_power_state() {
...
} else {
atomic_dec(&st->user_requested_state);
pm_runtime_use_autosuspend(&st->pdev->dev);
ret = pm_runtime_put_autosuspend(&st->pdev->dev);
}
...
}
Immediately after, hid_sensor_remove_trigger() is called:
hid_sensor_remove_trigger() {
if (atomic_read(&attrb->runtime_pm_enable))
pm_runtime_disable(&attrb->pdev->dev);
pm_runtime_set_suspended(&attrb->pdev->dev);
...
}
Because pm_runtime_disable() cancels any pending asynchronous idle/suspend
work, is it possible the autosuspend work gets cancelled before execution?
If the work is cancelled, pm_runtime_set_suspended() would only update the
software pm state, leaving the actual physical hardware permanently powered
on. Does the driver need a synchronous suspend in the removal path before
disabling runtime pm?
>
> @@ -322,6 +324,7 @@ static int __maybe_unused hid_sensor_runtime_resume(struct device *dev)
> {
> struct iio_dev *indio_dev = dev_get_drvdata(dev);
> struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
> +
> return _hid_sensor_power_state(attrb, true);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH 04/11] iio: magnetometer: hid-sensor-magn-3d: align parenthesis for readability
From: sashiko-bot @ 2026-06-16 11:19 UTC (permalink / raw)
To: Sanjay Chitroda; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-4-0cd544286575@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Blindly reading 32 or 64 bits from `raw_data` without verifying `raw_len` can lead to slab-out-of-bounds reads.
--
commit 13804edb2c46947f67fe0d4d5d050f780422d2f7
Author: Sanjay Chitroda <sanjayembeddedse@gmail.com>
iio: magnetometer: hid-sensor-magn-3d: align parenthesis for readability
This commit adjusts the alignment of parentheses across multiple functions in
the hid-sensor-magn-3d driver. This improves readability and maintains
consistency with the kernel coding style.
> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index 23884825eb00f..82507297c55bb 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
[ ... ]
> @@ -292,9 +291,9 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
>
> /* Capture samples in local storage */
> static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
> - u32 usage_id,
> - size_t raw_len, char *raw_data,
> - void *priv)
> + u32 usage_id,
> + size_t raw_len, char *raw_data,
> + void *priv)
> {
> struct iio_dev *indio_dev = platform_get_drvdata(priv);
> struct magn_3d_state *magn_state = iio_priv(indio_dev);
[Severity: High]
This is a pre-existing issue, but since this patch touches the signature of
magn_3d_capture_sample(), could we be at risk of a slab out-of-bounds read
further down in the function?
If a device presents a crafted HID report descriptor that defines the
HID_USAGE_SENSOR_TIME_TIMESTAMP field with a smaller size, it could result in
raw_len being less than 8. Reading 8 bytes from raw_data in this case:
case HID_USAGE_SENSOR_TIME_TIMESTAMP:
magn_state->timestamp =
hid_sensor_convert_timestamp(&magn_state->magn_flux_attributes,
*(s64 *)raw_data);
would read past the end of the buffer.
A similar dereference happens later in the function for the magnetic axes
where 4 bytes are read:
iio_val = magn_state->magn_val_addr[offset];
if (iio_val != NULL)
*iio_val = *((u32 *)raw_data);
Could this also trigger an out-of-bounds read if raw_len is smaller than 4?
Should we verify the size of raw_len before making these casts?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com?part=4
^ permalink raw reply
* [PATCH 11/11] iio: hid-sensor-accel-3d: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 11:11 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada, David Lechner, Nuno Sá,
Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 42c4259bf209..0999f7dbf434 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -329,7 +329,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct accel_3d_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* [PATCH 11/11] iio: hid-sensor-accel-3d: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 11:01 UTC (permalink / raw)
To: jikos, jic23, srinivas.pandruvada, David Lechner, Nuno Sá,
Andy Shevchenko
Cc: linux-input, linux-iio, linux-kernel
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-950d25a05d9a@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/accel/hid-sensor-accel-3d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
index 42c4259bf209..0999f7dbf434 100644
--- a/drivers/iio/accel/hid-sensor-accel-3d.c
+++ b/drivers/iio/accel/hid-sensor-accel-3d.c
@@ -329,7 +329,7 @@ static int hid_accel_3d_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct accel_3d_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* Re: [PATCH 08/11] iio: hid-sensor-magn-3d: use ! instead of explicit NULL check
From: Joshua Crofts @ 2026-06-16 11:01 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-8-0cd544286575@gmail.com>
On Tue, 16 Jun 2026 15:56:13 +0530
Sanjay Chitroda <sanjayembeddedse@gmail.com> wrote:
> From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>
> Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
> the preferred kernel style.
>
> No functional change.
>
> Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
> ---
Couldn't all of these null check patches be squashed into one,
considering these are all the same logical change and affect
HID drivers? Surely this is code churn a bit.
Same goes for the parentheses alignment patches.
--
Kind regards
CJD
^ permalink raw reply
* Re: [PATCH 02/11] iio: humidity: hid-sensor-humidity: align parenthesis for readability
From: Andy Shevchenko @ 2026-06-16 10:57 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-2-0cd544286575@gmail.com>
On Tue, Jun 16, 2026 at 03:56:07PM +0530, Sanjay Chitroda wrote:
> Adjust alignment of parentheses to improve readability and
> maintain consistency with kernel coding style
Same comment about punctuation.
...
> /* Callback handler to send event after all samples are received and captured */
> static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
> - unsigned int usage_id, void *pdev)
> + unsigned int usage_id, void *pdev)
Shouldn't usage_id be u32 to begin with? Can you check what's going on there?
(It used to be a change to implement that, maybe it was not comprehensive one and
needs a part 2?)
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 03/11] iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
From: Andy Shevchenko @ 2026-06-16 10:56 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada, linux-iio, linux-kernel,
linux-input
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-3-0cd544286575@gmail.com>
On Tue, Jun 16, 2026 at 03:56:08PM +0530, Sanjay Chitroda wrote:
> Adjust alignment of parentheses to improve readability and
> maintain consistency with kernel coding style
Respect English grammar and punctuation.
...
> /* Channel write_raw handler */
> static int gyro_3d_write_raw(struct iio_dev *indio_dev,
> - struct iio_chan_spec const *chan,
> - int val,
> - int val2,
> - long mask)
> + struct iio_chan_spec const *chan,
> + int val,
> + int val2,
> + long mask)
In cases like this have a chance to (re-)split logically.
static int gyro_3d_write_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int val, int val2, long mask)
...
These comments for the entire series.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 4/4] arm64: dts: qcom: Add PMIH0108 haptics device node
From: Konrad Dybcio @ 2026-06-16 10:27 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-4-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add haptics device node in the PMIH0108 PMIC base dtsi files, and enable
> it on several boards according to the LRA (Linear Resonant Actuator)
> component mounted on each of them.
>
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/kaanapali-mtp.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/kaanapali-qrd.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi | 9 +++++++++
> arch/arm64/boot/dts/qcom/pmih0108.dtsi | 9 +++++++++
> arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 7 +++++++
> arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 7 +++++++
One commit per board, please
> 6 files changed, 46 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> index 07247dc98b70..7e3f59fc008e 100644
> --- a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
> @@ -952,6 +952,13 @@ wifi@0 {
> };
> };
>
> +&pmih0108_e1_haptics {
> + status = "okay";
'status' should go last
> +
> + qcom,lra-period-us = <6667>;
> + qcom,vmax-mv = <3600>;
Do these properties depend on the physical characteristics on what's
connected to the other end of the haptics driver?
Konrad
^ permalink raw reply
* [PATCH 10/11] iio: hid-sensor-incl-3d: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/orientation/hid-sensor-incl-3d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c
index c7fbff498be7..4c4b0ef2e01c 100644
--- a/drivers/iio/orientation/hid-sensor-incl-3d.c
+++ b/drivers/iio/orientation/hid-sensor-incl-3d.c
@@ -307,7 +307,7 @@ static int hid_incl_3d_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct incl_3d_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* [PATCH 09/11] iio: hid-sensor-rotation: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/orientation/hid-sensor-rotation.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/orientation/hid-sensor-rotation.c b/drivers/iio/orientation/hid-sensor-rotation.c
index 20563d8efaf6..fc925ec12d37 100644
--- a/drivers/iio/orientation/hid-sensor-rotation.c
+++ b/drivers/iio/orientation/hid-sensor-rotation.c
@@ -278,7 +278,7 @@ static int hid_dev_rot_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct dev_rot_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* [PATCH 08/11] iio: hid-sensor-magn-3d: use ! instead of explicit NULL check
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Replace 'if (indio_dev == NULL)' with 'if (!indio_dev)' in to follow
the preferred kernel style.
No functional change.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 82507297c55b..8f7a0b2bd795 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -467,7 +467,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
indio_dev = devm_iio_device_alloc(&pdev->dev,
sizeof(struct magn_3d_state));
- if (indio_dev == NULL)
+ if (!indio_dev)
return -ENOMEM;
platform_set_drvdata(pdev, indio_dev);
--
2.34.1
^ permalink raw reply related
* [PATCH 07/11] iio: temperature: hid-sensor-temperature: use common device for devres
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
kmemdup() is used for memory that is logically tied to the HID
platform device, even though the driver binds into the IIO framework.
Using &indio_dev->dev for devres allocations works functionally, but it
results in two separate devres ownership trees—one for the HID
platform device (pdev) and another for the IIO device (indio_dev).
The devres framework is intended to have a single, well-defined parent
device. Since the memory originates from HID sensor probing and is not
IIO-specific, &pdev->dev is the correct and logical owner.
Switch to using the platform device for devm_kmemdup() so that all
resources are released deterministically and consistently.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/temperature/hid-sensor-temperature.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c
index 9f628a8e5cfb..31ebfd1a1fe2 100644
--- a/drivers/iio/temperature/hid-sensor-temperature.c
+++ b/drivers/iio/temperature/hid-sensor-temperature.c
@@ -213,8 +213,8 @@ static int hid_temperature_probe(struct platform_device *pdev)
if (ret)
return ret;
- temp_chans = devm_kmemdup(&indio_dev->dev, temperature_channels,
- sizeof(temperature_channels), GFP_KERNEL);
+ temp_chans = devm_kmemdup(&pdev->dev, temperature_channels,
+ sizeof(temperature_channels), GFP_KERNEL);
if (!temp_chans)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH 06/11] iio: position: hid-sensor-custom-intel-hinge: use common device for devres
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
kmemdup() is used for memory that is logically tied to the HID
platform device, even though the driver binds into the IIO framework.
Using &indio_dev->dev for devres allocations works functionally, but it
results in two separate devres ownership trees—one for the HID
platform device (pdev) and another for the IIO device (indio_dev).
The devres framework is intended to have a single, well-defined parent
device. Since the memory originates from HID sensor probing and is not
IIO-specific, &pdev->dev is the correct and logical owner.
Switch to using the platform device for devm_kmemdup() so that all
resources are released deterministically and consistently.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/position/hid-sensor-custom-intel-hinge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/position/hid-sensor-custom-intel-hinge.c b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
index a26d391661fd..0ba9d2d43913 100644
--- a/drivers/iio/position/hid-sensor-custom-intel-hinge.c
+++ b/drivers/iio/position/hid-sensor-custom-intel-hinge.c
@@ -292,7 +292,7 @@ static int hid_hinge_probe(struct platform_device *pdev)
}
indio_dev->num_channels = ARRAY_SIZE(hinge_channels);
- indio_dev->channels = devm_kmemdup(&indio_dev->dev, hinge_channels,
+ indio_dev->channels = devm_kmemdup(&pdev->dev, hinge_channels,
sizeof(hinge_channels), GFP_KERNEL);
if (!indio_dev->channels)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH 05/11] iio: humidity: hid-sensor-humidity: use common device for devres
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda, Zhang Lixu
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
kmemdup() is used for memory that is logically tied to the HID
platform device, even though the driver binds into the IIO framework.
Using &indio_dev->dev for devres allocations works functionally, but it
results in two separate devres ownership trees—one for the HID
platform device (pdev) and another for the IIO device (indio_dev).
The devres framework is intended to have a single, well-defined parent
device. Since the memory originates from HID sensor probing and is not
IIO-specific, &pdev->dev is the correct and logical owner.
Switch to using the platform device for devm_kmemdup() so that all
resources are released deterministically and consistently.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Tested-by: Zhang Lixu <lixu.zhang@intel.com>
---
drivers/iio/humidity/hid-sensor-humidity.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index 6fed019f4099..73120b87d641 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -215,7 +215,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
if (ret)
return ret;
- humid_chans = devm_kmemdup(&indio_dev->dev, humidity_channels,
+ humid_chans = devm_kmemdup(&pdev->dev, humidity_channels,
sizeof(humidity_channels), GFP_KERNEL);
if (!humid_chans)
return -ENOMEM;
--
2.34.1
^ permalink raw reply related
* [PATCH 04/11] iio: magnetometer: hid-sensor-magn-3d: align parenthesis for readability
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 78 +++++++++++++--------------
1 file changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index 23884825eb00..82507297c55b 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -135,9 +135,9 @@ static const struct iio_chan_spec magn_3d_channels[] = {
/* Channel read_raw handler */
static int magn_3d_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2,
- long mask)
+ struct iio_chan_spec const *chan,
+ int *val, int *val2,
+ long mask)
{
struct magn_3d_state *magn_state = iio_priv(indio_dev);
int report_id = -1;
@@ -167,8 +167,7 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
false);
return -EINVAL;
}
- hid_sensor_power_state(&magn_state->magn_flux_attributes,
- false);
+ hid_sensor_power_state(&magn_state->magn_flux_attributes, false);
ret_type = IIO_VAL_INT;
break;
case IIO_CHAN_INFO_SCALE:
@@ -229,10 +228,10 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
/* Channel write_raw handler */
static int magn_3d_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val,
- int val2,
- long mask)
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
{
struct magn_3d_state *magn_state = iio_priv(indio_dev);
int ret = 0;
@@ -270,8 +269,8 @@ static const struct iio_info magn_3d_info = {
/* Callback handler to send event after all samples are received and captured */
static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
- u32 usage_id,
- void *priv)
+ u32 usage_id,
+ void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct magn_3d_state *magn_state = iio_priv(indio_dev);
@@ -292,9 +291,9 @@ static int magn_3d_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int magn_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
- u32 usage_id,
- size_t raw_len, char *raw_data,
- void *priv)
+ u32 usage_id,
+ size_t raw_len, char *raw_data,
+ void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct magn_3d_state *magn_state = iio_priv(indio_dev);
@@ -353,11 +352,12 @@ static int magn_3d_parse_report(struct platform_device *pdev,
u32 address = magn_3d_addresses[i];
/* Check if usage attribute exists in the sensor hub device */
- status = sensor_hub_input_get_attribute_info(hsdev,
- HID_INPUT_REPORT,
- usage_id,
- address,
- &(st->magn[i]));
+ status = sensor_hub_input_get_attribute_info(
+ hsdev,
+ HID_INPUT_REPORT,
+ usage_id,
+ address,
+ &(st->magn[i]));
if (!status)
attr_count++;
}
@@ -368,18 +368,17 @@ static int magn_3d_parse_report(struct platform_device *pdev,
return -EINVAL;
}
- dev_dbg(&pdev->dev, "magn_3d Found %d usage attributes\n",
- attr_count);
+ 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",
- 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);
+ 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);
+ sizeof(struct iio_chan_spec),
+ GFP_KERNEL);
if (!_channels) {
dev_err(&pdev->dev,
"failed to allocate space for iio channels\n");
@@ -426,8 +425,7 @@ static int magn_3d_parse_report(struct platform_device *pdev,
*channels = _channels;
- dev_dbg(&pdev->dev, "magn_3d Setup %d IIO channels\n",
- *chan_count);
+ dev_dbg(&pdev->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,
@@ -442,7 +440,8 @@ static int magn_3d_parse_report(struct platform_device *pdev,
&st->rot_attr.scale_post_decml);
if (st->rot_attributes.sensitivity.index < 0) {
- sensor_hub_input_get_attribute_info(hsdev,
+ sensor_hub_input_get_attribute_info(
+ hsdev,
HID_FEATURE_REPORT, usage_id,
HID_USAGE_SENSOR_DATA_MOD_CHANGE_SENSITIVITY_ABS |
HID_USAGE_SENSOR_ORIENT_COMP_MAGN_NORTH,
@@ -477,11 +476,12 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
magn_state->magn_flux_attributes.hsdev = hsdev;
magn_state->magn_flux_attributes.pdev = pdev;
- ret = hid_sensor_parse_common_attributes(hsdev,
- HID_USAGE_SENSOR_COMPASS_3D,
- &magn_state->magn_flux_attributes,
- magn_3d_sensitivity_addresses,
- ARRAY_SIZE(magn_3d_sensitivity_addresses));
+ ret = hid_sensor_parse_common_attributes(
+ hsdev,
+ HID_USAGE_SENSOR_COMPASS_3D,
+ &magn_state->magn_flux_attributes,
+ magn_3d_sensitivity_addresses,
+ ARRAY_SIZE(magn_3d_sensitivity_addresses));
if (ret) {
dev_err(&pdev->dev, "failed to setup common attributes\n");
return ret;
@@ -491,8 +491,8 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
magn_state->rot_attributes.sensitivity.index = -1;
ret = magn_3d_parse_report(pdev, hsdev,
- &channels, &chan_count,
- HID_USAGE_SENSOR_COMPASS_3D, magn_state);
+ &channels, &chan_count,
+ HID_USAGE_SENSOR_COMPASS_3D, magn_state);
if (ret) {
dev_err(&pdev->dev, "failed to parse report\n");
return ret;
@@ -507,7 +507,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
atomic_set(&magn_state->magn_flux_attributes.data_ready, 0);
ret = hid_sensor_setup_trigger(indio_dev, name,
- &magn_state->magn_flux_attributes);
+ &magn_state->magn_flux_attributes);
if (ret < 0) {
dev_err(&pdev->dev, "trigger setup failed\n");
return ret;
@@ -523,7 +523,7 @@ static int hid_magn_3d_probe(struct platform_device *pdev)
magn_state->callbacks.capture_sample = magn_3d_capture_sample;
magn_state->callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_COMPASS_3D,
- &magn_state->callbacks);
+ &magn_state->callbacks);
if (ret < 0) {
dev_err(&pdev->dev, "callback reg failed\n");
goto error_iio_unreg;
--
2.34.1
^ permalink raw reply related
* [PATCH 03/11] iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 58 +++++++++++++++++------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index bbca2111e79b..fa604cf26d06 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -85,9 +85,9 @@ static const struct iio_chan_spec gyro_3d_channels[] = {
/* Channel read_raw handler */
static int gyro_3d_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2,
- long mask)
+ struct iio_chan_spec const *chan,
+ int *val, int *val2,
+ long mask)
{
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
int report_id = -1;
@@ -112,8 +112,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
min < 0);
else {
*val = 0;
- hid_sensor_power_state(&gyro_state->common_attributes,
- false);
+ hid_sensor_power_state(&gyro_state->common_attributes, false);
return -EINVAL;
}
hid_sensor_power_state(&gyro_state->common_attributes, false);
@@ -146,10 +145,10 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
/* Channel write_raw handler */
static int gyro_3d_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val,
- int val2,
- long mask)
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
{
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
int ret = 0;
@@ -177,8 +176,8 @@ 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,
- u32 usage_id,
- void *priv)
+ u32 usage_id,
+ void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
@@ -199,9 +198,9 @@ 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,
- u32 usage_id,
- size_t raw_len, char *raw_data,
- void *priv)
+ u32 usage_id,
+ size_t raw_len, char *raw_data,
+ void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
@@ -240,11 +239,12 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
int ret;
for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
- ret = sensor_hub_input_get_attribute_info(hsdev,
- HID_INPUT_REPORT,
- usage_id,
- HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
- &st->gyro[ch]);
+ ret = sensor_hub_input_get_attribute_info(
+ hsdev,
+ HID_INPUT_REPORT,
+ usage_id,
+ HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
+ &st->gyro[ch]);
if (ret < 0)
break;
channels[ch].scan_type = (struct iio_scan_type) {
@@ -254,10 +254,10 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
};
}
dev_dbg(&pdev->dev, "gyro_3d %x:%x, %x:%x, %x:%x\n",
- st->gyro[0].index,
- st->gyro[0].report_id,
- st->gyro[1].index, st->gyro[1].report_id,
- st->gyro[2].index, st->gyro[2].report_id);
+ st->gyro[0].index,
+ st->gyro[0].report_id,
+ st->gyro[1].index, st->gyro[1].report_id,
+ st->gyro[2].index, st->gyro[2].report_id);
st->scale_precision = hid_sensor_format_scale(
HID_USAGE_SENSOR_GYRO_3D,
@@ -286,10 +286,10 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
gyro_state->common_attributes.pdev = pdev;
ret = hid_sensor_parse_common_attributes(hsdev,
- HID_USAGE_SENSOR_GYRO_3D,
- &gyro_state->common_attributes,
- gyro_3d_sensitivity_addresses,
- ARRAY_SIZE(gyro_3d_sensitivity_addresses));
+ HID_USAGE_SENSOR_GYRO_3D,
+ &gyro_state->common_attributes,
+ gyro_3d_sensitivity_addresses,
+ ARRAY_SIZE(gyro_3d_sensitivity_addresses));
if (ret) {
dev_err(&pdev->dev, "failed to setup common attributes\n");
return ret;
@@ -318,7 +318,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
atomic_set(&gyro_state->common_attributes.data_ready, 0);
ret = hid_sensor_setup_trigger(indio_dev, name,
- &gyro_state->common_attributes);
+ &gyro_state->common_attributes);
if (ret < 0) {
dev_err(&pdev->dev, "trigger setup failed\n");
return ret;
@@ -334,7 +334,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
gyro_state->callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D,
- &gyro_state->callbacks);
+ &gyro_state->callbacks);
if (ret < 0) {
dev_err(&pdev->dev, "callback reg failed\n");
goto error_iio_unreg;
--
2.34.1
^ permalink raw reply related
* [PATCH 02/11] iio: humidity: hid-sensor-humidity: align parenthesis for readability
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/humidity/hid-sensor-humidity.c | 46 +++++++++++++++---------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index be2338d5f407..6fed019f4099 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -45,7 +45,7 @@ static const struct iio_chan_spec humidity_channels[] = {
/* Adjust channel real bits based on report descriptor */
static void humidity_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
+ int channel, int size)
{
channels[channel].scan_type.sign = 's';
/* Real storage bits will change based on the report desc. */
@@ -55,8 +55,8 @@ static void humidity_adjust_channel_bit_mask(struct iio_chan_spec *channels,
}
static int humidity_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2, long mask)
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
{
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -101,8 +101,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
}
static int humidity_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val, int val2, long mask)
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
{
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -127,7 +127,7 @@ static const struct iio_info humidity_info = {
/* Callback handler to send event after all samples are received and captured */
static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, void *pdev)
+ unsigned int usage_id, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -141,8 +141,8 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, size_t raw_len,
- char *raw_data, void *pdev)
+ unsigned int usage_id, size_t raw_len,
+ char *raw_data, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -159,17 +159,17 @@ static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
/* Parse report which is specific to an usage id */
static int humidity_parse_report(struct platform_device *pdev,
- struct hid_sensor_hub_device *hsdev,
- struct iio_chan_spec *channels,
- unsigned int usage_id,
- struct hid_humidity_state *st)
+ struct hid_sensor_hub_device *hsdev,
+ struct iio_chan_spec *channels,
+ unsigned int usage_id,
+ struct hid_humidity_state *st)
{
int ret;
ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
- usage_id,
- HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
- &st->humidity_attr);
+ usage_id,
+ HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
+ &st->humidity_attr);
if (ret < 0)
return ret;
@@ -208,20 +208,20 @@ static int hid_humidity_probe(struct platform_device *pdev)
humid_st->common_attributes.pdev = pdev;
ret = hid_sensor_parse_common_attributes(hsdev,
- HID_USAGE_SENSOR_HUMIDITY,
- &humid_st->common_attributes,
- humidity_sensitivity_addresses,
- ARRAY_SIZE(humidity_sensitivity_addresses));
+ HID_USAGE_SENSOR_HUMIDITY,
+ &humid_st->common_attributes,
+ humidity_sensitivity_addresses,
+ ARRAY_SIZE(humidity_sensitivity_addresses));
if (ret)
return ret;
humid_chans = devm_kmemdup(&indio_dev->dev, humidity_channels,
- sizeof(humidity_channels), GFP_KERNEL);
+ sizeof(humidity_channels), GFP_KERNEL);
if (!humid_chans)
return -ENOMEM;
ret = humidity_parse_report(pdev, hsdev, humid_chans,
- HID_USAGE_SENSOR_HUMIDITY, humid_st);
+ HID_USAGE_SENSOR_HUMIDITY, humid_st);
if (ret)
return ret;
@@ -234,7 +234,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
atomic_set(&humid_st->common_attributes.data_ready, 0);
ret = hid_sensor_setup_trigger(indio_dev, name,
- &humid_st->common_attributes);
+ &humid_st->common_attributes);
if (ret)
return ret;
@@ -242,7 +242,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
humidity_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
- &humidity_callbacks);
+ &humidity_callbacks);
if (ret)
goto error_remove_trigger;
--
2.34.1
^ permalink raw reply related
* [PATCH 01/11] iio: hid-sensors: add missing blank line after declarations
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Add a blank line after variable declarations to improve
readability and align with kernel coding style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
changes in v4:
- Rectify commit message with input from David
---
drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 417c4ab8c1b2..c8ccf96f3d03 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -314,7 +314,9 @@ static int __maybe_unused hid_sensor_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
+
schedule_work(&attrb->work);
+
return 0;
}
@@ -322,6 +324,7 @@ static int __maybe_unused hid_sensor_runtime_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
+
return _hid_sensor_power_state(attrb, true);
}
--
2.34.1
^ permalink raw reply related
* [PATCH 00/11] HID: iio: warning clean up and prefer kernel coding style
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda, Zhang Lixu
Hi all,
This series updates HID IIO drivers to resolve checkpatch and
kernel coding style issue.
This improves readability and follow standard kernel coding style
No functional changes are introduced.
Testing:
- Compiled with W=1 for each patch in the series
---
Sanjay Chitroda (11):
iio: hid-sensors: add missing blank line after declarations
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: hid-sensor-magn-3d: use ! instead of explicit NULL check
iio: hid-sensor-rotation: use ! instead of explicit NULL check
iio: hid-sensor-incl-3d: use ! instead of explicit NULL check
iio: hid-sensor-accel-3d: use ! instead of explicit NULL check
drivers/iio/accel/hid-sensor-accel-3d.c | 2 +-
.../iio/common/hid-sensors/hid-sensor-trigger.c | 3 +
drivers/iio/gyro/hid-sensor-gyro-3d.c | 58 ++++++++--------
drivers/iio/humidity/hid-sensor-humidity.c | 48 ++++++-------
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 80 +++++++++++-----------
drivers/iio/orientation/hid-sensor-incl-3d.c | 2 +-
drivers/iio/orientation/hid-sensor-rotation.c | 2 +-
.../iio/position/hid-sensor-custom-intel-hinge.c | 2 +-
drivers/iio/temperature/hid-sensor-temperature.c | 4 +-
9 files changed, 102 insertions(+), 99 deletions(-)
---
base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
change-id: 20260615-15-jun-hid-iio-alignment-46bba7279be3
Best regards,
--
Sanjay Chitroda <sanjayembeddedse@gmail.com>
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Konrad Dybcio @ 2026-06-16 10:25 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-3-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add an initial driver for the Qualcomm PMIH010x PMIC haptics module,
> named as HAP530_HV. This module supports several play modes, including
> DIRECT_PLAY, FIFO, PAT_MEM, and SWR, each with distinct data sourcing
> and hardware data handling logic. Currently, the driver provides support
> for two play modes using the input force-feedback framework: FF_CONSTANT
> effect for DIRECT_PLAY mode and FF_PERIODIC effect with FF_CUSTOM
> waveform for FIFO mode.
>
> Assisted-by: Claude:claude-4-6-sonnet
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
[...]
> +static int cfg_write(struct qcom_haptics *h, u32 off, u32 val)
static inline
although I have mixed feelings about having so many accessors
[...]
> +static int haptics_write_fifo_chunk(struct qcom_haptics *h,
> + const s8 *data, u32 len)
> +{
> + u32 i, bulk_len = ALIGN_DOWN(len, 4);
Please avoid mixing multiple declarations and assignments
> + int ret;
> +
> + for (i = 0; i < bulk_len; i += 4) {
You can do 'int i' in loops nowadays
> + ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
> + if (ret)
> + return ret;
> + }
> +
> + for (; i < len; i++) {
> + ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
> + if (ret)
> + return ret;
> + }
So if i'm reading this right, the first loop will always write
4*(len//4) bytes and the second one will be entered at most once,
to write len rem 4 bytes.. should this be an if instead?
> +
> + return 0;
> +}
> +
> +/*
> + * Configure the hardware FIFO memory boundary.
> + * FIFO occupies addresses [0, fifo_len).
> + */
> +static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
> +{
> + u32 fifo_len, fifo_units;
> +
> + /* Config all memory space for FIFO usage for now */
What's the not-"for now" endgame for this?
> + fifo_len = HAP530_MEM_TOTAL_BYTES;
> + fifo_len = ALIGN_DOWN(fifo_len, 64);
> + fifo_units = fifo_len / 64;
> + h->fifo_len = fifo_len;
> +
> + return ptn_write(h, HAP_PTN_MMAP_FIFO_REG,
> + MMAP_FIFO_EXIST_BIT |
> + FIELD_PREP(MMAP_FIFO_LEN_MASK, fifo_units - 1));
> +}
> +
> +static u32 haptics_gain_scaled_vmax(struct qcom_haptics *h, u32 vmax_mv)
> +{
> + u32 v = (u32)((u64)vmax_mv * h->gain / 0xFFFF);
mult_frac()
> +
> + return max_t(u32, v, VMAX_STEP_MV);
> +}
> +
> +static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
> +{
> + if (h->irq_enabled == enable)
> + return;
> +
> + if (enable)
> + enable_irq(h->fifo_empty_irq);
> + else
> + disable_irq_nosync(h->fifo_empty_irq);
This is called in the .remove() path, I think you may need the
sync variant as the underlying device may be destroyed before the
ISR completes if there's a late interrupt
[...]
> +static int haptics_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> +
> + h->cur_effect_id = effect_id;
> + h->play_request = (val > 0);
> + schedule_work(&h->play_work);
> + return 0;
nit: \n before return is 'nice'
[...]
> + ret = device_property_read_u32_array(&pdev->dev, "reg", regs,
> + ARRAY_SIZE(regs));
Here you use device_property_
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to read 'reg' property\n");
> +
> + h->cfg_base = regs[0];
> + h->ptn_base = regs[1];
> +
> + ret = of_property_read_u32(h->dev->of_node, "qcom,lra-period-us",
> + &h->t_lra_us);
And here you use of_property_ (please use device_)
[...]
> + input->name = "qcom-spmi-haptics";
Odd whitespace
Konrad
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: sashiko-bot @ 2026-06-16 10:23 UTC (permalink / raw)
To: Fenglin Wu; +Cc: robh, linux-input, conor+dt, devicetree, dmitry.torokhov
In-Reply-To: <20260616-qcom-spmi-haptics-v1-3-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] Repeated playback requests for an actively playing effect leak runtime PM usage counters, permanently preventing the device from suspending.
- [High] The threaded IRQ handler sleeps in atomic context by executing sleepable regmap operations while holding a spinlock.
- [High] Incorrect teardown ordering in `remove()` and `suspend()` allows a work item to be re-queued and executed on freed memory (Use-After-Free).
- [High] Erasing or updating a force feedback effect frees the effect's data buffer synchronously, causing a Use-After-Free in the IRQ handler.
- [High] A failed memory allocation during effect upload leaves internal state corrupted, guaranteeing a NULL pointer dereference upon playback.
- [High] A lockless race condition between the input playback callback and the FIFO empty IRQ handler can cause new playback requests to be silently dropped.
- [High] A data race on `h->cur_effect_id` allows userspace to misdirect the stop sequence, leaving hardware IRQs enabled while dropping PM references.
- [Medium] Dynamically allocated custom waveform buffers are permanently leaked when the device is unbound.
- [Medium] Missing `abs()` on `s16` magnitude level causes massive arithmetic overflow for negative effects.
- [Medium] Registering the input device before PM runtime is fully configured creates a race condition for early playback events.
--
commit 84c087855458c41a5a109b1ffb5594db116b5170
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
input: misc: Add Qualcomm SPMI PMIC haptics driver
Add an initial driver for the Qualcomm PMIH010x PMIC haptics module,
named as HAP530_HV. This module supports several play modes, including
DIRECT_PLAY, FIFO, PAT_MEM, and SWR, each with distinct data sourcing
and hardware data handling logic. Currently, the driver provides support
for two play modes using the input force-feedback framework: FF_CONSTANT
effect for DIRECT_PLAY mode and FF_PERIODIC effect with FF_CUSTOM
waveform for FIFO mode.
> diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
> new file mode 100644
> index 0000000000000..75b3e338b54eb
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
> @@ -0,0 +1,831 @@
[ ... ]
> +static int haptics_start_direct_play(struct qcom_haptics *h, int effect_id)
> +{
> + struct ff_effect *ffe = &h->input->ff->effects[effect_id];
> + u8 amplitude = (u8)((u32)ffe->u.constant.level * 255 / 0x7FFF);
[Severity: Medium]
Does this cast cause an arithmetic overflow for negative effect levels?
The ffe->u.constant.level is an s16 which can be negative to represent
opposite-direction effects. Casting a negative s16 directly to u32
sign-extends it, resulting in a large unsigned value. Multiplying this by
255 overflows the 32-bit boundary, producing a garbage amplitude for the
hardware register.
Consider taking the absolute value of the level before casting and scaling.
[ ... ]
> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
> +{
> + struct qcom_haptics *h = dev_id;
> + unsigned long flags;
> + u32 sts, to_write;
> + int ret;
> +
> + ret = regmap_read(h->regmap,
> + h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
> + if (ret || !(sts & FIFO_EMPTY_BIT))
> + return IRQ_HANDLED;
> +
> + spin_lock_irqsave(&h->fifo_lock, flags);
[Severity: High]
Is it safe to hold a spinlock while calling sleepable regmap operations?
The haptics_write_fifo_chunk() function maps to regmap_bulk_write(). For an
SPMI PMIC, the regmap operates over a slow bus and takes a mutex to sleep
during transactions. Calling these sleepable functions while holding
fifo_lock results in a scheduling while atomic bug.
[ ... ]
> + if (h->data_done) {
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> + h->fifo_data = NULL;
> + h->play_request = false;
> + schedule_work(&h->play_work);
[Severity: High]
Can a lockless race condition here cause new playback requests to be silently
dropped?
When an effect completes, this sets play_request to false. Concurrently, if a
new playback request arrives, haptics_playback() sets it to true. If the IRQ
handler runs immediately after haptics_playback(), it overwrites the new true
value with false.
When haptics_play_work() executes, it would observe play_request as false,
stop the hardware, and drop the newly queued playback request.
> + spin_unlock_irqrestore(&h->fifo_lock, flags);
> + return IRQ_HANDLED;
> + }
> +
> + /* Refill: write the next chunk, conservatively sized to the threshold */
> + to_write = min_t(u32, h->data_len - h->data_written,
> + h->fifo_len - FIFO_EMPTY_THRESH);
> + haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
[Severity: High]
Is there a risk of a use-after-free here if an effect is erased while playing?
The input framework allows effect upload or erase to occur concurrently with
playback. In haptics_erase() and haptics_upload_effect(), the driver calls
kfree(priv->fifo_data) synchronously.
However, the stop request only schedules an asynchronous work item to stop
playback. Because the hardware stop is deferred, the hardware may still be
actively playing and asserting the FIFO-empty interrupt. The IRQ handler
will then attempt to read from the freed fifo_data.
[ ... ]
> +static void haptics_play_work(struct work_struct *work)
> +{
> + struct qcom_haptics *h = container_of(work, struct qcom_haptics, play_work);
> + int id, ret;
> +
> + mutex_lock(&h->play_lock);
> +
> + if (!h->play_request) {
> + haptics_stop_locked(h);
> + if (h->pm_ref_held) {
> + pm_runtime_mark_last_busy(h->dev);
> + pm_runtime_put_autosuspend(h->dev);
> + h->pm_ref_held = false;
> + }
> + goto unlock;
> + }
> +
> + ret = pm_runtime_resume_and_get(h->dev);
> + if (ret < 0) {
> + dev_err(h->dev, "failed to resume device: %d\n", ret);
> + goto unlock;
> + }
> + h->pm_ref_held = true;
[Severity: High]
Can this leak runtime PM usage counters?
The pm_ref_held flag is set to true without checking if it is already true.
If userspace issues multiple start events without interleaved stops (for
example, to update parameters), the PM usage counter is incremented multiple
times.
When playback is eventually stopped, the single check drops only one
reference via pm_runtime_put_autosuspend(), permanently leaking all other
references and preventing the device from suspending.
[ ... ]
> +static int haptics_upload_effect(struct input_dev *dev,
> + struct ff_effect *effect,
> + struct ff_effect *old)
> +{
[ ... ]
> + priv->play_rate = (u8)buf[CUSTOM_DATA_RATE_IDX];
> + priv->vmax_mv = (u32)clamp_val(buf[CUSTOM_DATA_VMAX_IDX], 0, VMAX_MV_MAX);
> + priv->data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
> +
> + kfree(priv->fifo_data);
> + priv->fifo_data = kmalloc(priv->data_len, GFP_KERNEL);
> + if (!priv->fifo_data) {
> + kfree(buf);
> + return -ENOMEM;
> + }
[Severity: High]
Could a failed memory allocation leave the internal state corrupted?
When updating an existing FF_PERIODIC effect, data_len is updated to a new
size before kmalloc() is called. If kmalloc() fails, the function returns
-ENOMEM, leaving data_len non-zero while fifo_data is NULL.
The input core keeps the existing effect ID valid. If userspace subsequently
requests to play this effect, haptics_start_fifo() computes a non-zero
init_len and passes the NULL pointer to haptics_write_fifo_chunk(), causing
a crash.
[ ... ]
> +static int haptics_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> +
> + h->cur_effect_id = effect_id;
> + h->play_request = (val > 0);
[Severity: High]
Could this lockless update of cur_effect_id misdirect the stop sequence?
If a FIFO effect is actively playing and userspace issues a playback request
for a different DIRECT_PLAY effect, cur_effect_id is updated asynchronously.
When play_work eventually processes the stop/start sequence,
haptics_stop_locked() checks the newly updated mode:
if (h->effects[h->cur_effect_id].mode == HAPTICS_FIFO) {
ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
Seeing DIRECT_PLAY, it would skip disabling the FIFO IRQ and clearing
fifo_data, while still dropping the PM reference. The device auto-suspends,
but the IRQ remains enabled and fires spuriously.
[ ... ]
> +static int qcom_haptics_probe(struct platform_device *pdev)
> +{
[ ... ]
> + ff = input->ff;
> + ff->upload = haptics_upload_effect;
> + ff->playback = haptics_playback;
> + ff->erase = haptics_erase;
> + ff->set_gain = haptics_set_gain;
[Severity: Medium]
Should the ff->destroy callback be implemented to prevent memory leaks?
Custom waveform buffers are allocated using kmalloc() and stored in
fifo_data. This memory is normally freed in haptics_erase(). However, when
the driver is unbound and the input device is unregistered, the input core
destroys the ff device but does not automatically invoke erase() for
surviving effects.
Without a destroy callback to clean up the effects array, the allocated
buffers appear to be permanently leaked.
> +
> + ret = input_register_device(input);
> + if (ret) {
> + input_ff_destroy(input);
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register input device\n");
> + }
> +
> + platform_set_drvdata(pdev, h);
> +
> + /*
> + * Grab a reference on behalf of probe (usage_count → 1), mark the
> + * device active, then enable runtime PM.
> + */
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, HAPTICS_AUTOSUSPEND_MS);
> + devm_pm_runtime_set_active_enabled(&pdev->dev);
[Severity: Medium]
Is there a race condition by registering the input device before PM runtime
is fully configured?
This call makes the force-feedback device immediately visible to userspace.
If an application sends a play request before the probe function finishes,
haptics_play_work() will run and call pm_runtime_resume_and_get(). Since PM
runtime is not yet fully enabled, the PM runtime API will return an error
and fail the playback.
[ ... ]
> +static void qcom_haptics_remove(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> +
> + cancel_work_sync(&h->play_work);
> + mutex_lock(&h->play_lock);
> + haptics_stop_locked(h);
> + haptics_enable_module(h, false);
> + mutex_unlock(&h->play_lock);
> +
> + input_unregister_device(h->input);
[Severity: High]
Does this teardown ordering allow a work item to be re-queued and executed
on freed memory?
During driver removal, cancel_work_sync() is called before the input device
is unregistered. This creates a race window where the input device is still
visible to userspace, allowing applications to issue playback events that
call schedule_work().
Once remove() completes, the devres framework frees the qcom_haptics memory.
The mistakenly re-queued background work will then execute, dereferencing
the freed driver context.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
From: sashiko-bot @ 2026-06-16 10:18 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, robh, dmitry.torokhov, linux-input, devicetree
In-Reply-To: <20260616-qcom-spmi-haptics-v1-1-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree binding description contains Linux-specific driver implementation details, violating the OS-agnostic principle.
- [Low] Stale reference to an undefined property 'qcom,wf-play-rate' in the binding description.
--
commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
Add binding document for the haptics module inside Qualcomm PMIH010X.
> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
[ ... ]
> + In FIFO streaming mode, hardware-based LRA auto-resonance tracking is
> + disabled by default. Because this mode is intended to drive arbitrary
> + waveforms that may not follow the resonant frequency, autonomous hardware
> + resonance correction would interfere with the intended output.
> +
> + In the driver, FIFO streaming is implemented using an FF_PERIODIC effect
> + with an FF_CUSTOM waveform. The expected custom data layout is:
[Severity: Low]
Are these Linux-specific driver implementation details, such as FF_PERIODIC
and FF_CUSTOM, appropriate for this binding document?
Device tree bindings are typically expected to be strictly OS-agnostic
hardware descriptions. Should the documentation for the driver's ioctl payload
and custom_data memory layout be moved to the Linux driver documentation
instead?
> + custom_data[0] = play rate code (see qcom,wf-play-rate values below)
[Severity: Low]
Is there a missing property definition for qcom,wf-play-rate?
This line references qcom,wf-play-rate values, but the property doesn't appear
to be defined anywhere in the schema's properties section. Does this need to
be added, or is this a stale reference that should be removed?
> + custom_data[1] = vmax in mV; 0 = use device default (qcom,vmax-mv)
> + custom_data[2..] = signed 8-bit PCM samples (at least one required)
> +
> +properties:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=1
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: sashiko-bot @ 2026-06-16 10:17 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, robh, linux-input, dmitry.torokhov, devicetree
In-Reply-To: <20260616-qcom-spmi-haptics-v1-2-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Use of a wildcard ('x') in the compatible string.
--
commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
Add binding document for the haptics module inside Qualcomm PMIH010X.
> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
[ ... ]
> +properties:
> + compatible:
> + const: qcom,pmih010x-haptics
[Severity: Low]
Does this introduce a regression by using a wildcard character ('x') in the
compatible string?
Device tree specifications generally forbid wildcards in compatible strings.
Should the specific model name of the first chip in the family
(e.g., "qcom,pmih0108-haptics") be used instead of a wildcard?
[ ... ]
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pmic {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + haptics@f000 {
> + compatible = "qcom,pmih010x-haptics";
[Severity: Low]
Does this example share the same regression with the wildcard compatible
string?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
From: Konrad Dybcio @ 2026-06-16 10:11 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-1-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add binding document for the haptics module inside Qualcomm PMIH010X.
>
> Assisted-by: Claude:claude-4-6-sonnet
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
[...]
> + qcom,vmax-mv:
> + description:
> + Maximum allowed output driving voltage in millivolts, rounded to the
> + nearest 50 mV step. This is the peak driving voltage in DIRECT_PLAY mode
> + which outputs sinusoidal waveforms. The value should be equal to the square
> + root of 2 times the Vrms voltage of the LRA.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 50
> + maximum: 10000
multipleOf: 50?
Konrad
^ 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