From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: drm/nouveau: handle nouveau_hw_get_clock() return value
Date: Sat, 16 May 2026 10:30:38 +1000
Message-ID: <review-patch1-20260514174008.4515-1-ms.bulatov7@gmail.com>
In-Reply-To: <20260514174008.4515-1-ms.bulatov7@gmail.com>
References: <20260514174008.4515-1-ms.bulatov7@gmail.com> <20260514174008.4515-1-ms.bulatov7@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 concern =E2=80=94 uninitialized output parameters on early re=
turn:**

```c
if (MClk <=3D 0 || NVClk <=3D 0)
    return;
```

When this early return is taken, `nv04_update_arb()` never writes to `*burs=
t` or `*lwm`. The only caller path through `nouveau_calc_arb()` reaching th=
is code is in `crtc.c`:

```c
int arb_burst, arb_lwm;            /* line 848 =E2=80=94 uninitialized */
nouveau_calc_arb(dev, crtc->mode.clock, drm_fb->format->cpp[0] * 8,
                 &arb_burst, &arb_lwm);
regp->CRTC[NV_CIO_CRE_FF_INDEX] =3D arb_burst;       /* used uninitialized =
*/
regp->CRTC[NV_CIO_CRE_FFLWM__INDEX] =3D arb_lwm & 0xff;  /* used uninitiali=
zed */
```

This writes garbage stack data to hardware CRTC registers. The fix should s=
et `*burst` and `*lwm` to safe defaults (e.g., 0) before returning, or the =
function signature should be changed to return an error that the caller che=
cks.

The simplest fix would be:

```c
if (MClk <=3D 0 || NVClk <=3D 0) {
    *burst =3D 0;
    *lwm =3D 0;
    return;
}
```

**Minor: the `<=3D 0` check conflates two cases.** `nouveau_hw_get_clock()`=
 can return:
- `0` =E2=80=94 from `nouveau_hw_pllvals_to_clk()` when M1 or M2 is zero
- A negative errno (e.g., `-ENOENT`) =E2=80=94 from `nouveau_hw_get_pllvals=
()` failure

The `<=3D 0` check correctly catches both, so the logic is sound, but the c=
ommit message claims to "fix all this by adding a check" when the real root=
 cause (propagating error codes properly from `nouveau_hw_get_clock`) remai=
ns unfixed. This is acceptable for a targeted defensive fix, but worth noti=
ng.

**Commit message nit:** The Fixes tag references `6ee738610f41` ("drm/nouve=
au: Add DRM driver for NVIDIA GPUs"), which is the initial nouveau commit. =
While technically correct that the bug has existed since then, it would be =
more useful to reference the commit that introduced this specific code path=
 if one exists, since the Fixes tag drives stable-kernel backport decisions.

**Summary:** The identified bug (division by zero) is real and worth fixing=
. The patch needs a small revision to initialize the output parameters befo=
re the early return to avoid writing uninitialized values to hardware regis=
ters.

---
Generated by Claude Code Patch Reviewer