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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2977 bytes --]

On Thu, 10 Mar 2022, Miquel Raynal wrote:

> 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.

Thanks for the head up. I was only aware of the ones on linux-serial.

> > 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.

This should be rephrased. As is it doesn't make sense.

-- 
 i.

> >>  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
>