diff mbox series

[1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor head

Message ID 20230324094957.115071-2-biju.das.jz@bp.renesas.com (mailing list archive)
State Superseded
Headers show
Series RZ/G2L DMAC enhancements | expand

Commit Message

Biju Das March 24, 2023, 9:49 a.m. UTC
Reinitialize link mode descriptor head during terminate_all().
It fixes the incorrect serial messages during serial transfer when
DMA is enabled.

Based on a patch in the BSP by Long Luu
<long.luu.ur@renesas.com>

Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
 drivers/dma/sh/rz-dmac.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Vinod Koul March 31, 2023, 12:14 p.m. UTC | #1
On 24-03-23, 09:49, Biju Das wrote:
> Reinitialize link mode descriptor head during terminate_all().
> It fixes the incorrect serial messages during serial transfer when
> DMA is enabled.
> 
> Based on a patch in the BSP by Long Luu
> <long.luu.ur@renesas.com>
> 
> Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
>  drivers/dma/sh/rz-dmac.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> index 6b62e01ba658..a04a37ce03fd 100644
> --- a/drivers/dma/sh/rz-dmac.c
> +++ b/drivers/dma/sh/rz-dmac.c
> @@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>  static int rz_dmac_terminate_all(struct dma_chan *chan)
>  {
>  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> +	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
>  	unsigned long flags;
> +	unsigned int i;
> +
>  	LIST_HEAD(head);
>  
>  	rz_dmac_disable_hw(channel);
>  	spin_lock_irqsave(&channel->vc.lock, flags);
> +
> +	for (i = 0; i < DMAC_NR_LMDESC; i++)
> +		lmdesc[i].header = 0;

Any reason not to use memset for this?

> +
>  	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
>  	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
>  	spin_unlock_irqrestore(&channel->vc.lock, flags);
> -- 
> 2.25.1
Biju Das March 31, 2023, 3:20 p.m. UTC | #2
Hi Vinod,

Thanks for the feedback.

> -----Original Message-----
> From: Vinod Koul <vkoul@kernel.org>
> Sent: Friday, March 31, 2023 1:15 PM
> To: Biju Das <biju.das.jz@bp.renesas.com>
> Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>; Geert
> Uytterhoeven <geert+renesas@glider.be>; dmaengine@vger.kernel.org; linux-
> renesas-soc@vger.kernel.org
> Subject: Re: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor
> head
> 
> On 24-03-23, 09:49, Biju Das wrote:
> > Reinitialize link mode descriptor head during terminate_all().
> > It fixes the incorrect serial messages during serial transfer when DMA
> > is enabled.
> >
> > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > ---
> >  drivers/dma/sh/rz-dmac.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c index
> > 6b62e01ba658..a04a37ce03fd 100644
> > --- a/drivers/dma/sh/rz-dmac.c
> > +++ b/drivers/dma/sh/rz-dmac.c
> > @@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan,
> > struct scatterlist *sgl,  static int rz_dmac_terminate_all(struct
> > dma_chan *chan)  {
> >  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > +	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
> >  	unsigned long flags;
> > +	unsigned int i;
> > +
> >  	LIST_HEAD(head);
> >
> >  	rz_dmac_disable_hw(channel);
> >  	spin_lock_irqsave(&channel->vc.lock, flags);
> > +
> > +	for (i = 0; i < DMAC_NR_LMDESC; i++)
> > +		lmdesc[i].header = 0;
> 
> Any reason not to use memset for this?

1) If I use memset, then it need to set 64 * 8 * 4 = 2048 bytes compared to 
64 * 4 =256 bytes here and consistently we use the above logic in the driver.
https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/dma/sh/rz-dmac.c#L239

2) Another reason is if we do memset, eventhough there will not be any new irq,
irqthread may be still running which can cause DMA bus Error due to nxla
being null with memset. Currently nxla being assigned during probe.

https://elixir.bootlin.com/linux/v6.0-rc4/source/drivers/dma/sh/rz-dmac.c#L214

Cheers,
Biju


> 
> > +
> >  	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
> >  	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
> >  	spin_unlock_irqrestore(&channel->vc.lock, flags);
> > --
> > 2.25.1
> 
> --
> ~Vinod
Biju Das April 5, 2023, 9:23 a.m. UTC | #3
Hi Vinod,

> Subject: RE: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize lmdescriptor
> head
> 
> Hi Vinod,
> 
> Thanks for the feedback.
> 
> > -----Original Message-----
> > From: Vinod Koul <vkoul@kernel.org>
> > Sent: Friday, March 31, 2023 1:15 PM
> > To: Biju Das <biju.das.jz@bp.renesas.com>
> > Cc: Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>;
> > Geert Uytterhoeven <geert+renesas@glider.be>;
> > dmaengine@vger.kernel.org; linux- renesas-soc@vger.kernel.org
> > Subject: Re: [PATCH 1/3] dmaengine: sh: rz-dmac: Reinitialize
> > lmdescriptor head
> >
> > On 24-03-23, 09:49, Biju Das wrote:
> > > Reinitialize link mode descriptor head during terminate_all().
> > > It fixes the incorrect serial messages during serial transfer when
> > > DMA is enabled.
> > >
> > > Based on a patch in the BSP by Long Luu <long.luu.ur@renesas.com>
> > >
> > > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > ---
> > >  drivers/dma/sh/rz-dmac.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> > >
> > > diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> > > index 6b62e01ba658..a04a37ce03fd 100644
> > > --- a/drivers/dma/sh/rz-dmac.c
> > > +++ b/drivers/dma/sh/rz-dmac.c
> > > @@ -534,11 +534,18 @@ rz_dmac_prep_slave_sg(struct dma_chan *chan,
> > > struct scatterlist *sgl,  static int rz_dmac_terminate_all(struct
> > > dma_chan *chan)  {
> > >  	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
> > > +	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
> > >  	unsigned long flags;
> > > +	unsigned int i;
> > > +
> > >  	LIST_HEAD(head);
> > >
> > >  	rz_dmac_disable_hw(channel);
> > >  	spin_lock_irqsave(&channel->vc.lock, flags);
> > > +
> > > +	for (i = 0; i < DMAC_NR_LMDESC; i++)
> > > +		lmdesc[i].header = 0;
> >

I will send an improved version for invalidating lmdesc. 

Add helper function rz_dmac_invalidate_lmdesc() and share the code between
rz_dmac_free_chan_resources and rz_dmac_terminate_all(),so that hardware
descriptors can be reused in rz_dmac_lmdesc_recycle().

+static void rz_dmac_invalidate_lmdesc(struct rz_dmac_chan *channel)
+{
+	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
+
+	for (; lmdesc < channel->lmdesc.base + DMAC_NR_LMDESC; lmdesc++) {
+		if (lmdesc->header)
+			lmdesc->header = 0;
+	}
+}
+

static void rz_dmac_lmdesc_recycle(struct rz_dmac_chan *channel)
 {
 	struct rz_lmdesc *lmdesc = channel->lmdesc.head;
@@ -437,16 +447,11 @@ static void rz_dmac_free_chan_resources(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
 	struct rz_dmac *dmac = to_rz_dmac(chan->device);
-	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
 	struct rz_dmac_desc *desc, *_desc;
 	unsigned long flags;
-	unsigned int i;
 
 	spin_lock_irqsave(&channel->vc.lock, flags);
-
-	for (i = 0; i < DMAC_NR_LMDESC; i++)
-		lmdesc[i].header = 0;
-
+	rz_dmac_invalidate_lmdesc(channel);
 	rz_dmac_disable_hw(channel);
 	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
 	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
@@ -537,6 +542,7 @@ static int rz_dmac_terminate_all(struct dma_chan *chan)
 
 	rz_dmac_disable_hw(channel);
 	spin_lock_irqsave(&channel->vc.lock, flags);
+	rz_dmac_invalidate_lmdesc(channel);
diff mbox series

Patch

diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
index 6b62e01ba658..a04a37ce03fd 100644
--- a/drivers/dma/sh/rz-dmac.c
+++ b/drivers/dma/sh/rz-dmac.c
@@ -534,11 +534,18 @@  rz_dmac_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
 static int rz_dmac_terminate_all(struct dma_chan *chan)
 {
 	struct rz_dmac_chan *channel = to_rz_dmac_chan(chan);
+	struct rz_lmdesc *lmdesc = channel->lmdesc.base;
 	unsigned long flags;
+	unsigned int i;
+
 	LIST_HEAD(head);
 
 	rz_dmac_disable_hw(channel);
 	spin_lock_irqsave(&channel->vc.lock, flags);
+
+	for (i = 0; i < DMAC_NR_LMDESC; i++)
+		lmdesc[i].header = 0;
+
 	list_splice_tail_init(&channel->ld_active, &channel->ld_free);
 	list_splice_tail_init(&channel->ld_queue, &channel->ld_free);
 	spin_unlock_irqrestore(&channel->vc.lock, flags);