On 18/10/2024 13:18, Charles Wang wrote:

Hi Krzysztof,

On Fri, Oct 18, 2024 at 07:59:46AM +0200, Krzysztof Kozlowski wrote:

quoted

On 18/10/2024 04:08, Charles Wang wrote:

quoted

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 <redacted>
---
 .../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@chromium.org>
+  - Charles Wang <charles.goodix@gmail.com>
 
 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,

quoted

quoted

       - 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,

quoted

quoted

 
   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.

quoted

+
+  spi-max-frequency: true

Drop

Ack,

quoted

quoted

+
 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?

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.

Best regards,
Krzysztof