[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZxJCvwxwamvRZ3m9@ux-UP-WHL01>
Date: Fri, 18 Oct 2024 19:18:05 +0800
From: Charles Wang <charles.goodix@...il.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
Charles Wang <charles.goodix@...il.com>, dmitry.torokhov@...il.com,
hbarnor@...omium.org, dianders@...omium.org,
conor.dooley@...rochip.com
Cc: jikos@...nel.org, bentiss@...nel.org, linux-input@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] dt-bindings: input: Goodix SPI HID Touchscreen
Hi Krzysztof,
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@...il.com>
> > ---
> > .../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@...omium.org>
> > + - Charles Wang <charles.goodix@...il.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,
> > - 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.
>
> > +
> > + 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
the GT7375P does not support the properties 'goodix,hid-report-addr'
and 'spi-max-frequency. Is there any issue with writing it this way?
> > + reg:
> > + enum: [0x5d, 0x14]
> > +
> > additionalProperties: false
>
> This becomes now: unevaluatedProperties: false
>
Ack,
> >
> > examples:
> > @@ -87,3 +115,23 @@ examples:
> > vdd-supply = <&pp3300_ts>;
> > };
> > };
> > +
>
Best regards,
Charles
Powered by blists - more mailing lists