Re: [PATCH v4 7/9] dma: dw: Avoid partial transfers

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Andy,

andriy.shevchenko@linux.intel.com wrote on Thu, 10 Mar 2022 19:50:09
+0200:

> +Cc: Ilpo who is currently doing adjoining stuff.
> 
> Ilpo, this one affects Intel Bay Trail and Cherry Trail platforms.
> Not sure if it's in scope of your interest right now, but it might
> be useful to see how DMA <--> 8250 UART functioning.
> 
> On Thu, Mar 10, 2022 at 04:57:53PM +0100, Miquel Raynal wrote:
> > As investigated by Phil Edworthy <phil.edworthy@renesas.com> on RZN1 a  
> 
> Email can be dropped as you put it below, just (full) name is enough.

Sure.

> I'm wondering if Phil or anybody else who possess the hardware can
> test / tested this.

I have a board with an RZN1 SoC but I don't have exactly the same setup
as Phil (I only have one port with DMA working, while he used two as a
loopback device). I tried to reproduce the error with no luck so far. I
however verified that there was apparently no performance hit
whatsoever due to this change. IIRC Phil does not have the hardware
anymore.

> > while ago, pausing a partial transfer only causes data to be written to
> > memory that is a multiple of the memory width setting. Such a situation
> > can happen eg. because of a char timeout interrupt on a UART. In this
> > case, the current ->terminate_all() implementation does not always flush
> > the remaining data as it should.
> > 
> > In order to workaround this, a solutions is to resume and then pause
> > again the transfer before termination. The resume call in practice
> > actually flushes the remaining data.  
> 
> Perhaps Fixes tag?

I don't know exactly what hardware can suffer from this, hence I
decided not to add a Fixes tag given the fact that it was only observed
on RZN1 (which was until now not yet supported upstream).

> > Reported-by: Phil Edworthy <phil.edworthy@renesas.com>
> > Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > ---
> >  drivers/dma/dw/core.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
> > index 7ab83fe601ed..2f6183177ba5 100644
> > --- a/drivers/dma/dw/core.c
> > +++ b/drivers/dma/dw/core.c
> > @@ -862,6 +862,10 @@ static int dwc_terminate_all(struct dma_chan *chan)
> >  
> >  	clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
> >  
> > +	/* Ensure the last byte(s) are drained before disabling the channel */
> > +	if (test_bit(DW_DMA_IS_PAUSED, &dwc->flags))
> > +		dwc_chan_resume(dwc, true);
> > +
> >  	dwc_chan_pause(dwc, true);
> >  
> >  	dwc_chan_disable(dw, dwc);
> > -- 
> > 2.27.0
> >   
> 


Thanks,
Miquèl