| Message ID | 20210319124709.484859-1-linus.walleij@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iio: accel: bmc150: Accept any trigger | expand |
On Fri, 19 Mar 2021 13:47:09 +0100 Linus Walleij <linus.walleij@linaro.org> wrote: > The BMC150 driver checks to see that the trigger it is > using is one of its own. > > Not nice for my system where the IRQ line from the BMA222 > is not connected so there is *no* trigger I can use. > > This seems just plain wrong to me. Certainly a userspace > process will want to use the trigger from one single sensor > even of it is reading say three different sensors for some > sensor fusion. > > Delete this trigger check. Hi Linus, Given it's rare for anyone to implement the validate callback (and often only happens because I asked someone to add it) I had a bit of a dig... So the 'normal' reason for this is complexity around fifos that mean supporting other triggers requires a bunch of extra code (basically can't use a fifo without access to it's watermarks interrupt etc - it could be done but it's more than a little crazy - what does it meant to use an hrtimer to read from a fifo running at a different rate?). Now this driver has two different struct iio_info structures. Only one of those has this particular restriction - the other one just doesn't enable the fifo and will run in data-ready mode, or using another trigger. The decision is based on whether the bus supports block reads. So I'd argue what we actually need for your unwired interrupt case is to fallback to the same mode we get if we can't do the fifo anyway. if (block_supported && irq > 0) { at line 1734 should work I think... Who buys a nice device like this and doesn't wire up the interrupt? Crazy people :) Jonathan > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/iio/accel/bmc150-accel-core.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c > index 7e425ebcd7ea..ae81b547dcfc 100644 > --- a/drivers/iio/accel/bmc150-accel-core.c > +++ b/drivers/iio/accel/bmc150-accel-core.c > @@ -853,20 +853,6 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev, > return 0; > } > > -static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev, > - struct iio_trigger *trig) > -{ > - struct bmc150_accel_data *data = iio_priv(indio_dev); > - int i; > - > - for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) { > - if (data->triggers[i].indio_trig == trig) > - return 0; > - } > - > - return -EINVAL; > -} > - > static ssize_t bmc150_accel_get_fifo_watermark(struct device *dev, > struct device_attribute *attr, > char *buf) > @@ -1217,7 +1203,6 @@ static const struct iio_info bmc150_accel_info_fifo = { > .write_event_value = bmc150_accel_write_event, > .write_event_config = bmc150_accel_write_event_config, > .read_event_config = bmc150_accel_read_event_config, > - .validate_trigger = bmc150_accel_validate_trigger, > .hwfifo_set_watermark = bmc150_accel_set_watermark, > .hwfifo_flush_to_buffer = bmc150_accel_fifo_flush, > };
diff --git a/drivers/iio/accel/bmc150-accel-core.c b/drivers/iio/accel/bmc150-accel-core.c index 7e425ebcd7ea..ae81b547dcfc 100644 --- a/drivers/iio/accel/bmc150-accel-core.c +++ b/drivers/iio/accel/bmc150-accel-core.c @@ -853,20 +853,6 @@ static int bmc150_accel_write_event_config(struct iio_dev *indio_dev, return 0; } -static int bmc150_accel_validate_trigger(struct iio_dev *indio_dev, - struct iio_trigger *trig) -{ - struct bmc150_accel_data *data = iio_priv(indio_dev); - int i; - - for (i = 0; i < BMC150_ACCEL_TRIGGERS; i++) { - if (data->triggers[i].indio_trig == trig) - return 0; - } - - return -EINVAL; -} - static ssize_t bmc150_accel_get_fifo_watermark(struct device *dev, struct device_attribute *attr, char *buf) @@ -1217,7 +1203,6 @@ static const struct iio_info bmc150_accel_info_fifo = { .write_event_value = bmc150_accel_write_event, .write_event_config = bmc150_accel_write_event_config, .read_event_config = bmc150_accel_read_event_config, - .validate_trigger = bmc150_accel_validate_trigger, .hwfifo_set_watermark = bmc150_accel_set_watermark, .hwfifo_flush_to_buffer = bmc150_accel_fifo_flush, };
The BMC150 driver checks to see that the trigger it is using is one of its own. Not nice for my system where the IRQ line from the BMA222 is not connected so there is *no* trigger I can use. This seems just plain wrong to me. Certainly a userspace process will want to use the trigger from one single sensor even of it is reading say three different sensors for some sensor fusion. Delete this trigger check. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/iio/accel/bmc150-accel-core.c | 15 --------------- 1 file changed, 15 deletions(-)