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

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: Wed, 20 May 2026 09:35:34 +0200	[thread overview]
Message-ID: <1jqzn65y9l.fsf@starbuckisacylon.baylibre.com> (raw)
In-Reply-To: <8d89b669-e72e-4663-9596-999a12922d32@amlogic.com> (Jian Hu's message of "Wed, 20 May 2026 13:47:27 +0800")

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

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