arm: plat-omap: dmtimer: Add clock source from DT
Subsystems (3)
RFC PATCH 1/3 arm: plat-omap: dmtimer: Add clock source from DT
Add a function which sets the timer source from the clocks binding on dm_timer_prepare call.
In case the clocks property is not valid, it falls back to the set_source() with 32_KHZ clock as default.
--- arch/arm/plat-omap/dmtimer.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)
arch/arm/plat-omap/dmtimer.c +30 -2
@@ -137,9 +137,33 @@ static int omap_dm_timer_reset(struct omap_dm_timer *timer)return 0 ; } +static int omap_dm_timer_of_set_source (struct omap_dm_timer *timer) +{ +int ret; +struct clk *parent; + +if (unlikely (!timer)) +return -EINVAL ; + +if (IS_ERR (timer->fclk)) +return -EINVAL ; + + parent =clk_get (&timer->pdev->dev,NULL ); +if (IS_ERR (parent)) +return -ENODEV ; + + ret =clk_set_parent (timer->fclk, parent); +if (ret <0 ) +pr_err ("%s: failed to set parent\n" , __func__); + +clk_put (parent); + +return ret; +} +static int omap_dm_timer_prepare (struct omap_dm_timer *timer) { -int rc; +int rc, ret;/* * FIXME: OMAP1 devices do not use the clock framework for dmtimers so
@@ -166,7 +190,11 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)__omap_dm_timer_enable_posted (timer);omap_dm_timer_disable (timer); -return omap_dm_timer_set_source (timer,OMAP_TIMER_SRC_32_KHZ ); + ret =omap_dm_timer_of_set_source (timer); +if (ret <0 && ret == -ENODEV ) +return omap_dm_timer_set_source (timer,OMAP_TIMER_SRC_32_KHZ ); + +return ret; }static inline u32 omap_dm_timer_reserved_systimer (int id)
Signature · 1 line
1.9.1
Hi,
Quoted 17 lines · Neil Armstrong
Add a function which sets the timer source from the clocks binding on
dm_timer_preparecall.In case the clocks property is not valid, it falls back to the
set_source()with 32_KHZ clock as default.Suggested-by Tony Lindgren <[email protected]>Signed-off-by Neil Armstrong <[email protected]>--- arch/arm/plat-omap/dmtimer.c | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-)arch/arm/plat-omap/dmtimer.c +0
-0
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 146
Quoted 10 lines
arch/arm/plat-omap/dmtimer.c +7
-0@@ -137,9 +137,33 @@ static int omap_dm_timer_reset(struct omap_dm_timer *timer)return 0 ; } +static int omap_dm_timer_of_set_source (struct omap_dm_timer *timer) +{ +int ret; +struct clk *parent; + +if (unlikely (!timer)) +return -EINVAL ;
IMO, let this crash. If this happens we have bigger problems.
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 149
Quoted 2 lines
arch/arm/plat-omap/dmtimer.c +2
-0+if (IS_ERR (timer->fclk)) +return -EINVAL ;
We bail out if we can't get fclk, this check is unnecessary.
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 151
Quoted 1 line
arch/arm/plat-omap/dmtimer.c +1
-0+ parent =clk_get (&timer->pdev->dev,NULL );
why NULL ? You could use something more descriptive, but no strong feelings.
Quoted 6 lines
arch/arm/plat-omap/dmtimer.c +6
-0+if (IS_ERR (parent)) +return -ENODEV ; + + ret =clk_set_parent (timer->fclk, parent); +if (ret <0 ) +pr_err ("%s: failed to set parent\n" , __func__);
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 166
Quoted 10 lines
arch/arm/plat-omap/dmtimer.c +7
-1+ +clk_put (parent); + +return ret; +} +static int omap_dm_timer_prepare (struct omap_dm_timer *timer) { -int rc; +int rc, ret;
doesn't seem like you need this extra 'ret' variable. Just use 'rc'.
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_prepare(struct omap_dm_timer *timer) · line 194
Quoted 7 lines
arch/arm/plat-omap/dmtimer.c +2
-1@@ -166,7 +190,11 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)__omap_dm_timer_enable_posted (timer);omap_dm_timer_disable (timer); -return omap_dm_timer_set_source (timer,OMAP_TIMER_SRC_32_KHZ ); + ret =omap_dm_timer_of_set_source (timer); +if (ret <0 && ret == -ENODEV )
this < 0 check is pointless if you're going to check for equality to -ENODEV
Quoted 14 lines
arch/arm/plat-omap/dmtimer.c +3
-0+return omap_dm_timer_set_source (timer,OMAP_TIMER_SRC_32_KHZ ); + +return ret; }static inline u32 omap_dm_timer_reserved_systimer (int id)Signature · 7 lines
1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo at vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Signature · 8 lines
balbi -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 818 bytes Desc: not available URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151016/019159f9/attachment.sig>
Hi,
Quoted 10 lines · Felipe Balbi
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 146
Quoted 7 lines
arch/arm/plat-omap/dmtimer.c +7
-0+static int omap_dm_timer_of_set_source (struct omap_dm_timer *timer) +{ +int ret; +struct clk *parent; + +if (unlikely (!timer)) +return -EINVAL ;IMO, let this crash. If this happens we have bigger problems.
Quoted 4 lines
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 149
Quoted 2 lines
arch/arm/plat-omap/dmtimer.c +2
-0+if (IS_ERR (timer->fclk)) +return -EINVAL ;We bail out if we can't get fclk, this check is unnecessary.
Sure, I will remove the checks.
This was also breaking OMAP1, fclk must be checked against NULL here to detect OMAP1 and return -ENODEV;
Quoted 5 lines
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 151
Quoted 2 lines
arch/arm/plat-omap/dmtimer.c +1
-0+ parent =clk_get (&timer->pdev->dev,NULL );why NULL ? You could use something more descriptive, but no strong feelings.
Actually, I just wanted the "default" clock for this device, NULL will select the first OF clock found. If we specify a name, we should also specify it in the DT as clock-names, is it desired ? If you feel I should name it somehow... I don't want it to conflict all the HWMOD clk namings actually.
Quoted 24 lines
Quoted 7 lines
arch/arm/plat-omap/dmtimer.c +6
-0+if (IS_ERR (parent)) +return -ENODEV ; + + ret =clk_set_parent (timer->fclk, parent); +if (ret <0 ) +pr_err ("%s: failed to set parent\n" , __func__);arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_reset(struct omap_dm_timer *timer) · line 166
Quoted 10 lines
arch/arm/plat-omap/dmtimer.c +7
-1+ +clk_put (parent); + +return ret; +} +static int omap_dm_timer_prepare (struct omap_dm_timer *timer) { -int rc; +int rc, ret;doesn't seem like you need this extra 'ret' variable. Just use 'rc'.
Quoted 4 lines
arch/arm/plat-omap/dmtimer.c +0
-0@@ -166,7 +190,11 @@ static int omap_dm_timer_prepare(struct omap_dm_timer *timer)__omap_dm_timer_enable_posted (timer);omap_dm_timer_disable (timer);
Quoted 5 lines
arch/arm/plat-omap/dmtimer.c static int omap_dm_timer_prepare(struct omap_dm_timer *timer) · line 194
Quoted 3 lines
arch/arm/plat-omap/dmtimer.c +2
-1-return omap_dm_timer_set_source (timer,OMAP_TIMER_SRC_32_KHZ ); + ret =omap_dm_timer_of_set_source (timer); +if (ret <0 && ret == -ENODEV )this < 0 check is pointless if you're going to check for equality to
-ENODEV
Sure, I was certainly tired, this was a so stupid mistake...
Neil