All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH V2] dmaengine: imx-sdma: Add device to device support
@ 2015-07-07  9:20 Shengjiu Wang
  2015-07-10  7:58 ` Vinod Koul
  0 siblings, 1 reply; 4+ messages in thread
From: Shengjiu Wang @ 2015-07-07  9:20 UTC (permalink / raw)
  To: vinod.koul, dan.j.williams; +Cc: dmaengine, linux-kernel

This patch adds DEV_TO_DEV support for i.MX SDMA driver to support data
transfer between two peripheral FIFOs.
The per_2_per script requires two peripheral addresses and two DMA
requests, and it need to check the src addr and dst addr is in the SPBA
bus space or in the AIPS bus space.

Signed-off-by: Shengjiu Wang <shengjiu.wang@freescale.com>
---
changes in V2
- refine the sdma_set_watermarklevel_for_p2p(), add description.
- exchange the meaning of per_address, per_address2. per_address is dst_addr
  per_address2 is src_addr, which is align with not p2p case.

 drivers/dma/imx-sdma.c |  140 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 128 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 77b6aab..845f200 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -35,6 +35,7 @@
 #include <linux/platform_device.h>
 #include <linux/dmaengine.h>
 #include <linux/of.h>
+#include <linux/of_address.h>
 #include <linux/of_device.h>
 #include <linux/of_dma.h>
 
@@ -259,8 +260,9 @@ struct sdma_channel {
 	struct sdma_buffer_descriptor	*bd;
 	dma_addr_t			bd_phys;
 	unsigned int			pc_from_device, pc_to_device;
+	unsigned int			device_to_device;
 	unsigned long			flags;
-	dma_addr_t			per_address;
+	dma_addr_t			per_address, per_address2;
 	unsigned long			event_mask[2];
 	unsigned long			watermark_level;
 	u32				shp_addr, per_addr;
@@ -328,6 +330,8 @@ struct sdma_engine {
 	u32				script_number;
 	struct sdma_script_start_addrs	*script_addrs;
 	const struct sdma_driver_data	*drvdata;
+	u32				spba_start_addr;
+	u32				spba_end_addr;
 };
 
 static struct sdma_driver_data sdma_imx31 = {
@@ -705,6 +709,7 @@ static void sdma_get_pc(struct sdma_channel *sdmac,
 
 	sdmac->pc_from_device = 0;
 	sdmac->pc_to_device = 0;
+	sdmac->device_to_device = 0;
 
 	switch (peripheral_type) {
 	case IMX_DMATYPE_MEMORY:
@@ -780,6 +785,7 @@ static void sdma_get_pc(struct sdma_channel *sdmac,
 
 	sdmac->pc_from_device = per_2_emi;
 	sdmac->pc_to_device = emi_2_per;
+	sdmac->device_to_device = per_2_per;
 }
 
 static int sdma_load_context(struct sdma_channel *sdmac)
@@ -792,11 +798,12 @@ static int sdma_load_context(struct sdma_channel *sdmac)
 	int ret;
 	unsigned long flags;
 
-	if (sdmac->direction == DMA_DEV_TO_MEM) {
+	if (sdmac->direction == DMA_DEV_TO_MEM)
 		load_address = sdmac->pc_from_device;
-	} else {
+	else if (sdmac->direction == DMA_DEV_TO_DEV)
+		load_address = sdmac->device_to_device;
+	else
 		load_address = sdmac->pc_to_device;
-	}
 
 	if (load_address < 0)
 		return load_address;
@@ -851,6 +858,84 @@ static int sdma_disable_channel(struct dma_chan *chan)
 	return 0;
 }
 
+/*
+ *  p_2_p watermark_level description
+ *	Bits		Name			Description
+ *	0-7		Lower WML		Lower watermark level
+ *	8		PS			1: Pad Swallowing
+ *						0: No Pad Swallowing
+ *	9		PA			1: Pad Adding
+ *						0: No Pad Adding
+ * 	10		SPDIF			If this bit is set both source
+ *						and destination are on SPBA
+ *	11		Source Bit(SP)		1: Source on SPBA
+ *						0: Source on AIPS
+ *	12		Destination Bit(DP)	1: Destination on SPBA
+ *						0: Destination on AIPS
+ *	13-15		---------		MUST BE 0
+ *	16-23		Higher WML		HWML
+ *	24-27		N			Total number of samples after
+ *						which Pad adding/Swallowing
+ *						must be done. It must be odd.
+ *	28		Lower WML Event(LWE)	SDMA events reg to check for
+ *						LWML event mask
+ *						0: LWE in EVENTS register
+ *						1: LWE in EVENTS2 register
+ *	29		Higher WML Event(HWE)	SDMA events reg to check for
+ *						HWML event mask
+ *						0: HWE in EVENTS register
+ *						1: HWE in EVENTS2 register
+ *	30		---------		MUST BE 0
+ *	31		CONT			1: Amount of samples to be
+ *						transferred is unknown and
+ *						script will keep on
+ *						transferring samples as long as
+ *						both events are detected and
+ *						script must be manually stopped
+ *						by the application
+ *						0: The amount of samples to be
+ *						transferred is equal to the
+ *						count field of mode word
+ */
+static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
+{
+	struct sdma_engine *sdma = sdmac->sdma;
+
+	int lwml = sdmac->watermark_level & 0xff;
+	int hwml = (sdmac->watermark_level >> 16) & 0xff;
+
+	__set_bit(sdmac->event_id0 % 32, &sdmac->event_mask[1]);
+	__set_bit(sdmac->event_id1 % 32, &sdmac->event_mask[0]);
+
+	if (sdmac->event_id0 > 31)
+		__set_bit(28, &sdmac->watermark_level);
+
+	if (sdmac->event_id1 > 31)
+		__set_bit(29, &sdmac->watermark_level);
+
+	/*
+	 * If LWML(src_maxburst) > HWML(dst_maxburst), we need
+	 * swap LWML and HWML of INFO(A.3.2.5.1), also need swap
+	 * r0(event_mask[1]) and r1(event_mask[0]).
+	 */
+	if (lwml > hwml) {
+		sdmac->watermark_level &= ~0xff00ff;
+		sdmac->watermark_level |= hwml;
+		sdmac->watermark_level |= lwml << 16;
+		swap(sdmac->event_mask[0], sdmac->event_mask[1]);
+	}
+
+	if (sdmac->per_address2 >= sdma->spba_start_addr &&
+			sdmac->per_address2 <= sdma->spba_end_addr)
+		__set_bit(11, &sdmac->watermark_level);
+
+	if (sdmac->per_address >= sdma->spba_start_addr &&
+			sdmac->per_address <= sdma->spba_end_addr)
+		__set_bit(12, &sdmac->watermark_level);
+
+	__set_bit(31, &sdmac->watermark_level);
+}
+
 static int sdma_config_channel(struct dma_chan *chan)
 {
 	struct sdma_channel *sdmac = to_sdma_chan(chan);
@@ -869,6 +954,12 @@ static int sdma_config_channel(struct dma_chan *chan)
 		sdma_event_enable(sdmac, sdmac->event_id0);
 	}
 
+	if (sdmac->event_id1) {
+		if (sdmac->event_id1 >= sdmac->sdma->drvdata->num_events)
+			return -EINVAL;
+		sdma_event_enable(sdmac, sdmac->event_id1);
+	}
+
 	switch (sdmac->peripheral_type) {
 	case IMX_DMATYPE_DSP:
 		sdma_config_ownership(sdmac, false, true, true);
@@ -887,19 +978,17 @@ static int sdma_config_channel(struct dma_chan *chan)
 			(sdmac->peripheral_type != IMX_DMATYPE_DSP)) {
 		/* Handle multiple event channels differently */
 		if (sdmac->event_id1) {
-			sdmac->event_mask[1] = BIT(sdmac->event_id1 % 32);
-			if (sdmac->event_id1 > 31)
-				__set_bit(31, &sdmac->watermark_level);
-			sdmac->event_mask[0] = BIT(sdmac->event_id0 % 32);
-			if (sdmac->event_id0 > 31)
-				__set_bit(30, &sdmac->watermark_level);
-		} else {
+			if (sdmac->peripheral_type == IMX_DMATYPE_ASRC_SP ||
+			    sdmac->peripheral_type == IMX_DMATYPE_ASRC)
+				sdma_set_watermarklevel_for_p2p(sdmac);
+		} else
 			__set_bit(sdmac->event_id0, sdmac->event_mask);
-		}
+
 		/* Watermark Level */
 		sdmac->watermark_level |= sdmac->watermark_level;
 		/* Address */
 		sdmac->shp_addr = sdmac->per_address;
+		sdmac->per_addr = sdmac->per_address2;
 	} else {
 		sdmac->watermark_level = 0; /* FIXME: M3_BASE_ADDRESS */
 	}
@@ -987,6 +1076,7 @@ static int sdma_alloc_chan_resources(struct dma_chan *chan)
 
 	sdmac->peripheral_type = data->peripheral_type;
 	sdmac->event_id0 = data->dma_request;
+	sdmac->event_id1 = data->dma_request2;
 
 	clk_enable(sdmac->sdma->clk_ipg);
 	clk_enable(sdmac->sdma->clk_ahb);
@@ -1221,6 +1311,14 @@ static int sdma_config(struct dma_chan *chan,
 		sdmac->watermark_level = dmaengine_cfg->src_maxburst *
 			dmaengine_cfg->src_addr_width;
 		sdmac->word_size = dmaengine_cfg->src_addr_width;
+	} else if (dmaengine_cfg->direction == DMA_DEV_TO_DEV) {
+		sdmac->per_address2 = dmaengine_cfg->src_addr;
+		sdmac->per_address = dmaengine_cfg->dst_addr;
+		sdmac->watermark_level =
+			dmaengine_cfg->src_maxburst & 0xff;
+		sdmac->watermark_level |=
+			(dmaengine_cfg->dst_maxburst & 0xff) << 16;
+		sdmac->word_size = dmaengine_cfg->dst_addr_width;
 	} else {
 		sdmac->per_address = dmaengine_cfg->dst_addr;
 		sdmac->watermark_level = dmaengine_cfg->dst_maxburst *
@@ -1444,6 +1542,14 @@ static struct dma_chan *sdma_xlate(struct of_phandle_args *dma_spec,
 	data.dma_request = dma_spec->args[0];
 	data.peripheral_type = dma_spec->args[1];
 	data.priority = dma_spec->args[2];
+	/*
+	 * init dma_request2 to zero, which is not used by the dts.
+	 * For P2P, dma_request2 is init from dma_request_channel(),
+	 * chan->private will point to the imx_dma_data, and in
+	 * device_alloc_chan_resources(), imx_dma_data.dma_request2 will
+	 * be set to sdmac->event_id1.
+	 */
+	data.dma_request2 = 0;
 
 	return dma_request_channel(mask, sdma_filter_fn, &data);
 }
@@ -1453,10 +1559,12 @@ static int sdma_probe(struct platform_device *pdev)
 	const struct of_device_id *of_id =
 			of_match_device(sdma_dt_ids, &pdev->dev);
 	struct device_node *np = pdev->dev.of_node;
+	struct device_node *spba_bus;
 	const char *fw_name;
 	int ret;
 	int irq;
 	struct resource *iores;
+	struct resource spba_res;
 	struct sdma_platform_data *pdata = dev_get_platdata(&pdev->dev);
 	int i;
 	struct sdma_engine *sdma;
@@ -1608,6 +1716,14 @@ static int sdma_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "failed to register controller\n");
 			goto err_register;
 		}
+
+		spba_bus = of_find_compatible_node(NULL, NULL, "fsl,spba-bus");
+		ret = of_address_to_resource(spba_bus, 0, &spba_res);
+		if (!ret) {
+			sdma->spba_start_addr = spba_res.start;
+			sdma->spba_end_addr = spba_res.end;
+		}
+		of_node_put(spba_bus);
 	}
 
 	dev_info(sdma->dev, "initialized\n");
-- 
1.7.9.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] dmaengine: imx-sdma: Add device to device support
  2015-07-07  9:20 [PATCH V2] dmaengine: imx-sdma: Add device to device support Shengjiu Wang
@ 2015-07-10  7:58 ` Vinod Koul
  2015-07-10  7:58   ` Shengjiu Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Vinod Koul @ 2015-07-10  7:58 UTC (permalink / raw)
  To: Shengjiu Wang; +Cc: dan.j.williams, dmaengine, linux-kernel

On Tue, Jul 07, 2015 at 05:20:04PM +0800, Shengjiu Wang wrote:

> +/*
> + *  p_2_p watermark_level description
> + *	Bits		Name			Description
> + *	0-7		Lower WML		Lower watermark level
> + *	8		PS			1: Pad Swallowing
> + *						0: No Pad Swallowing
> + *	9		PA			1: Pad Adding
> + *						0: No Pad Adding
> + * 	10		SPDIF			If this bit is set both source
> + *						and destination are on SPBA
> + *	11		Source Bit(SP)		1: Source on SPBA
> + *						0: Source on AIPS
> + *	12		Destination Bit(DP)	1: Destination on SPBA
> + *						0: Destination on AIPS
> + *	13-15		---------		MUST BE 0
> + *	16-23		Higher WML		HWML
> + *	24-27		N			Total number of samples after
> + *						which Pad adding/Swallowing
> + *						must be done. It must be odd.
> + *	28		Lower WML Event(LWE)	SDMA events reg to check for
> + *						LWML event mask
> + *						0: LWE in EVENTS register
> + *						1: LWE in EVENTS2 register
> + *	29		Higher WML Event(HWE)	SDMA events reg to check for
> + *						HWML event mask
> + *						0: HWE in EVENTS register
> + *						1: HWE in EVENTS2 register
> + *	30		---------		MUST BE 0
> + *	31		CONT			1: Amount of samples to be
> + *						transferred is unknown and
> + *						script will keep on
> + *						transferring samples as long as
> + *						both events are detected and
> + *						script must be manually stopped
> + *						by the application
> + *						0: The amount of samples to be
> + *						transferred is equal to the
> + *						count field of mode word
> + */
> +static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
> +{
> +	struct sdma_engine *sdma = sdmac->sdma;
> +
> +	int lwml = sdmac->watermark_level & 0xff;
> +	int hwml = (sdmac->watermark_level >> 16) & 0xff;
> +
> +	__set_bit(sdmac->event_id0 % 32, &sdmac->event_mask[1]);
> +	__set_bit(sdmac->event_id1 % 32, &sdmac->event_mask[0]);
> +
> +	if (sdmac->event_id0 > 31)
> +		__set_bit(28, &sdmac->watermark_level);
> +
> +	if (sdmac->event_id1 > 31)
> +		__set_bit(29, &sdmac->watermark_level);
> +
> +	/*
> +	 * If LWML(src_maxburst) > HWML(dst_maxburst), we need
> +	 * swap LWML and HWML of INFO(A.3.2.5.1), also need swap
> +	 * r0(event_mask[1]) and r1(event_mask[0]).
> +	 */
> +	if (lwml > hwml) {
> +		sdmac->watermark_level &= ~0xff00ff;
> +		sdmac->watermark_level |= hwml;
> +		sdmac->watermark_level |= lwml << 16;
> +		swap(sdmac->event_mask[0], sdmac->event_mask[1]);
so typical practice is to define macros for these. For example above can be
defined as
#define SDMA_WATERMARK_LWML BIT(X)
#define SDMA_WATERMARK_PA BIT(Y)

Then here you can say
	sdmac->watermark_level &= SDMA_WATERMARK_PA | ....

That way it is easier to read, review and maintain


> +	}
> +
> +	if (sdmac->per_address2 >= sdma->spba_start_addr &&
> +			sdmac->per_address2 <= sdma->spba_end_addr)
> +		__set_bit(11, &sdmac->watermark_level);
> +
> +	if (sdmac->per_address >= sdma->spba_start_addr &&
> +			sdmac->per_address <= sdma->spba_end_addr)
> +		__set_bit(12, &sdmac->watermark_level);
> +
> +	__set_bit(31, &sdmac->watermark_level);
All these and few more below will look much better by have defines above

> @@ -1221,6 +1311,14 @@ static int sdma_config(struct dma_chan *chan,
>  		sdmac->watermark_level = dmaengine_cfg->src_maxburst *
>  			dmaengine_cfg->src_addr_width;
>  		sdmac->word_size = dmaengine_cfg->src_addr_width;
> +	} else if (dmaengine_cfg->direction == DMA_DEV_TO_DEV) {
> +		sdmac->per_address2 = dmaengine_cfg->src_addr;
> +		sdmac->per_address = dmaengine_cfg->dst_addr;
> +		sdmac->watermark_level =
> +			dmaengine_cfg->src_maxburst & 0xff;
> +		sdmac->watermark_level |=
> +			(dmaengine_cfg->dst_maxburst & 0xff) << 16;
> +		sdmac->word_size = dmaengine_cfg->dst_addr_width;
Please fix this as well

-- 
~Vinod

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] dmaengine: imx-sdma: Add device to device support
  2015-07-10  7:58 ` Vinod Koul
@ 2015-07-10  7:58   ` Shengjiu Wang
  2015-07-17  6:07     ` Shengjiu Wang
  0 siblings, 1 reply; 4+ messages in thread
From: Shengjiu Wang @ 2015-07-10  7:58 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, linux-kernel

On Fri, Jul 10, 2015 at 01:28:07PM +0530, Vinod Koul wrote:
> On Tue, Jul 07, 2015 at 05:20:04PM +0800, Shengjiu Wang wrote:
> 
> > +/*
> > + *  p_2_p watermark_level description
> > + *	Bits		Name			Description
> > + *	0-7		Lower WML		Lower watermark level
> > + *	8		PS			1: Pad Swallowing
> > + *						0: No Pad Swallowing
> > + *	9		PA			1: Pad Adding
> > + *						0: No Pad Adding
> > + * 	10		SPDIF			If this bit is set both source
> > + *						and destination are on SPBA
> > + *	11		Source Bit(SP)		1: Source on SPBA
> > + *						0: Source on AIPS
> > + *	12		Destination Bit(DP)	1: Destination on SPBA
> > + *						0: Destination on AIPS
> > + *	13-15		---------		MUST BE 0
> > + *	16-23		Higher WML		HWML
> > + *	24-27		N			Total number of samples after
> > + *						which Pad adding/Swallowing
> > + *						must be done. It must be odd.
> > + *	28		Lower WML Event(LWE)	SDMA events reg to check for
> > + *						LWML event mask
> > + *						0: LWE in EVENTS register
> > + *						1: LWE in EVENTS2 register
> > + *	29		Higher WML Event(HWE)	SDMA events reg to check for
> > + *						HWML event mask
> > + *						0: HWE in EVENTS register
> > + *						1: HWE in EVENTS2 register
> > + *	30		---------		MUST BE 0
> > + *	31		CONT			1: Amount of samples to be
> > + *						transferred is unknown and
> > + *						script will keep on
> > + *						transferring samples as long as
> > + *						both events are detected and
> > + *						script must be manually stopped
> > + *						by the application
> > + *						0: The amount of samples to be
> > + *						transferred is equal to the
> > + *						count field of mode word
> > + */
> > +static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
> > +{
> > +	struct sdma_engine *sdma = sdmac->sdma;
> > +
> > +	int lwml = sdmac->watermark_level & 0xff;
> > +	int hwml = (sdmac->watermark_level >> 16) & 0xff;
> > +
> > +	__set_bit(sdmac->event_id0 % 32, &sdmac->event_mask[1]);
> > +	__set_bit(sdmac->event_id1 % 32, &sdmac->event_mask[0]);
> > +
> > +	if (sdmac->event_id0 > 31)
> > +		__set_bit(28, &sdmac->watermark_level);
> > +
> > +	if (sdmac->event_id1 > 31)
> > +		__set_bit(29, &sdmac->watermark_level);
> > +
> > +	/*
> > +	 * If LWML(src_maxburst) > HWML(dst_maxburst), we need
> > +	 * swap LWML and HWML of INFO(A.3.2.5.1), also need swap
> > +	 * r0(event_mask[1]) and r1(event_mask[0]).
> > +	 */
> > +	if (lwml > hwml) {
> > +		sdmac->watermark_level &= ~0xff00ff;
> > +		sdmac->watermark_level |= hwml;
> > +		sdmac->watermark_level |= lwml << 16;
> > +		swap(sdmac->event_mask[0], sdmac->event_mask[1]);
> so typical practice is to define macros for these. For example above can be
> defined as
> #define SDMA_WATERMARK_LWML BIT(X)
> #define SDMA_WATERMARK_PA BIT(Y)
> 
> Then here you can say
> 	sdmac->watermark_level &= SDMA_WATERMARK_PA | ....
> 
> That way it is easier to read, review and maintain
> 
> 
> > +	}
> > +
> > +	if (sdmac->per_address2 >= sdma->spba_start_addr &&
> > +			sdmac->per_address2 <= sdma->spba_end_addr)
> > +		__set_bit(11, &sdmac->watermark_level);
> > +
> > +	if (sdmac->per_address >= sdma->spba_start_addr &&
> > +			sdmac->per_address <= sdma->spba_end_addr)
> > +		__set_bit(12, &sdmac->watermark_level);
> > +
> > +	__set_bit(31, &sdmac->watermark_level);
> All these and few more below will look much better by have defines above
> 
> > @@ -1221,6 +1311,14 @@ static int sdma_config(struct dma_chan *chan,
> >  		sdmac->watermark_level = dmaengine_cfg->src_maxburst *
> >  			dmaengine_cfg->src_addr_width;
> >  		sdmac->word_size = dmaengine_cfg->src_addr_width;
> > +	} else if (dmaengine_cfg->direction == DMA_DEV_TO_DEV) {
> > +		sdmac->per_address2 = dmaengine_cfg->src_addr;
> > +		sdmac->per_address = dmaengine_cfg->dst_addr;
> > +		sdmac->watermark_level =
> > +			dmaengine_cfg->src_maxburst & 0xff;
> > +		sdmac->watermark_level |=
> > +			(dmaengine_cfg->dst_maxburst & 0xff) << 16;
> > +		sdmac->word_size = dmaengine_cfg->dst_addr_width;
> Please fix this as well
>
I will refine this patch and send V3 for review. Thanks 

best regards
wang shengjiu
> -- 
> ~Vinod

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH V2] dmaengine: imx-sdma: Add device to device support
  2015-07-10  7:58   ` Shengjiu Wang
@ 2015-07-17  6:07     ` Shengjiu Wang
  0 siblings, 0 replies; 4+ messages in thread
From: Shengjiu Wang @ 2015-07-17  6:07 UTC (permalink / raw)
  To: Vinod Koul; +Cc: dan.j.williams, dmaengine, linux-kernel

On Fri, Jul 10, 2015 at 03:58:36PM +0800, Shengjiu Wang wrote:
> On Fri, Jul 10, 2015 at 01:28:07PM +0530, Vinod Koul wrote:
> > On Tue, Jul 07, 2015 at 05:20:04PM +0800, Shengjiu Wang wrote:
> > 
> > > +/*
> > > + *  p_2_p watermark_level description
> > > + *	Bits		Name			Description
> > > + *	0-7		Lower WML		Lower watermark level
> > > + *	8		PS			1: Pad Swallowing
> > > + *						0: No Pad Swallowing
> > > + *	9		PA			1: Pad Adding
> > > + *						0: No Pad Adding
> > > + * 	10		SPDIF			If this bit is set both source
> > > + *						and destination are on SPBA
> > > + *	11		Source Bit(SP)		1: Source on SPBA
> > > + *						0: Source on AIPS
> > > + *	12		Destination Bit(DP)	1: Destination on SPBA
> > > + *						0: Destination on AIPS
> > > + *	13-15		---------		MUST BE 0
> > > + *	16-23		Higher WML		HWML
> > > + *	24-27		N			Total number of samples after
> > > + *						which Pad adding/Swallowing
> > > + *						must be done. It must be odd.
> > > + *	28		Lower WML Event(LWE)	SDMA events reg to check for
> > > + *						LWML event mask
> > > + *						0: LWE in EVENTS register
> > > + *						1: LWE in EVENTS2 register
> > > + *	29		Higher WML Event(HWE)	SDMA events reg to check for
> > > + *						HWML event mask
> > > + *						0: HWE in EVENTS register
> > > + *						1: HWE in EVENTS2 register
> > > + *	30		---------		MUST BE 0
> > > + *	31		CONT			1: Amount of samples to be
> > > + *						transferred is unknown and
> > > + *						script will keep on
> > > + *						transferring samples as long as
> > > + *						both events are detected and
> > > + *						script must be manually stopped
> > > + *						by the application
> > > + *						0: The amount of samples to be
> > > + *						transferred is equal to the
> > > + *						count field of mode word
> > > + */
> > > +static void sdma_set_watermarklevel_for_p2p(struct sdma_channel *sdmac)
> > > +{
> > > +	struct sdma_engine *sdma = sdmac->sdma;
> > > +
> > > +	int lwml = sdmac->watermark_level & 0xff;
> > > +	int hwml = (sdmac->watermark_level >> 16) & 0xff;
> > > +
> > > +	__set_bit(sdmac->event_id0 % 32, &sdmac->event_mask[1]);
> > > +	__set_bit(sdmac->event_id1 % 32, &sdmac->event_mask[0]);
> > > +
> > > +	if (sdmac->event_id0 > 31)
> > > +		__set_bit(28, &sdmac->watermark_level);
> > > +
> > > +	if (sdmac->event_id1 > 31)
> > > +		__set_bit(29, &sdmac->watermark_level);
> > > +
> > > +	/*
> > > +	 * If LWML(src_maxburst) > HWML(dst_maxburst), we need
> > > +	 * swap LWML and HWML of INFO(A.3.2.5.1), also need swap
> > > +	 * r0(event_mask[1]) and r1(event_mask[0]).
> > > +	 */
> > > +	if (lwml > hwml) {
> > > +		sdmac->watermark_level &= ~0xff00ff;
> > > +		sdmac->watermark_level |= hwml;
> > > +		sdmac->watermark_level |= lwml << 16;
> > > +		swap(sdmac->event_mask[0], sdmac->event_mask[1]);
> > so typical practice is to define macros for these. For example above can be
> > defined as
> > #define SDMA_WATERMARK_LWML BIT(X)
> > #define SDMA_WATERMARK_PA BIT(Y)
> > 
> > Then here you can say
> > 	sdmac->watermark_level &= SDMA_WATERMARK_PA | ....
> > 
> > That way it is easier to read, review and maintain
> > 
> > 
> > > +	}
> > > +
> > > +	if (sdmac->per_address2 >= sdma->spba_start_addr &&
> > > +			sdmac->per_address2 <= sdma->spba_end_addr)
> > > +		__set_bit(11, &sdmac->watermark_level);
> > > +
> > > +	if (sdmac->per_address >= sdma->spba_start_addr &&
> > > +			sdmac->per_address <= sdma->spba_end_addr)
> > > +		__set_bit(12, &sdmac->watermark_level);
> > > +
> > > +	__set_bit(31, &sdmac->watermark_level);
> > All these and few more below will look much better by have defines above
> > 
> > > @@ -1221,6 +1311,14 @@ static int sdma_config(struct dma_chan *chan,
> > >  		sdmac->watermark_level = dmaengine_cfg->src_maxburst *
> > >  			dmaengine_cfg->src_addr_width;
> > >  		sdmac->word_size = dmaengine_cfg->src_addr_width;
> > > +	} else if (dmaengine_cfg->direction == DMA_DEV_TO_DEV) {
> > > +		sdmac->per_address2 = dmaengine_cfg->src_addr;
> > > +		sdmac->per_address = dmaengine_cfg->dst_addr;
> > > +		sdmac->watermark_level =
> > > +			dmaengine_cfg->src_maxburst & 0xff;
> > > +		sdmac->watermark_level |=
> > > +			(dmaengine_cfg->dst_maxburst & 0xff) << 16;
> > > +		sdmac->word_size = dmaengine_cfg->dst_addr_width;
> > Please fix this as well
> >
> I will refine this patch and send V3 for review. Thanks 
> 
> best regards
> wang shengjiu
> > -- 
> > ~Vinod

Hi Vinod
    V3 for this patch has been sent, please review. Thanks.

Best regards
Wang Shengjiu

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-07-17  7:16 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-07-07  9:20 [PATCH V2] dmaengine: imx-sdma: Add device to device support Shengjiu Wang
2015-07-10  7:58 ` Vinod Koul
2015-07-10  7:58   ` Shengjiu Wang
2015-07-17  6:07     ` Shengjiu Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.