[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f8ffc231-6961-44f8-ad61-3a07ab22e849@kernel.org>
Date: Fri, 18 Oct 2024 13:41:41 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: 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
On 18/10/2024 13:18, Charles Wang wrote:
> 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
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
Powered by blists - more mailing lists