diff mbox series

[4/4] dmaengine: dw: Simplify max-burst calculation procedure

Message ID 20240416162908.24180-5-fancer.lancer@gmail.com
State New
Headers show
Series dmaengine: dw: Fix src/dst addr width misconfig | expand

Commit Message

Serge Semin April 16, 2024, 4:28 p.m. UTC 
In order to have a more coherent DW AHB DMA slave configuration method
let's simplify the source and destination channel max-burst calculation
procedure:

1. Create the max-burst verification method as it has been just done for
the memory and peripheral address widths. Thus the DWC DMA slave config
method will turn to a set of the verification methods execution.

2. Since both the generic DW AHB DMA and Intel DMA32 engines support the
power-of-2 bursts only, then the specified by the client driver max-burst
values can be converted to being power-of-2 right in the max-burst
verification method.

3. Since max-burst encoded value is required on the CTL_LO fields
calculation stage, the encode_maxburst() callback can be easily dropped
from the dw_dma structure meanwhile the encoding procedure will be
executed right in the CTL_LO register value calculation.

Thus the update will provide the next positive effects: the internal
DMA-slave config structure will contain only the real DMA-transfer config
value, which will be encoded to the DMA-controller register fields only
when it's required on the buffer mapping; the redundant encode_maxburst()
callback will be dropped simplifying the internal HW-abstraction API;
DWC-config method will look more readable executing the verification
functions one-by-one.

Signed-off-by: Serge Semin <fancer.lancer@gmail.com>
---
 drivers/dma/dw/core.c   | 26 +++++++++++++++++---------
 drivers/dma/dw/dw.c     | 23 +++++++++++------------
 drivers/dma/dw/idma32.c | 15 +++++++--------
 drivers/dma/dw/regs.h   |  1 -
 4 files changed, 35 insertions(+), 30 deletions(-)

Comments

Serge Semin April 18, 2024, 7:10 p.m. UTC | #1 
On Thu, Apr 18, 2024 at 02:49:26PM +0300, Andy Shevchenko wrote:
> On Wed, Apr 17, 2024 at 11:35:39PM +0300, Serge Semin wrote:
> > On Tue, Apr 16, 2024 at 10:11:58PM +0300, Andy Shevchenko wrote:
> > > On Tue, Apr 16, 2024 at 07:28:58PM +0300, Serge Semin wrote:
> 
> ...
> 
> > > > +static void dwc_verify_maxburst(struct dma_chan *chan)
> > 
> > > It's inconsistent to the rest of _verify methods. It doesn't verify as it
> > > doesn't return anything. Make it int or rename the function.
> > 
> > Making it int won't make much sense since currently the method doesn't
> > imply returning an error status. IMO using "verify" was ok, but since
> > you don't see it suitable please suggest a better alternative. mend,
> > fix, align?
> 

> My suggestion is (and was) to have it return 0 for now.

Ok. Let's have it returning zero then.


-Serge(y)

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

Patch

diff --git a/drivers/dma/dw/core.c b/drivers/dma/dw/core.c
index 61e026310dd8..8b4ecd137ae2 100644
--- a/drivers/dma/dw/core.c
+++ b/drivers/dma/dw/core.c
@@ -779,6 +779,21 @@  bool dw_dma_filter(struct dma_chan *chan, void *param)
 }
 EXPORT_SYMBOL_GPL(dw_dma_filter);
 
+static void dwc_verify_maxburst(struct dma_chan *chan)
+{
+	struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
+
+	dwc->dma_sconfig.src_maxburst =
+		clamp(dwc->dma_sconfig.src_maxburst, 1U, dwc->max_burst);
+	dwc->dma_sconfig.dst_maxburst =
+		clamp(dwc->dma_sconfig.dst_maxburst, 1U, dwc->max_burst);
+
+	dwc->dma_sconfig.src_maxburst =
+		rounddown_pow_of_two(dwc->dma_sconfig.src_maxburst);
+	dwc->dma_sconfig.dst_maxburst =
+		rounddown_pow_of_two(dwc->dma_sconfig.dst_maxburst);
+}
+
 static int dwc_verify_p_buswidth(struct dma_chan *chan)
 {
 	struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
@@ -828,7 +843,7 @@  static int dwc_verify_m_buswidth(struct dma_chan *chan)
 		dwc->dma_sconfig.src_addr_width = mem_width;
 	} else if (dwc->dma_sconfig.direction == DMA_DEV_TO_MEM) {
 		reg_width = dwc->dma_sconfig.src_addr_width;
-		reg_burst = rounddown_pow_of_two(dwc->dma_sconfig.src_maxburst);
+		reg_burst = dwc->dma_sconfig.src_maxburst;
 
 		dwc->dma_sconfig.dst_addr_width = min(mem_width, reg_width * reg_burst);
 	}
@@ -839,15 +854,11 @@  static int dwc_verify_m_buswidth(struct dma_chan *chan)
 static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
 {
 	struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
-	struct dw_dma *dw = to_dw_dma(chan->device);
 	int err;
 
 	memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig));
 
-	dwc->dma_sconfig.src_maxburst =
-		clamp(dwc->dma_sconfig.src_maxburst, 1U, dwc->max_burst);
-	dwc->dma_sconfig.dst_maxburst =
-		clamp(dwc->dma_sconfig.dst_maxburst, 1U, dwc->max_burst);
+	dwc_verify_maxburst(chan);
 
 	err = dwc_verify_p_buswidth(chan);
 	if (err)
@@ -857,9 +868,6 @@  static int dwc_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
 	if (err)
 		return err;
 
-	dw->encode_maxburst(dwc, &dwc->dma_sconfig.src_maxburst);
-	dw->encode_maxburst(dwc, &dwc->dma_sconfig.dst_maxburst);
-
 	return 0;
 }
 
diff --git a/drivers/dma/dw/dw.c b/drivers/dma/dw/dw.c
index c65438d1f1ff..c52333646edd 100644
--- a/drivers/dma/dw/dw.c
+++ b/drivers/dma/dw/dw.c
@@ -64,6 +64,15 @@  static size_t dw_dma_block2bytes(struct dw_dma_chan *dwc, u32 block, u32 width)
 	return DWC_CTLH_BLOCK_TS(block) << width;
 }
 
+static inline u8 dw_dma_encode_maxburst(u32 maxburst)
+{
+	/*
+	 * Fix burst size according to dw_dmac. We need to convert them as:
+	 * 1 -> 0, 4 -> 1, 8 -> 2, 16 -> 3.
+	 */
+	return maxburst > 1 ? fls(maxburst) - 2 : 0;
+}
+
 static u32 dw_dma_prepare_ctllo(struct dw_dma_chan *dwc)
 {
 	struct dma_slave_config	*sconfig = &dwc->dma_sconfig;
@@ -73,10 +82,10 @@  static u32 dw_dma_prepare_ctllo(struct dw_dma_chan *dwc)
 		sms = dwc->dws.m_master;
 		smsize = 0;
 		dms = dwc->dws.p_master;
-		dmsize = sconfig->dst_maxburst;
+		dmsize = dw_dma_encode_maxburst(sconfig->dst_maxburst);
 	} else if (dwc->direction == DMA_DEV_TO_MEM) {
 		sms = dwc->dws.p_master;
-		smsize = sconfig->src_maxburst;
+		smsize = dw_dma_encode_maxburst(sconfig->src_maxburst);
 		dms = dwc->dws.m_master;
 		dmsize = 0;
 	} else /* DMA_MEM_TO_MEM */ {
@@ -91,15 +100,6 @@  static u32 dw_dma_prepare_ctllo(struct dw_dma_chan *dwc)
 	       DWC_CTLL_DMS(dms) | DWC_CTLL_SMS(sms);
 }
 
-static void dw_dma_encode_maxburst(struct dw_dma_chan *dwc, u32 *maxburst)
-{
-	/*
-	 * Fix burst size according to dw_dmac. We need to convert them as:
-	 * 1 -> 0, 4 -> 1, 8 -> 2, 16 -> 3.
-	 */
-	*maxburst = *maxburst > 1 ? fls(*maxburst) - 2 : 0;
-}
-
 static void dw_dma_set_device_name(struct dw_dma *dw, int id)
 {
 	snprintf(dw->name, sizeof(dw->name), "dw:dmac%d", id);
@@ -128,7 +128,6 @@  int dw_dma_probe(struct dw_dma_chip *chip)
 	dw->suspend_chan = dw_dma_suspend_chan;
 	dw->resume_chan = dw_dma_resume_chan;
 	dw->prepare_ctllo = dw_dma_prepare_ctllo;
-	dw->encode_maxburst = dw_dma_encode_maxburst;
 	dw->bytes2block = dw_dma_bytes2block;
 	dw->block2bytes = dw_dma_block2bytes;
 
diff --git a/drivers/dma/dw/idma32.c b/drivers/dma/dw/idma32.c
index 3a1b12517655..428aba9fc2db 100644
--- a/drivers/dma/dw/idma32.c
+++ b/drivers/dma/dw/idma32.c
@@ -199,6 +199,11 @@  static size_t idma32_block2bytes(struct dw_dma_chan *dwc, u32 block, u32 width)
 	return IDMA32C_CTLH_BLOCK_TS(block);
 }
 
+static inline u32 idma32_encode_maxburst(u32 maxburst)
+{
+	return maxburst > 1 ? fls(maxburst) - 1 : 0;
+}
+
 static u32 idma32_prepare_ctllo(struct dw_dma_chan *dwc)
 {
 	struct dma_slave_config	*sconfig = &dwc->dma_sconfig;
@@ -206,9 +211,9 @@  static u32 idma32_prepare_ctllo(struct dw_dma_chan *dwc)
 
 	if (dwc->direction == DMA_MEM_TO_DEV) {
 		smsize = 0;
-		dmsize = sconfig->dst_maxburst;
+		dmsize = idma32_encode_maxburst(sconfig->dst_maxburst);
 	} else if (dwc->direction == DMA_DEV_TO_MEM) {
-		smsize = sconfig->src_maxburst;
+		smsize = idma32_encode_maxburst(sconfig->src_maxburst);
 		dmsize = 0;
 	} else /* DMA_MEM_TO_MEM */ {
 		smsize = 0;
@@ -219,11 +224,6 @@  static u32 idma32_prepare_ctllo(struct dw_dma_chan *dwc)
 	       DWC_CTLL_DST_MSIZE(dmsize) | DWC_CTLL_SRC_MSIZE(smsize);
 }
 
-static void idma32_encode_maxburst(struct dw_dma_chan *dwc, u32 *maxburst)
-{
-	*maxburst = *maxburst > 1 ? fls(*maxburst) - 1 : 0;
-}
-
 static void idma32_set_device_name(struct dw_dma *dw, int id)
 {
 	snprintf(dw->name, sizeof(dw->name), "idma32:dmac%d", id);
@@ -280,7 +280,6 @@  int idma32_dma_probe(struct dw_dma_chip *chip)
 	dw->suspend_chan = idma32_suspend_chan;
 	dw->resume_chan = idma32_resume_chan;
 	dw->prepare_ctllo = idma32_prepare_ctllo;
-	dw->encode_maxburst = idma32_encode_maxburst;
 	dw->bytes2block = idma32_bytes2block;
 	dw->block2bytes = idma32_block2bytes;
 
diff --git a/drivers/dma/dw/regs.h b/drivers/dma/dw/regs.h
index 76654bd13c1a..5969d9cc8d7a 100644
--- a/drivers/dma/dw/regs.h
+++ b/drivers/dma/dw/regs.h
@@ -327,7 +327,6 @@  struct dw_dma {
 	void	(*suspend_chan)(struct dw_dma_chan *dwc, bool drain);
 	void	(*resume_chan)(struct dw_dma_chan *dwc, bool drain);
 	u32	(*prepare_ctllo)(struct dw_dma_chan *dwc);
-	void	(*encode_maxburst)(struct dw_dma_chan *dwc, u32 *maxburst);
 	u32	(*bytes2block)(struct dw_dma_chan *dwc, size_t bytes,
 			       unsigned int width, size_t *len);
 	size_t	(*block2bytes)(struct dw_dma_chan *dwc, u32 block, u32 width);