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: drm/xlnx/zynqmp-dpsub: Fix dependencies for COMPILE_TEST
Date: Sun, 12 Apr 2026 12:56:35 +1000
Message-ID: <review-patch1-20260408081430.1712335-1-wenst@chromium.org>
In-Reply-To: <20260408081430.1712335-1-wenst@chromium.org>
References: <20260408081430.1712335-1-wenst@chromium.org> <20260408081430.1712335-1-wenst@chromium.org>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0

Patch Review

**Correctness: Good**

I verified the actual build-time dependencies in the driver source:

1. `zynqmp_dp.c` includes `<linux/phy/phy.h>` and calls `devm_phy_get()`, `=
phy_init()`, `phy_power_on()`, `phy_configure()`, `phy_exit()`. These are a=
ll generic PHY framework functions that are available when `GENERIC_PHY` is=
 set =E2=80=94 and the Kconfig already has `select GENERIC_PHY`. There is n=
o direct include of or dependency on any `PHY_XILINX_ZYNQMP`-specific heade=
r.

2. `zynqmp_disp.c` includes `<linux/dma/xilinx_dpdma.h>` =E2=80=94 but this=
 header is just a trivial struct definition (`struct xilinx_dpdma_periphera=
l_config`) with no conditional compilation or dependency on `XILINX_ZYNQMP_=
DPDMA`. The actual DMA calls (`dma_request_chan()`, `dmaengine_prep_interle=
aved_dma()`, etc.) come from `<linux/dmaengine.h>`, which is provided by `D=
MA_ENGINE` (already selected).

So both `PHY_XILINX_ZYNQMP` and `XILINX_ZYNQMP_DPDMA` are indeed runtime-on=
ly dependencies =E2=80=94 they ensure the specific hardware drivers are pre=
sent so that `devm_phy_get()` and `dma_request_chan()` find their providers=
 at probe time. They are not needed for compilation.

**The Kconfig logic:**

```kconfig
-	depends on ARCH_ZYNQMP || COMPILE_TEST
+	depends on (ARCH_ZYNQMP && PHY_XILINX_ZYNQMP && XILINX_ZYNQMP_DPDMA) || C=
OMPILE_TEST
```

This correctly:
- Preserves the full dependency chain for real `ARCH_ZYNQMP` builds (PHY + =
DMA drivers must be enabled)
- Relaxes only for `COMPILE_TEST`, where those specific drivers aren't need=
ed to compile

**Minor observation:** The author notes in the cover message that the two d=
river dependencies "could be removed altogether" (i.e., not required even f=
or `ARCH_ZYNQMP` builds). This is technically true =E2=80=94 the driver wou=
ld just fail at probe with `-ENODEV`/`-EPROBE_DEFER` =E2=80=94 but keeping =
them as hard dependencies for real hardware builds is a reasonable user-exp=
erience choice, preventing users from building a kernel that would silently=
 fail to bring up the display. The current conservative approach is the rig=
ht call.

**No issues found.**=20

Reviewed-by worthy.

---
Generated by Claude Code Patch Reviewer