All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: mvebu: armada-38x: add support for missing clocks
@ 2018-03-08  9:03 Richard Genoud
  2018-03-08 13:22   ` Gregory CLEMENT
  0 siblings, 1 reply; 8+ messages in thread
From: Richard Genoud @ 2018-03-08  9:03 UTC (permalink / raw)
  To: Stephen Boyd, Michael Turquette
  Cc: Gregory CLEMENT, Ralph Sennhauser, linux-clk, linux-kernel,
	Richard Genoud

Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
or 1333MHz (industrial).

They have also some dip-switches to select a different clock (666, 800,
1066, 1200).

The funny thing is that the recovery button is on the MPP34 fq selector.
So, when booting an industrial board with this button down, the frequency
666MHz is selected (and the kernel didn't boot).

This patch add all the missing clocks.

The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).

Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
---
 drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
index 394aa6f03f01..9ff4ea63932d 100644
--- a/drivers/clk/mvebu/armada-38x.c
+++ b/drivers/clk/mvebu/armada-38x.c
@@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
 }
 
 static const u32 armada_38x_cpu_frequencies[] __initconst = {
-	0, 0, 0, 0,
-	1066 * 1000 * 1000, 0, 0, 0,
+	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
+	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
 	1332 * 1000 * 1000, 0, 0, 0,
 	1600 * 1000 * 1000, 0, 0, 0,
-	1866 * 1000 * 1000,
+	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
 };
 
 static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
@@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
 };
 
 static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
-	{0, 1}, {0, 1}, {0, 1}, {0, 1},
-	{1, 2}, {0, 1}, {0, 1}, {0, 1},
-	{1, 2}, {0, 1}, {0, 1}, {0, 1},
+	{1, 2}, {0, 1}, {1, 2}, {0, 1},
+	{1, 2}, {0, 1}, {1, 2}, {0, 1},
 	{1, 2}, {0, 1}, {0, 1}, {0, 1},
 	{1, 2}, {0, 1}, {0, 1}, {0, 1},
+	{1, 2}, {0, 1}, {0, 1}, {1, 2},
 	{0, 1}, {0, 1}, {0, 1}, {0, 1},
 	{0, 1}, {0, 1}, {0, 1}, {0, 1},
 	{0, 1}, {0, 1}, {0, 1}, {0, 1},
@@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
 	{1, 2}, {0, 1}, {0, 1}, {0, 1},
 	{1, 2}, {0, 1}, {0, 1}, {0, 1},
 	{1, 2}, {0, 1}, {0, 1}, {0, 1},
-	{1, 2}, {0, 1}, {0, 1}, {0, 1},
+	{1, 2}, {0, 1}, {0, 1}, {7, 15},
 	{0, 1}, {0, 1}, {0, 1}, {0, 1},
 	{0, 1}, {0, 1}, {0, 1}, {0, 1},
 	{0, 1}, {0, 1}, {0, 1}, {0, 1},

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
  2018-03-08  9:03 [PATCH] clk: mvebu: armada-38x: add support for missing clocks Richard Genoud
@ 2018-03-08 13:22   ` Gregory CLEMENT
  0 siblings, 0 replies; 8+ messages in thread
From: Gregory CLEMENT @ 2018-03-08 13:22 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

Hi Richard,
 
 On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:

> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
> or 1333MHz (industrial).
>
> They have also some dip-switches to select a different clock (666, 800,
> 1066, 1200).

The patch looks goo and it will also be usefull for any other board
using these frequencies, thanks for this. I have only one small comment,
see below.


>
> The funny thing is that the recovery button is on the MPP34 fq selector.
> So, when booting an industrial board with this button down, the frequency
> 666MHz is selected (and the kernel didn't boot).
>
> This patch add all the missing clocks.
>
> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
> index 394aa6f03f01..9ff4ea63932d 100644
> --- a/drivers/clk/mvebu/armada-38x.c
> +++ b/drivers/clk/mvebu/armada-38x.c
> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>  }
>  
>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
> -	0, 0, 0, 0,
> -	1066 * 1000 * 1000, 0, 0, 0,
> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>  	1332 * 1000 * 1000, 0, 0, 0,
>  	1600 * 1000 * 1000, 0, 0, 0,
> -	1866 * 1000 * 1000,
> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,

Maybe you could add a comment here to say that the 2GHz mode didn't have
been tested.

Thanks,
Gregory


>  };
>  
>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>  };
>  
>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
@ 2018-03-08 13:22   ` Gregory CLEMENT
  0 siblings, 0 replies; 8+ messages in thread
From: Gregory CLEMENT @ 2018-03-08 13:22 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

Hi Richard,
 
 On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:

> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
> or 1333MHz (industrial).
>
> They have also some dip-switches to select a different clock (666, 800,
> 1066, 1200).

The patch looks goo and it will also be usefull for any other board
using these frequencies, thanks for this. I have only one small comment,
see below.


>
> The funny thing is that the recovery button is on the MPP34 fq selector.
> So, when booting an industrial board with this button down, the frequency
> 666MHz is selected (and the kernel didn't boot).
>
> This patch add all the missing clocks.
>
> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>
> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
> ---
>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
> index 394aa6f03f01..9ff4ea63932d 100644
> --- a/drivers/clk/mvebu/armada-38x.c
> +++ b/drivers/clk/mvebu/armada-38x.c
> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>  }
>  
>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
> -	0, 0, 0, 0,
> -	1066 * 1000 * 1000, 0, 0, 0,
> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>  	1332 * 1000 * 1000, 0, 0, 0,
>  	1600 * 1000 * 1000, 0, 0, 0,
> -	1866 * 1000 * 1000,
> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,

Maybe you could add a comment here to say that the 2GHz mode didn't have
been tested.

Thanks,
Gregory


>  };
>  
>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>  };
>  
>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
  2018-03-08 13:22   ` Gregory CLEMENT
@ 2018-03-08 13:23     ` Gregory CLEMENT
  -1 siblings, 0 replies; 8+ messages in thread
From: Gregory CLEMENT @ 2018-03-08 13:23 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

Hi,
 
 On jeu., mars 08 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Hi Richard,
>  
>  On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
>> or 1333MHz (industrial).
>>
>> They have also some dip-switches to select a different clock (666, 800,
>> 1066, 1200).
>
> The patch looks goo and it will also be usefull for any other board
> using these frequencies, thanks for this. I have only one small comment,
> see below.


I forgot to mention that you can add my

Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Thanks,

Gregory

>
>
>>
>> The funny thing is that the recovery button is on the MPP34 fq selector.
>> So, when booting an industrial board with this button down, the frequency
>> 666MHz is selected (and the kernel didn't boot).
>>
>> This patch add all the missing clocks.
>>
>> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
>> index 394aa6f03f01..9ff4ea63932d 100644
>> --- a/drivers/clk/mvebu/armada-38x.c
>> +++ b/drivers/clk/mvebu/armada-38x.c
>> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>>  }
>>  
>>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
>> -	0, 0, 0, 0,
>> -	1066 * 1000 * 1000, 0, 0, 0,
>> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
>> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>>  	1332 * 1000 * 1000, 0, 0, 0,
>>  	1600 * 1000 * 1000, 0, 0, 0,
>> -	1866 * 1000 * 1000,
>> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
>
> Maybe you could add a comment here to say that the 2GHz mode didn't have
> been tested.
>
> Thanks,
> Gregory
>
>
>>  };
>>  
>>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
>> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>>  };
>>  
>>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
>> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>
> -- 
> Gregory Clement, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> http://bootlin.com

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
@ 2018-03-08 13:23     ` Gregory CLEMENT
  0 siblings, 0 replies; 8+ messages in thread
From: Gregory CLEMENT @ 2018-03-08 13:23 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

Hi,
 
 On jeu., mars 08 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote:

> Hi Richard,
>  
>  On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:
>
>> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
>> or 1333MHz (industrial).
>>
>> They have also some dip-switches to select a different clock (666, 800,
>> 1066, 1200).
>
> The patch looks goo and it will also be usefull for any other board
> using these frequencies, thanks for this. I have only one small comment,
> see below.


I forgot to mention that you can add my

Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>

Thanks,

Gregory

>
>
>>
>> The funny thing is that the recovery button is on the MPP34 fq selector.
>> So, when booting an industrial board with this button down, the frequency
>> 666MHz is selected (and the kernel didn't boot).
>>
>> This patch add all the missing clocks.
>>
>> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>>
>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>> ---
>>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
>> index 394aa6f03f01..9ff4ea63932d 100644
>> --- a/drivers/clk/mvebu/armada-38x.c
>> +++ b/drivers/clk/mvebu/armada-38x.c
>> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>>  }
>>  
>>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
>> -	0, 0, 0, 0,
>> -	1066 * 1000 * 1000, 0, 0, 0,
>> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
>> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>>  	1332 * 1000 * 1000, 0, 0, 0,
>>  	1600 * 1000 * 1000, 0, 0, 0,
>> -	1866 * 1000 * 1000,
>> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
>
> Maybe you could add a comment here to say that the 2GHz mode didn't have
> been tested.
>
> Thanks,
> Gregory
>
>
>>  };
>>  
>>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
>> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>>  };
>>  
>>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
>> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>
> -- 
> Gregory Clement, Bootlin (formerly Free Electrons)
> Embedded Linux and Kernel engineering
> http://bootlin.com

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
  2018-03-08 13:23     ` Gregory CLEMENT
  (?)
@ 2018-03-13 10:20     ` Richard Genoud
  2018-03-13 10:32         ` Gregory CLEMENT
  -1 siblings, 1 reply; 8+ messages in thread
From: Richard Genoud @ 2018-03-13 10:20 UTC (permalink / raw)
  To: Gregory CLEMENT
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

On 08/03/2018 14:23, Gregory CLEMENT wrote:
> Hi,
>  
>  On jeu., mars 08 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote:
> 
>> Hi Richard,
>>  
>>  On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:
>>
>>> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
>>> or 1333MHz (industrial).
>>>
>>> They have also some dip-switches to select a different clock (666, 800,
>>> 1066, 1200).
>>
>> The patch looks goo and it will also be usefull for any other board
>> using these frequencies, thanks for this. I have only one small comment,
>> see below.
> 
> 
> I forgot to mention that you can add my
> 
> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>
> 
> Thanks,
> 
> Gregory
> 
>>
>>
>>>
>>> The funny thing is that the recovery button is on the MPP34 fq selector.
>>> So, when booting an industrial board with this button down, the frequency
>>> 666MHz is selected (and the kernel didn't boot).
>>>
>>> This patch add all the missing clocks.
>>>
>>> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>>>
>>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>>> ---
>>>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
>>> index 394aa6f03f01..9ff4ea63932d 100644
>>> --- a/drivers/clk/mvebu/armada-38x.c
>>> +++ b/drivers/clk/mvebu/armada-38x.c
>>> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>>>  }
>>>  
>>>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
>>> -	0, 0, 0, 0,
>>> -	1066 * 1000 * 1000, 0, 0, 0,
>>> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
>>> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>>>  	1332 * 1000 * 1000, 0, 0, 0,
>>>  	1600 * 1000 * 1000, 0, 0, 0,
>>> -	1866 * 1000 * 1000,
>>> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
>>
>> Maybe you could add a comment here to say that the 2GHz mode didn't have
>> been tested.
Well, if I add a comment there saying "2GHz mode hasn't been tested",
I'm afraid it will stay there forever, even after being tested.
(if it's working, nobody will look at it, and if it's not, it's in the commit message anyway).

I was also thinking that this could go in -stable since it fixes a hang.
In this case, commit 9593f4f56cf5 ("clk: mvebu: armada-38x: add support for 1866MHz variants")
should also go with it.



Thanks !

>>
>> Thanks,
>> Gregory
>>
>>
>>>  };
>>>  
>>>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
>>> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>>>  };
>>>  
>>>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
>>> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>
>> -- 
>> Gregory Clement, Bootlin (formerly Free Electrons)
>> Embedded Linux and Kernel engineering
>> http://bootlin.com
> 

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
  2018-03-13 10:20     ` Richard Genoud
@ 2018-03-13 10:32         ` Gregory CLEMENT
  0 siblings, 0 replies; 8+ messages in thread
From: Gregory CLEMENT @ 2018-03-13 10:32 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

Hi Richard,
 
 On mar., mars 13 2018, Richard Genoud <richard.genoud@gmail.com> wrote:

> On 08/03/2018 14:23, Gregory CLEMENT wrote:
>> Hi,
>>  
>>  On jeu., mars 08 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote:
>> 
>>> Hi Richard,
>>>  
>>>  On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:
>>>
>>>> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
>>>> or 1333MHz (industrial).
>>>>
>>>> They have also some dip-switches to select a different clock (666, 800,
>>>> 1066, 1200).
>>>
>>> The patch looks goo and it will also be usefull for any other board
>>> using these frequencies, thanks for this. I have only one small comment,
>>> see below.
>> 
>> 
>> I forgot to mention that you can add my
>> 
>> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> 
>> Thanks,
>> 
>> Gregory
>> 
>>>
>>>
>>>>
>>>> The funny thing is that the recovery button is on the MPP34 fq selector.
>>>> So, when booting an industrial board with this button down, the frequency
>>>> 666MHz is selected (and the kernel didn't boot).
>>>>
>>>> This patch add all the missing clocks.
>>>>
>>>> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>>>>
>>>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>>>> ---
>>>>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
>>>> index 394aa6f03f01..9ff4ea63932d 100644
>>>> --- a/drivers/clk/mvebu/armada-38x.c
>>>> +++ b/drivers/clk/mvebu/armada-38x.c
>>>> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>>>>  }
>>>>  
>>>>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
>>>> -	0, 0, 0, 0,
>>>> -	1066 * 1000 * 1000, 0, 0, 0,
>>>> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
>>>> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>>>>  	1332 * 1000 * 1000, 0, 0, 0,
>>>>  	1600 * 1000 * 1000, 0, 0, 0,
>>>> -	1866 * 1000 * 1000,
>>>> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
>>>
>>> Maybe you could add a comment here to say that the 2GHz mode didn't have
>>> been tested.
> Well, if I add a comment there saying "2GHz mode hasn't been tested",
> I'm afraid it will stay there forever, even after being tested.
> (if it's working, nobody will look at it, and if it's not, it's in the
> commit message anyway).

OK fair enough.

>
> I was also thinking that this could go in -stable since it fixes a hang.
> In this case, commit 9593f4f56cf5 ("clk: mvebu: armada-38x: add support for 1866MHz variants")
> should also go with it.

In this case you have to send a new version of your patch with the cc:
stabale and Fixes: tags.

Gregory

>
>
>
> Thanks !
>
>>>
>>> Thanks,
>>> Gregory
>>>
>>>
>>>>  };
>>>>  
>>>>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
>>>> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>>>>  };
>>>>  
>>>>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
>>>> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>
>>> -- 
>>> Gregory Clement, Bootlin (formerly Free Electrons)
>>> Embedded Linux and Kernel engineering
>>> http://bootlin.com
>> 
>

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

* Re: [PATCH] clk: mvebu: armada-38x: add support for missing clocks
@ 2018-03-13 10:32         ` Gregory CLEMENT
  0 siblings, 0 replies; 8+ messages in thread
From: Gregory CLEMENT @ 2018-03-13 10:32 UTC (permalink / raw)
  To: Richard Genoud
  Cc: Stephen Boyd, Michael Turquette, Gregory CLEMENT,
	Ralph Sennhauser, linux-clk, linux-kernel

Hi Richard,
 
 On mar., mars 13 2018, Richard Genoud <richard.genoud@gmail.com> wrote:

> On 08/03/2018 14:23, Gregory CLEMENT wrote:
>> Hi,
>>  
>>  On jeu., mars 08 2018, Gregory CLEMENT <gregory.clement@bootlin.com> wrote:
>> 
>>> Hi Richard,
>>>  
>>>  On jeu., mars 08 2018, Richard Genoud <richard.genoud@gmail.com> wrote:
>>>
>>>> Clearfog boards can come with a CPU clocked at 1600MHz (commercial)
>>>> or 1333MHz (industrial).
>>>>
>>>> They have also some dip-switches to select a different clock (666, 800,
>>>> 1066, 1200).
>>>
>>> The patch looks goo and it will also be usefull for any other board
>>> using these frequencies, thanks for this. I have only one small comment,
>>> see below.
>> 
>> 
>> I forgot to mention that you can add my
>> 
>> Acked-by: Gregory CLEMENT <gregory.clement@bootlin.com>
>> 
>> Thanks,
>> 
>> Gregory
>> 
>>>
>>>
>>>>
>>>> The funny thing is that the recovery button is on the MPP34 fq selector.
>>>> So, when booting an industrial board with this button down, the frequency
>>>> 666MHz is selected (and the kernel didn't boot).
>>>>
>>>> This patch add all the missing clocks.
>>>>
>>>> The only mode I didn't test is 2GHz (uboot found 4294MHz instead :/ ).
>>>>
>>>> Signed-off-by: Richard Genoud <richard.genoud@gmail.com>
>>>> ---
>>>>  drivers/clk/mvebu/armada-38x.c | 14 +++++++-------
>>>>  1 file changed, 7 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/clk/mvebu/armada-38x.c b/drivers/clk/mvebu/armada-38x.c
>>>> index 394aa6f03f01..9ff4ea63932d 100644
>>>> --- a/drivers/clk/mvebu/armada-38x.c
>>>> +++ b/drivers/clk/mvebu/armada-38x.c
>>>> @@ -46,11 +46,11 @@ static u32 __init armada_38x_get_tclk_freq(void __iomem *sar)
>>>>  }
>>>>  
>>>>  static const u32 armada_38x_cpu_frequencies[] __initconst = {
>>>> -	0, 0, 0, 0,
>>>> -	1066 * 1000 * 1000, 0, 0, 0,
>>>> +	666 * 1000 * 1000,  0, 800 * 1000 * 1000, 0,
>>>> +	1066 * 1000 * 1000, 0, 1200 * 1000 * 1000, 0,
>>>>  	1332 * 1000 * 1000, 0, 0, 0,
>>>>  	1600 * 1000 * 1000, 0, 0, 0,
>>>> -	1866 * 1000 * 1000,
>>>> +	1866 * 1000 * 1000, 0, 0, 2000 * 1000 * 1000,
>>>
>>> Maybe you could add a comment here to say that the 2GHz mode didn't have
>>> been tested.
> Well, if I add a comment there saying "2GHz mode hasn't been tested",
> I'm afraid it will stay there forever, even after being tested.
> (if it's working, nobody will look at it, and if it's not, it's in the
> commit message anyway).

OK fair enough.

>
> I was also thinking that this could go in -stable since it fixes a hang.
> In this case, commit 9593f4f56cf5 ("clk: mvebu: armada-38x: add support for 1866MHz variants")
> should also go with it.

In this case you have to send a new version of your patch with the cc:
stabale and Fixes: tags.

Gregory

>
>
>
> Thanks !
>
>>>
>>> Thanks,
>>> Gregory
>>>
>>>
>>>>  };
>>>>  
>>>>  static u32 __init armada_38x_get_cpu_freq(void __iomem *sar)
>>>> @@ -76,11 +76,11 @@ static const struct coreclk_ratio armada_38x_coreclk_ratios[] __initconst = {
>>>>  };
>>>>  
>>>>  static const int armada_38x_cpu_l2_ratios[32][2] __initconst = {
>>>> -	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {1, 2}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {0, 1}, {1, 2},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>> @@ -91,7 +91,7 @@ static const int armada_38x_cpu_ddr_ratios[32][2] __initconst = {
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> -	{1, 2}, {0, 1}, {0, 1}, {0, 1},
>>>> +	{1, 2}, {0, 1}, {0, 1}, {7, 15},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>>  	{0, 1}, {0, 1}, {0, 1}, {0, 1},
>>>
>>> -- 
>>> Gregory Clement, Bootlin (formerly Free Electrons)
>>> Embedded Linux and Kernel engineering
>>> http://bootlin.com
>> 
>

-- 
Gregory Clement, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

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

end of thread, other threads:[~2018-03-13 10:32 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-08  9:03 [PATCH] clk: mvebu: armada-38x: add support for missing clocks Richard Genoud
2018-03-08 13:22 ` Gregory CLEMENT
2018-03-08 13:22   ` Gregory CLEMENT
2018-03-08 13:23   ` Gregory CLEMENT
2018-03-08 13:23     ` Gregory CLEMENT
2018-03-13 10:20     ` Richard Genoud
2018-03-13 10:32       ` Gregory CLEMENT
2018-03-13 10:32         ` 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.