Claude review: drm/panel: simple: add Waveshare LCD panels

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

Patch Review

This is the bulk of the series, adding 15 panel definitions. Several observations:

**1. Missing trailing commas in OF table entries.** Every new `.data` line is missing a trailing comma, while the existing entries all have them:

```c
	}, {
		.compatible = "waveshare,2.8inch-panel",
		.data = &waveshare_28_lcd_panel    /* <-- missing comma */
	}, {
```

Compare with existing style:
```c
	}, {
		.compatible = "waveshare,13.3inch-panel",
		.data = &waveshare_133inch,        /* <-- has comma */
	}, {
```

All 15 new entries are missing the trailing comma. While this compiles fine (the comma is optional before `}`), the existing code consistently uses trailing commas and this should match for style consistency.

**2. Forward declaration of `waveshare_80_lcd_c_mode`.** The patch uses:

```c
static const struct drm_display_mode waveshare_80_lcd_c_mode;
static const struct panel_desc waveshare_70_lcd_e_panel = {
	.modes = &waveshare_80_lcd_c_mode,
```

...with the actual definition appearing later. This works in C but is unusual for this file. It could be avoided by simply reordering the definitions so `waveshare_80_lcd_c_mode` and `waveshare_80_lcd_c_panel` appear before `waveshare_70_lcd_e_panel`. Since the panels are ordered numerically, moving 8.0" before 7.0"-E would be a minor ordering inconsistency, but the forward declaration is more surprising than a slightly out-of-order definition. Either approach is acceptable; just noting the trade-off.

**3. Mode reuse pattern.** Several panels share display modes:
- `waveshare_70_lcd_c_panel` reuses `waveshare_50_lcd_c_mode` (both 1024x600)
- `waveshare_70_lcd_e_panel` reuses `waveshare_80_lcd_c_mode` (both 1280x800)
- `waveshare_101_lcd_c_panel` reuses `waveshare_80_lcd_c_mode` (both 1280x800)

This is fine and follows existing practice in panel-simple.c where panels with identical timings share mode structs.

**4. Clock values.** The `waveshare_34_lcd_c_mode` has `.clock = 50000` for an 800x800 panel with htotal=958, vtotal=828. That gives a refresh rate of 50000000/(958*828) = ~63 Hz, which seems reasonable. The `.clock = 83333` entries (e.g., for 720x1280 @ 83333000/(1000*1340) = ~62 Hz) also check out. The timings appear plausible.

**5. Pixel format note.** The commit message honestly states: "since the panels are hidden behind the bridges which are not being programmed by the kernel, I could not confirm the pixel format for the panels." This is worth keeping in mind -- the LVDS panels all use `MEDIA_BUS_FMT_RGB888_1X7X4_SPWG` (JEIDA mapping), which is a reasonable default but may need correction if specific panels actually use VESA mapping (`MEDIA_BUS_FMT_RGB888_1X7X4_JEIDA` -- though SPWG and JEIDA are effectively the same thing in this context). Since the bridge handles the conversion, this shouldn't matter in practice.

**Summary for patch 4:** Add trailing commas to all 15 OF table entries. Consider eliminating the forward declaration by reordering definitions.

---
Generated by Claude Code Patch Reviewer