On 10/03/2023 at 10:12, Zheng Wang wrote:
In at91_adc_probe, &st->touch_st.workq is bound with
at91_adc_workq_handler. Then it will be started by irq
handler at91_adc_touch_data_handler
If we remove the driver which will call at91_adc_remove
to make cleanup, there may be a unfinished work.
The possible sequence is as follows:
Fix it by finishing the work before cleanup in the at91_adc_remove
CPU0 CPU1
|at91_adc_workq_handler
at91_adc_remove |
iio_device_unregister|
iio_dev_release |
kfree(iio_dev_opaque);|
|
|iio_push_to_buffers
|&iio_dev_opaque->buffer_list
|//use
There is no such thing as a SMP platform using this driver (yet?), so we
agree that this fix is purely theoretical, cannot be reproduced nor its
fix validated.
That being said, I'm happy that enhancements are provided to this
driver, no doubt about that.
Fixes: 23ec2774f1cc ("iio: adc: at91-sama5d2_adc: add support for position and pressure channels")
Signed-off-by: Zheng Wang <zyytlz.wz@xxxxxxx>
---
drivers/iio/adc/at91-sama5d2_adc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/iio/adc/at91-sama5d2_adc.c b/drivers/iio/adc/at91-sama5d2_adc.c
index 50d02e5fc6fc..1b95d18d9e0b 100644
--- a/drivers/iio/adc/at91-sama5d2_adc.c
+++ b/drivers/iio/adc/at91-sama5d2_adc.c
@@ -2495,6 +2495,8 @@ static int at91_adc_remove(struct platform_device *pdev)
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct at91_adc_state *st = iio_priv(indio_dev);
+ disable_irq_nosync(st->irq);
+ cancel_work_sync(&st->touch_st.workq);
About stopping the source of interrupt, I would recommend using a
sequence already exposed in at91_adc_hw_init (and possibly make it
common), like:
if (st->soc_info.platform->layout->EOC_IDR)
at91_adc_writel(st, EOC_IDR, 0xffffffff);
at91_adc_writel(st, IDR, 0xffffffff);
Regards,
Nicolas
iio_device_unregister(indio_dev);
at91_adc_dma_disable(st);
--
2.25.1
--
Nicolas Ferre
