Hi Krzysztof, On Fri, Oct 18, 2024 at 01:41:41PM +0200, Krzysztof Kozlowski wrote: > On 18/10/2024 13:18, Charles Wang wrote: > > > > On Fri, Oct 18, 2024 at 07:59:46AM +0200, Krzysztof Kozlowski wrote: > >> On 18/10/2024 04:08, Charles Wang wrote: > >>> The Goodix GT7986U touch controller report touch data according to the > >>> HID protocol through the SPI bus. However, it is incompatible with > >>> Microsoft's HID-over-SPI protocol. > >>> > >>> Signed-off-by: Charles Wang <charles.goodix@xxxxxxxxx> > >>> --- > >>> .../bindings/input/goodix,gt7375p.yaml | 68 ++++++++++++++++--- > >>> 1 file changed, 58 insertions(+), 10 deletions(-) > >>> > >>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml > >>> index 358cb8275..184d9c320 100644 > >>> --- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml > >>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml > >>> @@ -8,27 +8,27 @@ title: Goodix GT7375P touchscreen > >>> > >>> maintainers: > >>> - Douglas Anderson <dianders@xxxxxxxxxxxx> > >>> + - Charles Wang <charles.goodix@xxxxxxxxx> > >>> > >>> description: > >>> - Supports the Goodix GT7375P touchscreen. > >>> - This touchscreen uses the i2c-hid protocol but has some non-standard > >>> - power sequencing required. > >>> - > >>> -allOf: > >>> - - $ref: /schemas/input/touchscreen/touchscreen.yaml# > >>> + The Goodix GT7375P and GT7986U touchscreens support both SPI and I2C interfaces. > >>> + With the I2C interface, they use the i2c-hid protocol but require non-standard > >>> + power sequencing. With the SPI interface, they use a custom HID protocol that > >>> + is incompatible with Microsoft's HID-over-SPI protocol. > >>> > >>> properties: > >>> compatible: > >>> oneOf: > >>> - - const: goodix,gt7375p > >>> + - items: > >>> + - const: goodix,gt7375p > >> > >> That's not a necessary change. Keep old code here. > >> > > > > Ack, > > > >>> - items: > >>> - const: goodix,gt7986u > >>> - const: goodix,gt7375p > >>> + - items: > >>> + - const: goodix,gt7986u > >> > >> Hm? This does not make much sense. Device either is or is not compatible > >> with gt7375p. Cannot be both. > >> > > > > Ack, > > > >>> > >>> reg: > >>> - enum: > >>> - - 0x5d > >>> - - 0x14 > >>> + maxItems: 1 > >>> > >>> interrupts: > >>> maxItems: 1 > >>> @@ -57,6 +57,15 @@ properties: > >>> This property is used to avoid the back-powering issue. > >>> type: boolean > >>> > >>> + goodix,hid-report-addr: > >>> + $ref: /schemas/types.yaml#/definitions/uint32 > >>> + description: > >>> + The register address for retrieving HID report data. > >>> + This address is related to the device firmware and may > >>> + change after a firmware update. > >> > How is this supposed to work? DTS will stay fixed, you cannot change it > just because firmware changed. User loads new firmware with different > address, but DTS will have to use old address - so broken property. > Sorry for missing this issue in my previous response. Honestly, although the likelihood of this address changing is low, it is indeed possible for it to change due to a firmware update during the factory debugging phase. However, for machines that users have, we will ensure that this address will not be altered as a result of a firmware upgrade. > >>> + > >>> + spi-max-frequency: true > >> > >> Drop > >> > > > > Ack, > > > >>> + > >>> required: > >>> - compatible > >>> - reg > >>> @@ -64,6 +73,25 @@ required: > >>> - reset-gpios > >>> - vdd-supply > >>> > >>> +allOf: > >>> + - $ref: /schemas/input/touchscreen/touchscreen.yaml# > >>> + - $ref: /schemas/spi/spi-peripheral-props.yaml# > >>> + > >>> + - if: > >>> + properties: > >>> + compatible: > >>> + items: > >>> + - const: goodix,gt7986u > >>> + then: > >>> + required: > >>> + - goodix,hid-report-addr > >>> + else: > >>> + properties: > >>> + goodix,hid-report-addr: false > >>> + spi-max-frequency: false > >> > >> Why? GT7375P also supports SPI. > >> > > > > No, only GT7986U support SPI. What I'm trying to express here is that > > Description earlier said: > "The Goodix GT7375P and GT7986U touchscreens support both SPI and I2C > interfaces." > > so both support? > Sorry, there is an error in the description. Currently, only the GT7986U supports SPI, I will change the description. > > > the GT7375P does not support the properties 'goodix,hid-report-addr' > > and 'spi-max-frequency. Is there any issue with writing it this way? > > spi-max-frequency could stay, assuming device does not support SPI. > Ack, Best regards, Charles