[PATCH] clk: at91: sam9x7: update pll clk ranges

[PATCH] clk: at91: sam9x7: update pll clk ranges

[Varshini Rajendran]

Update the min, max ranges of the PLL clocks according to the latest
datasheet to be coherent in the driver. This patch apparently solves
issues in obtaining the right sdio frequency.

Fixes: 33013b43e271 ("clk: at91: sam9x7: add sam9x7 pmc driver")
Suggested-by: Patrice Vilchez <Patrice.Vilchez@microchip.com>
Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
---
 drivers/clk/at91/sam9x7.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
index cbb8b220f16b..ffab32b047a0 100644
--- a/drivers/clk/at91/sam9x7.c
+++ b/drivers/clk/at91/sam9x7.c
@@ -61,44 +61,44 @@ static const struct clk_master_layout sam9x7_master_layout = {
 
 /* Fractional PLL core output range. */
 static const struct clk_range plla_core_outputs[] = {
-	{ .min = 375000000, .max = 1600000000 },
+	{ .min = 800000000, .max = 1600000000 },
 };
 
 static const struct clk_range upll_core_outputs[] = {
-	{ .min = 600000000, .max = 1200000000 },
+	{ .min = 600000000, .max = 960000000 },
 };
 
 static const struct clk_range lvdspll_core_outputs[] = {
-	{ .min = 400000000, .max = 800000000 },
+	{ .min = 600000000, .max = 1200000000 },
 };
 
 static const struct clk_range audiopll_core_outputs[] = {
-	{ .min = 400000000, .max = 800000000 },
+	{ .min = 600000000, .max = 1200000000 },
 };
 
 static const struct clk_range plladiv2_core_outputs[] = {
-	{ .min = 375000000, .max = 1600000000 },
+	{ .min = 800000000, .max = 1600000000 },
 };
 
 /* Fractional PLL output range. */
 static const struct clk_range plla_outputs[] = {
-	{ .min = 732421, .max = 800000000 },
+	{ .min = 400000000, .max = 800000000 },
 };
 
 static const struct clk_range upll_outputs[] = {
-	{ .min = 300000000, .max = 600000000 },
+	{ .min = 300000000, .max = 480000000 },
 };
 
 static const struct clk_range lvdspll_outputs[] = {
-	{ .min = 10000000, .max = 800000000 },
+	{ .min = 175000000, .max = 550000000 },
 };
 
 static const struct clk_range audiopll_outputs[] = {
-	{ .min = 10000000, .max = 800000000 },
+	{ .min = 0, .max = 300000000 },
 };
 
 static const struct clk_range plladiv2_outputs[] = {
-	{ .min = 366210, .max = 400000000 },
+	{ .min = 200000000, .max = 400000000 },
 };
 
 /* PLL characteristics. */
-- 
2.45.2

Re: [PATCH] clk: at91: sam9x7: update pll clk ranges

[Claudiu Beznea]

Hi, Varshini,

On 10.06.2025 11:45, Varshini Rajendran wrote:
> Update the min, max ranges of the PLL clocks according to the latest
> datasheet to be coherent in the driver. This patch apparently solves
> issues in obtaining the right sdio frequency.
> 
> Fixes: 33013b43e271 ("clk: at91: sam9x7: add sam9x7 pmc driver")
> Suggested-by: Patrice Vilchez <Patrice.Vilchez@microchip.com>
> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
> ---
>  drivers/clk/at91/sam9x7.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
> index cbb8b220f16b..ffab32b047a0 100644
> --- a/drivers/clk/at91/sam9x7.c
> +++ b/drivers/clk/at91/sam9x7.c
> @@ -61,44 +61,44 @@ static const struct clk_master_layout sam9x7_master_layout = {
>  
>  /* Fractional PLL core output range. */
>  static const struct clk_range plla_core_outputs[] = {
> -	{ .min = 375000000, .max = 1600000000 },
> +	{ .min = 800000000, .max = 1600000000 },
>  };
>  
>  static const struct clk_range upll_core_outputs[] = {
> -	{ .min = 600000000, .max = 1200000000 },
> +	{ .min = 600000000, .max = 960000000 },
>  };
>  
>  static const struct clk_range lvdspll_core_outputs[] = {
> -	{ .min = 400000000, .max = 800000000 },
> +	{ .min = 600000000, .max = 1200000000 },
>  };
>  
>  static const struct clk_range audiopll_core_outputs[] = {
> -	{ .min = 400000000, .max = 800000000 },
> +	{ .min = 600000000, .max = 1200000000 },
>  };
>  
>  static const struct clk_range plladiv2_core_outputs[] = {
> -	{ .min = 375000000, .max = 1600000000 },
> +	{ .min = 800000000, .max = 1600000000 },
>  };
>  
>  /* Fractional PLL output range. */
>  static const struct clk_range plla_outputs[] = {
> -	{ .min = 732421, .max = 800000000 },
> +	{ .min = 400000000, .max = 800000000 },
>  };
>  
>  static const struct clk_range upll_outputs[] = {
> -	{ .min = 300000000, .max = 600000000 },
> +	{ .min = 300000000, .max = 480000000 },
>  };
>  
>  static const struct clk_range lvdspll_outputs[] = {
> -	{ .min = 10000000, .max = 800000000 },
> +	{ .min = 175000000, .max = 550000000 },
>  };
>  
>  static const struct clk_range audiopll_outputs[] = {
> -	{ .min = 10000000, .max = 800000000 },
> +	{ .min = 0, .max = 300000000 },

Is this min value something valid?

Thank you,
Claudiu

Re: [PATCH] clk: at91: sam9x7: update pll clk ranges

[Claudiu Beznea]

Hi, Varshini,

On 24.06.2025 12:09, Varshini.Rajendran@microchip.com wrote:
> Hi Claudiu,
> 
> On 24/06/25 12:34 pm, Claudiu Beznea wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi, Varshini,
>>
>> On 10.06.2025 11:45, Varshini Rajendran wrote:
>>> Update the min, max ranges of the PLL clocks according to the latest
>>> datasheet to be coherent in the driver. This patch apparently solves
>>> issues in obtaining the right sdio frequency.
>>>
>>> Fixes: 33013b43e271 ("clk: at91: sam9x7: add sam9x7 pmc driver")
>>> Suggested-by: Patrice Vilchez <Patrice.Vilchez@microchip.com>
>>> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
>>> ---
>>>   drivers/clk/at91/sam9x7.c | 20 ++++++++++----------
>>>   1 file changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
>>> index cbb8b220f16b..ffab32b047a0 100644
>>> --- a/drivers/clk/at91/sam9x7.c
>>> +++ b/drivers/clk/at91/sam9x7.c
>>> @@ -61,44 +61,44 @@ static const struct clk_master_layout sam9x7_master_layout = {
>>>
>>>   /* Fractional PLL core output range. */
>>>   static const struct clk_range plla_core_outputs[] = {
>>> -     { .min = 375000000, .max = 1600000000 },
>>> +     { .min = 800000000, .max = 1600000000 },
>>>   };
>>>
>>>   static const struct clk_range upll_core_outputs[] = {
>>> -     { .min = 600000000, .max = 1200000000 },
>>> +     { .min = 600000000, .max = 960000000 },
>>>   };
>>>
>>>   static const struct clk_range lvdspll_core_outputs[] = {
>>> -     { .min = 400000000, .max = 800000000 },
>>> +     { .min = 600000000, .max = 1200000000 },
>>>   };
>>>
>>>   static const struct clk_range audiopll_core_outputs[] = {
>>> -     { .min = 400000000, .max = 800000000 },
>>> +     { .min = 600000000, .max = 1200000000 },
>>>   };
>>>
>>>   static const struct clk_range plladiv2_core_outputs[] = {
>>> -     { .min = 375000000, .max = 1600000000 },
>>> +     { .min = 800000000, .max = 1600000000 },
>>>   };
>>>
>>>   /* Fractional PLL output range. */
>>>   static const struct clk_range plla_outputs[] = {
>>> -     { .min = 732421, .max = 800000000 },
>>> +     { .min = 400000000, .max = 800000000 },
>>>   };
>>>
>>>   static const struct clk_range upll_outputs[] = {
>>> -     { .min = 300000000, .max = 600000000 },
>>> +     { .min = 300000000, .max = 480000000 },
>>>   };
>>>
>>>   static const struct clk_range lvdspll_outputs[] = {
>>> -     { .min = 10000000, .max = 800000000 },
>>> +     { .min = 175000000, .max = 550000000 },
>>>   };
>>>
>>>   static const struct clk_range audiopll_outputs[] = {
>>> -     { .min = 10000000, .max = 800000000 },
>>> +     { .min = 0, .max = 300000000 },
>>
>> Is this min value something valid?
> 
> Yes. This is a valid value mentioned in the datasheet. This is the range 
> that fixes the issue of not being able to set low frequency values for 
> the SDIO interface for instance.

Sorry for the late reply. This range is for audiopll. Can you please
explain in the commit message why this fixes issues with the SDIO interface.

Also, having zero here as min rate would involve, as of my code inspection,
setting frac->mul = -1 for disabled PLLs on probe (I suppose audio PLLs are
among them). Please check:

https://elixir.bootlin.com/linux/v6.15.5/source/drivers/clk/at91/clk-sam9x60-pll.c#L693

Having frac->mul = -1 means frac->mul = 0xff meaning the HW registers will
be set with the maximum multiplier. If my assumption is right, for a
reference clock of 24MHz this will mean the Fcorepll will be 384MHz which
is not in range for Fcorepll specifications (described by
audiopll_core_outputs[]).

This may be harmless if the PLL is unused at boot time, or reconfigured
later, but if the PLL is just enabled later (w/o setting again the
frequency) and the core output frequency is not in range (described by
audiopll_core_outputs[]) it will lead to PLL lock to take forever and the
execution to stuck.

Can you please check this?

Thank you,
Claudiu

Re: [PATCH] clk: at91: sam9x7: update pll clk ranges

[Claudiu Beznea]

Hi, Varshini,

On 11.07.2025 10:38, Varshini.Rajendran@microchip.com wrote:
> Hi Claudiu,
> 
> On 10/07/25 1:48 pm, Claudiu Beznea wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> Hi, Varshini,
>>
>> On 24.06.2025 12:09, Varshini.Rajendran@microchip.com wrote:
>>> Hi Claudiu,
>>>
>>> On 24/06/25 12:34 pm, Claudiu Beznea wrote:
>>>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>>>
>>>> Hi, Varshini,
>>>>
>>>> On 10.06.2025 11:45, Varshini Rajendran wrote:
>>>>> Update the min, max ranges of the PLL clocks according to the latest
>>>>> datasheet to be coherent in the driver. This patch apparently solves
>>>>> issues in obtaining the right sdio frequency.
>>>>>
>>>>> Fixes: 33013b43e271 ("clk: at91: sam9x7: add sam9x7 pmc driver")
>>>>> Suggested-by: Patrice Vilchez <Patrice.Vilchez@microchip.com>
>>>>> Signed-off-by: Varshini Rajendran <varshini.rajendran@microchip.com>
>>>>> ---
>>>>>    drivers/clk/at91/sam9x7.c | 20 ++++++++++----------
>>>>>    1 file changed, 10 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/drivers/clk/at91/sam9x7.c b/drivers/clk/at91/sam9x7.c
>>>>> index cbb8b220f16b..ffab32b047a0 100644
>>>>> --- a/drivers/clk/at91/sam9x7.c
>>>>> +++ b/drivers/clk/at91/sam9x7.c
>>>>> @@ -61,44 +61,44 @@ static const struct clk_master_layout sam9x7_master_layout = {
>>>>>
>>>>>    /* Fractional PLL core output range. */
>>>>>    static const struct clk_range plla_core_outputs[] = {
>>>>> -     { .min = 375000000, .max = 1600000000 },
>>>>> +     { .min = 800000000, .max = 1600000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range upll_core_outputs[] = {
>>>>> -     { .min = 600000000, .max = 1200000000 },
>>>>> +     { .min = 600000000, .max = 960000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range lvdspll_core_outputs[] = {
>>>>> -     { .min = 400000000, .max = 800000000 },
>>>>> +     { .min = 600000000, .max = 1200000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range audiopll_core_outputs[] = {
>>>>> -     { .min = 400000000, .max = 800000000 },
>>>>> +     { .min = 600000000, .max = 1200000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range plladiv2_core_outputs[] = {
>>>>> -     { .min = 375000000, .max = 1600000000 },
>>>>> +     { .min = 800000000, .max = 1600000000 },
>>>>>    };
>>>>>
>>>>>    /* Fractional PLL output range. */
>>>>>    static const struct clk_range plla_outputs[] = {
>>>>> -     { .min = 732421, .max = 800000000 },
>>>>> +     { .min = 400000000, .max = 800000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range upll_outputs[] = {
>>>>> -     { .min = 300000000, .max = 600000000 },
>>>>> +     { .min = 300000000, .max = 480000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range lvdspll_outputs[] = {
>>>>> -     { .min = 10000000, .max = 800000000 },
>>>>> +     { .min = 175000000, .max = 550000000 },
>>>>>    };
>>>>>
>>>>>    static const struct clk_range audiopll_outputs[] = {
>>>>> -     { .min = 10000000, .max = 800000000 },
>>>>> +     { .min = 0, .max = 300000000 },
>>>>
>>>> Is this min value something valid?
>>>
>>> Yes. This is a valid value mentioned in the datasheet. This is the range
>>> that fixes the issue of not being able to set low frequency values for
>>> the SDIO interface for instance.
>>
>> Sorry for the late reply. This range is for audiopll. Can you please
>> explain in the commit message why this fixes issues with the SDIO interface.
> 
> I can do that, while doing so, I can rephrase it as well. SDIO is just 
> an example, It can potentially fix any range issues in any clock 
> associated with peripherals can have.
> 
>>
>> Also, having zero here as min rate would involve, as of my code inspection,
>> setting frac->mul = -1 for disabled PLLs on probe (I suppose audio PLLs are
>> among them). Please check:
>>
>> https://elixir.bootlin.com/linux/v6.15.5/source/drivers/clk/at91/clk-sam9x60-pll.c#L693
>>
>> Having frac->mul = -1 means frac->mul = 0xff meaning the HW registers will
>> be set with the maximum multiplier. If my assumption is right, for a
>> reference clock of 24MHz this will mean the Fcorepll will be 384MHz which
>> is not in range for Fcorepll specifications (described by
>> audiopll_core_outputs[]).
>>
>> This may be harmless if the PLL is unused at boot time, or reconfigured
>> later, but if the PLL is just enabled later (w/o setting again the
>> frequency) and the core output frequency is not in range (described by
>> audiopll_core_outputs[]) it will lead to PLL lock to take forever and the
>> execution to stuck.
>>
>> Can you please check this?
> 
> I inspected the code on my side and did the calculations and I didn't 
> quite get the part how you arrived at the value 384MHz, my bad. But I 
> did get your point.
> 
> What if I set the minimum AudioPLL output that can be obtained from the 
> coreclk output range which is 600MHz minimum, which gives us 2343750Hz 
> as the minimum AudioPLL frequency achievable from the divider part. If 
> that is okay I can send another version of this patch.
> 

I've rechecked this on my side and I concluded I was wrong. The call:

ret = sam9x60_frac_pll_compute_mul_frac(&frac->core,
                                       characteristics->core_output[0].min,
                                       parent_rate, true);

from sam9x60_clk_register_frac_pll() is using
characteristics->core_output[0].min as parameter but not
characteristics->output[0].min as I initially considered. So, the change
here should be OK.

Sorry for confusion! Please re-send with the patch description adjusted and
I'll apply it.

Thank you,
Claudiu


> Thank you for spending the time to review my patch and for the detailed 
> explanations.
> 
>>
>> Thank you,
>> Claudiu
>>
>>
>>
> 
>