On 17/10/2023 15:59, Conor Dooley wrote: > Yo, > > On Tue, Oct 17, 2023 at 05:18:57PM +0800, Tylor Yang wrote: >> The Himax HID-over-SPI framework support for Himax touchscreen ICs >> that report HID packet through SPI bus. The driver core need reset >> pin to meet reset timing spec. of IC. An interrupt to disable >> and enable interrupt when suspend/resume. Two optional power control >> if target board needed. >> >> Signed-off-by: Tylor Yang <tylor_yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx> >> --- >> .../devicetree/bindings/input/himax,hid.yaml | 123 ++++++++++++++++++ >> MAINTAINERS | 6 + >> 2 files changed, 129 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/input/himax,hid.yaml >> >> diff --git a/Documentation/devicetree/bindings/input/himax,hid.yaml b/Documentation/devicetree/bindings/input/himax,hid.yaml >> new file mode 100644 >> index 000000000000..9ba86fe1b7da >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/input/himax,hid.yaml >> @@ -0,0 +1,123 @@ >> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause >> +%YAML 1.2 >> +--- >> +$id: http://devicetree.org/schemas/input/himax,hid.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: Himax TDDI devices using SPI to send HID packets >> + >> +maintainers: >> + - Tylor Yang <tylor_yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx> >> + >> +description: | >> + Support the Himax TDDI devices which using SPI interface to acquire >> + HID packets from the device. The device needs to be initialized using >> + Himax protocol before it start sending HID packets. >> + >> +properties: >> + compatible: >> + const: himax,hid > > This compatible seems far too generic. Why are there not device specific > compatibles for each TDDI device? Which was pointed out by Rob in v2, so his feedback was ignored. > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + reset: >> + maxItems: 1 >> + description: Reset device, active low signal. No, come one, read feedback from Rob. >> + >> + vccd-supply: >> + description: >> + Optional regulator for the 1.8V voltage. >> + >> + vcca-supply: >> + description: >> + Optional regulator for the analog 3.3V voltage. >> + >> + himax,id-gpios: >> + maxItems: 8 >> + description: GPIOs to read physical Panel ID. Optional. >> + >> + spi-cpha: true >> + spi-cpol: true > >> + himax,ic-det-delay-ms: >> + description: >> + Due to TDDI properties, the TPIC detection timing must after the >> + display panel initialized. This property is used to specify the >> + delay time when TPIC detection and display panel initialization >> + timing are overlapped. How much milliseconds to delay before TPIC >> + detection start. >> + >> + himax,ic-resume-delay-ms: >> + description: >> + Due to TDDI properties, the TPIC resume timing must after the >> + display panel resumed. This property is used to specify the >> + delay time when TPIC resume and display panel resume >> + timing are overlapped. How much milliseconds to delay before TPIC >> + resume start. > No improvements here... You must implement all feedback from v2. Not pieces of it. Best regards, Krzysztof