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: Re: [PATCH v2 2/2] drm/bridge: lt9211: Add drive-strength-microamp DT property
Date: Sat, 16 May 2026 11:56:03 +1000
Message-ID: <review-patch2-8bf05bf2-66a3-49e0-972e-4ab7ad9f0ff0@nabladev.com>
In-Reply-To: <8bf05bf2-66a3-49e0-972e-4ab7ad9f0ff0@nabladev.com>
References: <20260513133127.376458-1-bstruempfel@data-modul.com> <8bf05bf2-66a3-49e0-972e-4ab7ad9f0ff0@nabladev.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 driver patch adds:
- A `static const u8` lookup table mapping register index to microamp value
- A `u8 lvds_hsdrv_isel` field in the `lt9211` struct
- DT parsing in `lt9211_parse_dt()`
- The register write change in `lt9211_configure_tx()`

**Register change is correct:**
```c
-		{ 0x823f, 0x48 },
+		/* bits 3:0: RG_MLTX_HSDRV_ISEL, LVDS TX driver current */
+		{ 0x823f, 0x40 | ctx->lvds_hsdrv_isel },
```
Original `0x48` =3D `0100_1000` =E2=80=94 bit 6 set, bits 3:0 =3D 8. New co=
de: `0x40` (bit 6) OR'd with the 4-bit index (0=E2=80=9315), giving range `=
0x40`=E2=80=93`0x4F`. Default index 8 =E2=86=92 `0x48`. Matches.

**DT parsing logic is sound:**
```c
+	ctx->lvds_hsdrv_isel =3D 8; /* default: 25 uA */
+	ret =3D of_property_read_u32(dev->of_node, "drive-strength-microamp",
+				   &microamp);
+	if (!ret) {
+		for (i =3D 0; i < ARRAY_SIZE(lt9211_hsdrv_microamp); i++) {
+			if (lt9211_hsdrv_microamp[i] =3D=3D microamp) {
+				ctx->lvds_hsdrv_isel =3D i;
+				break;
+			}
+		}
+		if (i =3D=3D ARRAY_SIZE(lt9211_hsdrv_microamp)) {
+			dev_err(dev, "Invalid drive-strength-microamp value %u\n",
+				microamp);
+			return -EINVAL;
+		}
+	} else if (ret !=3D -EINVAL) {
+		dev_warn(dev, "Failed to read drive-strength-microamp: %d, using default=
\n",
+			 ret);
+	}
```

The three-way handling is correct: success =E2=86=92 lookup, `-EINVAL` =E2=
=86=92 property absent (silent default), other error =E2=86=92 warn and con=
tinue with default. The linear scan of 16 entries is fine for a one-time pr=
obe path.

**Minor observations (non-blocking):**

1. The existing driver uses `of_property_read_u32()` via `dev->of_node`, wh=
ich is consistent with the driver's existing OF usage (`of_graph_get_port_b=
y_id`, etc.). Some reviewers may suggest `device_property_read_u32(dev, ...=
)` as the more generic API, but given this driver is inherently OF-only, ei=
ther is acceptable.

2. The `dev_err` path for an invalid microamp value would only trigger if t=
he DT binding schema validation is bypassed (since the binding constrains v=
alues via enum). Returning `-EINVAL` here is still the right defensive choi=
ce.

No functional issues.

---
Generated by Claude Code Patch Reviewer