linux-kernel - Re: [PATCH] clk: at91: sam9x7: update pll clk ranges

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d1a16315-8e17-446e-87f2-57f18046288c@tuxon.dev>
Date: Thu, 10 Jul 2025 11:18:34 +0300
From: Claudiu Beznea <claudiu.beznea@...on.dev>
To: Varshini.Rajendran@...rochip.com, mturquette@...libre.com,
 sboyd@...nel.org, Nicolas.Ferre@...rochip.com,
 alexandre.belloni@...tlin.com, linux-clk@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Cc: Patrice.Vilchez@...rochip.com
Subject: Re: [PATCH] clk: at91: sam9x7: update pll clk ranges

Hi, Varshini,

On 24.06.2025 12:09, Varshini.Rajendran@...rochip.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@...rochip.com>
>>> Signed-off-by: Varshini Rajendran <varshini.rajendran@...rochip.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