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/panel: feiyang-fy07024di26a30d: return display-on error Date: Tue, 28 Apr 2026 15:39:46 +1000 Message-ID: In-Reply-To: <20260425053948.117714-1-cvan20191@gmail.com> References: <20260425053948.117714-1-cvan20191@gmail.com> <20260425053948.117714-1-cvan20191@gmail.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 **Correctness: Good.** The original code: ```c mipi_dsi_dcs_set_display_on(ctx->dsi); return 0; ``` discards the return value from `mipi_dsi_dcs_set_display_on()`, uncondition= ally returning success. The replacement: ```c return mipi_dsi_dcs_set_display_on(ctx->dsi); ``` correctly propagates any error to the caller. **Consistency with the rest of the driver: Good.** The sibling function `fe= iyang_disable()` at line 110 already does exactly this pattern with `mipi_d= si_dcs_set_display_off()`: ```c static int feiyang_disable(struct drm_panel *panel) { struct feiyang *ctx =3D panel_to_feiyang(panel); return mipi_dsi_dcs_set_display_off(ctx->dsi); } ``` So this patch makes `_enable` consistent with `_disable`. **Minor observation (not blocking):** The `feiyang_unprepare()` function (l= ine 113) takes a different approach =E2=80=94 it checks the return value of= `mipi_dsi_dcs_set_display_off()` and logs an error with `dev_err()` before= continuing. One could argue that logging the error in `feiyang_enable()` b= efore returning it would aid debugging, but the `_disable` function doesn't= log either, so the patch as written is consistent with the existing patter= n for the enable/disable pair. This is a style choice and not something tha= t should block acceptance. **Reviewed-by worthy.** No issues found. --- Generated by Claude Code Patch Reviewer