| Message ID | 20220609083213.1795019-4-claudiu.beznea@microchip.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | iio: adc: at91-sama5d2_adc: add support for temperature sensor | expand |
On Thu, 9 Jun 2022 11:32:00 +0300 Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > When buffers are enabled conversion may start asynchronously thus > allowing changes on actual hardware could lead to bad behavior. Thus > do not allow changing oversampling ratio and sample frequency when > buffers are enabled. Less than desirable behavior perhaps, but broken? I don't see this as a fix from what you have mentioned - though I'm not against it. (just drop the fixes tag) It is an ABI change, but unlikely to be one any sane code hits. > > Fixes: 5e1a1da0f8c9 ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support") > Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> > --- > drivers/iio/adc/at91-sama5d2_adc.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c > index a672a520cdc0..b76328da0cb2 100644 > --- a/drivers/iio/adc/at91-sama5d2_adc.c > +++ b/drivers/iio/adc/at91-sama5d2_adc.c > @@ -1644,6 +1644,9 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, > { > struct at91_adc_state *st = iio_priv(indio_dev); > > + if (iio_buffer_enabled(indio_dev)) > + return -EBUSY; This is racy as nothing stops buffers being enabled after this point. Use the iio_device_claim_direct_mode() and release for this as they protect against the race. > + > switch (mask) { > case IIO_CHAN_INFO_OVERSAMPLING_RATIO: > if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) &&
On 11.06.2022 20:33, Jonathan Cameron wrote: > EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe > > On Thu, 9 Jun 2022 11:32:00 +0300 > Claudiu Beznea <claudiu.beznea@microchip.com> wrote: > >> When buffers are enabled conversion may start asynchronously thus >> allowing changes on actual hardware could lead to bad behavior. Thus >> do not allow changing oversampling ratio and sample frequency when >> buffers are enabled. > > Less than desirable behavior perhaps, but broken? I don't see this > as a fix from what you have mentioned - though I'm not against it. > (just drop the fixes tag) > It is an ABI change, but unlikely to be one any sane code hits. > >> >> Fixes: 5e1a1da0f8c9 ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support") >> Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> >> --- >> drivers/iio/adc/at91-sama5d2_adc.c | 3 +++ >> 1 file changed, 3 insertions(+) >> >> diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c >> index a672a520cdc0..b76328da0cb2 100644 >> --- a/drivers/iio/adc/at91-sama5d2_adc.c >> +++ b/drivers/iio/adc/at91-sama5d2_adc.c >> @@ -1644,6 +1644,9 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, >> { >> struct at91_adc_state *st = iio_priv(indio_dev); >> >> + if (iio_buffer_enabled(indio_dev)) >> + return -EBUSY; > > This is racy as nothing stops buffers being enabled after this point. > Use the iio_device_claim_direct_mode() and release for this as they > protect against the race. OK, thanks! > > >> + >> switch (mask) { >> case IIO_CHAN_INFO_OVERSAMPLING_RATIO: >> if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) && >
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c index a672a520cdc0..b76328da0cb2 100644 --- a/drivers/iio/adc/at91-sama5d2_adc.c +++ b/drivers/iio/adc/at91-sama5d2_adc.c @@ -1644,6 +1644,9 @@ static int at91_adc_write_raw(struct iio_dev *indio_dev, { struct at91_adc_state *st = iio_priv(indio_dev); + if (iio_buffer_enabled(indio_dev)) + return -EBUSY; + switch (mask) { case IIO_CHAN_INFO_OVERSAMPLING_RATIO: if ((val != AT91_OSR_1SAMPLES) && (val != AT91_OSR_4SAMPLES) &&
When buffers are enabled conversion may start asynchronously thus allowing changes on actual hardware could lead to bad behavior. Thus do not allow changing oversampling ratio and sample frequency when buffers are enabled. Fixes: 5e1a1da0f8c9 ("iio: adc: at91-sama5d2_adc: add hw trigger and buffer support") Signed-off-by: Claudiu Beznea <claudiu.beznea@microchip.com> --- drivers/iio/adc/at91-sama5d2_adc.c | 3 +++ 1 file changed, 3 insertions(+)