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: arm64: dts: imx93: Add parallel display output nodes
Date: Tue, 03 Mar 2026 13:09:10 +1000
Message-ID: <review-patch3-20260302-v6-18-topic-imx93-parallel-display-v10-3-634fe2778c7a@pengutronix.de>
In-Reply-To: <20260302-v6-18-topic-imx93-parallel-display-v10-3-634fe2778c7a@pengutronix.de>
References: <20260302-v6-18-topic-imx93-parallel-display-v10-0-634fe2778c7a@pengutronix.de> <20260302-v6-18-topic-imx93-parallel-display-v10-3-634fe2778c7a@pengutronix.de>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

**Subject:** `[PATCH v10 3/3] arm64: dts: imx93: Add parallel display output nodes`

This patch adds the DT nodes for the LCDIF display controller and wires it up to the PDFC bridge.

**`assigned-clock-rates` count mismatch:**

```dts
			assigned-clocks = <&clk IMX93_CLK_MEDIA_AXI>,
					  <&clk IMX93_CLK_MEDIA_APB>,
					  <&clk IMX93_CLK_MEDIA_DISP_PIX>;
			assigned-clock-parents = <&clk IMX93_CLK_SYS_PLL_PFD1>,
						 <&clk IMX93_CLK_SYS_PLL_PFD1_DIV2>,
						 <&clk IMX93_CLK_VIDEO_PLL>;
			assigned-clock-rates = <400000000>, <133333333>;
```

Three clocks are listed in `assigned-clocks` and `assigned-clock-parents`, but `assigned-clock-rates` only has two values. The third clock (`MEDIA_DISP_PIX`) has its parent set to `VIDEO_PLL` but no rate specified. This is intentionally leaving the pixel clock rate to be set at runtime (which is correct since the pixel clock rate depends on the display mode), but it would be clearer to explicitly pass `<0>` as the third rate to indicate "don't set a rate" (the standard DT convention). Some DT validation tools may warn about the length mismatch.

**The `dpi-bridge` compatible uses `nxp,imx93-pdfc` in the common dtsi:**

```dts
			dpi_bridge: dpi-bridge {
				compatible = "nxp,imx93-pdfc";
```

Since `imx91_93_common.dtsi` is shared between i.MX91 and i.MX93, using the `nxp,imx93-pdfc` compatible here means i.MX91-based boards would also use this compatible. If i.MX91 should use `nxp,imx91-pdfc` (as defined in the binding), then the compatible should be overridden in the i.MX91-specific dtsi file, or the common dtsi should use the i.MX91 compatible with a fallback. Currently there doesn't appear to be an i.MX91-specific override.

**LCDIF node looks correct:**

The `lcdif` node at `0x4ae30000` with appropriate clocks, power-domains, and port wiring to the PDFC bridge looks well-structured. The imx93.dtsi addition of `endpoint@1` (LDB) and `endpoint@2` (DSI) stubs for the other two bridge outputs is a clean forward-looking pattern.

**The patch closes the `media_blk_ctrl` node and adds `lcdif` as a sibling:**

```dts
+		};
+
+		lcdif: display-controller@4ae30000 {
```

This correctly makes `lcdif` a sibling of `media_blk_ctrl` in the SoC bus, not a child. This is the right approach since the LCDIF has its own register space.

**Verdict:** Mostly looks good. The `assigned-clock-rates` length mismatch is a minor DT style issue. The compatible string usage in the common dtsi should be considered in light of i.MX91 support.

---
Generated by Claude Code Patch Reviewer