dw_dmac: few cleanups to the driver
cover dw_dmac: few cleanups to the driver
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(-)
Signature · 1 line
1.7.10.4
PATCH 1/4 dmaengine: dw_dmac: use helper macro module_platform_driver()
From: Heikki Krogerus <[email protected]>
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.
--- drivers/dma/dw_dmac.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)
drivers/dma/dw_dmac.c +2 -11
@@ -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" );
Signature · 1 line
1.7.10.4
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Quoted 4 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver by using that macro.
Actually we can't do this. It will break initialization of some other drivers.
Quoted 40 lines
Signed-off-by Heikki Krogerus <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>Reviewed-by Felipe Balbi <[email protected]>Acked-by Viresh Kumar <[email protected]>--- drivers/dma/dw_dmac.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)drivers/dma/dw_dmac.c +2
-11@@ -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" );
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Quoted 8 lines · Andy Shevchenko
Quoted 4 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver by using that macro.Actually we can't do this. It will break initialization of some other drivers.
why?
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Quoted 5 lines · viresh kumar
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver 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.
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Quoted 10 lines · Andy Shevchenko
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_dmacloaded before them. Currently we have spi driver onsubsys_initcalllevel, and Mika, who is developing it, will change tomodule_init_calllevel. 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
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
In reply to Andy ShevchenkoQuoted 23 lines · Andy Shevchenko
Quoted 5 lines · viresh kumar
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver 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_dmacloaded before them. Currently we have spi driver onsubsys_initcalllevel, and Mika, who is developing it, will change tomodule_init_calllevel. 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.
Quoted 1 line
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.
Signature · 1 line
balbi
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Quoted 24 lines · Felipe Balbi
Quoted 23 lines · Andy Shevchenko
Quoted 5 lines · viresh kumar
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver 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_dmacloaded before them. Currently we have spi driver onsubsys_initcalllevel, and Mika, who is developing it, will change tomodule_init_calllevel. 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.
Quoted 5 lines
Quoted 1 line
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?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Hi,
Quoted 25 lines · Andy Shevchenko
Quoted 24 lines · Felipe Balbi
Quoted 23 lines · Andy Shevchenko
Quoted 5 lines · viresh kumar
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver 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_dmacloaded before them. Currently we have spi driver onsubsys_initcalllevel, and Mika, who is developing it, will change tomodule_init_calllevel. 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.
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
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.
Signature · 1 line
balbi
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
Hi,
Quoted 32 lines · Felipe Balbi
Quoted 31 lines · Andy Shevchenko
Quoted 29 lines · Felipe Balbi
Quoted 23 lines · Andy Shevchenko
Quoted 5 lines · viresh kumar
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver 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_dmacloaded before them. Currently we have spi driver onsubsys_initcalllevel, and Mika, who is developing it, will change tomodule_init_calllevel. 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.Quoted 1 line
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.
Quoted 8 lines
Quoted 2 lines
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
Kconfigchoice likeWHATEVER_PIO_ONLYand only return -EPROBE_DEFER ifndefWHATEVER_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 ?
Signature · 1 line
balbi
Re: [PATCH 1/4] dmaengine: dw_dmac: use helper macro module_platform_driver()
In reply to Felipe BalbiQuoted 34 lines · Felipe Balbi
Hi,
Quoted 31 lines · Andy Shevchenko
Quoted 29 lines · Felipe Balbi
Quoted 23 lines · Andy Shevchenko
Quoted 5 lines · viresh kumar
Quoted 3 lines · Andy Shevchenko
Quoted 2 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
Since v3.2 we have nice macro to define the platform driver's init and exit calls. This patch simplifies the
dw_dmacdriver 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_dmacloaded before them. Currently we have spi driver onsubsys_initcalllevel, and Mika, who is developing it, will change tomodule_init_calllevel. 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.Quoted 1 line
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.
Quoted 5 lines
Quoted 2 lines
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.
Quoted 2 lines
In any case, you can add a
Kconfigchoice likeWHATEVER_PIO_ONLYand only return -EPROBE_DEFER ifndefWHATEVER_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.
PATCH 2/4 dmaengine: dw_dmac: add module alias
In reply to Andy ShevchenkoFrom: Heikki Krogerus <[email protected]>
It's good to have a quasistatic name for the platform driver.
--- drivers/dma/dw_dmac.c | 1 + 1 file changed, 1 insertion(+)
drivers/dma/dw_dmac.c +1 -0
@@ -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)" );
Signature · 1 line
1.7.10.4
Re: [PATCH 2/4] dmaengine: dw_dmac: add module alias
Quoted 3 lines · Andy Shevchenko
From: Heikki Krogerus <[email protected]>
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.
Quoted 21 lines
Signed-off-by Heikki Krogerus <[email protected]>Signed-off-by Andy Shevchenko <[email protected]>Reviewed-by Felipe Balbi <[email protected]>Acked-by Viresh Kumar <[email protected]>--- drivers/dma/dw_dmac.c | 1 + 1 file changed, 1 insertion(+)drivers/dma/dw_dmac.c +1
-0@@ -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)" );
Signature · 2 lines
Andy Shevchenko <[email protected]> Intel Finland Oy
PATCH 3/4 dmaengine: dw_dmac: remove CLK dependency
In reply to Andy ShevchenkoFrom: Heikki Krogerus <[email protected]>
This driver could be used on different platforms. Thus, the HAVE_CLK dependency is dropped away.
--- drivers/dma/Kconfig | 1 - 1 file changed, 1 deletion(-)
drivers/dma/Kconfig +0 -1
@@ -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
Signature · 1 line
1.7.10.4
PATCH 4/4 dmaengine: dw_dmac: amend description and indentation
In reply to Andy ShevchenkoFrom: Heikki Krogerus <[email protected]>
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.
--- drivers/dma/dw_dmac.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/dma/dw_dmac.c +3 -3
@@ -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 ; }
Signature · 1 line
1.7.10.4
Quoted 2 lines · 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.
Looks good.