diff mbox series

[v2,3/4] iio: hid-sensor-trigger: Use iio->trig instead trig field internal structure

Message ID 20201119100331.2594-4-xiang.ye@intel.com (mailing list archive)
State Superseded
Headers show
Series add custom hinge sensor support | expand

Commit Message

Ye Xiang Nov. 19, 2020, 10:03 a.m. UTC
Use iio->trig instead of attrb->trig as parameter of iio_trigger_unregister
and iio_trigger_free. This allows one HID sensor driver to create
multiple iio devices. In this case common attributes are shared and
there can be one instance for the structure containing common attributes
for all iio devices.

Signed-off-by: Ye Xiang <xiang.ye@intel.com>
---
 drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Jonathan Cameron Nov. 21, 2020, 5:33 p.m. UTC | #1
On Thu, 19 Nov 2020 18:03:30 +0800
Ye Xiang <xiang.ye@intel.com> wrote:

> Use iio->trig instead of attrb->trig as parameter of iio_trigger_unregister
> and iio_trigger_free. This allows one HID sensor driver to create
> multiple iio devices. In this case common attributes are shared and
> there can be one instance for the structure containing common attributes
> for all iio devices.
> 
> Signed-off-by: Ye Xiang <xiang.ye@intel.com>
Whilst indio_dev->trig is set to the local trigger at startup,
I'm not seeing the validate callbacks that will be needed to ensure it 
is still set to that trigger when you remove the driver.

Specifically validate_trigger callback for all the hid sensor devices.

It is entirely possible that should be set and these devices can only
use the hid sensor trigger, but I don't think it currently is.

Thus this patch is going to cause some interesting crashes.
Note this is the reason all IIO drivers have to carry an extra copy
of the trig pointer.

So you'll have to do something a bit more interesting to pass that
trigger to the relevant devices that share it.

Arguably if they are actually sharing the trigger, then we shouldn't really
have separate IIO devices in the first place as we loose the
connection between the timing of the data across the different channels.

Jonathan

> ---
>  drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> index 30340abcbc8d..bb5e7c8ff15b 100644
> --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
> @@ -236,8 +236,8 @@ void hid_sensor_remove_trigger(struct iio_dev *indio_dev,
>  	pm_runtime_put_noidle(&attrb->pdev->dev);
>  
>  	cancel_work_sync(&attrb->work);
> -	iio_trigger_unregister(attrb->trigger);
> -	iio_trigger_free(attrb->trigger);
> +	iio_trigger_unregister(indio_dev->trig);
> +	iio_trigger_free(indio_dev->trig);

>  	iio_triggered_buffer_cleanup(indio_dev);
>  }
>  EXPORT_SYMBOL(hid_sensor_remove_trigger);
diff mbox series

Patch

diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 30340abcbc8d..bb5e7c8ff15b 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -236,8 +236,8 @@  void hid_sensor_remove_trigger(struct iio_dev *indio_dev,
 	pm_runtime_put_noidle(&attrb->pdev->dev);
 
 	cancel_work_sync(&attrb->work);
-	iio_trigger_unregister(attrb->trigger);
-	iio_trigger_free(attrb->trigger);
+	iio_trigger_unregister(indio_dev->trig);
+	iio_trigger_free(indio_dev->trig);
 	iio_triggered_buffer_cleanup(indio_dev);
 }
 EXPORT_SYMBOL(hid_sensor_remove_trigger);