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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <13871591.dW097sEU6C@phil>
Date: Tue, 21 Oct 2025 15:47:11 +0200
From: Heiko Stuebner <heiko@...ech.de>
To: mturquette@...libre.com, sboyd@...nel.org, sugar.zhang@...k-chips.com,
 zhangqing@...k-chips.com, robh@...nel.org, krzysztof.kozlowski+dt@...aro.org,
 conor+dt@...nel.org, Elaine Zhang <zhangqing@...k-chips.com>
Cc: devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-clk@...r.kernel.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org, huangtao@...k-chips.com,
 finley.xiao@...k-chips.com
Subject: Re: [PATCH v4 5/7] clk: rockchip: add support for pvtpll clk

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@...k-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