Linux-Clk Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
@ 2019-09-11  6:39 Eugen.Hristev
  2019-09-11  7:03 ` Nicolas.Ferre
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Eugen.Hristev @ 2019-09-11  6:39 UTC (permalink / raw)
  To: mturquette, sboyd, alexandre.belloni, linux-clk,
	linux-arm-kernel, linux-kernel
  Cc: Nicolas.Ferre, Eugen.Hristev

From: Eugen Hristev <eugen.hristev@microchip.com>

The PLL input range needs to be able to allow 24 Mhz crystal as input
Update the range accordingly in plla characteristics struct

Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
---
 drivers/clk/at91/sama5d2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/at91/sama5d2.c b/drivers/clk/at91/sama5d2.c
index 6509d09..0de1108 100644
--- a/drivers/clk/at91/sama5d2.c
+++ b/drivers/clk/at91/sama5d2.c
@@ -21,7 +21,7 @@ static const struct clk_range plla_outputs[] = {
 };
 
 static const struct clk_pll_characteristics plla_characteristics = {
-	.input = { .min = 12000000, .max = 12000000 },
+	.input = { .min = 12000000, .max = 24000000 },
 	.num_output = ARRAY_SIZE(plla_outputs),
 	.output = plla_outputs,
 	.icpll = plla_icpll,
-- 
2.7.4


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

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
  2019-09-11  6:39 [PATCH] clk: at91: allow 24 Mhz clock as input for PLL Eugen.Hristev
@ 2019-09-11  7:03 ` Nicolas.Ferre
  2019-09-12 11:06 ` Alexander Dahl
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Nicolas.Ferre @ 2019-09-11  7:03 UTC (permalink / raw)
  To: Eugen.Hristev, mturquette, sboyd, alexandre.belloni, linux-clk,
	linux-arm-kernel, linux-kernel

On 11/09/2019 at 08:39, Eugen Hristev - M18282 wrote:
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> The PLL input range needs to be able to allow 24 Mhz crystal as input
> Update the range accordingly in plla characteristics struct
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>

Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>
Thanks Eugen!

Best regards,
   Nicolas

> ---
>   drivers/clk/at91/sama5d2.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/sama5d2.c b/drivers/clk/at91/sama5d2.c
> index 6509d09..0de1108 100644
> --- a/drivers/clk/at91/sama5d2.c
> +++ b/drivers/clk/at91/sama5d2.c
> @@ -21,7 +21,7 @@ static const struct clk_range plla_outputs[] = {
>   };
>   
>   static const struct clk_pll_characteristics plla_characteristics = {
> -	.input = { .min = 12000000, .max = 12000000 },
> +	.input = { .min = 12000000, .max = 24000000 },
>   	.num_output = ARRAY_SIZE(plla_outputs),
>   	.output = plla_outputs,
>   	.icpll = plla_icpll,
> 


-- 
Nicolas Ferre

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

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
  2019-09-11  6:39 [PATCH] clk: at91: allow 24 Mhz clock as input for PLL Eugen.Hristev
  2019-09-11  7:03 ` Nicolas.Ferre
@ 2019-09-12 11:06 ` Alexander Dahl
  2019-09-12 11:28   ` Eugen.Hristev
  2019-09-16 19:52 ` Stephen Boyd
  2019-09-18  5:00 ` Stephen Boyd
  3 siblings, 1 reply; 7+ messages in thread
From: Alexander Dahl @ 2019-09-12 11:06 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: Eugen.Hristev, mturquette, sboyd, alexandre.belloni, linux-clk,
	linux-kernel

Hello, 

out of curiosity: The SAMA5D27-SOM1-EK board has a 24 MHz crystal, that is 
also what /sys/kernel/debug/clk/clk_summary says and the board runs without 
obvious problems. What is this change improving in real practice then?

Greets
Alex

Am Mittwoch, 11. September 2019, 06:39:20 CEST schrieb 
Eugen.Hristev@microchip.com:
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> The PLL input range needs to be able to allow 24 Mhz crystal as input
> Update the range accordingly in plla characteristics struct
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---
>  drivers/clk/at91/sama5d2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/at91/sama5d2.c b/drivers/clk/at91/sama5d2.c
> index 6509d09..0de1108 100644
> --- a/drivers/clk/at91/sama5d2.c
> +++ b/drivers/clk/at91/sama5d2.c
> @@ -21,7 +21,7 @@ static const struct clk_range plla_outputs[] = {
>  };
> 
>  static const struct clk_pll_characteristics plla_characteristics = {
> -	.input = { .min = 12000000, .max = 12000000 },
> +	.input = { .min = 12000000, .max = 24000000 },
>  	.num_output = ARRAY_SIZE(plla_outputs),
>  	.output = plla_outputs,
>  	.icpll = plla_icpll,



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

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
  2019-09-12 11:06 ` Alexander Dahl
@ 2019-09-12 11:28   ` Eugen.Hristev
  0 siblings, 0 replies; 7+ messages in thread
From: Eugen.Hristev @ 2019-09-12 11:28 UTC (permalink / raw)
  To: ada, linux-arm-kernel
  Cc: mturquette, sboyd, alexandre.belloni, linux-clk, linux-kernel



On 12.09.2019 14:06, Alexander Dahl wrote:

> 
> Hello,
> 
> out of curiosity: The SAMA5D27-SOM1-EK board has a 24 MHz crystal, that is
> also what /sys/kernel/debug/clk/clk_summary says and the board runs without
> obvious problems. What is this change improving in real practice then?
> 

The board works, but, the characteristics of the PLL are incorrect.
This can lead to unwanted behavior, like calculating wrong minimum 
values for multipliers, or other issues

In this code here in clk-pll.c for example

if (parent_rate > characteristics->input.max) {
                 tmpdiv = DIV_ROUND_UP(parent_rate, 
characteristics->input.max);
                 if (tmpdiv > PLL_DIV_MAX) 

                         return -ERANGE; 

 

                 if (tmpdiv > mindiv) 

                         mindiv = tmpdiv; 

         }

The divisor is capped by checks, but at another possible requested 
parent rate, this may lead to something wrong, like here, the minimum 
divisor might be greater than what is the real possible one. So in some 
cases it can happen that unwanted results occur.

We may consider at some points to rely on these values more, so, it's 
obvious that they should be correct in the characteristics

So short answer: no improve in your case , where the rates required are 
around 492 Mhz cpu/132 mhz bus (IIRC), but the characteristics need to 
be correct to cover all possible cases.

Eugen


> Greets
> Alex
> 
> Am Mittwoch, 11. September 2019, 06:39:20 CEST schrieb
> Eugen.Hristev@microchip.com:
>> From: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> The PLL input range needs to be able to allow 24 Mhz crystal as input
>> Update the range accordingly in plla characteristics struct
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
>>   drivers/clk/at91/sama5d2.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/at91/sama5d2.c b/drivers/clk/at91/sama5d2.c
>> index 6509d09..0de1108 100644
>> --- a/drivers/clk/at91/sama5d2.c
>> +++ b/drivers/clk/at91/sama5d2.c
>> @@ -21,7 +21,7 @@ static const struct clk_range plla_outputs[] = {
>>   };
>>
>>   static const struct clk_pll_characteristics plla_characteristics = {
>> -	.input = { .min = 12000000, .max = 12000000 },
>> +	.input = { .min = 12000000, .max = 24000000 },
>>   	.num_output = ARRAY_SIZE(plla_outputs),
>>   	.output = plla_outputs,
>>   	.icpll = plla_icpll,
> 
> 
> 
> 

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

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
  2019-09-11  6:39 [PATCH] clk: at91: allow 24 Mhz clock as input for PLL Eugen.Hristev
  2019-09-11  7:03 ` Nicolas.Ferre
  2019-09-12 11:06 ` Alexander Dahl
@ 2019-09-16 19:52 ` Stephen Boyd
  2019-09-17  5:59   ` Eugen.Hristev
  2019-09-18  5:00 ` Stephen Boyd
  3 siblings, 1 reply; 7+ messages in thread
From: Stephen Boyd @ 2019-09-16 19:52 UTC (permalink / raw)
  To: Eugen.Hristev, alexandre.belloni, linux-arm-kernel, linux-clk,
	linux-kernel, mturquette
  Cc: Nicolas.Ferre, Eugen.Hristev

Quoting Eugen.Hristev@microchip.com (2019-09-10 23:39:20)
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> The PLL input range needs to be able to allow 24 Mhz crystal as input
> Update the range accordingly in plla characteristics struct
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---

Is there a Fixes: tag for this? Seems like it was always wrong?


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

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
  2019-09-16 19:52 ` Stephen Boyd
@ 2019-09-17  5:59   ` Eugen.Hristev
  0 siblings, 0 replies; 7+ messages in thread
From: Eugen.Hristev @ 2019-09-17  5:59 UTC (permalink / raw)
  To: sboyd, alexandre.belloni, linux-arm-kernel, linux-clk,
	linux-kernel, mturquette
  Cc: Nicolas.Ferre



On 16.09.2019 22:52, Stephen Boyd wrote:

> Quoting Eugen.Hristev@microchip.com (2019-09-10 23:39:20)
>> From: Eugen Hristev <eugen.hristev@microchip.com>
>>
>> The PLL input range needs to be able to allow 24 Mhz crystal as input
>> Update the range accordingly in plla characteristics struct
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
>> ---
> 
> Is there a Fixes: tag for this? Seems like it was always wrong?
> 

Hi Stephen,

At the initial design , the 12 Mhz was the only possibility for the 
boards themselves. But, with the commit who added this:

Fixes: c561e41ce4d2 ("clk: at91: add sama5d2 PMC driver")

Eugen

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

* Re: [PATCH] clk: at91: allow 24 Mhz clock as input for PLL
  2019-09-11  6:39 [PATCH] clk: at91: allow 24 Mhz clock as input for PLL Eugen.Hristev
                   ` (2 preceding siblings ...)
  2019-09-16 19:52 ` Stephen Boyd
@ 2019-09-18  5:00 ` Stephen Boyd
  3 siblings, 0 replies; 7+ messages in thread
From: Stephen Boyd @ 2019-09-18  5:00 UTC (permalink / raw)
  To: Eugen.Hristev, alexandre.belloni, linux-arm-kernel, linux-clk,
	linux-kernel, mturquette
  Cc: Nicolas.Ferre, Eugen.Hristev

Quoting Eugen.Hristev@microchip.com (2019-09-10 23:39:20)
> From: Eugen Hristev <eugen.hristev@microchip.com>
> 
> The PLL input range needs to be able to allow 24 Mhz crystal as input
> Update the range accordingly in plla characteristics struct
> 
> Signed-off-by: Eugen Hristev <eugen.hristev@microchip.com>
> ---

Applied to clk-next


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

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-11  6:39 [PATCH] clk: at91: allow 24 Mhz clock as input for PLL Eugen.Hristev
2019-09-11  7:03 ` Nicolas.Ferre
2019-09-12 11:06 ` Alexander Dahl
2019-09-12 11:28   ` Eugen.Hristev
2019-09-16 19:52 ` Stephen Boyd
2019-09-17  5:59   ` Eugen.Hristev
2019-09-18  5:00 ` Stephen Boyd

Linux-Clk Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-clk/0 linux-clk/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-clk linux-clk/ https://lore.kernel.org/linux-clk \
		linux-clk@vger.kernel.org linux-clk@archiver.kernel.org
	public-inbox-index linux-clk


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-clk


AGPL code for this site: git clone https://public-inbox.org/ public-inbox