From: Eugen Hristev <[email protected]>
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 <[email protected]>
---
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
On 11/09/2019 at 08:39, Eugen Hristev - M18282 wrote:
> From: Eugen Hristev <[email protected]>
>
> 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 <[email protected]>
Acked-by: Nicolas Ferre <[email protected]>
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
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
[email protected]:
> From: Eugen Hristev <[email protected]>
>
> 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 <[email protected]>
> ---
> 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,
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
> [email protected]:
>> From: Eugen Hristev <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>> 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,
>
>
>
>
Quoting [email protected] (2019-09-10 23:39:20)
> From: Eugen Hristev <[email protected]>
>
> 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 <[email protected]>
> ---
Is there a Fixes: tag for this? Seems like it was always wrong?
On 16.09.2019 22:52, Stephen Boyd wrote:
> Quoting [email protected] (2019-09-10 23:39:20)
>> From: Eugen Hristev <[email protected]>
>>
>> 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 <[email protected]>
>> ---
>
> 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
Quoting [email protected] (2019-09-10 23:39:20)
> From: Eugen Hristev <[email protected]>
>
> 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 <[email protected]>
> ---
Applied to clk-next