From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: dt-bindings: display: renesas, rzg2l-du: Add RZ/T2H and RZ/N2H support
Date: Tue, 05 May 2026 11:22:42 +1000
Message-ID: <review-patch1-20260429170012.366537-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
In-Reply-To: <20260429170012.366537-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
References: <20260429170012.366537-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20260429170012.366537-2-prabhakar.mahadev-lad.rj@bp.renesas.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**The compatible strings and fallback pattern look fine.** The RZ/N2H=E2=86=
=92RZ/T2H fallback follows the established pattern used by RZ/V2L=E2=86=92R=
Z/G2L and RZ/V2N=E2=86=92RZ/V2H.

**Issue 1 =E2=80=94 Removing `resets` and `ports` from global required list=
:**

```yaml
 required:
   - compatible
   - reg
   - interrupts
   - clocks
   - clock-names
-  - resets
   - power-domains
-  - ports
   - renesas,vsps
```

Removing `resets` and `ports` from the global `required` and conditionally =
adding them back in the `else` clause is workable, but the `else` clause wi=
ll match _any_ compatible that does NOT contain `renesas,r9a09g077-du`. Thi=
s is fine today but could be fragile if another SoC is added that also lack=
s a reset or uses `port` instead of `ports`. A more robust pattern would be=
 to add an explicit `if` for the existing SoCs that require `resets`/`ports=
` rather than relying on an `else` that implicitly covers all non-T2H varia=
nts.

Actually, looking more carefully, the `else` is tied to the `r9a09g077-du` =
conditional:

```yaml
  - if:
      properties:
        compatible:
          contains:
            const: renesas,r9a09g077-du
    then:
      properties:
        resets: false
      required:
        - port
    else:
      required:
        - resets
        - ports
```

This is technically correct =E2=80=94 the `else` covers all non-RZ/T2H vari=
ants, re-requiring `resets` and `ports`. But note that `resets: false` in t=
he `then` clause actively forbids the property, which is a strong statement=
. The cover letter says "the DU does not have a reset line" =E2=80=94 if th=
at's definitive, this is fine, but it means DT authors can't even include a=
n empty/stub reset on T2H. Confirm this is the hardware intent.

**Issue 2 =E2=80=94 `port` vs `ports` with single port:**

The patch adds a new `port` property alongside the existing `ports`:

```yaml
  port:
    $ref: /schemas/graph.yaml#/properties/port
    description: Single output port for single-output DU variants.
```

The existing DU variants already handle single-output configurations using =
`ports` with a single `port@0` child (see the r9a07g043u and r9a09g057 case=
s). Using a bare `port` instead of `ports` for T2H creates an inconsistency=
 in the binding. Consider whether `ports` with a single `port@0` child woul=
d be more consistent with the existing binding structure. This is a DT bind=
ing convention question that the DT maintainers (Rob/Krzysztof) should weig=
h in on.

**Issue 3 =E2=80=94 No RZ/T2H-specific port description in allOf:**

The existing `allOf` blocks describe what each port represents (e.g., "DPI"=
, "DSI"). The T2H `then` clause only requires `port` but doesn't add a desc=
ription for what the port represents (presumably DPI). Minor but worth addi=
ng for consistency.

---
Generated by Claude Code Patch Reviewer