[PATCH V1 Resend 0/3] dmaengine: Allow controller drivers to set channel ids

[PATCH V1 Resend 0/3] dmaengine: Allow controller drivers to set channel ids

[Viresh Kumar]

Hi Dan,

Probably you missed this patchset earlier, so resending it again.

Currently value of chan_id field is updated by dmaengine.c while the dma device
is registered. In some cases the controller driver may need to assign channel
numbers by itself. This patchset introduces changes to do it.

Viresh Kumar (3):
  dmaengine: Allow controller drivers to set channel ids
  dmaengine/dw_dmac: Set channel id's in controller driver
  dmaengine/dw_dmac: Don't use magic number for total number of
    channels

 drivers/dma/dmaengine.c   |    7 ++++++-
 drivers/dma/dw_dmac.c     |    4 +++-
 include/linux/dmaengine.h |    5 +++++
 3 files changed, 14 insertions(+), 2 deletions(-)

-- 
1.7.8.110.g4cb5d

[PATCH V1 Resend 1/3] dmaengine: Allow controller drivers to set channel ids

[Viresh Kumar]

Currently value of chan_id field is updated by dmaengine.c while the dma device
is registered. In some cases the controller driver may need to assign channel
numbers by itself. For example, dw_dmac.c wants to control the order in which
channels are allocated by dmaengine. So it added channels inside channel list of
dma_device in reverse order. Now channel 7 is allocated first and channel 0 is
allocated last.

But as the channel ids are updated by dmaengine, channel numbers in sysfs will
be overwritten if controller has added channels in reverse order. To represent
correct channel number in sysfs, it is required that dmaengine must not assume
that first channel in list is channel 0 and last is 7.

To handled this situation, chan_ids_set is passed by controller driver inside
struct dma_device. Controller drivers that need to set channel numbers
themselves must pass this field as true.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/dma/dmaengine.c   |    7 ++++++-
 include/linux/dmaengine.h |    5 +++++
 2 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/dmaengine.c b/drivers/dma/dmaengine.c
index a6c6051..38feff3 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -736,7 +736,12 @@ int dma_async_device_register(struct dma_device *device)
 			goto err_out;
 		}
 
-		chan->chan_id = chancnt++;
+		/* channel ids are set by controller drivers, don't update it */
+		if (!device->chan_ids_set)
+			chan->chan_id = chancnt;
+
+		chancnt++;
+
 		chan->dev->device.class = &dma_devclass;
 		chan->dev->device.parent = device->dev;
 		chan->dev->chan = chan;
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 5532bb8..56f0dd2 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -497,6 +497,8 @@ struct dma_tx_state {
  * @xor_align: alignment shift for xor operations
  * @pq_align: alignment shift for pq operations
  * @fill_align: alignment shift for memset operations
+ * @chan_ids_set: channel id's are already set by driver and must not be
+ * reinitialized by dmaengine.
  * @dev_id: unique device ID
  * @dev: struct device reference for dma mapping api
  * @device_alloc_chan_resources: allocate resources and return the
@@ -537,6 +539,9 @@ struct dma_device {
 	u8 fill_align;
 	#define DMA_HAS_PQ_CONTINUE (1 << 15)
 
+	/* sysfs */
+	bool chan_ids_set;
+
 	int dev_id;
 	struct device *dev;
 
-- 
1.7.8.110.g4cb5d

[PATCH V1 Resend 2/3] dmaengine/dw_dmac: Set channel id's in controller driver

[Viresh Kumar]

dw_dmac may conditionally add channels in reverse order inside channel list in
struct dma_device. So, we must set chan_id's here and pass this to dmaengine, so
that it doesn't override these values.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/dma/dw_dmac.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 9b592b0..35c8829 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1417,6 +1417,7 @@ static int __init dw_probe(struct platform_device *pdev)
 	tasklet_init(&dw->tasklet, dw_dma_tasklet, (unsigned long)dw);
 
 	dw->all_chan_mask = (1 << pdata->nr_channels) - 1;
+	dw->dma.chan_ids_set = true;
 
 	INIT_LIST_HEAD(&dw->dma.channels);
 	for (i = 0; i < pdata->nr_channels; i++) {
@@ -1424,6 +1425,7 @@ static int __init dw_probe(struct platform_device *pdev)
 
 		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);
-- 
1.7.8.110.g4cb5d

[PATCH V1 Resend 3/3] dmaengine/dw_dmac: Don't use magic number for total number of channels

[Viresh Kumar]

Total number of channels is passed in pdata->nr_channels variable, thus we must
not use magic number '7' for total number of channels.

Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
---
 drivers/dma/dw_dmac.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 35c8829..96b2750 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1434,7 +1434,7 @@ static int __init dw_probe(struct platform_device *pdev)
 
 		/* 7 is highest priority & 0 is lowest. */
 		if (pdata->chan_priority == CHAN_PRIORITY_ASCENDING)
-			dwc->priority = 7 - i;
+			dwc->priority = pdata->nr_channels - i - 1;
 		else
 			dwc->priority = i;
 
-- 
1.7.8.110.g4cb5d

Re: [PATCH V1 Resend 0/3] dmaengine: Allow controller drivers to set channel ids

[Williams, Dan J]

On Sun, Dec 18, 2011 at 8:30 PM, Viresh Kumar <viresh.kumar@st.com> wrote:
> Hi Dan,
>
> Probably you missed this patchset earlier, so resending it again.
>
> Currently value of chan_id field is updated by dmaengine.c while the dma device
> is registered. In some cases the controller driver may need to assign channel
> numbers by itself. This patchset introduces changes to do it.

Didn't we already go through this discussion once?

Here is the old thread: http://marc.info/?t=131122111000001&r=2&w=2

The chan_id is really only a sysfs detail, and for the same reasons
that scsi_host adapters don't pick their own host number neither
should dma channels.

The driver can set a channel id in its private channel struct right?

--
Dan