Re: [PATCH v3 4/6] clk: meson: t7: add support for the T7 SoC PLL clock

From: Jian Hu <jian.hu@amlogic.com>
To: Jerome Brunet <jbrunet@baylibre.com>
Cc: Xianwei Zhao <xianwei.zhao@amlogic.com>,
	Chuan Liu <chuan.liu@amlogic.com>,
	Neil Armstrong <neil.armstrong@linaro.org>,
	Kevin Hilman <khilman@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	Michael Turquette <mturquette@baylibre.com>,
	Dmitry Rokosov <ddrokosov@sberdevices.ru>,
	robh+dt <robh+dt@kernel.org>, Rob Herring <robh@kernel.org>,
	devicetree <devicetree@vger.kernel.org>,
	linux-clk <linux-clk@vger.kernel.org>,
	linux-amlogic <linux-amlogic@lists.infradead.org>,
	linux-kernel <linux-kernel@vger.kernel.org>,
	linux-arm-kernel <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v3 4/6] clk: meson: t7: add support for the T7 SoC PLL clock
Date: Thu, 3 Jul 2025 11:42:19 +0800	[thread overview]
Message-ID: <bd68352f-7f8c-4cbc-9f4f-f83645cc1f70@amlogic.com> (raw)
In-Reply-To: <1j7c10jmbp.fsf@starbuckisacylon.baylibre.com>

On 2025/6/25 15:46, Jerome Brunet wrote:
> [ EXTERNAL EMAIL ]
>
> On Wed 25 Jun 2025 at 10:52, Jian Hu <jian.hu@amlogic.com> wrote:
>
>> On 2025/6/17 0:27, Jerome Brunet wrote:
>>> [ EXTERNAL EMAIL ]
>>>
>>> On Thu 12 Jun 2025 at 21:02, Jian Hu <jian.hu@amlogic.com> wrote:
>>>
>>>>>> +
>>>>>> +static struct clk_regmap t7_pcie_pll_od = {
>>>>>> +     .data = &(struct clk_regmap_div_data){
>>>>>> +             .offset = ANACTRL_PCIEPLL_CTRL0,
>>>>>> +             .shift = 16,
>>>>>> +             .width = 5,
>>>>>> +             .flags = CLK_DIVIDER_ONE_BASED |
>>>>>> +                      CLK_DIVIDER_ALLOW_ZERO,
>>>>> What's the behaviour of the divider on zero then ?
>>>> If there is no CLK_DIVDER_ALLOW_ZERO, there is a warning when registering
>>>> t7_pcie_pll_od.
>>>>
>>>> like this:
>>>>
>>>>     ------------[ cut here ]------------
>>>>     WARNING: CPU: 1 PID: 1 at drivers/clk/clk-divider.c:140
>>>> divider_recalc_rate+0xfc/0x100
>>>>     pcie_pll_od: Zero divisor and CLK_DIVIDER_ALLOW_ZERO not set
>>>>     Modules linked in:
>>>>    CPU: 1 PID: 1 Comm: swapper/0 Not tainted
>>>> 5.4.283-09976-ga803e94eed88-dirty #91
>>>>     Hardware name: tm2_t962e2_ab311 (DT)
>>>>    Call trace:
>>>>     [ffffffc020003750+  64][<ffffffc0100e3e3c>] dump_backtrace+0x0/0x1e4
>>>>     [ffffffc020003790+  32][<ffffffc0100e4044>] show_stack+0x24/0x34
>>>>     [ffffffc0200037b0+  96][<ffffffc01130a2e8>] dump_stack+0xbc/0x108
>>>>    [ffffffc020003810+ 144][<ffffffc01010c484>] __warn+0xf4/0x1b8
>>>>     [ffffffc0200038a0+  64][<ffffffc01010c5f4>] warn_slowpath_fmt+0xac/0xc8
>>>>    [ffffffc0200038e0+  64][<ffffffc01061d364>] divider_recalc_rate+0xfc/0x100
>>>>    [ffffffc020003920+  80][<ffffffc010624e84>]
>>>> clk_regmap_div_recalc_rate+0x74/0x88
>>>>     [ffffffc020003970+  96][<ffffffc010616a54>] __clk_register+0x62c/0xb78
>>>>
>>>> so add it to avoid the warning.
>>> That does not really answer my question
>>
>> Sorry, I did not get you before.
>>
>> I have set OD to 0.  And measure pcie frequency,  It is 37.5Mhz.
>>
>> 4800Mhz/2/32/2=37.5Mhz, and the OD equal zero means divided by 32 in fact.
>>
> CLK_DIVIDER_MAX_AT_ZERO maybe ?

You are right, CLK_DIVIDER_MAX_AT_ZERO makes sense here.

its width is 5, and the divisor is 2^5 = 32.

I will add CLK_DIVIDER_MAX_AT_ZERO  and remove CLK_DIVIDER_ALLOW_ZERO in 
the next version.

>> Here is the test result:
>>
>> devm 0xfe008140 32
>>
>> 0xD40C04C8
>>
>> cat /sys/kernel/debug/meson-clk-msr/measure_summary | grep pcie
>>
>> [16] pcie_clk_inp      37500000  +/1 3125HZ
>>
>>
>> the OD divider is N crossover. it is one based.
>>
>> and It's possible to go from 1 to 31 crossovers.
> --
> Jerome

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