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

[Konrad Dybcio]

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