Hi Quentin,
On 10/8/2026 4:59 PM, Quentin Schulz wrote:
> Hi Jonas,
>
> On 8/4/26 2:26 AM, Jonas Karlman wrote:
>> The ACLK_BUS clock is initialized to 150 MHz in SPL on RK3568.
>>
>> Ensure clock driver is always probed after bind for SPL builds to
>> initialize core PLLs and clocks in SPL phase on RK3568.
>>
>
> You explain what this does but not why? What are the symptoms? Why is
> this the proper way to fix the issue?
Hum, as mentioned above and in cover letter, a few clocks/PLLs needs
to be initialized at SPL phase.
"""
This series flag clock drivers for probe after bind in SPL builds for
a few RK35xx SoCs that require core PLLs and clocks to be initialized at
an early phase.
This should help e.g. ensure SPLL is initialized correctly on RK3588 and
CPU core rate is initialized on RK3576 when booting from RAM/USB.
"""
The clock driver always used to probe in SPL back when these drivers
where added, however changes/optimizations have with time changed
this and now there are boot paths that never trigger a clock driver
probe in SPL, so clocks may no longer be initialized as currently
expected.
This series ensure that the expected clocks are always initialized in
SPL, i.e. restores the old and tested behavior.
>
> Based on Heiko's message, I'm guessing the issue is that we need
> something to setup clocks that are only accessible from the secure world
> and SPL happens to be running in it, but not proper (nor is the kernel).
> Hence why Heiko needs this in order for the "DSI panel to work again"
> (I'm assuming in the kernel hence why any fix in any driver/device tree
> to enable the clock is not gonna help with the issue).
If I remember correctly one of the clocks used in DSI pipeline may depend
on the SPLL, possible use the SPLL in parent clock tree. I think a patch
to set SPLL rate to fix something in OS was applied some time ago.
>
> This seems to be a bandaid patch instead of fixing the issue (that is,
> the kernel should enable/control this clock through some mechanisms,
> e.g. by interacting with TF-A through SCMI).
A few PLLs and clocks use strange or bad reset defaults, so clock driver
tries to normalize/make adjustments of these clocks to help support the
remaining default clock tree and rates.
Some of these PLLs affects CPU core rate others use these clocks as
parent clocks, so we want to normalize/adjust to the safe and expected
defaults as early as possible (SPL phase).
For current mainline unsupported SoCs I added a brief comment for any
such adjustments that are made by clock driver, e.g. for RV1126B:
/*
* Change CLK_CPLL_DIV10 (100M) from using reset default 10 (9)
* divider to 12 (11) divider to prevent overclocking to 118.8
* MHz when GPLL rate is changed below, after changes: 99 MHz
*/
writel(FIELD_PREP_WM16(CLK_CPLL_DIV10_SEL_MASK,
CLK_CPLL_DIV10_SEL_GPLL) |
FIELD_PREP_WM16(CLK_CPLL_DIV10_DIV_MASK, 11),
RV1126B_CLKSEL_CON(1));
/*
* Change CLK_GPLL_DIV6 (200M) from using reset default 4 (3)
* divider to 6 (5) divider to prevent overclocking to 297 MHz
* when GPLL rate is changed below, after changes: 198 MHz
*/
writel(FIELD_PREP_WM16(CLK_GPLL_DIV6_DIV_MASK, 5),
RV1126B_CLKSEL_CON(2));
I have delayed any such analysis and change to existing clock drivers
for a future clock improvement/refactor series, in this series I only
intend to restore the old behavior and apply clocks before SPL boot
devices are tested and not put too much weight into why we do it.
>
> Heiko also said "my DSI panel work again" which implies it worked at
> some point and got broken since, so it'd be nice to know what broke it
> so we can have all the clues necessary to fix the issue in the long
> term. Considering we broke something, I won't resist too much on this
> patch even though based on the above guesses we're fixing a kernel bug
> in U-Boot.
>
> Can you please confirm my guess or clarify?
See above for some details, in short: old code depends on that probe of
a boot udevice has a side-effect that clock driver is probed. Here we
instead ensure clocks are probed/initialized in a more controlled way.
Following commits have at least helped escalate this latent issue:
- c6bba31dbdd4 rockchip: spl-boot-order: Defer probe of boot device
- e22335a221f8 rockchip: Add support for RAM boot from maskrom mode
All udevices are no longer probed before they are boot tested in SPL
and RAM boot does not need any clocks.
Regards,
Jonas
>
> Cheers,
> Quentin
>
>> Signed-off-by: Jonas Karlman <[email protected]>
>> ---
>> drivers/clk/rockchip/clk_rk3568.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/clk/rockchip/clk_rk3568.c
>> b/drivers/clk/rockchip/clk_rk3568.c
>> index 9f3e9322161e..5a05f1b4de00 100644
>> --- a/drivers/clk/rockchip/clk_rk3568.c
>> +++ b/drivers/clk/rockchip/clk_rk3568.c
>> @@ -2934,6 +2934,10 @@ static int rk3568_clk_bind(struct udevice *dev)
>> struct udevice *sys_child;
>> struct sysreset_reg *priv;
>>
>> + /* Ensure clocks are initialized after bind in SPL build */
>> + if (IS_ENABLED(CONFIG_SPL_BUILD))
>> + dev_or_flags(dev, DM_FLAG_PROBE_AFTER_BIND);
>> +
>> /* The reset driver does not have a device node, so bind it here */
>> ret = device_bind_driver(dev, "rockchip_sysreset", "sysreset",
>> &sys_child);
>
