| Message ID | 20221129164531.2164660-1-Frank.Li@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw() | expand |
On 11/29/22 08:45, Frank Li wrote: readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL)); > @@ -272,6 +275,10 @@ static irqreturn_t imx8qxp_adc_isr(int irq, void *dev_id) > if (fifo_count) > complete(&adc->completion); Shouldn't the completion be triggered after the reading of the samples? otherwise you have a race condition on a multi-processor system. > > + for (i = 0; i < fifo_count; i++) > + adc->fifo[i] = FIELD_GET(IMX8QXP_ADC_RESFIFO_VAL_MASK, > + readl_relaxed(adc->regs + IMX8QXP_ADR_ADC_RESFIFO)); > + > return IRQ_HANDLED; > } >
> On 11/29/22 08:45, Frank Li wrote: > readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL)); > > @@ -272,6 +275,10 @@ static irqreturn_t imx8qxp_adc_isr(int irq, void > *dev_id) > > if (fifo_count) > > complete(&adc->completion); > > Shouldn't the completion be triggered after the reading of the samples? > otherwise you have a race condition on a multi-processor system. Yes, you are right. I will send updated patch soon. > > > > > + for (i = 0; i < fifo_count; i++) > > + adc->fifo[i] = FIELD_GET(IMX8QXP_ADC_RESFIFO_VAL_MASK, > > + readl_relaxed(adc->regs + IMX8QXP_ADR_ADC_RESFIFO)); > > + > > return IRQ_HANDLED; > > } > >
diff --git a/drivers/iio/adc/imx8qxp-adc.c b/drivers/iio/adc/imx8qxp-adc.c index 36777b827165..3cfba5c0fa34 100644 --- a/drivers/iio/adc/imx8qxp-adc.c +++ b/drivers/iio/adc/imx8qxp-adc.c @@ -86,6 +86,8 @@ #define IMX8QXP_ADC_TIMEOUT msecs_to_jiffies(100) +#define IMX8QXP_ADC_MAX_FIFO_SIZE 16 + struct imx8qxp_adc { struct device *dev; void __iomem *regs; @@ -95,6 +97,7 @@ struct imx8qxp_adc { /* Serialise ADC channel reads */ struct mutex lock; struct completion completion; + u32 fifo[IMX8QXP_ADC_MAX_FIFO_SIZE]; }; #define IMX8QXP_ADC_CHAN(_idx) { \ @@ -238,8 +241,7 @@ static int imx8qxp_adc_read_raw(struct iio_dev *indio_dev, return ret; } - *val = FIELD_GET(IMX8QXP_ADC_RESFIFO_VAL_MASK, - readl(adc->regs + IMX8QXP_ADR_ADC_RESFIFO)); + *val = adc->fifo[0]; mutex_unlock(&adc->lock); return IIO_VAL_INT; @@ -265,6 +267,7 @@ static irqreturn_t imx8qxp_adc_isr(int irq, void *dev_id) { struct imx8qxp_adc *adc = dev_id; u32 fifo_count; + int i; fifo_count = FIELD_GET(IMX8QXP_ADC_FCTRL_FCOUNT_MASK, readl(adc->regs + IMX8QXP_ADR_ADC_FCTRL)); @@ -272,6 +275,10 @@ static irqreturn_t imx8qxp_adc_isr(int irq, void *dev_id) if (fifo_count) complete(&adc->completion); + for (i = 0; i < fifo_count; i++) + adc->fifo[i] = FIELD_GET(IMX8QXP_ADC_RESFIFO_VAL_MASK, + readl_relaxed(adc->regs + IMX8QXP_ADR_ADC_RESFIFO)); + return IRQ_HANDLED; }
irq flood happen when run cat /sys/bus/iio/devices/iio:device0/in_voltage1_raw imx8qxp_adc_read_raw() { ... enable irq /* adc start */ writel(1, adc->regs + IMX8QXP_ADR_ADC_SWTRIG); ^^^^ trigger irq flood. wait_for_completion_interruptible_timeout(); readl(adc->regs + IMX8QXP_ADR_ADC_RESFIFO); ^^^^ clear irq here. ... } There is only FIFO watermark interrupt at this ADC controller. IRQ line will be assert until software read data from FIFO. So IRQ flood happen during wait_for_completion_interruptible_timeout(). Move FIFO read into irq handle to avoid irq flood. Fixes: 1e23dcaa1a9f ("iio: imx8qxp-adc: Add driver support for NXP IMX8QXP ADC") Cc: stable@vger.kernel.org Signed-off-by: Frank Li <Frank.Li@nxp.com> --- drivers/iio/adc/imx8qxp-adc.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-)