* [PATCH v2 1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw()
@ 2022-12-01 14:01 Frank Li
2022-12-02 3:23 ` Bough Chen
0 siblings, 1 reply; 4+ messages in thread
From: Frank Li @ 2022-12-01 14:01 UTC (permalink / raw)
To: haibo.chen
Cc: cai.huoqing, festevam, frank.li, imx, jic23, kernel, lars,
linux-arm-kernel, linux-iio, linux-imx, linux-kernel, s.hauer,
shawnguo
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>
---
Change from v1 to v2
- move complete() after read fifo
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..f5a0fc9e64c5 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,10 +267,15 @@ 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));
+ 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));
+
if (fifo_count)
complete(&adc->completion);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* RE: [PATCH v2 1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw()
2022-12-01 14:01 [PATCH v2 1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw() Frank Li
@ 2022-12-02 3:23 ` Bough Chen
2022-12-02 9:05 ` Cai Huoqing
0 siblings, 1 reply; 4+ messages in thread
From: Bough Chen @ 2022-12-02 3:23 UTC (permalink / raw)
To: Frank Li
Cc: cai.huoqing, festevam, imx, jic23, kernel, lars,
linux-arm-kernel, linux-iio, dl-linux-imx, linux-kernel, s.hauer,
shawnguo
> -----Original Message-----
> From: Frank Li <frank.li@nxp.com>
> Sent: 2022年12月1日 22:01
> To: Bough Chen <haibo.chen@nxp.com>
> Cc: cai.huoqing@linux.dev; festevam@gmail.com; Frank Li <frank.li@nxp.com>;
> imx@lists.linux.dev; jic23@kernel.org; kernel@pengutronix.de;
> lars@metafoo.de; linux-arm-kernel@lists.infradead.org;
> linux-iio@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> linux-kernel@vger.kernel.org; s.hauer@pengutronix.de; shawnguo@kernel.org
> Subject: [PATCH v2 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: stable@vger.kernel.org
>
> Signed-off-by: Frank Li <Frank.Li@nxp.com>
Thanks for the quick fix. The total read count in irq handler is fifo_count which is read from register, this is reasonable.
Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
Best Regards
Haibo Chen
> ---
> Change from v1 to v2
> - move complete() after read fifo
>
>
> 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..f5a0fc9e64c5 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,10 +267,15 @@ 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));
>
> + 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));
> +
> if (fifo_count)
> complete(&adc->completion);
>
> --
> 2.34.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw()
2022-12-02 3:23 ` Bough Chen
@ 2022-12-02 9:05 ` Cai Huoqing
2022-12-04 15:53 ` Jonathan Cameron
0 siblings, 1 reply; 4+ messages in thread
From: Cai Huoqing @ 2022-12-02 9:05 UTC (permalink / raw)
To: Bough Chen
Cc: Frank Li, festevam, imx, jic23, kernel, lars, linux-arm-kernel,
linux-iio, dl-linux-imx, linux-kernel, s.hauer, shawnguo
On 02 12月 22 03:23:55, Bough Chen wrote:
> > -----Original Message-----
> > From: Frank Li <frank.li@nxp.com>
> > Sent: 2022年12月1日 22:01
> > To: Bough Chen <haibo.chen@nxp.com>
> > Cc: cai.huoqing@linux.dev; festevam@gmail.com; Frank Li <frank.li@nxp.com>;
> > imx@lists.linux.dev; jic23@kernel.org; kernel@pengutronix.de;
> > lars@metafoo.de; linux-arm-kernel@lists.infradead.org;
> > linux-iio@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > linux-kernel@vger.kernel.org; s.hauer@pengutronix.de; shawnguo@kernel.org
> > Subject: [PATCH v2 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: stable@vger.kernel.org
> >
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
>
> Thanks for the quick fix. The total read count in irq handler is fifo_count which is read from register, this is reasonable.
>
> Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
The same, thanks.
Reviewed-by: Cai Huoqing <cai.huoqing@linux.dev>
>
> Best Regards
> Haibo Chen
> > ---
> > Change from v1 to v2
> > - move complete() after read fifo
> >
> >
> > 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..f5a0fc9e64c5 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,10 +267,15 @@ 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));
> >
> > + 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));
> > +
> > if (fifo_count)
> > complete(&adc->completion);
> >
> > --
> > 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw()
2022-12-02 9:05 ` Cai Huoqing
@ 2022-12-04 15:53 ` Jonathan Cameron
0 siblings, 0 replies; 4+ messages in thread
From: Jonathan Cameron @ 2022-12-04 15:53 UTC (permalink / raw)
To: Cai Huoqing
Cc: Bough Chen, Frank Li, festevam, imx, kernel, lars,
linux-arm-kernel, linux-iio, dl-linux-imx, linux-kernel, s.hauer,
shawnguo
On Fri, 2 Dec 2022 17:05:54 +0800
Cai Huoqing <cai.huoqing@linux.dev> wrote:
> On 02 12月 22 03:23:55, Bough Chen wrote:
> > > -----Original Message-----
> > > From: Frank Li <frank.li@nxp.com>
> > > Sent: 2022年12月1日 22:01
> > > To: Bough Chen <haibo.chen@nxp.com>
> > > Cc: cai.huoqing@linux.dev; festevam@gmail.com; Frank Li <frank.li@nxp.com>;
> > > imx@lists.linux.dev; jic23@kernel.org; kernel@pengutronix.de;
> > > lars@metafoo.de; linux-arm-kernel@lists.infradead.org;
> > > linux-iio@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>;
> > > linux-kernel@vger.kernel.org; s.hauer@pengutronix.de; shawnguo@kernel.org
> > > Subject: [PATCH v2 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: stable@vger.kernel.org
> > >
> > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> >
> > Thanks for the quick fix. The total read count in irq handler is fifo_count which is read from register, this is reasonable.
> >
> > Reviewed-by: Haibo Chen <haibo.chen@nxp.com>
> The same, thanks.
> Reviewed-by: Cai Huoqing <cai.huoqing@linux.dev>
I briefly wondered if it would be better to return the newest value,
but given it's not obvious and they will be close together in this case,
I'm fine with this.
Applied to the fixes-togreg branch of iio.git and marked for
stable inclusion. Note I'm unlikely to send a pull request until after rc1 now
so this will take a few weeks to land upstream.
Thanks,
Jonathan
> >
> > Best Regards
> > Haibo Chen
> > > ---
> > > Change from v1 to v2
> > > - move complete() after read fifo
> > >
> > >
> > > 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..f5a0fc9e64c5 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,10 +267,15 @@ 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));
> > >
> > > + 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));
> > > +
> > > if (fifo_count)
> > > complete(&adc->completion);
> > >
> > > --
> > > 2.34.1
> >
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-12-04 15:40 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 14:01 [PATCH v2 1/1] iio: imx8qxp-adc: fix irq flood when call imx8qxp_adc_read_raw() Frank Li
2022-12-02 3:23 ` Bough Chen
2022-12-02 9:05 ` Cai Huoqing
2022-12-04 15:53 ` Jonathan Cameron
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).