* [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference
@ 2016-08-10 8:09 ` Gregory CLEMENT
0 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2016-08-10 8:09 UTC (permalink / raw)
To: linux-arm-kernel
While converting the init function to return an error, the wrong clock
was get. This lead to wrong clock rate and slow down the kernel. For
example, before the patch a typical boot was around 15s after it was 1
minute slower.
Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/clocksource/time-armada-370-xp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 719b478d136e..3c39e6f45971 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
struct clk *clk = of_clk_get_by_name(np, "fixed");
int ret;
- clk = of_clk_get(np, 0);
if (IS_ERR(clk)) {
pr_err("Failed to get clock");
return PTR_ERR(clk);
--
2.8.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference
@ 2016-08-10 8:09 ` Gregory CLEMENT
0 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2016-08-10 8:09 UTC (permalink / raw)
To: Daniel Lezcano, Thomas Gleixner, linux-kernel
Cc: Jason Cooper, Andrew Lunn, Sebastian Hesselbarth, Gregory CLEMENT,
Thomas Petazzoni, linux-arm-kernel, Ezequiel Garcia,
Maxime Ripard, Boris BREZILLON, Romain Perier, Stefan Roese,
Sebastian Careba
While converting the init function to return an error, the wrong clock
was get. This lead to wrong clock rate and slow down the kernel. For
example, before the patch a typical boot was around 15s after it was 1
minute slower.
Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
---
drivers/clocksource/time-armada-370-xp.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
index 719b478d136e..3c39e6f45971 100644
--- a/drivers/clocksource/time-armada-370-xp.c
+++ b/drivers/clocksource/time-armada-370-xp.c
@@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
struct clk *clk = of_clk_get_by_name(np, "fixed");
int ret;
- clk = of_clk_get(np, 0);
if (IS_ERR(clk)) {
pr_err("Failed to get clock");
return PTR_ERR(clk);
--
2.8.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference
2016-08-10 8:09 ` Gregory CLEMENT
@ 2016-08-10 12:06 ` Thomas Petazzoni
-1 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2016-08-10 12:06 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
On Wed, 10 Aug 2016 10:09:08 +0200, Gregory CLEMENT wrote:
> While converting the init function to return an error, the wrong clock
> was get. This lead to wrong clock rate and slow down the kernel. For
> example, before the patch a typical boot was around 15s after it was 1
> minute slower.
>
> Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/clocksource/time-armada-370-xp.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index 719b478d136e..3c39e6f45971 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
> struct clk *clk = of_clk_get_by_name(np, "fixed");
> int ret;
>
> - clk = of_clk_get(np, 0);
I think to avoid this mistake, we should rewrite the code as:
struct *clk;
int ret;
clk = of_clk_get_by_name(np, "fixed");
if (IS_ERR(clk)) {
...
Indeed, I find confusing a block that starts with error checking, and
it's probably what lead to this of_clk_get() being added here.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference
@ 2016-08-10 12:06 ` Thomas Petazzoni
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Petazzoni @ 2016-08-10 12:06 UTC (permalink / raw)
To: Gregory CLEMENT
Cc: Daniel Lezcano, Thomas Gleixner, linux-kernel, Andrew Lunn,
Romain Perier, Jason Cooper, Boris BREZILLON, Stefan Roese,
Ezequiel Garcia, Sebastian Careba, Maxime Ripard,
linux-arm-kernel, Sebastian Hesselbarth
Hello,
On Wed, 10 Aug 2016 10:09:08 +0200, Gregory CLEMENT wrote:
> While converting the init function to return an error, the wrong clock
> was get. This lead to wrong clock rate and slow down the kernel. For
> example, before the patch a typical boot was around 15s after it was 1
> minute slower.
>
> Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/clocksource/time-armada-370-xp.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
> index 719b478d136e..3c39e6f45971 100644
> --- a/drivers/clocksource/time-armada-370-xp.c
> +++ b/drivers/clocksource/time-armada-370-xp.c
> @@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
> struct clk *clk = of_clk_get_by_name(np, "fixed");
> int ret;
>
> - clk = of_clk_get(np, 0);
I think to avoid this mistake, we should rewrite the code as:
struct *clk;
int ret;
clk = of_clk_get_by_name(np, "fixed");
if (IS_ERR(clk)) {
...
Indeed, I find confusing a block that starts with error checking, and
it's probably what lead to this of_clk_get() being added here.
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference
2016-08-10 12:06 ` Thomas Petazzoni
@ 2016-08-10 13:05 ` Gregory CLEMENT
-1 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2016-08-10 13:05 UTC (permalink / raw)
To: linux-arm-kernel
Hi Thomas,
On mer., ao?t 10 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 10 Aug 2016 10:09:08 +0200, Gregory CLEMENT wrote:
>> While converting the init function to return an error, the wrong clock
>> was get. This lead to wrong clock rate and slow down the kernel. For
>> example, before the patch a typical boot was around 15s after it was 1
>> minute slower.
>>
>> Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> drivers/clocksource/time-armada-370-xp.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
>> index 719b478d136e..3c39e6f45971 100644
>> --- a/drivers/clocksource/time-armada-370-xp.c
>> +++ b/drivers/clocksource/time-armada-370-xp.c
>> @@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
>> struct clk *clk = of_clk_get_by_name(np, "fixed");
>> int ret;
>>
>> - clk = of_clk_get(np, 0);
>
> I think to avoid this mistake, we should rewrite the code as:
>
> struct *clk;
> int ret;
>
> clk = of_clk_get_by_name(np, "fixed");
> if (IS_ERR(clk)) {
> ...
>
>
OK I can do it. it will be alos more coherent with the other _init
function.
Gregory
> Indeed, I find confusing a block that starts with error checking, and
> it's probably what lead to this of_clk_get() being added here.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference
@ 2016-08-10 13:05 ` Gregory CLEMENT
0 siblings, 0 replies; 6+ messages in thread
From: Gregory CLEMENT @ 2016-08-10 13:05 UTC (permalink / raw)
To: Thomas Petazzoni
Cc: Daniel Lezcano, Thomas Gleixner, linux-kernel, Andrew Lunn,
Romain Perier, Jason Cooper, Boris BREZILLON, Stefan Roese,
Ezequiel Garcia, Sebastian Careba, Maxime Ripard,
linux-arm-kernel, Sebastian Hesselbarth
Hi Thomas,
On mer., août 10 2016, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Wed, 10 Aug 2016 10:09:08 +0200, Gregory CLEMENT wrote:
>> While converting the init function to return an error, the wrong clock
>> was get. This lead to wrong clock rate and slow down the kernel. For
>> example, before the patch a typical boot was around 15s after it was 1
>> minute slower.
>>
>> Fixes: 12549e27c63c ("clocksource/drivers/time-armada-370-xp: Convert init function to return error")
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>> ---
>> drivers/clocksource/time-armada-370-xp.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/drivers/clocksource/time-armada-370-xp.c b/drivers/clocksource/time-armada-370-xp.c
>> index 719b478d136e..3c39e6f45971 100644
>> --- a/drivers/clocksource/time-armada-370-xp.c
>> +++ b/drivers/clocksource/time-armada-370-xp.c
>> @@ -338,7 +338,6 @@ static int __init armada_xp_timer_init(struct device_node *np)
>> struct clk *clk = of_clk_get_by_name(np, "fixed");
>> int ret;
>>
>> - clk = of_clk_get(np, 0);
>
> I think to avoid this mistake, we should rewrite the code as:
>
> struct *clk;
> int ret;
>
> clk = of_clk_get_by_name(np, "fixed");
> if (IS_ERR(clk)) {
> ...
>
>
OK I can do it. it will be alos more coherent with the other _init
function.
Gregory
> Indeed, I find confusing a block that starts with error checking, and
> it's probably what lead to this of_clk_get() being added here.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
--
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-08-10 20:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-10 8:09 [PATCH] clocksource/drivers/time-armada-370-xp: Fix the clock reference Gregory CLEMENT
2016-08-10 8:09 ` Gregory CLEMENT
2016-08-10 12:06 ` Thomas Petazzoni
2016-08-10 12:06 ` Thomas Petazzoni
2016-08-10 13:05 ` Gregory CLEMENT
2016-08-10 13:05 ` Gregory CLEMENT
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.