[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

dma_async_device_register will re-init chan_id and chancnt,
so whatever chan_id and chancnt are set in drivers, they will
be re-written by dma_async_device_register.

Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Viresh Kumar <viresh.kumar@st.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
Cc: Yong Wang <yong.y.wang@intel.com>
Cc: Jaswinder Singh <jassi.brar@samsung.com>
Cc: Pelagicore AB <info@pelagicore.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/dma/at_hdmac.c      |    5 ++---
 drivers/dma/dw_dmac.c       |    5 ++---
 drivers/dma/intel_mid_dma.c |    2 --
 drivers/dma/mpc512x_dma.c   |    1 -
 drivers/dma/pch_dma.c       |    2 --
 drivers/dma/pl330.c         |    2 --
 drivers/dma/timb_dma.c      |    3 +--
 7 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 6a483ea..c1b8c15 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1260,12 +1260,11 @@ static int __init at_dma_probe(struct platform_device *pdev)
 
 	/* initialize channels related values */
 	INIT_LIST_HEAD(&atdma->dma_common.channels);
-	for (i = 0; i < pdata->nr_channels; i++, atdma->dma_common.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct at_dma_chan	*atchan = &atdma->chan[i];
 
 		atchan->chan_common.device = &atdma->dma_common;
 		atchan->chan_common.cookie = atchan->completed_cookie = 1;
-		atchan->chan_common.chan_id = i;
 		list_add_tail(&atchan->chan_common.device_node,
 				&atdma->dma_common.channels);
 
@@ -1308,7 +1307,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "Atmel AHB DMA Controller ( %s%s), %d channels\n",
 	  dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask) ? "cpy " : "",
 	  dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)  ? "slave " : "",
-	  atdma->dma_common.chancnt);
+	  pdata->nr_channels);
 
 	dma_async_device_register(&atdma->dma_common);
 
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 4d180ca..9bfd6d3 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1407,12 +1407,11 @@ static int __init dw_probe(struct platform_device *pdev)
 	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
 
 	INIT_LIST_HEAD(&dw->dma.channels);
-	for (i = 0; i < pdata->nr_channels; i++, dw->dma.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct dw_dma_chan	*dwc = &dw->chan[i];
 
 		dwc->chan.device = &dw->dma;
 		dwc->chan.cookie = dwc->completed = 1;
-		dwc->chan.chan_id = i;
 		if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
 			list_add_tail(&dwc->chan.device_node,
 					&dw->dma.channels);
@@ -1468,7 +1467,7 @@ static int __init dw_probe(struct platform_device *pdev)
 	dma_writel(dw, CFG, DW_CFG_DMA_EN);
 
 	printk(KERN_INFO "%s: DesignWare DMA Controller, %d channels\n",
-			dev_name(&pdev->dev), dw->dma.chancnt);
+			dev_name(&pdev->dev), pdata->nr_channels);
 
 	dma_async_device_register(&dw->dma);
 
diff --git a/drivers/dma/intel_mid_dma.c b/drivers/dma/intel_mid_dma.c
index 8a3fdd8..cf74a66 100644
--- a/drivers/dma/intel_mid_dma.c
+++ b/drivers/dma/intel_mid_dma.c
@@ -1114,7 +1114,6 @@ static int mid_setup_dma(struct pci_dev *pdev)
 
 		midch->chan.device = &dma->common;
 		midch->chan.cookie =  1;
-		midch->chan.chan_id = i;
 		midch->ch_id = dma->chan_base + i;
 		pr_debug("MDMA:Init CH %d, ID %d\n", i, midch->ch_id);
 
@@ -1150,7 +1149,6 @@ static int mid_setup_dma(struct pci_dev *pdev)
 	dma_cap_set(DMA_SLAVE, dma->common.cap_mask);
 	dma_cap_set(DMA_PRIVATE, dma->common.cap_mask);
 	dma->common.dev = &pdev->dev;
-	dma->common.chancnt = dma->max_chan;
 
 	dma->common.device_alloc_chan_resources =
 					intel_mid_dma_alloc_chan_resources;
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index b9bae94..8ba4edc 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -741,7 +741,6 @@ static int __devinit mpc_dma_probe(struct platform_device *op)
 		mchan = &mdma->channels[i];
 
 		mchan->chan.device = dma;
-		mchan->chan.chan_id = i;
 		mchan->chan.cookie = 1;
 		mchan->completed_cookie = mchan->chan.cookie;
 
diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index 1ac8d4b..5b65362 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -926,7 +926,6 @@ static int __devinit pch_dma_probe(struct pci_dev *pdev,
 	}
 
 	pd->dma.dev = &pdev->dev;
-	pd->dma.chancnt = nr_channels;
 
 	INIT_LIST_HEAD(&pd->dma.channels);
 
@@ -935,7 +934,6 @@ static int __devinit pch_dma_probe(struct pci_dev *pdev,
 
 		pd_chan->chan.device = &pd->dma;
 		pd_chan->chan.cookie = 1;
-		pd_chan->chan.chan_id = i;
 
 		pd_chan->membase = &regs->desc[i];
 
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 00eee59..7f86e7d 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -747,11 +747,9 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		spin_lock_init(&pch->lock);
 		pch->pl330_chid = NULL;
 		pch->chan.device = pd;
-		pch->chan.chan_id = i;
 		pch->dmac = pdmac;
 
 		/* Add the channel to the DMAC list */
-		pd->chancnt++;
 		list_add_tail(&pch->chan.device_node, &pd->channels);
 	}
 
diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
index f69f90a..6dbdf45 100644
--- a/drivers/dma/timb_dma.c
+++ b/drivers/dma/timb_dma.c
@@ -753,7 +753,7 @@ static int __devinit td_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&td->dma.channels);
 
-	for (i = 0; i < pdata->nr_channels; i++, td->dma.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct timb_dma_chan *td_chan = &td->channels[i];
 		struct timb_dma_platform_data_channel *pchan =
 			pdata->channels + i;
@@ -767,7 +767,6 @@ static int __devinit td_probe(struct platform_device *pdev)
 
 		td_chan->chan.device = &td->dma;
 		td_chan->chan.cookie = 1;
-		td_chan->chan.chan_id = i;
 		spin_lock_init(&td_chan->lock);
 		INIT_LIST_HEAD(&td_chan->active_list);
 		INIT_LIST_HEAD(&td_chan->queue);
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

dma_async_device_register will re-init chan_id and chancnt,
so whatever chan_id and chancnt are set in drivers, they will
be re-written by dma_async_device_register.

Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Viresh Kumar <viresh.kumar@st.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
Cc: Yong Wang <yong.y.wang@intel.com>
Cc: Jaswinder Singh <jassi.brar@samsung.com>
Cc: Pelagicore AB <info@pelagicore.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/dma/at_hdmac.c      |    5 ++---
 drivers/dma/dw_dmac.c       |    5 ++---
 drivers/dma/intel_mid_dma.c |    2 --
 drivers/dma/mpc512x_dma.c   |    1 -
 drivers/dma/pch_dma.c       |    2 --
 drivers/dma/pl330.c         |    2 --
 drivers/dma/timb_dma.c      |    3 +--
 7 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 6a483ea..c1b8c15 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1260,12 +1260,11 @@ static int __init at_dma_probe(struct platform_device *pdev)
 
 	/* initialize channels related values */
 	INIT_LIST_HEAD(&atdma->dma_common.channels);
-	for (i = 0; i < pdata->nr_channels; i++, atdma->dma_common.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct at_dma_chan	*atchan = &atdma->chan[i];
 
 		atchan->chan_common.device = &atdma->dma_common;
 		atchan->chan_common.cookie = atchan->completed_cookie = 1;
-		atchan->chan_common.chan_id = i;
 		list_add_tail(&atchan->chan_common.device_node,
 				&atdma->dma_common.channels);
 
@@ -1308,7 +1307,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "Atmel AHB DMA Controller ( %s%s), %d channels\n",
 	  dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask) ? "cpy " : "",
 	  dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)  ? "slave " : "",
-	  atdma->dma_common.chancnt);
+	  pdata->nr_channels);
 
 	dma_async_device_register(&atdma->dma_common);
 
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 4d180ca..9bfd6d3 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1407,12 +1407,11 @@ static int __init dw_probe(struct platform_device *pdev)
 	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
 
 	INIT_LIST_HEAD(&dw->dma.channels);
-	for (i = 0; i < pdata->nr_channels; i++, dw->dma.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct dw_dma_chan	*dwc = &dw->chan[i];
 
 		dwc->chan.device = &dw->dma;
 		dwc->chan.cookie = dwc->completed = 1;
-		dwc->chan.chan_id = i;
 		if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
 			list_add_tail(&dwc->chan.device_node,
 					&dw->dma.channels);
@@ -1468,7 +1467,7 @@ static int __init dw_probe(struct platform_device *pdev)
 	dma_writel(dw, CFG, DW_CFG_DMA_EN);
 
 	printk(KERN_INFO "%s: DesignWare DMA Controller, %d channels\n",
-			dev_name(&pdev->dev), dw->dma.chancnt);
+			dev_name(&pdev->dev), pdata->nr_channels);
 
 	dma_async_device_register(&dw->dma);
 
diff --git a/drivers/dma/intel_mid_dma.c b/drivers/dma/intel_mid_dma.c
index 8a3fdd8..cf74a66 100644
--- a/drivers/dma/intel_mid_dma.c
+++ b/drivers/dma/intel_mid_dma.c
@@ -1114,7 +1114,6 @@ static int mid_setup_dma(struct pci_dev *pdev)
 
 		midch->chan.device = &dma->common;
 		midch->chan.cookie =  1;
-		midch->chan.chan_id = i;
 		midch->ch_id = dma->chan_base + i;
 		pr_debug("MDMA:Init CH %d, ID %d\n", i, midch->ch_id);
 
@@ -1150,7 +1149,6 @@ static int mid_setup_dma(struct pci_dev *pdev)
 	dma_cap_set(DMA_SLAVE, dma->common.cap_mask);
 	dma_cap_set(DMA_PRIVATE, dma->common.cap_mask);
 	dma->common.dev = &pdev->dev;
-	dma->common.chancnt = dma->max_chan;
 
 	dma->common.device_alloc_chan_resources =
 					intel_mid_dma_alloc_chan_resources;
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index b9bae94..8ba4edc 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -741,7 +741,6 @@ static int __devinit mpc_dma_probe(struct platform_device *op)
 		mchan = &mdma->channels[i];
 
 		mchan->chan.device = dma;
-		mchan->chan.chan_id = i;
 		mchan->chan.cookie = 1;
 		mchan->completed_cookie = mchan->chan.cookie;
 
diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index 1ac8d4b..5b65362 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -926,7 +926,6 @@ static int __devinit pch_dma_probe(struct pci_dev *pdev,
 	}
 
 	pd->dma.dev = &pdev->dev;
-	pd->dma.chancnt = nr_channels;
 
 	INIT_LIST_HEAD(&pd->dma.channels);
 
@@ -935,7 +934,6 @@ static int __devinit pch_dma_probe(struct pci_dev *pdev,
 
 		pd_chan->chan.device = &pd->dma;
 		pd_chan->chan.cookie = 1;
-		pd_chan->chan.chan_id = i;
 
 		pd_chan->membase = &regs->desc[i];
 
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 00eee59..7f86e7d 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -747,11 +747,9 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		spin_lock_init(&pch->lock);
 		pch->pl330_chid = NULL;
 		pch->chan.device = pd;
-		pch->chan.chan_id = i;
 		pch->dmac = pdmac;
 
 		/* Add the channel to the DMAC list */
-		pd->chancnt++;
 		list_add_tail(&pch->chan.device_node, &pd->channels);
 	}
 
diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
index f69f90a..6dbdf45 100644
--- a/drivers/dma/timb_dma.c
+++ b/drivers/dma/timb_dma.c
@@ -753,7 +753,7 @@ static int __devinit td_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&td->dma.channels);
 
-	for (i = 0; i < pdata->nr_channels; i++, td->dma.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct timb_dma_chan *td_chan = &td->channels[i];
 		struct timb_dma_platform_data_channel *pchan =
 			pdata->channels + i;
@@ -767,7 +767,6 @@ static int __devinit td_probe(struct platform_device *pdev)
 
 		td_chan->chan.device = &td->dma;
 		td_chan->chan.cookie = 1;
-		td_chan->chan.chan_id = i;
 		spin_lock_init(&td_chan->lock);
 		INIT_LIST_HEAD(&td_chan->active_list);
 		INIT_LIST_HEAD(&td_chan->queue);
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
> dma_async_device_register will re-init chan_id and chancnt,
> so whatever chan_id and chancnt are set in drivers, they will
> be re-written by dma_async_device_register.
>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Viresh Kumar <viresh.kumar@st.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>
> Cc: Yong Wang <yong.y.wang@intel.com>
> Cc: Jaswinder Singh <jassi.brar@samsung.com>
> Cc: Pelagicore AB <info@pelagicore.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
>  drivers/dma/at_hdmac.c      |    5 ++---
>  drivers/dma/dw_dmac.c       |    5 ++---
>  drivers/dma/intel_mid_dma.c |    2 --
>  drivers/dma/mpc512x_dma.c   |    1 -
>  drivers/dma/pch_dma.c       |    2 --
>  drivers/dma/pl330.c         |    2 --
>  drivers/dma/timb_dma.c      |    3 +--

Apparently ....

drivers/dma/ppc4xx/adma.c
drivers/dma/ipu/ipu_idmac.c
       still write to chan_id.

drivers/dma/amba-pl08x.c
drivers/dma/fsldma.c
drivers/dma/ioat/dma_v2.c
drivers/dma/ioat/dma.c
drivers/dma/mpc512x_dma.c
drivers/dma/shdma.c
       still write to chancnt

Most of them are simply a matter of removal, but some seem
like really depending upon setting them(?)

Anyways, even after you modify those as well, chan_id and chancnt
are rendered overstaying guests in dmaengine core. Because chan_id
of each channel would be _precisely_ the order in which the
_dmac-driver_ added the channel to the 'channels' list.

So if their values are _always_ gonna be just contiguously incrementing
why need variables for that in the dmaengine api?
Dmac drivers could use local variables for that.

OTOH, why not chan_id be left solely for use by dmac drivers read by the
dmaengine only to create sysfs entries ?

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
> dma_async_device_register will re-init chan_id and chancnt,
> so whatever chan_id and chancnt are set in drivers, they will
> be re-written by dma_async_device_register.
>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Viresh Kumar <viresh.kumar@st.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>
> Cc: Yong Wang <yong.y.wang@intel.com>
> Cc: Jaswinder Singh <jassi.brar@samsung.com>
> Cc: Pelagicore AB <info@pelagicore.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>
> ---
> ?drivers/dma/at_hdmac.c ? ? ?| ? ?5 ++---
> ?drivers/dma/dw_dmac.c ? ? ? | ? ?5 ++---
> ?drivers/dma/intel_mid_dma.c | ? ?2 --
> ?drivers/dma/mpc512x_dma.c ? | ? ?1 -
> ?drivers/dma/pch_dma.c ? ? ? | ? ?2 --
> ?drivers/dma/pl330.c ? ? ? ? | ? ?2 --
> ?drivers/dma/timb_dma.c ? ? ?| ? ?3 +--

Apparently ....

drivers/dma/ppc4xx/adma.c
drivers/dma/ipu/ipu_idmac.c
       still write to chan_id.

drivers/dma/amba-pl08x.c
drivers/dma/fsldma.c
drivers/dma/ioat/dma_v2.c
drivers/dma/ioat/dma.c
drivers/dma/mpc512x_dma.c
drivers/dma/shdma.c
       still write to chancnt

Most of them are simply a matter of removal, but some seem
like really depending upon setting them(?)

Anyways, even after you modify those as well, chan_id and chancnt
are rendered overstaying guests in dmaengine core. Because chan_id
of each channel would be _precisely_ the order in which the
_dmac-driver_ added the channel to the 'channels' list.

So if their values are _always_ gonna be just contiguously incrementing
why need variables for that in the dmaengine api?
Dmac drivers could use local variables for that.

OTOH, why not chan_id be left solely for use by dmac drivers read by the
dmaengine only to create sysfs entries ?

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/21 Jassi Brar <jassisinghbrar@gmail.com>:
> On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
>> dma_async_device_register will re-init chan_id and chancnt,
>> so whatever chan_id and chancnt are set in drivers, they will
>> be re-written by dma_async_device_register.
>>
>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Cc: Viresh Kumar <viresh.kumar@st.com>
>> Cc: Vinod Koul <vinod.koul@intel.com>
>> Cc: Piotr Ziecik <kosmo@semihalf.com>
>> Cc: Yong Wang <yong.y.wang@intel.com>
>> Cc: Jaswinder Singh <jassi.brar@samsung.com>
>> Cc: Pelagicore AB <info@pelagicore.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>>  drivers/dma/at_hdmac.c      |    5 ++---
>>  drivers/dma/dw_dmac.c       |    5 ++---
>>  drivers/dma/intel_mid_dma.c |    2 --
>>  drivers/dma/mpc512x_dma.c   |    1 -
>>  drivers/dma/pch_dma.c       |    2 --
>>  drivers/dma/pl330.c         |    2 --
>>  drivers/dma/timb_dma.c      |    3 +--
>
> Apparently ....
>
> drivers/dma/ppc4xx/adma.c
> drivers/dma/ipu/ipu_idmac.c
>       still write to chan_id.
>
> drivers/dma/amba-pl08x.c
> drivers/dma/fsldma.c
> drivers/dma/ioat/dma_v2.c
> drivers/dma/ioat/dma.c
> drivers/dma/mpc512x_dma.c
> drivers/dma/shdma.c
>       still write to chancnt
>
> Most of them are simply a matter of removal, but some seem
> like really depending upon setting them(?)
>
> Anyways, even after you modify those as well, chan_id and chancnt
> are rendered overstaying guests in dmaengine core. Because chan_id
> of each channel would be _precisely_ the order in which the
> _dmac-driver_ added the channel to the 'channels' list.
>
> So if their values are _always_ gonna be just contiguously incrementing
> why need variables for that in the dmaengine api?
> Dmac drivers could use local variables for that.
>
> OTOH, why not chan_id be left solely for use by dmac drivers read by the
> dmaengine only to create sysfs entries ?

actually i don't like the way dmaengine core handles chan_id. in my
opinion, every dmac only needs to tell dmaengine core the chan_base.
then dmaengine core set the id in a global scale.

for example, if you have two dmac in system:
dmac0 chan_base 0
dmac1 chan_base 16
then core will have chan 0~31 by chan_id from 0 to 31 but not
dmac0chan0~15, dmac1chan0~15.

client drivers can request a specific chan_id just like it can request a gpio.

now dmac drivers and dmaengine core have repeated chan_id and chancnt,
anyway, it is a problem that should be fixed at first. then i'd like
to have some common function to support the request of specific
chan_id.

>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

-barry

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/21 Jassi Brar <jassisinghbrar@gmail.com>:
> On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
>> dma_async_device_register will re-init chan_id and chancnt,
>> so whatever chan_id and chancnt are set in drivers, they will
>> be re-written by dma_async_device_register.
>>
>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>> Cc: Viresh Kumar <viresh.kumar@st.com>
>> Cc: Vinod Koul <vinod.koul@intel.com>
>> Cc: Piotr Ziecik <kosmo@semihalf.com>
>> Cc: Yong Wang <yong.y.wang@intel.com>
>> Cc: Jaswinder Singh <jassi.brar@samsung.com>
>> Cc: Pelagicore AB <info@pelagicore.com>
>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>> ---
>> ?drivers/dma/at_hdmac.c ? ? ?| ? ?5 ++---
>> ?drivers/dma/dw_dmac.c ? ? ? | ? ?5 ++---
>> ?drivers/dma/intel_mid_dma.c | ? ?2 --
>> ?drivers/dma/mpc512x_dma.c ? | ? ?1 -
>> ?drivers/dma/pch_dma.c ? ? ? | ? ?2 --
>> ?drivers/dma/pl330.c ? ? ? ? | ? ?2 --
>> ?drivers/dma/timb_dma.c ? ? ?| ? ?3 +--
>
> Apparently ....
>
> drivers/dma/ppc4xx/adma.c
> drivers/dma/ipu/ipu_idmac.c
> ? ? ? still write to chan_id.
>
> drivers/dma/amba-pl08x.c
> drivers/dma/fsldma.c
> drivers/dma/ioat/dma_v2.c
> drivers/dma/ioat/dma.c
> drivers/dma/mpc512x_dma.c
> drivers/dma/shdma.c
> ? ? ? still write to chancnt
>
> Most of them are simply a matter of removal, but some seem
> like really depending upon setting them(?)
>
> Anyways, even after you modify those as well, chan_id and chancnt
> are rendered overstaying guests in dmaengine core. Because chan_id
> of each channel would be _precisely_ the order in which the
> _dmac-driver_ added the channel to the 'channels' list.
>
> So if their values are _always_ gonna be just contiguously incrementing
> why need variables for that in the dmaengine api?
> Dmac drivers could use local variables for that.
>
> OTOH, why not chan_id be left solely for use by dmac drivers read by the
> dmaengine only to create sysfs entries ?

actually i don't like the way dmaengine core handles chan_id. in my
opinion, every dmac only needs to tell dmaengine core the chan_base.
then dmaengine core set the id in a global scale.

for example, if you have two dmac in system:
dmac0 chan_base 0
dmac1 chan_base 16
then core will have chan 0~31 by chan_id from 0 to 31 but not
dmac0chan0~15, dmac1chan0~15.

client drivers can request a specific chan_id just like it can request a gpio.

now dmac drivers and dmaengine core have repeated chan_id and chancnt,
anyway, it is a problem that should be fixed at first. then i'd like
to have some common function to support the request of specific
chan_id.

>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

-barry

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Thu, Sep 22, 2011 at 5:38 AM, Barry Song <21cnbao@gmail.com> wrote:
> 2011/9/21 Jassi Brar <jassisinghbrar@gmail.com>:
>> On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
>>> dma_async_device_register will re-init chan_id and chancnt,
>>> so whatever chan_id and chancnt are set in drivers, they will
>>> be re-written by dma_async_device_register.
>>>
>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> Cc: Viresh Kumar <viresh.kumar@st.com>
>>> Cc: Vinod Koul <vinod.koul@intel.com>
>>> Cc: Piotr Ziecik <kosmo@semihalf.com>
>>> Cc: Yong Wang <yong.y.wang@intel.com>
>>> Cc: Jaswinder Singh <jassi.brar@samsung.com>
>>> Cc: Pelagicore AB <info@pelagicore.com>
>>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>>> ---
>>>  drivers/dma/at_hdmac.c      |    5 ++---
>>>  drivers/dma/dw_dmac.c       |    5 ++---
>>>  drivers/dma/intel_mid_dma.c |    2 --
>>>  drivers/dma/mpc512x_dma.c   |    1 -
>>>  drivers/dma/pch_dma.c       |    2 --
>>>  drivers/dma/pl330.c         |    2 --
>>>  drivers/dma/timb_dma.c      |    3 +--
>>
>> Apparently ....
>>
>> drivers/dma/ppc4xx/adma.c
>> drivers/dma/ipu/ipu_idmac.c
>>       still write to chan_id.
>>
>> drivers/dma/amba-pl08x.c
>> drivers/dma/fsldma.c
>> drivers/dma/ioat/dma_v2.c
>> drivers/dma/ioat/dma.c
>> drivers/dma/mpc512x_dma.c
>> drivers/dma/shdma.c
>>       still write to chancnt
>>
My question was if you really intended to convert only some of
the drivers and not others?

>> Most of them are simply a matter of removal, but some seem
>> like really depending upon setting them(?)
>>
>> Anyways, even after you modify those as well, chan_id and chancnt
>> are rendered overstaying guests in dmaengine core. Because chan_id
>> of each channel would be _precisely_ the order in which the
>> _dmac-driver_ added the channel to the 'channels' list.
>>
>> So if their values are _always_ gonna be just contiguously incrementing
>> why need variables for that in the dmaengine api?
>> Dmac drivers could use local variables for that.
>>
>> OTOH, why not chan_id be left solely for use by dmac drivers read by the
>> dmaengine only to create sysfs entries ?
>
> actually i don't like the way dmaengine core handles chan_id. in my
> opinion, every dmac only needs to tell dmaengine core the chan_base.
> then dmaengine core set the id in a global scale.
>
> for example, if you have two dmac in system:
> dmac0 chan_base 0
> dmac1 chan_base 16
> then core will have chan 0~31 by chan_id from 0 to 31 but not
> dmac0chan0~15, dmac1chan0~15.
But note the numbering is still decided by the dmac driver - directly
or indirectly.
That is, if the dmac driver reorder registering of dmac or adding
channels to the 'channels' list, the assigned chan_id's would change.
So why not rather get rid of chan_id assignment from dmaengine
and do explicitly in dmac drivers ?
Which I already submitted a patch for https://lkml.org/lkml/2011/7/21/1

> client drivers can request a specific chan_id just like it can request a gpio.
No dear. That's not gonna fly if we are to have common client drivers
across SoCs.

> now dmac drivers and dmaengine core have repeated chan_id and chancnt,
> anyway, it is a problem that should be fixed at first.
Correct, chan_id assignment should be completely either in dmaengine
or in dmac drivers. But this patch does neither.

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Thu, Sep 22, 2011 at 5:38 AM, Barry Song <21cnbao@gmail.com> wrote:
> 2011/9/21 Jassi Brar <jassisinghbrar@gmail.com>:
>> On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
>>> dma_async_device_register will re-init chan_id and chancnt,
>>> so whatever chan_id and chancnt are set in drivers, they will
>>> be re-written by dma_async_device_register.
>>>
>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>> Cc: Viresh Kumar <viresh.kumar@st.com>
>>> Cc: Vinod Koul <vinod.koul@intel.com>
>>> Cc: Piotr Ziecik <kosmo@semihalf.com>
>>> Cc: Yong Wang <yong.y.wang@intel.com>
>>> Cc: Jaswinder Singh <jassi.brar@samsung.com>
>>> Cc: Pelagicore AB <info@pelagicore.com>
>>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>>> ---
>>> ?drivers/dma/at_hdmac.c ? ? ?| ? ?5 ++---
>>> ?drivers/dma/dw_dmac.c ? ? ? | ? ?5 ++---
>>> ?drivers/dma/intel_mid_dma.c | ? ?2 --
>>> ?drivers/dma/mpc512x_dma.c ? | ? ?1 -
>>> ?drivers/dma/pch_dma.c ? ? ? | ? ?2 --
>>> ?drivers/dma/pl330.c ? ? ? ? | ? ?2 --
>>> ?drivers/dma/timb_dma.c ? ? ?| ? ?3 +--
>>
>> Apparently ....
>>
>> drivers/dma/ppc4xx/adma.c
>> drivers/dma/ipu/ipu_idmac.c
>> ? ? ? still write to chan_id.
>>
>> drivers/dma/amba-pl08x.c
>> drivers/dma/fsldma.c
>> drivers/dma/ioat/dma_v2.c
>> drivers/dma/ioat/dma.c
>> drivers/dma/mpc512x_dma.c
>> drivers/dma/shdma.c
>> ? ? ? still write to chancnt
>>
My question was if you really intended to convert only some of
the drivers and not others?

>> Most of them are simply a matter of removal, but some seem
>> like really depending upon setting them(?)
>>
>> Anyways, even after you modify those as well, chan_id and chancnt
>> are rendered overstaying guests in dmaengine core. Because chan_id
>> of each channel would be _precisely_ the order in which the
>> _dmac-driver_ added the channel to the 'channels' list.
>>
>> So if their values are _always_ gonna be just contiguously incrementing
>> why need variables for that in the dmaengine api?
>> Dmac drivers could use local variables for that.
>>
>> OTOH, why not chan_id be left solely for use by dmac drivers read by the
>> dmaengine only to create sysfs entries ?
>
> actually i don't like the way dmaengine core handles chan_id. in my
> opinion, every dmac only needs to tell dmaengine core the chan_base.
> then dmaengine core set the id in a global scale.
>
> for example, if you have two dmac in system:
> dmac0 chan_base 0
> dmac1 chan_base 16
> then core will have chan 0~31 by chan_id from 0 to 31 but not
> dmac0chan0~15, dmac1chan0~15.
But note the numbering is still decided by the dmac driver - directly
or indirectly.
That is, if the dmac driver reorder registering of dmac or adding
channels to the 'channels' list, the assigned chan_id's would change.
So why not rather get rid of chan_id assignment from dmaengine
and do explicitly in dmac drivers ?
Which I already submitted a patch for https://lkml.org/lkml/2011/7/21/1

> client drivers can request a specific chan_id just like it can request a gpio.
No dear. That's not gonna fly if we are to have common client drivers
across SoCs.

> now dmac drivers and dmaengine core have repeated chan_id and chancnt,
> anyway, it is a problem that should be fixed at first.
Correct, chan_id assignment should be completely either in dmaengine
or in dmac drivers. But this patch does neither.

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
> On Thu, Sep 22, 2011 at 5:38 AM, Barry Song <21cnbao@gmail.com> wrote:
>> 2011/9/21 Jassi Brar <jassisinghbrar@gmail.com>:
>>> On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
>>>> dma_async_device_register will re-init chan_id and chancnt,
>>>> so whatever chan_id and chancnt are set in drivers, they will
>>>> be re-written by dma_async_device_register.
>>>>
>>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> Cc: Viresh Kumar <viresh.kumar@st.com>
>>>> Cc: Vinod Koul <vinod.koul@intel.com>
>>>> Cc: Piotr Ziecik <kosmo@semihalf.com>
>>>> Cc: Yong Wang <yong.y.wang@intel.com>
>>>> Cc: Jaswinder Singh <jassi.brar@samsung.com>
>>>> Cc: Pelagicore AB <info@pelagicore.com>
>>>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>>>> ---
>>>>  drivers/dma/at_hdmac.c      |    5 ++---
>>>>  drivers/dma/dw_dmac.c       |    5 ++---
>>>>  drivers/dma/intel_mid_dma.c |    2 --
>>>>  drivers/dma/mpc512x_dma.c   |    1 -
>>>>  drivers/dma/pch_dma.c       |    2 --
>>>>  drivers/dma/pl330.c         |    2 --
>>>>  drivers/dma/timb_dma.c      |    3 +--
>>>
>>> Apparently ....
>>>
>>> drivers/dma/ppc4xx/adma.c
>>> drivers/dma/ipu/ipu_idmac.c
>>>       still write to chan_id.
>>>
>>> drivers/dma/amba-pl08x.c
>>> drivers/dma/fsldma.c
>>> drivers/dma/ioat/dma_v2.c
>>> drivers/dma/ioat/dma.c
>>> drivers/dma/mpc512x_dma.c
>>> drivers/dma/shdma.c
>>>       still write to chancnt
>>>
> My question was if you really intended to convert only some of
> the drivers and not others?

i intended to convert all. but i lost some....

>
>>> Most of them are simply a matter of removal, but some seem
>>> like really depending upon setting them(?)
>>>
>>> Anyways, even after you modify those as well, chan_id and chancnt
>>> are rendered overstaying guests in dmaengine core. Because chan_id
>>> of each channel would be _precisely_ the order in which the
>>> _dmac-driver_ added the channel to the 'channels' list.
>>>
>>> So if their values are _always_ gonna be just contiguously incrementing
>>> why need variables for that in the dmaengine api?
>>> Dmac drivers could use local variables for that.
>>>
>>> OTOH, why not chan_id be left solely for use by dmac drivers read by the
>>> dmaengine only to create sysfs entries ?
>>
>> actually i don't like the way dmaengine core handles chan_id. in my
>> opinion, every dmac only needs to tell dmaengine core the chan_base.
>> then dmaengine core set the id in a global scale.
>>
>> for example, if you have two dmac in system:
>> dmac0 chan_base 0
>> dmac1 chan_base 16
>> then core will have chan 0~31 by chan_id from 0 to 31 but not
>> dmac0chan0~15, dmac1chan0~15.
> But note the numbering is still decided by the dmac driver - directly
> or indirectly.
> That is, if the dmac driver reorder registering of dmac or adding
> channels to the 'channels' list, the assigned chan_id's would change.
> So why not rather get rid of chan_id assignment from dmaengine
> and do explicitly in dmac drivers ?
> Which I already submitted a patch for https://lkml.org/lkml/2011/7/21/1

i don't agree chan_id is just a property of dmac. i think chan_id is
also a property core wants.
why could not dmac driver decide the chan_id in dmaengine core? i
think that is just what dmac driver wants.dmac just tells the base,
core re-number them one by one.
if dmac can't decide the ID directly or indirectly, this will be
random. how could client driver request the channel it wants?

>
>> client drivers can request a specific chan_id just like it can request a gpio.
> No dear. That's not gonna fly if we are to have common client drivers
> across SoCs.
>
>> now dmac drivers and dmaengine core have repeated chan_id and chancnt,
>> anyway, it is a problem that should be fixed at first.
> Correct, chan_id assignment should be completely either in dmaengine
> or in dmac drivers. But this patch does neither.
>
i like the way gpiolib handles ID more.

-barry

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
> On Thu, Sep 22, 2011 at 5:38 AM, Barry Song <21cnbao@gmail.com> wrote:
>> 2011/9/21 Jassi Brar <jassisinghbrar@gmail.com>:
>>> On Fri, Sep 16, 2011 at 3:13 PM, Barry Song <Baohua.Song@csr.com> wrote:
>>>> dma_async_device_register will re-init chan_id and chancnt,
>>>> so whatever chan_id and chancnt are set in drivers, they will
>>>> be re-written by dma_async_device_register.
>>>>
>>>> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>> Cc: Viresh Kumar <viresh.kumar@st.com>
>>>> Cc: Vinod Koul <vinod.koul@intel.com>
>>>> Cc: Piotr Ziecik <kosmo@semihalf.com>
>>>> Cc: Yong Wang <yong.y.wang@intel.com>
>>>> Cc: Jaswinder Singh <jassi.brar@samsung.com>
>>>> Cc: Pelagicore AB <info@pelagicore.com>
>>>> Signed-off-by: Barry Song <Baohua.Song@csr.com>
>>>> ---
>>>> ?drivers/dma/at_hdmac.c ? ? ?| ? ?5 ++---
>>>> ?drivers/dma/dw_dmac.c ? ? ? | ? ?5 ++---
>>>> ?drivers/dma/intel_mid_dma.c | ? ?2 --
>>>> ?drivers/dma/mpc512x_dma.c ? | ? ?1 -
>>>> ?drivers/dma/pch_dma.c ? ? ? | ? ?2 --
>>>> ?drivers/dma/pl330.c ? ? ? ? | ? ?2 --
>>>> ?drivers/dma/timb_dma.c ? ? ?| ? ?3 +--
>>>
>>> Apparently ....
>>>
>>> drivers/dma/ppc4xx/adma.c
>>> drivers/dma/ipu/ipu_idmac.c
>>> ? ? ? still write to chan_id.
>>>
>>> drivers/dma/amba-pl08x.c
>>> drivers/dma/fsldma.c
>>> drivers/dma/ioat/dma_v2.c
>>> drivers/dma/ioat/dma.c
>>> drivers/dma/mpc512x_dma.c
>>> drivers/dma/shdma.c
>>> ? ? ? still write to chancnt
>>>
> My question was if you really intended to convert only some of
> the drivers and not others?

i intended to convert all. but i lost some....

>
>>> Most of them are simply a matter of removal, but some seem
>>> like really depending upon setting them(?)
>>>
>>> Anyways, even after you modify those as well, chan_id and chancnt
>>> are rendered overstaying guests in dmaengine core. Because chan_id
>>> of each channel would be _precisely_ the order in which the
>>> _dmac-driver_ added the channel to the 'channels' list.
>>>
>>> So if their values are _always_ gonna be just contiguously incrementing
>>> why need variables for that in the dmaengine api?
>>> Dmac drivers could use local variables for that.
>>>
>>> OTOH, why not chan_id be left solely for use by dmac drivers read by the
>>> dmaengine only to create sysfs entries ?
>>
>> actually i don't like the way dmaengine core handles chan_id. in my
>> opinion, every dmac only needs to tell dmaengine core the chan_base.
>> then dmaengine core set the id in a global scale.
>>
>> for example, if you have two dmac in system:
>> dmac0 chan_base 0
>> dmac1 chan_base 16
>> then core will have chan 0~31 by chan_id from 0 to 31 but not
>> dmac0chan0~15, dmac1chan0~15.
> But note the numbering is still decided by the dmac driver - directly
> or indirectly.
> That is, if the dmac driver reorder registering of dmac or adding
> channels to the 'channels' list, the assigned chan_id's would change.
> So why not rather get rid of chan_id assignment from dmaengine
> and do explicitly in dmac drivers ?
> Which I already submitted a patch for https://lkml.org/lkml/2011/7/21/1

i don't agree chan_id is just a property of dmac. i think chan_id is
also a property core wants.
why could not dmac driver decide the chan_id in dmaengine core? i
think that is just what dmac driver wants.dmac just tells the base,
core re-number them one by one.
if dmac can't decide the ID directly or indirectly, this will be
random. how could client driver request the channel it wants?

>
>> client drivers can request a specific chan_id just like it can request a gpio.
> No dear. That's not gonna fly if we are to have common client drivers
> across SoCs.
>
>> now dmac drivers and dmaengine core have repeated chan_id and chancnt,
>> anyway, it is a problem that should be fixed at first.
> Correct, chan_id assignment should be completely either in dmaengine
> or in dmac drivers. But this patch does neither.
>
i like the way gpiolib handles ID more.

-barry

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
>> client drivers can request a specific chan_id just like it can request a gpio.
> No dear. That's not gonna fly if we are to have common client drivers
> across SoCs.
>
Btw, I meant you can't simply have clients specify a channel number and
get the channel in a portable manner. Clients need to specify the capabilities
they need of the channel.

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>
>> client drivers can request a specific chan_id just like it can request a gpio.
> No dear. That's not gonna fly if we are to have common client drivers
> across SoCs.
>
Btw, I meant you can't simply have clients specify a channel number and
get the channel in a portable manner. Clients need to specify the capabilities
they need of the channel.

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
> On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>
>>> client drivers can request a specific chan_id just like it can request a gpio.
>> No dear. That's not gonna fly if we are to have common client drivers
>> across SoCs.
>>
> Btw, I meant you can't simply have clients specify a channel number and
> get the channel in a portable manner. Clients need to specify the capabilities
> they need of the channel.

if the dmac is flexible in design, what you said is right. but the
problem is that there are some chips which actually fix one channel to
one peripheral.

-barry

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
> On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>
>>> client drivers can request a specific chan_id just like it can request a gpio.
>> No dear. That's not gonna fly if we are to have common client drivers
>> across SoCs.
>>
> Btw, I meant you can't simply have clients specify a channel number and
> get the channel in a portable manner. Clients need to specify the capabilities
> they need of the channel.

if the dmac is flexible in design, what you said is right. but the
problem is that there are some chips which actually fix one channel to
one peripheral.

-barry

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Thu, Sep 22, 2011 at 12:01 PM, Barry Song <21cnbao@gmail.com> wrote:
> 2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
>> On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>>
>>>> client drivers can request a specific chan_id just like it can request a gpio.
>>> No dear. That's not gonna fly if we are to have common client drivers
>>> across SoCs.
>>>
>> Btw, I meant you can't simply have clients specify a channel number and
>> get the channel in a portable manner. Clients need to specify the capabilities
>> they need of the channel.
>
> if the dmac is flexible in design, what you said is right. but the
> problem is that there are some chips which actually fix one channel to
> one peripheral.
Nevertheless it is better to assume the dmac's req-signals could be hacked
to re-route at even runtime.
Also it won't work for client drivers that run the same controller on different
SoC with a different dmac.

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Jassi Brar]

On Thu, Sep 22, 2011 at 12:01 PM, Barry Song <21cnbao@gmail.com> wrote:
> 2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
>> On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>>
>>>> client drivers can request a specific chan_id just like it can request a gpio.
>>> No dear. That's not gonna fly if we are to have common client drivers
>>> across SoCs.
>>>
>> Btw, I meant you can't simply have clients specify a channel number and
>> get the channel in a portable manner. Clients need to specify the capabilities
>> they need of the channel.
>
> if the dmac is flexible in design, what you said is right. but the
> problem is that there are some chips which actually fix one channel to
> one peripheral.
Nevertheless it is better to assume the dmac's req-signals could be hacked
to re-route at even runtime.
Also it won't work for client drivers that run the same controller on different
SoC with a different dmac.

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
> On Thu, Sep 22, 2011 at 12:01 PM, Barry Song <21cnbao@gmail.com> wrote:
>> 2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
>>> On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>>>
>>>>> client drivers can request a specific chan_id just like it can request a gpio.
>>>> No dear. That's not gonna fly if we are to have common client drivers
>>>> across SoCs.
>>>>
>>> Btw, I meant you can't simply have clients specify a channel number and
>>> get the channel in a portable manner. Clients need to specify the capabilities
>>> they need of the channel.
>>
>> if the dmac is flexible in design, what you said is right. but the
>> problem is that there are some chips which actually fix one channel to
>> one peripheral.
> Nevertheless it is better to assume the dmac's req-signals could be hacked
> to re-route at even runtime.
> Also it won't work for client drivers that run the same controller on different
> SoC with a different dmac.
>
you mean there is a mapping table in dmac driver. otherwise, there
will always require some changes in client drivers.
-barry

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
> On Thu, Sep 22, 2011 at 12:01 PM, Barry Song <21cnbao@gmail.com> wrote:
>> 2011/9/22 Jassi Brar <jassisinghbrar@gmail.com>:
>>> On Thu, Sep 22, 2011 at 11:11 AM, Jassi Brar <jassisinghbrar@gmail.com> wrote:
>>>>
>>>>> client drivers can request a specific chan_id just like it can request a gpio.
>>>> No dear. That's not gonna fly if we are to have common client drivers
>>>> across SoCs.
>>>>
>>> Btw, I meant you can't simply have clients specify a channel number and
>>> get the channel in a portable manner. Clients need to specify the capabilities
>>> they need of the channel.
>>
>> if the dmac is flexible in design, what you said is right. but the
>> problem is that there are some chips which actually fix one channel to
>> one peripheral.
> Nevertheless it is better to assume the dmac's req-signals could be hacked
> to re-route at even runtime.
> Also it won't work for client drivers that run the same controller on different
> SoC with a different dmac.
>
you mean there is a mapping table in dmac driver. otherwise, there
will always require some changes in client drivers.
-barry

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Russell King - ARM Linux]

On Thu, Sep 22, 2011 at 08:08:44AM +0800, Barry Song wrote:
> actually i don't like the way dmaengine core handles chan_id. in my
> opinion, every dmac only needs to tell dmaengine core the chan_base.
> then dmaengine core set the id in a global scale.

Think about where we're heading with DT - with the description of the
hardware setup.  We're no longer interested in a global number from
0 to N.  We're more interested in per-device numbering.

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Russell King - ARM Linux]

On Thu, Sep 22, 2011 at 08:08:44AM +0800, Barry Song wrote:
> actually i don't like the way dmaengine core handles chan_id. in my
> opinion, every dmac only needs to tell dmaengine core the chan_base.
> then dmaengine core set the id in a global scale.

Think about where we're heading with DT - with the description of the
hardware setup.  We're no longer interested in a global number from
0 to N.  We're more interested in per-device numbering.

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Vinod Koul]

On Thu, 2011-09-15 at 03:06 -0700, Barry Song wrote:
> dma_async_device_register will re-init chan_id and chancnt,
> so whatever chan_id and chancnt are set in drivers, they will
> be re-written by dma_async_device_register.
> 
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Viresh Kumar <viresh.kumar@st.com>
> Cc: Vinod Koul <vinod.koul@intel.com>
> Cc: Piotr Ziecik <kosmo@semihalf.com>
> Cc: Yong Wang <yong.y.wang@intel.com>
> Cc: Jaswinder Singh <jassi.brar@samsung.com>
> Cc: Pelagicore AB <info@pelagicore.com>
> Signed-off-by: Barry Song <Baohua.Song@csr.com>

Applied, Thanks


-- 
~Vinod

Re: [PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Viresh Kumar]

On 9/15/2011 3:36 PM, Barry Song wrote:
> dma_async_device_register will re-init chan_id and chancnt,
> so whatever chan_id and chancnt are set in drivers, they will
> be re-written by dma_async_device_register.
> 
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 4d180ca..9bfd6d3 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1407,12 +1407,11 @@ static int __init dw_probe(struct platform_device *pdev)
>  	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
>  
>  	INIT_LIST_HEAD(&dw->dma.channels);
> -	for (i = 0; i < pdata->nr_channels; i++, dw->dma.chancnt++) {
> +	for (i = 0; i < pdata->nr_channels; i++) {
>  		struct dw_dma_chan	*dwc = &dw->chan[i];
>  
>  		dwc->chan.device = &dw->dma;
>  		dwc->chan.cookie = dwc->completed = 1;
> -		dwc->chan.chan_id = i;
>  		if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
>  			list_add_tail(&dwc->chan.device_node,
>  					&dw->dma.channels);
> @@ -1468,7 +1467,7 @@ static int __init dw_probe(struct platform_device *pdev)
>  	dma_writel(dw, CFG, DW_CFG_DMA_EN);
>  
>  	printk(KERN_INFO "%s: DesignWare DMA Controller, %d channels\n",
> -			dev_name(&pdev->dev), dw->dma.chancnt);
> +			dev_name(&pdev->dev), pdata->nr_channels);
>  
>  	dma_async_device_register(&dw->dma);
>  

Acked-by: Viresh Kumar <viresh.kumar@st.com>

-- 
viresh

[PATCH] dmaengine: delete redundant chan_id and chancnt initialization in dma drivers

[Barry Song]

dma_async_device_register will re-init chan_id and chancnt,
so whatever chan_id and chancnt are set in drivers, they will
be re-written by dma_async_device_register.

Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Viresh Kumar <viresh.kumar@st.com>
Cc: Vinod Koul <vinod.koul@intel.com>
Cc: Piotr Ziecik <kosmo@semihalf.com>
Cc: Yong Wang <yong.y.wang@intel.com>
Cc: Jaswinder Singh <jassi.brar@samsung.com>
Cc: Pelagicore AB <info@pelagicore.com>
Signed-off-by: Barry Song <Baohua.Song@csr.com>
---
 drivers/dma/at_hdmac.c      |    5 ++---
 drivers/dma/dw_dmac.c       |    5 ++---
 drivers/dma/intel_mid_dma.c |    2 --
 drivers/dma/mpc512x_dma.c   |    1 -
 drivers/dma/pch_dma.c       |    2 --
 drivers/dma/pl330.c         |    2 --
 drivers/dma/timb_dma.c      |    3 +--
 7 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 6a483ea..c1b8c15 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -1260,12 +1260,11 @@ static int __init at_dma_probe(struct platform_device *pdev)
 
 	/* initialize channels related values */
 	INIT_LIST_HEAD(&atdma->dma_common.channels);
-	for (i = 0; i < pdata->nr_channels; i++, atdma->dma_common.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct at_dma_chan	*atchan = &atdma->chan[i];
 
 		atchan->chan_common.device = &atdma->dma_common;
 		atchan->chan_common.cookie = atchan->completed_cookie = 1;
-		atchan->chan_common.chan_id = i;
 		list_add_tail(&atchan->chan_common.device_node,
 				&atdma->dma_common.channels);
 
@@ -1308,7 +1307,7 @@ static int __init at_dma_probe(struct platform_device *pdev)
 	dev_info(&pdev->dev, "Atmel AHB DMA Controller ( %s%s), %d channels\n",
 	  dma_has_cap(DMA_MEMCPY, atdma->dma_common.cap_mask) ? "cpy " : "",
 	  dma_has_cap(DMA_SLAVE, atdma->dma_common.cap_mask)  ? "slave " : "",
-	  atdma->dma_common.chancnt);
+	  pdata->nr_channels);
 
 	dma_async_device_register(&atdma->dma_common);
 
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 4d180ca..9bfd6d3 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1407,12 +1407,11 @@ static int __init dw_probe(struct platform_device *pdev)
 	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
 
 	INIT_LIST_HEAD(&dw->dma.channels);
-	for (i = 0; i < pdata->nr_channels; i++, dw->dma.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct dw_dma_chan	*dwc = &dw->chan[i];
 
 		dwc->chan.device = &dw->dma;
 		dwc->chan.cookie = dwc->completed = 1;
-		dwc->chan.chan_id = i;
 		if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
 			list_add_tail(&dwc->chan.device_node,
 					&dw->dma.channels);
@@ -1468,7 +1467,7 @@ static int __init dw_probe(struct platform_device *pdev)
 	dma_writel(dw, CFG, DW_CFG_DMA_EN);
 
 	printk(KERN_INFO "%s: DesignWare DMA Controller, %d channels\n",
-			dev_name(&pdev->dev), dw->dma.chancnt);
+			dev_name(&pdev->dev), pdata->nr_channels);
 
 	dma_async_device_register(&dw->dma);
 
diff --git a/drivers/dma/intel_mid_dma.c b/drivers/dma/intel_mid_dma.c
index 8a3fdd8..cf74a66 100644
--- a/drivers/dma/intel_mid_dma.c
+++ b/drivers/dma/intel_mid_dma.c
@@ -1114,7 +1114,6 @@ static int mid_setup_dma(struct pci_dev *pdev)
 
 		midch->chan.device = &dma->common;
 		midch->chan.cookie =  1;
-		midch->chan.chan_id = i;
 		midch->ch_id = dma->chan_base + i;
 		pr_debug("MDMA:Init CH %d, ID %d\n", i, midch->ch_id);
 
@@ -1150,7 +1149,6 @@ static int mid_setup_dma(struct pci_dev *pdev)
 	dma_cap_set(DMA_SLAVE, dma->common.cap_mask);
 	dma_cap_set(DMA_PRIVATE, dma->common.cap_mask);
 	dma->common.dev = &pdev->dev;
-	dma->common.chancnt = dma->max_chan;
 
 	dma->common.device_alloc_chan_resources =
 					intel_mid_dma_alloc_chan_resources;
diff --git a/drivers/dma/mpc512x_dma.c b/drivers/dma/mpc512x_dma.c
index b9bae94..8ba4edc 100644
--- a/drivers/dma/mpc512x_dma.c
+++ b/drivers/dma/mpc512x_dma.c
@@ -741,7 +741,6 @@ static int __devinit mpc_dma_probe(struct platform_device *op)
 		mchan = &mdma->channels[i];
 
 		mchan->chan.device = dma;
-		mchan->chan.chan_id = i;
 		mchan->chan.cookie = 1;
 		mchan->completed_cookie = mchan->chan.cookie;
 
diff --git a/drivers/dma/pch_dma.c b/drivers/dma/pch_dma.c
index 1ac8d4b..5b65362 100644
--- a/drivers/dma/pch_dma.c
+++ b/drivers/dma/pch_dma.c
@@ -926,7 +926,6 @@ static int __devinit pch_dma_probe(struct pci_dev *pdev,
 	}
 
 	pd->dma.dev = &pdev->dev;
-	pd->dma.chancnt = nr_channels;
 
 	INIT_LIST_HEAD(&pd->dma.channels);
 
@@ -935,7 +934,6 @@ static int __devinit pch_dma_probe(struct pci_dev *pdev,
 
 		pd_chan->chan.device = &pd->dma;
 		pd_chan->chan.cookie = 1;
-		pd_chan->chan.chan_id = i;
 
 		pd_chan->membase = &regs->desc[i];
 
diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
index 00eee59..7f86e7d 100644
--- a/drivers/dma/pl330.c
+++ b/drivers/dma/pl330.c
@@ -747,11 +747,9 @@ pl330_probe(struct amba_device *adev, const struct amba_id *id)
 		spin_lock_init(&pch->lock);
 		pch->pl330_chid = NULL;
 		pch->chan.device = pd;
-		pch->chan.chan_id = i;
 		pch->dmac = pdmac;
 
 		/* Add the channel to the DMAC list */
-		pd->chancnt++;
 		list_add_tail(&pch->chan.device_node, &pd->channels);
 	}
 
diff --git a/drivers/dma/timb_dma.c b/drivers/dma/timb_dma.c
index f69f90a..6dbdf45 100644
--- a/drivers/dma/timb_dma.c
+++ b/drivers/dma/timb_dma.c
@@ -753,7 +753,7 @@ static int __devinit td_probe(struct platform_device *pdev)
 
 	INIT_LIST_HEAD(&td->dma.channels);
 
-	for (i = 0; i < pdata->nr_channels; i++, td->dma.chancnt++) {
+	for (i = 0; i < pdata->nr_channels; i++) {
 		struct timb_dma_chan *td_chan = &td->channels[i];
 		struct timb_dma_platform_data_channel *pchan =
 			pdata->channels + i;
@@ -767,7 +767,6 @@ static int __devinit td_probe(struct platform_device *pdev)
 
 		td_chan->chan.device = &td->dma;
 		td_chan->chan.cookie = 1;
-		td_chan->chan.chan_id = i;
 		spin_lock_init(&td_chan->lock);
 		INIT_LIST_HEAD(&td_chan->active_list);
 		INIT_LIST_HEAD(&td_chan->queue);
-- 
1.7.1



Member of the CSR plc group of companies. CSR plc registered in England and Wales, registered number 4187346, registered office Churchill House, Cambridge Business Park, Cowley Road, Cambridge, CB4 0WZ, United Kingdom
More information can be found at www.csr.com. Follow CSR on Twitter at http://twitter.com/CSR_PLC and read our blog at www.csr.com/blog