* [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints
@ 2026-04-10 10:59 Swamil Jain
2026-04-11 14:07 ` Krzysztof Kozlowski
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Swamil Jain @ 2026-04-10 10:59 UTC (permalink / raw)
To: jyri.sarha, tomi.valkeinen, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, robh, krzk+dt, conor+dt, devarsht
Cc: dri-devel, devicetree, linux-kernel, praneeth, vigneshr, s-jain1
The AM62L DSS [1] support incorrectly used the same register and
clock constraints as AM65x, but AM62L has a single video port
Fix this by adding conditional constraints that properly define the
register regions and clocks for AM62L DSS (single video port) versus
other AM65x variants (dual video port).
[1]: Section 12.7 (Display Subsystem and Peripherals)
Link : https://www.ti.com/lit/pdf/sprujb4
Fixes: cb8d4323302c ("dt-bindings: display: ti,am65x-dss: Add support for AM62L DSS")
Cc: stable@vger.kernel.org
Signed-off-by: Swamil Jain <s-jain1@ti.com>
---
Validated the changes with some examples:
https://gist.github.com/swamiljain/79f30568c9ece89f5a20218f52647486
Changelog:
v2->v3:
- Reduce redundancy and use constraints suggested by maintainers
- Remove blank line between the tags
Link to v2:
https://lore.kernel.org/all/20260129150601.185882-1-s-jain1@ti.com/
v1->v2:
- Remove oneOf from top level constraints, it makes bindings redundant
- Remove minItems from top level constraints
- "dma-coherent" property shouldn't be changed in v1 itself
- Add description for reg-names, clock and clock-names
- Add constraints specific to AM62L and for other SoCs within allOf
check
Link to v1:
https://lore.kernel.org/all/20251224133150.2266524-1-s-jain1@ti.com/
---
.../bindings/display/ti/ti,am65x-dss.yaml | 66 ++++++++++++++-----
1 file changed, 48 insertions(+), 18 deletions(-)
diff --git a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
index 38fcee91211e..d8a05bf62c2f 100644
--- a/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
+++ b/Documentation/devicetree/bindings/display/ti/ti,am65x-dss.yaml
@@ -36,34 +36,50 @@ properties:
reg:
description:
Addresses to each DSS memory region described in the SoC's TRM.
- items:
- - description: common DSS register area
- - description: VIDL1 light video plane
- - description: VID video plane
- - description: OVR1 overlay manager for vp1
- - description: OVR2 overlay manager for vp2
- - description: VP1 video port 1
- - description: VP2 video port 2
- - description: common1 DSS register area
+ oneOf:
+ - items:
+ - description: common DSS register area
+ - description: VIDL1 light video plane
+ - description: VID video plane
+ - description: OVR1 overlay manager for vp1
+ - description: OVR2 overlay manager for vp2
+ - description: VP1 video port 1
+ - description: VP2 video port 2
+ - description: common1 DSS register area
+ - items:
+ - description: common DSS register area
+ - description: VIDL1 light video plane
+ - description: OVR1 overlay manager for vp1
+ - description: VP1 video port 1
+ - description: common1 DSS register area
reg-names:
- items:
- - const: common
- - const: vidl1
- - const: vid
- - const: ovr1
- - const: ovr2
- - const: vp1
- - const: vp2
- - const: common1
+ oneOf:
+ - items:
+ - const: common
+ - const: vidl1
+ - const: vid
+ - const: ovr1
+ - const: ovr2
+ - const: vp1
+ - const: vp2
+ - const: common1
+ - items:
+ - const: common
+ - const: vidl1
+ - const: ovr1
+ - const: vp1
+ - const: common1
clocks:
+ minItems: 2
items:
- description: fck DSS functional clock
- description: vp1 Video Port 1 pixel clock
- description: vp2 Video Port 2 pixel clock
clock-names:
+ minItems: 2
items:
- const: fck
- const: vp1
@@ -179,6 +195,20 @@ allOf:
ports:
properties:
port@1: false
+ clock-names:
+ maxItems: 2
+ clocks:
+ maxItems: 2
+ reg:
+ maxItems: 5
+ else:
+ properties:
+ clock-names:
+ minItems: 3
+ clocks:
+ minItems: 3
+ reg:
+ minItems: 8
- if:
properties:
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints
2026-04-10 10:59 [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints Swamil Jain
@ 2026-04-11 14:07 ` Krzysztof Kozlowski
2026-04-11 23:43 ` Claude review: " Claude Code Review Bot
2026-04-11 23:43 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Krzysztof Kozlowski @ 2026-04-11 14:07 UTC (permalink / raw)
To: Swamil Jain
Cc: jyri.sarha, tomi.valkeinen, maarten.lankhorst, mripard,
tzimmermann, airlied, simona, robh, krzk+dt, conor+dt, devarsht,
dri-devel, devicetree, linux-kernel, praneeth, vigneshr
On Fri, Apr 10, 2026 at 04:29:55PM +0530, Swamil Jain wrote:
> clocks:
> + minItems: 2
> items:
> - description: fck DSS functional clock
> - description: vp1 Video Port 1 pixel clock
> - description: vp2 Video Port 2 pixel clock
>
> clock-names:
> + minItems: 2
> items:
> - const: fck
> - const: vp1
> @@ -179,6 +195,20 @@ allOf:
> ports:
> properties:
> port@1: false
> + clock-names:
> + maxItems: 2
> + clocks:
> + maxItems: 2
> + reg:
> + maxItems: 5
Also constrain for reg-names,
> + else:
> + properties:
> + clock-names:
> + minItems: 3
> + clocks:
> + minItems: 3
> + reg:
> + minItems: 8
Same here, please.
And if you are sending new version: they should be listed in the same
order as in top-level properties, so reg, reg-names, clocks and
clock-names. (juging by the diff)
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints
2026-04-10 10:59 [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints Swamil Jain
2026-04-11 14:07 ` Krzysztof Kozlowski
@ 2026-04-11 23:43 ` Claude Code Review Bot
2026-04-11 23:43 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-04-11 23:43 UTC (permalink / raw)
To: dri-devel-reviews
Overall Series Review
Subject: dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints
Author: Swamil Jain <s-jain1@ti.com>
Patches: 2
Reviewed: 2026-04-12T09:43:11.719203
---
This is a single patch (v3) fixing the DT binding for the AM62L DSS in `ti,am65x-dss.yaml`. The AM62L DSS has a single video port with only a "lite plane" (no VID plane, no OVR2, no VP2), but the original AM62L binding addition incorrectly reused the same 8-register, 3-clock constraints as the dual-video-port AM65x/AM625/AM62A7 variants.
The fix is well-structured and uses two complementary mechanisms:
1. `oneOf` at the property level for `reg`/`reg-names` to define both valid register layouts (8-item for dual-VP and 5-item for AM62L).
2. `if`/`then`/`else` conditionals in `allOf` to bind the correct register/clock counts to the correct `compatible` string.
The approach is clean and follows established dt-binding conventions. The `Fixes:` and `Cc: stable` tags are appropriate since the original AM62L binding would have validated incorrectly (requiring 8 registers and 3 clocks that don't exist on AM62L hardware).
**Verdict: Patch looks correct and ready to apply, with minor observations noted below.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
* Claude review: dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints
2026-04-10 10:59 [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints Swamil Jain
2026-04-11 14:07 ` Krzysztof Kozlowski
2026-04-11 23:43 ` Claude review: " Claude Code Review Bot
@ 2026-04-11 23:43 ` Claude Code Review Bot
2 siblings, 0 replies; 4+ messages in thread
From: Claude Code Review Bot @ 2026-04-11 23:43 UTC (permalink / raw)
To: dri-devel-reviews
Patch Review
**Overall: Good patch. The logic is correct and the YAML schema structure is sound.**
**Positive aspects:**
- The `oneOf` for `reg` correctly differentiates AM62L's 5 registers (common, vidl1, ovr1, vp1, common1) from the dual-VP variants' 8 registers (adding vid, ovr2, vp2). This accurately reflects that AM62L lacks the full VID plane, the second overlay manager, and the second video port.
- The `reg-names` `oneOf` is kept in sync with the `reg` `oneOf`, maintaining the correct name-to-register mapping.
- The `if`/`then`/`else` conditional tightly binds the register and clock counts to the `compatible` string, preventing mismatched DTs from validating:
```yaml
then:
properties:
clock-names:
maxItems: 2
clocks:
maxItems: 2
reg:
maxItems: 5
else:
properties:
clock-names:
minItems: 3
clocks:
minItems: 3
reg:
minItems: 8
```
**Minor observations (not blocking):**
1. **Base `clocks`/`clock-names` still list 3 items:** The base `items` array still describes all 3 clocks (fck, vp1, vp2), with `minItems: 2` added to relax the requirement. For AM62L (maxItems: 2), the third item schema (vp2) becomes dead/unreachable. This is functionally correct — JSON Schema tuple validation simply won't exercise the third schema — but it could be slightly clearer if a comment or the description noted that vp2 only applies to dual-VP variants. This is purely a readability nit.
2. **Commit message formatting:** The subject line wraps oddly across two lines in the mbox:
```
Subject: [PATCH v3] dt-bindings: display: ti,
am65x-dss: Fix AM62L DSS reg and clock constraints
```
This appears to be an artifact of the mail header folding (the comma after "ti" triggers the line wrap). It should be fine when applied via `git am`, but worth verifying the subject renders correctly.
3. **No example DT for AM62L:** The `examples:` section has DT snippets for `am65x-dss` and `am625-dss` but none for `am62l-dss`. Adding an AM62L example with the 5-register, 2-clock layout would be helpful for validation and documentation, but this could reasonably be done as a separate follow-up patch. The gist linked in the changelog shows the author has validated with examples externally.
4. **Schema cross-validation:** The `oneOf` on `reg`/`reg-names` and the `if`/`then`/`else` on counts are intentionally redundant — the `oneOf` constrains the valid layouts structurally, while the conditional constrains the counts per-compatible. Together they ensure that e.g. `ti,am62l-dss` with 8 registers would fail both the `maxItems: 5` check and (potentially) the `oneOf` matching. This belt-and-suspenders approach is good practice in dt-bindings.
**No functional issues found. The patch correctly fixes the AM62L DSS binding constraints.**
---
Generated by Claude Code Patch Reviewer
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-04-11 23:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-10 10:59 [PATCH v3] dt-bindings: display: ti, am65x-dss: Fix AM62L DSS reg and clock constraints Swamil Jain
2026-04-11 14:07 ` Krzysztof Kozlowski
2026-04-11 23:43 ` Claude review: " Claude Code Review Bot
2026-04-11 23:43 ` Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox