Re: [PATCH 1/2] clk: qcom: gcc-msm8660: register CE2 H clock

[Dmitry Baryshkov]

On Mon, Jun 15, 2026 at 07:21:24PM +0200, me@xxxxxxxxxx wrote:
> On 2026-06-15 18:36, Konrad Dybcio wrote:
> > On 6/2/26 6:27 AM, Herman van Hazendonk wrote:
> > > On MSM8x60 the Crypto Engine 2 (CE2) block at 0x18500000 is gated by
> > > a single hardware enable in GCC_CE2_HCLK_CTL (0x2740, BIT(4)). The
> > > existing dt-binding header already reserves CE2_H_CLK (ID 77) for
> > > this clock but the driver never registered an entry for it, so probe
> > > of any consumer that resolves the binding fails: the CE2 MMIO window
> > > reads back 0x0 and qce's DMA hangs indefinitely waiting for handshake
> > > signals that never arrive.
> > 
> > [...]
> > 
> > > +/*
> > > + * Crypto Engine 2 (CE2) clock.
> > > + *
> > > + * On MSM8x60 the CE2 block at 0x18500000 is gated by a single
> > > hardware
> > > + * enable in GCC_CE2_HCLK_CTL (0x2740, BIT(4)). The vendor MSM8660
> > > + * clock-8x60.c routes both the "core" and "iface" consumer-name
> > > lookups
> > > + * to this one register, and the upstream QCE crypto driver requests
> > > + * both clock names via devm_clk_get_optional_enabled(). Without the
> > > + * clock present at probe the QCE MMIO window reads back 0x0 and DMA
> > > + * hangs indefinitely waiting for handshake signals that never
> > > arrive.
> > > + *
> > > + * Register a single clk_branch: the consumer DT can reference the
> > > same
> > > + * clock phandle twice under different clock-names ("core" and
> > > "iface"),
> > > + * which yields the same struct clk for both clk_get() calls. Per-
> > > + * consumer refcounting then works correctly and the single
> > > underlying
> > > + * enable bit is asserted while either consumer holds the clock
> > > + * prepared, instead of having two independent clk_branch structs
> > > + * racing the same hardware bit.
> > > + */
> > 
> > I don't find this comment particularly valuable, given it ends up
> > describing the fact that the common clock framework has refcounted
> > enables (pretty widely known) and details how the DT is going to use
> > this (which we can read in the DT itself)
> > 
> > I think the commit message is really exhaustive and it's a better
> > place for this info anyway
> > 
> > Konrad
> Hi Konrad,
> 
> Happy to clean it up. MSM8x60 is poorly documented in public and whatever is
> available in downstream kernels is often incomplete, so I tried to document
> most in the various commits. Happy to put it in another place if that's more
> appropriate. A lot of the info was found by register poking and trial and
> error because the lack of documentation.

You can move it to the commit message. Another point would be to make
QCE driver request only one clock on this platform. Is there a point in
having two names for a single instance? (well, unless there is a good
reason in the driver).

> 
> Thanks,
> Herman

-- 
With best wishes
Dmitry