* [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 13:47 ` Vinod Koul
2013-01-09 8:17 ` [resend][PATCH 02/16] dw_dmac: absence of pdata isn't critical when autocfg is set Andy Shevchenko
` (15 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
If we don't yet have the platform device for the driver when it is being loaded
we fail to probe the driver. So instead of calling probe() directly we call
platform_driver_register(). It will call the probe() immediately if we have the
device but also makes the driver to work on platforms where the platform device
is created later.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 476e9c8..b5f21ba 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1829,6 +1829,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
#endif
static struct platform_driver dw_driver = {
+ .probe = dw_probe,
.remove = dw_remove,
.shutdown = dw_shutdown,
.driver = {
@@ -1840,7 +1841,7 @@ static struct platform_driver dw_driver = {
static int __init dw_init(void)
{
- return platform_driver_probe(&dw_driver, dw_probe);
+ return platform_driver_register(&dw_driver);
}
subsys_initcall(dw_init);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 8:17 ` [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place Andy Shevchenko
@ 2013-01-09 13:47 ` Vinod Koul
2013-01-09 14:56 ` Andy Shevchenko
0 siblings, 1 reply; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 13:47 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 10:17:00AM +0200, Andy Shevchenko wrote:
> If we don't yet have the platform device for the driver when it is being loaded
> we fail to probe the driver. So instead of calling probe() directly we call
> platform_driver_register(). It will call the probe() immediately if we have the
> device but also makes the driver to work on platforms where the platform device
> is created later.
Why wont you have the platform device? Is this only due to your approach of
creating the device in PCI probe?
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/dma/dw_dmac.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 476e9c8..b5f21ba 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1829,6 +1829,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
> #endif
>
> static struct platform_driver dw_driver = {
> + .probe = dw_probe,
> .remove = dw_remove,
> .shutdown = dw_shutdown,
> .driver = {
> @@ -1840,7 +1841,7 @@ static struct platform_driver dw_driver = {
>
> static int __init dw_init(void)
> {
> - return platform_driver_probe(&dw_driver, dw_probe);
> + return platform_driver_register(&dw_driver);
> }
> subsys_initcall(dw_init);
>
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 13:47 ` Vinod Koul
@ 2013-01-09 14:56 ` Andy Shevchenko
2013-01-09 15:38 ` Vinod Koul
0 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 14:56 UTC (permalink / raw)
To: Vinod Koul; +Cc: Andy Shevchenko, linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 9, 2013 at 3:47 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jan 09, 2013 at 10:17:00AM +0200, Andy Shevchenko wrote:
>> If we don't yet have the platform device for the driver when it is being loaded
>> we fail to probe the driver. So instead of calling probe() directly we call
>> platform_driver_register(). It will call the probe() immediately if we have the
>> device but also makes the driver to work on platforms where the platform device
>> is created later.
> Why wont you have the platform device?
It's still the same device as before. The only difference is in
boot/initialization sequence.
> Is this only due to your approach of
> creating the device in PCI probe?
No. We had an issue when driver was initialized (and with old version
immediately probed)
just before it happened in the platform devices. In such case .probe() fails.
Above patch makes sure that platform devices are already in the
internal structures and we could match them safely.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>> drivers/dma/dw_dmac.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index 476e9c8..b5f21ba 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -1829,6 +1829,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
>> #endif
>>
>> static struct platform_driver dw_driver = {
>> + .probe = dw_probe,
>> .remove = dw_remove,
>> .shutdown = dw_shutdown,
>> .driver = {
>> @@ -1840,7 +1841,7 @@ static struct platform_driver dw_driver = {
>>
>> static int __init dw_init(void)
>> {
>> - return platform_driver_probe(&dw_driver, dw_probe);
>> + return platform_driver_register(&dw_driver);
>> }
>> subsys_initcall(dw_init);
>>
>> --
>> 1.7.10.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 14:56 ` Andy Shevchenko
@ 2013-01-09 15:38 ` Vinod Koul
2013-01-09 18:04 ` Mika Westerberg
2013-01-10 8:34 ` Viresh Kumar
0 siblings, 2 replies; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 15:38 UTC (permalink / raw)
To: Andy Shevchenko, Linus Walleij
Cc: Andy Shevchenko, linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 04:56:45PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 9, 2013 at 3:47 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Wed, Jan 09, 2013 at 10:17:00AM +0200, Andy Shevchenko wrote:
> >> If we don't yet have the platform device for the driver when it is being loaded
> >> we fail to probe the driver. So instead of calling probe() directly we call
> >> platform_driver_register(). It will call the probe() immediately if we have the
> >> device but also makes the driver to work on platforms where the platform device
> >> is created later.
> > Why wont you have the platform device?
>
> It's still the same device as before. The only difference is in
> boot/initialization sequence.
>
> > Is this only due to your approach of
> > creating the device in PCI probe?
>
> No. We had an issue when driver was initialized (and with old version
> immediately probed)
> just before it happened in the platform devices. In such case .probe() fails.
> Above patch makes sure that platform devices are already in the
> internal structures and we could match them safely.
hmmm, but that its still not clear why that would happen. Driver init is called
later when all device creation should already be done.
> >> }
> >> subsys_initcall(dw_init);
Viresh, Linus
We had this discussion sometime back. pl022 use dma and we need DMA before SPI.
So if we have pl022 as late_init and dma as device_init. If we have one more
dependency then moving dw_dmac to fs_init should help too. But arch/subsys seems
overkill here?
--
~Vinod
why is dw_dmac a subsys init. One reason could be ensure these are before before
the client devices, but
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 15:38 ` Vinod Koul
@ 2013-01-09 18:04 ` Mika Westerberg
2013-01-12 12:28 ` Vinod Koul
2013-01-10 8:34 ` Viresh Kumar
1 sibling, 1 reply; 40+ messages in thread
From: Mika Westerberg @ 2013-01-09 18:04 UTC (permalink / raw)
To: Vinod Koul
Cc: Andy Shevchenko, Linus Walleij, Andy Shevchenko, linux-kernel,
Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 07:38:48AM -0800, Vinod Koul wrote:
> On Wed, Jan 09, 2013 at 04:56:45PM +0200, Andy Shevchenko wrote:
> > On Wed, Jan 9, 2013 at 3:47 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > > On Wed, Jan 09, 2013 at 10:17:00AM +0200, Andy Shevchenko wrote:
> > >> If we don't yet have the platform device for the driver when it is being loaded
> > >> we fail to probe the driver. So instead of calling probe() directly we call
> > >> platform_driver_register(). It will call the probe() immediately if we have the
> > >> device but also makes the driver to work on platforms where the platform device
> > >> is created later.
> > > Why wont you have the platform device?
> >
> > It's still the same device as before. The only difference is in
> > boot/initialization sequence.
> >
> > > Is this only due to your approach of
> > > creating the device in PCI probe?
> >
> > No. We had an issue when driver was initialized (and with old version
> > immediately probed)
> > just before it happened in the platform devices. In such case .probe() fails.
> > Above patch makes sure that platform devices are already in the
> > internal structures and we could match them safely.
> hmmm, but that its still not clear why that would happen. Driver init is called
> later when all device creation should already be done.
If the platform device is not yet created (like it might be when it is
enumerated from ACPI for example), the probe fails completely. Certainly
not the situation we want.
Calling platform_driver_register() handles both cases properly.
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 18:04 ` Mika Westerberg
@ 2013-01-12 12:28 ` Vinod Koul
0 siblings, 0 replies; 40+ messages in thread
From: Vinod Koul @ 2013-01-12 12:28 UTC (permalink / raw)
To: Mika Westerberg
Cc: Andy Shevchenko, Linus Walleij, Andy Shevchenko, linux-kernel,
Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 08:04:28PM +0200, Mika Westerberg wrote:
>
> If the platform device is not yet created (like it might be when it is
> enumerated from ACPI for example), the probe fails completely. Certainly
> not the situation we want.
>
> Calling platform_driver_register() handles both cases properly.
Yes this is fix for that kind of problem. But the main issue I am concerned
about is that devices should have created much before drivers started loading.
Obviosuly due to SPI/DMA dependencies we have to do this.
--
~Vinod
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-09 15:38 ` Vinod Koul
2013-01-09 18:04 ` Mika Westerberg
@ 2013-01-10 8:34 ` Viresh Kumar
2013-01-12 12:30 ` Vinod Koul
1 sibling, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2013-01-10 8:34 UTC (permalink / raw)
To: Vinod Koul
Cc: Andy Shevchenko, Linus Walleij, Andy Shevchenko, linux-kernel,
spear-devel
On 9 January 2013 21:08, Vinod Koul <vinod.koul@intel.com> wrote:
>> >> subsys_initcall(dw_init);
> Viresh, Linus
>
> We had this discussion sometime back. pl022 use dma and we need DMA before SPI.
> So if we have pl022 as late_init and dma as device_init. If we have one more
> dependency then moving dw_dmac to fs_init should help too. But arch/subsys seems
> overkill here?
We probably can't make spi late_init as they may be others waiting for
it to be in. And
so did subsys_init for dma. fs is for filesystem? then that may not be
logically correct too.
What's the problem you are seeing here with subsys_init?
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place
2013-01-10 8:34 ` Viresh Kumar
@ 2013-01-12 12:30 ` Vinod Koul
0 siblings, 0 replies; 40+ messages in thread
From: Vinod Koul @ 2013-01-12 12:30 UTC (permalink / raw)
To: Viresh Kumar
Cc: Andy Shevchenko, Linus Walleij, Andy Shevchenko, linux-kernel,
spear-devel
On Thu, Jan 10, 2013 at 02:04:53PM +0530, Viresh Kumar wrote:
> On 9 January 2013 21:08, Vinod Koul <vinod.koul@intel.com> wrote:
> >> >> subsys_initcall(dw_init);
> > Viresh, Linus
> >
> > We had this discussion sometime back. pl022 use dma and we need DMA before SPI.
> > So if we have pl022 as late_init and dma as device_init. If we have one more
> > dependency then moving dw_dmac to fs_init should help too. But arch/subsys seems
> > overkill here?
>
> We probably can't make spi late_init as they may be others waiting for
> it to be in. And
> so did subsys_init for dma. fs is for filesystem? then that may not be
> logically correct too.
well subsys_init doesnt seem to be logical for dma driver too!
--
~Vinod
^ permalink raw reply [flat|nested] 40+ messages in thread
* [resend][PATCH 02/16] dw_dmac: absence of pdata isn't critical when autocfg is set
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function Andy Shevchenko
` (14 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The patch allows to probe the device when platform data is absent and hardware
auto configuration is enabled. In that case the default platform data is used
where the channel allocation order is set to ascending, channel priority is set
to ascending, and private property is set to true.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 23 ++++++++++++++++-------
1 file changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index b5f21ba..bf32797 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1600,13 +1600,6 @@ static int dw_probe(struct platform_device *pdev)
int err;
int i;
- pdata = dev_get_platdata(&pdev->dev);
- if (!pdata)
- pdata = dw_dma_parse_dt(pdev);
-
- if (!pdata || pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS)
- return -EINVAL;
-
io = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!io)
return -EINVAL;
@@ -1622,6 +1615,22 @@ static int dw_probe(struct platform_device *pdev)
dw_params = dma_read_byaddr(regs, DW_PARAMS);
autocfg = dw_params >> DW_PARAMS_EN & 0x1;
+ pdata = dev_get_platdata(&pdev->dev);
+ if (!pdata)
+ pdata = dw_dma_parse_dt(pdev);
+
+ if (!pdata && autocfg) {
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return -ENOMEM;
+
+ /* Fill platform data with the default values */
+ pdata->is_private = true;
+ pdata->chan_allocation_order = CHAN_ALLOCATION_ASCENDING;
+ pdata->chan_priority = CHAN_PRIORITY_ASCENDING;
+ } else if (!pdata || pdata->nr_channels > DW_DMA_MAX_NR_CHANNELS)
+ return -EINVAL;
+
if (autocfg)
nr_channels = (dw_params >> DW_PARAMS_NR_CHAN & 0x7) + 1;
else
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 01/16] dw_dmac: call .probe after we have a device in place Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 02/16] dw_dmac: absence of pdata isn't critical when autocfg is set Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 13:50 ` Vinod Koul
2013-01-09 8:17 ` [resend][PATCH 04/16] dma: at_hdmac: check direction properly for cyclic transfers Andy Shevchenko
` (13 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
This function helps to distinguish the slave type of transfer by checking the
direction parameter.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
include/linux/dmaengine.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
index 4ca9cf7..b196ea3 100644
--- a/include/linux/dmaengine.h
+++ b/include/linux/dmaengine.h
@@ -621,6 +621,11 @@ static inline int dmaengine_slave_config(struct dma_chan *chan,
(unsigned long)config);
}
+static inline bool is_slave_xfer(enum dma_transfer_direction direction)
+{
+ return (direction == DMA_MEM_TO_DEV) || (direction == DMA_DEV_TO_MEM);
+}
+
static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
struct dma_chan *chan, dma_addr_t buf, size_t len,
enum dma_transfer_direction dir, unsigned long flags)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function
2013-01-09 8:17 ` [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function Andy Shevchenko
@ 2013-01-09 13:50 ` Vinod Koul
2013-01-09 14:19 ` Viresh Kumar
0 siblings, 1 reply; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 13:50 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 10:17:02AM +0200, Andy Shevchenko wrote:
> This function helps to distinguish the slave type of transfer by checking the
> direction parameter.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Cc: Nicolas Ferre <nicolas.ferre@atmel.com>
> Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
> ---
> include/linux/dmaengine.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h
> index 4ca9cf7..b196ea3 100644
> --- a/include/linux/dmaengine.h
> +++ b/include/linux/dmaengine.h
> @@ -621,6 +621,11 @@ static inline int dmaengine_slave_config(struct dma_chan *chan,
> (unsigned long)config);
> }
>
> +static inline bool is_slave_xfer(enum dma_transfer_direction direction)
> +{
> + return (direction == DMA_MEM_TO_DEV) || (direction == DMA_DEV_TO_MEM);
> +}
After reading the subsequent patch I understand what is intent here. Perhaps
is_slave_dirn() would have been a beter one...
> +
> static inline struct dma_async_tx_descriptor *dmaengine_prep_slave_single(
> struct dma_chan *chan, dma_addr_t buf, size_t len,
> enum dma_transfer_direction dir, unsigned long flags)
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function
2013-01-09 13:50 ` Vinod Koul
@ 2013-01-09 14:19 ` Viresh Kumar
2013-01-09 14:58 ` Andy Shevchenko
0 siblings, 1 reply; 40+ messages in thread
From: Viresh Kumar @ 2013-01-09 14:19 UTC (permalink / raw)
To: Vinod Koul; +Cc: Andy Shevchenko, linux-kernel, spear-devel
n 9 January 2013 19:20, Vinod Koul <vinod.koul@intel.com> wrote:
>> +static inline bool is_slave_xfer(enum dma_transfer_direction direction)
>> +{
>> + return (direction == DMA_MEM_TO_DEV) || (direction == DMA_DEV_TO_MEM);
>> +}
> After reading the subsequent patch I understand what is intent here. Perhaps
> is_slave_dirn() would have been a beter one...
dirn isn't readable, it took me some time to understand the intent...
Either have xfer or direction :)
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function
2013-01-09 14:19 ` Viresh Kumar
@ 2013-01-09 14:58 ` Andy Shevchenko
2013-01-09 15:10 ` Vinod Koul
0 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 14:58 UTC (permalink / raw)
To: Viresh Kumar; +Cc: Vinod Koul, Andy Shevchenko, linux-kernel, spear-devel
On Wed, Jan 9, 2013 at 4:19 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> n 9 January 2013 19:20, Vinod Koul <vinod.koul@intel.com> wrote:
>>> +static inline bool is_slave_xfer(enum dma_transfer_direction direction)
>>> +{
>>> + return (direction == DMA_MEM_TO_DEV) || (direction == DMA_DEV_TO_MEM);
>>> +}
>> After reading the subsequent patch I understand what is intent here. Perhaps
>> is_slave_dirn() would have been a beter one...
>
> dirn isn't readable, it took me some time to understand the intent...
> Either have xfer or direction :)
Agree with Viresh. I might rename it to is_slave_direction if you want to.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function
2013-01-09 14:58 ` Andy Shevchenko
@ 2013-01-09 15:10 ` Vinod Koul
2013-01-09 15:51 ` Andy Shevchenko
0 siblings, 1 reply; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 15:10 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Viresh Kumar, Andy Shevchenko, linux-kernel, spear-devel
On Wed, Jan 09, 2013 at 04:58:02PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 9, 2013 at 4:19 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
> > n 9 January 2013 19:20, Vinod Koul <vinod.koul@intel.com> wrote:
> >>> +static inline bool is_slave_xfer(enum dma_transfer_direction direction)
> >>> +{
> >>> + return (direction == DMA_MEM_TO_DEV) || (direction == DMA_DEV_TO_MEM);
> >>> +}
> >> After reading the subsequent patch I understand what is intent here. Perhaps
> >> is_slave_dirn() would have been a beter one...
> >
> > dirn isn't readable, it took me some time to understand the intent...
> > Either have xfer or direction :)
>
> Agree with Viresh. I might rename it to is_slave_direction if you want to.
direction sounds fine too.
--
~Vinod
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function
2013-01-09 15:10 ` Vinod Koul
@ 2013-01-09 15:51 ` Andy Shevchenko
0 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 15:51 UTC (permalink / raw)
To: Vinod Koul; +Cc: Viresh Kumar, Andy Shevchenko, linux-kernel, spear-devel
On Wed, Jan 9, 2013 at 5:10 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jan 09, 2013 at 04:58:02PM +0200, Andy Shevchenko wrote:
>> On Wed, Jan 9, 2013 at 4:19 PM, Viresh Kumar <viresh.kumar@linaro.org> wrote:
>> > n 9 January 2013 19:20, Vinod Koul <vinod.koul@intel.com> wrote:
>> >>> +static inline bool is_slave_xfer(enum dma_transfer_direction direction)
>> >>> +{
>> >>> + return (direction == DMA_MEM_TO_DEV) || (direction == DMA_DEV_TO_MEM);
>> >>> +}
>> >> After reading the subsequent patch I understand what is intent here. Perhaps
>> >> is_slave_dirn() would have been a beter one...
>> >
>> > dirn isn't readable, it took me some time to understand the intent...
>> > Either have xfer or direction :)
>>
>> Agree with Viresh. I might rename it to is_slave_direction if you want to.
> direction sounds fine too.
Okay, I'll rename it tomorrow and send v2 for that subseries and the
other patch where changelog should be fixed.
>
> --
> ~Vinod
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 40+ messages in thread
* [resend][PATCH 04/16] dma: at_hdmac: check direction properly for cyclic transfers
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (2 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 03/16] dmaengine: introduce is_slave_xfer function Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 05/16] dma: dw_dmac: check direction properly in dw_dma_cyclic_prep Andy Shevchenko
` (12 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
dma_transfer_direction is a normal enum. It means we can't usually use the
values as bit fields. Let's adjust this check and move it above the usage of
the direction parameter, due to the nature of the following usage of it.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
---
drivers/dma/at_hdmac.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/at_hdmac.c b/drivers/dma/at_hdmac.c
index 13a02f4..997ae54 100644
--- a/drivers/dma/at_hdmac.c
+++ b/drivers/dma/at_hdmac.c
@@ -778,7 +778,7 @@ err:
*/
static int
atc_dma_cyclic_check_values(unsigned int reg_width, dma_addr_t buf_addr,
- size_t period_len, enum dma_transfer_direction direction)
+ size_t period_len)
{
if (period_len > (ATC_BTSIZE_MAX << reg_width))
goto err_out;
@@ -786,8 +786,6 @@ atc_dma_cyclic_check_values(unsigned int reg_width, dma_addr_t buf_addr,
goto err_out;
if (unlikely(buf_addr & ((1 << reg_width) - 1)))
goto err_out;
- if (unlikely(!(direction & (DMA_DEV_TO_MEM | DMA_MEM_TO_DEV))))
- goto err_out;
return 0;
@@ -886,14 +884,16 @@ atc_prep_dma_cyclic(struct dma_chan *chan, dma_addr_t buf_addr, size_t buf_len,
return NULL;
}
+ if (unlikely(!is_slave_xfer(direction)))
+ goto err_out;
+
if (sconfig->direction == DMA_MEM_TO_DEV)
reg_width = convert_buswidth(sconfig->dst_addr_width);
else
reg_width = convert_buswidth(sconfig->src_addr_width);
/* Check for too big/unaligned periods and unaligned DMA buffer */
- if (atc_dma_cyclic_check_values(reg_width, buf_addr,
- period_len, direction))
+ if (atc_dma_cyclic_check_values(reg_width, buf_addr, period_len))
goto err_out;
/* build cyclic linked list */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 05/16] dma: dw_dmac: check direction properly in dw_dma_cyclic_prep
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (3 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 04/16] dma: at_hdmac: check direction properly for cyclic transfers Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 06/16] dma: ep93xx_dma: reuse is_slave_xfer helper Andy Shevchenko
` (11 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
dma_transfer_direction is a normal enum. It means we can't usually use the
values as bit fields. Let's adjust this check and move it above the usage of
the direction parameter, due to the nature of the following usage of it.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index bf32797..5392b55 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1336,6 +1336,9 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
retval = ERR_PTR(-EINVAL);
+ if (unlikely(!is_slave_xfer(direction)))
+ goto out_err;
+
if (direction == DMA_MEM_TO_DEV)
reg_width = __ffs(sconfig->dst_addr_width);
else
@@ -1350,8 +1353,6 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
goto out_err;
if (unlikely(buf_addr & ((1 << reg_width) - 1)))
goto out_err;
- if (unlikely(!(direction & (DMA_MEM_TO_DEV | DMA_DEV_TO_MEM))))
- goto out_err;
retval = ERR_PTR(-ENOMEM);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 06/16] dma: ep93xx_dma: reuse is_slave_xfer helper
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (4 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 05/16] dma: dw_dmac: check direction properly in dw_dma_cyclic_prep Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 07/16] dma: ipu_idmac: " Andy Shevchenko
` (10 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The is_slave_xfer helps to check if the transfer type is slave.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>
---
drivers/dma/ep93xx_dma.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/dma/ep93xx_dma.c b/drivers/dma/ep93xx_dma.c
index bcfde40..a2f1d9f 100644
--- a/drivers/dma/ep93xx_dma.c
+++ b/drivers/dma/ep93xx_dma.c
@@ -903,8 +903,7 @@ static int ep93xx_dma_alloc_chan_resources(struct dma_chan *chan)
switch (data->port) {
case EP93XX_DMA_SSP:
case EP93XX_DMA_IDE:
- if (data->direction != DMA_MEM_TO_DEV &&
- data->direction != DMA_DEV_TO_MEM)
+ if (!is_slave_xfer(data->direction))
return -EINVAL;
break;
default:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 07/16] dma: ipu_idmac: reuse is_slave_xfer helper
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (5 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 06/16] dma: ep93xx_dma: reuse is_slave_xfer helper Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 08/16] dma: ste_dma40: " Andy Shevchenko
` (9 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The is_slave_xfer helps to check if the transfer type is slave.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Cc: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
---
drivers/dma/ipu/ipu_idmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/ipu/ipu_idmac.c b/drivers/dma/ipu/ipu_idmac.c
index 6585537..c8d0254 100644
--- a/drivers/dma/ipu/ipu_idmac.c
+++ b/drivers/dma/ipu/ipu_idmac.c
@@ -1347,7 +1347,7 @@ static struct dma_async_tx_descriptor *idmac_prep_slave_sg(struct dma_chan *chan
chan->chan_id != IDMAC_IC_7)
return NULL;
- if (direction != DMA_DEV_TO_MEM && direction != DMA_MEM_TO_DEV) {
+ if (!is_slave_xfer(direction)) {
dev_err(chan->device->dev, "Invalid DMA direction %d!\n", direction);
return NULL;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 08/16] dma: ste_dma40: reuse is_slave_xfer helper
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (6 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 07/16] dma: ipu_idmac: " Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 09/16] dw_dmac: store direction in the custom channel structure Andy Shevchenko
` (8 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The is_slave_xfer helps to check if the transfer type is slave.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Viresh Kumar <viresh.kumar@linaro.org>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/dma/ste_dma40.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/ste_dma40.c b/drivers/dma/ste_dma40.c
index 23c5573..9f4fdf4 100644
--- a/drivers/dma/ste_dma40.c
+++ b/drivers/dma/ste_dma40.c
@@ -2337,7 +2337,7 @@ static struct dma_async_tx_descriptor *d40_prep_slave_sg(struct dma_chan *chan,
unsigned long dma_flags,
void *context)
{
- if (direction != DMA_DEV_TO_MEM && direction != DMA_MEM_TO_DEV)
+ if (!is_slave_xfer(direction))
return NULL;
return d40_prep_sg(chan, sgl, sgl, sg_len, direction, dma_flags);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 09/16] dw_dmac: store direction in the custom channel structure
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (7 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 08/16] dma: ste_dma40: " Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 10/16] dw_dmac: make usage of dw_dma_slave optional Andy Shevchenko
` (7 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
Currently the direction value comes from the generic slave configuration
structure and explicitly as a preparation function parameter. The first one is
kinda obsoleted. Thus, we have to store the value passed to the preparation
function somewhere in our structures to be able to use it later. The best
candidate to provide the storage is a custom channel structure. Until now we
still keep and check the direction field of the slave config structure as well.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 12 ++++++++++--
drivers/dma/dw_dmac_regs.h | 14 ++++++++------
2 files changed, 18 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 5392b55..6b3bc23 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -178,9 +178,9 @@ static void dwc_initialize(struct dw_dma_chan *dwc)
cfghi = dws->cfg_hi;
cfglo |= dws->cfg_lo & ~DWC_CFGL_CH_PRIOR_MASK;
} else {
- if (dwc->dma_sconfig.direction == DMA_MEM_TO_DEV)
+ if (dwc->direction == DMA_MEM_TO_DEV)
cfghi = DWC_CFGH_DST_PER(dwc->dma_sconfig.slave_id);
- else if (dwc->dma_sconfig.direction == DMA_DEV_TO_MEM)
+ else if (dwc->direction == DMA_DEV_TO_MEM)
cfghi = DWC_CFGH_SRC_PER(dwc->dma_sconfig.slave_id);
}
@@ -723,6 +723,8 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
return NULL;
}
+ dwc->direction = DMA_MEM_TO_MEM;
+
data_width = min_t(unsigned int, dwc->dw->data_width[dwc_get_sms(dws)],
dwc->dw->data_width[dwc_get_dms(dws)]);
@@ -807,6 +809,8 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
if (unlikely(!dws || !sg_len))
return NULL;
+ dwc->direction = direction;
+
prev = first = NULL;
switch (direction) {
@@ -983,6 +987,7 @@ set_runtime_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
return -EINVAL;
memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig));
+ dwc->direction = sconfig->direction;
convert_burst(&dwc->dma_sconfig.src_maxburst);
convert_burst(&dwc->dma_sconfig.dst_maxburst);
@@ -1339,6 +1344,8 @@ struct dw_cyclic_desc *dw_dma_cyclic_prep(struct dma_chan *chan,
if (unlikely(!is_slave_xfer(direction)))
goto out_err;
+ dwc->direction = direction;
+
if (direction == DMA_MEM_TO_DEV)
reg_width = __ffs(sconfig->dst_addr_width);
else
@@ -1713,6 +1720,7 @@ static int dw_probe(struct platform_device *pdev)
channel_clear_bit(dw, CH_EN, dwc->mask);
dwc->dw = dw;
+ dwc->direction = DMA_TRANS_NONE;
/* hardware configuration */
if (autocfg) {
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index 8881e9b..f9532c2 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -9,6 +9,7 @@
* published by the Free Software Foundation.
*/
+#include <linux/dmaengine.h>
#include <linux/dw_dmac.h>
#define DW_DMA_MAX_NR_CHANNELS 8
@@ -184,12 +185,13 @@ enum dw_dmac_flags {
};
struct dw_dma_chan {
- struct dma_chan chan;
- void __iomem *ch_regs;
- u8 mask;
- u8 priority;
- bool paused;
- bool initialized;
+ struct dma_chan chan;
+ void __iomem *ch_regs;
+ u8 mask;
+ u8 priority;
+ enum dma_transfer_direction direction;
+ bool paused;
+ bool initialized;
/* software emulation of the LLP transfers */
struct list_head *tx_list;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 10/16] dw_dmac: make usage of dw_dma_slave optional
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (8 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 09/16] dw_dmac: store direction in the custom channel structure Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous Andy Shevchenko
` (6 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The driver requires a custom slave configuration to be present to be able to
make the slave transfers. Nevertheless, in some cases we need only the request
line as an additional information to the generic slave configuration. The
request line is provided by slave_id parameter of the dma_slave_config
structure. That's why the custom slave configuration could be optional for such
cases.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 6b3bc23..ff9d68b 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -50,11 +50,12 @@ static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave)
struct dw_dma_slave *__slave = (_chan->private); \
struct dw_dma_chan *_dwc = to_dw_dma_chan(_chan); \
struct dma_slave_config *_sconfig = &_dwc->dma_sconfig; \
+ bool _is_slave = is_slave_xfer(_dwc->direction); \
int _dms = dwc_get_dms(__slave); \
int _sms = dwc_get_sms(__slave); \
- u8 _smsize = __slave ? _sconfig->src_maxburst : \
+ u8 _smsize = _is_slave ? _sconfig->src_maxburst : \
DW_DMA_MSIZE_16; \
- u8 _dmsize = __slave ? _sconfig->dst_maxburst : \
+ u8 _dmsize = _is_slave ? _sconfig->dst_maxburst : \
DW_DMA_MSIZE_16; \
\
(DWC_CTLL_DST_MSIZE(_dmsize) \
@@ -325,7 +326,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
list_splice_init(&desc->tx_list, &dwc->free_list);
list_move(&desc->desc_node, &dwc->free_list);
- if (!dwc->chan.private) {
+ if (!is_slave_xfer(dwc->direction)) {
struct device *parent = chan2parent(&dwc->chan);
if (!(txd->flags & DMA_COMPL_SKIP_DEST_UNMAP)) {
if (txd->flags & DMA_COMPL_DEST_UNMAP_SINGLE)
@@ -806,7 +807,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
dev_vdbg(chan2dev(chan), "%s\n", __func__);
- if (unlikely(!dws || !sg_len))
+ if (unlikely(!is_slave_xfer(direction) || !sg_len))
return NULL;
dwc->direction = direction;
@@ -982,8 +983,8 @@ set_runtime_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
{
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
- /* Check if it is chan is configured for slave transfers */
- if (!chan->private)
+ /* Check if chan will be configured for slave transfers */
+ if (!is_slave_xfer(sconfig->direction))
return -EINVAL;
memcpy(&dwc->dma_sconfig, sconfig, sizeof(*sconfig));
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (9 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 10/16] dw_dmac: make usage of dw_dma_slave optional Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 14:24 ` Vinod Koul
2013-01-09 8:17 ` [resend][PATCH 12/16] dw_dmac: check for mapping errors Andy Shevchenko
` (5 subsequent siblings)
16 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The same information could be exctracted from the struct dma_chan.
The patch introduces helper function dwc_get_data_width() as well.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 27 ++++++++++++++++++++-------
drivers/dma/dw_dmac_regs.h | 3 ---
2 files changed, 20 insertions(+), 10 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index ff9d68b..39a6506 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -73,6 +73,22 @@ static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave)
*/
#define NR_DESCS_PER_CHANNEL 64
+#define SRC_MASTER 0
+#define DST_MASTER 1
+
+static inline unsigned int dwc_get_data_width(struct dma_chan *chan, int master)
+{
+ struct dw_dma *dw = to_dw_dma(chan->device);
+ struct dw_dma_slave *dws = chan->private;
+
+ if (master == SRC_MASTER)
+ return dw->data_width[dwc_get_sms(dws)];
+ else if (master == DST_MASTER)
+ return dw->data_width[dwc_get_dms(dws)];
+
+ return 0;
+}
+
/*----------------------------------------------------------------------*/
/*
@@ -703,7 +719,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
size_t len, unsigned long flags)
{
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
- struct dw_dma_slave *dws = chan->private;
struct dw_desc *desc;
struct dw_desc *first;
struct dw_desc *prev;
@@ -726,8 +741,8 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
dwc->direction = DMA_MEM_TO_MEM;
- data_width = min_t(unsigned int, dwc->dw->data_width[dwc_get_sms(dws)],
- dwc->dw->data_width[dwc_get_dms(dws)]);
+ data_width = min_t(unsigned int, dwc_get_data_width(chan, SRC_MASTER),
+ dwc_get_data_width(chan, DST_MASTER));
src_width = dst_width = min_t(unsigned int, data_width,
dwc_fast_fls(src | dest | len));
@@ -792,7 +807,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
unsigned long flags, void *context)
{
struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
- struct dw_dma_slave *dws = chan->private;
struct dma_slave_config *sconfig = &dwc->dma_sconfig;
struct dw_desc *prev;
struct dw_desc *first;
@@ -826,7 +840,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
DWC_CTLL_FC(DW_DMA_FC_D_M2P);
- data_width = dwc->dw->data_width[dwc_get_sms(dws)];
+ data_width = dwc_get_data_width(chan, SRC_MASTER);
for_each_sg(sgl, sg, sg_len, i) {
struct dw_desc *desc;
@@ -889,7 +903,7 @@ slave_sg_todev_fill_desc:
ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
DWC_CTLL_FC(DW_DMA_FC_D_P2M);
- data_width = dwc->dw->data_width[dwc_get_dms(dws)];
+ data_width = dwc_get_data_width(chan, DST_MASTER);
for_each_sg(sgl, sg, sg_len, i) {
struct dw_desc *desc;
@@ -1720,7 +1734,6 @@ static int dw_probe(struct platform_device *pdev)
channel_clear_bit(dw, CH_EN, dwc->mask);
- dwc->dw = dw;
dwc->direction = DMA_TRANS_NONE;
/* hardware configuration */
diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
index f9532c2..577f2dd 100644
--- a/drivers/dma/dw_dmac_regs.h
+++ b/drivers/dma/dw_dmac_regs.h
@@ -214,9 +214,6 @@ struct dw_dma_chan {
/* configuration passed via DMA_SLAVE_CONFIG */
struct dma_slave_config dma_sconfig;
-
- /* backlink to dw_dma */
- struct dw_dma *dw;
};
static inline struct dw_dma_chan_regs __iomem *
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
2013-01-09 8:17 ` [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous Andy Shevchenko
@ 2013-01-09 14:24 ` Vinod Koul
2013-01-09 15:02 ` Andy Shevchenko
0 siblings, 1 reply; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 14:24 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 10:17:10AM +0200, Andy Shevchenko wrote:
> The same information could be exctracted from the struct dma_chan.
typo... ^^^^^^^^^^
> The patch introduces helper function dwc_get_data_width() as well.
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/dma/dw_dmac.c | 27 ++++++++++++++++++++-------
> drivers/dma/dw_dmac_regs.h | 3 ---
> 2 files changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index ff9d68b..39a6506 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -73,6 +73,22 @@ static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave)
> */
> #define NR_DESCS_PER_CHANNEL 64
>
> +#define SRC_MASTER 0
> +#define DST_MASTER 1
> +
> +static inline unsigned int dwc_get_data_width(struct dma_chan *chan, int master)
> +{
> + struct dw_dma *dw = to_dw_dma(chan->device);
> + struct dw_dma_slave *dws = chan->private;
> +
> + if (master == SRC_MASTER)
> + return dw->data_width[dwc_get_sms(dws)];
> + else if (master == DST_MASTER)
> + return dw->data_width[dwc_get_dms(dws)];
> +
> + return 0;
> +}
> +
> /*----------------------------------------------------------------------*/
>
> /*
> @@ -703,7 +719,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
> size_t len, unsigned long flags)
> {
> struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
> - struct dw_dma_slave *dws = chan->private;
> struct dw_desc *desc;
> struct dw_desc *first;
> struct dw_desc *prev;
> @@ -726,8 +741,8 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>
> dwc->direction = DMA_MEM_TO_MEM;
>
> - data_width = min_t(unsigned int, dwc->dw->data_width[dwc_get_sms(dws)],
> - dwc->dw->data_width[dwc_get_dms(dws)]);
> + data_width = min_t(unsigned int, dwc_get_data_width(chan, SRC_MASTER),
> + dwc_get_data_width(chan, DST_MASTER));
>
> src_width = dst_width = min_t(unsigned int, data_width,
> dwc_fast_fls(src | dest | len));
> @@ -792,7 +807,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> unsigned long flags, void *context)
> {
> struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
> - struct dw_dma_slave *dws = chan->private;
> struct dma_slave_config *sconfig = &dwc->dma_sconfig;
> struct dw_desc *prev;
> struct dw_desc *first;
> @@ -826,7 +840,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
> ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
> DWC_CTLL_FC(DW_DMA_FC_D_M2P);
>
> - data_width = dwc->dw->data_width[dwc_get_sms(dws)];
> + data_width = dwc_get_data_width(chan, SRC_MASTER);
>
> for_each_sg(sgl, sg, sg_len, i) {
> struct dw_desc *desc;
> @@ -889,7 +903,7 @@ slave_sg_todev_fill_desc:
> ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
> DWC_CTLL_FC(DW_DMA_FC_D_P2M);
>
> - data_width = dwc->dw->data_width[dwc_get_dms(dws)];
> + data_width = dwc_get_data_width(chan, DST_MASTER);
>
> for_each_sg(sgl, sg, sg_len, i) {
> struct dw_desc *desc;
> @@ -1720,7 +1734,6 @@ static int dw_probe(struct platform_device *pdev)
>
> channel_clear_bit(dw, CH_EN, dwc->mask);
>
> - dwc->dw = dw;
> dwc->direction = DMA_TRANS_NONE;
>
> /* hardware configuration */
> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
> index f9532c2..577f2dd 100644
> --- a/drivers/dma/dw_dmac_regs.h
> +++ b/drivers/dma/dw_dmac_regs.h
> @@ -214,9 +214,6 @@ struct dw_dma_chan {
>
> /* configuration passed via DMA_SLAVE_CONFIG */
> struct dma_slave_config dma_sconfig;
> -
> - /* backlink to dw_dma */
> - struct dw_dma *dw;
> };
>
> static inline struct dw_dma_chan_regs __iomem *
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
2013-01-09 14:24 ` Vinod Koul
@ 2013-01-09 15:02 ` Andy Shevchenko
2013-01-09 15:11 ` Vinod Koul
0 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 15:02 UTC (permalink / raw)
To: Vinod Koul; +Cc: Andy Shevchenko, linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 9, 2013 at 4:24 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jan 09, 2013 at 10:17:10AM +0200, Andy Shevchenko wrote:
>> The same information could be exctracted from the struct dma_chan.
> typo... ^^^^^^^^^^
Thanks for finding this. Fixed in my local tree.
Shall I resubmit it here?
>> The patch introduces helper function dwc_get_data_width() as well.
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>> drivers/dma/dw_dmac.c | 27 ++++++++++++++++++++-------
>> drivers/dma/dw_dmac_regs.h | 3 ---
>> 2 files changed, 20 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index ff9d68b..39a6506 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -73,6 +73,22 @@ static inline unsigned int dwc_get_sms(struct dw_dma_slave *slave)
>> */
>> #define NR_DESCS_PER_CHANNEL 64
>>
>> +#define SRC_MASTER 0
>> +#define DST_MASTER 1
>> +
>> +static inline unsigned int dwc_get_data_width(struct dma_chan *chan, int master)
>> +{
>> + struct dw_dma *dw = to_dw_dma(chan->device);
>> + struct dw_dma_slave *dws = chan->private;
>> +
>> + if (master == SRC_MASTER)
>> + return dw->data_width[dwc_get_sms(dws)];
>> + else if (master == DST_MASTER)
>> + return dw->data_width[dwc_get_dms(dws)];
>> +
>> + return 0;
>> +}
>> +
>> /*----------------------------------------------------------------------*/
>>
>> /*
>> @@ -703,7 +719,6 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>> size_t len, unsigned long flags)
>> {
>> struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
>> - struct dw_dma_slave *dws = chan->private;
>> struct dw_desc *desc;
>> struct dw_desc *first;
>> struct dw_desc *prev;
>> @@ -726,8 +741,8 @@ dwc_prep_dma_memcpy(struct dma_chan *chan, dma_addr_t dest, dma_addr_t src,
>>
>> dwc->direction = DMA_MEM_TO_MEM;
>>
>> - data_width = min_t(unsigned int, dwc->dw->data_width[dwc_get_sms(dws)],
>> - dwc->dw->data_width[dwc_get_dms(dws)]);
>> + data_width = min_t(unsigned int, dwc_get_data_width(chan, SRC_MASTER),
>> + dwc_get_data_width(chan, DST_MASTER));
>>
>> src_width = dst_width = min_t(unsigned int, data_width,
>> dwc_fast_fls(src | dest | len));
>> @@ -792,7 +807,6 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>> unsigned long flags, void *context)
>> {
>> struct dw_dma_chan *dwc = to_dw_dma_chan(chan);
>> - struct dw_dma_slave *dws = chan->private;
>> struct dma_slave_config *sconfig = &dwc->dma_sconfig;
>> struct dw_desc *prev;
>> struct dw_desc *first;
>> @@ -826,7 +840,7 @@ dwc_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
>> ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_M2P) :
>> DWC_CTLL_FC(DW_DMA_FC_D_M2P);
>>
>> - data_width = dwc->dw->data_width[dwc_get_sms(dws)];
>> + data_width = dwc_get_data_width(chan, SRC_MASTER);
>>
>> for_each_sg(sgl, sg, sg_len, i) {
>> struct dw_desc *desc;
>> @@ -889,7 +903,7 @@ slave_sg_todev_fill_desc:
>> ctllo |= sconfig->device_fc ? DWC_CTLL_FC(DW_DMA_FC_P_P2M) :
>> DWC_CTLL_FC(DW_DMA_FC_D_P2M);
>>
>> - data_width = dwc->dw->data_width[dwc_get_dms(dws)];
>> + data_width = dwc_get_data_width(chan, DST_MASTER);
>>
>> for_each_sg(sgl, sg, sg_len, i) {
>> struct dw_desc *desc;
>> @@ -1720,7 +1734,6 @@ static int dw_probe(struct platform_device *pdev)
>>
>> channel_clear_bit(dw, CH_EN, dwc->mask);
>>
>> - dwc->dw = dw;
>> dwc->direction = DMA_TRANS_NONE;
>>
>> /* hardware configuration */
>> diff --git a/drivers/dma/dw_dmac_regs.h b/drivers/dma/dw_dmac_regs.h
>> index f9532c2..577f2dd 100644
>> --- a/drivers/dma/dw_dmac_regs.h
>> +++ b/drivers/dma/dw_dmac_regs.h
>> @@ -214,9 +214,6 @@ struct dw_dma_chan {
>>
>> /* configuration passed via DMA_SLAVE_CONFIG */
>> struct dma_slave_config dma_sconfig;
>> -
>> - /* backlink to dw_dma */
>> - struct dw_dma *dw;
>> };
>>
>> static inline struct dw_dma_chan_regs __iomem *
>> --
>> 1.7.10.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
2013-01-09 15:02 ` Andy Shevchenko
@ 2013-01-09 15:11 ` Vinod Koul
0 siblings, 0 replies; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 15:11 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: Andy Shevchenko, linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 05:02:39PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 9, 2013 at 4:24 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> > On Wed, Jan 09, 2013 at 10:17:10AM +0200, Andy Shevchenko wrote:
> >> The same information could be exctracted from the struct dma_chan.
> > typo... ^^^^^^^^^^
>
> Thanks for finding this. Fixed in my local tree.
> Shall I resubmit it here?
Never mind, this is minor one, will fix while applying
--
~Vinod
^ permalink raw reply [flat|nested] 40+ messages in thread
* [resend][PATCH 12/16] dw_dmac: check for mapping errors
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (10 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 11/16] dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 13/16] dw_dmac: remove redundant check Andy Shevchenko
` (4 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
Otherwise we get a warning in case of CONFIG_DMA_API_DEBUG=y
[ 45.775943] WARNING: at lib/dma-debug.c:933 check_unmap+0x5d6/0x6ac()
[ 45.782369] dw_dmac dw_dmac.0: DMA-API: device driver failed to check map error[device address=0x00000000356efcc0] [size=28 bytes] [mapped as single]
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 39a6506..83e0f9e 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1107,6 +1107,7 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
struct dw_desc *desc;
int i;
unsigned long flags;
+ int ret;
dev_vdbg(chan2dev(chan), "%s\n", __func__);
@@ -1130,12 +1131,8 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
spin_unlock_irqrestore(&dwc->lock, flags);
desc = kzalloc(sizeof(struct dw_desc), GFP_KERNEL);
- if (!desc) {
- dev_info(chan2dev(chan),
- "only allocated %d descriptors\n", i);
- spin_lock_irqsave(&dwc->lock, flags);
- break;
- }
+ if (!desc)
+ goto err_desc_alloc;
INIT_LIST_HEAD(&desc->tx_list);
dma_async_tx_descriptor_init(&desc->txd, chan);
@@ -1143,6 +1140,10 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
desc->txd.flags = DMA_CTRL_ACK;
desc->txd.phys = dma_map_single(chan2parent(chan), &desc->lli,
sizeof(desc->lli), DMA_TO_DEVICE);
+ ret = dma_mapping_error(chan2parent(chan), desc->txd.phys);
+ if (ret)
+ goto err_desc_alloc;
+
dwc_desc_put(dwc, desc);
spin_lock_irqsave(&dwc->lock, flags);
@@ -1154,6 +1155,13 @@ static int dwc_alloc_chan_resources(struct dma_chan *chan)
dev_dbg(chan2dev(chan), "%s: allocated %d descriptors\n", __func__, i);
return i;
+
+err_desc_alloc:
+ kfree(desc);
+
+ dev_info(chan2dev(chan), "only allocated %d descriptors\n", i);
+
+ return i;
}
static void dwc_free_chan_resources(struct dma_chan *chan)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 13/16] dw_dmac: remove redundant check
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (11 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 12/16] dw_dmac: check for mapping errors Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 14/16] dw_dmac: update tx_node_active in dwc_do_single_block Andy Shevchenko
` (3 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
There is no need to check the callback_required parameter, due to we check the
callback pointer to be a non-NULL.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 83e0f9e..05a8265 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -364,7 +364,7 @@ dwc_descriptor_complete(struct dw_dma_chan *dwc, struct dw_desc *desc,
spin_unlock_irqrestore(&dwc->lock, flags);
- if (callback_required && callback)
+ if (callback)
callback(param);
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 14/16] dw_dmac: update tx_node_active in dwc_do_single_block
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (12 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 13/16] dw_dmac: remove redundant check Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 15/16] dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume Andy Shevchenko
` (2 subsequent siblings)
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
The "else" keyword in the dw_dma_tasklet is removed as well. All together
simplifies the logic of the code and understanding of what is happening there.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 05a8265..8afbda6 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -264,6 +264,9 @@ static inline void dwc_do_single_block(struct dw_dma_chan *dwc,
channel_writel(dwc, CTL_LO, ctllo);
channel_writel(dwc, CTL_HI, desc->lli.ctlhi);
channel_set_bit(dw, CH_EN, dwc->mask);
+
+ /* Move pointer to next descriptor */
+ dwc->tx_node_active = dwc->tx_node_active->next;
}
/* Called with dwc->lock held and bh disabled */
@@ -295,7 +298,7 @@ static void dwc_dostart(struct dw_dma_chan *dwc, struct dw_desc *first)
dwc_initialize(dwc);
dwc->tx_list = &first->tx_list;
- dwc->tx_node_active = first->tx_list.next;
+ dwc->tx_node_active = &first->tx_list;
dwc_do_single_block(dwc, first);
@@ -621,18 +624,13 @@ static void dw_dma_tasklet(unsigned long data)
dma_writel(dw, CLEAR.XFER, dwc->mask);
- /* move pointer to next descriptor */
- dwc->tx_node_active =
- dwc->tx_node_active->next;
-
dwc_do_single_block(dwc, desc);
spin_unlock_irqrestore(&dwc->lock, flags);
continue;
- } else {
- /* we are done here */
- clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
}
+ /* we are done here */
+ clear_bit(DW_DMA_IS_SOFT_LLP, &dwc->flags);
}
spin_unlock_irqrestore(&dwc->lock, flags);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 15/16] dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (13 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 14/16] dw_dmac: update tx_node_active in dwc_do_single_block Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 8:17 ` [resend][PATCH 16/16] dma: dw_dmac: clear suspend bit during termination Andy Shevchenko
2013-01-09 15:44 ` [resend][PATCH 00/16] dw_dmac related patchset Vinod Koul
16 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
We will use at least the dwc_chan_resume() later.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 8afbda6..6a8f8c7 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1008,6 +1008,26 @@ set_runtime_config(struct dma_chan *chan, struct dma_slave_config *sconfig)
return 0;
}
+static inline void dwc_chan_pause(struct dw_dma_chan *dwc)
+{
+ u32 cfglo = channel_readl(dwc, CFG_LO);
+
+ channel_writel(dwc, CFG_LO, cfglo | DWC_CFGL_CH_SUSP);
+ while (!(channel_readl(dwc, CFG_LO) & DWC_CFGL_FIFO_EMPTY))
+ cpu_relax();
+
+ dwc->paused = true;
+}
+
+static inline void dwc_chan_resume(struct dw_dma_chan *dwc)
+{
+ u32 cfglo = channel_readl(dwc, CFG_LO);
+
+ channel_writel(dwc, CFG_LO, cfglo & ~DWC_CFGL_CH_SUSP);
+
+ dwc->paused = false;
+}
+
static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
unsigned long arg)
{
@@ -1015,18 +1035,13 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
struct dw_dma *dw = to_dw_dma(chan->device);
struct dw_desc *desc, *_desc;
unsigned long flags;
- u32 cfglo;
LIST_HEAD(list);
if (cmd == DMA_PAUSE) {
spin_lock_irqsave(&dwc->lock, flags);
- cfglo = channel_readl(dwc, CFG_LO);
- channel_writel(dwc, CFG_LO, cfglo | DWC_CFGL_CH_SUSP);
- while (!(channel_readl(dwc, CFG_LO) & DWC_CFGL_FIFO_EMPTY))
- cpu_relax();
+ dwc_chan_pause(dwc);
- dwc->paused = true;
spin_unlock_irqrestore(&dwc->lock, flags);
} else if (cmd == DMA_RESUME) {
if (!dwc->paused)
@@ -1034,9 +1049,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
spin_lock_irqsave(&dwc->lock, flags);
- cfglo = channel_readl(dwc, CFG_LO);
- channel_writel(dwc, CFG_LO, cfglo & ~DWC_CFGL_CH_SUSP);
- dwc->paused = false;
+ dwc_chan_resume(dwc);
spin_unlock_irqrestore(&dwc->lock, flags);
} else if (cmd == DMA_TERMINATE_ALL) {
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* [resend][PATCH 16/16] dma: dw_dmac: clear suspend bit during termination
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (14 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 15/16] dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume Andy Shevchenko
@ 2013-01-09 8:17 ` Andy Shevchenko
2013-01-09 14:18 ` Vinod Koul
2013-01-09 15:44 ` [resend][PATCH 00/16] dw_dmac related patchset Vinod Koul
16 siblings, 1 reply; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 8:17 UTC (permalink / raw)
To: Vinod Koul, linux-kernel, Viresh Kumar, spear-devel
From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
The DMA transfer could not be established if previously it was paused and
terminated. In that case the channel's suspend bit remains set that prevents to
transfer anything until channel is resumed.
The patch adds a code that clears the DWC_CFGL_CH_SUSP bit during termination.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
drivers/dma/dw_dmac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 6a8f8c7..c6ebea1 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1059,7 +1059,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
dwc_chan_disable(dw, dwc);
- dwc->paused = false;
+ dwc_chan_resume(dwc);
/* active_list entries will end up before queued entries */
list_splice_init(&dwc->queue, &list);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 40+ messages in thread* Re: [resend][PATCH 16/16] dma: dw_dmac: clear suspend bit during termination
2013-01-09 8:17 ` [resend][PATCH 16/16] dma: dw_dmac: clear suspend bit during termination Andy Shevchenko
@ 2013-01-09 14:18 ` Vinod Koul
2013-01-09 15:07 ` Andy Shevchenko
0 siblings, 1 reply; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 14:18 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 10:17:15AM +0200, Andy Shevchenko wrote:
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>
> The DMA transfer could not be established if previously it was paused and
> terminated. In that case the channel's suspend bit remains set that prevents to
> transfer anything until channel is resumed.
>
> The patch adds a code that clears the DWC_CFGL_CH_SUSP bit during termination.
No this patch calls resume to do so... Can you update the change log
>
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> drivers/dma/dw_dmac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 6a8f8c7..c6ebea1 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1059,7 +1059,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>
> dwc_chan_disable(dw, dwc);
>
> - dwc->paused = false;
> + dwc_chan_resume(dwc);
>
> /* active_list entries will end up before queued entries */
> list_splice_init(&dwc->queue, &list);
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [resend][PATCH 16/16] dma: dw_dmac: clear suspend bit during termination
2013-01-09 14:18 ` Vinod Koul
@ 2013-01-09 15:07 ` Andy Shevchenko
0 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-09 15:07 UTC (permalink / raw)
To: Vinod Koul; +Cc: Andy Shevchenko, linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 9, 2013 at 4:18 PM, Vinod Koul <vinod.koul@intel.com> wrote:
> On Wed, Jan 09, 2013 at 10:17:15AM +0200, Andy Shevchenko wrote:
>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>
>> The DMA transfer could not be established if previously it was paused and
>> terminated. In that case the channel's suspend bit remains set that prevents to
>> transfer anything until channel is resumed.
>>
>> The patch adds a code that clears the DWC_CFGL_CH_SUSP bit during termination.
> No this patch calls resume to do so... Can you update the change log
Updated. Will occur in patch v2.
>>
>> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
>> ---
>> drivers/dma/dw_dmac.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
>> index 6a8f8c7..c6ebea1 100644
>> --- a/drivers/dma/dw_dmac.c
>> +++ b/drivers/dma/dw_dmac.c
>> @@ -1059,7 +1059,7 @@ static int dwc_control(struct dma_chan *chan, enum dma_ctrl_cmd cmd,
>>
>> dwc_chan_disable(dw, dwc);
>>
>> - dwc->paused = false;
>> + dwc_chan_resume(dwc);
>>
>> /* active_list entries will end up before queued entries */
>> list_splice_init(&dwc->queue, &list);
>> --
>> 1.7.10.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 40+ messages in thread
* Re: [resend][PATCH 00/16] dw_dmac related patchset
2013-01-09 8:16 ` [resend][PATCH 00/16] dw_dmac related patchset Andy Shevchenko
` (15 preceding siblings ...)
2013-01-09 8:17 ` [resend][PATCH 16/16] dma: dw_dmac: clear suspend bit during termination Andy Shevchenko
@ 2013-01-09 15:44 ` Vinod Koul
2013-01-10 8:28 ` Andy Shevchenko
16 siblings, 1 reply; 40+ messages in thread
From: Vinod Koul @ 2013-01-09 15:44 UTC (permalink / raw)
To: Andy Shevchenko; +Cc: linux-kernel, Viresh Kumar, spear-devel
On Wed, Jan 09, 2013 at 10:16:59AM +0200, Andy Shevchenko wrote:
> There is a patchset against dw_dmac driver. It has been resent for Vinod to
> review after he returned back from vacation and holidays. That's why there is
> no Cc list.
Applied these:
21fe3c5 dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume
f5c6a7d dw_dmac: update tx_node_active in dwc_do_single_block
21e93c1 dw_dmac: remove redundant check
cbd6531 dw_dmac: check for mapping errors
123de54 dw_dmac: absence of pdata isn't critical when autocfg is set
2, 9, 11(after fixing changelog) failed, possibly due to dependecy in series
>
> Andy Shevchenko (15):
> dw_dmac: call .probe after we have a device in place
> dw_dmac: absence of pdata isn't critical when autocfg is set
> dmaengine: introduce is_slave_xfer function
> dma: at_hdmac: check direction properly for cyclic transfers
> dma: dw_dmac: check direction properly in dw_dma_cyclic_prep
> dma: ep93xx_dma: reuse is_slave_xfer helper
> dma: ipu_idmac: reuse is_slave_xfer helper
> dma: ste_dma40: reuse is_slave_xfer helper
> dw_dmac: store direction in the custom channel structure
> dw_dmac: make usage of dw_dma_slave optional
> dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
> dw_dmac: check for mapping errors
> dw_dmac: remove redundant check
> dw_dmac: update tx_node_active in dwc_do_single_block
> dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume
>
> Heikki Krogerus (1):
> dma: dw_dmac: clear suspend bit during termination
>
> drivers/dma/at_hdmac.c | 10 +--
> drivers/dma/dw_dmac.c | 152 +++++++++++++++++++++++++++++--------------
> drivers/dma/dw_dmac_regs.h | 17 +++--
> drivers/dma/ep93xx_dma.c | 3 +-
> drivers/dma/ipu/ipu_idmac.c | 2 +-
> drivers/dma/ste_dma40.c | 2 +-
> include/linux/dmaengine.h | 5 ++
> 7 files changed, 123 insertions(+), 68 deletions(-)
>
> --
> 1.7.10.4
>
^ permalink raw reply [flat|nested] 40+ messages in thread* Re: [resend][PATCH 00/16] dw_dmac related patchset
2013-01-09 15:44 ` [resend][PATCH 00/16] dw_dmac related patchset Vinod Koul
@ 2013-01-10 8:28 ` Andy Shevchenko
0 siblings, 0 replies; 40+ messages in thread
From: Andy Shevchenko @ 2013-01-10 8:28 UTC (permalink / raw)
To: Vinod Koul; +Cc: Andy Shevchenko, linux-kernel, Viresh Kumar, spear-devel
On Wed, 2013-01-09 at 07:44 -0800, Vinod Koul wrote:
> On Wed, Jan 09, 2013 at 10:16:59AM +0200, Andy Shevchenko wrote:
> > There is a patchset against dw_dmac driver. It has been resent for Vinod to
> > review after he returned back from vacation and holidays. That's why there is
> > no Cc list.
> Applied these:
> 21fe3c5 dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume
> f5c6a7d dw_dmac: update tx_node_active in dwc_do_single_block
> 21e93c1 dw_dmac: remove redundant check
> cbd6531 dw_dmac: check for mapping errors
> 123de54 dw_dmac: absence of pdata isn't critical when autocfg is set
>
> 2, 9, 11(after fixing changelog) failed, possibly due to dependecy in series
It sounds strange. I just rebased against your recent next branch, and
it went fine.
Anyway, I'll submit v2 soon with changed name of the function and fixed
changelogs.
>
> >
> > Andy Shevchenko (15):
> > dw_dmac: call .probe after we have a device in place
> > dw_dmac: absence of pdata isn't critical when autocfg is set
> > dmaengine: introduce is_slave_xfer function
> > dma: at_hdmac: check direction properly for cyclic transfers
> > dma: dw_dmac: check direction properly in dw_dma_cyclic_prep
> > dma: ep93xx_dma: reuse is_slave_xfer helper
> > dma: ipu_idmac: reuse is_slave_xfer helper
> > dma: ste_dma40: reuse is_slave_xfer helper
> > dw_dmac: store direction in the custom channel structure
> > dw_dmac: make usage of dw_dma_slave optional
> > dw_dmac: backlink to dw_dma in dw_dma_chan is superfluous
> > dw_dmac: check for mapping errors
> > dw_dmac: remove redundant check
> > dw_dmac: update tx_node_active in dwc_do_single_block
> > dma: dw_dmac: add dwc_chan_pause and dwc_chan_resume
> >
> > Heikki Krogerus (1):
> > dma: dw_dmac: clear suspend bit during termination
> >
> > drivers/dma/at_hdmac.c | 10 +--
> > drivers/dma/dw_dmac.c | 152 +++++++++++++++++++++++++++++--------------
> > drivers/dma/dw_dmac_regs.h | 17 +++--
> > drivers/dma/ep93xx_dma.c | 3 +-
> > drivers/dma/ipu/ipu_idmac.c | 2 +-
> > drivers/dma/ste_dma40.c | 2 +-
> > include/linux/dmaengine.h | 5 ++
> > 7 files changed, 123 insertions(+), 68 deletions(-)
> >
> > --
> > 1.7.10.4
> >
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply [flat|nested] 40+ messages in thread