| Message ID | 20201006215509.GA2556081@dtor-ws |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] iio: adc: exynos: do not rely on 'users' counter in ISR | expand |
On Tue, 6 Oct 2020 14:55:09 -0700 dmitry.torokhov@gmail.com wrote: > The order in which 'users' counter is decremented vs calling drivers' > close() method is implementation specific, and we should not rely on > it. Let's introduce driver private flag and use it to signal ISR > to exit when device is being closed. > > This has a side-effect of fixing issue of accessing inut->users > outside of input->mutex protection. > > Reported-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com> > Acked-by: Krzysztof Kozlowski <krzk@kernel.org> > Reviewed-by: Michał Mirosław <mirq-linux@rere.qmqm.pl> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> Applied to the togreg branch of iio.git and pushed out as testing for the autobuilders to work their magic. Given this doesn't have a fixes tag etc I'm assuming it isn't high priority etc. Let me know if it is! Thanks, Jonathan > --- > > v3: fixed typo in exynos_adc_ts_close() per Michał Mirosław > v2: switched from ordinary read/write to READ_ONCE/WRITE_ONCE per Michał > Mirosław > > drivers/iio/adc/exynos_adc.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c > index 22131a677445..908df4b9b93c 100644 > --- a/drivers/iio/adc/exynos_adc.c > +++ b/drivers/iio/adc/exynos_adc.c > @@ -7,6 +7,7 @@ > * Copyright (C) 2013 Naveen Krishna Chatradhi <ch.naveen@samsung.com> > */ > > +#include <linux/compiler.h> > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/interrupt.h> > @@ -135,6 +136,8 @@ struct exynos_adc { > u32 value; > unsigned int version; > > + bool ts_enabled; > + > bool read_ts; > u32 ts_x; > u32 ts_y; > @@ -633,7 +636,7 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id) > bool pressed; > int ret; > > - while (info->input->users) { > + while (READ_ONCE(info->ts_enabled)) { > ret = exynos_read_s3c64xx_ts(dev, &x, &y); > if (ret == -ETIMEDOUT) > break; > @@ -712,6 +715,7 @@ static int exynos_adc_ts_open(struct input_dev *dev) > { > struct exynos_adc *info = input_get_drvdata(dev); > > + WRITE_ONCE(info->ts_enabled, true); > enable_irq(info->tsirq); > > return 0; > @@ -721,6 +725,7 @@ static void exynos_adc_ts_close(struct input_dev *dev) > { > struct exynos_adc *info = input_get_drvdata(dev); > > + WRITE_ONCE(info->ts_enabled, false); > disable_irq(info->tsirq); > } >
diff --git a/drivers/iio/adc/exynos_adc.c b/drivers/iio/adc/exynos_adc.c index 22131a677445..908df4b9b93c 100644 --- a/drivers/iio/adc/exynos_adc.c +++ b/drivers/iio/adc/exynos_adc.c @@ -7,6 +7,7 @@ * Copyright (C) 2013 Naveen Krishna Chatradhi <ch.naveen@samsung.com> */ +#include <linux/compiler.h> #include <linux/module.h> #include <linux/platform_device.h> #include <linux/interrupt.h> @@ -135,6 +136,8 @@ struct exynos_adc { u32 value; unsigned int version; + bool ts_enabled; + bool read_ts; u32 ts_x; u32 ts_y; @@ -633,7 +636,7 @@ static irqreturn_t exynos_ts_isr(int irq, void *dev_id) bool pressed; int ret; - while (info->input->users) { + while (READ_ONCE(info->ts_enabled)) { ret = exynos_read_s3c64xx_ts(dev, &x, &y); if (ret == -ETIMEDOUT) break; @@ -712,6 +715,7 @@ static int exynos_adc_ts_open(struct input_dev *dev) { struct exynos_adc *info = input_get_drvdata(dev); + WRITE_ONCE(info->ts_enabled, true); enable_irq(info->tsirq); return 0; @@ -721,6 +725,7 @@ static void exynos_adc_ts_close(struct input_dev *dev) { struct exynos_adc *info = input_get_drvdata(dev); + WRITE_ONCE(info->ts_enabled, false); disable_irq(info->tsirq); }