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: rcar-du: Fix crash when no CMM is available Date: Sun, 12 Apr 2026 12:28:45 +1000 Message-ID: In-Reply-To: <20260408124205.1962448-1-laurent.pinchart+renesas@ideasonboard.com> References: <20260408124205.1962448-1-laurent.pinchart+renesas@ideasonboard.com> <20260408124205.1962448-1-laurent.pinchart+renesas@ideasonboard.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.** All three sites are correctly changed from `rcrtc->c= mm->dev` to `rcrtc->cmm`. Looking at the initialization code at line 1302-1= 304: ```c /* CMM might be disabled for this CRTC. */ if (rcdu->cmms[swindex].dev) { rcrtc->cmm =3D &rcdu->cmms[swindex]; ``` `rcrtc->cmm` is only set when the corresponding `cmms[].dev` is non-NULL, w= hich means if `rcrtc->cmm` is non-NULL, then `rcrtc->cmm->dev` is guarantee= d non-NULL. So testing `rcrtc->cmm` is both sufficient and correct =E2=80= =94 the two conditions are semantically equivalent when `cmm` is non-NULL, = and only the new form is safe when `cmm` is NULL. **Site 1 (line 516):** `rcar_du_cmm_setup()` =E2=80=94 the early return gua= rd. After the fix, `rcrtc->cmm->dev` is still used on line 522 in `rcar_cmm= _setup(rcrtc->cmm->dev, &cmm_config)`, which is safe because the NULL check= now correctly gates it. **Site 2 (line 670):** `rcar_du_crtc_stop()` =E2=80=94 guards the `rcar_cmm= _disable(rcrtc->cmm->dev)` call on the next line. Correct. **Site 3 (line 729):** `rcar_du_crtc_atomic_enable()` =E2=80=94 guards the = `rcar_cmm_enable(rcrtc->cmm->dev)` call. Correct. **No issues found.** The commit message is clear, the Fixes tag correctly i= dentifies the offending commit, it has a Tested-by from the reporter, and t= he fix is the minimal correct change. Good to merge. --- Generated by Claude Code Patch Reviewer