Hi Quentin,
On 10/9/2026 11:48 AM, Quentin Schulz wrote:
> Hi Jonas,
>
> On 8/4/26 11:44 PM, Jonas Karlman wrote:
>> The RK3568 PMU PLL helpers lookup the PMU CRU udevice before accessing
>> PMU PLL regs. This may trigger a full PMU clock driver probe to get the
>> PMU CRU base address from the clock driver private data, something that
>> can take several ms.
>>
>> Change to use the RK3568 specific PMU CRU base address consant directly
>> to simplify and remove the unneeded complexity of using runtime lookup
>> in SoC specific code.
>>
>> Signed-off-by: Jonas Karlman <[email protected]>
>> ---
>> drivers/clk/rockchip/clk_rk3568.c | 34 +++++--------------------------
>> 1 file changed, 5 insertions(+), 29 deletions(-)
>>
>> diff --git a/drivers/clk/rockchip/clk_rk3568.c
>> b/drivers/clk/rockchip/clk_rk3568.c
>> index 5a05f1b4de00..eaf51728520a 100644
>> --- a/drivers/clk/rockchip/clk_rk3568.c
>> +++ b/drivers/clk/rockchip/clk_rk3568.c
>> @@ -94,44 +94,20 @@ static ulong
>> rk3568_pmu_pll_set_rate(struct rk3568_clk_priv *priv,
>> ulong pll_id, ulong rate)
>> {
>> - struct udevice *pmucru_dev;
>> - struct rk3568_pmuclk_priv *pmu_priv;
>> - int ret;
>> -
>> - ret = uclass_get_device_by_driver(UCLASS_CLK,
>> - DM_DRIVER_GET(rockchip_rk3568_pmucru),
>> - &pmucru_dev);
>> - if (ret) {
>> - printf("%s: could not find pmucru device\n", __func__);
>> - return ret;
>> - }
>> - pmu_priv = dev_get_priv(pmucru_dev);
>> -
>> - rockchip_pll_set_rate(&rk3568_pll_clks[pll_id],
>> - pmu_priv->pmucru, pll_id, rate);
>> + static void * const pmu_cru_base = (void *)RK3568_PMU_CRU_BASE;
>
> Do we really need a static variable here?
I guess not, there seem to be a mix use of static vs non-static for
similar variables in Rockchip code, have changed to following for v2:
struct rk3568_pmucru * const pmu_cru_base = (void *)RK3568_PMU_CRU_BASE;
There is no change in code so the compiler strips out the static vs
non-static variable away the same.
>
>>
>> - return 0;
>> + return rockchip_pll_set_rate(&rk3568_pll_clks[pll_id],
>> + pmu_cru_base, pll_id, rate);
>
> This is a change in behavior, please have the change from return 0 to
> return rockchip_pll_set_rate in a separate commit.
Sure, I will split the change to make set_rate return the configured
rate to a separate patch.
>
>> }
>> #endif
>>
>> static ulong rk3568_pmu_pll_get_rate(struct rk3568_clk_priv *priv,
>> ulong pll_id)
>> {
>> - struct udevice *pmucru_dev;
>> - struct rk3568_pmuclk_priv *pmu_priv;
>> - int ret;
>> -
>> - ret = uclass_get_device_by_driver(UCLASS_CLK,
>> - DM_DRIVER_GET(rockchip_rk3568_pmucru),
>> - &pmucru_dev);
>> - if (ret) {
>> - printf("%s: could not find pmucru device\n", __func__);
>> - return ret;
>> - }
>> - pmu_priv = dev_get_priv(pmucru_dev);
>> + static void * const pmu_cru_base = (void *)RK3568_PMU_CRU_BASE;
>>
>
> Ditto for the static keyword here.
Have made same change here as above for v2.
Regards,
Jonas
>
> Changes are fine otherwise.
>
> Cheers,
> Quentin
