From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm: renesas: rz-du: mipi_dsi: Fix return path on error Date: Thu, 23 Apr 2026 07:31:41 +1000 Message-ID: In-Reply-To: <20260422145146.1638998-1-chris.brandt@renesas.com> References: <20260422145146.1638998-1-chris.brandt@renesas.com> <20260422145146.1638998-1-chris.brandt@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 bug (confirmed in tree):** In the current code at line 1476-1479 of `= rzg2l_mipi_dsi.c`: ```c dsi->dcs_buf_virt =3D dma_alloc_coherent(dsi->host.dev, RZG2L_DCS_BUF_SIZE, &dsi->dcs_buf_phys, GFP_KERNEL); if (!dsi->dcs_buf_virt) return -ENOMEM; ``` This bare `return -ENOMEM` leaks two resources: 1. The `mipi_dsi_host_register` at line 1472 is never unregistered. 2. The `pm_runtime_enable` at line 1446 is never disabled. **The fix is correct and clean:** 1. **`dma_alloc_coherent` =E2=86=92 `dmam_alloc_coherent`**: The managed va= riant ties the DMA buffer lifetime to the device via devres, so no explicit= free is needed. This eliminates the `dma_free_coherent` call in `rzg2l_mip= i_dsi_remove`, which is correct. 2. **Moving allocation before `platform_set_drvdata` / `pm_runtime_enable`*= *: At this point in probe, only `devm_`-managed resources (clocks, resets, = ioremap) have been acquired. A plain `return -ENOMEM` is safe here =E2=80= =94 all prior allocations are device-managed and will be cleaned up automat= ically. The DMA buffer isn't used until actual DSI host transfers, so early= allocation is fine. 3. **Device parameter change**: The original used `dsi->host.dev`, the new = code uses `dsi->dev`. This is correct because `dsi->host.dev` hasn't been a= ssigned yet at the new call site, and they are set to the same value anyway= (line 1470: `dsi->host.dev =3D dsi->dev`). **Minor observations (not blocking):** - The commit message could be slightly more explicit about *what* was leaki= ng (host registration + pm_runtime), but "unwind correctly" is adequate for= a Fixes patch. - The ordering in `remove` now has `mipi_dsi_host_unregister` first and `pm= _runtime_disable` second, with the DMA free handled by devres (which runs a= fter `remove` returns). This ordering is correct =E2=80=94 devres cleanup h= appens during device unbind after the driver's remove callback. **No issues found. Patch is ready.** Reviewed-by: Dave Airlie --- Generated by Claude Code Patch Reviewer