[2/4] dmaengine: dw: Add memory bus width verification

Currently in case of the DEV_TO_MEM or MEM_TO_DEV DMA transfers the memory
data width (single transfer width) is determined based on the buffer
length, buffer base address or DMA master-channel max address width
capability. It isn't enough in case of the channel disabling prior the
block transfer is finished. Here is what DW AHB DMA IP-core databook says
regarding the port suspension (DMA-transfer pause) implementation in the
controller:

"When CTLx.SRC_TR_WIDTH < CTLx.DST_TR_WIDTH and the CFGx.CH_SUSP bit is
high, the CFGx.FIFO_EMPTY is asserted once the contents of the FIFO do not
permit a single word of CTLx.DST_TR_WIDTH to be formed. However, there may
still be data in the channel FIFO, but not enough to form a single
transfer of CTLx.DST_TR_WIDTH. In this scenario, once the channel is
disabled, the remaining data in the channel FIFO is not transferred to the
destination peripheral."

So in case if the port gets to be suspended and then disabled it's
possible to have the data silently discarded even though the controller
reported that FIFO is empty and the CTLx.BLOCK_TS indicated the dropped
data already received from the source device. This looks as if the data
somehow got lost on a way from the peripheral device to memory and causes
problems for instance in the DW APB UART driver, which pauses and disables
the DMA-transfer as soon as the recv data timeout happens. Here is the way
it looks:

 Memory <------- DMA FIFO <------ UART FIFO <---------------- UART
  DST_TR_WIDTH -+--------|       |         |
                |        |       |         |                No more data
   Current lvl -+--------|       |---------+- DMA-burst lvl
                |        |       |---------+- Leftover data
                |        |       |---------+- SRC_TR_WIDTH
               -+--------+-------+---------+

In the example above: no more data is getting received over the UART port
and BLOCK_TS is not even close to be fully received; some data is left in
the UART FIFO, but not enough to perform a bursted DMA-xfer to the DMA
FIFO; some data is left in the DMA FIFO, but not enough to be passed
further to the system memory in a single transfer. In this situation the
8250 UART driver catches the recv timeout interrupt, pauses the
DMA-transfer and terminates it completely, after which the IRQ handler
manually fetches the leftover data from the UART FIFO into the
recv-buffer. But since the DMA-channel has been disabled with the data
left in the DMA FIFO, that data will be just discarded and the recv-buffer
will have a gap of the "current lvl" size in the recv-buffer at the tail
of the lately received data portion. So the data will be lost just due to
the misconfigured DMA transfer.

Note this is only relevant for the case of the transfer suspension and
_disabling_. No problem will happen if the transfer will be re-enabled
afterwards or the block transfer is fully completed. In the later case the
"FIFO flush mode" will be executed at the transfer final stage in order to
push out the data left in the DMA FIFO.

In order to fix the denoted problem the DW AHB DMA-engine driver needs to
make sure that the _bursted_ source transfer width is greater or equal to
the single destination transfer (note the HW databook describes more
strict constraint than actually required). Since the peripheral-device
side is prescribed by the client driver logic, the memory-side can be only
used for that. The solution can be easily implemented for the DEV_TO_MEM
transfers just by adjusting the memory-channel address width. Sadly it's
not that easy for the MEM_TO_DEV transfers since the mem-to-dma burst size
is normally dynamically determined by the controller. So the only thing
that can be done is to make sure that memory-side address width can be
greater than the peripheral device address width.

Fixes: a09820043c9e ("dw_dmac: autoconfigure data_width or get it via platform data")
Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/dma/dw/core.c | 41 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 34 insertions(+), 7 deletions(-)

On Wed, Apr 17, 2024 at 09:52:42PM +0300, Serge Semin wrote:
> On Wed, Apr 17, 2024 at 08:28:06PM +0300, Andy Shevchenko wrote:
> > On Wed, Apr 17, 2024 at 08:13:59PM +0300, Serge Semin wrote:

...

> > Got it. Maybe a little summary in the code to explain all this magic?
> 
> Will it be enough to add something like this:
> /*
>  * It's possible to have a data portion locked in the DMA FIFO in case
>  * of the channel suspension. Subsequent channel disabling will cause
>  * that data silent loss. In order to prevent that maintain the src
>  * and dst transfer widths coherency by means of the relation:
>  * (CTLx.SRC_TR_WIDTH * CTLx.SRC_MSIZE >= CTLx.DST_TR_WIDTH)
>  */

Yes, and you may add something like
"Look for the details in the commit message that brings this change."
at the end of it.

On Thu, Apr 18, 2024 at 12:37:09PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 17, 2024 at 09:52:42PM +0300, Serge Semin wrote:
> > On Wed, Apr 17, 2024 at 08:28:06PM +0300, Andy Shevchenko wrote:
> > > On Wed, Apr 17, 2024 at 08:13:59PM +0300, Serge Semin wrote:
> 
> ...
> 
> > > Got it. Maybe a little summary in the code to explain all this magic?
> > 
> > Will it be enough to add something like this:
> > /*
> >  * It's possible to have a data portion locked in the DMA FIFO in case
> >  * of the channel suspension. Subsequent channel disabling will cause
> >  * that data silent loss. In order to prevent that maintain the src
> >  * and dst transfer widths coherency by means of the relation:
> >  * (CTLx.SRC_TR_WIDTH * CTLx.SRC_MSIZE >= CTLx.DST_TR_WIDTH)
> >  */
> 
> Yes, and you may add something like
> "Look for the details in the commit message that brings this change."
> at the end of it.

Agreed. Thanks.

-Serge(y)

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>