Re: [PATCH v4 5/7] clk: rockchip: add support for pvtpll clk

[Heiko Stuebner <heiko@sntech.de>]

Am Dienstag, 21. Oktober 2025, 08:52:30 Mitteleuropäische Sommerzeit schrieb Elaine Zhang:
> Support to adjust pvtpll by volt-sel and otp.
> Support calibrate pvtpll init frequency.
> 
> PVTPLL is used to monitor the chip performance variance caused by chip
> process, voltage and temperature, and generate a set of reference signals
> for adjusting the voltage of the chip.
> 
> PVTPLL supports the following features:
> 
> 1. A clock oscillation ring is integrated and used to generate a clock
> like signal (osc_clk),the frequency of this clock is determined
> by the cell delay value of clock oscillation ring circuit
> 
> 2. A frequency counter(osc_cnt) is used to measure the frequency of osc_clk.
> 
> 3. A externally input clock (ref_clk) is used as a reference clock for
> detecting the frequency of osc_clk.
> 
> 4. A calculation counter uses ref_clk to generate a configurable
> periodic timing window.
> 
> 5. Two clock counters are used to measure the frequency of the clock
> generated by OSC_WRAPPER?
> 
> 6. Support for dividing the ref_clk and osc_clk
> 
> 7. Support for configuring the effective polarity of the voltage
> regulator signal 'OUT'
> 
> The clock path of cpu used pvtpll:
> 
>     --gpll--|--\
>             |   \                                 | \
>             |    \                                |  \
>             |     \                               |   \
>    --v0pll--| mux |--[gate]--[div]--clk_core_src--|mux |--clk_core
>             |     /                               |   /
>             |    /   --ref_clk--[div]-cpu_pvtpll--|  /
>    --v1pll--|   /                                 | /
>             |--/
> 
> Signed-off-by: Elaine Zhang <zhangqing@rock-chips.com>

[...]

> +	pvtpll->regmap_cru = syscon_regmap_lookup_by_phandle_optional(np, "rockchip,cru");

Not convinced about that :-) .

(1) the cru itself already ioremaps the CRU memory, so having a syscon
    there would ioremap that memory a second time.
(2) we should definitly not expose the whole CRU io-memory to other
    drivers to write "random" stuff to. This will just invited further
    hacks, where people want to take shortcuts with clock settings.

Also this seems highly specific to the rv1126b.

Looking at the registers, this is a clk-mux between that deepslow clock
and the actual pvtpll output and the config function really only
reparents to the pvtpll in all cases.

So I believe this should in the worst case just be mux clock, but also
I see that the "correct" setting will already be set by the

        /* pvtpll src init */
        writel_relaxed(PVTPLL_SRC_SEL_PVTPLL, reg_base + RV1126B_CORECLKSEL_CON(0));
        writel_relaxed(PVTPLL_SRC_SEL_PVTPLL, reg_base + RV1126B_NPUCLKSEL_CON(0));
        writel_relaxed(PVTPLL_SRC_SEL_PVTPLL, reg_base + RV1126B_VICLKSEL_CON(0));
        writel_relaxed(PVTPLL_SRC_SEL_PVTPLL, reg_base + RV1126B_VEPUCLKSEL_CON(0));
        writel_relaxed(PVTPLL_SRC_SEL_PVTPLL, reg_base + RV1126B_VCPCLKSEL_CON(0));

in the rv1126b clock driver?

So that whole mode setting should not be necessary at all maybe?

Thanks
Heiko