Re: [PATCH 5/8] dma: dw: Avoid partial transfers

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Mon, Feb 21, 2022 at 08:14:47AM +0000, Phil Edworthy wrote:
> Hi Andy,
> 
> I wrote the patch a few years ago, but didn't get the time to upstream it.
> 
> I am not aware of a HW integration bug on the RZ/N1 device but can't rule it
> out. I am struggling to see what kind of HW issue this could be as, iirc,
> word accesses work fine when the size of the transfer is a multiple of the
> MEM width.
> 
> I found the issue when testing DMA with the UART transferring different amounts of data.

Can you tell more about the setup and test cases?

Also, which version of the DW DMAC IP is being used in this SoC?

...

> > > +		if (sconfig->dst_addr_width && sconfig->dst_addr_width <
> > data_width)
> > > +			data_width = sconfig->dst_addr_width;
> > 
> > But here no check that you do it for explicitly peripheral to memory, so
> > this
> > will affect memory to peripheral transfers as well.
> No, this should be ok as this change is within:
> 	case DMA_DEV_TO_MEM:

Ah, it's better. But still unclear to me why we need this.

P.S. Please avoid top-postings.

> > -----Original Message-----
> > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > Sent: 20 February 2022 10:50
> > On Fri, Feb 18, 2022 at 07:12:23PM +0100, Miquel Raynal wrote:

> > > Pausing a partial transfer only causes data to be written to mem that is
> > > a multiple of the memory width setting.
> > >
> > > However, when a DMA client driver finishes DMA early, e.g. due to UART
> > > char timeout interrupt, all data read from the DEV must be written to
> > MEM.
> > >
> > > Therefore, allow the slave to limit the memory width to ensure all data
> > > read from the DEV is written to MEM when DMA is paused.
> > 
> > Is this a fix?
> > What happens to the data if you don't do this?
> > As far as I understood the Synopsys DesignWare specification the DMA
> > controller
> > is capable of flushing FIFO in that case on byte-by-byte basis. Do you
> > have an
> > HW integration bug?
> > 
> > TL;DR: tell us more about this.
> > 
> > ...
> > 
> > > +		if (sconfig->dst_addr_width && sconfig->dst_addr_width <
> > data_width)
> > > +			data_width = sconfig->dst_addr_width;
> > 
> > But here no check that you do it for explicitly peripheral to memory, so
> > this
> > will affect memory to peripheral transfers as well.

-- 
With Best Regards,
Andy Shevchenko