Re: [PATCH 07/10] clk: amlogic: Support POWER_OF_TWO for PLL pre-divider

[Jian Hu <jian.hu@amlogic.com>]

On 5/29/2026 3:08 PM, Jian Hu wrote:
> On 5/26/2026 8:27 PM, Jerome Brunet wrote:
>> [ EXTERNAL EMAIL ]
>>
>> On mar. 26 mai 2026 at 17:58, Jian Hu <jian.hu@amlogic.com> wrote:
>>
>>> On 5/20/2026 3:35 PM, Jerome Brunet wrote:
>>>> [ EXTERNAL EMAIL ]
>>>>
>>>> On mer. 20 mai 2026 at 13:47, Jian Hu <jian.hu@amlogic.com> wrote:
>>>>
>>>>> On 5/14/2026 11:11 PM, Jerome Brunet wrote:
>>>>>> [ EXTERNAL EMAIL ]
>>>>>>
>>>>>> On lun. 11 mai 2026 at 20:47, Jian Hu via B4 Relay 
>>>>>> <devnull+jian.hu.amlogic.com@kernel.org> wrote:
>>>>>>
>>>>>>> From: Jian Hu <jian.hu@amlogic.com>
>>>>>>>
>>>>>>> The A9 PLL pre-divider uses a division factor of 2^n to ensure a 
>>>>>>> clock
>>>>>>> duty cycle of 50% after predivision.
>>>>>>>
>>>>>>> Add flag 'CLK_MESON_PLL_N_POWER_OF_TWO' to indicate that the PLL
>>>>>>> pre-divider division factor is 2^n.
>>>>>> I understand what you are doing here but I have to ask why this 
>>>>>> can't be
>>>>>> implemented with independent dividers that already supports power 
>>>>>> of 2 ?
>>>>> If we use independent dividers, the n member would have to be 
>>>>> removed from
>>>>> meson_clk_pll_data.
>>>>>
>>>>> However, n is referenced 35 times in clk-pll.c, which means we 
>>>>> would need
>>>>> to modify all
>>>>> related logic across the file. This would be a relatively large
>>>>> change.
>>>> Yes
>>>>
>>>>> Moreover, for all Amlogic chips, the n divider is an indispensable 
>>>>> part of
>>>>> the DCO clock.
>>>> There is hardly a justification here
>>>>
>>>>> The difference between SoC generations is as follows:
>>>>>       Previous SoCs PLL: n = 1, 2, 3, 4... (linear divider)
>>>>>       A9 SoC PLL:            n = 2^0, 2^1, 2^2, 2^3, 2^4... 
>>>>> (power-of-two
>>>>> divider)
>>>> Yes that was fairly obvious
>>>>
>>>>> Therefore, splitting out the n divider from the DCO clock might 
>>>>> not be a
>>>>> good design choice.
>>>> I'm not sure I agree and you've only stated your point of view without
>>>> providing any technical justification here.
>>>>
>>>>   From the datasheets of the different SoC we have, the documented
>>>> limitation is always the DCO output rate range. Nothing related to 
>>>> n (or
>>>> m, or the mult-range for that matter). This is a legacy problem, we
>>>> started with monolithic driver and slowly simplified it.
>>>>
>>>> As far as I can see now, reworking the PLL driver to be a simple
>>>> multiplier driver with range output rate constraint could actually be
>>>> simpler than the current code. I would also make simpler to accomodate
>>>> differences such as the one presented here.
>>>>
>>>> Unless you can provide technical reasons why going in this direction
>>>> would be incorrect, that's where I'd prefer to go.
>>>>
>>>>> [...]
>>>>>
>>>>> Best regards,
>>>>>
>>>>> Jian
>>>> -- 
>>>> Jerome
>>>
>>> I agree that having an independent N divider would simplify the PLL 
>>> rate
>>> calculation.
>>>
>>> A separate pre-divider for N is technically possible, but there are 
>>> some
>>> hardware constraints that need to be considered:
>>>
>>> N = 1 is the preferred operating mode except a few fixed-frequency 
>>> PLLs.
>>> Larger N values reduce the PLL phase detector frequency, which may
>>> negatively impact
>>> jitter performance and overall PLL stability.
>> Understood. You could really make a difference by going deeper and
>> explaining what those constraints are, especially since you ask question
>> internally at Amlogic.
>>
>> At the moment what is documented is a range regarding the output rate of
>> the PLLs. A PLL is made of a pre-divider and fractional multiplier.
>> and you are saying that for the multiplier to work and lock, there is
>> actually a constraint the input rate too.
>>
>> If you can discuss with your HW team and clarify what the constraints
>> really are, that would help to better model the PLL. In then more likely
>> for us to figure out the best way to drive it.
>
>
> I have discussed with the HW PLL team. And here is the discussion 
> results:
>
> When N increases by a factor of X, the PLL bandwidth decreases by a 
> factor of X,
> deviating from the default optimal bandwidth, which leads to a 
> decrease in
> clock performance and affects the stability of clock-dependent modules 
> in the chip.
>
>>
>>> Because of this, we cannot guarantee stable system operation when 
>>> arbitrary
>>> larger
>>> N values are used.
>>>
>>> Some PLLs require non-1 N values to generate specific fixed output
>>> frequencies because
>>> the target rate cannot be achieved with N = 1 while keeping the PLL 
>>> while
>>> keeping the
>>> PLL within its valid operating range. So N is designed to have other 
>>> values
>>> ​​to
>>> satisfy this requirement.
>> Again it seems like the constraints we are using are not the real
>> limitation, just by-products, which the situation unclear.
>
>
> When N=3, this type of PLL is designed with the optimal bandwidth 
> based on N=3.
>
>>> For example, the AXG PCIe PLL uses N = 3 to generate the required 
>>> 100 MHz
>>> output frequency,
>>> since the target frequency cannot be achieved with N = 1.
>>>
>> PCIe is a topic in itself. It uses different ops for historic reasons 
>> though
>> I suspect, with proper constraints, it would not really need to.
>
>
> PCIe has a strict protocol that must be followed. It is also a highly 
> sensitive block
> with hundreds of complex and stringent lab test requirements.
>
> The PCIe PLL lock sequence needs to fully follow the Amlogic HW team's 
> released demo code.
>>> Additionally, is the refactored pre-divider N implemented as a separate
>>> patchset,
>>> independent from the A9 PLL changes?
>> I could be seen as a pre-requisite.
>
>
> Understood.
>

Regarding the suggestion to remove N from the DCO and make it a separate 
prescaler,

I have a local draft for this change. Would you like me to send it as a 
follow-up patch,

or are you planning to address it?

>>>
>>> Best regards,
>>>
>>>
>>> Jian
>> -- 
>> Jerome
>
>
> Best regards,
>
>
> Jian

--

Jian


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic