Claude review: fbdev: Consistently define pci_device_ids using named initializers

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Minor observations (non-blocking):**

1. **nvidia/nvidia.c — `PCI_DEVICE` with `PCI_ANY_ID`**: The conversion is semantically correct:

```c
-	{PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
-	 PCI_BASE_CLASS_DISPLAY << 16, 0xff0000, 0},
+	{
+		PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID),
+		.class = PCI_BASE_CLASS_DISPLAY << 16,
+		.class_mask = 0xff0000
+	},
```

`PCI_DEVICE(PCI_VENDOR_ID_NVIDIA, PCI_ANY_ID)` works because PCI matching treats `PCI_ANY_ID` in the device field as a wildcard. However, `PCI_DEVICE_CLASS()` would be a more semantically precise choice here, since this entry really matches by vendor + display class, not by a specific device. That said, `PCI_DEVICE_CLASS` sets vendor to `PCI_ANY_ID` too, so it would change the semantics (broadening the match beyond NVIDIA). The current conversion preserves the original behavior, which is the right call.

2. **riva/fbdev.c — `PCI_DEVICE` instead of `PCI_VDEVICE`**: All riva entries use `PCI_DEVICE(PCI_VENDOR_ID_XXX, ...)` rather than `PCI_VDEVICE(XXX, ...)`. This is because the first entry uses `PCI_VENDOR_ID_NVIDIA_SGS` (a different vendor from the rest), so `PCI_DEVICE` was chosen for uniformity across the table. Reasonable stylistic choice.

3. **pm3fb.c — device ID formatting**:

```c
-	{ PCI_VENDOR_ID_3DLABS, 0x0a,
+	{ PCI_VDEVICE(3DLABS, 0x000a) },
```

The value change from `0x0a` to `0x000a` is cosmetic (same value), but it's a nice touch to normalize to 4-digit hex for consistency with PCI device ID conventions.

4. **sis/sis_main.h — explicit `.driver_data = 0`**: The SIS_300 entry explicitly sets `.driver_data = 0`:

```c
+	{ PCI_VDEVICE(SI, PCI_DEVICE_ID_SI_300), .driver_data = 0 },
```

This is slightly redundant since `PCI_VDEVICE` already zeros the subsequent fields via its trailing `0, 0`, and the struct is zero-initialized. However, it does improve readability by making the indexing scheme explicit when other entries in the same table use non-zero `driver_data` values (1 through 11). Good call.

5. **matroxfb_base.c — local `PCI_DEVICE` to `PCI_VDEVICE` in stack variable**: The `intel_82437` local array inside `initMatrox2()` is converted:

```c
-		{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82437) },
+		{ PCI_VDEVICE(INTEL, PCI_DEVICE_ID_INTEL_82437) },
```

This is correct and equivalent since `driver_data` is unused.

6. **Style consistency**: The patch uses two formatting styles for entries with `driver_data`:
   - Single-line: `{ PCI_VDEVICE(ATI, ...), .driver_data = rage_M3 },` (aty128fb, neofb, s3fb)
   - Multi-line with `}, {` continuation: `PCI_VDEVICE(...),\n.driver_data = ...,\n}, {` (cyber2000fb, savagefb, tdfxfb)

   This isn't an issue — the multi-line form is used when the line would be too long otherwise — but worth noting for consistency if the maintainer has a preference.

7. **Missing trailing comma in nvidia.c**: The last field in the nvidia entry lacks a trailing comma:

```c
+		.class_mask = 0xff0000
+	},
```

While not a bug (it's the last field before the `}`), adding a trailing comma (`.class_mask = 0xff0000,`) is more consistent with kernel style and would make future additions cleaner. The geode/gx1fb_core.c conversion does include the trailing comma by contrast.

**No correctness issues found.** The patch is a clean mechanical transformation that preserves the compiled output. The use of `PCI_VDEVICE` vs `PCI_DEVICE` is appropriate in each case, and the class-match conversions (nvidia, tdfxfb, gx1fb) correctly use named `.class` / `.class_mask` fields.

---
Generated by Claude Code Patch Reviewer