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 <[email protected]>
---
drivers/dma/imx-sdma.c | 137 +++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 124 insertions(+), 13 deletions(-)
diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 77b6aab..5e44821 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,75 @@ static int sdma_disable_channel(struct dma_chan *chan)
return 0;
}
+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;
+
+ if (sdmac->event_id0 > 31) {
+ sdmac->event_mask[0] |= 0;
+ __set_bit(28, &sdmac->watermark_level);
+ sdmac->event_mask[1] |=
+ BIT(sdmac->event_id0 % 32);
+ } else {
+ sdmac->event_mask[0] |= 0;
+ sdmac->event_mask[1] |=
+ BIT(sdmac->event_id0 % 32);
+ }
+ if (sdmac->event_id1 > 31) {
+ sdmac->event_mask[1] |= 0;
+ __set_bit(29, &sdmac->watermark_level);
+ sdmac->event_mask[0] |=
+ BIT(sdmac->event_id1 % 32);
+ } else {
+ sdmac->event_mask[1] |= 0;
+ sdmac->event_mask[0] |=
+ BIT(sdmac->event_id1 % 32);
+ }
+
+ /*
+ * 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]);
+ }
+ /* BIT 11:
+ * 1 : Source on SPBA
+ * 0 : Source on AIPS
+ */
+ if (sdmac->per_address >= sdma->spba_start_addr &&
+ sdmac->per_address <= sdma->spba_end_addr)
+ __set_bit(11, &sdmac->watermark_level);
+ /* BIT 12:
+ * 1 : Destination on SPBA
+ * 0 : Destination on AIPS
+ */
+ if (sdmac->per_address2 >= sdma->spba_start_addr &&
+ sdmac->per_address2 <= sdma->spba_end_addr)
+ __set_bit(12, &sdmac->watermark_level);
+
+ __set_bit(31, &sdmac->watermark_level);
+ /* BIT 31:
+ * 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 is equal to
+ * the count field of mode word
+ *
+ */
+ __set_bit(25, &sdmac->watermark_level);
+ __clear_bit(24, &sdmac->watermark_level);
+}
+
static int sdma_config_channel(struct dma_chan *chan)
{
struct sdma_channel *sdmac = to_sdma_chan(chan);
@@ -869,6 +945,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 +969,21 @@ 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;
+ if (sdmac->direction == DMA_DEV_TO_DEV) {
+ sdmac->shp_addr = sdmac->per_address2;
+ sdmac->per_addr = sdmac->per_address;
+ } else {
+ sdmac->shp_addr = sdmac->per_address;
+ }
} else {
sdmac->watermark_level = 0; /* FIXME: M3_BASE_ADDRESS */
}
@@ -987,6 +1071,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 +1306,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_address = dmaengine_cfg->src_addr;
+ sdmac->per_address2 = 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 +1537,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 +1554,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 +1711,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
On Tue, Jun 23, 2015 at 04:42:54PM +0800, Shengjiu Wang wrote:
> +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;
> +
> + if (sdmac->event_id0 > 31) {
> + sdmac->event_mask[0] |= 0;
> + __set_bit(28, &sdmac->watermark_level);
why not use set_bit(), you are modifying driver memory
> + sdmac->event_mask[1] |=
> + BIT(sdmac->event_id0 % 32);
and then why not use set_bit() here too?
> + } else {
> + sdmac->event_mask[0] |= 0;
> + sdmac->event_mask[1] |=
> + BIT(sdmac->event_id0 % 32);
> + }
> + if (sdmac->event_id1 > 31) {
> + sdmac->event_mask[1] |= 0;
> + __set_bit(29, &sdmac->watermark_level);
> + sdmac->event_mask[0] |=
> + BIT(sdmac->event_id1 % 32);
> + } else {
> + sdmac->event_mask[1] |= 0;
> + sdmac->event_mask[0] |=
> + BIT(sdmac->event_id1 % 32);
> + }
pattern for eventidX is repeated, also in that we can make generic macro to
handle and reduce code size
> +
> + /*
> + * 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;
Magic number?
> static int sdma_config_channel(struct dma_chan *chan)
> {
> struct sdma_channel *sdmac = to_sdma_chan(chan);
> @@ -869,6 +945,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 +969,21 @@ 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;
> + if (sdmac->direction == DMA_DEV_TO_DEV) {
Okay the direction is depreciated, so can you store both source and
destination and use them based on direction in prepare()
Also I see driver is not doing this, so while at it, can you fix this is
current code as well
--
~Vinod
Hi vinod
On Tue, Jul 07, 2015 at 09:50:57AM +0530, Vinod Koul wrote:
> On Tue, Jun 23, 2015 at 04:42:54PM +0800, Shengjiu Wang wrote:
> > +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;
> > +
> > + if (sdmac->event_id0 > 31) {
> > + sdmac->event_mask[0] |= 0;
> > + __set_bit(28, &sdmac->watermark_level);
> why not use set_bit(), you are modifying driver memory
Original driver all use the __set_bit. do you think we need to change
all the __set_bit to set_bit? And from the header file "arch/arm/include/asm
/bitops.h", the set_bit is same as __set_bit.
>
>
> > + sdmac->event_mask[1] |=
> > + BIT(sdmac->event_id0 % 32);
> and then why not use set_bit() here too?
>
> > + } else {
> > + sdmac->event_mask[0] |= 0;
> > + sdmac->event_mask[1] |=
> > + BIT(sdmac->event_id0 % 32);
> > + }
> > + if (sdmac->event_id1 > 31) {
> > + sdmac->event_mask[1] |= 0;
> > + __set_bit(29, &sdmac->watermark_level);
> > + sdmac->event_mask[0] |=
> > + BIT(sdmac->event_id1 % 32);
> > + } else {
> > + sdmac->event_mask[1] |= 0;
> > + sdmac->event_mask[0] |=
> > + BIT(sdmac->event_id1 % 32);
> > + }
> pattern for eventidX is repeated, also in that we can make generic macro to
> handle and reduce code size
I will change this.
>
> > +
> > + /*
> > + * 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;
> Magic number?
>
> > static int sdma_config_channel(struct dma_chan *chan)
> > {
> > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > @@ -869,6 +945,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 +969,21 @@ 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;
> > + if (sdmac->direction == DMA_DEV_TO_DEV) {
> Okay the direction is depreciated, so can you store both source and
> destination and use them based on direction in prepare()
>
> Also I see driver is not doing this, so while at it, can you fix this is
> current code as well
>
which prepare() do you mean? sdma_prep_dma_cyclic, sdma_prep_slave_sg?
> --
> ~Vinod
>
best regards
wang shengjiu
On Tue, Jul 07, 2015 at 01:24:20PM +0800, Shengjiu Wang wrote:
> Hi vinod
>
> On Tue, Jul 07, 2015 at 09:50:57AM +0530, Vinod Koul wrote:
> > On Tue, Jun 23, 2015 at 04:42:54PM +0800, Shengjiu Wang wrote:
> > > +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;
> > > +
> > > + if (sdmac->event_id0 > 31) {
> > > + sdmac->event_mask[0] |= 0;
> > > + __set_bit(28, &sdmac->watermark_level);
> > why not use set_bit(), you are modifying driver memory
> Original driver all use the __set_bit. do you think we need to change
> all the __set_bit to set_bit? And from the header file "arch/arm/include/asm
> /bitops.h", the set_bit is same as __set_bit.
> >
> >
> > > + sdmac->event_mask[1] |=
> > > + BIT(sdmac->event_id0 % 32);
> > and then why not use set_bit() here too?
> >
> > > + } else {
> > > + sdmac->event_mask[0] |= 0;
> > > + sdmac->event_mask[1] |=
> > > + BIT(sdmac->event_id0 % 32);
> > > + }
> > > + if (sdmac->event_id1 > 31) {
> > > + sdmac->event_mask[1] |= 0;
> > > + __set_bit(29, &sdmac->watermark_level);
> > > + sdmac->event_mask[0] |=
> > > + BIT(sdmac->event_id1 % 32);
> > > + } else {
> > > + sdmac->event_mask[1] |= 0;
> > > + sdmac->event_mask[0] |=
> > > + BIT(sdmac->event_id1 % 32);
> > > + }
> > pattern for eventidX is repeated, also in that we can make generic macro to
> > handle and reduce code size
> I will change this.
> >
> > > +
> > > + /*
> > > + * 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;
> > Magic number?
> >
> > > static int sdma_config_channel(struct dma_chan *chan)
> > > {
> > > struct sdma_channel *sdmac = to_sdma_chan(chan);
> > > @@ -869,6 +945,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 +969,21 @@ 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;
> > > + if (sdmac->direction == DMA_DEV_TO_DEV) {
> > Okay the direction is depreciated, so can you store both source and
> > destination and use them based on direction in prepare()
> >
> > Also I see driver is not doing this, so while at it, can you fix this is
> > current code as well
> >
> which prepare() do you mean? sdma_prep_dma_cyclic, sdma_prep_slave_sg?
>
I exchange the meaning of per_address and per_address2 for p2p. So can remove
the direction checking here.
> > --
> > ~Vinod
> >
>
>
I have sent a V2 patch for review. Thanks.
> best regards
> wang shengjiu
On Fri, Jul 10, 2015 at 11:52:05AM +0530, Vinod Koul wrote:
> On Tue, Jul 07, 2015 at 01:24:22PM +0800, Shengjiu Wang wrote:
> > > why not use set_bit(), you are modifying driver memory
> > Original driver all use the __set_bit. do you think we need to change
> > all the __set_bit to set_bit? And from the header file "arch/arm/include/asm
> > /bitops.h", the set_bit is same as __set_bit.
> New changes should be rightly done. __xxx denotes typically internal API to
> a subsystem, so if they are same I would suggest to use set_bit()
Ok, I agree with you.
>
> > > > + /*
> > > > + * 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;
> > > Magic number?
> ??
I have add a description on V2 patch for the watermark_level definition.
>
> > > Okay the direction is depreciated, so can you store both source and
> > > destination and use them based on direction in prepare()
> > >
> > > Also I see driver is not doing this, so while at it, can you fix this is
> > > current code as well
> > >
> > which prepare() do you mean? sdma_prep_dma_cyclic, sdma_prep_slave_sg?
> both have direction as an argument. So you need to use that
I have removed the direction checking. So I think don't need to change the
sdma_prep_dma_cyclic, sdma_prep_slave_sg.
Anyway please review the V2 patch for this, I have sent it out. In V2, I still
use the __set_bit. After you review the V2 patch, I will change them together.
Thanks.
wang shengjiu
>
> --
> ~Vinod
>
On Tue, Jul 07, 2015 at 01:24:22PM +0800, Shengjiu Wang wrote:
> > why not use set_bit(), you are modifying driver memory
> Original driver all use the __set_bit. do you think we need to change
> all the __set_bit to set_bit? And from the header file "arch/arm/include/asm
> /bitops.h", the set_bit is same as __set_bit.
New changes should be rightly done. __xxx denotes typically internal API to
a subsystem, so if they are same I would suggest to use set_bit()
> > > + /*
> > > + * 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;
> > Magic number?
??
> > Okay the direction is depreciated, so can you store both source and
> > destination and use them based on direction in prepare()
> >
> > Also I see driver is not doing this, so while at it, can you fix this is
> > current code as well
> >
> which prepare() do you mean? sdma_prep_dma_cyclic, sdma_prep_slave_sg?
both have direction as an argument. So you need to use that
--
~Vinod