Re: [PATCH 1/3] dt-bindings: clock: qcom,sm8450-videocc: Fix clock inputs for Glymur

[Shawn Guo <shengchao.guo@oss.qualcomm.com>]

On Tue, Sep 29, 2026 at 09:57:40AM +0200, Krzysztof Kozlowski wrote:
> On Fri, Sep 25, 2026 at 12:11:50AM +0800, Shawn Guo wrote:
> > The schema describes exactly two clock inputs for every compatible it
> > covers, a board XO and a video AHB clock from GCC. That is only true
> > for part of the drivers bound to these compatibles. videocc-glymur.c,
> > which handles qcom,glymur-videocc and qcom,nord-videocc, and
> > videocc-sm8750.c both declare their DT inputs as DT_BI_TCXO,
> > DT_BI_TCXO_AO and DT_SLEEP_CLK, and parent video_cc_sleep_clk_src on
> > DT_SLEEP_CLK.
> 
> I don't understand what you are saying here. You are mixing drivers and
> compatibles.

You are right! The argument was the wrong way round. What I should have
written for a schema is the hardware, not the Linux driver.  Sorry about
that.

Per the hardware documentation, the video clock controller on Glymur,
Nord and SM8750 has three clock inputs: the board XO, the always-on board
XO that feeds its PLL reference, and the 32 kHz chip sleep clock that
sources its sleep clock generator. There is no AHB clock input
on the block; the AHB clock the controller uses for itself is generated
internally from the XO input.

So the schema is wrong in two ways for these three compatibles: the sleep
clock input cannot be described at all, and the second item is described as
a clock that is not routed into the controller. I will respin with the
commit messages rewritten in those terms, with no driver references.

> 
> > 
> > Because the schema stops at two items, no device tree can supply the
> > third input, so video_cc_sleep_clk_src can never resolve its parent
> > and registers as an orphan clock. It also documents the second input
> > as an AHB clock, which no device tree using these two drivers passes,
> > and which those drivers would interpret as the always-on XO.
> > 
> > Describe three inputs for the Glymur, Nord and SM8750 compatibles,
> > keeping the existing two-input description for the rest. The sibling
> > qcom,glymur-evacc.yaml, whose driver has the same shape, already
> > documents a sleep clock this way.
> > 
> > Fixes: ed9ca8296147 ("dt-bindings: clock: qcom: Add video clock controller on Glymur SoC")
> > Fixes: b190eaea5780 ("dt-bindings: clock: qcom: Add SM8750 video clock controller")
> 
> Are you sure that you are not reverting review like it happened this
> week in IPQ? You know, the trick with reverting maintainer's review I
> mentioned on DT IRC?

That's definitely not my intention! I know nothing about the trick.
Would you point me to the IPQ thread or the IRC discussion, so that
I understand your comment better?

I checked review threads for both commits. Nothing seems to be reverted
here. Or if you would rather not have the Fixes tags point at those
commits, I'm happy to drop them.

Shawn