Claude review: drm/panel: Add panel driver for ChipWealth CH13726A based panels

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

Patch Review

Several issues:

1. **Unnecessary and incorrect Kconfig selects.** The Kconfig entry selects `DRM_DISPLAY_DP_HELPER` and `DRM_DISPLAY_HELPER`:

```
+	select DRM_DISPLAY_DP_HELPER
+	select DRM_DISPLAY_HELPER
```

This is a MIPI-DSI panel, not a DisplayPort panel. The driver doesn't use any DP helpers. These selects should be removed entirely — no replacement is needed since `drm_mipi_dsi.h` and `drm_panel.h` don't depend on them.

2. **`prepared` flag is deprecated.** The driver manually tracks `prepared` state:

```c
+	bool prepared;
```

```c
+	if (ctx->prepared)
+		return 0;
```

The DRM panel framework has handled prepare/unprepare idempotency since commit work in recent kernels. Only 2 legacy drivers in the tree still use this pattern. Remove the `prepared` field and the guard checks in `ch13726a_prepare()` and `ch13726a_unprepare()`.

3. **`of_device_get_match_data()` cast drops const.** The probe function casts away const:

```c
+	ctx->desc = (struct ch13726a_desc *)of_device_get_match_data(dev);
```

The `desc` field in the struct should be `const struct ch13726a_desc *desc` and the cast removed. Similarly, `thor_bottom_desc` should be `static const`:

```c
+static struct ch13726a_desc thor_bottom_desc = {
```

Should be `static const struct ch13726a_desc thor_bottom_desc`.

4. **`uint8_t` is not kernel style.** The kernel uses `u8` not `uint8_t`:

```c
+	for (uint8_t i = 0; i < ctx->desc->num_modes; i++) {
```

Should use `unsigned int i` (since `num_modes` is `unsigned int` and the value is small, there's no reason to use `u8` here anyway).

5. **Reset GPIO polarity may be inverted.** The reset sequence does high-low-high:

```c
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
+	usleep_range(10000, 11000);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
+	usleep_range(10000, 11000);
+	gpiod_set_value_cansleep(ctx->reset_gpio, 1);
```

The GPIO is obtained with `GPIOD_OUT_LOW` (meaning initially deasserted). Typically a reset is assert-deassert (1-0-1 asserts then deasserts), but the dt-binding example uses `GPIO_ACTIVE_HIGH`, which means `gpiod_set_value(1)` drives the pin high. This is plausible for an active-high reset, but worth confirming the hardware — most AMOLED panels use active-low reset where the sequence would be 0-1-0 (assert low, release high). If the GPIO is active-high reset, then the unprepare path sets reset to 0 (deasserted/released), which means the panel leaves reset deasserted during power-down — that's backwards. Usually you want to assert reset *before* cutting power.

6. **`ch13726a_disable()` clears `MIPI_DSI_MODE_LPM` but `ch13726a_on()` sets it.** In `ch13726a_disable()`:

```c
+	ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
```

This means the display-off and sleep-enter commands are sent in HS mode, but the init sequence sends commands in LP mode. This is fine if intentional, but the backlight handler also toggles LPM:

```c
+	dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+	ret = mipi_dsi_dcs_set_display_brightness(dsi, brightness);
+	...
+	dsi->mode_flags |= MIPI_DSI_MODE_LPM;
```

Toggling `mode_flags` like this is racy if backlight updates can happen concurrently with enable/disable. Consider setting the mode flags once in probe and not toggling them per-operation.

7. **Missing `#include <linux/of.h>` guard.** The driver includes `<linux/of.h>` but could use `<linux/mod_devicetable.h>` instead for `of_device_id`, matching newer driver conventions (see `panel-boe-td4320.c`). Minor style point.

8. **`ch13726a_remove()` error check on detach is unnecessary boilerplate.** `mipi_dsi_detach()` return value logging in remove is acceptable but some maintainers prefer to just ignore it since there's nothing useful to do with the error in the remove path.

---
Generated by Claude Code Patch Reviewer