| Message ID | 58d40383f6e1faa14850f3a9b694b8e4cb4fbbec.1607162799.git.lorenzo@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] iio: common: st_sensors: fix possible infinite loop in st_sensors_irq_thread | expand |
On Sat, 5 Dec 2020 11:10:41 +0100 Lorenzo Bianconi <lorenzo@kernel.org> wrote: > Return a boolean value in st_sensors_new_samples_available routine in > order to avoid an infinite loop in st_sensors_irq_thread if > stat_drdy.addr is not defined or stat_drdy read fails > > Fixes: 90efe05562921 ("iio: st_sensors: harden interrupt handling") > Reported-by: Jonathan Cameron <jic23@kernel.org> > Reviewed-by: Linus Walleij <linus.walleij@linaro.org> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> Oops. I should not noticed this and replied here. See reply to previous version. I think this will create an infinite loop in the obscure case of no status register + an edge interrupt. My gut feeling is we can't support that combination so should fail at probe time if it is specified. It's broken hardware / dt. Jonathan > --- > Changes since v1: > - return true if the sensor does not have stat_drdy register > --- > .../common/st_sensors/st_sensors_trigger.c | 20 ++++++++----------- > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c > index 0507283bd4c1..d3f047e9d778 100644 > --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c > +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c > @@ -23,35 +23,31 @@ > * @sdata: Sensor data. > * > * returns: > - * 0 - no new samples available > - * 1 - new samples available > - * negative - error or unknown > + * false - no new samples available or read error > + * true - new samples available > */ > -static int st_sensors_new_samples_available(struct iio_dev *indio_dev, > - struct st_sensor_data *sdata) > +static bool st_sensors_new_samples_available(struct iio_dev *indio_dev, > + struct st_sensor_data *sdata) > { > int ret, status; > > /* How would I know if I can't check it? */ > if (!sdata->sensor_settings->drdy_irq.stat_drdy.addr) > - return -EINVAL; > + return true; > > /* No scan mask, no interrupt */ > if (!indio_dev->active_scan_mask) > - return 0; > + return false; > > ret = regmap_read(sdata->regmap, > sdata->sensor_settings->drdy_irq.stat_drdy.addr, > &status); > if (ret < 0) { > dev_err(sdata->dev, "error checking samples available\n"); > - return ret; > + return false; > } > > - if (status & sdata->sensor_settings->drdy_irq.stat_drdy.mask) > - return 1; > - > - return 0; > + return !!(status & sdata->sensor_settings->drdy_irq.stat_drdy.mask); > } > > /**
diff --git a/drivers/iio/common/st_sensors/st_sensors_trigger.c b/drivers/iio/common/st_sensors/st_sensors_trigger.c index 0507283bd4c1..d3f047e9d778 100644 --- a/drivers/iio/common/st_sensors/st_sensors_trigger.c +++ b/drivers/iio/common/st_sensors/st_sensors_trigger.c @@ -23,35 +23,31 @@ * @sdata: Sensor data. * * returns: - * 0 - no new samples available - * 1 - new samples available - * negative - error or unknown + * false - no new samples available or read error + * true - new samples available */ -static int st_sensors_new_samples_available(struct iio_dev *indio_dev, - struct st_sensor_data *sdata) +static bool st_sensors_new_samples_available(struct iio_dev *indio_dev, + struct st_sensor_data *sdata) { int ret, status; /* How would I know if I can't check it? */ if (!sdata->sensor_settings->drdy_irq.stat_drdy.addr) - return -EINVAL; + return true; /* No scan mask, no interrupt */ if (!indio_dev->active_scan_mask) - return 0; + return false; ret = regmap_read(sdata->regmap, sdata->sensor_settings->drdy_irq.stat_drdy.addr, &status); if (ret < 0) { dev_err(sdata->dev, "error checking samples available\n"); - return ret; + return false; } - if (status & sdata->sensor_settings->drdy_irq.stat_drdy.mask) - return 1; - - return 0; + return !!(status & sdata->sensor_settings->drdy_irq.stat_drdy.mask); } /**
Return a boolean value in st_sensors_new_samples_available routine in order to avoid an infinite loop in st_sensors_irq_thread if stat_drdy.addr is not defined or stat_drdy read fails Fixes: 90efe05562921 ("iio: st_sensors: harden interrupt handling") Reported-by: Jonathan Cameron <jic23@kernel.org> Reviewed-by: Linus Walleij <linus.walleij@linaro.org> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org> --- Changes since v1: - return true if the sensor does not have stat_drdy register --- .../common/st_sensors/st_sensors_trigger.c | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-)