* [PATCH 0/2] iio: hid-sensor-gyro-3d: fix typo and probe race condition
@ 2026-02-28 19:13 Bhargav Joshi
2026-02-28 19:13 ` [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe() Bhargav Joshi
2026-02-28 19:14 ` [PATCH 2/2] iio: hid-sensor-gyro-3d: fix typo in array name Bhargav Joshi
0 siblings, 2 replies; 9+ messages in thread
From: Bhargav Joshi @ 2026-02-28 19:13 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, rougueprince47,
linux-input
This patch series introduces two changes to the hid-sensor-gyro-3d
driver:
- Patch 1: Move iio_device_register() to the end of the probe()
function and updates the error handling and remove() sequences to
strictly follow the LIFO teardown order.
- Patch 2: Fixes a trivial spelling mistake in a array name ('gryo' to
'gyro').
Bhargav Joshi (2):
iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
iio: hid-sensor-gyro-3d: fix typo in array name
drivers/iio/gyro/hid-sensor-gyro-3d.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
--
2.53.0
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
2026-02-28 19:13 [PATCH 0/2] iio: hid-sensor-gyro-3d: fix typo and probe race condition Bhargav Joshi
@ 2026-02-28 19:13 ` Bhargav Joshi
2026-03-01 11:44 ` Jonathan Cameron
2026-02-28 19:14 ` [PATCH 2/2] iio: hid-sensor-gyro-3d: fix typo in array name Bhargav Joshi
1 sibling, 1 reply; 9+ messages in thread
From: Bhargav Joshi @ 2026-02-28 19:13 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, rougueprince47,
linux-input
Currently, calling iio_device_register() before
sensor_hub_register_callback() may create a race condition where the
device is exposed to userspace before callbacks are wired.
Move iio_device_register() to the end of the probe() function to prevent
race condition.
Consequently, update the error handling path in probe() and in remove()
ensuring that iio_device_unregister() is called first to cut off
userspace access before the hardware callbacks are removed.
Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index c43990c518f7..8e3628cd8529 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
return ret;
}
- ret = iio_device_register(indio_dev);
- if (ret) {
- dev_err(&pdev->dev, "device register failed\n");
- goto error_remove_trigger;
- }
-
gyro_state->callbacks.send_event = gyro_3d_proc_event;
gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
gyro_state->callbacks.pdev = pdev;
@@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
&gyro_state->callbacks);
if (ret < 0) {
dev_err(&pdev->dev, "callback reg failed\n");
- goto error_iio_unreg;
+ goto error_remove_trigger;
+ }
+
+ ret = iio_device_register(indio_dev);
+ if (ret) {
+ dev_err(&pdev->dev, "device register failed\n");
+ goto error_remove_callback;
}
return ret;
-error_iio_unreg:
- iio_device_unregister(indio_dev);
+error_remove_callback:
+ sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
error_remove_trigger:
hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
return ret;
@@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
- sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
iio_device_unregister(indio_dev);
+ sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
}
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] iio: hid-sensor-gyro-3d: fix typo in array name
2026-02-28 19:13 [PATCH 0/2] iio: hid-sensor-gyro-3d: fix typo and probe race condition Bhargav Joshi
2026-02-28 19:13 ` [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe() Bhargav Joshi
@ 2026-02-28 19:14 ` Bhargav Joshi
2026-03-01 11:46 ` Jonathan Cameron
1 sibling, 1 reply; 9+ messages in thread
From: Bhargav Joshi @ 2026-02-28 19:14 UTC (permalink / raw)
To: jikos, srinivas.pandruvada, jic23
Cc: dlechner, nuno.sa, andy, linux-iio, linux-kernel, rougueprince47,
linux-input
The array 'gryo_3d_sensitivity_addresses' has a clear spelling mistake
in its prefix. Rename it to 'gyro_3d_sensitivity_addresses' to correctly
match the naming convention.
Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index 8e3628cd8529..4f88fe478d84 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -42,7 +42,7 @@ static const u32 gyro_3d_addresses[GYRO_3D_CHANNEL_MAX] = {
HID_USAGE_SENSOR_ANGL_VELOCITY_Z_AXIS
};
-static const u32 gryo_3d_sensitivity_addresses[] = {
+static const u32 gyro_3d_sensitivity_addresses[] = {
HID_USAGE_SENSOR_DATA_ANGL_VELOCITY,
};
@@ -297,8 +297,8 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
ret = hid_sensor_parse_common_attributes(hsdev,
HID_USAGE_SENSOR_GYRO_3D,
&gyro_state->common_attributes,
- gryo_3d_sensitivity_addresses,
- ARRAY_SIZE(gryo_3d_sensitivity_addresses));
+ gyro_3d_sensitivity_addresses,
+ ARRAY_SIZE(gyro_3d_sensitivity_addresses));
if (ret) {
dev_err(&pdev->dev, "failed to setup common attributes\n");
return ret;
--
2.53.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
2026-02-28 19:13 ` [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe() Bhargav Joshi
@ 2026-03-01 11:44 ` Jonathan Cameron
2026-03-01 19:30 ` Bhargav Joshi
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2026-03-01 11:44 UTC (permalink / raw)
To: Bhargav Joshi
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, linux-iio,
linux-kernel, linux-input
On Sun, 1 Mar 2026 00:43:59 +0530
Bhargav Joshi <rougueprince47@gmail.com> wrote:
> Currently, calling iio_device_register() before
> sensor_hub_register_callback() may create a race condition where the
> device is exposed to userspace before callbacks are wired.
We needs some more here on 'why' this is a problem.
Whilst this is an unusual arrangement, I couldn't immediately find
anything that was actually broken. It's possible data will turn up
after we tear down the callbacks and before the userspace interfaces
are removed, but I think that just results in dropping data. Given it's
in a race anyway I don't think we care about that. The callbacks
don't seem to be involved in device configuration which can go on whether
or not the callbacks are registered. Maybe there is something that
won't get acknowledged if they aren't in place in time?
For a fix like this we'd normally want to see a clear flow that
leads to a bug.
Also a fixes tag so we know how far to backport.
>
> Move iio_device_register() to the end of the probe() function to prevent
> race condition.
>
> Consequently, update the error handling path in probe() and in remove()
> ensuring that iio_device_unregister() is called first to cut off
> userspace access before the hardware callbacks are removed.
>
> Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
> ---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index c43990c518f7..8e3628cd8529 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> return ret;
> }
>
> - ret = iio_device_register(indio_dev);
> - if (ret) {
> - dev_err(&pdev->dev, "device register failed\n");
> - goto error_remove_trigger;
> - }
> -
> gyro_state->callbacks.send_event = gyro_3d_proc_event;
> gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
> gyro_state->callbacks.pdev = pdev;
> @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> &gyro_state->callbacks);
> if (ret < 0) {
> dev_err(&pdev->dev, "callback reg failed\n");
> - goto error_iio_unreg;
> + goto error_remove_trigger;
> + }
> +
> + ret = iio_device_register(indio_dev);
> + if (ret) {
> + dev_err(&pdev->dev, "device register failed\n");
> + goto error_remove_callback;
> }
>
> return ret;
>
> -error_iio_unreg:
> - iio_device_unregister(indio_dev);
> +error_remove_callback:
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> error_remove_trigger:
> hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> return ret;
> @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
> struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
>
> - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> iio_device_unregister(indio_dev);
> + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> }
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] iio: hid-sensor-gyro-3d: fix typo in array name
2026-02-28 19:14 ` [PATCH 2/2] iio: hid-sensor-gyro-3d: fix typo in array name Bhargav Joshi
@ 2026-03-01 11:46 ` Jonathan Cameron
0 siblings, 0 replies; 9+ messages in thread
From: Jonathan Cameron @ 2026-03-01 11:46 UTC (permalink / raw)
To: Bhargav Joshi
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, linux-iio,
linux-kernel, linux-input
On Sun, 1 Mar 2026 00:44:00 +0530
Bhargav Joshi <rougueprince47@gmail.com> wrote:
> The array 'gryo_3d_sensitivity_addresses' has a clear spelling mistake
> in its prefix. Rename it to 'gyro_3d_sensitivity_addresses' to correctly
> match the naming convention.
>
> Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
Hi Bhargav,
This stands on it's own so I've picked it up.
Applied to the togreg branch of iio.git and pushed out as testing for
all the normal reasons.
Thanks,
Jonathan
> ---
> drivers/iio/gyro/hid-sensor-gyro-3d.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index 8e3628cd8529..4f88fe478d84 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> @@ -42,7 +42,7 @@ static const u32 gyro_3d_addresses[GYRO_3D_CHANNEL_MAX] = {
> HID_USAGE_SENSOR_ANGL_VELOCITY_Z_AXIS
> };
>
> -static const u32 gryo_3d_sensitivity_addresses[] = {
> +static const u32 gyro_3d_sensitivity_addresses[] = {
> HID_USAGE_SENSOR_DATA_ANGL_VELOCITY,
> };
>
> @@ -297,8 +297,8 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> ret = hid_sensor_parse_common_attributes(hsdev,
> HID_USAGE_SENSOR_GYRO_3D,
> &gyro_state->common_attributes,
> - gryo_3d_sensitivity_addresses,
> - ARRAY_SIZE(gryo_3d_sensitivity_addresses));
> + gyro_3d_sensitivity_addresses,
> + ARRAY_SIZE(gyro_3d_sensitivity_addresses));
> if (ret) {
> dev_err(&pdev->dev, "failed to setup common attributes\n");
> return ret;
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
2026-03-01 11:44 ` Jonathan Cameron
@ 2026-03-01 19:30 ` Bhargav Joshi
2026-03-07 14:21 ` Jonathan Cameron
0 siblings, 1 reply; 9+ messages in thread
From: Bhargav Joshi @ 2026-03-01 19:30 UTC (permalink / raw)
To: Jonathan Cameron
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, linux-iio,
linux-kernel, linux-input
On Sun, Mar 1, 2026 at 5:15 PM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Sun, 1 Mar 2026 00:43:59 +0530
> Bhargav Joshi <rougueprince47@gmail.com> wrote:
>
> > Currently, calling iio_device_register() before
> > sensor_hub_register_callback() may create a race condition where the
> > device is exposed to userspace before callbacks are wired.
>
> We needs some more here on 'why' this is a problem.
> Whilst this is an unusual arrangement, I couldn't immediately find
> anything that was actually broken. It's possible data will turn up
> after we tear down the callbacks and before the userspace interfaces
> are removed, but I think that just results in dropping data. Given it's
> in a race anyway I don't think we care about that. The callbacks
> don't seem to be involved in device configuration which can go on whether
> or not the callbacks are registered. Maybe there is something that
> won't get acknowledged if they aren't in place in time?
>
> For a fix like this we'd normally want to see a clear flow that
> leads to a bug.
>
> Also a fixes tag so we know how far to backport.
>
Hi Jonathan,
I originally submitted the patch for the driver because calling
iio_device_register() before the callbacks are wired up violates
standard IIO LIFO ordering. I assumed this created a dangerous race
condition with userspace. However, as you correctly pointed out, the
HID sensor hub safely drops those early events without crashing, even
if it is unusual behaviour still won't have a hard crash.
My underlying goal was simply a structural cleanup to align the
probe() and remove() sequences with standard IIO architecture.
Would you like me to send a v2 of this patch with a revised commit
message classifying it purely as a structural cleanup (and without a
Fixes tag), or is the current driver behavior acceptable enough that I
should just drop this patch entirely?
Thanks,
Bhargav
> >
> > Move iio_device_register() to the end of the probe() function to prevent
> > race condition.
> >
> > Consequently, update the error handling path in probe() and in remove()
> > ensuring that iio_device_unregister() is called first to cut off
> > userspace access before the hardware callbacks are removed.
> >
> > Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
> > ---
> > drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++----------
> > 1 file changed, 10 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > index c43990c518f7..8e3628cd8529 100644
> > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> > return ret;
> > }
> >
> > - ret = iio_device_register(indio_dev);
> > - if (ret) {
> > - dev_err(&pdev->dev, "device register failed\n");
> > - goto error_remove_trigger;
> > - }
> > -
> > gyro_state->callbacks.send_event = gyro_3d_proc_event;
> > gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
> > gyro_state->callbacks.pdev = pdev;
> > @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> > &gyro_state->callbacks);
> > if (ret < 0) {
> > dev_err(&pdev->dev, "callback reg failed\n");
> > - goto error_iio_unreg;
> > + goto error_remove_trigger;
> > + }
> > +
> > + ret = iio_device_register(indio_dev);
> > + if (ret) {
> > + dev_err(&pdev->dev, "device register failed\n");
> > + goto error_remove_callback;
> > }
> >
> > return ret;
> >
> > -error_iio_unreg:
> > - iio_device_unregister(indio_dev);
> > +error_remove_callback:
> > + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > error_remove_trigger:
> > hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> > return ret;
> > @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
> > struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
> >
> > - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > iio_device_unregister(indio_dev);
> > + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> > }
> >
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
2026-03-01 19:30 ` Bhargav Joshi
@ 2026-03-07 14:21 ` Jonathan Cameron
2026-03-09 15:42 ` srinivas pandruvada
0 siblings, 1 reply; 9+ messages in thread
From: Jonathan Cameron @ 2026-03-07 14:21 UTC (permalink / raw)
To: Bhargav Joshi
Cc: jikos, srinivas.pandruvada, dlechner, nuno.sa, andy, linux-iio,
linux-kernel, linux-input
On Mon, 2 Mar 2026 01:00:06 +0530
Bhargav Joshi <rougueprince47@gmail.com> wrote:
> On Sun, Mar 1, 2026 at 5:15 PM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Sun, 1 Mar 2026 00:43:59 +0530
> > Bhargav Joshi <rougueprince47@gmail.com> wrote:
> >
> > > Currently, calling iio_device_register() before
> > > sensor_hub_register_callback() may create a race condition where the
> > > device is exposed to userspace before callbacks are wired.
> >
> > We needs some more here on 'why' this is a problem.
> > Whilst this is an unusual arrangement, I couldn't immediately find
> > anything that was actually broken. It's possible data will turn up
> > after we tear down the callbacks and before the userspace interfaces
> > are removed, but I think that just results in dropping data. Given it's
> > in a race anyway I don't think we care about that. The callbacks
> > don't seem to be involved in device configuration which can go on whether
> > or not the callbacks are registered. Maybe there is something that
> > won't get acknowledged if they aren't in place in time?
> >
> > For a fix like this we'd normally want to see a clear flow that
> > leads to a bug.
> >
> > Also a fixes tag so we know how far to backport.
> >
>
> Hi Jonathan,
>
> I originally submitted the patch for the driver because calling
> iio_device_register() before the callbacks are wired up violates
> standard IIO LIFO ordering. I assumed this created a dangerous race
> condition with userspace. However, as you correctly pointed out, the
> HID sensor hub safely drops those early events without crashing, even
> if it is unusual behaviour still won't have a hard crash.
>
> My underlying goal was simply a structural cleanup to align the
> probe() and remove() sequences with standard IIO architecture.
>
> Would you like me to send a v2 of this patch with a revised commit
> message classifying it purely as a structural cleanup (and without a
> Fixes tag), or is the current driver behavior acceptable enough that I
> should just drop this patch entirely?
My gut is leave this one alone. It's a rather unusual driver in lots
of ways, so I don't really mind one more ;)
Lets see if anyone else has strong views one way or the other.
Srinivas?
>
> Thanks,
> Bhargav
>
> > >
> > > Move iio_device_register() to the end of the probe() function to prevent
> > > race condition.
> > >
> > > Consequently, update the error handling path in probe() and in remove()
> > > ensuring that iio_device_unregister() is called first to cut off
> > > userspace access before the hardware callbacks are removed.
> > >
> > > Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
> > > ---
> > > drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++----------
> > > 1 file changed, 10 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > index c43990c518f7..8e3628cd8529 100644
> > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> > > return ret;
> > > }
> > >
> > > - ret = iio_device_register(indio_dev);
> > > - if (ret) {
> > > - dev_err(&pdev->dev, "device register failed\n");
> > > - goto error_remove_trigger;
> > > - }
> > > -
> > > gyro_state->callbacks.send_event = gyro_3d_proc_event;
> > > gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
> > > gyro_state->callbacks.pdev = pdev;
> > > @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
> > > &gyro_state->callbacks);
> > > if (ret < 0) {
> > > dev_err(&pdev->dev, "callback reg failed\n");
> > > - goto error_iio_unreg;
> > > + goto error_remove_trigger;
> > > + }
> > > +
> > > + ret = iio_device_register(indio_dev);
> > > + if (ret) {
> > > + dev_err(&pdev->dev, "device register failed\n");
> > > + goto error_remove_callback;
> > > }
> > >
> > > return ret;
> > >
> > > -error_iio_unreg:
> > > - iio_device_unregister(indio_dev);
> > > +error_remove_callback:
> > > + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > > error_remove_trigger:
> > > hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> > > return ret;
> > > @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct platform_device *pdev)
> > > struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > > struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
> > >
> > > - sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > > iio_device_unregister(indio_dev);
> > > + sensor_hub_remove_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D);
> > > hid_sensor_remove_trigger(indio_dev, &gyro_state->common_attributes);
> > > }
> > >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
2026-03-07 14:21 ` Jonathan Cameron
@ 2026-03-09 15:42 ` srinivas pandruvada
2026-03-14 16:36 ` Bhargav Joshi
0 siblings, 1 reply; 9+ messages in thread
From: srinivas pandruvada @ 2026-03-09 15:42 UTC (permalink / raw)
To: Jonathan Cameron, Bhargav Joshi
Cc: jikos, dlechner, nuno.sa, andy, linux-iio, linux-kernel,
linux-input
On Sat, 2026-03-07 at 14:21 +0000, Jonathan Cameron wrote:
> On Mon, 2 Mar 2026 01:00:06 +0530
> Bhargav Joshi <rougueprince47@gmail.com> wrote:
>
> > On Sun, Mar 1, 2026 at 5:15 PM Jonathan Cameron <jic23@kernel.org>
> > wrote:
> > >
> > > On Sun, 1 Mar 2026 00:43:59 +0530
> > > Bhargav Joshi <rougueprince47@gmail.com> wrote:
> > >
> > > > Currently, calling iio_device_register() before
> > > > sensor_hub_register_callback() may create a race condition
> > > > where the
> > > > device is exposed to userspace before callbacks are wired.
> > >
> > > We needs some more here on 'why' this is a problem.
> > > Whilst this is an unusual arrangement, I couldn't immediately
> > > find
> > > anything that was actually broken. It's possible data will turn
> > > up
> > > after we tear down the callbacks and before the userspace
> > > interfaces
> > > are removed, but I think that just results in dropping data.
> > > Given it's
> > > in a race anyway I don't think we care about that. The callbacks
> > > don't seem to be involved in device configuration which can go on
> > > whether
> > > or not the callbacks are registered. Maybe there is something
> > > that
> > > won't get acknowledged if they aren't in place in time?
> > >
> > > For a fix like this we'd normally want to see a clear flow that
> > > leads to a bug.
> > >
> > > Also a fixes tag so we know how far to backport.
> > >
> >
> > Hi Jonathan,
> >
> > I originally submitted the patch for the driver because calling
> > iio_device_register() before the callbacks are wired up violates
> > standard IIO LIFO ordering. I assumed this created a dangerous race
> > condition with userspace. However, as you correctly pointed out,
> > the
> > HID sensor hub safely drops those early events without crashing,
> > even
> > if it is unusual behaviour still won't have a hard crash.
> >
> > My underlying goal was simply a structural cleanup to align the
> > probe() and remove() sequences with standard IIO architecture.
> >
> > Would you like me to send a v2 of this patch with a revised commit
> > message classifying it purely as a structural cleanup (and without
> > a
> > Fixes tag), or is the current driver behavior acceptable enough
> > that I
> > should just drop this patch entirely?
>
> My gut is leave this one alone. It's a rather unusual driver in lots
> of ways, so I don't really mind one more ;)
>
> Lets see if anyone else has strong views one way or the other.
>
> Srinivas?
>
I don't see anything wrong with this. If you register callback before,
then iio_push_to_buffers_with_timestamp() can be called before
iio_device_register(), although it will not happen because nobody
powered on sensors in the hub.
Thanks,
Srinivs
> >
> > Thanks,
> > Bhargav
> >
> > > >
> > > > Move iio_device_register() to the end of the probe() function
> > > > to prevent
> > > > race condition.
> > > >
> > > > Consequently, update the error handling path in probe() and in
> > > > remove()
> > > > ensuring that iio_device_unregister() is called first to cut
> > > > off
> > > > userspace access before the hardware callbacks are removed.
> > > >
> > > > Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
> > > > ---
> > > > drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++---------
> > > > -
> > > > 1 file changed, 10 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > index c43990c518f7..8e3628cd8529 100644
> > > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct
> > > > platform_device *pdev)
> > > > return ret;
> > > > }
> > > >
> > > > - ret = iio_device_register(indio_dev);
> > > > - if (ret) {
> > > > - dev_err(&pdev->dev, "device register failed\n");
> > > > - goto error_remove_trigger;
> > > > - }
> > > > -
> > > > gyro_state->callbacks.send_event = gyro_3d_proc_event;
> > > > gyro_state->callbacks.capture_sample =
> > > > gyro_3d_capture_sample;
> > > > gyro_state->callbacks.pdev = pdev;
> > > > @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct
> > > > platform_device *pdev)
> > > > &gyro_state->callbacks);
> > > > if (ret < 0) {
> > > > dev_err(&pdev->dev, "callback reg failed\n");
> > > > - goto error_iio_unreg;
> > > > + goto error_remove_trigger;
> > > > + }
> > > > +
> > > > + ret = iio_device_register(indio_dev);
> > > > + if (ret) {
> > > > + dev_err(&pdev->dev, "device register failed\n");
> > > > + goto error_remove_callback;
> > > > }
> > > >
> > > > return ret;
> > > >
> > > > -error_iio_unreg:
> > > > - iio_device_unregister(indio_dev);
> > > > +error_remove_callback:
> > > > + sensor_hub_remove_callback(hsdev,
> > > > HID_USAGE_SENSOR_GYRO_3D);
> > > > error_remove_trigger:
> > > > hid_sensor_remove_trigger(indio_dev, &gyro_state-
> > > > >common_attributes);
> > > > return ret;
> > > > @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct
> > > > platform_device *pdev)
> > > > struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > > > struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
> > > >
> > > > - sensor_hub_remove_callback(hsdev,
> > > > HID_USAGE_SENSOR_GYRO_3D);
> > > > iio_device_unregister(indio_dev);
> > > > + sensor_hub_remove_callback(hsdev,
> > > > HID_USAGE_SENSOR_GYRO_3D);
> > > > hid_sensor_remove_trigger(indio_dev, &gyro_state-
> > > > >common_attributes);
> > > > }
> > > >
> > >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe()
2026-03-09 15:42 ` srinivas pandruvada
@ 2026-03-14 16:36 ` Bhargav Joshi
0 siblings, 0 replies; 9+ messages in thread
From: Bhargav Joshi @ 2026-03-14 16:36 UTC (permalink / raw)
To: srinivas pandruvada
Cc: Jonathan Cameron, jikos, dlechner, nuno.sa, andy, linux-iio,
linux-kernel, linux-input
Hi,
On Mon, Mar 9, 2026 at 9:12 PM srinivas pandruvada
<srinivas.pandruvada@linux.intel.com> wrote:
>
> On Sat, 2026-03-07 at 14:21 +0000, Jonathan Cameron wrote:
> > On Mon, 2 Mar 2026 01:00:06 +0530
> > Bhargav Joshi <rougueprince47@gmail.com> wrote:
> >
> > > On Sun, Mar 1, 2026 at 5:15 PM Jonathan Cameron <jic23@kernel.org>
> > > wrote:
> > > >
> > > > On Sun, 1 Mar 2026 00:43:59 +0530
> > > > Bhargav Joshi <rougueprince47@gmail.com> wrote:
> > > >
> > > > > Currently, calling iio_device_register() before
> > > > > sensor_hub_register_callback() may create a race condition
> > > > > where the
> > > > > device is exposed to userspace before callbacks are wired.
> > > >
> > > > We needs some more here on 'why' this is a problem.
> > > > Whilst this is an unusual arrangement, I couldn't immediately
> > > > find
> > > > anything that was actually broken. It's possible data will turn
> > > > up
> > > > after we tear down the callbacks and before the userspace
> > > > interfaces
> > > > are removed, but I think that just results in dropping data.
> > > > Given it's
> > > > in a race anyway I don't think we care about that. The callbacks
> > > > don't seem to be involved in device configuration which can go on
> > > > whether
> > > > or not the callbacks are registered. Maybe there is something
> > > > that
> > > > won't get acknowledged if they aren't in place in time?
> > > >
> > > > For a fix like this we'd normally want to see a clear flow that
> > > > leads to a bug.
> > > >
> > > > Also a fixes tag so we know how far to backport.
> > > >
> > >
> > > Hi Jonathan,
> > >
> > > I originally submitted the patch for the driver because calling
> > > iio_device_register() before the callbacks are wired up violates
> > > standard IIO LIFO ordering. I assumed this created a dangerous race
> > > condition with userspace. However, as you correctly pointed out,
> > > the
> > > HID sensor hub safely drops those early events without crashing,
> > > even
> > > if it is unusual behaviour still won't have a hard crash.
> > >
> > > My underlying goal was simply a structural cleanup to align the
> > > probe() and remove() sequences with standard IIO architecture.
> > >
> > > Would you like me to send a v2 of this patch with a revised commit
> > > message classifying it purely as a structural cleanup (and without
> > > a
> > > Fixes tag), or is the current driver behavior acceptable enough
> > > that I
> > > should just drop this patch entirely?
> >
> > My gut is leave this one alone. It's a rather unusual driver in lots
> > of ways, so I don't really mind one more ;)
> >
> > Lets see if anyone else has strong views one way or the other.
> >
> > Srinivas?
> >
> I don't see anything wrong with this. If you register callback before,
> then iio_push_to_buffers_with_timestamp() can be called before
> iio_device_register(), although it will not happen because nobody
> powered on sensors in the hub.
>
Thank you for the clarification, given that current code won't cause
any issues, Jonathan I am completely fine with dropping this one.
Best Regards,
Bhargav
> Thanks,
> Srinivs
>
>
>
>
>
>
> > >
> > > Thanks,
> > > Bhargav
> > >
> > > > >
> > > > > Move iio_device_register() to the end of the probe() function
> > > > > to prevent
> > > > > race condition.
> > > > >
> > > > > Consequently, update the error handling path in probe() and in
> > > > > remove()
> > > > > ensuring that iio_device_unregister() is called first to cut
> > > > > off
> > > > > userspace access before the hardware callbacks are removed.
> > > > >
> > > > > Signed-off-by: Bhargav Joshi <rougueprince47@gmail.com>
> > > > > ---
> > > > > drivers/iio/gyro/hid-sensor-gyro-3d.c | 20 ++++++++++---------
> > > > > -
> > > > > 1 file changed, 10 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > > b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > > index c43990c518f7..8e3628cd8529 100644
> > > > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> > > > > @@ -333,12 +333,6 @@ static int hid_gyro_3d_probe(struct
> > > > > platform_device *pdev)
> > > > > return ret;
> > > > > }
> > > > >
> > > > > - ret = iio_device_register(indio_dev);
> > > > > - if (ret) {
> > > > > - dev_err(&pdev->dev, "device register failed\n");
> > > > > - goto error_remove_trigger;
> > > > > - }
> > > > > -
> > > > > gyro_state->callbacks.send_event = gyro_3d_proc_event;
> > > > > gyro_state->callbacks.capture_sample =
> > > > > gyro_3d_capture_sample;
> > > > > gyro_state->callbacks.pdev = pdev;
> > > > > @@ -346,13 +340,19 @@ static int hid_gyro_3d_probe(struct
> > > > > platform_device *pdev)
> > > > > &gyro_state->callbacks);
> > > > > if (ret < 0) {
> > > > > dev_err(&pdev->dev, "callback reg failed\n");
> > > > > - goto error_iio_unreg;
> > > > > + goto error_remove_trigger;
> > > > > + }
> > > > > +
> > > > > + ret = iio_device_register(indio_dev);
> > > > > + if (ret) {
> > > > > + dev_err(&pdev->dev, "device register failed\n");
> > > > > + goto error_remove_callback;
> > > > > }
> > > > >
> > > > > return ret;
> > > > >
> > > > > -error_iio_unreg:
> > > > > - iio_device_unregister(indio_dev);
> > > > > +error_remove_callback:
> > > > > + sensor_hub_remove_callback(hsdev,
> > > > > HID_USAGE_SENSOR_GYRO_3D);
> > > > > error_remove_trigger:
> > > > > hid_sensor_remove_trigger(indio_dev, &gyro_state-
> > > > > >common_attributes);
> > > > > return ret;
> > > > > @@ -365,8 +365,8 @@ static void hid_gyro_3d_remove(struct
> > > > > platform_device *pdev)
> > > > > struct iio_dev *indio_dev = platform_get_drvdata(pdev);
> > > > > struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
> > > > >
> > > > > - sensor_hub_remove_callback(hsdev,
> > > > > HID_USAGE_SENSOR_GYRO_3D);
> > > > > iio_device_unregister(indio_dev);
> > > > > + sensor_hub_remove_callback(hsdev,
> > > > > HID_USAGE_SENSOR_GYRO_3D);
> > > > > hid_sensor_remove_trigger(indio_dev, &gyro_state-
> > > > > >common_attributes);
> > > > > }
> > > > >
> > > >
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-14 16:37 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-28 19:13 [PATCH 0/2] iio: hid-sensor-gyro-3d: fix typo and probe race condition Bhargav Joshi
2026-02-28 19:13 ` [PATCH 1/2] iio: hid-sensor-gyro-3d: move iio_device_register() to end of probe() Bhargav Joshi
2026-03-01 11:44 ` Jonathan Cameron
2026-03-01 19:30 ` Bhargav Joshi
2026-03-07 14:21 ` Jonathan Cameron
2026-03-09 15:42 ` srinivas pandruvada
2026-03-14 16:36 ` Bhargav Joshi
2026-02-28 19:14 ` [PATCH 2/2] iio: hid-sensor-gyro-3d: fix typo in array name Bhargav Joshi
2026-03-01 11:46 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox