Re: [PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
On Mon, 20 Aug 2018 17:37:08 +0100
ji...@kernel.org wrote:
> On 20.08.2018 16:47, Lars-Peter Clausen wrote:
> > On 08/20/2018 04:53 PM, Stefan Popa wrote:
> >> This patch provides a validate_device callback for the trigger which
> >> makes
> >> sure that other devices are rejected.
> >>
> >> Signed-off-by: Stefan Popa >> ---
> >> drivers/iio/accel/adxl372.c | 13 +
> >> 1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> >> index d2fdc75..5a039ba 100644
> >> --- a/drivers/iio/accel/adxl372.c
> >> +++ b/drivers/iio/accel/adxl372.c
> >> @@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
> >> iio_trigger *trig,
> >>return adxl372_set_interrupts(st, mask, 0);
> >> }
> >>
> >> +static int adxl372_validate_trigger(struct iio_dev *indio_dev,
> >> + struct iio_trigger *trig)
> >> +{
> >> + struct adxl372_state *st = iio_priv(indio_dev);
> >> +
> >> + if (st->dready_trig != trig)
> >> + return -EINVAL;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static const struct iio_trigger_ops adxl372_trigger_ops = {
> >> + .validate_device = _trigger_validate_own_device,
> >>.set_trigger_state = adxl372_dready_trig_set_state,
> >> };
> >>
> >> static const struct iio_info adxl372_info = {
> >> + .validate_trigger = _validate_trigger,
> >
> > I wonder, if the device only works with the trigger and the trigger
> > only
> > works with the device should we actually register a trigger?
> >
> > Seems to be just extra hassle when setting up the device without any
> > extra
> > benefits.
>
> I wondered the same, but there is a reason to do this if we think we
> will eventually have support for other triggers (which looks possible
> for
> this device as we can bypass the fifo). Then we want to do it in order
> to avoid a breaking ABI change. There is a way around that by setting
> a default trigger so that it'll still use this one unless it is
> explicitly
> set but that is rather ugly!
>
> Jonathan
Hi Lars, Stefan,
I'll apply this as is, but if we continue this debate and reach another
conclusion we can always make a change during this cycle.
Plenty of time left!
Thanks,
Jonathan
Re: [PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
On Mon, 20 Aug 2018 17:37:08 +0100
ji...@kernel.org wrote:
> On 20.08.2018 16:47, Lars-Peter Clausen wrote:
> > On 08/20/2018 04:53 PM, Stefan Popa wrote:
> >> This patch provides a validate_device callback for the trigger which
> >> makes
> >> sure that other devices are rejected.
> >>
> >> Signed-off-by: Stefan Popa >> ---
> >> drivers/iio/accel/adxl372.c | 13 +
> >> 1 file changed, 13 insertions(+)
> >>
> >> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> >> index d2fdc75..5a039ba 100644
> >> --- a/drivers/iio/accel/adxl372.c
> >> +++ b/drivers/iio/accel/adxl372.c
> >> @@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
> >> iio_trigger *trig,
> >>return adxl372_set_interrupts(st, mask, 0);
> >> }
> >>
> >> +static int adxl372_validate_trigger(struct iio_dev *indio_dev,
> >> + struct iio_trigger *trig)
> >> +{
> >> + struct adxl372_state *st = iio_priv(indio_dev);
> >> +
> >> + if (st->dready_trig != trig)
> >> + return -EINVAL;
> >> +
> >> + return 0;
> >> +}
> >> +
> >> static const struct iio_trigger_ops adxl372_trigger_ops = {
> >> + .validate_device = _trigger_validate_own_device,
> >>.set_trigger_state = adxl372_dready_trig_set_state,
> >> };
> >>
> >> static const struct iio_info adxl372_info = {
> >> + .validate_trigger = _validate_trigger,
> >
> > I wonder, if the device only works with the trigger and the trigger
> > only
> > works with the device should we actually register a trigger?
> >
> > Seems to be just extra hassle when setting up the device without any
> > extra
> > benefits.
>
> I wondered the same, but there is a reason to do this if we think we
> will eventually have support for other triggers (which looks possible
> for
> this device as we can bypass the fifo). Then we want to do it in order
> to avoid a breaking ABI change. There is a way around that by setting
> a default trigger so that it'll still use this one unless it is
> explicitly
> set but that is rather ugly!
>
> Jonathan
Hi Lars, Stefan,
I'll apply this as is, but if we continue this debate and reach another
conclusion we can always make a change during this cycle.
Plenty of time left!
Thanks,
Jonathan
Re: [PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
On 20.08.2018 16:47, Lars-Peter Clausen wrote:
On 08/20/2018 04:53 PM, Stefan Popa wrote:
This patch provides a validate_device callback for the trigger which
makes
sure that other devices are rejected.
Signed-off-by: Stefan Popa @@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
iio_trigger *trig,
return adxl372_set_interrupts(st, mask, 0);
}
+static int adxl372_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct adxl372_state *st = iio_priv(indio_dev);
+
+ if (st->dready_trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct iio_trigger_ops adxl372_trigger_ops = {
+ .validate_device = _trigger_validate_own_device,
.set_trigger_state = adxl372_dready_trig_set_state,
};
static const struct iio_info adxl372_info = {
+ .validate_trigger = _validate_trigger,
I wonder, if the device only works with the trigger and the trigger
only
works with the device should we actually register a trigger?
Seems to be just extra hassle when setting up the device without any
extra
benefits.
I wondered the same, but there is a reason to do this if we think we
will eventually have support for other triggers (which looks possible
for
this device as we can bypass the fifo). Then we want to do it in order
to avoid a breaking ABI change. There is a way around that by setting
a default trigger so that it'll still use this one unless it is
explicitly
set but that is rather ugly!
Jonathan
Re: [PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
On 20.08.2018 16:47, Lars-Peter Clausen wrote:
On 08/20/2018 04:53 PM, Stefan Popa wrote:
This patch provides a validate_device callback for the trigger which
makes
sure that other devices are rejected.
Signed-off-by: Stefan Popa @@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
iio_trigger *trig,
return adxl372_set_interrupts(st, mask, 0);
}
+static int adxl372_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct adxl372_state *st = iio_priv(indio_dev);
+
+ if (st->dready_trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct iio_trigger_ops adxl372_trigger_ops = {
+ .validate_device = _trigger_validate_own_device,
.set_trigger_state = adxl372_dready_trig_set_state,
};
static const struct iio_info adxl372_info = {
+ .validate_trigger = _validate_trigger,
I wonder, if the device only works with the trigger and the trigger
only
works with the device should we actually register a trigger?
Seems to be just extra hassle when setting up the device without any
extra
benefits.
I wondered the same, but there is a reason to do this if we think we
will eventually have support for other triggers (which looks possible
for
this device as we can bypass the fifo). Then we want to do it in order
to avoid a breaking ABI change. There is a way around that by setting
a default trigger so that it'll still use this one unless it is
explicitly
set but that is rather ugly!
Jonathan
Re: [PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
On 08/20/2018 04:53 PM, Stefan Popa wrote:
> This patch provides a validate_device callback for the trigger which makes
> sure that other devices are rejected.
>
> Signed-off-by: Stefan Popa ---
> drivers/iio/accel/adxl372.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index d2fdc75..5a039ba 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
> iio_trigger *trig,
> return adxl372_set_interrupts(st, mask, 0);
> }
>
> +static int adxl372_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct adxl372_state *st = iio_priv(indio_dev);
> +
> + if (st->dready_trig != trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static const struct iio_trigger_ops adxl372_trigger_ops = {
> + .validate_device = _trigger_validate_own_device,
> .set_trigger_state = adxl372_dready_trig_set_state,
> };
>
> static const struct iio_info adxl372_info = {
> + .validate_trigger = _validate_trigger,
I wonder, if the device only works with the trigger and the trigger only
works with the device should we actually register a trigger?
Seems to be just extra hassle when setting up the device without any extra
benefits.
Re: [PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
On 08/20/2018 04:53 PM, Stefan Popa wrote:
> This patch provides a validate_device callback for the trigger which makes
> sure that other devices are rejected.
>
> Signed-off-by: Stefan Popa ---
> drivers/iio/accel/adxl372.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
> index d2fdc75..5a039ba 100644
> --- a/drivers/iio/accel/adxl372.c
> +++ b/drivers/iio/accel/adxl372.c
> @@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
> iio_trigger *trig,
> return adxl372_set_interrupts(st, mask, 0);
> }
>
> +static int adxl372_validate_trigger(struct iio_dev *indio_dev,
> + struct iio_trigger *trig)
> +{
> + struct adxl372_state *st = iio_priv(indio_dev);
> +
> + if (st->dready_trig != trig)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> static const struct iio_trigger_ops adxl372_trigger_ops = {
> + .validate_device = _trigger_validate_own_device,
> .set_trigger_state = adxl372_dready_trig_set_state,
> };
>
> static const struct iio_info adxl372_info = {
> + .validate_trigger = _validate_trigger,
I wonder, if the device only works with the trigger and the trigger only
works with the device should we actually register a trigger?
Seems to be just extra hassle when setting up the device without any extra
benefits.
[PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
This patch provides a validate_device callback for the trigger which makes
sure that other devices are rejected.
Signed-off-by: Stefan Popa
---
drivers/iio/accel/adxl372.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index d2fdc75..5a039ba 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
iio_trigger *trig,
return adxl372_set_interrupts(st, mask, 0);
}
+static int adxl372_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct adxl372_state *st = iio_priv(indio_dev);
+
+ if (st->dready_trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct iio_trigger_ops adxl372_trigger_ops = {
+ .validate_device = _trigger_validate_own_device,
.set_trigger_state = adxl372_dready_trig_set_state,
};
static const struct iio_info adxl372_info = {
+ .validate_trigger = _validate_trigger,
.read_raw = adxl372_read_raw,
.debugfs_reg_access = _reg_access,
.hwfifo_set_watermark = adxl372_set_watermark,
--
2.7.4
[PATCH 1/3] iio: adxl372: Provide validate_trigger and validate_device callbacks
This patch provides a validate_device callback for the trigger which makes
sure that other devices are rejected.
Signed-off-by: Stefan Popa
---
drivers/iio/accel/adxl372.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/iio/accel/adxl372.c b/drivers/iio/accel/adxl372.c
index d2fdc75..5a039ba 100644
--- a/drivers/iio/accel/adxl372.c
+++ b/drivers/iio/accel/adxl372.c
@@ -762,11 +762,24 @@ static int adxl372_dready_trig_set_state(struct
iio_trigger *trig,
return adxl372_set_interrupts(st, mask, 0);
}
+static int adxl372_validate_trigger(struct iio_dev *indio_dev,
+ struct iio_trigger *trig)
+{
+ struct adxl372_state *st = iio_priv(indio_dev);
+
+ if (st->dready_trig != trig)
+ return -EINVAL;
+
+ return 0;
+}
+
static const struct iio_trigger_ops adxl372_trigger_ops = {
+ .validate_device = _trigger_validate_own_device,
.set_trigger_state = adxl372_dready_trig_set_state,
};
static const struct iio_info adxl372_info = {
+ .validate_trigger = _validate_trigger,
.read_raw = adxl372_read_raw,
.debugfs_reg_access = _reg_access,
.hwfifo_set_watermark = adxl372_set_watermark,
--
2.7.4
