Re: [PATCH v8 7/7] clk: rockchip: implement proper GATE_LINK support

From: Dmitry Osipenko <dmitry.osipenko@collabora.com>
To: Sebastian Reichel <sebastian.reichel@collabora.com>
Cc: Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	linux-clk@vger.kernel.org,
	Elaine Zhang <zhangqing@rock-chips.com>,
	Kever Yang <kever.yang@rock-chips.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Conor Dooley <conor+dt@kernel.org>,
	huangtao@rock-chips.com, andy.yan@rock-chips.com,
	devicetree@vger.kernel.org, linux-rockchip@lists.infradead.org,
	kernel@collabora.com
Subject: Re: [PATCH v8 7/7] clk: rockchip: implement proper GATE_LINK support
Date: Wed, 31 Jan 2024 19:10:39 +0300	[thread overview]
Message-ID: <a0f26bfd-5c78-4c5f-9c37-dc1cc3925ede@collabora.com> (raw)
In-Reply-To: <i5qagtdg73rhojifmublx2w2kxvjeisd6qwqbo5vnhrgyedx3i@pyckzpstlnvb>

On 1/30/24 17:47, Sebastian Reichel wrote:
> Hi Dmitry,
> 
> On Fri, Jan 26, 2024 at 10:36:13PM +0300, Dmitry Osipenko wrote:
>> On 1/26/24 21:18, Sebastian Reichel wrote:
>>> Recent Rockchip SoCs have a new hardware block called Native Interface
>>> Unit (NIU), which gates clocks to devices behind them. These effectively
>>> need two parent clocks.
>>>
>>> GATE_LINK type clocks handle the second parent via 'linkedclk' by using
>>> runtime PM clocks. To make that possible a new platform device is created
>>> for every clock handled in this way.
>>>
>>> Note, that before this patch clk_rk3588_probe() has never been called,
>>> because CLK_OF_DECLARE marks the DT node as processed. This patch replaces
>>> that with CLK_OF_DECLARE_DRIVER and thus the probe function is used now.
>>> This is necessary to have 'struct device' available.
>>>
>>> Also instead of builtin_platform_driver_probe, the driver has been
>>> switched to use core_initcall, since it should be fully probed before
>>> the Rockchip PM domain driver (and that is using postcore_initcall).
>>
>> Why clk driver needs to be fully probed before PD? The PD driver
>> shouldn't probe until all clk providers that it uses are registered, and
>> then both clk and PD should be registered at the default level.
> 
> The error handling in the rockchip PD driver needs rework to
> properly handle -EPROBE_DEFER, which I consider a separate series.
> 
> Note, that the driver currently has 'builtin_platform_driver_probe',
> but does not actually probe anything. All clocks are registered via
> CLK_OF_DECLARE, which happens even before core_initcall. So this
> does not make things worse.
> 
> Also the OF node is marked as initialized by the early clocks
> (CLK_OF_DECLARE_DRIVER) via the call to of_clk_add_provider(). This
> is necessary, since otherwise the early clocks cannot be referenced
> and we need the early clocks for the timer registration (so it's not
> possible to move all the clocks to late init). This effectively
> results in fw_devlink not working properly. It will tell PM domain
> driver too early, that it may start probing (so a bunch of useless
> -EPROBE_DEFER will happen).

Thanks for the clarification! Definitely will be good to improve the
probe defer handling in the future. And indeed, it can be done
separately from this patchset.

-- 
Best regards,
Dmitry