Re: [PATCH v5 1/7] dt-bindings: display: verisilicon,dc: generalize for single-output variants
From: Joey Lu
Date: Tue Jun 30 2026 - 03:08:39 EST
On 6/29/2026 11:02 PM, Conor Dooley wrote:
On Mon, Jun 29, 2026 at 01:31:46PM +0800, Icenowy Zheng wrote:You are right. The current MA35D1 clock driver does not correctly reflect
No, this doesn't correctly model the hardware topology -- this willThank you all for the detailed review and discussion, it reallyAh in this case this should be converted to an enum, I think.It does, yes.Ah does the list constraint the order of items? If itI don't think it is that strange, as the binding hasWell the implicit rules of DT binding schemas are quite+This is just maxItems: 1.
+ resets:
+ minItems: 1
+ maxItems: 1
+
+ reset-names:
+ items:
+ - const: core
weird...
reset-names:
items:
- const: core
- const: axi
- const: ahb
constrains
the
Alternatively, using an enum permits free ordering.
Should I send a patch for converting it?
Thanks,
Icenowy
helped
clarify the right approach.
Since I will supply all four clocks with the same phandle for
core/axi/ahb,
and only one reset "core" for MA35D1, the ordering constraint in the
`items` list is not a problem, "core" is already the first entry.
There
is no need to convert to an enum.
Regarding the clock situation for the MA35D1: I agree with supplying
all
four clocks (core, axi, ahb, pix0) in the devicetree, even though the
MA35D1 clock controller gates core/axi/ahb with a single bit. The DT
will
use the same clock phandle for core, axi, and ahb:
clocks = <&clk X>, <&clk X>, <&clk X>, <&pix_clk Y>;
clock-names = "core", "axi", "ahb", "pix0";
This correctly models the hardware topology. Since all three names
lead to clk_get_rate() return the rate of DC core clock when checking
the AXI clock rate, which is problematic because both clocks are
limiting the performance of the DC.
the real AXI/AHB clock rates, and I think it needs a more comprehensive
review and rework. I will address this in a separate clock driver patch
series in the future.
For this DRM patch series, the AXI and AHB clocks are shared system bus
clocks fixed by the SoC clock tree and are not managed by the display
driver. The driver does not query their frequencies for mode validation
or bandwidth decisions. Therefore, driver correctness is independent of
the reported AXI/AHB clock rates, meaning it will remain unaffected when
the clock driver is later reworked.
Got it.resolveThe patch is sent.
to the same underlying clock node, the CCF's standard enable
refcounting
handles the shared gate correctly without any custom implementation
needed.
I will also revert the change in patch 4/7 that made axi and ahb
clocks
optional, since they will now always be provided in the devicetree.
Regarding moving `resets` and `reset-names` to the top-level
`required:`,
I will wait for Icenowy's patch to land before sending v6 to avoid
duplicating the work.
Understood. I will keep the `thead,th1520-dc8200` conditional block.Correct. When the outer constraints are relaxed to deal with the newIn v6 I will update patch 1/7 with:I think this conditional block will still be needed, because it will
- Update the subject to "dt-bindings: display: verisilicon,dc: add
support for nuvoton,ma35d1-dcu"
- Lower `clocks`/`clock-names` `minItems` to 4 at the top level
- Remove the `thead,th1520-dc8200` conditional block entirely
need to constrain the minItems to ensure all clocks / resets are
populated.
device the conditional block for the th1520 becomes required. Or having
an else, but if all devices are likely to be different in terms of
configuration specific conditional blocks is better.