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: Fix assert of CMN_RSTB signal Date: Sun, 22 Mar 2026 04:12:40 +1000 Message-ID: In-Reply-To: <20260319164833.409126-3-biju.das.jz@bp.renesas.com> References: <20260319164833.409126-1-biju.das.jz@bp.renesas.com> <20260319164833.409126-3-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 Moves `reset_control_assert(dsi->rstc)` from `rzg2l_mipi_dsi_dphy_exit()` t= o `rzg2l_mipi_dsi_stop()`, placing it **before** the `dphy_exit()` call. ```c static void rzg2l_mipi_dsi_stop(struct rzg2l_mipi_dsi *dsi) { + reset_control_assert(dsi->rstc); dsi->info->dphy_exit(dsi); pm_runtime_put(dsi->dev); } ``` **Question on ordering:** The commit message says "placing it before the dp= hy_exit() call." The original code had the assert at the *end* of `dphy_exi= t()` =E2=80=94 so it was after the PHY power-down sequence. Now it's *befor= e* the PHY power-down. Is asserting CMN_RSTB before powering down the PHY (= clearing EN_LDO1200, EN_BGR) the correct hardware sequence? The commit mess= age doesn't explain *why* the assert must come before `dphy_exit()` rather = than after. The hardware manual section cited is about the reset sequence, = so if the manual says assert first then power down, this is fine, but it wo= uld be good to have that clarified in the commit message. **Fixes tags:** The two `Fixes:` tags reference commits that add OF data su= pport and lanes checking. These seem tangential =E2=80=94 the original plac= ement of `reset_control_assert()` in `dphy_exit()` dates back to the initia= l driver commit, not to these fixes-referenced commits. The `Fixes:` tags s= hould point to the commit that originally placed the assert in the wrong lo= cation. **RZ/V2H note:** The commit message says "Since this reset signal is option= al for RZ/V2H, the call is a no-op on that SoC." This is correct =E2=80=94 = `dsi->rstc` is obtained via `devm_reset_control_get_optional_exclusive()`, = so it will be NULL when not present, and `reset_control_assert(NULL)` is a = no-op. --- Generated by Claude Code Patch Reviewer