Claude review: ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver

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

Overall Series Review

Subject: ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver
Author: =?iso-8859-1?Q?Bartholom=E4us_Steinmayr?= <BSteinmayr@thorlabs.com>
Patches: 2
Reviewed: 2026-03-13T14:42:09.867822

---

**Incomplete submission — only the cover letter (0/2) is present in the mbox. Patches 1/2 and 2/2 are missing**, so no code changes can be reviewed directly. The review below is based on the design described in the cover letter and the existing driver code.

The series aims to add support for the Densitron DMT050WVNMCMI-1A panel to the ILI9806E DSI driver. The panel initialization code part is standard and should be straightforward. However, the proposed mechanism for handling the shared reset line with the Goodix GT911 touchscreen controller raises significant design concerns.

**Key design concerns from the cover letter:**

1. **Invented `i2c-frag` DT property**: The cover letter describes adding an optional `i2c-frag` devicetree node to the ILI9806E driver that causes it to defer probing until an I2C device (the Goodix touchscreen) has initialized. This is a non-standard, ad-hoc probe-ordering mechanism. The DT bindings maintainers will very likely reject this — devicetree should describe hardware, not encode driver probe ordering. The proper kernel mechanisms for this are:
   - **`gpio-hog`** or shared GPIO with proper flags
   - Using the **component framework** or **device links** (`device_link_add`)
   - Expressing the dependency through standard DT properties like `power-supply` phandles or similar that naturally create deferred probe chains
   - The reset controller subsystem (`reset-controller`) which is specifically designed for shared reset lines

2. **Making `reset-gpios` optional**: The cover letter says the reset GPIO is made optional so the ILI9806E driver won't touch the shared reset line. This changes existing behavior for all panels using this driver — currently `reset-gpios` is required (line 184: `devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW)`). Making it optional without breaking existing panels needs careful handling, but more fundamentally, a panel driver that can't control its own reset is fragile.

3. **Better alternatives exist**: The shared reset line between the GT911 and ILI9806E should ideally be modeled using the **reset controller subsystem** (`drivers/reset/`), where a simple GPIO-based reset controller provides the reset to both consumers. This is exactly the pattern the kernel has for shared reset lines. Alternatively, the GT911 driver already handles the reset+I2C-address-selection dance — the panel driver could simply declare the reset GPIO as `GPIOD_ASIS` (not asserting it) if it knows another driver will handle it.

---
Generated by Claude Code Patch Reviewer