linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] dw_dmac: few cleanups to the driver
@ 2012-10-02 11:41 Andy Shevchenko
  2012-10-02 11:41 ` [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver() Andy Shevchenko
                   ` (4 more replies)
  0 siblings, 5 replies; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-02 11:41 UTC (permalink / raw)
  To: Viresh Kumar, Vinod Koul, linux-kernel, spear-devel, balbi
  Cc: Andy Shevchenko

There are few cleanups to the driver which already acked and reviewed.
I decide to split last series to two parts. This is first part.

Heikki Krogerus (4):
  dmaengine: dw_dmac: use helper macro module_platform_driver()
  dmaengine: dw_dmac: add module alias
  dmaengine: dw_dmac: remove CLK dependency
  dmaengine: dw_dmac: amend description and indentation

 drivers/dma/Kconfig   |    1 -
 drivers/dma/dw_dmac.c |   20 ++++++--------------
 2 files changed, 6 insertions(+), 15 deletions(-)

-- 
1.7.10.4


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-02 11:41 [PATCH 0/4] dw_dmac: few cleanups to the driver Andy Shevchenko
@ 2012-10-02 11:41 ` Andy Shevchenko
  2012-10-10  9:04   ` Andy Shevchenko
  2012-10-02 11:41 ` [PATCH 2/4] dmaengine: dw_dmac: add module alias Andy Shevchenko
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-02 11:41 UTC (permalink / raw)
  To: Viresh Kumar, Vinod Koul, linux-kernel, spear-devel, balbi
  Cc: Heikki Krogerus, Andy Shevchenko

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Since v3.2 we have nice macro to define the platform driver's init and exit
calls. This patch simplifies the dw_dmac driver by using that macro.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/dma/dw_dmac.c |   13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index c4b0eb3..0b88ced 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1700,6 +1700,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
 #endif
 
 static struct platform_driver dw_driver = {
+	.probe		= dw_probe,
 	.remove		= __devexit_p(dw_remove),
 	.shutdown	= dw_shutdown,
 	.driver = {
@@ -1709,17 +1710,7 @@ static struct platform_driver dw_driver = {
 	},
 };
 
-static int __init dw_init(void)
-{
-	return platform_driver_probe(&dw_driver, dw_probe);
-}
-subsys_initcall(dw_init);
-
-static void __exit dw_exit(void)
-{
-	platform_driver_unregister(&dw_driver);
-}
-module_exit(dw_exit);
+module_platform_driver(dw_driver);
 
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/4] dmaengine: dw_dmac: add module alias
  2012-10-02 11:41 [PATCH 0/4] dw_dmac: few cleanups to the driver Andy Shevchenko
  2012-10-02 11:41 ` [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver() Andy Shevchenko
@ 2012-10-02 11:41 ` Andy Shevchenko
  2012-10-10  9:00   ` Andy Shevchenko
  2012-10-02 11:42 ` [PATCH 3/4] dmaengine: dw_dmac: remove CLK dependency Andy Shevchenko
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-02 11:41 UTC (permalink / raw)
  To: Viresh Kumar, Vinod Koul, linux-kernel, spear-devel, balbi
  Cc: Heikki Krogerus, Andy Shevchenko

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

It's good to have a quasistatic name for the platform driver.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/dma/dw_dmac.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index 0b88ced..bbb2a82 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1712,6 +1712,7 @@ static struct platform_driver dw_driver = {
 
 module_platform_driver(dw_driver);
 
+MODULE_ALIAS("platform:dw_dmac");
 MODULE_LICENSE("GPL v2");
 MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");
 MODULE_AUTHOR("Haavard Skinnemoen (Atmel)");
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/4] dmaengine: dw_dmac: remove CLK dependency
  2012-10-02 11:41 [PATCH 0/4] dw_dmac: few cleanups to the driver Andy Shevchenko
  2012-10-02 11:41 ` [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver() Andy Shevchenko
  2012-10-02 11:41 ` [PATCH 2/4] dmaengine: dw_dmac: add module alias Andy Shevchenko
@ 2012-10-02 11:42 ` Andy Shevchenko
  2012-10-02 11:42 ` [PATCH 4/4] dmaengine: dw_dmac: amend description and indentation Andy Shevchenko
  2012-10-03  3:25 ` [PATCH 0/4] dw_dmac: few cleanups to the driver viresh kumar
  4 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-02 11:42 UTC (permalink / raw)
  To: Viresh Kumar, Vinod Koul, linux-kernel, spear-devel, balbi
  Cc: Heikki Krogerus, Andy Shevchenko

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

This driver could be used on different platforms. Thus, the HAVE_CLK dependency
is dropped away.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Felipe Balbi <balbi@ti.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
 drivers/dma/Kconfig |    1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 677cd6e..df32537 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -83,7 +83,6 @@ config INTEL_IOP_ADMA
 
 config DW_DMAC
 	tristate "Synopsys DesignWare AHB DMA support"
-	depends on HAVE_CLK
 	select DMA_ENGINE
 	default y if CPU_AT32AP7000
 	help
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/4] dmaengine: dw_dmac: amend description and indentation
  2012-10-02 11:41 [PATCH 0/4] dw_dmac: few cleanups to the driver Andy Shevchenko
                   ` (2 preceding siblings ...)
  2012-10-02 11:42 ` [PATCH 3/4] dmaengine: dw_dmac: remove CLK dependency Andy Shevchenko
@ 2012-10-02 11:42 ` Andy Shevchenko
  2012-10-03  3:25 ` [PATCH 0/4] dw_dmac: few cleanups to the driver viresh kumar
  4 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-02 11:42 UTC (permalink / raw)
  To: Viresh Kumar, Vinod Koul, linux-kernel, spear-devel, balbi
  Cc: Heikki Krogerus, Andy Shevchenko

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>

The driver will be used as a core part for various implementations of the
DesignWare DMA device. The patch adjusts description on the top and corrects
paragraph indentation in few places across the code.

Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/dma/dw_dmac.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
index bbb2a82..236ccb5 100644
--- a/drivers/dma/dw_dmac.c
+++ b/drivers/dma/dw_dmac.c
@@ -1,6 +1,5 @@
 /*
- * Driver for the Synopsys DesignWare DMA Controller (aka DMACA on
- * AVR32 systems.)
+ * Core driver for the Synopsys DesignWare DMA Controller
  *
  * Copyright (C) 2007-2008 Atmel Corporation
  * Copyright (C) 2010-2011 ST Microelectronics
@@ -9,6 +8,7 @@
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
 #include <linux/bitops.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
@@ -222,7 +222,6 @@ static inline void dwc_dump_chan_regs(struct dw_dma_chan *dwc)
 		channel_readl(dwc, CTL_LO));
 }
 
-
 static inline void dwc_chan_disable(struct dw_dma *dw, struct dw_dma_chan *dwc)
 {
 	channel_clear_bit(dw, CH_EN, dwc->mask);
@@ -1679,6 +1678,7 @@ static int dw_resume_noirq(struct device *dev)
 
 	clk_prepare_enable(dw->clk);
 	dma_writel(dw, CFG, DW_CFG_DMA_EN);
+
 	return 0;
 }
 
-- 
1.7.10.4


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH 0/4] dw_dmac: few cleanups to the driver
  2012-10-02 11:41 [PATCH 0/4] dw_dmac: few cleanups to the driver Andy Shevchenko
                   ` (3 preceding siblings ...)
  2012-10-02 11:42 ` [PATCH 4/4] dmaengine: dw_dmac: amend description and indentation Andy Shevchenko
@ 2012-10-03  3:25 ` viresh kumar
  4 siblings, 0 replies; 16+ messages in thread
From: viresh kumar @ 2012-10-03  3:25 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Vinod Koul, linux-kernel, spear-devel, balbi

On Tue, Oct 2, 2012 at 5:11 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> There are few cleanups to the driver which already acked and reviewed.
> I decide to split last series to two parts. This is first part.

Looks good.

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 2/4] dmaengine: dw_dmac: add module alias
  2012-10-02 11:41 ` [PATCH 2/4] dmaengine: dw_dmac: add module alias Andy Shevchenko
@ 2012-10-10  9:00   ` Andy Shevchenko
  0 siblings, 0 replies; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-10  9:00 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vinod Koul, linux-kernel, spear-devel, balbi, Heikki Krogerus

On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote: 
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> It's good to have a quasistatic name for the platform driver.
Please, do not push this one.
It makes no sense now. Better we have it after a split.

> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/dma/dw_dmac.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index 0b88ced..bbb2a82 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1712,6 +1712,7 @@ static struct platform_driver dw_driver = {
>  
>  module_platform_driver(dw_driver);
>  
> +MODULE_ALIAS("platform:dw_dmac");
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");
>  MODULE_AUTHOR("Haavard Skinnemoen (Atmel)");

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-02 11:41 ` [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver() Andy Shevchenko
@ 2012-10-10  9:04   ` Andy Shevchenko
  2012-10-10  9:08     ` viresh kumar
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-10  9:04 UTC (permalink / raw)
  To: Viresh Kumar
  Cc: Vinod Koul, linux-kernel, spear-devel, balbi, Heikki Krogerus

On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote: 
> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> 
> Since v3.2 we have nice macro to define the platform driver's init and exit
> calls. This patch simplifies the dw_dmac driver by using that macro.

Actually we can't do this. It will break initialization of some other
drivers.


> 
> Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
>  drivers/dma/dw_dmac.c |   13 ++-----------
>  1 file changed, 2 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/dma/dw_dmac.c b/drivers/dma/dw_dmac.c
> index c4b0eb3..0b88ced 100644
> --- a/drivers/dma/dw_dmac.c
> +++ b/drivers/dma/dw_dmac.c
> @@ -1700,6 +1700,7 @@ MODULE_DEVICE_TABLE(of, dw_dma_id_table);
>  #endif
>  
>  static struct platform_driver dw_driver = {
> +	.probe		= dw_probe,
>  	.remove		= __devexit_p(dw_remove),
>  	.shutdown	= dw_shutdown,
>  	.driver = {
> @@ -1709,17 +1710,7 @@ static struct platform_driver dw_driver = {
>  	},
>  };
>  
> -static int __init dw_init(void)
> -{
> -	return platform_driver_probe(&dw_driver, dw_probe);
> -}
> -subsys_initcall(dw_init);
> -
> -static void __exit dw_exit(void)
> -{
> -	platform_driver_unregister(&dw_driver);
> -}
> -module_exit(dw_exit);
> +module_platform_driver(dw_driver);
>  
>  MODULE_LICENSE("GPL v2");
>  MODULE_DESCRIPTION("Synopsys DesignWare DMA Controller driver");

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10  9:04   ` Andy Shevchenko
@ 2012-10-10  9:08     ` viresh kumar
  2012-10-10  9:21       ` Andy Shevchenko
  0 siblings, 1 reply; 16+ messages in thread
From: viresh kumar @ 2012-10-10  9:08 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Vinod Koul, linux-kernel, spear-devel, balbi, Heikki Krogerus

On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>
>> Since v3.2 we have nice macro to define the platform driver's init and exit
>> calls. This patch simplifies the dw_dmac driver by using that macro.
>
> Actually we can't do this. It will break initialization of some other
> drivers.

why?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10  9:08     ` viresh kumar
@ 2012-10-10  9:21       ` Andy Shevchenko
  2012-10-10  9:23         ` Viresh Kumar
  2012-10-10 12:40         ` Felipe Balbi
  0 siblings, 2 replies; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-10  9:21 UTC (permalink / raw)
  To: viresh kumar
  Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel, balbi,
	Heikki Krogerus, Mika Westerberg

On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
> On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
>>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>>>
>>> Since v3.2 we have nice macro to define the platform driver's init and exit
>>> calls. This patch simplifies the dw_dmac driver by using that macro.
>>
>> Actually we can't do this. It will break initialization of some other
>> drivers.
>
> why?

We have spi, i2c and hsuart devices connected to the DMA controller.
In case we would like to use DMA we have to have the dw_dmac loaded
before them. Currently we have spi driver on subsys_initcall level,
and Mika, who is developing it, will change to module_init_call level.
However, it will just hide the potential issue. He also tried to use
deferred module loading, but we don't know if it's good solution or
not, and that solution requires something to stop deferring at some
moment.

Might be we missed something and there is a better solution.

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10  9:21       ` Andy Shevchenko
@ 2012-10-10  9:23         ` Viresh Kumar
  2012-10-10 12:40         ` Felipe Balbi
  1 sibling, 0 replies; 16+ messages in thread
From: Viresh Kumar @ 2012-10-10  9:23 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andy Shevchenko, Vinod Koul, linux-kernel, spear-devel, balbi,
	Heikki Krogerus, Mika Westerberg

On 10 October 2012 14:51, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> We have spi, i2c and hsuart devices connected to the DMA controller.
> In case we would like to use DMA we have to have the dw_dmac loaded
> before them. Currently we have spi driver on subsys_initcall level,
> and Mika, who is developing it, will change to module_init_call level.
> However, it will just hide the potential issue. He also tried to use
> deferred module loading, but we don't know if it's good solution or
> not, and that solution requires something to stop deferring at some
> moment.
>
> Might be we missed something and there is a better solution.

My eyes aren't 6/6 now. I need specs now.
How could i miss that you just removed the very intentional subsys_init()
call. My bad.

I take my Acked-by: back :)

--
viresh

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10  9:21       ` Andy Shevchenko
  2012-10-10  9:23         ` Viresh Kumar
@ 2012-10-10 12:40         ` Felipe Balbi
  2012-10-10 12:52           ` Andy Shevchenko
  1 sibling, 1 reply; 16+ messages in thread
From: Felipe Balbi @ 2012-10-10 12:40 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: viresh kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	spear-devel, balbi, Heikki Krogerus, Mika Westerberg

[-- Attachment #1: Type: text/plain, Size: 1404 bytes --]

On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
> > On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> >> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
> >>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >>>
> >>> Since v3.2 we have nice macro to define the platform driver's init and exit
> >>> calls. This patch simplifies the dw_dmac driver by using that macro.
> >>
> >> Actually we can't do this. It will break initialization of some other
> >> drivers.
> >
> > why?
> 
> We have spi, i2c and hsuart devices connected to the DMA controller.
> In case we would like to use DMA we have to have the dw_dmac loaded
> before them. Currently we have spi driver on subsys_initcall level,
> and Mika, who is developing it, will change to module_init_call level.
> However, it will just hide the potential issue. He also tried to use
> deferred module loading, but we don't know if it's good solution or
> not, and that solution requires something to stop deferring at some
> moment.
> 
> Might be we missed something and there is a better solution.

if they can only work with DMA, they should return -EPROBE_DEFER so
their probe() function can be called after DMA driver has finished
probing.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10 12:40         ` Felipe Balbi
@ 2012-10-10 12:52           ` Andy Shevchenko
  2012-10-10 13:42             ` Felipe Balbi
  0 siblings, 1 reply; 16+ messages in thread
From: Andy Shevchenko @ 2012-10-10 12:52 UTC (permalink / raw)
  To: balbi
  Cc: viresh kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	spear-devel, Heikki Krogerus, Mika Westerberg

On Wed, Oct 10, 2012 at 3:40 PM, Felipe Balbi <balbi@ti.com> wrote:
> On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote:
>> On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
>> > On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
>> > <andriy.shevchenko@linux.intel.com> wrote:
>> >> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
>> >>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
>> >>>
>> >>> Since v3.2 we have nice macro to define the platform driver's init and exit
>> >>> calls. This patch simplifies the dw_dmac driver by using that macro.
>> >>
>> >> Actually we can't do this. It will break initialization of some other
>> >> drivers.
>> >
>> > why?
>>
>> We have spi, i2c and hsuart devices connected to the DMA controller.
>> In case we would like to use DMA we have to have the dw_dmac loaded
>> before them. Currently we have spi driver on subsys_initcall level,
>> and Mika, who is developing it, will change to module_init_call level.
>> However, it will just hide the potential issue. He also tried to use
>> deferred module loading, but we don't know if it's good solution or
>> not, and that solution requires something to stop deferring at some
>> moment.
>>
>> Might be we missed something and there is a better solution.
>
> if they can only work with DMA, they should return -EPROBE_DEFER so
> their probe() function can be called after DMA driver has finished
> probing.

They could work either with DMA or via PIO mode.
How does the driver know when to stop to return -EPROBE_DEFER?

-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10 12:52           ` Andy Shevchenko
@ 2012-10-10 13:42             ` Felipe Balbi
  2012-10-10 13:45               ` Felipe Balbi
  2012-10-11  5:39               ` Mika Westerberg
  0 siblings, 2 replies; 16+ messages in thread
From: Felipe Balbi @ 2012-10-10 13:42 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: balbi, viresh kumar, Andy Shevchenko, Vinod Koul, linux-kernel,
	spear-devel, Heikki Krogerus, Mika Westerberg

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

Hi,

On Wed, Oct 10, 2012 at 03:52:40PM +0300, Andy Shevchenko wrote:
> On Wed, Oct 10, 2012 at 3:40 PM, Felipe Balbi <balbi@ti.com> wrote:
> > On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote:
> >> On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
> >> > On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
> >> > <andriy.shevchenko@linux.intel.com> wrote:
> >> >> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
> >> >>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >> >>>
> >> >>> Since v3.2 we have nice macro to define the platform driver's init and exit
> >> >>> calls. This patch simplifies the dw_dmac driver by using that macro.
> >> >>
> >> >> Actually we can't do this. It will break initialization of some other
> >> >> drivers.
> >> >
> >> > why?
> >>
> >> We have spi, i2c and hsuart devices connected to the DMA controller.
> >> In case we would like to use DMA we have to have the dw_dmac loaded
> >> before them. Currently we have spi driver on subsys_initcall level,
> >> and Mika, who is developing it, will change to module_init_call level.
> >> However, it will just hide the potential issue. He also tried to use
> >> deferred module loading, but we don't know if it's good solution or
> >> not, and that solution requires something to stop deferring at some
> >> moment.
> >>
> >> Might be we missed something and there is a better solution.
> >
> > if they can only work with DMA, they should return -EPROBE_DEFER so
> > their probe() function can be called after DMA driver has finished
> > probing.
> 
> They could work either with DMA or via PIO mode.
> How does the driver know when to stop to return -EPROBE_DEFER?

Why would you even allow to work as PIO-only ? Who would even want to
use the driver as PIO only ?

In any case, you can add a Kconfig choice like WHATEVER_PIO_ONLY and
only return -EPROBE_DEFER ifndef WHATEVER_PIO_ONLY.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10 13:42             ` Felipe Balbi
@ 2012-10-10 13:45               ` Felipe Balbi
  2012-10-11  5:39               ` Mika Westerberg
  1 sibling, 0 replies; 16+ messages in thread
From: Felipe Balbi @ 2012-10-10 13:45 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Andy Shevchenko, viresh kumar, Andy Shevchenko, Vinod Koul,
	linux-kernel, spear-devel, Heikki Krogerus, Mika Westerberg

[-- Attachment #1: Type: text/plain, Size: 2367 bytes --]

Hi,

On Wed, Oct 10, 2012 at 04:42:00PM +0300, Felipe Balbi wrote:
> On Wed, Oct 10, 2012 at 03:52:40PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 10, 2012 at 3:40 PM, Felipe Balbi <balbi@ti.com> wrote:
> > > On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote:
> > >> On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
> > >> > On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
> > >> > <andriy.shevchenko@linux.intel.com> wrote:
> > >> >> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
> > >> >>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >> >>>
> > >> >>> Since v3.2 we have nice macro to define the platform driver's init and exit
> > >> >>> calls. This patch simplifies the dw_dmac driver by using that macro.
> > >> >>
> > >> >> Actually we can't do this. It will break initialization of some other
> > >> >> drivers.
> > >> >
> > >> > why?
> > >>
> > >> We have spi, i2c and hsuart devices connected to the DMA controller.
> > >> In case we would like to use DMA we have to have the dw_dmac loaded
> > >> before them. Currently we have spi driver on subsys_initcall level,
> > >> and Mika, who is developing it, will change to module_init_call level.
> > >> However, it will just hide the potential issue. He also tried to use
> > >> deferred module loading, but we don't know if it's good solution or
> > >> not, and that solution requires something to stop deferring at some
> > >> moment.
> > >>
> > >> Might be we missed something and there is a better solution.
> > >
> > > if they can only work with DMA, they should return -EPROBE_DEFER so
> > > their probe() function can be called after DMA driver has finished
> > > probing.
> > 
> > They could work either with DMA or via PIO mode.
> > How does the driver know when to stop to return -EPROBE_DEFER?
> 
> Why would you even allow to work as PIO-only ? Who would even want to
> use the driver as PIO only ?
> 
> In any case, you can add a Kconfig choice like WHATEVER_PIO_ONLY and
> only return -EPROBE_DEFER ifndef WHATEVER_PIO_ONLY.

yet another comment is that any user of this dma engine will have to
pass the filter function as argument to request_channel, which means
Modules.dep will make sure for dw_dmac to probe before its users.

What am I missing here ?

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
  2012-10-10 13:42             ` Felipe Balbi
  2012-10-10 13:45               ` Felipe Balbi
@ 2012-10-11  5:39               ` Mika Westerberg
  1 sibling, 0 replies; 16+ messages in thread
From: Mika Westerberg @ 2012-10-11  5:39 UTC (permalink / raw)
  To: Felipe Balbi
  Cc: Andy Shevchenko, viresh kumar, Andy Shevchenko, Vinod Koul,
	linux-kernel, spear-devel, Heikki Krogerus

On Wed, Oct 10, 2012 at 04:42:00PM +0300, Felipe Balbi wrote:
> Hi,
> 
> On Wed, Oct 10, 2012 at 03:52:40PM +0300, Andy Shevchenko wrote:
> > On Wed, Oct 10, 2012 at 3:40 PM, Felipe Balbi <balbi@ti.com> wrote:
> > > On Wed, Oct 10, 2012 at 12:21:04PM +0300, Andy Shevchenko wrote:
> > >> On Wed, Oct 10, 2012 at 12:08 PM, viresh kumar <viresh.kumar@linaro.org> wrote:
> > >> > On Wed, Oct 10, 2012 at 2:34 PM, Andy Shevchenko
> > >> > <andriy.shevchenko@linux.intel.com> wrote:
> > >> >> On Tue, 2012-10-02 at 14:41 +0300, Andy Shevchenko wrote:
> > >> >>> From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >> >>>
> > >> >>> Since v3.2 we have nice macro to define the platform driver's init and exit
> > >> >>> calls. This patch simplifies the dw_dmac driver by using that macro.
> > >> >>
> > >> >> Actually we can't do this. It will break initialization of some other
> > >> >> drivers.
> > >> >
> > >> > why?
> > >>
> > >> We have spi, i2c and hsuart devices connected to the DMA controller.
> > >> In case we would like to use DMA we have to have the dw_dmac loaded
> > >> before them. Currently we have spi driver on subsys_initcall level,
> > >> and Mika, who is developing it, will change to module_init_call level.
> > >> However, it will just hide the potential issue. He also tried to use
> > >> deferred module loading, but we don't know if it's good solution or
> > >> not, and that solution requires something to stop deferring at some
> > >> moment.
> > >>
> > >> Might be we missed something and there is a better solution.
> > >
> > > if they can only work with DMA, they should return -EPROBE_DEFER so
> > > their probe() function can be called after DMA driver has finished
> > > probing.
> > 
> > They could work either with DMA or via PIO mode.
> > How does the driver know when to stop to return -EPROBE_DEFER?
> 
> Why would you even allow to work as PIO-only ? Who would even want to
> use the driver as PIO only ?

Think about SPI or I2C, if we don't have DMA available we are still able to
use the driver (and the bus) instead of just failing.

> In any case, you can add a Kconfig choice like WHATEVER_PIO_ONLY and
> only return -EPROBE_DEFER ifndef WHATEVER_PIO_ONLY.

Why would we add more Kconfig options for things that can be checked
runtime? Distro makers need to select that option anyway so it doesn't gain
anything, except confuses users.

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2012-10-11  5:37 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-02 11:41 [PATCH 0/4] dw_dmac: few cleanups to the driver Andy Shevchenko
2012-10-02 11:41 ` [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver() Andy Shevchenko
2012-10-10  9:04   ` Andy Shevchenko
2012-10-10  9:08     ` viresh kumar
2012-10-10  9:21       ` Andy Shevchenko
2012-10-10  9:23         ` Viresh Kumar
2012-10-10 12:40         ` Felipe Balbi
2012-10-10 12:52           ` Andy Shevchenko
2012-10-10 13:42             ` Felipe Balbi
2012-10-10 13:45               ` Felipe Balbi
2012-10-11  5:39               ` Mika Westerberg
2012-10-02 11:41 ` [PATCH 2/4] dmaengine: dw_dmac: add module alias Andy Shevchenko
2012-10-10  9:00   ` Andy Shevchenko
2012-10-02 11:42 ` [PATCH 3/4] dmaengine: dw_dmac: remove CLK dependency Andy Shevchenko
2012-10-02 11:42 ` [PATCH 4/4] dmaengine: dw_dmac: amend description and indentation Andy Shevchenko
2012-10-03  3:25 ` [PATCH 0/4] dw_dmac: few cleanups to the driver viresh kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).