* [PATCH 0/2] ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver
@ 2026-03-11 15:54 Bartholomäus Steinmayr
2026-03-12 8:30 ` Michael Walle
2026-03-13 4:42 ` Claude review: " Claude Code Review Bot
0 siblings, 2 replies; 3+ messages in thread
From: Bartholomäus Steinmayr @ 2026-03-11 15:54 UTC (permalink / raw)
To: mwalle@kernel.org, dri-devel@lists.freedesktop.org
Hi,
this patch series adds support for the Densitron DMT050WVNMCMI-1A panel
to the ILI9806E DSI driver. The patch mainly contains initialization
code for the panel.
However, the display also has a peculiarity which required some more
changes to the driver. The display contains a Goodix GT911 touchpanel
controller. The GT911 and ILI9806E share a single reset line. The i2c
address of the GT911 chip is set by manipulating an IO line during
reset. This is already handled by the existing GT911 driver, but it
means that the reset line MUST be controlled by the Goodix driver
(drivers/input/touchscreen/goodix.c). The ILI9806E should defer its
probing until the Goodix driver has completed its reset. The ILI9806E
should then probe with asserting the reset line.
To achieve this, this patch adds an optional dt node "i2c-frag" to the
ILI9806E driver. If this node exists, the Ilitek driver defers its
probing until the i2c node has been initialized. Furthermore, the
reset-gpios property has been made optional. To keep the Ilitek driver
from asserting the reset line, the reset-gpios property should be left
out for the DMT050 display.
This solution does not seem particularly elegant, but I could not find a
more straight-forward one. This is also my first kernel patch, so I
appreciate your patience.
Thank you.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH 0/2] ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver
2026-03-11 15:54 [PATCH 0/2] ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver Bartholomäus Steinmayr
@ 2026-03-12 8:30 ` Michael Walle
2026-03-13 4:42 ` Claude review: " Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Michael Walle @ 2026-03-12 8:30 UTC (permalink / raw)
To: Bartholomäus Steinmayr, dri-devel@lists.freedesktop.org
Cc: Saravana Kannan
[-- Attachment #1: Type: text/plain, Size: 2645 bytes --]
Hi,
[+ Saravana]
On Wed Mar 11, 2026 at 4:54 PM CET, Bartholomäus Steinmayr wrote:
> this patch series adds support for the Densitron DMT050WVNMCMI-1A panel
> to the ILI9806E DSI driver. The patch mainly contains initialization
> code for the panel.
>
> However, the display also has a peculiarity which required some more
> changes to the driver. The display contains a Goodix GT911 touchpanel
> controller. The GT911 and ILI9806E share a single reset line. The i2c
> address of the GT911 chip is set by manipulating an IO line during
> reset. This is already handled by the existing GT911 driver, but it
> means that the reset line MUST be controlled by the Goodix driver
> (drivers/input/touchscreen/goodix.c). The ILI9806E should defer its
> probing until the Goodix driver has completed its reset. The ILI9806E
> should then probe with asserting the reset line.
I see. This explanation should definitely go into the commit message
of the second patch.
So basically, you 'just' need to have the driver for the touch panel
probed first. I'm not really sure you can just move the reset line
to the touch controller or if it has to be shared between both.
Also, what happens if the dsi panel is turned off, which right now,
would assert the reset line.
I think device-links is what you need ([1], maybe
Documentation/driver-api/device_link.rst). IIRC they get added
automatically by parsing the device tree. So you might just need a
phandle pointint to the goodix touchscreen driver, to have it probed
first. But there is probably more you'd need to handle the "turn the
DSI display off and on again" case.
I'd start by adding a phandle called "touch-controller" to the panel
node (similar to your i2c-frag I think).
> To achieve this, this patch adds an optional dt node "i2c-frag" to the
> ILI9806E driver. If this node exists, the Ilitek driver defers its
> probing until the i2c node has been initialized. Furthermore, the
> reset-gpios property has been made optional. To keep the Ilitek driver
> from asserting the reset line, the reset-gpios property should be left
> out for the DMT050 display.
Any devicetree bindings (i2c-frag, optional reset) have to be
documented in the corresponding binding, i.e. see your first patch.
> This solution does not seem particularly elegant, but I could not find a
> more straight-forward one. This is also my first kernel patch, so I
> appreciate your patience.
Thanks for joining the linux kernel community and don't worry.
-michael
[1] https://lore.kernel.org/all/20190731221721.187713-1-saravanak@google.com/
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 297 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Claude review: ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver
2026-03-11 15:54 [PATCH 0/2] ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver Bartholomäus Steinmayr
2026-03-12 8:30 ` Michael Walle
@ 2026-03-13 4:42 ` Claude Code Review Bot
1 sibling, 0 replies; 3+ messages in thread
From: Claude Code Review Bot @ 2026-03-13 4:42 UTC (permalink / raw)
To: dri-devel-reviews
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-03-13 4:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-11 15:54 [PATCH 0/2] ili9806e: Add Densitron DMT050WVNMCMI-1A to ILI9806E DSI driver Bartholomäus Steinmayr
2026-03-12 8:30 ` Michael Walle
2026-03-13 4:42 ` Claude review: " Claude Code Review Bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox