From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs
Date: Tue, 31 Mar 2026 17:49:51 +1000 [thread overview]
Message-ID: <review-patch1-20260329-fsl_ldb_schema_fix-v1-1-351372754bc0@nxp.com> (raw)
In-Reply-To: <20260329-fsl_ldb_schema_fix-v1-1-351372754bc0@nxp.com>
Patch Review
**Changes summary:**
1. Adds `minItems: 1` to the `reg` property definition
2. Removes `reg` from the global `required` list
3. Adds a new `if/then` block requiring `reg` only for `fsl,imx6sx-ldb` and `fsl,imx8mp-ldb`
4. Restructures the `reg-names` conditional from a negated check ("not imx6sx") to a positive match ("only imx8mp")
**Analysis:**
The logic changes are correct:
- **Before:** `reg` required for all three variants (imx6sx, imx8mp, imx93); `reg-names` required for imx8mp and imx93.
- **After:** `reg` required only for imx6sx and imx8mp; `reg-names` required only for imx8mp.
This is consistent — imx93 LDB needs neither `reg` nor `reg-names` since it lives under a syscon parent.
**Minor nits:**
1. `minItems: 1` on `reg` is technically redundant (1 is the implicit minimum for array properties in dt-schema), but it's harmless and clarifies intent alongside `maxItems: 2`. Not worth blocking over.
2. The commit message says "it's" (line 153 of the mbox) where it should be "its":
```
LDB's parent device could be a syscon which doesn't allow a reg property
to be present in it's child devices
```
Should be `its child devices`.
3. The restructuring of the `reg-names` conditional is a nice cleanup. The old form:
```yaml
- if:
not:
properties:
compatible:
contains:
const: fsl,imx6sx-ldb
then:
required:
- reg-names
```
used a negation that implicitly required `reg-names` for any future compatible added to the schema. The new positive match:
```yaml
- if:
properties:
compatible:
contains:
const: fsl,imx8mp-ldb
then:
required:
- reg-names
```
is more explicit and safer for future compatible additions, which is the right direction.
**Verdict:** Looks good. The Fixes tag is appropriate. Only the "it's → its" typo in the commit message is worth mentioning to the author.
---
Generated by Claude Code Patch Reviewer
prev parent reply other threads:[~2026-03-31 7:49 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-03-29 7:51 [PATCH] dt-bindings: display: bridge: ldb: Require reg property only for i.MX6SX/8MP LDBs Liu Ying
2026-03-29 17:42 ` Marco Felsch
2026-03-29 18:29 ` Marek Vasut
2026-03-30 6:29 ` Marco Felsch
2026-03-30 23:22 ` Marek Vasut
2026-03-30 2:05 ` Liu Ying
2026-03-30 10:28 ` Marco Felsch
2026-03-31 2:01 ` Liu Ying
2026-03-31 7:49 ` Claude review: " Claude Code Review Bot
2026-03-31 7:49 ` Claude Code Review Bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=review-patch1-20260329-fsl_ldb_schema_fix-v1-1-351372754bc0@nxp.com \
--to=claude-review@example.com \
--cc=dri-devel-reviews@example.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox