* [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
* 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 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
* [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 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
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