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

[me@herrie.org]

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.

Thanks,
Herman