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/nouveau/pci: use nv46 MSI rearm for G94 (NVAC/MCP79) Date: Sun, 12 Apr 2026 10:25:00 +1000 Message-ID: In-Reply-To: <20260409172126.115441-2-marek@czernohous.de> References: <20260409172126.115441-1-marek@czernohous.de> <20260409172126.115441-2-marek@czernohous.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Verdict: Likely correct fix, but commit message needs correction.** The one-line change itself is sound: ```c -.msi_rearm =3D nv40_pci_msi_rearm, +.msi_rearm =3D nv46_pci_msi_rearm, ``` Looking at the two implementations: - `nv40_pci_msi_rearm()` (nv40.c:29): `nvkm_pci_wr08(pci, 0x0068, 0xff)` = =E2=80=94 MMIO/PRI register write - `nv46_pci_msi_rearm()` (nv46.c:36): `pci_write_config_byte(pdev, 0x68, 0x= ff)` =E2=80=94 PCI config space write The comment in nv46.c:28-30 explicitly states: *"MSI re-arm through the PRI= appears to be broken on NV46/NV50/G84/G86/G92, so we access it via alterna= te PCI config space mechanisms."* Both g84 and g92 already use `nv46_pci_ms= i_rearm`, so g94 using `nv40_pci_msi_rearm` was arguably the odd one out in= its family. **Issue 1: Factually incorrect claim about NVAA.** The commit message state= s: *"The closely related NVAA (MCP77/MCP73) chipset has MSI disabled entire= ly in the driver (.msi_rearm is not set)."* This is **wrong**. Looking at `= base.c:1209`, NVAA uses `.pci =3D { 0x00000001, g94_pci_new }` =E2=80=94 th= e exact same PCI function as NVAC. NVAA has the same `msi_rearm` setting as= every other g94-family chip. This claim should be removed or corrected. **Issue 2: Scope wider than described.** The `g94_pci_func` is shared by ap= proximately 10 chipsets (G94, G96, G98, NVAA, NVAC, NVAF/MCP89, and several= GT2xx variants). The commit message frames this as an NVAC-specific fix, b= ut it changes behavior for all of them. The change is likely safe (and argu= ably correct for the whole family given the nv46.c comment and g84/g92 prec= edent), but the commit message should acknowledge this broader scope. --- Generated by Claude Code Patch Reviewer