Claude review: Re: [PATCH 1/2] dt-bindings: display: verisilicon,dc: generalize for DCUltra Lite variant

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**1. `additionalProperties: true` is wrong — should be `unevaluatedProperties: false`**

```yaml
-additionalProperties: false
+additionalProperties: true
```

Setting `additionalProperties: true` in the base schema means **any** property name is accepted without validation, defeating the purpose of the schema. The standard pattern for DT binding inheritance is to use `unevaluatedProperties: false` in the sub-schema (which this patch already does in `nuvoton,ma35d1-dcu.yaml`) and either remove `additionalProperties` from the base or also use `unevaluatedProperties: false` there. The base schema should NOT use `additionalProperties: true`.

**2. The `compatible` change from `items` to `contains` weakens DC8000 validation**

```yaml
-    items:
-      - enum:
-          - thead,th1520-dc8200
-      - const: verisilicon,dc # DC IPs have discoverable ID/revision registers
+    contains:
+      enum:
+        - verisilicon,dc
+        - thead,th1520-dc8200
+        - nuvoton,ma35d1-dcu
```

The original binding required `compatible = "thead,th1520-dc8200", "verisilicon,dc"` — a two-element ordered list where the SoC-specific string comes first and the generic fallback second. The new `contains` schema merely checks that *at least one* of the listed strings appears somewhere in the compatible property. This means `compatible = "verisilicon,dc"` alone would now pass validation for DC8000 nodes, losing the requirement for the SoC-specific string. The DC8000 sub-schema (or a `thead,th1520-dc8200.yaml`) should enforce the ordered two-item compatible, while the base schema can use a looser constraint.

**3. The `select` stanza in the base schema lists `nuvoton,ma35d1-dcu`**

```yaml
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - verisilicon,dc
+          - thead,th1520-dc8200
+          - nuvoton,ma35d1-dcu
```

The base schema's `select` should not need to enumerate every downstream SoC-specific compatible. Each sub-schema has its own `select`. If the intent is that the base schema should validate *all* DC-derived nodes, this creates a maintenance burden — every new variant requires updating two files. Consider whether the base schema needs a `select` at all, or use a pattern match.

**4. Removing `ports` from required is correct for the base, but no sub-schema enforces it for DC8000**

```yaml
-  - ports
```

The `ports` property was required for DC8000 but is removed from the base. This is fine for the DCUltra Lite (which uses `port` singular), but there's no `thead,th1520-dc8200.yaml` sub-schema that adds `ports` back as required. This means existing DC8000 DT nodes would no longer be validated for having `ports`. A sub-schema for the TH1520 should be created or the existing `verisilicon,dc.yaml` should use a conditional (`if/then`) to require `ports` when the compatible matches DC8000.

**5. The `nuvoton,ma35d1-dcu.yaml` sub-schema looks reasonable**

The sub-schema properly:
- References the base schema via `allOf: - $ref`
- Defines `port` (singular) as required
- Lists 2 clocks with proper names
- Marks resets as optional with a description explaining firmware usage
- Has a complete example

Minor: The example includes `resets = <&sys MA35D1_RESET_DISP>` but the `required` list does not include `resets`. This is fine (optional reset), but it's worth confirming the driver doesn't try to use the reset on this variant (which the driver patch confirms — it doesn't).

---

---
Generated by Claude Code Patch Reviewer