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: [email protected]
Signed-off-by: Frank Li <[email protected]>
---
drivers/iio/adc/imx8qxp-adc.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
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;
}
--
2.34.1
> -----Original Message-----
> From: Frank Li <[email protected]>
> Sent: 2022??11??30?? 0:46
> To: Cai Huoqing <[email protected]>; Bough Chen <[email protected]>;
> Jonathan Cameron <[email protected]>; Lars-Peter Clausen <[email protected]>;
> Shawn Guo <[email protected]>; Sascha Hauer <[email protected]>;
> Pengutronix Kernel Team <[email protected]>; Fabio Estevam
> <[email protected]>; dl-linux-imx <[email protected]>; open list:NXP i.MX
> 8QXP ADC DRIVER <[email protected]>; moderated
> list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> <[email protected]>; open list <[email protected]>
> Cc: [email protected]
> Subject: [PATCH 1/1] iio: imx8qxp-adc: fix irq flood when call
> imx8qxp_adc_read_raw()
>
> 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: [email protected]
>
> Signed-off-by: Frank Li <[email protected]>
> ---
> drivers/iio/adc/imx8qxp-adc.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> 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));
Hi Frank,
Since the ADC mode is config as single-ended 12-bit conversion, every time when trigger the ADC conversion, only one 12-bit data push into fifo.
And we also config the fifo watermark as 1. So here only need to read once. I already confirmed on my board.
Best Regards
Haibo Chen
> +
> return IRQ_HANDLED;
> }
>
> --
> 2.34.1
> -----Original Message-----
> From: Bough Chen <[email protected]>
> Sent: Wednesday, November 30, 2022 3:58 AM
> To: Frank Li <[email protected]>; Cai Huoqing <[email protected]>;
> Jonathan Cameron <[email protected]>; Lars-Peter Clausen
> <[email protected]>; Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>; Pengutronix Kernel Team
> <[email protected]>; Fabio Estevam <[email protected]>; dl-linux-
> imx <[email protected]>; open list:NXP i.MX 8QXP ADC DRIVER <linux-
> [email protected]>; moderated list:ARM/FREESCALE IMX / MXC ARM
> ARCHITECTURE <[email protected]>; open list <linux-
> [email protected]>
> Cc: [email protected]
> Subject: RE: [PATCH 1/1] iio: imx8qxp-adc: fix irq flood when call
> imx8qxp_adc_read_raw()
>
> > -----Original Message-----
> > From: Frank Li <[email protected]>
> > Sent: 2022年11月30日 0:46
> > To: Cai Huoqing <[email protected]>; Bough Chen
> <[email protected]>;
> > Jonathan Cameron <[email protected]>; Lars-Peter Clausen
> <[email protected]>;
> > Shawn Guo <[email protected]>; Sascha Hauer
> <[email protected]>;
> > Pengutronix Kernel Team <[email protected]>; Fabio Estevam
> > <[email protected]>; dl-linux-imx <[email protected]>; open list:NXP
> i.MX
> > 8QXP ADC DRIVER <[email protected]>; moderated
> > list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
> > <[email protected]>; open list <linux-
> [email protected]>
> > Cc: [email protected]
> > Subject: [PATCH 1/1] iio: imx8qxp-adc: fix irq flood when call
> > imx8qxp_adc_read_raw()
> >
> > 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: [email protected]
> >
> > Signed-off-by: Frank Li <[email protected]>
> > ---
> > drivers/iio/adc/imx8qxp-adc.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > 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));
>
> Hi Frank,
>
> Since the ADC mode is config as single-ended 12-bit conversion, every time
> when trigger the ADC conversion, only one 12-bit data push into fifo.
> And we also config the fifo watermark as 1. So here only need to read once. I
> already confirmed on my board.
Interrupt handle should guarantee read all data from FIFO. If somewhere increase
Watermark or set auto continue sample, irq flood will be happen again.
Best regards
Frank Li
>
> Best Regards
> Haibo Chen
> > +
> > return IRQ_HANDLED;
> > }
> >
> > --
> > 2.34.1