Re: [PATCH 2/2] dt-bindings: Add clock guard DT description

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2282 bytes --]

On Mon, Mar 23, 2026 at 02:52:21PM +0100, Vyacheslav Yurkov wrote:
> On 19.03.2026 17:50, Conor Dooley wrote:
> 
> > > I described a use case in my cover letter (PATCH 0). Perhaps our approach to
> > > tackle the issue is not correct in the first place. The term "virtual clock
> > > controller guard" is something we named it, but it's literally just a clock
> > > provider which combines several other clocks and input GPIO signals in order
> > > for the consumers to check whether they are allowed to probe already or have
> > > to wait until the input clocks are enabled.
> > 
> > Can you explain how this is different to gpio-gate-clock? AFAICT, you're
> > trying to support clocks that are enabled by a gpio, and that's what it
> > is for.
> > 
> It partially covers the similar use case, but differs in the sense that
> gpio-gate-clock controls the clock via GPIO (enable/disable), the
> clock-controller-guard gets the GPIO status signals whether the clock _was_
> enabled externally because a CPU has no direct access to the clock. So
> perhaps the terminology I came up with is not so self-explanatory, that's
> why I posted it for review and other opinions.

The binding you've got says "GPIOs used to control or guard the clocks",
which is not what you're saying that is going on in this mail. A more
suitable description would be "GPIOs used to check the status of the
clocks".

I want to see an example dts user for this please.

TBH, I don't understand your driver implementation either and why it has

+static const struct clk_ops clkctrl_guard_ops = {

+	.enable =	clkctrl_guard_enable,
+	.disable =	clkctrl_guard_disable,
+	.prepare =	clkctrl_guard_prepare,
+	.unprepare =	clkctrl_guard_unprepare,
+	.is_prepared =	clkctrl_guard_is_prepared,

any of these 4 implemented when you have no control over the clock.
I didn't think it was required to call your parent clocks enables in
your own enable either, thought that was handled by the core recursively
calling clk_enable() on clk->parent. The one thing I would expect you to
have implemented ops wise is is_enabled, which you don't have.
Also no sign of any rate acquisition functions, which I thought were
mandatory.

+	.get_parent =	clkctrl_guard_get_parent,
+};

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]