Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver
On 05/21/2014 11:45 AM, Vinod Koul wrote:
On Thu, May 08, 2014 at 05:52:37PM +0800, Hongbo Zhang wrote:
On 05/07/2014 04:31 PM, Shevchenko, Andriy wrote:
On Sun, 2014-05-04 at 18:22 +0800, Hongbo Zhang wrote:
On 05/03/2014 12:46 AM, Vinod Koul wrote:
On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds suspend resume functions for Freescale DMA driver.
.prepare callback is used to stop further descriptors from being added into the
pending queue, and also issue pending queues into execution if there is any.
.suspend callback makes sure all the pending jobs are cleaned up and all the
channels are idle, and save the mode registers.
.resume callback re-initializes the channels by restore the mode registers.
+
+static const struct dev_pm_ops fsldma_pm_ops = {
+ .prepare= fsldma_prepare,
+ .suspend= fsldma_suspend,
+ .resume = fsldma_resume,
+};
I think this is not correct. We discussed this sometime back on list. The
DMAengine drivers should use late resume and early suspend to ensure they get
suspended after clients (who should use normal ones) and resume before them
OK, will update it like this:
use .suspend to take place of current .prepare
Could you remove this at all?
Answering to your previous statements I could say that.
Device drivers (DMAc users) that don't implement .suspend callback are
on their own with troubles, you have not to care about them in the DMA
driver.
Thanks for pointing out this issue.
Then how to handle the descriptors in the pending list if there is any?
a. let them finished.
but if the DMA user has already suspended prior DMA controller,
it is meaningless somehow and may even ask for trouble.
b. don't touch them.
after resume these pending descriptors could be executed, it is
also meaningless because the resumed DMA user may in different state
from before being suspended.
c. delete them.
should we do this? is is a bit crude?
d. return a non-success value
then the whole suspend process is reversed, e.g. suspend fails.
I've looked through some dma drivers, most of them is in case b.
Yes and that makese sense.
In calssic suspend case we maybe in middle so graceful behaviour would be to for
client to PAUSE or terminate and then suspend followed by DMA suspend.
You need to rely on client doing the right thing here
OK, will resend this 6/8, 7/8 and 8/8 for another iteration, and will
let the current 6/8 to be the last one for being reviewed and merged easier.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver
On 05/07/2014 04:31 PM, Shevchenko, Andriy wrote:
On Sun, 2014-05-04 at 18:22 +0800, Hongbo Zhang wrote:
On 05/03/2014 12:46 AM, Vinod Koul wrote:
On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds suspend resume functions for Freescale DMA driver.
.prepare callback is used to stop further descriptors from being added into the
pending queue, and also issue pending queues into execution if there is any.
.suspend callback makes sure all the pending jobs are cleaned up and all the
channels are idle, and save the mode registers.
.resume callback re-initializes the channels by restore the mode registers.
+
+static const struct dev_pm_ops fsldma_pm_ops = {
+ .prepare= fsldma_prepare,
+ .suspend= fsldma_suspend,
+ .resume = fsldma_resume,
+};
I think this is not correct. We discussed this sometime back on list. The
DMAengine drivers should use late resume and early suspend to ensure they get
suspended after clients (who should use normal ones) and resume before them
OK, will update it like this:
use .suspend to take place of current .prepare
Could you remove this at all?
Answering to your previous statements I could say that.
Device drivers (DMAc users) that don't implement .suspend callback are
on their own with troubles, you have not to care about them in the DMA
driver.
Thanks for pointing out this issue.
Then how to handle the descriptors in the pending list if there is any?
a. let them finished.
but if the DMA user has already suspended prior DMA controller, it
is meaningless somehow and may even ask for trouble.
b. don't touch them.
after resume these pending descriptors could be executed, it is
also meaningless because the resumed DMA user may in different state
from before being suspended.
c. delete them.
should we do this? is is a bit crude?
d. return a non-success value
then the whole suspend process is reversed, e.g. suspend fails.
I've looked through some dma drivers, most of them is in case b.
use .suspend_late to take place of current .suspend
use .resume_early to take place of current .resume
--
To unsubscribe from this list: send the line unsubscribe dmaengine in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 6/8] DMA: Freescale: change descriptor release process for supporting async_tx
On 05/03/2014 12:50 AM, Vinod Koul wrote: On Fri, Apr 18, 2014 at 04:17:49PM +0800, hongbo.zh...@freescale.com wrote: This need review from Dan ... Dan, could you please have a look at this? thanks. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 7/8] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
On 05/03/2014 12:51 AM, Vinod Koul wrote: On Fri, Apr 18, 2014 at 04:17:50PM +0800, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com The usage of spin_lock_irqsave() is a stronger locking mechanism than is required throughout the driver. The minimum locking required should be used instead. Interrupts will be turned off and context will be saved, it is unnecessary to use irqsave. This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All manipulation of protected fields is done using tasklet context or weaker, which makes spin_lock_bh() the correct choice. This doesnt apply, perhpas due to depends on 6/8 So let's wait for the review result of 6/8. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v4 8/8] DMA: Freescale: add suspend resume functions for DMA driver
On 05/03/2014 12:46 AM, Vinod Koul wrote:
On Fri, Apr 18, 2014 at 04:17:51PM +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds suspend resume functions for Freescale DMA driver.
.prepare callback is used to stop further descriptors from being added into the
pending queue, and also issue pending queues into execution if there is any.
.suspend callback makes sure all the pending jobs are cleaned up and all the
channels are idle, and save the mode registers.
.resume callback re-initializes the channels by restore the mode registers.
+
+static const struct dev_pm_ops fsldma_pm_ops = {
+ .prepare= fsldma_prepare,
+ .suspend= fsldma_suspend,
+ .resume = fsldma_resume,
+};
I think this is not correct. We discussed this sometime back on list. The
DMAengine drivers should use late resume and early suspend to ensure they get
suspended after clients (who should use normal ones) and resume before them
OK, will update it like this:
use .suspend to take place of current .prepare
use .suspend_late to take place of current .suspend
use .resume_early to take place of current .resume
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 4/8] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication
On 04/14/2014 09:40 PM, Andy Shevchenko wrote: On Fri, 2014-04-11 at 16:14 +0800, Hongbo Zhang wrote: On 04/10/2014 07:29 PM, Andy Shevchenko wrote: On Thu, 2014-04-10 at 15:10 +0800, hongbo.zh...@freescale.com wrote: [] @@ -819,8 +826,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan, dma_run_dependencies(txd); dma_descriptor_unmap(txd); - chan_dbg(chan, LD %p free\n, desc); - dma_pool_free(chan-desc_pool, desc, txd-phys); + fsl_dma_free_descriptor(chan, desc); Here is no list_del() call since it's been called in dma_do_tasklet(). What will be the result of double list_del() against the same node? Not clear with your point. This patch is only introducing a common fsl_dma_free_descriptor() to reduce code duplication. And later in the patch 6/8 the fsldma_cleanup_descriptor() is replaced by fsldma_cleanup_descriptorS(). In the last case you could have a broken kernel which will fails on double list_del(). I think it's better to leave this one untouched and you may remove it later. Or move this patch after you have removed that lines. Good catch, thank you Andy. Yes I prefer to leave this untouched and handle it later. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 8/8] DMA: Freescale: add suspend resume functions for DMA driver
On 04/10/2014 08:05 PM, Andy Shevchenko wrote:
On Thu, 2014-04-10 at 15:10 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds suspend resume functions for Freescale DMA driver.
.prepare callback is used to stop further descriptors from being added into the
pending queue, and also issue pending queues into execution if there is any.
.suspend callback makes sure all the pending jobs are cleaned up and all the
channels are idle, and save the mode registers.
.resume callback re-initializes the channels by restore the mode registers.
Like we discussed with Vinod [1] the DMA controller drivers should go to
suspend after users and come back before them.
After you reconsider this point the patch logic might be modified a lot.
Looked through that discussions, I really had thought such problem for a
while.
For the dma-controller and dma-user, we don't know which .suspend
callback is executed firstly, so the idea would be: which ever is called
earlier, the dma-controller driver suspend function should be as robust
as possible.
It is better the dma-user .suspend callback is called earlier, some
clean-ups should be done here such as stop dma request, but we cannot
make sure every dma-user has a .suspend callback, some dma-users don't
pay attention to or even don't care the suspend at all for some reason.
So even the suspend_late is used, we cannot assume every dma-user's
activity is cleaned up neatly, dma-controller driver should be robust to
handle this gracefully, that was my design target.
In the prepare() function, clean up the pending queue and stop receive
new coming request, and in the suspend() function I do some register
saving works. I don't think my code needs to be modified much, a
possible change according to Vinod's idea would be:
use .suspend instead of my current .prepare
use . suspend_late instead of my current .suspend
e.g. postpone all my functions to be executed later, this method works
and seems better for client/low-level module drivers.
The reason I didn't use the above functions was that I had read this
Documentation/power/devices.txt, search the definitions of prepare,
suspend, suspend_late and suspend_noirq, I think my usage complies with
that definitions strictly, I can do the modification above if the
maintainer like and if nobody says I break this law.
(Moreover, you abuse your own position to use only setters/getters to
access to the DMAc registers)
My shame, I will update it.
(reason is the setters/getters patch isn't merged into our internal
tree, but this suspend patch has been done. I am trying to sync our
internal kernel and the community now)
[1] http://www.spinics.net/lists/kernel/msg1650974.html
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
drivers/dma/fsldma.c | 100 ++
drivers/dma/fsldma.h | 16
2 files changed, 116 insertions(+)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index c9bf54a..d6da222 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -400,6 +400,14 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
spin_lock_bh(chan-desc_lock);
+#ifdef CONFIG_PM
+ if (unlikely(chan-pm_state != RUNNING)) {
+ chan_dbg(chan, cannot submit due to suspend\n);
+ spin_unlock_bh(chan-desc_lock);
+ return -1;
+ }
+#endif
+
/*
* assign cookies to all of the software descriptors
* that make up this transaction
@@ -1311,6 +1319,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
INIT_LIST_HEAD(chan-ld_running);
INIT_LIST_HEAD(chan-ld_completed);
chan-idle = true;
+#ifdef CONFIG_PM
+ chan-pm_state = RUNNING;
+#endif
chan-common.device = fdev-common;
dma_cookie_init(chan-common);
@@ -1450,6 +1461,92 @@ static int fsldma_of_remove(struct platform_device *op)
return 0;
}
+#ifdef CONFIG_PM
+static int fsldma_prepare(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct fsldma_device *fdev = platform_get_drvdata(pdev);
+ struct fsldma_chan *chan;
+ int i;
+
+ for (i = 0; i FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+ chan = fdev-chan[i];
+ if (!chan)
+ continue;
+
+ spin_lock_bh(chan-desc_lock);
+ chan-pm_state = SUSPENDING;
+ if (!list_empty(chan-ld_pending))
+ fsl_chan_xfer_ld_queue(chan);
+ spin_unlock_bh(chan-desc_lock);
+ }
+
+ return 0;
+}
+
+static int fsldma_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct fsldma_device *fdev = platform_get_drvdata(pdev);
+ struct fsldma_chan *chan;
+ int i;
+
+ for (i = 0; i
Re: [PATCH v3 6/8] DMA: Freescale: change descriptor release process for supporting async_tx
On 04/10/2014 07:56 PM, Andy Shevchenko wrote:
On Thu, 2014-04-10 at 15:10 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Fix the potential risk when enable config NET_DMA and ASYNC_TX. Async_tx is
lack of support in current release process of dma descriptor, all descriptors
will be released whatever is acked or no-acked by async_tx, so there is a
potential race condition when dma engine is uesd by others clients (e.g. when
enable NET_DMA to offload TCP).
In our case, a race condition which is raised when use both of talitos and
dmaengine to offload xor is because napi scheduler will sync all pending
requests in dma channels, it affects the process of raid operations due to
ack_tx is not checked in fsl dma. The no-acked descriptor is freed which is
submitted just now, as a dependent tx, this freed descriptor trigger
BUG_ON(async_tx_test_ack(depend_tx)) in async_tx_submit().
TASK = ee1a94a0[1390] 'md0_raid5' THREAD: ecf4 CPU: 0
GPR00: 0001 ecf41ca0 ee44/921a94a0 003f 0001 c00593e4
0001
GPR08: a7a7a7a7 0001 045/92002 42028042 100a38d4 ed576d98
GPR16: ed5a11b0 2b162000 0200 046/92000 2d555000 ed3015e8
c15a7aa0
GPR24: c155fc40 ecb63220 ecf41d28 e47/92f640bb0 ef640c30
ecf41ca0
NIP [c02b048c] async_tx_submit+0x6c/0x2b4
LR [c02b068c] async_tx_submit+0x26c/0x2b4
Call Trace:
[ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
[ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
[ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
[ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
[ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
[ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
[ecf41f40] [c04329b8] md_thread+0x138/0x16c
[ecf41f90] [c008277c] kthread+0x8c/0x90
[ecf41ff0] [c0011630] kernel_thread+0x4c/0x68
Another modification in this patch is the change of completed descriptors,
there is a potential risk which caused by exception interrupt, all descriptors
in ld_running list are seemed completed when an interrupt raised, it works fine
under normal condition, but if there is an exception occured, it cannot work as
our excepted. Hardware should not be depend on s/w list, the right way is to
read current descriptor address register to find the last completed descriptor.
If an interrupt is raised by an error, all descriptors in ld_running should not
be seemed finished, or these unfinished descriptors in ld_running will be
released wrongly.
A simple way to reproduce:
Enable dmatest first, then insert some bad descriptors which can trigger
Programming Error interrupts before the good descriptors. Last, the good
descriptors will be freed before they are processsed because of the exception
intrerrupt.
Note: the bad descriptors are only for simulating an exception interrupt. This
case can illustrate the potential risk in current fsl-dma very well.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
Signed-off-by: Qiang Liu qiang@freescale.com
Signed-off-by: Ira W. Snyder i...@ovro.caltech.edu
---
drivers/dma/fsldma.c | 195 --
drivers/dma/fsldma.h | 17 -
2 files changed, 158 insertions(+), 54 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 968877f..f8eee60 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -459,6 +459,87 @@ static struct fsl_desc_sw *fsl_dma_alloc_descriptor(struct
fsldma_chan *chan)
}
/**
+ * fsldma_clean_completed_descriptor - free all descriptors which
+ * has been completed and acked
IIRC the summary should be oneliner.
Check the rest of the code as well.
I don't think so.
See this Documentation/kernel-doc-nano-HOWTO.txt, and you can find this
sentence The short description following the subject can span multiple
lines
+ * @chan: Freescale DMA channel
+ *
+ * This function is used on all completed and acked descriptors.
+ * All descriptors should only be freed in this function.
+ */
+static void fsldma_clean_completed_descriptor(struct fsldma_chan *chan)
+{
+ struct fsl_desc_sw *desc, *_desc;
+
+ /* Run the callback for each descriptor, in order */
+ list_for_each_entry_safe(desc, _desc, chan-ld_completed, node)
+ if (async_tx_test_ack(desc-async_tx))
+ fsl_dma_free_descriptor(chan, desc);
+}
+
+/**
+ * fsldma_run_tx_complete_actions - cleanup a single link descriptor
+ * @chan: Freescale DMA channel
+ * @desc: descriptor to cleanup and free
+ * @cookie: Freescale DMA transaction identifier
+ *
+ * This function is used on a descriptor which has been executed by the DMA
+ * controller. It will run any callbacks, submit any dependencies.
+ */
+static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
+ struct fsl_desc_sw *desc, dma_cookie_t cookie)
Maybe you could use cookie as local variable?
Yes.. it doesn't seem good to set a value to input parameter
Re: [PATCH v3 4/8] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication
On 04/10/2014 07:29 PM, Andy Shevchenko wrote:
On Thu, 2014-04-10 at 15:10 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
There are several places where descriptors are freed using identical code.
This patch puts this code into a function to reduce code duplication.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
Signed-off-by: Qiang Liu qiang@freescale.com
---
drivers/dma/fsldma.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index b71cc04..b5a0ffa 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -418,6 +418,19 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
}
/**
+ * fsl_dma_free_descriptor - Free descriptor from channel's DMA pool.
+ * @chan : Freescale DMA channel
+ * @desc: descriptor to be freed
+ */
+static void fsl_dma_free_descriptor(struct fsldma_chan *chan,
+ struct fsl_desc_sw *desc)
+{
+ list_del(desc-node);
+ chan_dbg(chan, LD %p free\n, desc);
+ dma_pool_free(chan-desc_pool, desc, desc-async_tx.phys);
+}
+
+/**
* fsl_dma_alloc_descriptor - Allocate descriptor from channel's DMA pool.
* @chan : Freescale DMA channel
*
@@ -489,11 +502,8 @@ static void fsldma_free_desc_list(struct fsldma_chan *chan,
{
struct fsl_desc_sw *desc, *_desc;
- list_for_each_entry_safe(desc, _desc, list, node) {
- list_del(desc-node);
- chan_dbg(chan, LD %p free\n, desc);
- dma_pool_free(chan-desc_pool, desc, desc-async_tx.phys);
- }
+ list_for_each_entry_safe(desc, _desc, list, node)
+ fsl_dma_free_descriptor(chan, desc);
}
static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
@@ -501,11 +511,8 @@ static void fsldma_free_desc_list_reverse(struct
fsldma_chan *chan,
{
struct fsl_desc_sw *desc, *_desc;
- list_for_each_entry_safe_reverse(desc, _desc, list, node) {
- list_del(desc-node);
- chan_dbg(chan, LD %p free\n, desc);
- dma_pool_free(chan-desc_pool, desc, desc-async_tx.phys);
- }
+ list_for_each_entry_safe_reverse(desc, _desc, list, node)
+ fsl_dma_free_descriptor(chan, desc);
}
/**
@@ -819,8 +826,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan
*chan,
dma_run_dependencies(txd);
dma_descriptor_unmap(txd);
- chan_dbg(chan, LD %p free\n, desc);
- dma_pool_free(chan-desc_pool, desc, txd-phys);
+ fsl_dma_free_descriptor(chan, desc);
Here is no list_del() call since it's been called in dma_do_tasklet().
What will be the result of double list_del() against the same node?
Not clear with your point.
This patch is only introducing a common fsl_dma_free_descriptor() to
reduce code duplication. And later in the patch 6/8 the
fsldma_cleanup_descriptor() is replaced by fsldma_cleanup_descriptorS().
}
/**
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 6/8] DMA: Freescale: change descriptor release process for supporting async_tx
On 04/11/2014 04:00 PM, Hongbo Zhang wrote:
On 04/10/2014 07:56 PM, Andy Shevchenko wrote:
On Thu, 2014-04-10 at 15:10 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Fix the potential risk when enable config NET_DMA and ASYNC_TX.
Async_tx is
lack of support in current release process of dma descriptor, all
descriptors
will be released whatever is acked or no-acked by async_tx, so there
is a
potential race condition when dma engine is uesd by others clients
(e.g. when
enable NET_DMA to offload TCP).
In our case, a race condition which is raised when use both of
talitos and
dmaengine to offload xor is because napi scheduler will sync all
pending
requests in dma channels, it affects the process of raid operations
due to
ack_tx is not checked in fsl dma. The no-acked descriptor is freed
which is
submitted just now, as a dependent tx, this freed descriptor trigger
BUG_ON(async_tx_test_ack(depend_tx)) in async_tx_submit().
TASK = ee1a94a0[1390] 'md0_raid5' THREAD: ecf4 CPU: 0
GPR00: 0001 ecf41ca0 ee44/921a94a0 003f 0001 c00593e4
0001
GPR08: a7a7a7a7 0001 045/92002 42028042 100a38d4
ed576d98
GPR16: ed5a11b0 2b162000 0200 046/92000 2d555000
ed3015e8 c15a7aa0
GPR24: c155fc40 ecb63220 ecf41d28 e47/92f640bb0
ef640c30 ecf41ca0
NIP [c02b048c] async_tx_submit+0x6c/0x2b4
LR [c02b068c] async_tx_submit+0x26c/0x2b4
Call Trace:
[ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
[ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
[ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
[ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
[ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
[ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
[ecf41f40] [c04329b8] md_thread+0x138/0x16c
[ecf41f90] [c008277c] kthread+0x8c/0x90
[ecf41ff0] [c0011630] kernel_thread+0x4c/0x68
Another modification in this patch is the change of completed
descriptors,
there is a potential risk which caused by exception interrupt, all
descriptors
in ld_running list are seemed completed when an interrupt raised, it
works fine
under normal condition, but if there is an exception occured, it
cannot work as
our excepted. Hardware should not be depend on s/w list, the right
way is to
read current descriptor address register to find the last completed
descriptor.
If an interrupt is raised by an error, all descriptors in ld_running
should not
be seemed finished, or these unfinished descriptors in ld_running
will be
released wrongly.
A simple way to reproduce:
Enable dmatest first, then insert some bad descriptors which can
trigger
Programming Error interrupts before the good descriptors. Last, the
good
descriptors will be freed before they are processsed because of the
exception
intrerrupt.
Note: the bad descriptors are only for simulating an exception
interrupt. This
case can illustrate the potential risk in current fsl-dma very well.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
Signed-off-by: Qiang Liu qiang@freescale.com
Signed-off-by: Ira W. Snyder i...@ovro.caltech.edu
---
drivers/dma/fsldma.c | 195
--
drivers/dma/fsldma.h | 17 -
2 files changed, 158 insertions(+), 54 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 968877f..f8eee60 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -459,6 +459,87 @@ static struct fsl_desc_sw
*fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
}
/**
+ * fsldma_clean_completed_descriptor - free all descriptors which
+ * has been completed and acked
IIRC the summary should be oneliner.
Check the rest of the code as well.
I don't think so.
See this Documentation/kernel-doc-nano-HOWTO.txt, and you can find
this sentence The short description following the subject can span
multiple lines
+ * @chan: Freescale DMA channel
+ *
+ * This function is used on all completed and acked descriptors.
+ * All descriptors should only be freed in this function.
+ */
+static void fsldma_clean_completed_descriptor(struct fsldma_chan
*chan)
+{
+struct fsl_desc_sw *desc, *_desc;
+
+/* Run the callback for each descriptor, in order */
+list_for_each_entry_safe(desc, _desc, chan-ld_completed, node)
+if (async_tx_test_ack(desc-async_tx))
+fsl_dma_free_descriptor(chan, desc);
+}
+
+/**
+ * fsldma_run_tx_complete_actions - cleanup a single link descriptor
+ * @chan: Freescale DMA channel
+ * @desc: descriptor to cleanup and free
+ * @cookie: Freescale DMA transaction identifier
+ *
+ * This function is used on a descriptor which has been executed by
the DMA
+ * controller. It will run any callbacks, submit any dependencies.
+ */
+static dma_cookie_t fsldma_run_tx_complete_actions(struct
fsldma_chan *chan,
+struct fsl_desc_sw *desc, dma_cookie_t cookie)
Maybe you could use cookie as local variable?
Yes.. it doesn't
Re: [PATCH v2 8/8] DMA: Freescale: add suspend resume functions for DMA driver
On 04/04/2014 11:27 AM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds suspend resume functions for Freescale DMA driver.
.prepare callback is used to stop further descriptors from being added into the
pending queue, and also issue pending queues into execution if there is any.
.suspend callback makes sure all the pending jobs are cleaned up and all the
channels are idle, and save the mode registers.
.resume callback re-initializes the channels by restore the mode registers.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
drivers/dma/fsldma.c | 99 ++
drivers/dma/fsldma.h | 16
2 files changed, 115 insertions(+)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index c9bf54a..91482d2 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -400,6 +400,14 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
spin_lock_bh(chan-desc_lock);
+#ifdef CONFIG_PM
+ if (unlikely(chan-pm_state != RUNNING)) {
+ chan_dbg(chan, cannot submit due to suspend\n);
+ spin_unlock_bh(chan-desc_lock);
+ return -1;
+ }
+#endif
+
/*
* assign cookies to all of the software descriptors
* that make up this transaction
@@ -1311,6 +1319,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
INIT_LIST_HEAD(chan-ld_running);
INIT_LIST_HEAD(chan-ld_completed);
chan-idle = true;
+#ifdef CONFIG_PM
+ chan-pm_state = RUNNING;
+#endif
chan-common.device = fdev-common;
dma_cookie_init(chan-common);
@@ -1450,6 +1461,91 @@ static int fsldma_of_remove(struct platform_device *op)
return 0;
}
+#ifdef CONFIG_PM
+static int fsldma_prepare(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct fsldma_device *fdev = platform_get_drvdata(pdev);
+ struct fsldma_chan *chan;
+ int i;
+
+ for (i = 0; i FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+ chan = fdev-chan[i];
+ if (!chan)
+ continue;
+
+ spin_lock_bh(chan-desc_lock);
+ chan-pm_state = SUSPENDING;
+ if (!list_empty(chan-ld_pending))
+ fsl_chan_xfer_ld_queue(chan);
+ spin_unlock_bh(chan-desc_lock);
+ }
+
+ return 0;
+}
+
+static int fsldma_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct fsldma_device *fdev = platform_get_drvdata(pdev);
+ struct fsldma_chan *chan;
+ int i;
+
+ for (i = 0; i FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+ chan = fdev-chan[i];
+ if (!chan)
+ continue;
+
+ spin_lock_bh(chan-desc_lock);
+ if (!chan-idle)
+ goto out;
+ chan-regs_save.mr = DMA_IN(chan, chan-regs-mr, 32);
+ chan-pm_state = SUSPENDED;
+ spin_unlock_bh(chan-desc_lock);
+ }
+ return 0;
+
+out:
+ for (; i = 0; i--) {
+ chan = fdev-chan[i];
+ if (!chan)
+ continue;
the pm_state should be restored too, e.g.
chan-pm_state = RUNNING;
I will send new iteration for adding this.
+ spin_unlock_bh(chan-desc_lock);
+ }
+ return -EBUSY;
+}
+
+static int fsldma_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct fsldma_device *fdev = platform_get_drvdata(pdev);
+ struct fsldma_chan *chan;
+ u32 mode;
+ int i;
+
+ for (i = 0; i FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+ chan = fdev-chan[i];
+ if (!chan)
+ continue;
+
+ spin_lock_bh(chan-desc_lock);
+ mode = chan-regs_save.mr
+~FSL_DMA_MR_CS ~FSL_DMA_MR_CC ~FSL_DMA_MR_CA;
+ DMA_OUT(chan, chan-regs-mr, mode, 32);
+ chan-pm_state = RUNNING;
+ spin_unlock_bh(chan-desc_lock);
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops fsldma_pm_ops = {
+ .prepare= fsldma_prepare,
+ .suspend= fsldma_suspend,
+ .resume = fsldma_resume,
+};
+#endif
+
static const struct of_device_id fsldma_of_ids[] = {
{ .compatible = fsl,elo3-dma, },
{ .compatible = fsl,eloplus-dma, },
@@ -1462,6 +1558,9 @@ static struct platform_driver fsldma_of_driver = {
.name = fsl-elo-dma,
.owner = THIS_MODULE,
.of_match_table = fsldma_of_ids,
+#ifdef CONFIG_PM
+ .pm = fsldma_pm_ops,
+#endif
},
.probe = fsldma_of_probe,
.remove = fsldma_of_remove,
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index
[PATCH v3 0/8] DMA: Freescale: driver cleanups and enhancements
Sorry, forgot the cover letter, plus it here. From: Hongbo Zhang hongbo.zh...@freescale.com Date: Thu, 10 Apr 2014 15:16:31 +0800 Subject: [PATCH v3 0/8] DMA: Freescale: driver cleanups and enhancements Hi Vinod Koul, Please have a look at the v3 patch set. v2 - v3 change: Only add chan-pm_state = RUNNING for patch[8/8]. v1 - v2 change: The only one change is introducing a new patch[1/7] to remove the unnecessary macro FSL_DMA_LD_DEBUG, thus the total patches number is 8 now (was 7) Hongbo Zhang (8): DMA: Freescale: remove the unnecessary FSL_DMA_LD_DEBUG DMA: Freescale: unify register access methods DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication DMA: Freescale: move functions to avoid forward declarations DMA: Freescale: change descriptor release process for supporting async_tx DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave DMA: Freescale: add suspend resume functions for DMA driver drivers/dma/fsldma.c | 591 -- drivers/dma/fsldma.h | 33 ++- 2 files changed, 409 insertions(+), 215 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 2/8] DMA: Freescale: unify register access methods
On 04/10/2014 04:46 PM, David Laight wrote: From: hongbo.zh...@freescale.com Methods of accessing DMA contorller registers are inconsistent, some registers ^^ Thanks. sorry, that it a typo. I would wait to see if there are other defects I have to correct, if yes I can send a new iteration including this update, if no I would like to know if the maintainer can do me the favor to correct it when merging this patch, if still no, I will send a new iteration for this then. are accessed by DMA_IN/OUT directly, while others are accessed by functions get/set_* which are wrappers of DMA_IN/OUT, and even for the BCR register, it is read by get_bcr but written by DMA_OUT. This patch unifies the inconsistent methods, all registers are accessed by get/set_* now. David ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 6/7] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
On 04/03/2014 12:35 AM, Vinod Koul wrote:
On Mon, Mar 31, 2014 at 12:08:55PM +0800, Hongbo Zhang wrote:
On 03/29/2014 09:45 PM, Vinod Koul wrote:
On Fri, Mar 28, 2014 at 02:33:37PM +0800, Hongbo Zhang wrote:
On 03/26/2014 03:01 PM, Vinod Koul wrote:
On Thu, 2014-01-16 at 13:47 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
The usage of spin_lock_irqsave() is a stronger locking mechanism than is
required throughout the driver. The minimum locking required should be used
instead. Interrupts will be turned off and context will be saved, it is
unnecessary to use irqsave.
This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
manipulation of protected fields is done using tasklet context or weaker, which
makes spin_lock_bh() the correct choice.
/**
@@ -1124,11 +1120,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
static void dma_do_tasklet(unsigned long data)
{
struct fsldma_chan *chan = (struct fsldma_chan *)data;
- unsigned long flags;
chan_dbg(chan, tasklet entry\n);
- spin_lock_irqsave(chan-desc_lock, flags);
+ spin_lock_bh(chan-desc_lock);
okay here is the problem :(
You moved to _bh variant. So if you grab the lock in rest of the code
and irq gets triggered then here we will be spinning to grab the lock.
So effectively you made right locking solution into deadlock situation!
If the rest code grabs lock by spin_lock_bh(), and if irq raised,
the tasklet could not be executed because it has been disabled by
the _bh variant function.
yes if you are accessing resources only in tasklet and rest of the code, then
_bh variant works well. The problem here is usage in irq handler
The name dma_do_tasklet may mislead, it is tasklet handler, not irq
handler, not a trigger to load tasklet.
the irq handler is fsldma_chan_irq, and I don't use lock in it.
sorry my bad, i misread this as code in fsldma_chan_irq() insteadof
dma_do_tasklet(). In that case patch is doing the right thing.
OK, so I will send a v2 series with only updating 3/7 soon.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/7] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication
On 03/28/2014 11:44 AM, Hongbo Zhang wrote:
On 03/11/2014 07:06 PM, Vinod Koul wrote:
On Thu, Jan 16, 2014 at 01:47:22PM +0800, hongbo.zh...@freescale.com
wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
There are several places where descriptors are freed using identical
code.
This patch puts this code into a function to reduce code duplication.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
Signed-off-by: Qiang Liu qiang@freescale.com
---
drivers/dma/fsldma.c | 38 --
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 95236e6..ad73538 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -418,6 +418,21 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
}
/**
+ * fsl_dma_free_descriptor - Free descriptor from channel's DMA pool.
+ * @chan : Freescale DMA channel
+ * @desc: descriptor to be freed
+ */
+static void fsl_dma_free_descriptor(struct fsldma_chan *chan,
+struct fsl_desc_sw *desc)
+{
+list_del(desc-node);
+#ifdef FSL_DMA_LD_DEBUG
+chan_dbg(chan, LD %p free\n, desc);
+#endif
why not wrap the define stuff in the defination of chan_dbg rather
than its
usage :(
OK, I will fix it by another separate patch.
Thanks.
Think it again, I'd like to remove the FSL_DMA_LD_DEBUG usage, because
the chan_dbg is a wrapper of dev_dbg, we do have macro to switch on/off
dev_dbg, and most of other codes are calling chan_dbg directly without
FSL_DMA_LD_DEBUG, the FSL_DMA_LD_DEBUG only shows up 3 times unnecessarily.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 6/7] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
On 03/26/2014 03:01 PM, Vinod Koul wrote:
On Thu, 2014-01-16 at 13:47 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
The usage of spin_lock_irqsave() is a stronger locking mechanism than is
required throughout the driver. The minimum locking required should be used
instead. Interrupts will be turned off and context will be saved, it is
unnecessary to use irqsave.
This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
manipulation of protected fields is done using tasklet context or weaker, which
makes spin_lock_bh() the correct choice.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
Signed-off-by: Qiang Liu qiang@freescale.com
---
drivers/dma/fsldma.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index bbace54..437794e 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -396,10 +396,9 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
struct fsldma_chan *chan = to_fsl_chan(tx-chan);
struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
struct fsl_desc_sw *child;
- unsigned long flags;
dma_cookie_t cookie = -EINVAL;
- spin_lock_irqsave(chan-desc_lock, flags);
+ spin_lock_bh(chan-desc_lock);
/*
* assign cookies to all of the software descriptors
@@ -412,7 +411,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
/* put this transaction onto the tail of the pending queue */
append_ld_queue(chan, desc);
- spin_unlock_irqrestore(chan-desc_lock, flags);
+ spin_unlock_bh(chan-desc_lock);
return cookie;
}
@@ -731,15 +730,14 @@ static void fsldma_free_desc_list_reverse(struct
fsldma_chan *chan,
static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
{
struct fsldma_chan *chan = to_fsl_chan(dchan);
- unsigned long flags;
chan_dbg(chan, free all channel resources\n);
- spin_lock_irqsave(chan-desc_lock, flags);
+ spin_lock_bh(chan-desc_lock);
fsldma_cleanup_descriptors(chan);
fsldma_free_desc_list(chan, chan-ld_pending);
fsldma_free_desc_list(chan, chan-ld_running);
fsldma_free_desc_list(chan, chan-ld_completed);
- spin_unlock_irqrestore(chan-desc_lock, flags);
+ spin_unlock_bh(chan-desc_lock);
dma_pool_destroy(chan-desc_pool);
chan-desc_pool = NULL;
@@ -958,7 +956,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
{
struct dma_slave_config *config;
struct fsldma_chan *chan;
- unsigned long flags;
int size;
if (!dchan)
@@ -968,7 +965,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
switch (cmd) {
case DMA_TERMINATE_ALL:
- spin_lock_irqsave(chan-desc_lock, flags);
+ spin_lock_bh(chan-desc_lock);
/* Halt the DMA engine */
dma_halt(chan);
@@ -979,7 +976,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
fsldma_free_desc_list(chan, chan-ld_completed);
chan-idle = true;
- spin_unlock_irqrestore(chan-desc_lock, flags);
+ spin_unlock_bh(chan-desc_lock);
return 0;
case DMA_SLAVE_CONFIG:
@@ -1021,11 +1018,10 @@ static int fsl_dma_device_control(struct dma_chan
*dchan,
static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
{
struct fsldma_chan *chan = to_fsl_chan(dchan);
- unsigned long flags;
- spin_lock_irqsave(chan-desc_lock, flags);
+ spin_lock_bh(chan-desc_lock);
fsl_chan_xfer_ld_queue(chan);
- spin_unlock_irqrestore(chan-desc_lock, flags);
+ spin_unlock_bh(chan-desc_lock);
}
/**
@@ -1124,11 +1120,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
static void dma_do_tasklet(unsigned long data)
{
struct fsldma_chan *chan = (struct fsldma_chan *)data;
- unsigned long flags;
chan_dbg(chan, tasklet entry\n);
- spin_lock_irqsave(chan-desc_lock, flags);
+ spin_lock_bh(chan-desc_lock);
okay here is the problem :(
You moved to _bh variant. So if you grab the lock in rest of the code
and irq gets triggered then here we will be spinning to grab the lock.
So effectively you made right locking solution into deadlock situation!
If the rest code grabs lock by spin_lock_bh(), and if irq raised, the
tasklet could not be executed because it has been disabled by the _bh
variant function.
Right?
/* the hardware is now idle and ready for more */
chan-idle = true;
@@ -1136,7 +1131,7 @@ static void dma_do_tasklet(unsigned long data)
/* Run all cleanup for descriptors which have been completed */
fsldma_cleanup_descriptors(chan);
- spin_unlock_irqrestore(chan-desc_lock, flags);
+ spin_unlock_bh(chan-desc_lock);
chan_dbg(chan
Re: [PATCH 3/7] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication
On 03/11/2014 07:06 PM, Vinod Koul wrote:
On Thu, Jan 16, 2014 at 01:47:22PM +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
There are several places where descriptors are freed using identical code.
This patch puts this code into a function to reduce code duplication.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
Signed-off-by: Qiang Liu qiang@freescale.com
---
drivers/dma/fsldma.c | 38 --
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 95236e6..ad73538 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -418,6 +418,21 @@ static dma_cookie_t fsl_dma_tx_submit(struct
dma_async_tx_descriptor *tx)
}
/**
+ * fsl_dma_free_descriptor - Free descriptor from channel's DMA pool.
+ * @chan : Freescale DMA channel
+ * @desc: descriptor to be freed
+ */
+static void fsl_dma_free_descriptor(struct fsldma_chan *chan,
+ struct fsl_desc_sw *desc)
+{
+ list_del(desc-node);
+#ifdef FSL_DMA_LD_DEBUG
+ chan_dbg(chan, LD %p free\n, desc);
+#endif
why not wrap the define stuff in the defination of chan_dbg rather than its
usage :(
OK, I will fix it by another separate patch.
Thanks.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/7] DMA: Freescale: driver cleanups and enhancements
Hi Vinod, How about these patches? Thanks. On 01/16/2014 01:47 PM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Hi Vinod Koul and Dan Williams, Please have a look at these patches. Note that patch 2~6 had beed sent out for upstream before, but were together with other storage patches at that time, that was not easy for being reviewed and merged, so I send them separately this time. Thanks. Hongbo Zhang (7): DMA: Freescale: unify register access methods DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication DMA: Freescale: move functions to avoid forward declarations DMA: Freescale: change descriptor release process for supporting async_tx DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave DMA: Freescale: add suspend resume functions for DMA driver drivers/dma/fsldma.c | 592 -- drivers/dma/fsldma.h | 33 ++- 2 files changed, 412 insertions(+), 213 deletions(-) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/5] powerpc/85xx/dts: add third elo3 dma component
On 12/13/2013 01:43 PM, Liu Shengzhou-B36685 wrote: -Original Message- From: Hongbo Zhang [mailto:hongbo.zh...@freescale.com] Sent: Thursday, December 12, 2013 5:57 PM To: Liu Shengzhou-B36685; linuxppc-dev@lists.ozlabs.org; Wood Scott- B07421 Subject: Re: [PATCH 1/5] powerpc/85xx/dts: add third elo3 dma component Shengzhou, I canceled my patch http://patchwork.ozlabs.org/patch/295157/ because the original wrong elo3-dma-2.dtsi hadn't been merged. But please pay attention to comments from Scott about my changes of adding 208 for some interrupts, and take some actions if needed, or further discussions. Below comments form Scott: The FSL MPIC binding should be updated to point out how this works. Technically it's not a change to the binding itself, since it's defined in terms of register offset, but the explanatory text says So interrupt 0 is at offset 0x0, interrupt 1 is at offset 0x20, and so on. which is not accurate for these new high interrupt numbers. Hongbo, Could you update FSL MPIC binding as Scott pointed out? We only need to add more explanatory text after the sentence Scott pointed out, like: But for some hardwares, the MPIC registers for interrupts are not continuous in address, in such cases, an offset can be added to the interrupt number to skip the registers which is not for interrupts. Scott, is that OK? Thanks. thanks, Shengzhou ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] DTS: DMA: Fix DMA3 interrupts
On 12/11/2013 02:33 AM, Scott Wood wrote:
On Tue, 2013-12-10 at 18:33 +0800, Hongbo Zhang wrote:
Scott,
This issue is due to the non-continuous MPIC register, I think there is
two ways to fix it.
The first one is as what we are discussing, in fact the Bman/Qman DT
author had introduced this way, and I had to follow it, it is a trick,
adding 208 is a bit ugly I think, and even difficult to explain it to
customers etc, but this way changes less codes.
The second one is editing MPIC related codes without adding 208 to high
interrupts. The point of translate interrupt number to MPIC register
address is a so called 'isu' mechanism, we can do like the following
example codes, then the tricky adding 208 isn't needed any more.
Which one do you prefer?
In fact I myself prefer the second, if the idea is acceptable, I will
send a patch instead of this one. (and also alone with the internal
patch decreasing 208 for the Bman/Qman)
void __init corenet_ds_pic_init(void)
{
..
mpic = mpic_alloc(NULL, 0, flags, 0, 512, OpenPIC);
BUG_ON(mpic == NULL);
// Add this start
for (i = 0; i 17; i++) {
if (i 11)
addr_off = 0x1 + 0x20 * 16 * i;
else
addr_off = 0x13000 + 0x20 * 16 * (i - 11); /* scape the
address not for interrupts */
mpic_assign_isu(mpic, i, mpic-paddr + addr_off);
}
// Add this end
mpic_init(mpic);
}
NACK
We already have a binding that states that the interrupt number is based
on the register offset, rather than whatever arbitrary numbers hardware
documenters decide to use next week.
While I'm not terribly happy with the usability of this, especially now
that it's not a simple add 16, redefining the existing binding is not
OK (and in any case the code above seems obfuscatory). If we decide to
do something other than continue with register offset divided by 32,
then we need to define a new interrupt type (similar to current defined
types of error interrupt, timer, and IPI) for the new numberspace -- and
it should be handled when decoding that type of interrupt specifier,
rather than with the isu mechanism.
-Scott
Scott,
Thanks for your comments. Since the second way isn't so good, let's
choose the original one.
But we meet a small accident now.
My patch is based on the http://patchwork.ozlabs.org/patch/291553/,
which had been superseded, so this thread can be closed now.
And Shenzhou has already sent a complete dma3 dtsi patch including
correct interrupt numbers, http://patchwork.ozlabs.org/patch/300026/, so
let's focus on this patch, and I will forward your first comments of my
patch there.
Thanks.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/5] powerpc/85xx/dts: add third elo3 dma component
Shengzhou,
I canceled my patch http://patchwork.ozlabs.org/patch/295157/ because
the original wrong elo3-dma-2.dtsi hadn't been merged.
But please pay attention to comments from Scott about my changes of
adding 208 for some interrupts, and take some actions if needed, or
further discussions.
Below comments form Scott:
The FSL MPIC binding should be updated to point out how this works.
Technically it's not a change to the binding itself, since it's defined
in terms of register offset, but the explanatory text says So interrupt
0 is at offset 0x0, interrupt 1 is at offset 0x20, and so on. which is
not accurate for these new high interrupt numbers.
On 12/11/2013 07:19 PM, Shengzhou Liu wrote:
Add elo3-dma-2.dtsi to support the third DMA controller.
This is used on T2080, T4240, etc.
MPIC registers for internal interrupts is non-continous in address, any
internal interrupt number greater than 159 should be added (16+208) to work,
adding 16 is due to external interrupts as usual, adding 208 is due to
non-continous MPIC register space.
Signed-off-by: Shengzhou Liu shengzhou@freescale.com
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
arch/powerpc/boot/dts/fsl/elo3-dma-2.dtsi | 82 +++
1 file changed, 82 insertions(+)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-2.dtsi
diff --git a/arch/powerpc/boot/dts/fsl/elo3-dma-2.dtsi
b/arch/powerpc/boot/dts/fsl/elo3-dma-2.dtsi
new file mode 100644
index 000..d3cc8d0
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/elo3-dma-2.dtsi
@@ -0,0 +1,82 @@
+/*
+ * QorIQ Elo3 DMA device tree stub [ controller @ offset 0x102300 ]
+ *
+ * Copyright 2013 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ * names of its contributors may be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+dma2: dma@102300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
+ reg = 0x102300 0x4,
+ 0x102600 0x4;
+ ranges = 0x0 0x102100 0x500;
+ dma-channel@0 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x0 0x80;
+ interrupts = 464 2 0 0;
+ };
+ dma-channel@80 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x80 0x80;
+ interrupts = 465 2 0 0;
+ };
+ dma-channel@100 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x100 0x80;
+ interrupts = 466 2 0 0;
+ };
+ dma-channel@180 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x180 0x80;
+ interrupts = 467 2 0 0;
+ };
+ dma-channel@300 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x300 0x80;
+ interrupts = 468 2 0 0;
+ };
+ dma-channel@380 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x380 0x80;
+ interrupts = 469 2 0 0;
+ };
+ dma-channel@400 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x400 0x80;
+ interrupts = 470 2 0 0;
+ };
+ dma-channel@480 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x480 0x80;
+ interrupts = 471 2 0 0
Re: [PATCH] DTS: DMA: Fix DMA3 interrupts
Scott,
This issue is due to the non-continuous MPIC register, I think there is
two ways to fix it.
The first one is as what we are discussing, in fact the Bman/Qman DT
author had introduced this way, and I had to follow it, it is a trick,
adding 208 is a bit ugly I think, and even difficult to explain it to
customers etc, but this way changes less codes.
The second one is editing MPIC related codes without adding 208 to high
interrupts. The point of translate interrupt number to MPIC register
address is a so called 'isu' mechanism, we can do like the following
example codes, then the tricky adding 208 isn't needed any more.
Which one do you prefer?
In fact I myself prefer the second, if the idea is acceptable, I will
send a patch instead of this one. (and also alone with the internal
patch decreasing 208 for the Bman/Qman)
void __init corenet_ds_pic_init(void)
{
..
mpic = mpic_alloc(NULL, 0, flags, 0, 512, OpenPIC);
BUG_ON(mpic == NULL);
// Add this start
for (i = 0; i 17; i++) {
if (i 11)
addr_off = 0x1 + 0x20 * 16 * i;
else
addr_off = 0x13000 + 0x20 * 16 * (i - 11); /* scape the
address not for interrupts */
mpic_assign_isu(mpic, i, mpic-paddr + addr_off);
}
// Add this end
mpic_init(mpic);
}
On 12/07/2013 03:21 AM, Scott Wood wrote:
On Fri, 2013-11-29 at 16:07 +0800, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
MPIC registers for internal interrupts is non-continous in address, any
internal interrupt number greater than 159 should be added (16+208) to work.
16 is due to external interrupts as usual, 208 is due to the non-continous MPIC
register space.
Tested on T4240 rev2 with SRIO2 disabled.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
arch/powerpc/boot/dts/fsl/elo3-dma-2.dtsi | 16
1 file changed, 8 insertions(+), 8 deletions(-)
The FSL MPIC binding should be updated to point out how this works.
Technically it's not a change to the binding itself, since it's defined
in terms of register offset, but the explanatory text says So interrupt
0 is at offset 0x0, interrupt 1 is at offset 0x20, and so on. which is
not accurate for these new high interrupt numbers.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v11 0/3] DMA: Freescale: Add support for 8-channel DMA engine
On 11/13/2013 04:57 PM, Vinod Koul wrote: On Thu, Sep 26, 2013 at 05:33:40PM +0800, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Hi DMA and DT maintainers, please have a look at these V11 patches. Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch set adds support this DMA engine. Applied all Thanks. Thanks ~Vinod ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
On 11/08/2013 10:45 AM, Dan Williams wrote:
On Mon, Nov 4, 2013 at 6:31 PM, Hongbo Zhang hongbo.zh...@freescale.com wrote:
Hi Vinod Koul and Dan Williams,
Ping?
Not much to review from the dmaengine side, just one question below.
It would be helpful if you can send these to the new dmaengine
patchwork at dmaeng...@vger.kernel.org with the Acks you have already
collected.
Sorry didn't notice this new mailing list.
I will resend these patches to it again.
On 10/17/2013 01:56 PM, Hongbo Zhang wrote:
Hi Vinod,
I have gotten ACK from Mark for both the 1/3 and 2/3 patches.
Thanks.
On 09/26/2013 05:33 PM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds support to 8-channel DMA engine, thus the driver works
for both
the new 8-channel and the legacy 4-channel DMA engines.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
drivers/dma/Kconfig |9 +
drivers/dma/fsldma.c |9 ++---
drivers/dma/fsldma.h |2 +-
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6825957..3979c65 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -89,14 +89,15 @@ config AT_HDMAC
Support the Atmel AHB DMA controller.
config FSL_DMA
-tristate Freescale Elo and Elo Plus DMA support
+tristate Freescale Elo series DMA support
depends on FSL_SOC
select DMA_ENGINE
select ASYNC_TX_ENABLE_CHANNEL_SWITCH
---help---
- Enable support for the Freescale Elo and Elo Plus DMA controllers.
- The Elo is the DMA controller on some 82xx and 83xx parts, and the
- Elo Plus is the DMA controller on 85xx and 86xx parts.
+ Enable support for the Freescale Elo series DMA controllers.
+ The Elo is the DMA controller on some mpc82xx and mpc83xx parts,
the
+ EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is
on
+ some Txxx and Bxxx parts.
config MPC512X_DMA
tristate Freescale MPC512x built-in DMA engine support
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 49e8fbd..16a9a48 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct fsldma_device
*fdev,
WARN_ON(fdev-feature != chan-feature);
chan-dev = fdev-dev;
-chan-id = ((res.start - 0x100) 0xfff) 7;
+chan-id = (res.start 0xfff) 0x300 ?
+ ((res.start - 0x100) 0xfff) 7 :
+ ((res.start - 0x200) 0xfff) 7;
if (chan-id = FSL_DMA_MAX_CHANS_PER_DEVICE) {
Isn't it a bit fragile to have this based on the resource address?
Can't device tree tell you the channel id directly by an index into
the dma0: dma@100300 node?
Yes, both this way and putting a cell-index into device tree work.
This won't be fragile, because the resource address should always be
defined correctly, otherwise even if we can tell a channel id by
cell-index but with wrong resource address, nothing will work.
This piece of code only doesn't seem as neat as using cell-index, but
we prefer the style that let the device tree describes as true as what
hardware really has. This doesn't mean cell-index isn't acceptable, if
it is necessary and unavoidable, we can send another patch to add it,
but currently there is no need and we don't have to do this.
--
Dan
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
On 11/12/2013 08:09 AM, Dan Williams wrote:
On Mon, Nov 11, 2013 at 1:12 AM, Hongbo Zhang
hongbo.zh...@freescale.com wrote:
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 49e8fbd..16a9a48 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct
fsldma_device
*fdev,
WARN_ON(fdev-feature != chan-feature);
chan-dev = fdev-dev;
-chan-id = ((res.start - 0x100) 0xfff) 7;
+chan-id = (res.start 0xfff) 0x300 ?
+ ((res.start - 0x100) 0xfff) 7 :
+ ((res.start - 0x200) 0xfff) 7;
if (chan-id = FSL_DMA_MAX_CHANS_PER_DEVICE) {
Isn't it a bit fragile to have this based on the resource address?
Can't device tree tell you the channel id directly by an index into
the dma0: dma@100300 node?
Yes, both this way and putting a cell-index into device tree work.
This won't be fragile, because the resource address should always be defined
correctly, otherwise even if we can tell a channel id by cell-index but
with wrong resource address, nothing will work.
This piece of code only doesn't seem as neat as using cell-index, but we
prefer the style that let the device tree describes as true as what hardware
really has. This doesn't mean cell-index isn't acceptable, if it is
necessary and unavoidable, we can send another patch to add it, but
currently there is no need and we don't have to do this.
I'm pointing it out because we just had a bug fix to another driver
motivated by the fact that resource addresses may move from one
implementation to another, whereas the cell index provided by device
tree is static. Just a note, no need to fix it now.
Get it, and will remember it, thank you Dan.
--
Dan
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
Hi Vinod Koul and Dan Williams,
Ping?
On 10/17/2013 01:56 PM, Hongbo Zhang wrote:
Hi Vinod,
I have gotten ACK from Mark for both the 1/3 and 2/3 patches.
Thanks.
On 09/26/2013 05:33 PM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds support to 8-channel DMA engine, thus the driver
works for both
the new 8-channel and the legacy 4-channel DMA engines.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
drivers/dma/Kconfig |9 +
drivers/dma/fsldma.c |9 ++---
drivers/dma/fsldma.h |2 +-
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6825957..3979c65 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -89,14 +89,15 @@ config AT_HDMAC
Support the Atmel AHB DMA controller.
config FSL_DMA
-tristate Freescale Elo and Elo Plus DMA support
+tristate Freescale Elo series DMA support
depends on FSL_SOC
select DMA_ENGINE
select ASYNC_TX_ENABLE_CHANNEL_SWITCH
---help---
- Enable support for the Freescale Elo and Elo Plus DMA
controllers.
- The Elo is the DMA controller on some 82xx and 83xx parts, and
the
- Elo Plus is the DMA controller on 85xx and 86xx parts.
+ Enable support for the Freescale Elo series DMA controllers.
+ The Elo is the DMA controller on some mpc82xx and mpc83xx
parts, the
+ EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3
is on
+ some Txxx and Bxxx parts.
config MPC512X_DMA
tristate Freescale MPC512x built-in DMA engine support
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 49e8fbd..16a9a48 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct
fsldma_device *fdev,
WARN_ON(fdev-feature != chan-feature);
chan-dev = fdev-dev;
-chan-id = ((res.start - 0x100) 0xfff) 7;
+chan-id = (res.start 0xfff) 0x300 ?
+ ((res.start - 0x100) 0xfff) 7 :
+ ((res.start - 0x200) 0xfff) 7;
if (chan-id = FSL_DMA_MAX_CHANS_PER_DEVICE) {
dev_err(fdev-dev, too many channels for device\n);
err = -EINVAL;
@@ -1434,6 +1436,7 @@ static int fsldma_of_remove(struct
platform_device *op)
}
static const struct of_device_id fsldma_of_ids[] = {
+{ .compatible = fsl,elo3-dma, },
{ .compatible = fsl,eloplus-dma, },
{ .compatible = fsl,elo-dma, },
{}
@@ -1455,7 +1458,7 @@ static struct platform_driver fsldma_of_driver = {
static __init int fsldma_init(void)
{
-pr_info(Freescale Elo / Elo Plus DMA driver\n);
+pr_info(Freescale Elo series DMA driver\n);
return platform_driver_register(fsldma_of_driver);
}
@@ -1467,5 +1470,5 @@ static void __exit fsldma_exit(void)
subsys_initcall(fsldma_init);
module_exit(fsldma_exit);
-MODULE_DESCRIPTION(Freescale Elo / Elo Plus DMA driver);
+MODULE_DESCRIPTION(Freescale Elo series DMA driver);
MODULE_LICENSE(GPL);
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index f5c3879..1ffc244 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -112,7 +112,7 @@ struct fsldma_chan_regs {
};
struct fsldma_chan;
-#define FSL_DMA_MAX_CHANS_PER_DEVICE 4
+#define FSL_DMA_MAX_CHANS_PER_DEVICE 8
struct fsldma_device {
void __iomem *regs;/* DGSR register base */
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v11 3/3] DMA: Freescale: update driver to support 8-channel DMA engine
Hi Vinod,
I have gotten ACK from Mark for both the 1/3 and 2/3 patches.
Thanks.
On 09/26/2013 05:33 PM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds support to 8-channel DMA engine, thus the driver works for both
the new 8-channel and the legacy 4-channel DMA engines.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
drivers/dma/Kconfig |9 +
drivers/dma/fsldma.c |9 ++---
drivers/dma/fsldma.h |2 +-
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 6825957..3979c65 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -89,14 +89,15 @@ config AT_HDMAC
Support the Atmel AHB DMA controller.
config FSL_DMA
- tristate Freescale Elo and Elo Plus DMA support
+ tristate Freescale Elo series DMA support
depends on FSL_SOC
select DMA_ENGINE
select ASYNC_TX_ENABLE_CHANNEL_SWITCH
---help---
- Enable support for the Freescale Elo and Elo Plus DMA controllers.
- The Elo is the DMA controller on some 82xx and 83xx parts, and the
- Elo Plus is the DMA controller on 85xx and 86xx parts.
+ Enable support for the Freescale Elo series DMA controllers.
+ The Elo is the DMA controller on some mpc82xx and mpc83xx parts, the
+ EloPlus is on mpc85xx and mpc86xx and Pxxx parts, and the Elo3 is on
+ some Txxx and Bxxx parts.
config MPC512X_DMA
tristate Freescale MPC512x built-in DMA engine support
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 49e8fbd..16a9a48 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1261,7 +1261,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
WARN_ON(fdev-feature != chan-feature);
chan-dev = fdev-dev;
- chan-id = ((res.start - 0x100) 0xfff) 7;
+ chan-id = (res.start 0xfff) 0x300 ?
+ ((res.start - 0x100) 0xfff) 7 :
+ ((res.start - 0x200) 0xfff) 7;
if (chan-id = FSL_DMA_MAX_CHANS_PER_DEVICE) {
dev_err(fdev-dev, too many channels for device\n);
err = -EINVAL;
@@ -1434,6 +1436,7 @@ static int fsldma_of_remove(struct platform_device *op)
}
static const struct of_device_id fsldma_of_ids[] = {
+ { .compatible = fsl,elo3-dma, },
{ .compatible = fsl,eloplus-dma, },
{ .compatible = fsl,elo-dma, },
{}
@@ -1455,7 +1458,7 @@ static struct platform_driver fsldma_of_driver = {
static __init int fsldma_init(void)
{
- pr_info(Freescale Elo / Elo Plus DMA driver\n);
+ pr_info(Freescale Elo series DMA driver\n);
return platform_driver_register(fsldma_of_driver);
}
@@ -1467,5 +1470,5 @@ static void __exit fsldma_exit(void)
subsys_initcall(fsldma_init);
module_exit(fsldma_exit);
-MODULE_DESCRIPTION(Freescale Elo / Elo Plus DMA driver);
+MODULE_DESCRIPTION(Freescale Elo series DMA driver);
MODULE_LICENSE(GPL);
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index f5c3879..1ffc244 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -112,7 +112,7 @@ struct fsldma_chan_regs {
};
struct fsldma_chan;
-#define FSL_DMA_MAX_CHANS_PER_DEVICE 4
+#define FSL_DMA_MAX_CHANS_PER_DEVICE 8
struct fsldma_device {
void __iomem *regs; /* DGSR register base */
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v11 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 10/15/2013 09:38 PM, Mark Rutland wrote:
On Tue, Oct 08, 2013 at 04:22:07AM +0100, Hongbo Zhang wrote:
Hi Mark, Stephen and other DT maintainers?
The 1/3 had already been acked by Mark, and please have a further look
at this patch 2/3.
The DMA maintainer Vinod needs ack for the DT related patches so that
he can take all this patch set.
Sorry for the delay.
This looks ok to me.
Acked-by: Mark Rutland mark.rutl...@arm.com
Thanks, Mark.
On 09/26/2013 05:33 PM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt| 70 +
arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 82
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 82
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
5 files changed, 238 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index 0584168..7fc1b01 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -128,6 +128,76 @@ Example:
};
};
+** Freescale Elo3 DMA Controller
+ DMA controller which has same function as EloPlus except that Elo3 has 8
+ channels while EloPlus has only 4, it is used in Freescale Txxx and Bxxx
+ series chips, such as t1040, t4240, b4860.
+
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : contains two entries for DMA General Status Registers,
+ i.e. DGSR0 which includes status for channel 1~4, and
+ DGSR1 for channel 5~8
+- ranges: describes the mapping between the address space of the
+ DMA channels and the address space of the DMA controller
+
+- DMA channel nodes:
+- compatible: must include fsl,eloplus-dma-channel
+- reg : DMA channel specific registers
+- interrupts: interrupt specifier for DMA channel IRQ
+- interrupt-parent : optional, if needed for interrupt mapping
+
+Example:
+dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
+ reg = 0x100300 0x4,
+ 0x100600 0x4;
+ ranges = 0x0 0x100100 0x500;
+ dma-channel@0 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x0 0x80;
+ interrupts = 28 2 0 0;
+ };
+ dma-channel@80 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x80 0x80;
+ interrupts = 29 2 0 0;
+ };
+ dma-channel@100 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x100 0x80;
+ interrupts = 30 2 0 0;
+ };
+ dma-channel@180 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x180 0x80;
+ interrupts = 31 2 0 0;
+ };
+ dma-channel@300 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x300 0x80;
+ interrupts = 76 2 0 0;
+ };
+ dma-channel@380 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x380 0x80;
+ interrupts = 77 2 0 0;
+ };
+ dma-channel@400 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x400 0x80;
+ interrupts = 78 2 0 0;
+ };
+ dma-channel@480 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x480 0x80;
+ interrupts = 79 2 0 0;
+ };
+};
+
Note on DMA channel compatible properties: The compatible property must say
fsl,elo-dma-channel or fsl,eloplus-dma-channel to be used by the Elo DMA
driver (fsldma). Any DMA channel used by fsldma cannot be used by another
diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 7399154..ea53ea1 100644
--- a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
@@ -223,13 +223,13 @@
reg = 0xe2000 0x1000;
};
-/include/ qoriq-dma-0.dtsi
+/include/ elo3-dma-0.dtsi
dma@100300 {
fsl,iommu-parent = pamu0;
fsl,liodn-reg = guts 0x580; /* DMA1LIODNR */
};
-/include/ qoriq-dma-1.dtsi
+/include/ elo3-dma-1.dtsi
dma@101300 {
fsl,iommu-parent = pamu0;
fsl,liodn-reg = guts 0x584; /* DMA2LIODNR */
diff --git a/arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
b/arch/powerpc/boot/dts/fsl/elo3-dma-0
Re: [PATCH v11 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
Hi Mark, Stephen and other DT maintainers?
The 1/3 had already been acked by Mark, and please have a further look
at this patch 2/3.
The DMA maintainer Vinod needs ack for the DT related patches so that
he can take all this patch set.
On 09/26/2013 05:33 PM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt| 70 +
arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 82
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 82
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
5 files changed, 238 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index 0584168..7fc1b01 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -128,6 +128,76 @@ Example:
};
};
+** Freescale Elo3 DMA Controller
+ DMA controller which has same function as EloPlus except that Elo3 has 8
+ channels while EloPlus has only 4, it is used in Freescale Txxx and Bxxx
+ series chips, such as t1040, t4240, b4860.
+
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : contains two entries for DMA General Status Registers,
+ i.e. DGSR0 which includes status for channel 1~4, and
+ DGSR1 for channel 5~8
+- ranges: describes the mapping between the address space of the
+ DMA channels and the address space of the DMA controller
+
+- DMA channel nodes:
+- compatible: must include fsl,eloplus-dma-channel
+- reg : DMA channel specific registers
+- interrupts: interrupt specifier for DMA channel IRQ
+- interrupt-parent : optional, if needed for interrupt mapping
+
+Example:
+dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
+ reg = 0x100300 0x4,
+ 0x100600 0x4;
+ ranges = 0x0 0x100100 0x500;
+ dma-channel@0 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x0 0x80;
+ interrupts = 28 2 0 0;
+ };
+ dma-channel@80 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x80 0x80;
+ interrupts = 29 2 0 0;
+ };
+ dma-channel@100 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x100 0x80;
+ interrupts = 30 2 0 0;
+ };
+ dma-channel@180 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x180 0x80;
+ interrupts = 31 2 0 0;
+ };
+ dma-channel@300 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x300 0x80;
+ interrupts = 76 2 0 0;
+ };
+ dma-channel@380 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x380 0x80;
+ interrupts = 77 2 0 0;
+ };
+ dma-channel@400 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x400 0x80;
+ interrupts = 78 2 0 0;
+ };
+ dma-channel@480 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x480 0x80;
+ interrupts = 79 2 0 0;
+ };
+};
+
Note on DMA channel compatible properties: The compatible property must say
fsl,elo-dma-channel or fsl,eloplus-dma-channel to be used by the Elo DMA
driver (fsldma). Any DMA channel used by fsldma cannot be used by another
diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 7399154..ea53ea1 100644
--- a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
@@ -223,13 +223,13 @@
reg = 0xe2000 0x1000;
};
-/include/ qoriq-dma-0.dtsi
+/include/ elo3-dma-0.dtsi
dma@100300 {
fsl,iommu-parent = pamu0;
fsl,liodn-reg = guts 0x580; /* DMA1LIODNR */
};
-/include/ qoriq-dma-1.dtsi
+/include/ elo3-dma-1.dtsi
dma@101300 {
fsl,iommu-parent = pamu0;
fsl,liodn-reg = guts 0x584; /* DMA2LIODNR */
diff --git a/arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
b/arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
new file mode 100644
index 000..3c210e0
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
Re: [PATCH v10 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 09/25/2013 01:31 AM, Stephen Warren wrote: On 09/24/2013 04:30 AM, Hongbo Zhang wrote: On 09/24/2013 01:04 AM, Stephen Warren wrote: On 09/18/2013 04:15 AM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds the device tree nodes for them. diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt +Required properties: + +- compatible: must include fsl,elo3-dma +- reg : DMA General Status Registers, i.e. DGSR0 which contains + status for channel 1~4, and DGSR1 for channel 5~8 Is that a single entry, which is large enough to cover both registers, or a pair of entries, one per register? Reading the text, I might assume the former, but looking at the examples, it's the latter. My impression is that I cannot tell it is one larger entry or two entries by reading the description text, but the example gives the answer. Is it so important to specify it is only one entry or entries list? I prefer language as concise as possible, especially for the common properties such as reg and interrupt (eg the reg is implicitly offset and length of registers, can be continuous or not), it is difficult or unnecessary or impossible to describe much details, the example can also work as a complementary description, otherwise no need to put an example in the binding document. The description of the properties should fully describe them. The example is just an example, not a specification of the properties. It is OK for me to update the description like this: reg:containing two entries for DMA General Status Registers, i.e. DGSR0 which contains + status for channel 1~4, and DGSR1 for channel 5~8 and let me wait one or more days to see if other reviewers/maintainers have further comments before I send our another iteration. By the way, I know maybe it is difficult, but why not introduce a document of maintaining rules for the dt binding docs? we have dedicated maintainers for this part now. Description language from one submitter cannot satisfy every reviewer/maintainer, for a reg property, is it necessary to say offset and length, to say how many entries, to say register functions and even names? If there is specific rules (even with good examples), it will be convenient for both submitter and reviewers. Without rules/guidelines, new submitter would like to follow old bad samples. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v10 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 09/26/2013 10:28 AM, David Gibson wrote: On Wed, Sep 25, 2013 at 08:46:32PM -0500, Scott Wood wrote: On Wed, 2013-09-25 at 15:35 +0800, Hongbo Zhang wrote: By the way, I know maybe it is difficult, but why not introduce a document of maintaining rules for the dt binding docs? we have dedicated maintainers for this part now. Description language from one submitter cannot satisfy every reviewer/maintainer, for a reg property, is it necessary to say offset and length, Don't say offset and length. It's both redundant with the base definition of the reg property, and overly specific because it makes assumptions about how the parent node's ranges are set up (sometimes we want to be that specific, but usually not). Thanks for your answer Scott. In fact my questions are mainly sample questions to file the necessary rules of dt binding. To look at it another way, the format of the 'reg' property is defined by the parent bus's binding, not the binding of the node itself. Whatever the rule is, if it is reasonable and accepted, just as I said, we need to file it. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v10 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 09/24/2013 01:04 AM, Stephen Warren wrote:
On 09/18/2013 04:15 AM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
the device tree nodes for them.
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : DMA General Status Registers, i.e. DGSR0 which contains
+ status for channel 1~4, and DGSR1 for channel 5~8
Is that a single entry, which is large enough to cover both registers,
or a pair of entries, one per register? Reading the text, I might assume
the former, but looking at the examples, it's the latter.
My impression is that I cannot tell it is one larger entry or two
entries by reading the description text, but the example gives the answer.
Is it so important to specify it is only one entry or entries list?
I prefer language as concise as possible, especially for the common
properties such as reg and interrupt (eg the reg is implicitly offset
and length of registers, can be continuous or not), it is difficult or
unnecessary or impossible to describe much details, the example can also
work as a complementary description, otherwise no need to put an example
in the binding document.
...
+Example:
+dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
+ reg = 0x100300 0x4,
+ 0x100600 0x4;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v9 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 09/13/2013 01:15 AM, Mark Rutland wrote: On Tue, Sep 03, 2013 at 10:01:50AM +0100, Hongbo Zhang wrote: On 09/02/2013 11:58 PM, Mark Rutland wrote: Hi, On Fri, Aug 30, 2013 at 12:26:19PM +0100, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds the device tree nodes for them. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- .../devicetree/bindings/powerpc/fsl/dma.txt| 67 arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 82 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 82 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +- 5 files changed, 235 insertions(+), 4 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt index ddf17af..332ac77 100644 --- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt +++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt @@ -126,6 +126,73 @@ Example: }; }; +** Freescale Elo3 DMA Controller + This is EloPlus controller with 8 channels, used in Freescale Txxx and Bxxx I was under the impression EloPlus was the previous revision. Should that say Elo3, or is Elo3 considered to be an EloPlus implementation? In this patch 1/3 I revise the doc to make it clear we have Elo and EloPlus, and I'm adding another new Elo3. Yes the only difference between Elo3 and EloPlus is channel numbers(8 channels vs 4 channels), so we can call Elo3 is an 8-channel EloPlus Ok. + series chips, such as t1040, t4240, b4860. + +Required properties: + +- compatible: must include fsl,elo3-dma +- reg : registers specifier for DMA general status reg The example has two reg entries. What both are should be specified. From what you described last time, it sounds like each is a status register for four channels. Presumably the first covers the channels at 0x0,0x80,0x100,0x180, and the second covers the channels at 0x300,0x380,0x400,0x480? If the registers have specific names in a datasheet, it would be worth mentioning them. Yes, each is a status register for four channels, you got it -- this means my statement works. Is it necessary to specify all the register names? I can describe my two registers, but in other cases the reg entryies can cover tens even hundreds of registers, just a summary is OK I think. I think there should at least be a description of which channels each reg entry corresponds to. I see this hasn't been done so far for the older Elo DMAs, but they only had 4 channels max, and one status reg. OK, I will update the reg description to make it more clear. If the specification of the DMA controller allows for more channels, it may be worth describing that case now. This DMA controller doesn't allows for more channels. (Even if it does, it should be another new controller) Ok. +- ranges: describes the mapping between the address space of the + DMA channels and the address space of the DMA controller This looks odd as a required property, and I'm slightly confused. Is this used to map the reg values of the DMA channels, or is it used when mapping the DMA address space (for which dma-ranges exists in ePAPR and other bindings). It is used to map the reg values of DMA channels. Ok, I guess that makes sense. + +- DMA channel nodes: +- compatible: must include fsl,eloplus-dma-channel +- reg : registers specifier for channel What does this represent? What are valid values? In the example below it looks like these are offsets of control registers within the dma controller. Yes, they are offsets of control registers within dma controller, but the contents in these registers are for dma channels. Physically we have dma controller registers and dma channel registers, they are in one continuous physical address space, we divide all these registers into two controller/channel parts, according to contents in these registers, common status registers for all channels are called dma controller registers, otherwise channel specific registers are called dma channel registers. I see, so this reg represents a channels channel specific registers (which are distinct from the shared status registers). I was confused initially as to what address space they were in, but that makes sense with your description of ranges above. If the reg property may have any value, how do they get mapped to bits in the status register(s)? In fact, each channel has its own status register(and also other registers), the dma controller status register is just aggregation of all
Re: [PATCH v9 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
Mark? ping.
On 09/03/2013 05:01 PM, Hongbo Zhang wrote:
On 09/02/2013 11:58 PM, Mark Rutland wrote:
Hi,
On Fri, Aug 30, 2013 at 12:26:19PM +0100, hongbo.zh...@freescale.com
wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this
patch adds
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt| 67
arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 82
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 82
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
5 files changed, 235 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index ddf17af..332ac77 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -126,6 +126,73 @@ Example:
};
};
+** Freescale Elo3 DMA Controller
+ This is EloPlus controller with 8 channels, used in Freescale
Txxx and Bxxx
I was under the impression EloPlus was the previous revision. Should
that say Elo3, or is Elo3 considered to be an EloPlus implementation?
In this patch 1/3 I revise the doc to make it clear we have Elo and
EloPlus, and I'm adding another new Elo3. Yes the only difference
between Elo3 and EloPlus is channel numbers(8 channels vs 4 channels),
so we can call Elo3 is an 8-channel EloPlus
+ series chips, such as t1040, t4240, b4860.
+
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : registers specifier for DMA general status reg
The example has two reg entries. What both are should be specified. From
what you described last time, it sounds like each is a status register
for four channels.
Presumably the first covers the channels at 0x0,0x80,0x100,0x180, and
the second covers the channels at 0x300,0x380,0x400,0x480? If the
registers have specific names in a datasheet, it would be worth
mentioning them.
Yes, each is a status register for four channels, you got it -- this
means my statement works.
Is it necessary to specify all the register names?
I can describe my two registers, but in other cases the reg entryies
can cover tens even hundreds of registers, just a summary is OK I think.
If the specification of the DMA controller allows for more channels, it
may be worth describing that case now.
This DMA controller doesn't allows for more channels. (Even if it
does, it should be another new controller)
+- ranges: describes the mapping between the address
space of the
+ DMA channels and the address space of the DMA
controller
This looks odd as a required property, and I'm slightly confused. Is
this used to map the reg values of the DMA channels, or is it used when
mapping the DMA address space (for which dma-ranges exists in ePAPR and
other bindings).
It is used to map the reg values of DMA channels.
+
+- DMA channel nodes:
+- compatible: must include fsl,eloplus-dma-channel
+- reg : registers specifier for channel
What does this represent? What are valid values?
In the example below it looks like these are offsets of control
registers within the dma controller.
Yes, they are offsets of control registers within dma controller, but
the contents in these registers are for dma channels.
Physically we have dma controller registers and dma channel registers,
they are in one continuous physical address space, we divide all these
registers into two controller/channel parts, according to contents in
these registers, common status registers for all channels are called
dma controller registers, otherwise channel specific registers are
called dma channel registers.
If the reg property may have any value, how do they get mapped to bits
in the status register(s)?
In fact, each channel has its own status register(and also other
registers), the dma controller status register is just aggregation of
all channel status register. (that seems duplicated somehow, maybe
this is due to hardware compatibility with legacy one, and the device
tree just describes the physical hardware without lie)
May some channels be unusable for some reason, or will all eight
channels be wired on any given Elo3 DMA?
Sorry, not get your point clearly, maybe you are clear now because of
my previous explanations.
Cheers,
Mark.
+- interrupts: interrupt specifier for DMA channel
IRQ
+- interrupt-parent : optional, if needed for interrupt
mapping
+
+Example:
+dma@100300 {
+ #address-cells = 1
Re: [PATCH v9 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 09/02/2013 11:58 PM, Mark Rutland wrote:
Hi,
On Fri, Aug 30, 2013 at 12:26:19PM +0100, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt| 67
arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 82
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 82
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
5 files changed, 235 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index ddf17af..332ac77 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -126,6 +126,73 @@ Example:
};
};
+** Freescale Elo3 DMA Controller
+ This is EloPlus controller with 8 channels, used in Freescale Txxx and Bxxx
I was under the impression EloPlus was the previous revision. Should
that say Elo3, or is Elo3 considered to be an EloPlus implementation?
In this patch 1/3 I revise the doc to make it clear we have Elo and
EloPlus, and I'm adding another new Elo3. Yes the only difference
between Elo3 and EloPlus is channel numbers(8 channels vs 4 channels),
so we can call Elo3 is an 8-channel EloPlus
+ series chips, such as t1040, t4240, b4860.
+
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : registers specifier for DMA general status reg
The example has two reg entries. What both are should be specified. From
what you described last time, it sounds like each is a status register
for four channels.
Presumably the first covers the channels at 0x0,0x80,0x100,0x180, and
the second covers the channels at 0x300,0x380,0x400,0x480? If the
registers have specific names in a datasheet, it would be worth
mentioning them.
Yes, each is a status register for four channels, you got it -- this
means my statement works.
Is it necessary to specify all the register names?
I can describe my two registers, but in other cases the reg entryies can
cover tens even hundreds of registers, just a summary is OK I think.
If the specification of the DMA controller allows for more channels, it
may be worth describing that case now.
This DMA controller doesn't allows for more channels. (Even if it does,
it should be another new controller)
+- ranges: describes the mapping between the address space of the
+ DMA channels and the address space of the DMA controller
This looks odd as a required property, and I'm slightly confused. Is
this used to map the reg values of the DMA channels, or is it used when
mapping the DMA address space (for which dma-ranges exists in ePAPR and
other bindings).
It is used to map the reg values of DMA channels.
+
+- DMA channel nodes:
+- compatible: must include fsl,eloplus-dma-channel
+- reg : registers specifier for channel
What does this represent? What are valid values?
In the example below it looks like these are offsets of control
registers within the dma controller.
Yes, they are offsets of control registers within dma controller, but
the contents in these registers are for dma channels.
Physically we have dma controller registers and dma channel registers,
they are in one continuous physical address space, we divide all these
registers into two controller/channel parts, according to contents in
these registers, common status registers for all channels are called dma
controller registers, otherwise channel specific registers are called
dma channel registers.
If the reg property may have any value, how do they get mapped to bits
in the status register(s)?
In fact, each channel has its own status register(and also other
registers), the dma controller status register is just aggregation of
all channel status register. (that seems duplicated somehow, maybe this
is due to hardware compatibility with legacy one, and the device tree
just describes the physical hardware without lie)
May some channels be unusable for some reason, or will all eight
channels be wired on any given Elo3 DMA?
Sorry, not get your point clearly, maybe you are clear now because of my
previous explanations.
Cheers,
Mark.
+- interrupts: interrupt specifier for DMA channel IRQ
+- interrupt-parent : optional, if needed for interrupt mapping
+
+Example:
+dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma
Re: [PATCH v8 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 08/27/2013 07:35 PM, Mark Rutland wrote:
On Tue, Aug 27, 2013 at 11:42:02AM +0100, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt| 66
arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 81
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 81
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
5 files changed, 232 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index ddf17af..10fd031 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -126,6 +126,72 @@ Example:
};
};
+** Freescale Elo3 DMA Controller
+ This is EloPlus controller with 8 channels, used in Freescale Txxx and Bxxx
+ series chips, such as t1040, t4240, b4860.
+
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : registers specifier for DMA general status reg
+- ranges: describes the mapping between the address space of the
+ DMA channels and the address space of the DMA controller
+
+- DMA channel nodes:
+- compatible: must include fsl,eloplus-dma-channel
+- reg : registers specifier for channel
+- interrupts: interrupt specifier for DMA channel IRQ
+- interrupt-parent : optional, if needed for interrupt mapping
+
+Example:
+dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
+ reg = 0x100300 0x4 0x100600 0x4;
Is that one reg entry where #size-cells=2 and #address-cells=2?
That's what the binding implies (given it only describes a single reg
entry).
if it's two entries, we should make that explicit (both in the binding
and example):
reg = 0x100300 0x4,
0x100600 0x4;
Yes they are two entries, I will change it this way.
+ ranges = 0x0 0x100100 0x500;
If it is one reg entry then the example ranges property isn't big enough
to contain the parent-bus-address.
They are two reg entries, so the range is big enough.
+ dma-channel@0 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x0 0x80;
+ interrupts = 28 2 0 0;
+ };
+ dma-channel@80 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x80 0x80;
+ interrupts = 29 2 0 0;
+ };
+ dma-channel@100 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x100 0x80;
+ interrupts = 30 2 0 0;
+ };
+ dma-channel@180 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x180 0x80;
+ interrupts = 31 2 0 0;
+ };
+ dma-channel@300 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x300 0x80;
+ interrupts = 76 2 0 0;
+ };
+ dma-channel@380 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x380 0x80;
+ interrupts = 77 2 0 0;
+ };
+ dma-channel@400 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x400 0x80;
+ interrupts = 78 2 0 0;
+ };
+ dma-channel@480 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x480 0x80;
+ interrupts = 79 2 0 0;
+ };
+};
+
Note on DMA channel compatible properties: The compatible property must say
fsl,elo-dma-channel or fsl,eloplus-dma-channel to be used by the Elo DMA
driver (fsldma). Any DMA channel used by fsldma cannot be used by another
diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 7399154..ea53ea1 100644
--- a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
+++ b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
@@ -223,13 +223,13 @@
reg = 0xe2000 0x1000;
};
-/include/ qoriq-dma-0.dtsi
+/include/ elo3-dma-0.dtsi
dma@100300 {
fsl,iommu-parent = pamu0;
fsl,liodn-reg = guts 0x580; /* DMA1LIODNR */
};
-/include/ qoriq-dma-1.dtsi
+/include/ elo3-dma-1.dtsi
dma@101300 {
fsl,iommu-parent = pamu0;
fsl,liodn-reg = guts 0x584; /* DMA2LIODNR */
diff --git a/arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
b/arch/powerpc/boot/dts/fsl/elo3-dma-0
Re: [PATCH v8 1/3] DMA: Freescale: revise device tree binding document
On 08/27/2013 07:25 PM, Mark Rutland wrote: On Tue, Aug 27, 2013 at 11:42:01AM +0100, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com This patch updates the discription of each type of DMA controller and its channels, it is preparation for adding another new DMA controller binding, it also fixes some defects of indent for text alignment at the same time. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- .../devicetree/bindings/powerpc/fsl/dma.txt| 62 +--- 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt index 2a4b4bc..ddf17af 100644 --- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt +++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt @@ -1,33 +1,29 @@ -* Freescale 83xx DMA Controller +* Freescale DMA Controllers -Freescale PowerPC 83xx have on chip general purpose DMA controllers. +** Freescale Elo DMA Controller + This is a little-endian DMA controller, used in Freescale mpc83xx series + chips such as mpc8315, mpc8349, mpc8379 etc. Required properties: -- compatible: compatible list, contains 2 entries, first is -fsl,CHIP-dma, where CHIP is the processor -(mpc8349, mpc8360, etc.) and the second is -fsl,elo-dma -- reg : registers mapping for DMA general status reg -- ranges : Should be defined as specified in 1) to describe the - DMA controller channels. +- compatible: must include fsl,elo-dma We should list the other values that may be in the list also, unless they are really of no consequence, in which case their presence in dt is questionable. Hmm. Stephen questioned here too, it seems this is a default rule. Although Scott@freescale had explained our thoughts, I'd like to edit this item like this: must include fsl,eloplus-dma, and a fsl,CHIP-dma is optional, where CHIP is the processor name We don't list all the chip name because we have tens of them and we cannot list all of them, and it is unnecessary to list them because we even don't use fsl,CHIP-dma in the new driver, add fsl,CHIP-dma here just make it questionable when it presents in example and old dts files. I remove the examples in bracket (mpc8349, mpc8360, etc.) because we can see the real example below. I don't say if fsl,CHIP-dma presents, it should be the first one, and the fsl,eloplus-dma should be the second because it is common rule. the description language should be clear and concise too I think. +- reg : registers specifier for DMA general status reg +- ranges: describes the mapping between the address space of the + DMA channels and the address space of the DMA controller - cell-index: controller index. 0 for controller @ 0x8100 -- interrupts: interrupt mapping for DMA IRQ +- interrupts: interrupt specifier for DMA IRQ - interrupt-parent : optional, if needed for interrupt mapping - - DMA channel nodes: -- compatible: compatible list, contains 2 entries, first is -fsl,CHIP-dma-channel, where CHIP is the processor -(mpc8349, mpc8350, etc.) and the second is -fsl,elo-dma-channel. However, see note below. -- reg : registers mapping for channel +- compatible: must include fsl,elo-dma-channel + However, see note below. Again, I think we should list the other entries that may be in the list. Otherwise it's not clear what the binding defines. Similarly for the other compatible list definitions below... +- reg : registers specifier for channel - cell-index: dma channel index starts at 0. I realise you haven't changed it, but it's unclear what the cell-index property is (and somewhat confusingly there seem to be multiple defnitions). It might be worth clarifying it while performing the other cleanup. not clear with your point multiple definitions, we really have multiple dma channels for one dma controller. cell-index is used as channel index, this is an old method used by old driver, my patch didn't touch this part. Optional properties: -- interrupts: interrupt mapping for DMA channel IRQ - (on 83xx this is expected to be identical to - the interrupts property of the parent node) +- interrupts: interrupt specifier for DMA channel IRQ + (on 83xx this is expected to be identical to + the interrupts property of the parent node) - interrupt-parent : optional, if needed for interrupt mapping Example: @@ -70,30 +66,26 @@ Example
Re: [PATCH v8 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 08/28/2013 08:51 PM, Mark Rutland wrote:
On Wed, Aug 28, 2013 at 07:54:01AM +0100, Hongbo Zhang wrote:
On 08/27/2013 07:35 PM, Mark Rutland wrote:
On Tue, Aug 27, 2013 at 11:42:02AM +0100, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
.../devicetree/bindings/powerpc/fsl/dma.txt| 66
arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +-
arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 81
arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 81
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +-
5 files changed, 232 insertions(+), 4 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi
diff --git a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
index ddf17af..10fd031 100644
--- a/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
+++ b/Documentation/devicetree/bindings/powerpc/fsl/dma.txt
@@ -126,6 +126,72 @@ Example:
};
};
+** Freescale Elo3 DMA Controller
+ This is EloPlus controller with 8 channels, used in Freescale Txxx and Bxxx
+ series chips, such as t1040, t4240, b4860.
+
+Required properties:
+
+- compatible: must include fsl,elo3-dma
+- reg : registers specifier for DMA general status reg
+- ranges: describes the mapping between the address space of the
+ DMA channels and the address space of the DMA controller
+
+- DMA channel nodes:
+- compatible: must include fsl,eloplus-dma-channel
+- reg : registers specifier for channel
+- interrupts: interrupt specifier for DMA channel IRQ
+- interrupt-parent : optional, if needed for interrupt mapping
+
+Example:
+dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,elo3-dma;
+ reg = 0x100300 0x4 0x100600 0x4;
Is that one reg entry where #size-cells=2 and #address-cells=2?
That's what the binding implies (given it only describes a single reg
entry).
if it's two entries, we should make that explicit (both in the binding
and example):
reg = 0x100300 0x4,
0x100600 0x4;
Yes they are two entries, I will change it this way.
Ok. Could you make sure you document what the two reg entries correspond
to? That's not clear from registers specifier for channel.
Yes I am sure, we have reg for DMA controller and also reg for each DMA
channel.
these two reg entries are registers specifier for DMA general status
reg, not registers specifier for channel
because this is an 8-channel DMA controller, we have two general status
registers (vs. one status register for 4-chanel DMA controller previously )
+ ranges = 0x0 0x100100 0x500;
If it is one reg entry then the example ranges property isn't big enough
to contain the parent-bus-address.
They are two reg entries, so the range is big enough.
Ok.
+ dma-channel@0 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x0 0x80;
+ interrupts = 28 2 0 0;
+ };
+ dma-channel@80 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x80 0x80;
+ interrupts = 29 2 0 0;
+ };
+ dma-channel@100 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x100 0x80;
+ interrupts = 30 2 0 0;
+ };
+ dma-channel@180 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x180 0x80;
+ interrupts = 31 2 0 0;
+ };
+ dma-channel@300 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x300 0x80;
+ interrupts = 76 2 0 0;
+ };
+ dma-channel@380 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x380 0x80;
+ interrupts = 77 2 0 0;
+ };
+ dma-channel@400 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x400 0x80;
+ interrupts = 78 2 0 0;
+ };
+ dma-channel@480 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x480 0x80;
+ interrupts = 79 2 0 0;
+ };
+};
+
Note on DMA channel compatible properties: The compatible property must say
fsl,elo-dma-channel or fsl,eloplus-dma-channel to be used by the Elo DMA
driver (fsldma). Any DMA channel used by fsldma cannot be used by another
diff --git a/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
b/arch/powerpc/boot/dts/fsl/b4si-post.dtsi
index 7399154..ea53ea1 100644
--- a/arch/powerpc/boot/dts/fsl/b4si
Re: [PATCH v7 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 08/23/2013 11:17 AM, Hongbo Zhang wrote: On 08/22/2013 07:16 AM, Stephen Warren wrote: On 08/21/2013 05:00 PM, Scott Wood wrote: On Wed, 2013-08-21 at 16:40 -0600, Stephen Warren wrote: On 07/29/2013 04:49 AM, hongbo.zh...@freescale.com wrote: +- reg : registers mapping for channel +- interrupts: interrupt mapping for DMA channel IRQ s/interrupts/specifier/ Do you mean s/interrupt mapping/interrupt specifier/? And probably s/registers mapping/register specifier/ as well. Yup. OK, I will update these descriptions. Since Scott has clarified all the doubts, and no further comment till now, so my next iteration will include this s/mapping/specifier only. I will sent it out this Tuesday, if there is still any comment/doubt, please let me know. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 08/22/2013 07:16 AM, Stephen Warren wrote: On 08/21/2013 05:00 PM, Scott Wood wrote: On Wed, 2013-08-21 at 16:40 -0600, Stephen Warren wrote: On 07/29/2013 04:49 AM, hongbo.zh...@freescale.com wrote: +- reg : registers mapping for channel +- interrupts: interrupt mapping for DMA channel IRQ s/interrupts/specifier/ Do you mean s/interrupt mapping/interrupt specifier/? And probably s/registers mapping/register specifier/ as well. Yup. OK, I will update these descriptions. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 0/3] DMA: Freescale: Add support for 8-channel DMA engine
Hi DT maintainers, Rob Herring, Pawel Moll, Mark Rutland, Stephen Warren, Ian Campbell, could you please have a look at [1/3] and [2/3] of these patch set. These patches have been fully reviewed by Scott Wood, and the DMA maintainer Vinod needs a Acted-by: from DT maintainers. Thanks. On 08/20/2013 04:15 PM, Vinod Koul wrote: On Tue, Aug 20, 2013 at 04:33:46PM +0800, Hongbo Zhang wrote: On 07/29/2013 06:59 PM, Vinod Koul wrote: On Mon, Jul 29, 2013 at 06:49:01PM +0800, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Hi Vinod, Dan, Scott and Leo, please have a look at these V7 patches. The dma relates changes look okay to me. I need someone to review and ACK the DT bindings. ~Vinod Vinod, Are you using this tree? http://git.infradead.org/users/vkoul/slave-dma.git Yes Did you merge these patches? No As I said I would like someone who know DT and dma binding to ack them. I see devicetree ML has been cced, can Arnd or someone else review these... ~Vinod ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 0/3] DMA: Freescale: Add support for 8-channel DMA engine
On 07/29/2013 06:59 PM, Vinod Koul wrote: On Mon, Jul 29, 2013 at 06:49:01PM +0800, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Hi Vinod, Dan, Scott and Leo, please have a look at these V7 patches. The dma relates changes look okay to me. I need someone to review and ACK the DT bindings. ~Vinod Vinod, Are you using this tree? http://git.infradead.org/users/vkoul/slave-dma.git Did you merge these patches? Thanks. Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch set adds support this DMA engine. V6-V7 changes: - only remove unnecessary CHIP-dma explanations in [1/3] V5-V6 changes: - minor updates of descriptions in binding document and Kconfig - remove [4/4], that should be another patch in future V4-V5 changes: - update description in the dt binding document, to make it more resonable - add new patch [4/4] to eliminate a compiling warning which already exists for a long time V3-V4 changes: - introduce new patch [1/3] to revise the legacy dma binding document - and then add new paragraph to describe new dt node binding in [2/3] - rebase to latest kernel v3.11-rc1 V2-V3 changes: - edit Documentation/devicetree/bindings/powerpc/fsl/dma.txt - edit text string in Kconfig and the driver files, using elo series to mention all the current elo* V1-V2 changes: - removed the codes handling the register dgsr1, since it isn't used currently - renamed the DMA DT compatible to fsl,elo3-dma - renamed the new dts files to elo3-dma-n.dtsi Hongbo Zhang (3): DMA: Freescale: revise device tree binding document DMA: Freescale: Add new 8-channel DMA engine device tree nodes DMA: Freescale: update driver to support 8-channel DMA engine .../devicetree/bindings/powerpc/fsl/dma.txt| 114 +++- arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 81 ++ arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 81 ++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +- drivers/dma/Kconfig|9 +- drivers/dma/fsldma.c |9 +- drivers/dma/fsldma.h |2 +- 8 files changed, 264 insertions(+), 40 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 0/3] DMA: Freescale: Add support for 8-channel DMA engine
On 07/29/2013 06:59 PM, Vinod Koul wrote: On Mon, Jul 29, 2013 at 06:49:01PM +0800, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Hi Vinod, Dan, Scott and Leo, please have a look at these V7 patches. The dma relates changes look okay to me. I need someone to review and ACK the DT bindings. Scott Wood has ACKed the [1/3] and [2/3]. Thank you Vinod. ~Vinod Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch set adds support this DMA engine. V6-V7 changes: - only remove unnecessary CHIP-dma explanations in [1/3] V5-V6 changes: - minor updates of descriptions in binding document and Kconfig - remove [4/4], that should be another patch in future V4-V5 changes: - update description in the dt binding document, to make it more resonable - add new patch [4/4] to eliminate a compiling warning which already exists for a long time V3-V4 changes: - introduce new patch [1/3] to revise the legacy dma binding document - and then add new paragraph to describe new dt node binding in [2/3] - rebase to latest kernel v3.11-rc1 V2-V3 changes: - edit Documentation/devicetree/bindings/powerpc/fsl/dma.txt - edit text string in Kconfig and the driver files, using elo series to mention all the current elo* V1-V2 changes: - removed the codes handling the register dgsr1, since it isn't used currently - renamed the DMA DT compatible to fsl,elo3-dma - renamed the new dts files to elo3-dma-n.dtsi Hongbo Zhang (3): DMA: Freescale: revise device tree binding document DMA: Freescale: Add new 8-channel DMA engine device tree nodes DMA: Freescale: update driver to support 8-channel DMA engine .../devicetree/bindings/powerpc/fsl/dma.txt| 114 +++- arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 81 ++ arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 81 ++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +- drivers/dma/Kconfig|9 +- drivers/dma/fsldma.c |9 +- drivers/dma/fsldma.h |2 +- 8 files changed, 264 insertions(+), 40 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi -- 1.7.9.5 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v7 2/3] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/30/2013 06:10 AM, Scott Wood wrote: On 07/29/2013 05:49:03 AM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch adds the device tree nodes for them. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- .../devicetree/bindings/powerpc/fsl/dma.txt| 66 arch/powerpc/boot/dts/fsl/b4si-post.dtsi |4 +- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 81 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 81 arch/powerpc/boot/dts/fsl/t4240si-post.dtsi|4 +- 5 files changed, 232 insertions(+), 4 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi ACK Thank you Scott for all the review comments. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v5 4/4] DMA: Freescale: eliminate a compiling warning
On 07/25/2013 03:33 AM, Scott Wood wrote: On 07/24/2013 01:21:09 AM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com The variable cookie is initialized in a list_for_each_entry loop, if(unlikely) the list is empty, this variable will be used uninitialized, so we get a gcc compiling warning about this. This patch fixes this defect by setting an initial value to the varialble cookie. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- drivers/dma/fsldma.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c index 16a9a48..14d68a4 100644 --- a/drivers/dma/fsldma.c +++ b/drivers/dma/fsldma.c @@ -406,7 +406,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx) struct fsl_desc_sw *desc = tx_to_fsl_desc(tx); struct fsl_desc_sw *child; unsigned long flags; -dma_cookie_t cookie; +dma_cookie_t cookie = 0; spin_lock_irqsave(chan-desc_lock, flags); This patch is unrelated to the rest of the patch series... What are the semantics of this function if there are multiple entries in the list? Returning the last cookie seems a bit odd. Is zero the proper error value? include/linux/dmaengine.h suggests that cookies should be 0 to indicate error. I found this compiling warning since the beginning of this work, it is better somebody fixes it sooner or later, so I take it at last. Yes it was a bit hard to define the initial value, I saw the dmaengine.h, and I searched all the other DMA drivers with initial value before making the decision: drivers/dma/mv_xor.c:dma_cookie_t cookie = 0; drivers/dma/sh/shdma-base.c:dma_cookie_t cookie = 0; drivers/dma/mmp_pdma.c:dma_cookie_t cookie = -EBUSY; drivers/dma/ppc4xx/adma.c:dma_cookie_t cookie = 0; drivers/dma/iop-adma.c:dma_cookie_t cookie = 0; most of them using 0, and only one negative value, it seems better? but -EBUSY isn't so accurate I think. My thought is to drop this in the next iteration, and back to this after the first 3 get merged. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/15/2013 09:31 PM, Kumar Gala wrote: On Jul 5, 2013, at 1:27 AM, hongbo.zh...@freescale.com hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch add the device tree nodes for them. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +- 3 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi Why didn't you update b4si-post.dtsi as well? OK, will update it too, thanks. - k ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH V2 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/10/2013 12:48 AM, Scott Wood wrote: On 07/05/2013 01:27:05 AM, hongbo.zh...@freescale.com wrote: From: Hongbo Zhang hongbo.zh...@freescale.com Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch add the device tree nodes for them. Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com --- arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi | 90 +++ arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +- 3 files changed, 182 insertions(+), 2 deletions(-) create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-0.dtsi create mode 100644 arch/powerpc/boot/dts/fsl/elo3-dma-1.dtsi Please update Documentation/devicetree/bindings/powerpc/fsl/dma.txt for the new compatible and dgsr1. OK, thanks. What's more, some text string in the driver and Kconfig files should be updated too, e.g. Elo / Elo Plus DMA may be changed to Elo series DMA, will send out v3 patches soon. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
On 07/03/2013 11:53 AM, Hongbo Zhang wrote:
hmm...add the devicetree-disc...@lists.ozlabs.org into list.
Note that we are discussing a better naming for this new compatible
property in the corresponding [PATCH 2/2], so I will resend a v2 of
this patch.
On 07/01/2013 11:46 AM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this
patch add
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi | 90
+++
arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi | 90
+++
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +-
3 files changed, 182 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
Scott, any comment of these two file names?
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
b/arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
new file mode 100644
index 000..c626c49
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
@@ -0,0 +1,90 @@
+/*
+ * QorIQ DMA device tree stub [ controller @ offset 0x10 ]
+ *
+ * Copyright 2011-2013 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following
conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
copyright
+ * notice, this list of conditions and the following
disclaimer in the
+ * documentation and/or other materials provided with the
distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ * names of its contributors may be used to endorse or promote
products
+ * derived from this software without specific prior written
permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms
of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS''
AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE
FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+dma0: dma@100300 {
+#address-cells = 1;
+#size-cells = 1;
+compatible = fsl,eloplus-dma2;
+reg = 0x100300 0x4 0x100600 0x4;
+ranges = 0x0 0x100100 0x500;
+cell-index = 0;
+dma-channel@0 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x0 0x80;
+cell-index = 0;
+interrupts = 28 2 0 0;
+};
+dma-channel@80 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x80 0x80;
+cell-index = 1;
+interrupts = 29 2 0 0;
+};
+dma-channel@100 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x100 0x80;
+cell-index = 2;
+interrupts = 30 2 0 0;
+};
+dma-channel@180 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x180 0x80;
+cell-index = 3;
+interrupts = 31 2 0 0;
+};
+dma-channel@300 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x300 0x80;
+cell-index = 4;
+interrupts = 76 2 0 0;
+};
+dma-channel@380 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x380 0x80;
+cell-index = 5;
+interrupts = 77 2 0 0;
+};
+dma-channel@400 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x400 0x80;
+cell-index = 6;
+interrupts = 78 2 0 0;
+};
+dma-channel@480 {
+compatible = fsl,eloplus-dma-channel;
+reg = 0x480 0x80;
+cell-index = 7;
+interrupts = 79 2 0 0;
+};
+};
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
b/arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
new file mode 100644
index 000..980ea77
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
@@ -0,0 +1,90 @@
+/*
+ * QorIQ DMA device tree stub [ controller @ offset 0x101000 ]
+ *
+ * Copyright 2011-2013 Freescale Semiconductor Inc
Re: [PATCH 2/2] DMA: Freescale: update driver to support 8-channel DMA engine
On 07/03/2013 07:13 AM, Scott Wood wrote:
On 06/30/2013 10:46:18 PM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
This patch adds support to 8-channel DMA engine, thus the driver
works for both
the new 8-channel and the legacy 4-channel DMA engines.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
drivers/dma/fsldma.c | 48
++--
drivers/dma/fsldma.h |4 ++--
2 files changed, 36 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 4fc2980..0f453ea 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -1119,27 +1119,33 @@ static irqreturn_t fsldma_ctrl_irq(int irq,
void *data)
struct fsldma_device *fdev = data;
struct fsldma_chan *chan;
unsigned int handled = 0;
-u32 gsr, mask;
+u8 chan_sr[round_up(FSL_DMA_MAX_CHANS_PER_DEVICE, 4)];
+u32 gsr;
int i;
-gsr = (fdev-feature FSL_DMA_BIG_ENDIAN) ? in_be32(fdev-regs)
- : in_le32(fdev-regs);
-mask = 0xff00;
-dev_dbg(fdev-dev, IRQ: gsr 0x%.8x\n, gsr);
+memset(chan_sr, 0, sizeof(chan_sr));
+gsr = (fdev-feature FSL_DMA_BIG_ENDIAN) ? in_be32(fdev-regs0)
+ : in_le32(fdev-regs0);
+memcpy(chan_sr[0], gsr, 4);
+dev_dbg(fdev-dev, IRQ: gsr0 0x%.8x\n, gsr);
+
+if (of_device_is_compatible(fdev-dev-of_node,
fsl,eloplus-dma2)) {
NACK; Figure out what sort of device you've got when you first probe
the device, and store the information for later. Do not call device
tree stuff in an interrupt handler.
+gsr = (fdev-feature FSL_DMA_BIG_ENDIAN) ?
+in_be32(fdev-regs1) : in_le32(fdev-regs1);
+memcpy(chan_sr[4], gsr, 4);
+dev_dbg(fdev-dev, IRQ: gsr1 0x%.8x\n, gsr);
+}
Do these memcpy()s get inlined? If not (and maybe even if they do),
it'd be better to use a union instead.
For this and the first comments: good catches, thank you.
But it is very likely I will remove these codes, see the last comments
of yours and mine.
Wait a second -- how are we even getting into this code on these new
DMA controllers? All 85xx-family DMA controllers use fsldma_chan_irq
directly.
Right, we are using fsldma_chan_irq, this code never run.
I just see there is such code for elo/eloplus DMA controllers, so I
update it for the new 8-channel DMA.
@@ -1341,13 +1349,22 @@ static int fsldma_of_probe(struct
platform_device *op)
INIT_LIST_HEAD(fdev-common.channels);
/* ioremap the registers for use */
-fdev-regs = of_iomap(op-dev.of_node, 0);
-if (!fdev-regs) {
-dev_err(op-dev, unable to ioremap registers\n);
+fdev-regs0 = of_iomap(op-dev.of_node, 0);
+if (!fdev-regs0) {
+dev_err(op-dev, unable to ioremap register0\n);
err = -ENOMEM;
goto out_free_fdev;
}
+if (of_device_is_compatible(op-dev.of_node, fsl,eloplus-dma2)) {
Not fsl,eloplusplus-dma? :-)
More seriously, if we're sticking with this elo naming, maybe
fsl,elo3-dma would be better. It would be odd to have 2 in the
name of the third generation of this hardware.
It was really hard for me to name this new controller.
Yes fsl,elo3-dma seems better.
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index f5c3879..880664d 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -112,10 +112,10 @@ struct fsldma_chan_regs {
};
struct fsldma_chan;
-#define FSL_DMA_MAX_CHANS_PER_DEVICE 4
+#define FSL_DMA_MAX_CHANS_PER_DEVICE 8
struct fsldma_device {
-void __iomem *regs;/* DGSR register base */
+void __iomem *regs0, *regs1;/* DGSR registers */
Either give these meaningful names, or use an array. Or both (dgsr[2]).
Or just get rid of this, since I don't see why we need DGSR1 at all,
as previously noted.
I choose the names regs* just to follow the previous pattern.
Here comes the key point: both the previous DGSR and the new DGSR0/DGSR1
are not actually used because we are using per channel irq.
I see we had such codes to handle DGSR, so I just follow the same
pattern to handle the new DGSR0/DGSR1.
Since getting rid of this unused DGSR1 is permitted, I'd like to remove
all the related codes, then this patch becomes simple :)
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] DMA: Freescale: Add new 8-channel DMA engine device tree nodes
hmm...add the devicetree-disc...@lists.ozlabs.org into list.
Note that we are discussing a better naming for this new compatible
property in the corresponding [PATCH 2/2], so I will resend a v2 of this
patch.
On 07/01/2013 11:46 AM, hongbo.zh...@freescale.com wrote:
From: Hongbo Zhang hongbo.zh...@freescale.com
Freescale QorIQ T4 and B4 introduce new 8-channel DMA engines, this patch add
the device tree nodes for them.
Signed-off-by: Hongbo Zhang hongbo.zh...@freescale.com
---
arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi | 90 +++
arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi | 90 +++
arch/powerpc/boot/dts/fsl/t4240si-post.dtsi |4 +-
3 files changed, 182 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
create mode 100644 arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
b/arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
new file mode 100644
index 000..c626c49
--- /dev/null
+++ b/arch/powerpc/boot/dts/fsl/qoriq-dma2-0.dtsi
@@ -0,0 +1,90 @@
+/*
+ * QorIQ DMA device tree stub [ controller @ offset 0x10 ]
+ *
+ * Copyright 2011-2013 Freescale Semiconductor Inc.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in the
+ * documentation and/or other materials provided with the distribution.
+ * * Neither the name of Freescale Semiconductor nor the
+ * names of its contributors may be used to endorse or promote products
+ * derived from this software without specific prior written permission.
+ *
+ *
+ * ALTERNATIVELY, this software may be distributed under the terms of the
+ * GNU General Public License (GPL) as published by the Free Software
+ * Foundation, either version 2 of that License or (at your option) any
+ * later version.
+ *
+ * THIS SOFTWARE IS PROVIDED BY Freescale Semiconductor ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL Freescale Semiconductor BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+dma0: dma@100300 {
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = fsl,eloplus-dma2;
+ reg = 0x100300 0x4 0x100600 0x4;
+ ranges = 0x0 0x100100 0x500;
+ cell-index = 0;
+ dma-channel@0 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x0 0x80;
+ cell-index = 0;
+ interrupts = 28 2 0 0;
+ };
+ dma-channel@80 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x80 0x80;
+ cell-index = 1;
+ interrupts = 29 2 0 0;
+ };
+ dma-channel@100 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x100 0x80;
+ cell-index = 2;
+ interrupts = 30 2 0 0;
+ };
+ dma-channel@180 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x180 0x80;
+ cell-index = 3;
+ interrupts = 31 2 0 0;
+ };
+ dma-channel@300 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x300 0x80;
+ cell-index = 4;
+ interrupts = 76 2 0 0;
+ };
+ dma-channel@380 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x380 0x80;
+ cell-index = 5;
+ interrupts = 77 2 0 0;
+ };
+ dma-channel@400 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x400 0x80;
+ cell-index = 6;
+ interrupts = 78 2 0 0;
+ };
+ dma-channel@480 {
+ compatible = fsl,eloplus-dma-channel;
+ reg = 0x480 0x80;
+ cell-index = 7;
+ interrupts = 79 2 0 0;
+ };
+};
diff --git a/arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
b/arch/powerpc/boot/dts/fsl/qoriq-dma2-1.dtsi
new file mode 100644
index 000..980ea77
--- /dev/null
+++ b/arch/powerpc/boot
