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: Make DU reset control optional for RZ/T2H support Date: Tue, 05 May 2026 11:22:42 +1000 Message-ID: In-Reply-To: <20260429170012.366537-3-prabhakar.mahadev-lad.rj@bp.renesas.com> References: <20260429170012.366537-1-prabhakar.mahadev-lad.rj@bp.renesas.com> <20260429170012.366537-3-prabhakar.mahadev-lad.rj@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 The change itself is a single-line diff: ```c - rcrtc->rstc =3D devm_reset_control_get_shared(rcdu->dev, NULL); + rcrtc->rstc =3D devm_reset_control_get_optional_shared(rcdu->dev, NULL); ``` **Issue 1 =E2=80=94 NULL reset not guarded in callers:** When the reset is optional, `devm_reset_control_get_optional_shared()` retu= rns `NULL` if no reset is specified in DT. The error path immediately after: ```c if (IS_ERR(rcrtc->rstc)) { dev_err(rcdu->dev, "can't get cpg reset\n"); return PTR_ERR(rcrtc->rstc); } ``` This correctly handles the `IS_ERR` case, and `NULL` will pass through (not= an error). However, we need to verify that all callers of `rcrtc->rstc` ha= ndle `NULL` safely. The reset control API (`reset_control_deassert`, `reset= _control_assert`) does handle `NULL` arguments gracefully (they're no-ops),= so this should be safe. This is fine. **Issue 2 =E2=80=94 Ordering concern:** This patch makes the reset optional unconditionally for all SoCs, but the D= T binding in patch 1 uses `resets: false` to forbid it only on T2H. The dri= ver change is broader than the binding constraint, which is acceptable sinc= e the optional API is safe for all variants. But the commit message should = clarify that the reset API itself is NULL-safe, so existing SoCs with reset= s continue to work unchanged. **Minor =E2=80=94 Error message is now misleading:** The error log `"can't get cpg reset\n"` still triggers on failure, but sinc= e the reset is optional, the error path only fires for actual errors (not a= bsence). This is fine, but the message could note it's a lookup error vs. a= bsence. --- Generated by Claude Code Patch Reviewer