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: rzg2l_mipi_dsi: Move rzg2l_mipi_dsi_set_display_timing() Date: Sun, 22 Mar 2026 04:12:40 +1000 Message-ID: In-Reply-To: <20260319164833.409126-2-biju.das.jz@bp.renesas.com> References: <20260319164833.409126-1-biju.das.jz@bp.renesas.com> <20260319164833.409126-2-biju.das.jz@bp.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 This moves `rzg2l_mipi_dsi_set_display_timing()` from `atomic_pre_enable` t= o `atomic_enable`, placing it after `rzg2l_mipi_dsi_start_hs_clock()`. The = hardware manual requires display timings to be set after HS clock is starte= d. **Issue: Silent dropping of error handling for rzg2l_mipi_dsi_startup()** In the original code: ```c ret =3D rzg2l_mipi_dsi_startup(dsi, mode); if (ret < 0) return; ``` The patch changes this to: ```c rzg2l_mipi_dsi_startup(dsi, mode); ``` `rzg2l_mipi_dsi_startup()` returns an int and can fail (e.g., `pm_runtime_r= esume_and_get()`, `dphy_init()`, `dphy_conf_clks()` can all return errors).= Silently ignoring the return value means that if startup fails, the driver= will proceed to `atomic_enable` and attempt to start the HS clock on unini= tialized hardware. The commit message mentions "Drop the unused ret variabl= e" but `ret` was being *used* for error checking =E2=80=94 it just couldn't= propagate the error since the bridge callback returns void. Still, the ear= ly return prevented further damage. This should be preserved: ```c if (rzg2l_mipi_dsi_startup(dsi, mode) < 0) return; ``` **Duplicated connector/crtc/mode lookup:** The mode lookup code is now dupl= icated in both `atomic_pre_enable` and `atomic_enable`. Consider caching th= e mode pointer in `struct rzg2l_mipi_dsi` during `atomic_pre_enable` to avo= id the duplication, though this is a minor style point. Otherwise the fix itself is correct per the hardware manual requirements. --- Generated by Claude Code Patch Reviewer