Following set of patches extends dw_dmac support and fixes minor bugs.
This patch series has earlier been posted to [email protected].
Didn't get any review there, so including [email protected].
Viresh Kumar (8):
dw_dmac: Remove compilation dependency from AVR32
dw_dmac: Move single descriptor from dwc->queue to dwc->active_list
in dwc_complete_all
dw_dmac: call dwc_scan_descriptor from dwc_issue_pending only if
active list is empty
dw_dmac: calling dwc_scan_descriptors from dwc_tx_status() after
taking lock
dw_dmac: adding support for 64 bit access width for memcpy xfers
dw_dmac: Mark all tx_descriptors with DMA_CRTL_ACK after xfer finish
dw_dmac.c: Pass Channel Allocation Order from platform_data
dw_dmac.c: Pass Channel Priority from platform_data
drivers/dma/Kconfig | 1 -
drivers/dma/dw_dmac.c | 52 ++++++++++++++++++++++++++++++-------------
drivers/dma/dw_dmac_regs.h | 3 ++
include/linux/dw_dmac.h | 7 +++++-
4 files changed, 45 insertions(+), 18 deletions(-)
--
1.7.2.2
This will be used in SPEAr, ARM family.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/Kconfig | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 1c28816..95c7db7 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -82,7 +82,6 @@ config INTEL_IOP_ADMA
config DW_DMAC
tristate "Synopsys DesignWare AHB DMA support"
- depends on AVR32
select DMA_ENGINE
default y if CPU_AT32AP7000
help
--
1.7.2.2
dwc_complete_all and other routines were removing all descriptors from dwc->queue
and pushing them to dwc->active_list. Only one was required to be removed. Also
we are calling dwc_dostart, once list is fixed.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 20 ++++++++------------
1 files changed, 8 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index a3991ab..6dd03b0 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -84,11 +84,6 @@ static struct dw_desc *dwc_first_active(struct dw_dma_chan *dwc)
return list_entry(dwc->active_list.next, struct dw_desc, desc_node);
}
-static struct dw_desc *dwc_first_queued(struct dw_dma_chan *dwc)
-{
- return list_entry(dwc->queue.next, struct dw_desc, desc_node);
-}
-
static struct dw_desc *dwc_desc_get(struct dw_dma_chan *dwc)
{
struct dw_desc *desc, *_desc;
@@ -259,10 +254,11 @@ static void dwc_complete_all(struct dw_dma *dw, struct dw_dma_chan *dwc)
* Submit queued descriptors ASAP, i.e. before we go through
* the completed ones.
*/
- if (!list_empty(&dwc->queue))
- dwc_dostart(dwc, dwc_first_queued(dwc));
list_splice_init(&dwc->active_list, &list);
- list_splice_init(&dwc->queue, &dwc->active_list);
+ if (!list_empty(&dwc->queue)) {
+ list_move(dwc->queue.next, &dwc->active_list);
+ dwc_dostart(dwc, dwc_first_active(dwc));
+ }
list_for_each_entry_safe(desc, _desc, &list, desc_node)
dwc_descriptor_complete(dwc, desc);
@@ -319,8 +315,8 @@ static void dwc_scan_descriptors(struct dw_dma *dw, struct dw_dma_chan *dwc)
cpu_relax();
if (!list_empty(&dwc->queue)) {
- dwc_dostart(dwc, dwc_first_queued(dwc));
- list_splice_init(&dwc->queue, &dwc->active_list);
+ list_move(dwc->queue.next, &dwc->active_list);
+ dwc_dostart(dwc, dwc_first_active(dwc));
}
}
@@ -346,7 +342,7 @@ static void dwc_handle_error(struct dw_dma *dw, struct dw_dma_chan *dwc)
*/
bad_desc = dwc_first_active(dwc);
list_del_init(&bad_desc->desc_node);
- list_splice_init(&dwc->queue, dwc->active_list.prev);
+ list_move(dwc->queue.next, dwc->active_list.prev);
/* Clear the error flag and try to restart the controller */
dma_writel(dw, CLEAR.ERROR, dwc->mask);
@@ -541,8 +537,8 @@ static dma_cookie_t dwc_tx_submit(struct dma_async_tx_descriptor *tx)
if (list_empty(&dwc->active_list)) {
dev_vdbg(chan2dev(tx->chan), "tx_submit: started %u\n",
desc->txd.cookie);
- dwc_dostart(dwc, desc);
list_add_tail(&desc->desc_node, &dwc->active_list);
+ dwc_dostart(dwc, dwc_first_active(dwc));
} else {
dev_vdbg(chan2dev(tx->chan), "tx_submit: queued %u\n",
desc->txd.cookie);
--
1.7.2.2
dwc_scan_descriptor was called even when there were descriptors in active list.
Checking if active list is empty or not.
Signed-off-by: Viresh Kumar <[email protected]>
---
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 6dd03b0..3bf4772 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -848,7 +848,7 @@ static void dwc_issue_pending(struct dma_chan *chan)
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
spin_lock_bh(&dwc->lock);
- if (!list_empty(&dwc->queue))
+ if (!list_empty(&dwc->queue) && list_empty(&dwc->active_list))
dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
spin_unlock_bh(&dwc->lock);
}
--
1.7.2.2
Signed-off-by: Viresh Kumar <[email protected]>
---
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 3bf4772..5cc5abf 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -830,7 +830,9 @@ dwc_tx_status(struct dma_chan *chan,
ret = dma_async_is_complete(cookie, last_complete, last_used);
if (ret != DMA_SUCCESS) {
+ spin_lock_bh(&dwc->lock);
dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
+ spin_unlock_bh(&dwc->lock);
last_complete = dwc->completed;
last_used = chan->cookie;
--
1.7.2.2
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 5cc5abf..c40b89f 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -577,7 +577,9 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
* We can be a lot more clever here, but this should take care
* of the most common optimization.
*/
- if (!((src | dest | len) & 3))
+ if (!((src | dest | len) & 7))
+ src_width = dst_width = 3;
+ else if (!((src | dest | len) & 3))
src_width = dst_width = 2;
else if (!((src | dest | len) & 1))
src_width = dst_width = 1;
--
1.7.2.2
dwc_desc_get checks all descriptors for DMA_CTRL_ACK before allocating them for
transfers. And descriptors are not marked with DMA_CRTL_ACK after transfer
finishes. Thus descriptor once used is not usable again. This patch marks
descriptors with DMA_CRTL_ACK after dma xfer finishes
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index c40b89f..01f783d 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -196,6 +196,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
dma_async_tx_callback callback;
void *param;
struct dma_async_tx_descriptor *txd = &desc->txd;
+ struct dw_desc *child;
dev_vdbg(chan2dev(&dwc->chan), "descriptor %u complete\n", txd->cookie);
@@ -204,6 +205,12 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
param = txd->callback_param;
dwc_sync_desc_for_cpu(dwc, desc);
+
+ /* async_tx_ack */
+ list_for_each_entry(child, &desc->tx_list, desc_node)
+ async_tx_ack(&child->txd);
+ async_tx_ack(&desc->txd);
+
list_splice_init(&desc->tx_list, &dwc->free_list);
list_move(&desc->desc_node, &dwc->free_list);
--
1.7.2.2
In SPEAr Platform channels 4-7 have more Fifo depth. So we must get better
channel first. This patch introduces concept of channel allocation order in
dw_dmac. If user doesn't paas anything or 0, than normal (ascending) channel
allocation will follow, else channels will be allocated in descending order.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 6 +++++-
include/linux/dw_dmac.h | 3 +++
2 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 01f783d..37ffd2c 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1314,7 +1314,11 @@ 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;
- list_add_tail(&dwc->chan.device_node, &dw->dma.channels);
+ if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
+ list_add_tail(&dwc->chan.device_node,
+ &dw->dma.channels);
+ else
+ list_add(&dwc->chan.device_node, &dw->dma.channels);
dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
spin_lock_init(&dwc->lock);
diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
index c8aad71..057e883 100644
--- a/include/linux/dw_dmac.h
+++ b/include/linux/dw_dmac.h
@@ -19,6 +19,9 @@
*/
struct dw_dma_platform_data {
unsigned int nr_channels;
+#define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */
+#define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero */
+ unsigned int chan_allocation_order;
};
/**
--
1.7.2.2
In Synopsys designware, channel priority is programmable. This patch adds
support for passing channel priority through platform data. By default Ascending
channel priority will be followed, i.e. channel 0 will get highest priority and
channel 7 will get lowest.
Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/dma/dw_dmac.c | 11 ++++++++++-
drivers/dma/dw_dmac_regs.h | 3 +++
include/linux/dw_dmac.h | 4 +++-
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 37ffd2c..edb3d3b 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -896,8 +896,11 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
BUG_ON(!dws->dma_dev || dws->dma_dev != dw->dma.dev);
cfghi = dws->cfg_hi;
- cfglo = dws->cfg_lo;
+ cfglo = dws->cfg_lo & ~DWC_CFGL_CH_PRIOR_MASK;
}
+
+ cfglo |= DWC_CFGL_CH_PRIOR(dwc->priority);
+
channel_writel(dwc, CFG_LO, cfglo);
channel_writel(dwc, CFG_HI, cfghi);
@@ -1320,6 +1323,12 @@ static int __init dw_probe(struct platform_device *pdev)
else
list_add(&dwc->chan.device_node, &dw->dma.channels);
+ /* 7 is highest priority & 0 is lowest. */
+ if (pdata->chan_priority == CHAN_PRIORITY_ASCENDING)
+ dwc->priority = 7 - i;
+ else
+ dwc->priority = i;
+
dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
spin_lock_init(&dwc->lock);
dwc->mask = 1 << i;
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index d9a939f..6a8e6d3 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -101,6 +101,8 @@ struct dw_dma_regs {
#define DWC_CTLH_BLOCK_TS_MASK 0x00000fff
/* Bitfields in CFG_LO. Platform-configurable bits are in <linux/dw_dmac.h> */
+#define DWC_CFGL_CH_PRIOR_MASK (0x7 << 5) /* priority mask */
+#define DWC_CFGL_CH_PRIOR(x) ((x) << 5) /* priority */
#define DWC_CFGL_CH_SUSP (1 << 8) /* pause xfer */
#define DWC_CFGL_FIFO_EMPTY (1 << 9) /* pause xfer */
#define DWC_CFGL_HS_DST (1 << 10) /* handshake w/dst */
@@ -134,6 +136,7 @@ struct dw_dma_chan {
struct dma_chan chan;
void __iomem *ch_regs;
u8 mask;
+ u8 priority;
spinlock_t lock;
diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
index 057e883..53072c8 100644
--- a/include/linux/dw_dmac.h
+++ b/include/linux/dw_dmac.h
@@ -22,6 +22,9 @@ struct dw_dma_platform_data {
#define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */
#define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero */
unsigned int chan_allocation_order;
+#define CHAN_PRIORITY_ASCENDING 0 /* chan0 highest */
+#define CHAN_PRIORITY_DESCENDING 1 /* chan7 highest */
+ unsigned int chan_priority;
};
/**
@@ -65,7 +68,6 @@ struct dw_dma_slave {
#define DWC_CFGH_DST_PER(x) ((x) << 11)
/* Platform-configurable bits in CFG_LO */
-#define DWC_CFGL_PRIO(x) ((x) << 5) /* priority */
#define DWC_CFGL_LOCK_CH_XFER (0 << 12) /* scope of LOCK_CH */
#define DWC_CFGL_LOCK_CH_BLOCK (1 << 12)
#define DWC_CFGL_LOCK_CH_XACT (2 << 12)
--
1.7.2.2
On Mon, Feb 28, 2011 at 04:11:18PM +0530, Viresh Kumar wrote:
> dwc_scan_descriptor was called even when there were descriptors in active list.
> Checking if active list is empty or not.
>
> Signed-off-by: Viresh Kumar <[email protected]>
Hi Viresh,
I have a similar patch to this that is already in next that should
achieve the same thing (dmaengine/dw_dmac: don't scan descriptors if no
xfers in progress).
Jamie
On 02/28/2011 04:16 PM, Jamie Iles wrote:
> On Mon, Feb 28, 2011 at 04:11:18PM +0530, Viresh Kumar wrote:
>> > dwc_scan_descriptor was called even when there were descriptors in active list.
>> > Checking if active list is empty or not.
>> >
>> > Signed-off-by: Viresh Kumar <[email protected]>
> Hi Viresh,
>
> I have a similar patch to this that is already in next that should
> achieve the same thing (dmaengine/dw_dmac: don't scan descriptors if no
> xfers in progress).
Jamie,
Sorry for missing that!!
This patch can be dropped.
--
viresh
Hello.
On 28-02-2011 13:41, Viresh Kumar wrote:
> dwc_desc_get checks all descriptors for DMA_CTRL_ACK before allocating them for
> transfers. And descriptors are not marked with DMA_CRTL_ACK after transfer
> finishes. Thus descriptor once used is not usable again. This patch marks
> descriptors with DMA_CRTL_ACK after dma xfer finishes
> Signed-off-by: Viresh Kumar<[email protected]>
> ---
> drivers/dma/dw_dmac.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index c40b89f..01f783d 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -196,6 +196,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
> dma_async_tx_callback callback;
> void *param;
> struct dma_async_tx_descriptor *txd =&desc->txd;
> + struct dw_desc *child;
Shouldn't this varaible name be aligned with the above variable names?
WBR, Sergei
On 2/28/11, Sergei Shtylyov <[email protected]> wrote:
> Hello.
>
> On 28-02-2011 13:41, Viresh Kumar wrote:
>
>> dwc_desc_get checks all descriptors for DMA_CTRL_ACK before allocating
>> them for
>> transfers. And descriptors are not marked with DMA_CRTL_ACK after transfer
>> finishes. Thus descriptor once used is not usable again. This patch marks
>> descriptors with DMA_CRTL_ACK after dma xfer finishes
>
>> Signed-off-by: Viresh Kumar<[email protected]>
>> ---
>> drivers/dma/dw_dmac.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index c40b89f..01f783d 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -196,6 +196,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc,
>> struct dw_desc *desc)
>> dma_async_tx_callback callback;
>> void *param;
>> struct dma_async_tx_descriptor *txd =&desc->txd;
>> + struct dw_desc *child;
>
> Shouldn't this varaible name be aligned with the above variable names?
>
Should be. Will take care of this while resending this patch series,
after getting all
review comments.
--
viresh
On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
> Following set of patches extends dw_dmac support and fixes minor bugs.
>
> This patch series has earlier been posted to [email protected].
> Didn't get any review there, so including [email protected].
>
Please remember to copy the MAINTAINERS, (in this case Dan and me) thats
why didn't get review last time.
Will look into this series now...
--
~Vinod
On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
> This will be used in SPEAr, ARM family.
Does this mean it can be used on AVR32 now? Did you implay it will
*also* be....
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/dma/Kconfig | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> index 1c28816..95c7db7 100644
> --- a/drivers/dma/Kconfig
> +++ b/drivers/dma/Kconfig
> @@ -82,7 +82,6 @@ config INTEL_IOP_ADMA
>
> config DW_DMAC
> tristate "Synopsys DesignWare AHB DMA support"
> - depends on AVR32
Shouldn't you be adding a corresponding depends on new arch? And since
this supports old arch as well, it should say depends on both...
> select DMA_ENGINE
> default y if CPU_AT32AP7000
> help
--
~Vinod
On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> 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 3bf4772..5cc5abf 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -830,7 +830,9 @@ dwc_tx_status(struct dma_chan *chan,
>
> ret = dma_async_is_complete(cookie, last_complete, last_used);
> if (ret != DMA_SUCCESS) {
> + spin_lock_bh(&dwc->lock);
> dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
> + spin_unlock_bh(&dwc->lock);
>
> last_complete = dwc->completed;
> last_used = chan->cookie;
Please always add a short description in the patch, helps in long run
Shouldnt you be doing this for dwc_handle_error() as well? I see thats
called without taking the lock....
--
~Vinod
On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
> dwc_desc_get checks all descriptors for DMA_CTRL_ACK before allocating them for
> transfers. And descriptors are not marked with DMA_CRTL_ACK after transfer
> finishes. Thus descriptor once used is not usable again. This patch marks
> descriptors with DMA_CRTL_ACK after dma xfer finishes
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/dma/dw_dmac.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index c40b89f..01f783d 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -196,6 +196,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
> dma_async_tx_callback callback;
> void *param;
> struct dma_async_tx_descriptor *txd = &desc->txd;
> + struct dw_desc *child;
Please align this with previous ones....
--
~Vinod
On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
> In SPEAr Platform channels 4-7 have more Fifo depth. So we must get better
> channel first. This patch introduces concept of channel allocation order in
> dw_dmac. If user doesn't paas anything or 0, than normal (ascending) channel
pass?
> allocation will follow, else channels will be allocated in descending order.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/dma/dw_dmac.c | 6 +++++-
> include/linux/dw_dmac.h | 3 +++
> 2 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 01f783d..37ffd2c 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1314,7 +1314,11 @@ 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;
> - list_add_tail(&dwc->chan.device_node, &dw->dma.channels);
> + if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
> + list_add_tail(&dwc->chan.device_node,
> + &dw->dma.channels);
> + else
> + list_add(&dwc->chan.device_node, &dw->dma.channels);
>
> dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
> spin_lock_init(&dwc->lock);
> diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
> index c8aad71..057e883 100644
> --- a/include/linux/dw_dmac.h
> +++ b/include/linux/dw_dmac.h
> @@ -19,6 +19,9 @@
> */
> struct dw_dma_platform_data {
> unsigned int nr_channels;
> +#define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */
> +#define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero */
Can you add these defines outside of this struct?
> + unsigned int chan_allocation_order;
> };
>
> /**
--
~Vinod
On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
> In Synopsys designware, channel priority is programmable. This patch adds
> support for passing channel priority through platform data. By default Ascending
> channel priority will be followed, i.e. channel 0 will get highest priority and
> channel 7 will get lowest.
>
> Signed-off-by: Viresh Kumar <[email protected]>
> ---
> drivers/dma/dw_dmac.c | 11 ++++++++++-
> drivers/dma/dw_dmac_regs.h | 3 +++
> include/linux/dw_dmac.h | 4 +++-
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 37ffd2c..edb3d3b 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -896,8 +896,11 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
> BUG_ON(!dws->dma_dev || dws->dma_dev != dw->dma.dev);
>
> cfghi = dws->cfg_hi;
> - cfglo = dws->cfg_lo;
> + cfglo = dws->cfg_lo & ~DWC_CFGL_CH_PRIOR_MASK;
> }
> +
> + cfglo |= DWC_CFGL_CH_PRIOR(dwc->priority);
> +
> channel_writel(dwc, CFG_LO, cfglo);
> channel_writel(dwc, CFG_HI, cfghi);
>
> @@ -1320,6 +1323,12 @@ static int __init dw_probe(struct platform_device *pdev)
> else
> list_add(&dwc->chan.device_node, &dw->dma.channels);
>
> + /* 7 is highest priority & 0 is lowest. */
> + if (pdata->chan_priority == CHAN_PRIORITY_ASCENDING)
> + dwc->priority = 7 - i;
> + else
> + dwc->priority = i;
> +
> dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
> spin_lock_init(&dwc->lock);
> dwc->mask = 1 << i;
> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
> index d9a939f..6a8e6d3 100644
> --- a/drivers/dma/dw_dmac_regs.h
> +++ b/drivers/dma/dw_dmac_regs.h
> @@ -101,6 +101,8 @@ struct dw_dma_regs {
> #define DWC_CTLH_BLOCK_TS_MASK 0x00000fff
>
> /* Bitfields in CFG_LO. Platform-configurable bits are in <linux/dw_dmac.h> */
> +#define DWC_CFGL_CH_PRIOR_MASK (0x7 << 5) /* priority mask */
> +#define DWC_CFGL_CH_PRIOR(x) ((x) << 5) /* priority */
> #define DWC_CFGL_CH_SUSP (1 << 8) /* pause xfer */
> #define DWC_CFGL_FIFO_EMPTY (1 << 9) /* pause xfer */
> #define DWC_CFGL_HS_DST (1 << 10) /* handshake w/dst */
> @@ -134,6 +136,7 @@ struct dw_dma_chan {
> struct dma_chan chan;
> void __iomem *ch_regs;
> u8 mask;
> + u8 priority;
>
> spinlock_t lock;
>
> diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
> index 057e883..53072c8 100644
> --- a/include/linux/dw_dmac.h
> +++ b/include/linux/dw_dmac.h
> @@ -22,6 +22,9 @@ struct dw_dma_platform_data {
> #define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */
> #define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero */
> unsigned int chan_allocation_order;
> +#define CHAN_PRIORITY_ASCENDING 0 /* chan0 highest */
> +#define CHAN_PRIORITY_DESCENDING 1 /* chan7 highest */
How about generic CHAN_ORDER_ASCENDING which you can use in both?
> + unsigned int chan_priority;
> };
>
> /**
> @@ -65,7 +68,6 @@ struct dw_dma_slave {
> #define DWC_CFGH_DST_PER(x) ((x) << 11)
>
> /* Platform-configurable bits in CFG_LO */
> -#define DWC_CFGL_PRIO(x) ((x) << 5) /* priority */
> #define DWC_CFGL_LOCK_CH_XFER (0 << 12) /* scope of LOCK_CH */
> #define DWC_CFGL_LOCK_CH_BLOCK (1 << 12)
> #define DWC_CFGL_LOCK_CH_XACT (2 << 12)
--
~Vinod
On 03/02/2011 10:09 PM, Koul, Vinod wrote:
>> >
> Please remember to copy the MAINTAINERS, (in this case Dan and me) thats
> why didn't get review last time.
Sorry for missing that!!
--
viresh
On 03/02/2011 10:16 PM, Koul, Vinod wrote:
> On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>> This will be used in SPEAr, ARM family.
> Does this mean it can be used on AVR32 now? Did you implay it will
> *also* be....
Yes, also be used in spear.
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> drivers/dma/Kconfig | 1 -
>> 1 files changed, 0 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>> index 1c28816..95c7db7 100644
>> --- a/drivers/dma/Kconfig
>> +++ b/drivers/dma/Kconfig
>> @@ -82,7 +82,6 @@ config INTEL_IOP_ADMA
>>
>> config DW_DMAC
>> tristate "Synopsys DesignWare AHB DMA support"
>> - depends on AVR32
> Shouldn't you be adding a corresponding depends on new arch? And since
> this supports old arch as well, it should say depends on both...
Why should this driver be dependent on ARM or AVR32? It can be present
on any other arch too.. So i thought removing this dependency all together
is better.
On 03/02/2011 11:43 PM, Koul, Vinod wrote:
> On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> 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 3bf4772..5cc5abf 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -830,7 +830,9 @@ dwc_tx_status(struct dma_chan *chan,
>>
>> ret = dma_async_is_complete(cookie, last_complete, last_used);
>> if (ret != DMA_SUCCESS) {
>> + spin_lock_bh(&dwc->lock);
>> dwc_scan_descriptors(to_dw_dma(chan->device), dwc);
>> + spin_unlock_bh(&dwc->lock);
>>
>> last_complete = dwc->completed;
>> last_used = chan->cookie;
>
> Please always add a short description in the patch, helps in long run
>
Sure.
> Shouldnt you be doing this for dwc_handle_error() as well? I see thats
> called without taking the lock....
>
dwc_handle_error is called from dw_dma_tasklet with lock held. So its not
required there.
--
viresh
On 03/03/2011 12:04 AM, Koul, Vinod wrote:
> On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>> dwc_desc_get checks all descriptors for DMA_CTRL_ACK before allocating them for
>> transfers. And descriptors are not marked with DMA_CRTL_ACK after transfer
>> finishes. Thus descriptor once used is not usable again. This patch marks
>> descriptors with DMA_CRTL_ACK after dma xfer finishes
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> drivers/dma/dw_dmac.c | 7 +++++++
>> 1 files changed, 7 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index c40b89f..01f783d 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -196,6 +196,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc)
>> dma_async_tx_callback callback;
>> void *param;
>> struct dma_async_tx_descriptor *txd = &desc->txd;
>> + struct dw_desc *child;
> Please align this with previous ones....
>
Will be done
--
viresh
On 03/03/2011 12:07 AM, Koul, Vinod wrote:
> On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>> In SPEAr Platform channels 4-7 have more Fifo depth. So we must get better
>> channel first. This patch introduces concept of channel allocation order in
>> dw_dmac. If user doesn't paas anything or 0, than normal (ascending) channel
> pass?
Oops!!
>> allocation will follow, else channels will be allocated in descending order.
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> drivers/dma/dw_dmac.c | 6 +++++-
>> include/linux/dw_dmac.h | 3 +++
>> 2 files changed, 8 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index 01f783d..37ffd2c 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -1314,7 +1314,11 @@ 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;
>> - list_add_tail(&dwc->chan.device_node, &dw->dma.channels);
>> + if (pdata->chan_allocation_order == CHAN_ALLOCATION_ASCENDING)
>> + list_add_tail(&dwc->chan.device_node,
>> + &dw->dma.channels);
>> + else
>> + list_add(&dwc->chan.device_node, &dw->dma.channels);
>>
>> dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
>> spin_lock_init(&dwc->lock);
>> diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
>> index c8aad71..057e883 100644
>> --- a/include/linux/dw_dmac.h
>> +++ b/include/linux/dw_dmac.h
>> @@ -19,6 +19,9 @@
>> */
>> struct dw_dma_platform_data {
>> unsigned int nr_channels;
>> +#define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */
>> +#define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero */
> Can you add these defines outside of this struct?
I did this deliberately. I feel this is probably the better way as it
tells us _clearly_ the place where this macro is going to be used.
So i would insist on keeping it as it is, if you agree??
>> + unsigned int chan_allocation_order;
--
viresh
On 03/03/2011 12:16 AM, Koul, Vinod wrote:
> On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>> In Synopsys designware, channel priority is programmable. This patch adds
>> support for passing channel priority through platform data. By default Ascending
>> channel priority will be followed, i.e. channel 0 will get highest priority and
>> channel 7 will get lowest.
>>
>> Signed-off-by: Viresh Kumar <[email protected]>
>> ---
>> drivers/dma/dw_dmac.c | 11 ++++++++++-
>> drivers/dma/dw_dmac_regs.h | 3 +++
>> include/linux/dw_dmac.h | 4 +++-
>> 3 files changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index 37ffd2c..edb3d3b 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -896,8 +896,11 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
>> BUG_ON(!dws->dma_dev || dws->dma_dev != dw->dma.dev);
>>
>> cfghi = dws->cfg_hi;
>> - cfglo = dws->cfg_lo;
>> + cfglo = dws->cfg_lo & ~DWC_CFGL_CH_PRIOR_MASK;
>> }
>> +
>> + cfglo |= DWC_CFGL_CH_PRIOR(dwc->priority);
>> +
>> channel_writel(dwc, CFG_LO, cfglo);
>> channel_writel(dwc, CFG_HI, cfghi);
>>
>> @@ -1320,6 +1323,12 @@ static int __init dw_probe(struct platform_device *pdev)
>> else
>> list_add(&dwc->chan.device_node, &dw->dma.channels);
>>
>> + /* 7 is highest priority & 0 is lowest. */
>> + if (pdata->chan_priority == CHAN_PRIORITY_ASCENDING)
>> + dwc->priority = 7 - i;
>> + else
>> + dwc->priority = i;
>> +
>> dwc->ch_regs = &__dw_regs(dw)->CHAN[i];
>> spin_lock_init(&dwc->lock);
>> dwc->mask = 1 << i;
>> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
>> index d9a939f..6a8e6d3 100644
>> --- a/drivers/dma/dw_dmac_regs.h
>> +++ b/drivers/dma/dw_dmac_regs.h
>> @@ -101,6 +101,8 @@ struct dw_dma_regs {
>> #define DWC_CTLH_BLOCK_TS_MASK 0x00000fff
>>
>> /* Bitfields in CFG_LO. Platform-configurable bits are in <linux/dw_dmac.h> */
>> +#define DWC_CFGL_CH_PRIOR_MASK (0x7 << 5) /* priority mask */
>> +#define DWC_CFGL_CH_PRIOR(x) ((x) << 5) /* priority */
>> #define DWC_CFGL_CH_SUSP (1 << 8) /* pause xfer */
>> #define DWC_CFGL_FIFO_EMPTY (1 << 9) /* pause xfer */
>> #define DWC_CFGL_HS_DST (1 << 10) /* handshake w/dst */
>> @@ -134,6 +136,7 @@ struct dw_dma_chan {
>> struct dma_chan chan;
>> void __iomem *ch_regs;
>> u8 mask;
>> + u8 priority;
>>
>> spinlock_t lock;
>>
>> diff --git a/include/linux/dw_dmac.h b/include/linux/dw_dmac.h
>> index 057e883..53072c8 100644
>> --- a/include/linux/dw_dmac.h
>> +++ b/include/linux/dw_dmac.h
>> @@ -22,6 +22,9 @@ struct dw_dma_platform_data {
>> #define CHAN_ALLOCATION_ASCENDING 0 /* zero to seven */
>> #define CHAN_ALLOCATION_DESCENDING 1 /* seven to zero */
>> unsigned int chan_allocation_order;
>> +#define CHAN_PRIORITY_ASCENDING 0 /* chan0 highest */
>> +#define CHAN_PRIORITY_DESCENDING 1 /* chan7 highest */
> How about generic CHAN_ORDER_ASCENDING which you can use in both?
By both, you probably mean, both for allocation and priority??
Actually i thought of this, but realized thought, people might want
to control them separately. They may want priority ascending (0 to 7),
but may need allocation in reverse order (7 to 0). So kept them separate.
What do you say??
--
viresh
On Thu, Mar 03, 2011 at 11:42:42AM +0800, Viresh KUMAR wrote:
> On 03/02/2011 10:16 PM, Koul, Vinod wrote:
> > On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
[...]
> >> drivers/dma/Kconfig | 1 -
> >> 1 files changed, 0 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> >> index 1c28816..95c7db7 100644
> >> --- a/drivers/dma/Kconfig
> >> +++ b/drivers/dma/Kconfig
> >> @@ -82,7 +82,6 @@ config INTEL_IOP_ADMA
> >>
> >> config DW_DMAC
> >> tristate "Synopsys DesignWare AHB DMA support"
> >> - depends on AVR32
> > Shouldn't you be adding a corresponding depends on new arch? And since
> > this supports old arch as well, it should say depends on both...
>
> Why should this driver be dependent on ARM or AVR32? It can be present
> on any other arch too.. So i thought removing this dependency all together
> is better.
There could be a dependency on HAVE_CLK as it uses clock APIs.
--
regards
Shiraz
On Thu, Mar 03, 2011 at 09:49:09AM +0530, Shiraz Hashim wrote:
> On Thu, Mar 03, 2011 at 11:42:42AM +0800, Viresh KUMAR wrote:
> > On 03/02/2011 10:16 PM, Koul, Vinod wrote:
> > > On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>
> [...]
>
> > >> drivers/dma/Kconfig | 1 -
> > >> 1 files changed, 0 insertions(+), 1 deletions(-)
> > >>
> > >> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
> > >> index 1c28816..95c7db7 100644
> > >> --- a/drivers/dma/Kconfig
> > >> +++ b/drivers/dma/Kconfig
> > >> @@ -82,7 +82,6 @@ config INTEL_IOP_ADMA
> > >>
> > >> config DW_DMAC
> > >> tristate "Synopsys DesignWare AHB DMA support"
> > >> - depends on AVR32
> > > Shouldn't you be adding a corresponding depends on new arch? And since
> > > this supports old arch as well, it should say depends on both...
> >
> > Why should this driver be dependent on ARM or AVR32? It can be present
> > on any other arch too.. So i thought removing this dependency all together
> > is better.
>
> There could be a dependency on HAVE_CLK as it uses clock APIs.
If it uses the clk APIs then it should already depend on HAVE_CLK.
On 03/05/2011 05:08 PM, Russell King - ARM Linux wrote:
> On Thu, Mar 03, 2011 at 09:49:09AM +0530, Shiraz Hashim wrote:
>> > On Thu, Mar 03, 2011 at 11:42:42AM +0800, Viresh KUMAR wrote:
>>> > > On 03/02/2011 10:16 PM, Koul, Vinod wrote:
>>>> > > > On Mon, 2011-02-28 at 16:11 +0530, Viresh Kumar wrote:
>> >
>> > [...]
>> >
>>>>> > > >> drivers/dma/Kconfig | 1 -
>>>>> > > >> 1 files changed, 0 insertions(+), 1 deletions(-)
>>>>> > > >>
>>>>> > > >> diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
>>>>> > > >> index 1c28816..95c7db7 100644
>>>>> > > >> --- a/drivers/dma/Kconfig
>>>>> > > >> +++ b/drivers/dma/Kconfig
>>>>> > > >> @@ -82,7 +82,6 @@ config INTEL_IOP_ADMA
>>>>> > > >>
>>>>> > > >> config DW_DMAC
>>>>> > > >> tristate "Synopsys DesignWare AHB DMA support"
>>>>> > > >> - depends on AVR32
>>>> > > > Shouldn't you be adding a corresponding depends on new arch? And since
>>>> > > > this supports old arch as well, it should say depends on both...
>>> > >
>>> > > Why should this driver be dependent on ARM or AVR32? It can be present
>>> > > on any other arch too.. So i thought removing this dependency all together
>>> > > is better.
>> >
>> > There could be a dependency on HAVE_CLK as it uses clock APIs.
> If it uses the clk APIs then it should already depend on HAVE_CLK.
It wasn't. But i have done that in my patchset.
--
viresh