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

[PATCH 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 0e5f684..1d09258 100644
--- a/drivers/dma/dmaengine.c
+++ b/drivers/dma/dmaengine.c
@@ -735,7 +735,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 19cbe39..b4607f4 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -417,6 +417,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
@@ -456,6 +458,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.2.2

[PATCH 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 3550042..e2c8e4e 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1397,6 +1397,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++) {
@@ -1404,6 +1405,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.2.2

[PATCH 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 e2c8e4e..4010044 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1414,7 +1414,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.2.2

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

[Vinod Koul]

On Thu, 2011-09-22 at 16:13 +0530, Viresh Kumar wrote:
> 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 dmaengine, this is just a channel representation in sysfs and nothing
more. If damc is assuming that this is same as what dmaengine will do
then dmac is wrong.

Nevertheless, Dan was okay with changing this representation.
Dan I would need your comments/Ack before we discuss this further
> 
> 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 0e5f684..1d09258 100644
> --- a/drivers/dma/dmaengine.c
> +++ b/drivers/dma/dmaengine.c
> @@ -735,7 +735,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 19cbe39..b4607f4 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -417,6 +417,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
> @@ -456,6 +458,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;
>  


-- 
~Vinod

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

[Viresh Kumar]

On 9/28/2011 2:38 PM, Vinod Koul wrote:
> to dmaengine, this is just a channel representation in sysfs and nothing
> more. If damc is assuming that this is same as what dmaengine will do
> then dmac is wrong.
> 
> Nevertheless, Dan was okay with changing this representation.
> Dan I would need your comments/Ack before we discuss this further

Hi Vinod/Dan,

Any update/feedback on this?

-- 
viresh

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

[Vinod Koul]

On Thu, 2011-10-13 at 14:16 +0530, Viresh Kumar wrote:
> On 9/28/2011 2:38 PM, Vinod Koul wrote:
> > to dmaengine, this is just a channel representation in sysfs and nothing
> > more. If damc is assuming that this is same as what dmaengine will do
> > then dmac is wrong.
> > 
> > Nevertheless, Dan was okay with changing this representation.
> > Dan I would need your comments/Ack before we discuss this further
> 
> Hi Vinod/Dan,
> 
> Any update/feedback on this?
Since this touches non slave stuff as well, I need Dan to ack it first

-- 
~Vinod

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

[Viresh Kumar]

On 10/14/2011 11:13 AM, Vinod Koul wrote:
>> > Hi Vinod/Dan,
>> > 
>> > Any update/feedback on this?
> Since this touches non slave stuff as well, I need Dan to ack it first

Hi Dan,

Can you please provide some comments on this.

-- 
viresh

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

[Viresh Kumar]

On 10/31/2011 3:52 PM, Viresh Kumar wrote:
> On 10/14/2011 11:13 AM, Vinod Koul wrote:
>>>> >> > Hi Vinod/Dan,
>>>> >> > 
>>>> >> > Any update/feedback on this?
>> > Since this touches non slave stuff as well, I need Dan to ack it first
> Hi Dan,
> 
> Can you please provide some comments on this.

Vinod,

It looks Dan has missed this patchset somehow. :(
Can you please get his views about this patchset, if possible.

-- 
viresh

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

[Viresh Kumar]

On 9/28/2011 2:38 PM, Vinod Koul wrote:
> On Thu, 2011-09-22 at 16:13 +0530, Viresh Kumar wrote:
>> > 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 dmaengine, this is just a channel representation in sysfs and nothing
> more. If damc is assuming that this is same as what dmaengine will do
> then dmac is wrong.
> 
> Nevertheless, Dan was okay with changing this representation.
> Dan I would need your comments/Ack before we discuss this further

Dan/Vinod,

Any feedback on this patchset?

-- 
viresh