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: Tue, 05 May 2026 09:27:22 +1000 Message-ID: In-Reply-To: <20260501132135.196701-1-chris.brandt@renesas.com> References: <20260501132135.196701-1-chris.brandt@renesas.com> <20260501132135.196701-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 **Bug analysis**: In the original code, `dma_alloc_coherent()` was the last= operation in `rzg2l_mipi_dsi_probe()`, placed after `mipi_dsi_host_registe= r()`. On allocation failure it executed: ```c return -ENOMEM; ``` This bypassed the error cleanup labels (`err_phy`, `err_pm_disable`), leaki= ng the `pm_runtime_enable()` state and the registered MIPI DSI host. **Fix correctness**: The allocation is moved to before `platform_set_drvdata()` and `pm_runtime_= enable()`: ```c dsi->dcs_buf_virt =3D dmam_alloc_coherent(dsi->dev, RZG2L_DCS_BUF_SIZE, &dsi->dcs_buf_phys, GFP_KERNEL); if (!dsi->dcs_buf_virt) return -ENOMEM; ``` At this point in the probe function, only `devm_*` managed resources have b= een acquired (clocks, resets, ioremap), so a plain `return` is safe =E2=80= =94 `devm` teardown handles everything. The switch from `dma_alloc_coherent= ` to `dmam_alloc_coherent` is the right choice: it's the device-managed var= iant, so the buffer is automatically freed when the device is unbound, elim= inating the need for the explicit `dma_free_coherent()` in `remove()`. **Remove path**: The `dma_free_coherent()` call is correctly removed from `= rzg2l_mipi_dsi_remove()` since `dmam_alloc_coherent()` handles cleanup auto= matically. **Device argument consistency**: The original code used `dsi->host.dev` as = the device for `dma_alloc_coherent()` / `dma_free_coherent()`, while the ne= w code uses `dsi->dev`. Looking at the probe function, `dsi->host.dev` is a= ssigned to `dsi->dev` later in the function (line 1475: `dsi->host.dev =3D = dsi->dev`), so using `dsi->dev` directly is correct and actually necessary = since `dsi->host.dev` isn't set yet at the new call site. **No issues found.** The patch is minimal, well-targeted, and correctly fix= es both the immediate error-path bug and the broader resource management co= ncern by using the managed API. --- Generated by Claude Code Patch Reviewer