From: Jerome Brunet <jbrunet@baylibre•com>
To: Jian Hu <jian.hu@amlogic•com>
Cc: Jian Hu via B4 Relay <devnull+jian.hu.amlogic.com@kernel•org>,
Michael Turquette <mturquette@baylibre•com>,
Stephen Boyd <sboyd@kernel•org>, Rob Herring <robh@kernel•org>,
Krzysztof Kozlowski <krzk+dt@kernel•org>,
Conor Dooley <conor+dt@kernel•org>,
Neil Armstrong <neil.armstrong@linaro•org>,
Xianwei Zhao <xianwei.zhao@amlogic•com>,
Kevin Hilman <khilman@baylibre•com>,
Martin Blumenstingl <martin.blumenstingl@googlemail•com>,
linux-kernel@vger•kernel.org, linux-clk@vger•kernel.org,
devicetree@vger•kernel.org, linux-amlogic@lists•infradead.org,
linux-arm-kernel@lists•infradead.org
Subject: Re: [PATCH 07/10] clk: amlogic: Support POWER_OF_TWO for PLL pre-divider
Date: Tue, 26 May 2026 14:27:12 +0200 [thread overview]
Message-ID: <1ja4tm5pb3.fsf@starbuckisacylon.baylibre.com> (raw)
In-Reply-To: <3fda1592-f7d0-4e86-8615-602804673414@amlogic.com> (Jian Hu's message of "Tue, 26 May 2026 17:58:19 +0800")
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.
>
> 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.
>
> 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.
>
> 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.
>
>
> Best regards,
>
>
> Jian
--
Jerome
next prev parent reply other threads:[~2026-05-26 12:27 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-11 12:47 [PATCH 00/10] Add support for A9 family clock controller Jian Hu via B4 Relay
2026-05-11 12:47 ` [PATCH 01/10] dt-bindings: clock: Add Amlogic A9 SCMI " Jian Hu via B4 Relay
2026-05-11 12:47 ` [PATCH 02/10] dt-bindings: clock: Add Amlogic A9 PLL " Jian Hu via B4 Relay
2026-05-15 8:09 ` Krzysztof Kozlowski
2026-05-22 6:20 ` Jian Hu
2026-05-22 9:16 ` Krzysztof Kozlowski
2026-05-22 11:44 ` Jian Hu
2026-05-11 12:47 ` [PATCH 03/10] dt-bindings: clock: Add Amlogic A9 peripherals " Jian Hu via B4 Relay
2026-05-14 16:15 ` Jerome Brunet
2026-05-20 3:16 ` Jian Hu
2026-05-15 8:10 ` Krzysztof Kozlowski
2026-05-22 7:49 ` Jian Hu
2026-05-11 12:47 ` [PATCH 04/10] dt-bindings: clock: Add Amlogic A9 AO " Jian Hu via B4 Relay
2026-05-15 8:10 ` Krzysztof Kozlowski
2026-05-22 8:14 ` Jian Hu
2026-05-11 12:47 ` [PATCH 05/10] clk: amlogic: PLL l_detect signal supports active-high configuration Jian Hu via B4 Relay
2026-05-11 15:47 ` Brian Masney
2026-05-14 15:13 ` Jerome Brunet
2026-05-20 3:25 ` Jian Hu
2026-05-20 7:24 ` Jerome Brunet
2026-05-20 8:46 ` Jian Hu
2026-05-11 12:47 ` [PATCH 06/10] clk: amlogic: PLL reset signal supports active-low configuration Jian Hu via B4 Relay
2026-05-11 15:21 ` Brian Masney
2026-05-13 3:53 ` Jian Hu
2026-05-14 15:16 ` Jerome Brunet
2026-05-20 3:35 ` Jian Hu
2026-05-11 12:47 ` [PATCH 07/10] clk: amlogic: Support POWER_OF_TWO for PLL pre-divider Jian Hu via B4 Relay
2026-05-11 15:23 ` Brian Masney
2026-05-14 15:11 ` Jerome Brunet
2026-05-20 5:47 ` Jian Hu
2026-05-20 7:35 ` Jerome Brunet
2026-05-26 9:58 ` Jian Hu
2026-05-26 12:27 ` Jerome Brunet [this message]
2026-05-29 7:08 ` Jian Hu
2026-05-11 12:47 ` [PATCH 08/10] clk: amlogic: Add A9 PLL clock controller driver Jian Hu via B4 Relay
2026-05-11 15:36 ` Brian Masney
2026-05-13 7:25 ` Jian Hu
2026-05-14 16:12 ` Jerome Brunet
2026-05-20 7:33 ` Jian Hu
2026-05-11 12:47 ` [PATCH 09/10] clk: amlogic: Add A9 peripherals " Jian Hu via B4 Relay
2026-05-11 15:42 ` Brian Masney
2026-05-13 8:50 ` Jian Hu
2026-05-11 12:47 ` [PATCH 10/10] clk: amlogic: Add A9 AO " Jian Hu via B4 Relay
2026-05-11 15:45 ` Brian Masney
2026-05-13 9:19 ` Jian Hu
2026-05-14 16:27 ` Jerome Brunet
2026-05-20 7:37 ` Jian Hu
2026-05-26 7:33 ` [PATCH 00/10] Add support for A9 family clock controller Jerome Brunet
2026-05-26 10:05 ` Jian Hu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1ja4tm5pb3.fsf@starbuckisacylon.baylibre.com \
--to=jbrunet@baylibre$(echo .)com \
--cc=conor+dt@kernel$(echo .)org \
--cc=devicetree@vger$(echo .)kernel.org \
--cc=devnull+jian.hu.amlogic.com@kernel$(echo .)org \
--cc=jian.hu@amlogic$(echo .)com \
--cc=khilman@baylibre$(echo .)com \
--cc=krzk+dt@kernel$(echo .)org \
--cc=linux-amlogic@lists$(echo .)infradead.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-clk@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=martin.blumenstingl@googlemail$(echo .)com \
--cc=mturquette@baylibre$(echo .)com \
--cc=neil.armstrong@linaro$(echo .)org \
--cc=robh@kernel$(echo .)org \
--cc=sboyd@kernel$(echo .)org \
--cc=xianwei.zhao@amlogic$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox