Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants

[Conor Dooley <conor@kernel.org>]

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

On Fri, Jun 26, 2026 at 05:09:12PM +0800, Icenowy Zheng wrote:
> 在 2026-06-26五的 09:57 +0100，Conor Dooley写道：
> > On Fri, Jun 26, 2026 at 03:58:14PM +0800, Icenowy Zheng wrote:
> > > 在 2026-06-26五的 08:22 +0100，Conor Dooley写道：
> > > > On Thu, Jun 25, 2026 at 05:33:37PM +0100, Conor Dooley wrote:
> > > > > On Thu, Jun 25, 2026 at 05:44:43PM +0800, Joey Lu wrote:
> > > > > > +
> > > > > > +  - if:
> > > > > > +      properties:
> > > > > > +        compatible:
> > > > > > +          contains:
> > > > > > +            const: nuvoton,ma35d1-dcu
> > > > > > +    then:
> > > > > > +      properties:
> > > > > > +        clocks:
> > > > > > +          minItems: 2
> > > > > 
> > > > > Anything that updates the minimum constraint should be done at
> > > > > the
> > > > > top
> > > > > level of this schema. The conditional section should then
> > > > > tighten
> > > > > the
> > > > > constraint, in this case that means only having maxItems.
> > > > > 
> > > > > > +          maxItems: 2
> > > > > > +
> > > > > > +        clock-names:
> > > > > > +          items:
> > > > > > +            - const: core
> > > > > > +            - const: pix0
> > > > > 
> > > > > Does this even work when the top level schema thinks clock 2
> > > > > should
> > > > > be
> > > > > called axi?
> > > > 
> > > > Additionally here, only have core and pix0 seems like it might be
> > > > an
> > > > oversimplification. I doubt removing the second output port means
> > > > that
> > > > the axi and ahb clocks are no longer needed.
> > > > Is it the case that your device supplies the same clock to core,
> > > > ahb
> > > > and
> > > > axi? If so, then you should fill those clocks in in your
> > > > devicetree
> > > > and
> > > > this can just constrain the number of clocks/clock-names to 4.
> > > 
> > > The clock controller of that SoC is quite weird -- it has only a
> > > single
> > > gate bit, but controlling 3 clock gates. All core, ahb and axi
> > > clocks
> > > have gates controlled by this single bit, so it's why currently
> > > it's
> > > modelled as only core clock supplied.
> > 
> > Yeah, then what's in the binding is definitely wrong.
> > Even if the same clock was provided to all clock inputs in the IP,
> > all
> > individual clock should be listed in the devicetree - although it
> > will
> > look a little silly to see clocks = <&foo 2>, <&foo 2>, <&foo 2>,
> > <&foo 2>;
> > In this case, 3 clocks controlled by 1 gate bit is an implementation
> > detail
> > of the SoC's clocking hardware, and not relevant to how the dc
> > instance
> > should be described.
> > 
> > > Well it might be worthful to supply the bus clock before the gate
> > > as
> > > ahb/axi, especially axi, because both the AXI clock and the core
> > > clock
> > > constraints the maximum pixel clock.
> > 
> > Right. And looking at patch 4/7, and the wording:
> > > The Nuvoton MA35D1 SoC integrates a DCUltraLite display controller
> > > whose
> > > AXI and AHB bus clocks share a single gate enable bit with the
> > > display
> > > core clock, so the clock driver does not expose them separately.
> > > This
> > > patch makes the axi and ahb clocks optional in the probe.
> > 
> > It sounds like there's probably some issues with how things are
> > modelled
> > clock wise in this device, unless this is not an accurate statement
> > and
> > there's actually one clock provided to all three inputs. If they're
> > distinct clocks, with different rates, only having one exposed has a
> > lot
> > of potential to be problematic!
> 
> Yes, I agree with this, they're different clocks according to the
> manual.
> 
> I added the clk people to the CC list in a reply of the previous
> revision, but they didn't react yet. I don't know how to represent
> multiple clock gates sharing a single control bit in the clock
> framework...

Yeah, I have absolutely no idea. Maybe it requires custom refcounting?
Surely this cannot be the only device that does something like this
though.

> Maybe just supplying the ungated AXI/AHB clocks here, and let the core
> clock manage the gate?

I guess, but that seems incorrect and would require commentary about why
it's being done. Feel like they (the missing axi/ahb clocks) should be
added to the clock driver and binding, and any special workarounds done
there.
Of course letting the core clock manage the gate and making the enable
method for the gated AXI/AHB clocks be a NOP is one way of handling it
in the clock driver. Still a bit of a hack compared to refcounting it,
but it makes me happier to have the correct clock tree modelled in DT.

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