dmaengine Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
@ 2019-06-21  8:23 yibin.gong
  2019-06-21 12:53 ` Michael Olbrich
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: yibin.gong @ 2019-06-21  8:23 UTC (permalink / raw)
  To: shawnguo, s.hauer, festevam, vkoul, dan.j.williams, thesven73, m.olbrich
  Cc: linux-imx, stable, linux-arm-kernel, dmaengine, kernel

From: Robin Gong <yibin.gong@nxp.com>

It is possible for an irq triggered by channel0 to be received later
after clks are disabled once firmware loaded during sdma probe. If
that happens then clearing them by writing to SDMA_H_INTR won't work
and the kernel will hang processing infinite interrupts. Actually,
don't need interrupt triggered on channel0 since it's pollling
SDMA_H_STATSTOP to know channel0 done rather than interrupt in
current code, just clear BD_INTR to disable channel0 interrupt to
avoid the above case.
This issue was brought by commit 1d069bfa3c78 ("dmaengine: imx-sdma:
ack channel 0 IRQ in the interrupt handler") which didn't take care
the above case.

Fixes: 1d069bfa3c78 ("dmaengine: imx-sdma: ack channel 0 IRQ in the interrupt handler")
Cc: stable@vger.kernel.org #5.0+
Signed-off-by: Robin Gong <yibin.gong@nxp.com>
Reported-by: Sven Van Asbroeck <thesven73@gmail.com>
Tested-by: Sven Van Asbroeck <thesven73@gmail.com>
---
 drivers/dma/imx-sdma.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index deea9aa..b5a1ee2 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -742,7 +742,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
 	spin_lock_irqsave(&sdma->channel_0_lock, flags);
 
 	bd0->mode.command = C0_SETPM;
-	bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD;
+	bd0->mode.status = BD_DONE | BD_WRAP | BD_EXTD;
 	bd0->mode.count = size / 2;
 	bd0->buffer_addr = buf_phys;
 	bd0->ext_buffer_addr = address;
@@ -1064,7 +1064,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 	context->gReg[7] = sdmac->watermark_level;
 
 	bd0->mode.command = C0_SETDM;
-	bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD;
+	bd0->mode.status = BD_DONE | BD_WRAP | BD_EXTD;
 	bd0->mode.count = sizeof(*context) / 4;
 	bd0->buffer_addr = sdma->context_phys;
 	bd0->ext_buffer_addr = 2048 + (sizeof(*context) / 4) * channel;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
  2019-06-21  8:23 [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0 yibin.gong
@ 2019-06-21 12:53 ` Michael Olbrich
  2019-06-21 13:12 ` Sven Van Asbroeck
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Michael Olbrich @ 2019-06-21 12:53 UTC (permalink / raw)
  To: yibin.gong
  Cc: shawnguo, s.hauer, festevam, vkoul, dan.j.williams, thesven73,
	linux-imx, stable, linux-arm-kernel, dmaengine, kernel

On Fri, Jun 21, 2019 at 04:23:06PM +0800, yibin.gong@nxp.com wrote:
> From: Robin Gong <yibin.gong@nxp.com>
> 
> It is possible for an irq triggered by channel0 to be received later
> after clks are disabled once firmware loaded during sdma probe. If
> that happens then clearing them by writing to SDMA_H_INTR won't work
> and the kernel will hang processing infinite interrupts. Actually,
> don't need interrupt triggered on channel0 since it's pollling
> SDMA_H_STATSTOP to know channel0 done rather than interrupt in
> current code, just clear BD_INTR to disable channel0 interrupt to
> avoid the above case.
> This issue was brought by commit 1d069bfa3c78 ("dmaengine: imx-sdma:
> ack channel 0 IRQ in the interrupt handler") which didn't take care
> the above case.
> 
> Fixes: 1d069bfa3c78 ("dmaengine: imx-sdma: ack channel 0 IRQ in the interrupt handler")
> Cc: stable@vger.kernel.org #5.0+
> Signed-off-by: Robin Gong <yibin.gong@nxp.com>
> Reported-by: Sven Van Asbroeck <thesven73@gmail.com>
> Tested-by: Sven Van Asbroeck <thesven73@gmail.com>

Reviewed-by: Michael Olbrich <m.olbrich@pengutronix.de>

> ---
>  drivers/dma/imx-sdma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index deea9aa..b5a1ee2 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -742,7 +742,7 @@ static int sdma_load_script(struct sdma_engine *sdma, void *buf, int size,
>  	spin_lock_irqsave(&sdma->channel_0_lock, flags);
>  
>  	bd0->mode.command = C0_SETPM;
> -	bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD;
> +	bd0->mode.status = BD_DONE | BD_WRAP | BD_EXTD;
>  	bd0->mode.count = size / 2;
>  	bd0->buffer_addr = buf_phys;
>  	bd0->ext_buffer_addr = address;
> @@ -1064,7 +1064,7 @@ static int sdma_load_context(struct sdma_channel *sdmac)
>  	context->gReg[7] = sdmac->watermark_level;
>  
>  	bd0->mode.command = C0_SETDM;
> -	bd0->mode.status = BD_DONE | BD_INTR | BD_WRAP | BD_EXTD;
> +	bd0->mode.status = BD_DONE | BD_WRAP | BD_EXTD;
>  	bd0->mode.count = sizeof(*context) / 4;
>  	bd0->buffer_addr = sdma->context_phys;
>  	bd0->ext_buffer_addr = 2048 + (sizeof(*context) / 4) * channel;
> -- 
> 2.7.4
> 
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
  2019-06-21  8:23 [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0 yibin.gong
  2019-06-21 12:53 ` Michael Olbrich
@ 2019-06-21 13:12 ` Sven Van Asbroeck
  2019-06-24 12:54 ` Fabio Estevam
  2019-07-05  7:46 ` Vinod Koul
  3 siblings, 0 replies; 7+ messages in thread
From: Sven Van Asbroeck @ 2019-06-21 13:12 UTC (permalink / raw)
  To: Robin Gong
  Cc: Shawn Guo, Sascha Hauer, Fabio Estevam, Vinod, Dan Williams,
	Michael Olbrich, NXP Linux Team, Stable,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dmaengine, Sascha Hauer

Looking great.

Thank you for taking the time for this, Robin !

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
  2019-06-21  8:23 [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0 yibin.gong
  2019-06-21 12:53 ` Michael Olbrich
  2019-06-21 13:12 ` Sven Van Asbroeck
@ 2019-06-24 12:54 ` Fabio Estevam
  2019-06-25  7:36   ` Robin Gong
  2019-07-05  7:46 ` Vinod Koul
  3 siblings, 1 reply; 7+ messages in thread
From: Fabio Estevam @ 2019-06-24 12:54 UTC (permalink / raw)
  To: Robin Gong
  Cc: Shawn Guo, Sascha Hauer, Vinod, Dan Williams, Sven Van Asbroeck,
	Michael Olbrich, NXP Linux Team, stable,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dmaengine, Sascha Hauer

Hi Robin,

On Fri, Jun 21, 2019 at 5:21 AM <yibin.gong@nxp.com> wrote:
>
> From: Robin Gong <yibin.gong@nxp.com>
>
> It is possible for an irq triggered by channel0 to be received later
> after clks are disabled once firmware loaded during sdma probe. If
> that happens then clearing them by writing to SDMA_H_INTR won't work
> and the kernel will hang processing infinite interrupts. Actually,
> don't need interrupt triggered on channel0 since it's pollling
> SDMA_H_STATSTOP to know channel0 done rather than interrupt in
> current code, just clear BD_INTR to disable channel0 interrupt to
> avoid the above case.
> This issue was brought by commit 1d069bfa3c78 ("dmaengine: imx-sdma:
> ack channel 0 IRQ in the interrupt handler") which didn't take care
> the above case.
>
> Fixes: 1d069bfa3c78 ("dmaengine: imx-sdma: ack channel 0 IRQ in the interrupt handler")
> Cc: stable@vger.kernel.org #5.0+

This 5.0 notation does not look correct, as 1d069bfa3c78 was introduced in 4.10.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
  2019-06-24 12:54 ` Fabio Estevam
@ 2019-06-25  7:36   ` Robin Gong
  2019-06-25 15:45     ` Fabio Estevam
  0 siblings, 1 reply; 7+ messages in thread
From: Robin Gong @ 2019-06-25  7:36 UTC (permalink / raw)
  To: Fabio Estevam
  Cc: Shawn Guo, Sascha Hauer, Vinod, Dan Williams, Sven Van Asbroeck,
	Michael Olbrich, dl-linux-imx, stable,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dmaengine, Sascha Hauer

> -----Original Message-----
> From: Fabio Estevam <festevam@gmail.com>
> Sent: 2019年6月24日 20:55
> 
> Hi Robin,
> 
> On Fri, Jun 21, 2019 at 5:21 AM <yibin.gong@nxp.com> wrote:
> >
> > From: Robin Gong <yibin.gong@nxp.com>
> >
> > It is possible for an irq triggered by channel0 to be received later
> > after clks are disabled once firmware loaded during sdma probe. If
> > that happens then clearing them by writing to SDMA_H_INTR won't work
> > and the kernel will hang processing infinite interrupts. Actually,
> > don't need interrupt triggered on channel0 since it's pollling
> > SDMA_H_STATSTOP to know channel0 done rather than interrupt in current
> > code, just clear BD_INTR to disable channel0 interrupt to avoid the
> > above case.
> > This issue was brought by commit 1d069bfa3c78 ("dmaengine: imx-sdma:
> > ack channel 0 IRQ in the interrupt handler") which didn't take care
> > the above case.
> >
> > Fixes: 1d069bfa3c78 ("dmaengine: imx-sdma: ack channel 0 IRQ in the
> > interrupt handler")
> > Cc: stable@vger.kernel.org #5.0+
> 
> This 5.0 notation does not look correct, as 1d069bfa3c78 was introduced in
> 4.10.
Yes, although Sven only met this issue after v4.19, this potential issue should be there.
But 1d069bfa3c78 seems merged from v4.7 instead?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
  2019-06-25  7:36   ` Robin Gong
@ 2019-06-25 15:45     ` Fabio Estevam
  0 siblings, 0 replies; 7+ messages in thread
From: Fabio Estevam @ 2019-06-25 15:45 UTC (permalink / raw)
  To: Robin Gong
  Cc: Shawn Guo, Sascha Hauer, Vinod, Dan Williams, Sven Van Asbroeck,
	Michael Olbrich, dl-linux-imx, stable,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	dmaengine, Sascha Hauer

Hi Robin,

On Tue, Jun 25, 2019 at 4:36 AM Robin Gong <yibin.gong@nxp.com> wrote:

> Yes, although Sven only met this issue after v4.19, this potential issue should be there.
> But 1d069bfa3c78 seems merged from v4.7 instead?

Yes, you could simply do:

Fixes: 1d069bfa3c78 ("dmaengine: imx-sdma: ack channel 0 IRQ in the
interrupt handler")
Cc: stable@vger.kernel.org

And it will get applied to all relevant stable trees.

Thanks

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0
  2019-06-21  8:23 [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0 yibin.gong
                   ` (2 preceding siblings ...)
  2019-06-24 12:54 ` Fabio Estevam
@ 2019-07-05  7:46 ` Vinod Koul
  3 siblings, 0 replies; 7+ messages in thread
From: Vinod Koul @ 2019-07-05  7:46 UTC (permalink / raw)
  To: yibin.gong
  Cc: shawnguo, s.hauer, festevam, dan.j.williams, thesven73,
	m.olbrich, linux-imx, stable, linux-arm-kernel, dmaengine,
	kernel

On 21-06-19, 16:23, yibin.gong@nxp.com wrote:
> From: Robin Gong <yibin.gong@nxp.com>
> 
> It is possible for an irq triggered by channel0 to be received later
> after clks are disabled once firmware loaded during sdma probe. If
> that happens then clearing them by writing to SDMA_H_INTR won't work
> and the kernel will hang processing infinite interrupts. Actually,
> don't need interrupt triggered on channel0 since it's pollling
> SDMA_H_STATSTOP to know channel0 done rather than interrupt in
> current code, just clear BD_INTR to disable channel0 interrupt to
> avoid the above case.
> This issue was brought by commit 1d069bfa3c78 ("dmaengine: imx-sdma:
> ack channel 0 IRQ in the interrupt handler") which didn't take care
> the above case.

Applied, thanks

-- 
~Vinod

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21  8:23 [PATCH v2] dmaengine: imx-sdma: remove BD_INTR for channel0 yibin.gong
2019-06-21 12:53 ` Michael Olbrich
2019-06-21 13:12 ` Sven Van Asbroeck
2019-06-24 12:54 ` Fabio Estevam
2019-06-25  7:36   ` Robin Gong
2019-06-25 15:45     ` Fabio Estevam
2019-07-05  7:46 ` Vinod Koul

dmaengine Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dmaengine/0 dmaengine/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dmaengine dmaengine/ https://lore.kernel.org/dmaengine \
		dmaengine@vger.kernel.org dmaengine@archiver.kernel.org
	public-inbox-index dmaengine


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.dmaengine


AGPL code for this site: git clone https://public-inbox.org/ public-inbox