clk: Add non CONFIG_HAVE_CLK routines
Subsystems (12)
v3 cover clk: Add non CONFIG_HAVE_CLK routines
Many drivers are shared between architectures that may or may not have HAVE_CLK selected for them. To remove compilation errors for them we enclose clk_*() calls in these drivers within #ifdef CONFIG_HAVE_CLK, #endif.
This patchset removes the need of these CONFIG_HAVE_CLK statements, by introducing dummy routines when HAVE_CLK is not selected by platforms. So, definition of these routines will always be available. These calls will return error for platforms that don't select HAVE_CLK.
I hope i don't break anything now. ;)
V2->V3:
- Dummy routines now return NULL or 0.
- All user drivers must fail if clk_get returned error other than NULL.
- All user drivers don't need to validate their clk pointer before every call to clk_*() routines.
- Patches dropped earlier are again taken back, as they were following similar approach to what is implemented now.
V1->V2:
- Removed few patches as they might break working drivers
- Updated 1st patch, as it doesn't apply cleanly on latest linux-next after this got applied.
commit a8a97db984bdc5e89d42e41891543d2daaf314cb Author: Mark Brown <[email protected]> Date: Thu Apr 5 11:42:09 2012 +0100 ARM: 7376/1: clkdev: Implement managed clk_get()
- Similarly, updated stmmac patch as there were updates for it too.
Viresh Kumar (12):
clk: Add non CONFIG_HAVE_CLK routines
clk: Remove redundant depends on from drivers/Kconfig i2c/i2c-pxa: Remove conditional compilation of clk code usb/marvell: Remove conditional compilation of clk code usb/musb: Remove conditional compilation of clk code ata/pata_arasan: Remove conditional compilation of clk code ata/sata_mv: Remove conditional compilation of clk code net/c_can: Remove conditional compilation of clk code net/stmmac: Remove conditional compilation of clk code gadget/m66592: Remove conditional compilation of clk code gadget/r8a66597: Remove conditional compilation of clk code usb/host/r8a66597: Remove conditional compilation of clk code
drivers/ata/pata_arasan_cf.c | 14 +-- drivers/ata/sata_mv.c | 32 ++--- drivers/clk/Kconfig | 2 - drivers/i2c/busses/i2c-pxa.c | 7 - drivers/net/can/c_can/c_can_platform.c | 8 - drivers/net/ethernet/stmicro/stmmac/stmmac.h | 41 ----- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 33 ++-- drivers/usb/gadget/m66592-udc.c | 9 +- drivers/usb/gadget/m66592-udc.h | 5 - drivers/usb/gadget/r8a66597-udc.c | 11 +- drivers/usb/gadget/r8a66597-udc.h | 5 - drivers/usb/host/r8a66597-hcd.c | 12 -- drivers/usb/host/r8a66597.h | 5 - drivers/usb/musb/musb_core.h | 8 - include/linux/clk.h | 168 +++++++++++++------- include/linux/platform_data/mv_usb.h | 9 - 16 files changed, 141 insertions(+), 228 deletions(-)
Signature · 1 line
1.7.9
PATCH v3 1/12 clk: Add non CONFIG_HAVE_CLK routines
Many drivers are shared between architectures that may or may not have HAVE_CLK selected for them. To remove compilation errors for them we enclose clk_*() calls in these drivers within #ifdef CONFIG_HAVE_CLK, #endif.
This patch removes the need of these CONFIG_HAVE_CLK statements, by introducing dummy routines when HAVE_CLK is not selected by platforms. So, definition of these routines will always be available. These calls will return error for platforms that don't select HAVE_CLK.
--- include/linux/clk.h | 168 +++++++++++++++++++++++++++++++++------------------ 1 files changed, 109 insertions(+), 59 deletions(-)
include/linux/clk.h +109 -59
@@ -84,6 +84,43 @@ int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb);#endif /* !CONFIG_COMMON_CLK */ /** +* clk_prepare - prepare a clock source +* @clk: clock source +* +* This prepares the clock source for use. +* +* Must not be called from within atomic context. +*/ +#ifdef CONFIG_HAVE_CLK_PREPARE +int clk_prepare (struct clk *clk); +#else +static inline int clk_prepare (struct clk *clk) +{ +might_sleep (); +return 0 ; +} +#endif + +/** +* clk_unprepare - undo preparation of a clock source +* @clk: clock source +* +* This undoes a previously prepared clock. The caller must balance +* the number of prepare and unprepare calls. +* +* Must not be called from within atomic context. +*/ +#ifdef CONFIG_HAVE_CLK_PREPARE +void clk_unprepare (struct clk *clk); +#else +static inline void clk_unprepare (struct clk *clk) +{ +might_sleep (); +} +#endif + +#ifdef CONFIG_HAVE_CLK +/** * clk_get - lookup and obtain a reference to a clock producer. * @dev: device for clock "consumer" * @id: clock comsumer ID
@@ -121,24 +158,6 @@ struct clk *clk_get(struct device *dev, const char *id);struct clk *devm_clk_get (struct device *dev,const char *id);/** -* clk_prepare - prepare a clock source -* @clk: clock source -* -* This prepares the clock source for use. -* -* Must not be called from within atomic context. -*/ -#ifdef CONFIG_HAVE_CLK_PREPARE -int clk_prepare (struct clk *clk); -#else -static inline int clk_prepare (struct clk *clk) -{ -might_sleep (); -return 0 ; -} -#endif - -/** * clk_enable - inform the system when the clock source should be running. * @clk: clock source *
@@ -166,47 +185,6 @@ int clk_enable(struct clk *clk);*/ void clk_disable (struct clk *clk); - -/** -* clk_unprepare - undo preparation of a clock source -* @clk: clock source -* -* This undoes a previously prepared clock. The caller must balance -* the number of prepare and unprepare calls. -* -* Must not be called from within atomic context. -*/ -#ifdef CONFIG_HAVE_CLK_PREPARE -void clk_unprepare (struct clk *clk); -#else -static inline void clk_unprepare (struct clk *clk) -{ -might_sleep (); -} -#endif - -/* clk_prepare_enable helps cases using clk_enable in non-atomic context. */ -static inline int clk_prepare_enable (struct clk *clk) -{ -int ret; - - ret =clk_prepare (clk); -if (ret) -return ret; - ret =clk_enable (clk); -if (ret) -clk_unprepare (clk); - -return ret; -} - -/* clk_disable_unprepare helps cases using clk_disable in non-atomic context. */ -static inline void clk_disable_unprepare (struct clk *clk) -{ -clk_disable (clk); -clk_unprepare (clk); -} -/** * clk_get_rate - obtain the current clock rate (in Hz) for a clock source. * This is only valid once the clock source has been enabled.
@@ -297,6 +275,78 @@ struct clk *clk_get_parent(struct clk *clk);*/ struct clk *clk_get_sys (const char *dev_id,const char *con_id); +#else /* !CONFIG_HAVE_CLK */ + +static inline struct clk *clk_get (struct device *dev,const char *id) +{ +return NULL ; +} + +static inline struct clk *devm_clk_get (struct device *dev,const char *id) +{ +return NULL ; +} + +static inline void clk_put (struct clk *clk) {} + +static inline void devm_clk_put (struct device *dev,struct clk *clk) {} + +static inline int clk_enable (struct clk *clk) +{ +return 0 ; +} + +static inline void clk_disable (struct clk *clk) {} + +static inline unsigned long clk_get_rate (struct clk *clk) +{ +return 0 ; +} + +static inline int clk_set_rate (struct clk *clk,unsigned long rate) +{ +return 0 ; +} + +static inline long clk_round_rate (struct clk *clk,unsigned long rate) +{ +return 0 ; +} + +static inline int clk_set_parent (struct clk *clk,struct clk *parent) +{ +return 0 ; +} + +static inline struct clk *clk_get_parent (struct clk *clk) +{ +return NULL ; +} + +#endif + +/* clk_prepare_enable helps cases using clk_enable in non-atomic context. */ +static inline int clk_prepare_enable (struct clk *clk) +{ +int ret; + + ret =clk_prepare (clk); +if (ret) +return ret; + ret =clk_enable (clk); +if (ret) +clk_unprepare (clk); + +return ret; +} + +/* clk_disable_unprepare helps cases using clk_disable in non-atomic context. */ +static inline void clk_disable_unprepare (struct clk *clk) +{ +clk_disable (clk); +clk_unprepare (clk); +} +/** * clk_add_alias - add a new clock alias * @alias: name for clock alias
Signature · 1 line
1.7.9
PATCH v3 2/12 clk: Remove redundant depends on from drivers/Kconfig
In reply to Viresh Kumarmenu "Common Clock Framework" has "depends on COMMON_CLK" and so configs defined within menu don't require these "depends on COMMON_CLK again".
--- drivers/clk/Kconfig | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
drivers/clk/Kconfig +0 -2
@@ -24,7 +24,6 @@ menu "Common Clock Framework" config COMMON_CLK_DISABLE_UNUSED bool "Disabled unused clocks at boot" - depends on COMMON_CLK ---help--- Traverses the entire clock tree and disables any clocks that are enabled in hardware but have not been enabled by any device drivers.
@@ -35,7 +34,6 @@ config COMMON_CLK_DISABLE_UNUSED config COMMON_CLK_DEBUG bool "DebugFS representation of clock tree" - depends on COMMON_CLK select DEBUG_FS ---help--- Creates a directory hierchy in debugfs for visualizing the clk
Signature · 1 line
1.7.9
PATCH v3 3/12 i2c/i2c-pxa: Remove conditional compilation of clk code
In reply to Viresh KumarFrom: Viresh Kumar <[email protected]>
With addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
pxa i2c also has these dummy macros defined locally. Remove them as they aren't required anymore.
--- drivers/i2c/busses/i2c-pxa.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
drivers/i2c/busses/i2c-pxa.c +0 -7
@@ -41,13 +41,6 @@#include <asm/irq.h> -#ifndef CONFIG_HAVE_CLK -#define clk_get (dev, id) NULL -#define clk_put (clk) do { } while (0) -#define clk_disable (clk) do { } while (0) -#define clk_enable (clk) do { } while (0) -#endif -struct pxa_reg_layout {u32 ibmr;u32 idbr;
Signature · 1 line
1.7.9
Re: [PATCH V3 03/12] i2c/i2c-pxa: Remove conditional compilation of clk code
Quoted 6 lines · Viresh Kumar
From: Viresh Kumar <[email protected]>
With addition of dummy clk_*() calls for non
CONFIG_HAVE_CLKcases inclk.h, there is no need to have clk code enclosed in #ifdefCONFIG_HAVE_CLK, #endif macros.
Quoted 6 lines
pxa i2c also has these dummy macros defined locally. Remove them as they aren't required anymore.
Signed-off-by Viresh Kumar <[email protected]>Cc Wolfram Sang <[email protected]>Cc linux-i2c at vger.kernel.org
Shall probably not go in via I2C-tree, so:
Signature · 9 lines
Pengutronix e.K. | Wolfram Sang | Industrial Linux Solutions | http://www.pengutronix.de/ | -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 198 bytes Desc: Digital signature URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20120424/e1afffa4/attachment.sig>
PATCH v3 4/12 usb/marvell: Remove conditional compilation of clk code
In reply to Viresh KumarFrom: Viresh Kumar <[email protected]>
With addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
Marvell usb also has these dummy macros defined locally. Remove them as they aren't required anymore.
--- include/linux/platform_data/mv_usb.h | 9 --------- 1 files changed, 0 insertions(+), 9 deletions(-)
include/linux/platform_data/mv_usb.h +0 -9
@@ -52,13 +52,4 @@ struct mv_usb_platform_data {int (*set_vbus)(unsigned int vbus); int (*private_init)(void __iomem *opregs, void __iomem *phyregs); }; - -#ifndef CONFIG_HAVE_CLK -/* Dummy stub for clk framework */ -#define clk_get (dev, id) NULL -#define clk_put (clock) do {} while (0) -#define clk_enable (clock) do {} while (0) -#define clk_disable (clock) do {} while (0) -#endif -#endif
Signature · 1 line
1.7.9
PATCH v3 5/12 usb/musb: Remove conditional compilation of clk code
In reply to Viresh KumarFrom: Viresh Kumar <[email protected]>
With addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
musb also has these dummy macros defined locally. Remove them as they aren't required anymore.
--- drivers/usb/musb/musb_core.h | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
drivers/usb/musb/musb_core.h +0 -8
@@ -81,14 +81,6 @@ struct musb_ep;#define is_peripheral_active (m) (!(m)->is_host)#define is_host_active (m) ((m)->is_host) -#ifndef CONFIG_HAVE_CLK -/* Dummy stub for clk framework */ -#define clk_get (dev, id) NULL -#define clk_put (clock) do {} while (0) -#define clk_enable (clock) do {} while (0) -#define clk_disable (clock) do {} while (0) -#endif -#ifdef CONFIG_PROC_FS #include <linux/fs.h> #define MUSB_CONFIG_PROC_FS
Signature · 1 line
1.7.9
PATCH v3 6/12 ata/pata_arasan: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/ata/pata_arasan_cf.c | 14 +------------- 1 files changed, 1 insertions(+), 13 deletions(-)
drivers/ata/pata_arasan_cf.c +1 -13
@@ -184,10 +184,8 @@struct arasan_cf_dev {/* pointer to ata_host structure */ struct ata_host *host; -/* clk structure, only if HAVE_CLK is defined */ -#ifdef CONFIG_HAVE_CLK +/* clk structure */ struct clk *clk; -#endif /* physical base address of controller */ dma_addr_t pbase;
@@ -312,13 +310,11 @@ static int cf_init(struct arasan_cf_dev *acdev)unsigned long flags;int ret =0 ; -#ifdef CONFIG_HAVE_CLK ret =clk_enable (acdev->clk);if (ret) {dev_dbg (acdev->host->dev,"clock enable failed" );return ret; } -#endif spin_lock_irqsave (&acdev->host->lock, flags);/* configure CF interface clock */
@@ -344,9 +340,7 @@ static void cf_exit(struct arasan_cf_dev *acdev)writel (readl (acdev->vbase +OP_MODE ) & ~CFHOST_ENB , acdev->vbase +OP_MODE );spin_unlock_irqrestore (&acdev->host->lock, flags); -#ifdef CONFIG_HAVE_CLK clk_disable (acdev->clk); -#endif }static void dma_callback (void *dev)
@@ -828,13 +822,11 @@ static int __devinit arasan_cf_probe(struct platform_device *pdev)return -ENOMEM ; } -#ifdef CONFIG_HAVE_CLK acdev->clk =clk_get (&pdev->dev,NULL );if (IS_ERR (acdev->clk)) {dev_warn (&pdev->dev,"Clock not found\n" );return PTR_ERR (acdev->clk); } -#endif /* allocate host */ host =ata_host_alloc (&pdev->dev,1 );
@@ -899,9 +891,7 @@ static int __devinit arasan_cf_probe(struct platform_device *pdev) &arasan_cf_sht); free_clk: -#ifdef CONFIG_HAVE_CLK clk_put (acdev->clk); -#endif return ret; }
@@ -912,9 +902,7 @@ static int __devexit arasan_cf_remove(struct platform_device *pdev)ata_host_detach (host);cf_exit (acdev); -#ifdef CONFIG_HAVE_CLK clk_put (acdev->clk); -#endif return 0 ; }
Signature · 1 line
1.7.9
PATCH v3 7/12 ata/sata_mv: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/ata/sata_mv.c | 32 ++++++++++---------------------- 1 files changed, 10 insertions(+), 22 deletions(-)
drivers/ata/sata_mv.c +10 -22
@@ -551,9 +551,7 @@ struct mv_host_priv {u32 irq_mask_offset;u32 unmask_all_irqs; -#if defined(CONFIG_HAVE_CLK )struct clk *clk; -#endif /* * These consistent DMA memory pools give us guaranteed * alignment for hardware-accessed data structures,
@@ -4063,13 +4061,13 @@ static int mv_platform_probe(struct platform_device *pdev) resource_size(res)); hpriv->base -=SATAHC0_REG_BASE ; -#if defined(CONFIG_HAVE_CLK ) hpriv->clk =clk_get (&pdev->dev,NULL ); -if (IS_ERR (hpriv->clk)) -dev_notice (&pdev->dev,"cannot get clkdev\n" ); -else -clk_enable (hpriv->clk); -#endif +if (IS_ERR (hpriv->clk)) { +dev_err (&pdev->dev,"cannot get clkdev\n" ); +return PTR_ERR (hpriv->clk); + } + +clk_enable (hpriv->clk);/* * (Re-)program MBUS remapping windows if we are asked to.
@@ -4096,12 +4094,8 @@ static int mv_platform_probe(struct platform_device *pdev)return 0 ; err: -#if defined(CONFIG_HAVE_CLK ) -if (!IS_ERR (hpriv->clk)) { -clk_disable (hpriv->clk); -clk_put (hpriv->clk); - } -#endif +clk_disable (hpriv->clk); +clk_put (hpriv->clk);return rc; }
@@ -4117,17 +4111,11 @@ err: static int __devexit mv_platform_remove(struct platform_device *pdev) {struct ata_host *host =platform_get_drvdata (pdev); -#if defined(CONFIG_HAVE_CLK )struct mv_host_priv *hpriv = host->private_data; -#endif ata_host_detach (host); -#if defined(CONFIG_HAVE_CLK ) -if (!IS_ERR (hpriv->clk)) { -clk_disable (hpriv->clk); -clk_put (hpriv->clk); - } -#endif +clk_disable (hpriv->clk); +clk_put (hpriv->clk);return 0 ; }
Signature · 1 line
1.7.9
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
drivers/ata/sata_mv.c static int mv_platform_probe(struct platform_device *pdev) · line 4070
Quoted 13 lines
drivers/ata/sata_mv.c +6
-6-#if defined(CONFIG_HAVE_CLK ) hpriv->clk =clk_get (&pdev->dev,NULL ); -if (IS_ERR (hpriv->clk)) -dev_notice (&pdev->dev,"cannot get clkdev\n" ); -else -clk_enable (hpriv->clk); -#endif +if (IS_ERR (hpriv->clk)) { +dev_err (&pdev->dev,"cannot get clkdev\n" ); +return PTR_ERR (hpriv->clk); + } + +clk_enable (hpriv->clk);
Sorry, but still wrong.
The clock is optional. If we can find a clock, turn it on. If not, keep going....
You patch causes the missing clock to become a fatal error.
This sata_mv exists in multiple forms. It can be part of a SoC. It can also be on a PCI bus. In the PCI form, it is unlikely to have a clk which can be controlled. When built into a SoC, namely one of the Orion family, dove, orion5x, mv78xx0 do not have a clock which can be controlled. However kirkwood does have a clock.
So, kirkwood will provide a clock and expects that sata_mv will turn it on. All the other ways of using sata_mv will not provide a clock, but still expect the driver to be happy.
Andrew
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Hi,
Quoted 16 lines · Andrew Lunn
drivers/ata/sata_mv.c static int mv_platform_probe(struct platform_device *pdev) · line 4070
Quoted 13 lines
drivers/ata/sata_mv.c +6
-6-#if defined(CONFIG_HAVE_CLK ) hpriv->clk =clk_get (&pdev->dev,NULL ); -if (IS_ERR (hpriv->clk)) -dev_notice (&pdev->dev,"cannot get clkdev\n" ); -else -clk_enable (hpriv->clk); -#endif +if (IS_ERR (hpriv->clk)) { +dev_err (&pdev->dev,"cannot get clkdev\n" ); +return PTR_ERR (hpriv->clk); + } + +clk_enable (hpriv->clk);Sorry, but still wrong.
Quoted 5 lines
The clock is optional. If we can find a clock, turn it on. If not, keep going....
You patch causes the missing clock to become a fatal error.
The clock API should accept NULL pointers as valid clocks and treat them as NOP.
Thus drivers wouldn't have to worry about whether they actually got a clock to manage or if their clocks are just dummies.
Lothar Wa?mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
In reply to Andrew LunnQuoted 13 lines · Andrew Lunn
Sorry, but still wrong.
The clock is optional. If we can find a clock, turn it on. If not, keep going....
You patch causes the missing clock to become a fatal error.
This
sata_mvexists in multiple forms. It can be part of a SoC. It can also be on a PCI bus. In the PCI form, it is unlikely to have a clk which can be controlled. When built into a SoC, namely one of the Orion family, dove, orion5x, mv78xx0 do not have a clock which can be controlled. However kirkwood does have a clock.
Quoted 3 lines
So, kirkwood will provide a clock and expects that
sata_mvwill turn it on. All the other ways of usingsata_mvwill not provide a clock, but still expect the driver to be happy.
Hmm. What this code does now is:
If HAVE_CLK is selected, then there must be a clock for the device. Otherwise it will always pass.
You want not to return error if a platform does have HAVE_CLK, but doesn't have a clock for sata? That would be simple to fix, but want to confirm if this is actually required.
@Russell: Can we have your view also?
--
viresh
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 14 lines · viresh kumar
Quoted 13 lines · Andrew Lunn
Sorry, but still wrong.
The clock is optional. If we can find a clock, turn it on. If not, keep going....
You patch causes the missing clock to become a fatal error.
This
sata_mvexists in multiple forms. It can be part of a SoC. It can also be on a PCI bus. In the PCI form, it is unlikely to have a clk which can be controlled. When built into a SoC, namely one of the Orion family, dove, orion5x, mv78xx0 do not have a clock which can be controlled. However kirkwood does have a clock.
Quoted 8 lines
Quoted 3 lines
So, kirkwood will provide a clock and expects that
sata_mvwill turn it on. All the other ways of usingsata_mvwill not provide a clock, but still expect the driver to be happy.Hmm. What this code does now is:
IfHAVE_CLKis selected, then there must be a clock for the device. Otherwise it will always pass.
-#if defined(CONFIG_HAVE_CLK)
hpriv->clk = clk_get(&pdev->dev, NULL);
- if (IS_ERR(hpriv->clk))
- dev_notice(&pdev->dev, "cannot get clkdev\n");
- else
- clk_enable(hpriv->clk);
-#endif
+ if (IS_ERR(hpriv->clk)) {
+ dev_err(&pdev->dev, "cannot get clkdev\n");
+ return PTR_ERR(hpriv->clk);
+ }
+
+ clk_enable(hpriv->clk);
There are three use cases....
1) The platform does not implement HAVE_CLK. So we are using your NOP operations.
clk_get() returns NULL.
IS_ERR(NULL) is false. So it keeps going, calls clk_enable() which is also NOP and the driver is happy.
2) The platform does have HAVE_CLK. So we are using driver/clk/ code. There is no clk defined for the device, since there is no controllable clk. Its using the PCI clock, or some hard wired internal SoC clock.
clk_get() returns ERR_PTR(-ENOENT)
IS_ERR(hpriv->clk) is true, you output a dev_err() and the device probe fails.
This is wrong. Not having the clk is not fatal. The old code would carefully not call clk_enable(), since it has an error pointer, not have a valid clk, and would also not call clk_disable during removal.
3) The platform does have HAVE_CLK. So we are using driver/clk/ code. There is however a gateable clock driving this lump of silicon.
clk_get() returns a valid pointer to a clk. IS_ERR(hpriv->clk) is false, so it keeps going. clk_enable() is called and the driver is happy.
Well, actually, his brings up a new issues
static int __clk_enable(struct clk *clk)
{
int ret = 0;
if (!clk)
return 0;
if (WARN_ON(clk->prepare_count == 0))
return -ESHUTDOWN; this should actually be clk_prepare_enable(). Did you see my patches adding generic clk framework support for Orion. I fixed this as part of that patch set.
Anyway.... You asked:
Quoted 3 lines
You want not to return error if a platform does have
HAVE_CLK, but doesn't have a clock for sata? That would be simple to fix, but want to confirm if this is actually required.
Correct.
Andrew
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 4 lines · Andrew Lunn
This is wrong. Not having the clk is not fatal. The old code would carefully not call
clk_enable(), since it has an error pointer, not have a valid clk, and would also not callclk_disableduring removal.
Whatever i wrote was intentional, because i didn't had this expectation of yours. :(
Please see if following is fine:
From: Viresh Kumar <[email protected]>
Date: Sat, 21 Apr 2012 11:16:53 +0530
Subject: [PATCH V3 Resend] ata/sata_mv: Remove conditional compilation of clk code
With addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/ata/sata_mv.c | 10 ---------- 1 files changed, 0 insertions(+), 10 deletions(-)
drivers/ata/sata_mv.c +0 -2
@@ -551,9 +551,7 @@ struct mv_host_priv {u32 irq_mask_offset;u32 unmask_all_irqs; -#if defined(CONFIG_HAVE_CLK )struct clk *clk; -#endif /* * These consistent DMA memory pools give us guaranteed * alignment for hardware-accessed data structures,
@@ -4063,13 +4061,11 @@ static int mv_platform_probe(struct
platform_device *pdev)
resource_size(res));
hpriv->base -= SATAHC0_REG_BASE;
-#if defined(CONFIG_HAVE_CLK)
hpriv->clk = clk_get(&pdev->dev, NULL);
if (IS_ERR(hpriv->clk))
dev_notice(&pdev->dev, "cannot get clkdev\n");
else
clk_enable(hpriv->clk);
-#endif
/*
* (Re-)program MBUS remapping windows if we are asked to.
@@ -4096,12 +4092,10 @@ static int mv_platform_probe(struct platform_device *pdev)
return 0;
err:
-#if defined(CONFIG_HAVE_CLK)
if (!IS_ERR(hpriv->clk)) {
clk_disable(hpriv->clk);
clk_put(hpriv->clk);
}
-#endif
return rc;
}
@@ -4117,17 +4111,13 @@ err:
static int __devexit mv_platform_remove(struct platform_device *pdev) {
struct ata_host *host = platform_get_drvdata(pdev);
-#if defined(CONFIG_HAVE_CLK)
struct mv_host_priv *hpriv = host->private_data;
-#endif
ata_host_detach(host);
-#if defined(CONFIG_HAVE_CLK)
if (!IS_ERR(hpriv->clk)) {
clk_disable(hpriv->clk);
clk_put(hpriv->clk);
}
-#endif
return 0;
}
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 1 line
Please see if following is fine:
Hi Viresh
This looks good. Thanks
Andrew
Quoted 68 lines
Signed-off-by Viresh Kumar <[email protected]>Cc Jeff Garzik <[email protected]>Cc Andrew Lunn <[email protected]>--- drivers/ata/sata_mv.c | 10 ---------- 1 files changed, 0 insertions(+), 10 deletions(-)drivers/ata/sata_mv.c +0
-4@@ -551,9 +551,7 @@ struct mv_host_priv {u32 irq_mask_offset;u32 unmask_all_irqs; -#if defined(CONFIG_HAVE_CLK )struct clk *clk; -#endif /* * These consistent DMA memory pools give us guaranteed * alignment for hardware-accessed data structures, @@ -4063,13 +4061,11 @@ static int mv_platform_probe(struct platform_device *pdev) resource_size(res)); hpriv->base -=SATAHC0_REG_BASE ; -#if defined(CONFIG_HAVE_CLK ) hpriv->clk =clk_get (&pdev->dev,NULL );if (IS_ERR (hpriv->clk))dev_notice (&pdev->dev,"cannot get clkdev\n" );else clk_enable (hpriv->clk); -#endif /* drivers/ata/sata_mv.c +0
-2* (Re-)program MBUS remapping windows if we are asked to. @@ -4096,12 +4092,10 @@ static int mv_platform_probe(struct platform_device *pdev)return 0 ; err: -#if defined(CONFIG_HAVE_CLK )if (!IS_ERR (hpriv->clk)) {clk_disable (hpriv->clk);clk_put (hpriv->clk); } -#endif return rc;}
drivers/ata/sata_mv.c +0
-4@@ -4117,17 +4111,13 @@ err: static int __devexit mv_platform_remove(struct platform_device *pdev) {struct ata_host *host =platform_get_drvdata (pdev); -#if defined(CONFIG_HAVE_CLK )struct mv_host_priv *hpriv = host->private_data; -#endif ata_host_detach (host); -#if defined(CONFIG_HAVE_CLK )if (!IS_ERR (hpriv->clk)) {clk_disable (hpriv->clk);clk_put (hpriv->clk); } -#endif return 0 ; }
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
In reply to Andrew LunnQuoted 10 lines · Andrew Lunn
static int
__clk_enable(struct clk *clk){ int ret = 0; if (!clk) return 0; if (WARN_ON(clk->prepare_count == 0)) return -ESHUTDOWN;
Quoted 3 lines
this should actually be
clk_prepare_enable(). Did you see my patches adding generic clk framework support for Orion. I fixed this as part of that patch set.
Sorry for missing it earlier. Ya i know it must be clk_prepare_enable(), but it wasn't motive of my patchset.
Sorry, i haven't gone through your patches for Orion.
--
viresh
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
In reply to viresh kumarQuoted 22 lines · viresh kumar
Quoted 13 lines · Andrew Lunn
Sorry, but still wrong.
The clock is optional. If we can find a clock, turn it on. If not, keep going....
You patch causes the missing clock to become a fatal error.
This
sata_mvexists in multiple forms. It can be part of a SoC. It can also be on a PCI bus. In the PCI form, it is unlikely to have a clk which can be controlled. When built into a SoC, namely one of the Orion family, dove, orion5x, mv78xx0 do not have a clock which can be controlled. However kirkwood does have a clock.Quoted 3 lines
So, kirkwood will provide a clock and expects that
sata_mvwill turn it on. All the other ways of usingsata_mvwill not provide a clock, but still expect the driver to be happy.Hmm. What this code does now is:
IfHAVE_CLKis selected, then there must be a clock for the device. Otherwise it will always pass.
Quoted 5 lines
You want not to return error if a platform does have
HAVE_CLK, but doesn't have a clock for sata? That would be simple to fix, but want to confirm if this is actually required.@Russell: Can we have your view also?
Look, it's very very simple.
As far as drivers are concerned:
clk_get() returns an opaque cookie which _happens_ to be called 'struct clk'. Drivers _must_ _not_ dereference or interpret this cookie in any way, apart from the singular case where they use IS_ERR() to determine if clk_get() failed, and PTR_ERR() to translate that into an error value. As far as drivers are concerned _everything_ _else_ is a valid cookie and must never be treated specially.
That much I hope is (and has been) totally crystal clear for some time.
Now, for drivers which use the clk API, and are used on platforms which have the clk API and those which do not have the CLK API. Those which do have the clk API define HAVE_CLK. We know how to deal with those, and that's through having a correct and valid clk API implementation.
For those which don't, as I've already suggested, we need clk_get() to return a non-error value. I really don't care what value it returns, because as far as drivers using the clk API are concerned, they are not allowed to interpret the value in _any_ _other_ _way_ other than whether it is an error or not. So NULL is a good value for this. It's a non-error cookie value, but (void *)1 is also good too.
Now, the question comes: do we want to provide a dummy API? Yes. How do we want to enable the provision of the dummy API? Through !HAVE_CLK? I think that's a sane move, and any driver which _really_ _does_ have a hard dependency on the clk API (eg, amba-clcd needing the clk API to control the LCD pixel clock rate) should depend on this symbol.
As for drivers printing out crap if they don't have the clk API configured, wtf? What does it matter? If the clk API is not configured, it means the platform has no control over the clocking, and the clocking is fixed. So why tell the user of each driver which could have clk API support that same fact over and over again during the kernel boot process? What do you expect the user to do about it? Scream at the manufacturer that they didn't implement a feature found on embedded devices on their swankey platform? Maybe its not appropriate there?
Finally, if a platform has clk API support enabled, and a driver requests a clock, and clk_get() returns an error, it means the clock was not found. That's a fatal error for the driver, because it means that something is wrong in the lookup tables - moreover, it means that _potentially_ someone screwed up the clk matching and this device has a clock which needs some control, but wasn't found. I don't think ignoring that kind of error, even by printing a warning, is a particularly good approach - it seems to me it makes things fragile. What if this missing clock causes the bus to your device to ultimately hang?
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 1 line · Russell King - ARM Linux
Look, it's very very simple.
Thanks for explaining.
Quoted 9 lines
Finally, if a platform has clk API support enabled, and a driver requests a clock, and
clk_get()returns an error, it means the clock was not found. That's a fatal error for the driver, because it means that something is wrong in the lookup tables - moreover, it means that _potentially_ someone screwed up the clk matching and this device has a clock which needs some control, but wasn't found. ?I don't think ignoring that kind of error, even by printing a warning, is a particularly good approach - it seems to me it makes things fragile. ?What if this missing clock causes the bus to your device to ultimately hang?
This is what i was thinking too and thats why floated this version of patch. But as Andrew said, clk API support is enabled for them, but still they don't have a clk for sata. To get this working, there are two solutions:
- Create dummy clk for sata for that platform, so clk_get doesn't fail.
- Check for error before every call to clk APIs after clk_get().
Andrew favored the second one. Which means, even on platforms with clk API defined and clk enable required, if there are some issues with lookup table, and clk_get() fails, system may hang when registers are accessed. For this i favored first one.
--
viresh
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 10 lines
This is what i was thinking too and thats why floated this version of patch. But as Andrew said, clk API support is enabled for them, but still they don't have a clk for sata. To get this working, there are two solutions:
- Create dummy clk for sata for that platform, soclk_getdoesn't fail.
- Check for error before every call to clk APIs afterclk_get().Andrew favored the second one. Which means, even on platforms with clk API defined and clk enable required, if there are some issues with lookup table, and
clk_get()fails, system may hang when registers are accessed. For this i favored first one.
I'm not too sure how you are going to achieve 1)
config SATA_MV
tristate "Marvell SATA support"
help
This option enables support for the Marvell Serial ATA family. Currently supports 88SX[56]0[48][01] PCI(-X) chips, as well as the newer [67]042 PCI-X/PCIe and SOC devices.
So this driver can be used with anything which has a PCI(-X) or PCIe bus, or Orion SoC and a few PowerPC SoCs.
The SoCs are not too bad, we know which ones they are and we can add dummy entries to their device tree. However, how do you want to handle the PCI devices? Create the dummy entry somewhere in the middle of the PCI core?
Thought not.
A comment to:
Quoted 1 line
- Check for error before every call to clk APIs after
clk_get().
There are only two calls. clk_prepare_enable() in the probe and clk_disable_unprepare() in the remove function.
Andrew
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Hi,
Quoted 23 lines · Andrew Lunn
Quoted 10 lines
This is what i was thinking too and thats why floated this version of patch. But as Andrew said, clk API support is enabled for them, but still they don't have a clk for sata. To get this working, there are two solutions:
- Create dummy clk for sata for that platform, soclk_getdoesn't fail.
- Check for error before every call to clk APIs afterclk_get().Andrew favored the second one. Which means, even on platforms with clk API defined and clk enable required, if there are some issues with lookup table, and
clk_get()fails, system may hang when registers are accessed. For this i favored first one.I'm not too sure how you are going to achieve 1)
config
SATA_MV
tristate "Marvell SATA support"
help
This option enables support for the Marvell Serial ATA family. Currently supports 88SX[56]0[48][01] PCI(-X) chips, as well as the newer [67]042 PCI-X/PCIe and SOC devices.So this driver can be used with anything which has a PCI(-X) or PCIe bus, or Orion SoC and a few PowerPC SoCs.
Quoted 7 lines
The SoCs are not too bad, we know which ones they are and we can add dummy entries to their device tree. However, how do you want to handle the PCI devices? Create the dummy entry somewhere in the middle of the PCI core?
Thought not.
AFAICT the PCI archs do not have HAVE_CLK enabled. Thus they will have a dummy clk API in place that provides the driver with a non-error clk_get() return value.
Lothar Wa?mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 3 lines
AFAICT the PCI archs do not have
HAVE_CLKenabled. Thus they will have a dummy clk API in place that provides the driver with a non-errorclk_get()return value.
Hi Lothar
Please could you explain this a little bit more. What exactly is a PCI arch?
Most of the Orion SoC have PCIe busses. Does this make them an PCI arch? Because they also have HAVE_CLK. There are more SoCs with PCI.
Thanks
Andrew
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 4 lines · Andrew Lunn
Quoted 3 lines
AFAICT the PCI archs do not have
HAVE_CLKenabled. Thus they will have a dummy clk API in place that provides the driver with a non-errorclk_get()return value.
Quoted 8 lines
Hi Lothar
Please could you explain this a little bit more. What exactly is a PCI arch?
Most of the Orion SoC have PCIe busses. Does this make them an PCI arch? Because they also have
HAVE_CLK. There are more SoCs with PCI.
If an arch has HAVE_CLK enabled it must provide valid clocks (be it dummy clocks) for all devices it supports.
Lothar Wa?mann
Signature · 9 lines
___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra?e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch?ftsf?hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info at karo-electronics.de ___________________________________________________________
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 2 lines
If an arch has
HAVE_CLKenabled it must provide valid clocks (be it dummy clocks) for all devices it supports.
So, lets take the theoretical exaple of a unicore32 PUV3
config ARCH_PUV3
def_bool y
select CPU_UCV2
select GENERIC_CLOCKEVENTS
select HAVE_CLK
select ARCH_REQUIRE_GPIOLIB
select ARCH_HAS_CPUFREQ
Seems like this somewhat unknown, to me at least, architecture, also supports PCI. So i plug in an HP Adaptec AIC8120 SATA host bus adapter into a spare PCI slot. This uses the Marvell 88SX6041, which the SATA_MV driver supports.
Should i expect that the unicore32 PUV3 has created a dummy clk for this case?
Thanks
Andrew
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
Quoted 13 lines · Andrew Lunn
Quoted 2 lines
If an arch has
HAVE_CLKenabled it must provide valid clocks (be it dummy clocks) for all devices it supports.So, lets take the theoretical exaple of a unicore32 PUV3
config
ARCH_PUV3
def_booly
selectCPU_UCV2
selectGENERIC_CLOCKEVENTS
selectHAVE_CLK
selectARCH_REQUIRE_GPIOLIB
selectARCH_HAS_CPUFREQ
Quoted 7 lines
Seems like this somewhat unknown, to me at least, architecture, also supports PCI. So i plug in an HP Adaptec AIC8120 SATA host bus adapter into a spare PCI slot. This uses the Marvell 88SX6041, which the
SATA_MVdriver supports.Should i expect that the unicore32 PUV3 has created a dummy clk for this case?
Are you going to bother answering the detailed email I sent describing my POV on this case?
Re: [PATCH V3 07/12] ata/sata_mv: Remove conditional compilation of clk code
In reply to Russell King - ARM LinuxQuoted 60 lines · Russell King - ARM Linux
Quoted 22 lines · viresh kumar
Quoted 13 lines · Andrew Lunn
Sorry, but still wrong.
The clock is optional. If we can find a clock, turn it on. If not, keep going....
You patch causes the missing clock to become a fatal error.
This
sata_mvexists in multiple forms. It can be part of a SoC. It can also be on a PCI bus. In the PCI form, it is unlikely to have a clk which can be controlled. When built into a SoC, namely one of the Orion family, dove, orion5x, mv78xx0 do not have a clock which can be controlled. However kirkwood does have a clock.Quoted 3 lines
So, kirkwood will provide a clock and expects that
sata_mvwill turn it on. All the other ways of usingsata_mvwill not provide a clock, but still expect the driver to be happy.Hmm. What this code does now is:
IfHAVE_CLKis selected, then there must be a clock for the device. Otherwise it will always pass.Quoted 5 lines
You want not to return error if a platform does have
HAVE_CLK, but doesn't have a clock for sata? That would be simple to fix, but want to confirm if this is actually required.@Russell: Can we have your view also?
Look, it's very very simple.
As far as drivers are concerned:
clk_get()returns an opaque cookie which _happens_ to be called 'struct clk'. Drivers _must_ _not_ dereference or interpret this cookie in any way, apart from the singular case where they useIS_ERR()to determine ifclk_get()failed, andPTR_ERR()to translate that into an error value. As far as drivers are concerned _everything_ _else_ is a valid cookie and must never be treated specially.That much I hope is (and has been) totally crystal clear for some time.
Now, for drivers which use the clk API, and are used on platforms which have the clk API and those which do not have the CLK API. Those which do have the clk API define
HAVE_CLK. We know how to deal with those, and that's through having a correct and valid clk API implementation.For those which don't, as I've already suggested, we need
clk_get()to return a non-error value. I really don't care what value it returns, because as far as drivers using the clk API are concerned, they are not allowed to interpret the value in _any_ _other_ _way_ other than whether it is an error or not. So NULL is a good value for this. It's a non-error cookie value, but (void *)1 is also good too.Now, the question comes: do we want to provide a dummy API? Yes. How do we want to enable the provision of the dummy API? Through !HAVE_CLK? I think that's a sane move, and any driver which _really_ _does_ have a hard dependency on the clk API (eg, amba-clcd needing the clk API to control the LCD pixel clock rate) should depend on this symbol.
Quoted 9 lines
As for drivers printing out crap if they don't have the clk API configured, wtf? What does it matter? If the clk API is not configured, it means the platform has no control over the clocking, and the clocking is fixed. So why tell the user of each driver which could have clk API support that same fact over and over again during the kernel boot process? What do you expect the user to do about it? Scream at the manufacturer that they didn't implement a feature found on embedded devices on their swankey platform? Maybe its not appropriate there?
Hi Russell
No problems with what is above. The bit in contention is this
Quoted 9 lines
Finally, if a platform has clk API support enabled, and a driver requests a clock, and
clk_get()returns an error, it means the clock was not found. That's a fatal error for the driver, because it means that something is wrong in the lookup tables - moreover, it means that _potentially_ someone screwed up the clk matching and this device has a clock which needs some control, but wasn't found. I don't think ignoring that kind of error, even by printing a warning, is a particularly good approach - it seems to me it makes things fragile. What if this missing clock causes the bus to your device to ultimately hang?
You are correct about lockup. I made a typo, match failed, lateinit turned the clock off, and the device hung on the next access. Is that fragile? It should only happen to somebody porting to a new SoC playing with clks.
Anyway, what you want, is that the MV_SATA driver knows if there should be a clock and only then call clk_get(). How do we reliably teach the MV_SATA driver this, so we don't cause an regressions?
If this platform_device is for a PCI bus device, there probably won't be a clock. If this is a SoC platform_device is for a Dove, Orion5x, mv78xx0, there won't be a clock. If its a kirkwood SoC platform device, there should be a clock. If its a PowerPC platform_device, i've no idea.
Have i missed something? It seems to come down to, a bit of fragile handling of clks, or possibly regressing some PowerPC machines.
Andrew
PATCH v3 8/12 net/c_can: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/net/can/c_can/c_can_platform.c | 8 -------- 1 files changed, 0 insertions(+), 8 deletions(-)
drivers/net/can/c_can/c_can_platform.c +0 -8
@@ -73,7 +73,6 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev)struct c_can_priv *priv;struct resource *mem;int irq; -#ifdef CONFIG_HAVE_CLK struct clk *clk;/* get the appropriate clk */
@@ -83,7 +82,6 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev) ret = -ENODEV ; goto exit; } -#endif /* get the platform data */ mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 );
@@ -118,10 +116,8 @@ static int __devinit c_can_plat_probe(struct platform_device *pdev) dev->irq = irq; priv->regs = addr; -#ifdef CONFIG_HAVE_CLK priv->can.clock.freq =clk_get_rate (clk); priv->priv = clk; -#endif switch (mem->flags &IORESOURCE_MEM_TYPE_MASK ) {case IORESOURCE_MEM_32BIT :
@@ -157,10 +153,8 @@ exit_iounmap: exit_release_mem:release_mem_region (mem->start,resource_size (mem)); exit_free_clk: -#ifdef CONFIG_HAVE_CLKclk_put (clk); exit: -#endifdev_err (&pdev->dev,"probe failed\n" );return ret;
@@ -181,9 +175,7 @@ static int __devexit c_can_plat_remove(struct platform_device *pdev) mem =platform_get_resource (pdev,IORESOURCE_MEM ,0 );release_mem_region (mem->start,resource_size (mem)); -#ifdef CONFIG_HAVE_CLK clk_put (priv->priv); -#endif return 0 ; }
Signature · 1 line
1.7.9
PATCH v3 9/12 net/stmmac: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
This also fixes error paths of probe(), as a goto is required in this patch.
--- drivers/net/ethernet/stmicro/stmmac/stmmac.h | 41 --------------------- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 33 +++++++++-------- 2 files changed, 17 insertions(+), 57 deletions(-)
drivers/net/ethernet/stmicro/stmmac/stmmac.h +0 -41
@@ -81,9 +81,7 @@ struct stmmac_priv {struct stmmac_counters mmc;struct dma_features dma_cap;int hw_cap_support; -#ifdef CONFIG_HAVE_CLK struct clk *stmmac_clk; -#endif int clk_csr; };
@@ -103,42 +101,3 @@ int stmmac_dvr_remove(struct net_device *ndev);struct stmmac_priv *stmmac_dvr_probe (struct device *device,struct plat_stmmacenet_data *plat_dat,void __iomem *addr); - -#ifdef CONFIG_HAVE_CLK -static inline int stmmac_clk_enable (struct stmmac_priv *priv) -{ -if (!IS_ERR (priv->stmmac_clk)) -return clk_enable (priv->stmmac_clk); - -return 0 ; -} - -static inline void stmmac_clk_disable (struct stmmac_priv *priv) -{ -if (IS_ERR (priv->stmmac_clk)) -return ; - -clk_disable (priv->stmmac_clk); -} -static inline int stmmac_clk_get (struct stmmac_priv *priv) -{ - priv->stmmac_clk =clk_get (priv->device,NULL ); - -if (IS_ERR (priv->stmmac_clk)) -return PTR_ERR (priv->stmmac_clk); - -return 0 ; -} -#else -static inline int stmmac_clk_enable (struct stmmac_priv *priv) -{ -return 0 ; -} -static inline void stmmac_clk_disable (struct stmmac_priv *priv) -{ -} -static inline int stmmac_clk_get (struct stmmac_priv *priv) -{ -return 0 ; -} -#endif /* CONFIG_HAVE_CLK */
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +17 -16
@@ -28,6 +28,7 @@https://bugzilla.stlinux.com/ *******************************************************************************/ +#include <linux/clk.h> #include <linux/kernel.h> #include <linux/interrupt.h> #include <linux/ip.h>
@@ -165,12 +166,8 @@ static void stmmac_verify_args(void)static void stmmac_clk_csr_set (struct stmmac_priv *priv) { -#ifdef CONFIG_HAVE_CLK u32 clk_rate; -if (IS_ERR (priv->stmmac_clk)) -return ; - clk_rate =clk_get_rate (priv->stmmac_clk);/* Platform provided default clk_csr would be assumed valid
@@ -192,7 +189,6 @@ static void stmmac_clk_csr_set(struct stmmac_priv *priv)* we can not estimate the proper divider as it is not known * the frequency of clk_csr_i. So we do not change the default * divider. */ -#endif }#if defined(STMMAC_XMIT_DEBUG ) || defined(STMMAC_RX_DEBUG )
@@ -971,7 +967,7 @@ static int stmmac_open(struct net_device *dev) } else priv->tm->enable =1 ;#endif -stmmac_clk_enable (priv); +clk_enable (priv->stmmac_clk);stmmac_check_ether_addr (priv);
@@ -1075,7 +1071,7 @@ open_error:if (priv->phydev)phy_disconnect (priv->phydev); -stmmac_clk_disable (priv); +clk_disable (priv->stmmac_clk);return ret; }
@@ -1128,7 +1124,7 @@ static int stmmac_release(struct net_device *dev)#ifdef CONFIG_STMMAC_DEBUG_FS stmmac_exit_fs ();#endif -stmmac_clk_disable (priv); +clk_disable (priv->stmmac_clk);return 0 ; }
@@ -1922,11 +1918,14 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device, ret =register_netdev (ndev);if (ret) {pr_err ("%s: ERROR %i registering the device\n" , __func__, ret); - goto error; + goto error_netdev_register; } -if (stmmac_clk_get (priv)) + priv->stmmac_clk =clk_get (priv->device,NULL ); +if (IS_ERR (priv->stmmac_clk)) {pr_warning ("%s: warning: cannot get CSR clock\n" , __func__); + goto error_clk_get; + }/* If a specific clk_csr value is passed from the platform * this means that the CSR Clock Range selection cannot be
@@ -1944,15 +1943,17 @@ struct stmmac_priv *stmmac_dvr_probe(struct device *device,if (ret <0 ) {pr_debug ("%s: MDIO bus (id: %d) registration failed" , __func__, priv->plat->bus_id); - goto error; + goto error_mdio_register; }return priv; -error: -netif_napi_del (&priv->napi); - +error_mdio_register: +clk_put (priv->stmmac_clk); +error_clk_get:unregister_netdev (ndev); +error_netdev_register: +netif_napi_del (&priv->napi);free_netdev (ndev);return NULL ;
@@ -2020,7 +2021,7 @@ int stmmac_suspend(struct net_device *ndev)else {stmmac_set_mac (priv->ioaddr, false);/* Disable clock in case of PWM is off */ -stmmac_clk_disable (priv); +clk_disable (priv->stmmac_clk); }spin_unlock (&priv->lock);return 0 ;
@@ -2044,7 +2045,7 @@ int stmmac_resume(struct net_device *ndev) priv->hw->mac->pmt (priv->ioaddr,0 ); else/* enable the clk prevously disabled */ -stmmac_clk_enable (priv ); + clk_enable(priv->stmmac_clk);netif_device_attach (ndev);
Signature · 1 line
1.7.9
PATCH v3 10/12 gadget/m66592: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/usb/gadget/m66592-udc.c | 9 +-------- drivers/usb/gadget/m66592-udc.h | 5 ----- 2 files changed, 1 insertions(+), 13 deletions(-)
drivers/usb/gadget/m66592-udc.c +1 -8
@@ -1583,12 +1583,10 @@ static int __exit m66592_remove(struct platform_device *pdev)iounmap (m66592->reg);free_irq (platform_get_irq (pdev,0 ), m66592);m66592_free_request (&m66592->ep[0 ].ep, m66592->ep0_req); -#ifdef CONFIG_HAVE_CLK if (m66592->pdata->on_chip) {clk_disable (m66592->clk);clk_put (m66592->clk); } -#endif kfree (m66592);return 0 ; }
@@ -1602,9 +1600,7 @@ static int __init m66592_probe(struct platform_device *pdev)struct resource *res, *ires;void __iomem *reg =NULL ;struct m66592 *m66592 =NULL ; -#ifdef CONFIG_HAVE_CLK char clk_name[8 ]; -#endif int ret =0 ;int i;
@@ -1671,7 +1667,6 @@ static int __init m66592_probe(struct platform_device *pdev) goto clean_up; } -#ifdef CONFIG_HAVE_CLK if (m66592->pdata->on_chip) {snprintf (clk_name,sizeof (clk_name),"usbf%d" , pdev->id); m66592->clk =clk_get (&pdev->dev, clk_name);
@@ -1683,7 +1678,7 @@ static int __init m66592_probe(struct platform_device *pdev) }clk_enable (m66592->clk); } -#endif +INIT_LIST_HEAD (&m66592->gadget.ep_list); m66592->gadget.ep0 = &m66592->ep[0 ].ep;INIT_LIST_HEAD (&m66592->gadget.ep0->ep_list);
@@ -1731,13 +1726,11 @@ err_add_udc:m66592_free_request (&m66592->ep[0 ].ep, m66592->ep0_req); clean_up3: -#ifdef CONFIG_HAVE_CLK if (m66592->pdata->on_chip) {clk_disable (m66592->clk);clk_put (m66592->clk); } clean_up2: -#endif free_irq (ires->start, m66592); clean_up:if (m66592) {
drivers/usb/gadget/m66592-udc.h +0 -5
@@ -13,10 +13,7 @@#ifndef __M66592_UDC_H__#define __M66592_UDC_H__ -#ifdef CONFIG_HAVE_CLK #include <linux/clk.h> -#endif -#include <linux/usb/m66592.h> #define M66592_SYSCFG 0x00
@@ -468,9 +465,7 @@ struct m66592_ep {struct m66592 {spinlock_t lock;void __iomem *reg; -#ifdef CONFIG_HAVE_CLK struct clk *clk; -#endif struct m66592_platdata *pdata;unsigned long irq_trigger;
Signature · 1 line
1.7.9
PATCH v3 11/12 gadget/r8a66597: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/usb/gadget/r8a66597-udc.c | 11 +++-------- drivers/usb/gadget/r8a66597-udc.h | 5 ----- 2 files changed, 3 insertions(+), 13 deletions(-)
drivers/usb/gadget/r8a66597-udc.c +3 -8
@@ -1818,12 +1818,12 @@ static int __exit r8a66597_remove(struct platform_device *pdev)iounmap (r8a66597->sudmac_reg);free_irq (platform_get_irq (pdev,0 ), r8a66597);r8a66597_free_request (&r8a66597->ep[0 ].ep, r8a66597->ep0_req); -#ifdef CONFIG_HAVE_CLK +if (r8a66597->pdata->on_chip) {clk_disable (r8a66597->clk);clk_put (r8a66597->clk); } -#endif +device_unregister (&r8a66597->gadget.dev);kfree (r8a66597);return 0 ;
@@ -1855,9 +1855,7 @@ static int __init r8a66597_sudmac_ioremap(struct r8a66597 *r8a66597,static int __init r8a66597_probe (struct platform_device *pdev) { -#ifdef CONFIG_HAVE_CLK char clk_name[8 ]; -#endif struct resource *res, *ires;int irq;void __iomem *reg =NULL ;
@@ -1921,7 +1919,6 @@ static int __init r8a66597_probe(struct platform_device *pdev) r8a66597->timer.data = (unsigned long )r8a66597; r8a66597->reg = reg; -#ifdef CONFIG_HAVE_CLK if (r8a66597->pdata->on_chip) {snprintf (clk_name,sizeof (clk_name),"usb%d" , pdev->id); r8a66597->clk =clk_get (&pdev->dev, clk_name);
@@ -1933,7 +1930,7 @@ static int __init r8a66597_probe(struct platform_device *pdev) }clk_enable (r8a66597->clk); } -#endif +if (r8a66597->pdata->sudmac) { ret =r8a66597_sudmac_ioremap (r8a66597, pdev);if (ret <0 )
@@ -1993,13 +1990,11 @@ err_add_udc: clean_up3:free_irq (irq, r8a66597); clean_up2: -#ifdef CONFIG_HAVE_CLK if (r8a66597->pdata->on_chip) {clk_disable (r8a66597->clk);clk_put (r8a66597->clk); } clean_up_dev: -#endif device_unregister (&r8a66597->gadget.dev); clean_up:if (r8a66597) {
drivers/usb/gadget/r8a66597-udc.h +0 -5
@@ -13,10 +13,7 @@#ifndef __R8A66597_H__#define __R8A66597_H__ -#ifdef CONFIG_HAVE_CLK #include <linux/clk.h> -#endif -#include <linux/usb/r8a66597.h> #define R8A66597_MAX_SAMPLING 10
@@ -92,9 +89,7 @@ struct r8a66597 {void __iomem *reg;void __iomem *sudmac_reg; -#ifdef CONFIG_HAVE_CLK struct clk *clk; -#endif struct r8a66597_platdata *pdata;struct usb_gadget gadget;
Signature · 1 line
1.7.9
PATCH v3 12/12 usb/host/r8a66597: Remove conditional compilation of clk code
In reply to Viresh KumarWith addition of dummy clk_*() calls for non CONFIG_HAVE_CLK cases in clk.h, there is no need to have clk code enclosed in #ifdef CONFIG_HAVE_CLK, #endif macros.
--- drivers/usb/host/r8a66597-hcd.c | 12 ------------ drivers/usb/host/r8a66597.h | 5 ----- 2 files changed, 0 insertions(+), 17 deletions(-)
drivers/usb/host/r8a66597-hcd.c +0 -12
@@ -95,9 +95,7 @@ static int r8a66597_clock_enable(struct r8a66597 *r8a66597)int i =0 ;if (r8a66597->pdata->on_chip) { -#ifdef CONFIG_HAVE_CLK clk_enable (r8a66597->clk); -#endif do { r8a66597_write(r8a66597, SCKE, SYSCFG0); tmp = r8a66597_read(r8a66597, SYSCFG0);
@@ -141,9 +139,7 @@ static void r8a66597_clock_disable(struct r8a66597 *r8a66597)udelay (1 );if (r8a66597->pdata->on_chip) { -#ifdef CONFIG_HAVE_CLK clk_disable (r8a66597->clk); -#endif }else {r8a66597_bclr (r8a66597,PLLC ,SYSCFG0 );r8a66597_bclr (r8a66597,XCKE ,SYSCFG0 );
@@ -2406,19 +2402,15 @@ static int __devexit r8a66597_remove(struct platform_device *pdev)del_timer_sync (&r8a66597->rh_timer);usb_remove_hcd (hcd);iounmap (r8a66597->reg); -#ifdef CONFIG_HAVE_CLK if (r8a66597->pdata->on_chip)clk_put (r8a66597->clk); -#endif usb_put_hcd (hcd);return 0 ; }static int __devinit r8a66597_probe (struct platform_device *pdev) { -#ifdef CONFIG_HAVE_CLK char clk_name[8 ]; -#endif struct resource *res =NULL , *ires;int irq =-1 ;void __iomem *reg =NULL ;
@@ -2482,7 +2474,6 @@ static int __devinit r8a66597_probe(struct platform_device *pdev) r8a66597->irq_sense_low = irq_trigger ==IRQF_TRIGGER_LOW ;if (r8a66597->pdata->on_chip) { -#ifdef CONFIG_HAVE_CLK snprintf (clk_name,sizeof (clk_name),"usb%d" , pdev->id); r8a66597->clk =clk_get (&pdev->dev, clk_name);if (IS_ERR (r8a66597->clk)) {
@@ -2491,7 +2482,6 @@ static int __devinit r8a66597_probe(struct platform_device *pdev) ret =PTR_ERR (r8a66597->clk); goto clean_up2; } -#endif r8a66597->max_root_hub =1 ; }else r8a66597->max_root_hub =2 ;
@@ -2531,11 +2521,9 @@ static int __devinit r8a66597_probe(struct platform_device *pdev)return 0 ; clean_up3: -#ifdef CONFIG_HAVE_CLK if (r8a66597->pdata->on_chip)clk_put (r8a66597->clk); clean_up2: -#endif usb_put_hcd (hcd); clean_up:
drivers/usb/host/r8a66597.h +0 -5
@@ -26,10 +26,7 @@#ifndef __R8A66597_H__#define __R8A66597_H__ -#ifdef CONFIG_HAVE_CLK #include <linux/clk.h> -#endif -#include <linux/usb/r8a66597.h> #define R8A66597_MAX_NUM_PIPE 10
@@ -113,9 +110,7 @@ struct r8a66597_root_hub {struct r8a66597 {spinlock_t lock;void __iomem *reg; -#ifdef CONFIG_HAVE_CLK struct clk *clk; -#endif struct r8a66597_platdata *pdata;struct r8a66597_device device0;struct r8a66597_root_hub root_hub[R8A66597_MAX_ROOT_HUB ];
Signature · 1 line
1.7.9