diff mbox series

[4/5] dmaengine: dw: Ignore burst setting for memory peripherals

Message ID 20200730154545.3965-5-Sergey.Semin@baikalelectronics.ru (mailing list archive)
State Changes Requested
Headers show
Series dmaengine: dw: Introduce non-mem peripherals optimizations | expand

Commit Message

Serge Semin July 30, 2020, 3:45 p.m. UTC
According to the DW DMA controller Databook (page 40 "3.5 Memory
Peripherals") memory peripherals don't have handshaking interface
connected to the controller, therefore they can never be a flow
controller. Since the CTLx.SRC_MSIZE and CTLx.DEST_MSIZE are
properties valid only for peripherals with a handshaking
interface, we can freely zero these fields out if the memory peripheral
is selected to be the source or the destination of the DMA transfers.

Note according to the databook, length of burst transfers to memory is
always equal to the number of data items available in a channel FIFO or
data items required to complete the block transfer, whichever is smaller;
length of burst transfers from memory is always equal to the space
available in a channel FIFO or number of data items required to complete
the block transfer, whichever is smaller.

Signed-off-by: Serge Semin <Sergey.Semin@baikalelectronics.ru>
---
 drivers/dma/dw/dw.c     | 5 ++---
 drivers/dma/dw/idma32.c | 5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

Comments

Andy Shevchenko July 30, 2020, 4:31 p.m. UTC | #1
On Thu, Jul 30, 2020 at 06:45:44PM +0300, Serge Semin wrote:
> According to the DW DMA controller Databook (page 40 "3.5 Memory

Which version of it?

> Peripherals") memory peripherals don't have handshaking interface
> connected to the controller, therefore they can never be a flow
> controller. Since the CTLx.SRC_MSIZE and CTLx.DEST_MSIZE are
> properties valid only for peripherals with a handshaking
> interface, we can freely zero these fields out if the memory peripheral
> is selected to be the source or the destination of the DMA transfers.
> 
> Note according to the databook, length of burst transfers to memory is
> always equal to the number of data items available in a channel FIFO or
> data items required to complete the block transfer, whichever is smaller;
> length of burst transfers from memory is always equal to the space
> available in a channel FIFO or number of data items required to complete
> the block transfer, whichever is smaller.

But does it really matter if you program there something or not?
Serge Semin July 30, 2020, 4:37 p.m. UTC | #2
On Thu, Jul 30, 2020 at 07:31:22PM +0300, Andy Shevchenko wrote:
> On Thu, Jul 30, 2020 at 06:45:44PM +0300, Serge Semin wrote:
> > According to the DW DMA controller Databook (page 40 "3.5 Memory
> 

> Which version of it?

2.18b

> 
> > Peripherals") memory peripherals don't have handshaking interface
> > connected to the controller, therefore they can never be a flow
> > controller. Since the CTLx.SRC_MSIZE and CTLx.DEST_MSIZE are
> > properties valid only for peripherals with a handshaking
> > interface, we can freely zero these fields out if the memory peripheral
> > is selected to be the source or the destination of the DMA transfers.
> > 
> > Note according to the databook, length of burst transfers to memory is
> > always equal to the number of data items available in a channel FIFO or
> > data items required to complete the block transfer, whichever is smaller;
> > length of burst transfers from memory is always equal to the space
> > available in a channel FIFO or number of data items required to complete
> > the block transfer, whichever is smaller.
> 

> But does it really matter if you program there something or not?

For memory peripherals it doesn't. But it's better to remove the redundant
settings for consistency with the doc and to get rid of the unneeded
stack-variable declaration.

-Sergey

> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>
diff mbox series

Patch

diff --git a/drivers/dma/dw/dw.c b/drivers/dma/dw/dw.c
index d9810980920a..a4862263ff14 100644
--- a/drivers/dma/dw/dw.c
+++ b/drivers/dma/dw/dw.c
@@ -67,9 +67,8 @@  static size_t dw_dma_block2bytes(struct dw_dma_chan *dwc, u32 block, u32 width)
 static u32 dw_dma_prepare_ctllo(struct dw_dma_chan *dwc)
 {
 	struct dma_slave_config	*sconfig = &dwc->dma_sconfig;
-	bool is_slave = is_slave_direction(dwc->direction);
-	u8 smsize = is_slave ? sconfig->src_maxburst : DW_DMA_MSIZE_16;
-	u8 dmsize = is_slave ? sconfig->dst_maxburst : DW_DMA_MSIZE_16;
+	u8 smsize = (dwc->direction == DMA_DEV_TO_MEM) ? sconfig->src_maxburst : 0;
+	u8 dmsize = (dwc->direction == DMA_MEM_TO_DEV) ? sconfig->dst_maxburst : 0;
 	u8 p_master = dwc->dws.p_master;
 	u8 m_master = dwc->dws.m_master;
 	u8 dms = (dwc->direction == DMA_MEM_TO_DEV) ? p_master : m_master;
diff --git a/drivers/dma/dw/idma32.c b/drivers/dma/dw/idma32.c
index f00657308811..3ce44de25d33 100644
--- a/drivers/dma/dw/idma32.c
+++ b/drivers/dma/dw/idma32.c
@@ -73,9 +73,8 @@  static size_t idma32_block2bytes(struct dw_dma_chan *dwc, u32 block, u32 width)
 static u32 idma32_prepare_ctllo(struct dw_dma_chan *dwc)
 {
 	struct dma_slave_config	*sconfig = &dwc->dma_sconfig;
-	bool is_slave = is_slave_direction(dwc->direction);
-	u8 smsize = is_slave ? sconfig->src_maxburst : IDMA32_MSIZE_8;
-	u8 dmsize = is_slave ? sconfig->dst_maxburst : IDMA32_MSIZE_8;
+	u8 smsize = (dwc->direction == DMA_DEV_TO_MEM) ? sconfig->src_maxburst : 0;
+	u8 dmsize = (dwc->direction == DMA_MEM_TO_DEV) ? sconfig->dst_maxburst : 0;
 
 	return DWC_CTLL_LLP_D_EN | DWC_CTLL_LLP_S_EN |
 	       DWC_CTLL_DST_MSIZE(dmsize) | DWC_CTLL_SRC_MSIZE(smsize);