[v8] HID: add initial support for Goodix HID-over-SPI touchscreen

[PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Charles Wang 2 months ago

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.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Charles Wang <charles.goodix@gmail.com>
---
 .../bindings/input/goodix,gt7986u.yaml        | 71 +++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml

diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
new file mode 100644
index 000000000..849117639
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
@@ -0,0 +1,71 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GOODIX GT7986U SPI HID Touchscreen
+
+maintainers:
+  - Charles Wang <charles.goodix@gmail.com>
+
+description: Supports the Goodix GT7986U touchscreen.
+  This touch controller reports data packaged according to the HID protocol,
+  but is incompatible with Microsoft's HID-over-SPI protocol.
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - goodix,gt7986u-spi
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  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.
+
+  spi-max-frequency: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - reset-gpios
+  - goodix,hid-report-addr
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    spi {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touchscreen@0 {
+        compatible = "goodix,gt7986u-spi";
+        reg = <0>;
+        interrupt-parent = <&gpio>;
+        interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+        reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+        spi-max-frequency = <10000000>;
+        goodix,hid-report-addr = <0x22c8c>;
+      };
+    };
+
+...
-- 
2.43.0

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Conor Dooley 2 months ago

On Thu, Sep 26, 2024 at 12:42:17PM +0800, 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.
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Charles Wang <charles.goodix@gmail.com>

Please remove my reviewed by, I didn't review this new "spi"
compatible.

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Krzysztof Kozlowski 2 months ago

On 26/09/2024 06:42, 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.
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Charles Wang <charles.goodix@gmail.com>

Why do you send the same? The patches were applied three weeks ago! You
were supposed to respond that time by fixing the issue with incremental
patch.

Now, work on top of next branch, bring back removed OF support and add
missing properties to existing binding.


> ---
>  .../bindings/input/goodix,gt7986u.yaml        | 71 +++++++++++++++++++
>  1 file changed, 71 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> new file mode 100644
> index 000000000..849117639
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> @@ -0,0 +1,71 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GOODIX GT7986U SPI HID Touchscreen
> +
> +maintainers:
> +  - Charles Wang <charles.goodix@gmail.com>
> +
> +description: Supports the Goodix GT7986U touchscreen.
> +  This touch controller reports data packaged according to the HID protocol,
> +  but is incompatible with Microsoft's HID-over-SPI protocol.
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - goodix,gt7986u-spi

NAK, you duplicate again the binding. You cannot have bus-flavors.
Device is the same.


Best regards,
Krzysztof

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Charles Wang 2 months ago

Hi Krzysztof,

On Thu, Sep 26, 2024 at 08:29:38AM +0200, Krzysztof Kozlowski wrote:
> On 26/09/2024 06:42, 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.
> > 
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > Signed-off-by: Charles Wang <charles.goodix@gmail.com>
> 
> Why do you send the same? The patches were applied three weeks ago! You
> were supposed to respond that time by fixing the issue with incremental
> patch.
> 
> Now, work on top of next branch, bring back removed OF support and add
> missing properties to existing binding.

Sorry again for this.

> 
> > ---
> >  .../bindings/input/goodix,gt7986u.yaml        | 71 +++++++++++++++++++
> >  1 file changed, 71 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> > new file mode 100644
> > index 000000000..849117639
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> > @@ -0,0 +1,71 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GOODIX GT7986U SPI HID Touchscreen
> > +
> > +maintainers:
> > +  - Charles Wang <charles.goodix@gmail.com>
> > +
> > +description: Supports the Goodix GT7986U touchscreen.
> > +  This touch controller reports data packaged according to the HID protocol,
> > +  but is incompatible with Microsoft's HID-over-SPI protocol.
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - goodix,gt7986u-spi
> 
> NAK, you duplicate again the binding. You cannot have bus-flavors.
> Device is the same.
>

Could you provide some suggestions regarding this issue?

Regards!
Charles

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Krzysztof Kozlowski 2 months ago

On 26/09/2024 11:57, Charles Wang wrote:
>>>  1 file changed, 71 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>> new file mode 100644
>>> index 000000000..849117639
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>> @@ -0,0 +1,71 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: GOODIX GT7986U SPI HID Touchscreen
>>> +
>>> +maintainers:
>>> +  - Charles Wang <charles.goodix@gmail.com>
>>> +
>>> +description: Supports the Goodix GT7986U touchscreen.
>>> +  This touch controller reports data packaged according to the HID protocol,
>>> +  but is incompatible with Microsoft's HID-over-SPI protocol.
>>> +
>>> +allOf:
>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - goodix,gt7986u-spi
>>
>> NAK, you duplicate again the binding. You cannot have bus-flavors.
>> Device is the same.
>>
> 
> Could you provide some suggestions regarding this issue?

What is exactly the question or problem? There is a binding for this
device. Extend it with SPI parts, e.g.
https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml#L22

Best regards,
Krzysztof

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Charles Wang 1 month, 4 weeks ago

Hi Krzysztof,
Thank you very much for your advice.

On Thu, Sep 26, 2024 at 02:32:05PM +0200, Krzysztof Kozlowski wrote:
> On 26/09/2024 11:57, Charles Wang wrote:
> >>>  1 file changed, 71 insertions(+)
> >>>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>
> >>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>> new file mode 100644
> >>> index 000000000..849117639
> >>> --- /dev/null
> >>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>> @@ -0,0 +1,71 @@
> >>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>> +%YAML 1.2
> >>> +---
> >>> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> >>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>> +
> >>> +title: GOODIX GT7986U SPI HID Touchscreen
> >>> +
> >>> +maintainers:
> >>> +  - Charles Wang <charles.goodix@gmail.com>
> >>> +
> >>> +description: Supports the Goodix GT7986U touchscreen.
> >>> +  This touch controller reports data packaged according to the HID protocol,
> >>> +  but is incompatible with Microsoft's HID-over-SPI protocol.
> >>> +
> >>> +allOf:
> >>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >>> +
> >>> +properties:
> >>> +  compatible:
> >>> +    enum:
> >>> +      - goodix,gt7986u-spi
> >>
> >> NAK, you duplicate again the binding. You cannot have bus-flavors.
> >> Device is the same.
> >>
> > 
> > Could you provide some suggestions regarding this issue?
> 
> What is exactly the question or problem? There is a binding for this
> device. Extend it with SPI parts, e.g.
> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml#L22
> 

This seems a little different from the adxl313.yaml.

The issue we're encountering involves the chip model gt7986u,
which supports both I2C and SPI interfaces. For the I2C interface
(using the HID-over-I2C driver), it has already been declared in
the goodix,gt7375p.yaml file as follows:

i2c {
  #address-cells = <1>;
  #size-cells = <0>;

  ap_ts: touchscreen@5d {
    compatible = "goodix,gt7986u";
  }
}

Currently, our design requires utilizing the SPI interface with
a custom SPI driver. However, the declarations within the binding
file have led to conflicts, as shown here:

spi {
  #address-cells = <1>;
  #size-cells = <0>;

  touchscreen@0 {
    compatible = "goodix,gt7986u";
  }
}

Should I consider merging both YAML files into a single one to fix this?

Best regards!
Charles

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Krzysztof Kozlowski 1 month, 4 weeks ago

On 30/09/2024 05:22, Charles Wang wrote:
> Hi Krzysztof,
> Thank you very much for your advice.
> 
> On Thu, Sep 26, 2024 at 02:32:05PM +0200, Krzysztof Kozlowski wrote:
>> On 26/09/2024 11:57, Charles Wang wrote:
>>>>>  1 file changed, 71 insertions(+)
>>>>>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>>> new file mode 100644
>>>>> index 000000000..849117639
>>>>> --- /dev/null
>>>>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>>> @@ -0,0 +1,71 @@
>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +%YAML 1.2
>>>>> +---
>>>>> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>> +
>>>>> +title: GOODIX GT7986U SPI HID Touchscreen
>>>>> +
>>>>> +maintainers:
>>>>> +  - Charles Wang <charles.goodix@gmail.com>
>>>>> +
>>>>> +description: Supports the Goodix GT7986U touchscreen.
>>>>> +  This touch controller reports data packaged according to the HID protocol,
>>>>> +  but is incompatible with Microsoft's HID-over-SPI protocol.
>>>>> +
>>>>> +allOf:
>>>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    enum:
>>>>> +      - goodix,gt7986u-spi
>>>>
>>>> NAK, you duplicate again the binding. You cannot have bus-flavors.
>>>> Device is the same.
>>>>
>>>
>>> Could you provide some suggestions regarding this issue?
>>
>> What is exactly the question or problem? There is a binding for this
>> device. Extend it with SPI parts, e.g.
>> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml#L22
>>
> 
> This seems a little different from the adxl313.yaml.

Hm? I am reading below:

> 
> The issue we're encountering involves the chip model gt7986u,
> which supports both I2C and SPI interfaces. For the I2C interface
> (using the HID-over-I2C driver), it has already been declared in
> the goodix,gt7375p.yaml file as follows:
> 
> i2c {
>   #address-cells = <1>;
>   #size-cells = <0>;
> 
>   ap_ts: touchscreen@5d {
>     compatible = "goodix,gt7986u";
>   }
> }
> 
> Currently, our design requires utilizing the SPI interface with
> a custom SPI driver. However, the declarations within the binding
> file have led to conflicts, as shown here:
> 
> spi {
>   #address-cells = <1>;
>   #size-cells = <0>;
> 
>   touchscreen@0 {
>     compatible = "goodix,gt7986u";
>   }
> }
> 
> Should I consider merging both YAML files into a single one to fix this?

And there is no difference. I don't understand the problem.

Best regards,
Krzysztof

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Charles Wang 1 month, 4 weeks ago

Hi Krzysztof,

On Mon, Sep 30, 2024 at 08:42:22AM +0200, Krzysztof Kozlowski wrote:
> On 30/09/2024 05:22, Charles Wang wrote:
> > Hi Krzysztof,
> > Thank you very much for your advice.
> > 
> > On Thu, Sep 26, 2024 at 02:32:05PM +0200, Krzysztof Kozlowski wrote:
> >> On 26/09/2024 11:57, Charles Wang wrote:
> >>>>>  1 file changed, 71 insertions(+)
> >>>>>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>>>
> >>>>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>>> new file mode 100644
> >>>>> index 000000000..849117639
> >>>>> --- /dev/null
> >>>>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>>> @@ -0,0 +1,71 @@
> >>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>> +%YAML 1.2
> >>>>> +---
> >>>>> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> >>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>> +
> >>>>> +title: GOODIX GT7986U SPI HID Touchscreen
> >>>>> +
> >>>>> +maintainers:
> >>>>> +  - Charles Wang <charles.goodix@gmail.com>
> >>>>> +
> >>>>> +description: Supports the Goodix GT7986U touchscreen.
> >>>>> +  This touch controller reports data packaged according to the HID protocol,
> >>>>> +  but is incompatible with Microsoft's HID-over-SPI protocol.
> >>>>> +
> >>>>> +allOf:
> >>>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >>>>> +
> >>>>> +properties:
> >>>>> +  compatible:
> >>>>> +    enum:
> >>>>> +      - goodix,gt7986u-spi
> >>>>
> >>>> NAK, you duplicate again the binding. You cannot have bus-flavors.
> >>>> Device is the same.
> >>>>
> >>>
> >>> Could you provide some suggestions regarding this issue?
> >>
> >> What is exactly the question or problem? There is a binding for this
> >> device. Extend it with SPI parts, e.g.
> >> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml#L22
> >>
> > 
> > This seems a little different from the adxl313.yaml.
> 
> Hm? I am reading below:
> 
> > 
> > The issue we're encountering involves the chip model gt7986u,
> > which supports both I2C and SPI interfaces. For the I2C interface
> > (using the HID-over-I2C driver), it has already been declared in
> > the goodix,gt7375p.yaml file as follows:
> > 
> > i2c {
> >   #address-cells = <1>;
> >   #size-cells = <0>;
> > 
> >   ap_ts: touchscreen@5d {
> >     compatible = "goodix,gt7986u";
> >   }
> > }
> > 
> > Currently, our design requires utilizing the SPI interface with
> > a custom SPI driver. However, the declarations within the binding
> > file have led to conflicts, as shown here:
> > 
> > spi {
> >   #address-cells = <1>;
> >   #size-cells = <0>;
> > 
> >   touchscreen@0 {
> >     compatible = "goodix,gt7986u";
> >   }
> > }
> > 
> > Should I consider merging both YAML files into a single one to fix this?
> 
> And there is no difference. I don't understand the problem.
> 
I'm sorry for the confusion regarding your comment

"And there is no difference." 

Are you implying that the issue we are encountering is no different from
'adi,adxl313.yaml'? Or are you suggesting that the gt7986u device should
be treated as the same entity for both I2C and SPI interfaces?

Original error messages: https://lore.kernel.org/all/CAL_Jsq+QfTtRj_JCqXzktQ49H8VUnztVuaBjvvkg3fwEHniUHw@mail.gmail.com/

+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+  compatible:
+    enum:
+      - goodix,gt7986u

This is already documented in goodix,gt7375p.yaml. Now linux-next has warnings:

/builds/robherring/linux-dt/Documentation/devicetree/bindings/input/goodix,gt7986u.example.dtb:
touchscreen@0: compatible: 'oneOf' conditional failed, one must be
fixed:
        ['goodix,gt7986u'] is too short
        'goodix,gt7375p' was expected
        from schema $id:

I understand this error message to mean the same chip model is redundantly declared
in two separate files.

Is my understanding incorrect? Could you provide more explicit guidance?

Note that the 'gt7986u' uses different drivers and has distinct device property
for its I2C and SPI interfaces.

Best regards,
Charles

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Krzysztof Kozlowski 1 month, 4 weeks ago

On 30/09/2024 09:56, Charles Wang wrote:
> Hi Krzysztof,
> 
> On Mon, Sep 30, 2024 at 08:42:22AM +0200, Krzysztof Kozlowski wrote:
>> On 30/09/2024 05:22, Charles Wang wrote:
>>> Hi Krzysztof,
>>> Thank you very much for your advice.
>>>
>>> On Thu, Sep 26, 2024 at 02:32:05PM +0200, Krzysztof Kozlowski wrote:
>>>> On 26/09/2024 11:57, Charles Wang wrote:
>>>>>>>  1 file changed, 71 insertions(+)
>>>>>>>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>>>>>
>>>>>>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>>>>> new file mode 100644
>>>>>>> index 000000000..849117639
>>>>>>> --- /dev/null
>>>>>>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
>>>>>>> @@ -0,0 +1,71 @@
>>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>>>> +%YAML 1.2
>>>>>>> +---
>>>>>>> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
>>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>>>>> +
>>>>>>> +title: GOODIX GT7986U SPI HID Touchscreen
>>>>>>> +
>>>>>>> +maintainers:
>>>>>>> +  - Charles Wang <charles.goodix@gmail.com>
>>>>>>> +
>>>>>>> +description: Supports the Goodix GT7986U touchscreen.
>>>>>>> +  This touch controller reports data packaged according to the HID protocol,
>>>>>>> +  but is incompatible with Microsoft's HID-over-SPI protocol.
>>>>>>> +
>>>>>>> +allOf:
>>>>>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
>>>>>>> +
>>>>>>> +properties:
>>>>>>> +  compatible:
>>>>>>> +    enum:
>>>>>>> +      - goodix,gt7986u-spi
>>>>>>
>>>>>> NAK, you duplicate again the binding. You cannot have bus-flavors.
>>>>>> Device is the same.
>>>>>>
>>>>>
>>>>> Could you provide some suggestions regarding this issue?
>>>>
>>>> What is exactly the question or problem? There is a binding for this
>>>> device. Extend it with SPI parts, e.g.
>>>> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml#L22
>>>>
>>>
>>> This seems a little different from the adxl313.yaml.
>>
>> Hm? I am reading below:
>>
>>>
>>> The issue we're encountering involves the chip model gt7986u,
>>> which supports both I2C and SPI interfaces. For the I2C interface
>>> (using the HID-over-I2C driver), it has already been declared in
>>> the goodix,gt7375p.yaml file as follows:
>>>
>>> i2c {
>>>   #address-cells = <1>;
>>>   #size-cells = <0>;
>>>
>>>   ap_ts: touchscreen@5d {
>>>     compatible = "goodix,gt7986u";
>>>   }
>>> }
>>>
>>> Currently, our design requires utilizing the SPI interface with
>>> a custom SPI driver. However, the declarations within the binding
>>> file have led to conflicts, as shown here:
>>>
>>> spi {
>>>   #address-cells = <1>;
>>>   #size-cells = <0>;
>>>
>>>   touchscreen@0 {
>>>     compatible = "goodix,gt7986u";
>>>   }
>>> }
>>>
>>> Should I consider merging both YAML files into a single one to fix this?
>>
>> And there is no difference. I don't understand the problem.
>>
> I'm sorry for the confusion regarding your comment
> 
> "And there is no difference." 
> 
> Are you implying that the issue we are encountering is no different from

I don't understand the issue.

> 'adi,adxl313.yaml'? Or are you suggesting that the gt7986u device should
> be treated as the same entity for both I2C and SPI interfaces?

I told you what to do - extend existing binding. I gave you example how
one binding is for both I2C and SPI devices.

> 
> Original error messages: https://lore.kernel.org/all/CAL_Jsq+QfTtRj_JCqXzktQ49H8VUnztVuaBjvvkg3fwEHniUHw@mail.gmail.com/
> 
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - goodix,gt7986u
> 
> This is already documented in goodix,gt7375p.yaml. Now linux-next has warnings:
> 
> /builds/robherring/linux-dt/Documentation/devicetree/bindings/input/goodix,gt7986u.example.dtb:
> touchscreen@0: compatible: 'oneOf' conditional failed, one must be
> fixed:
>         ['goodix,gt7986u'] is too short
>         'goodix,gt7375p' was expected
>         from schema $id:
> 
> I understand this error message to mean the same chip model is redundantly declared
> in two separate files.

That was old error, which I fixed by reverting your patches.

> 
> Is my understanding incorrect? Could you provide more explicit guidance?
> 
> Note that the 'gt7986u' uses different drivers and has distinct device property
> for its I2C and SPI interfaces.

Drivers don't matter. We talk about bindings here. One device, one
binding, one compatible. Regardless of the bus.


Best regards,
Krzysztof

Re: [PATCH v8 2/2] dt-bindings: input: Goodix SPI HID Touchscreen

Posted by Charles Wang 1 month, 3 weeks ago

On Mon, Sep 30, 2024 at 10:08:52AM +0200, Krzysztof Kozlowski wrote:
> On 30/09/2024 09:56, Charles Wang wrote:
> > Hi Krzysztof,
> > 
> > On Mon, Sep 30, 2024 at 08:42:22AM +0200, Krzysztof Kozlowski wrote:
> >> On 30/09/2024 05:22, Charles Wang wrote:
> >>> Hi Krzysztof,
> >>> Thank you very much for your advice.
> >>>
> >>> On Thu, Sep 26, 2024 at 02:32:05PM +0200, Krzysztof Kozlowski wrote:
> >>>> On 26/09/2024 11:57, Charles Wang wrote:
> >>>>>>>  1 file changed, 71 insertions(+)
> >>>>>>>  create mode 100644 Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>>>>>
> >>>>>>> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>>>>> new file mode 100644
> >>>>>>> index 000000000..849117639
> >>>>>>> --- /dev/null
> >>>>>>> +++ b/Documentation/devicetree/bindings/input/goodix,gt7986u.yaml
> >>>>>>> @@ -0,0 +1,71 @@
> >>>>>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >>>>>>> +%YAML 1.2
> >>>>>>> +---
> >>>>>>> +$id: http://devicetree.org/schemas/input/goodix,gt7986u.yaml#
> >>>>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >>>>>>> +
> >>>>>>> +title: GOODIX GT7986U SPI HID Touchscreen
> >>>>>>> +
> >>>>>>> +maintainers:
> >>>>>>> +  - Charles Wang <charles.goodix@gmail.com>
> >>>>>>> +
> >>>>>>> +description: Supports the Goodix GT7986U touchscreen.
> >>>>>>> +  This touch controller reports data packaged according to the HID protocol,
> >>>>>>> +  but is incompatible with Microsoft's HID-over-SPI protocol.
> >>>>>>> +
> >>>>>>> +allOf:
> >>>>>>> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >>>>>>> +
> >>>>>>> +properties:
> >>>>>>> +  compatible:
> >>>>>>> +    enum:
> >>>>>>> +      - goodix,gt7986u-spi
> >>>>>>
> >>>>>> NAK, you duplicate again the binding. You cannot have bus-flavors.
> >>>>>> Device is the same.
> >>>>>>
> >>>>>
> >>>>> Could you provide some suggestions regarding this issue?
> >>>>
> >>>> What is exactly the question or problem? There is a binding for this
> >>>> device. Extend it with SPI parts, e.g.
> >>>> https://elixir.bootlin.com/linux/v6.4-rc7/source/Documentation/devicetree/bindings/iio/accel/adi,adxl313.yaml#L22
> >>>>
> >>>
> >>> This seems a little different from the adxl313.yaml.
> >>
> >> Hm? I am reading below:
> >>
> >>>
> >>> The issue we're encountering involves the chip model gt7986u,
> >>> which supports both I2C and SPI interfaces. For the I2C interface
> >>> (using the HID-over-I2C driver), it has already been declared in
> >>> the goodix,gt7375p.yaml file as follows:
> >>>
> >>> i2c {
> >>>   #address-cells = <1>;
> >>>   #size-cells = <0>;
> >>>
> >>>   ap_ts: touchscreen@5d {
> >>>     compatible = "goodix,gt7986u";
> >>>   }
> >>> }
> >>>
> >>> Currently, our design requires utilizing the SPI interface with
> >>> a custom SPI driver. However, the declarations within the binding
> >>> file have led to conflicts, as shown here:
> >>>
> >>> spi {
> >>>   #address-cells = <1>;
> >>>   #size-cells = <0>;
> >>>
> >>>   touchscreen@0 {
> >>>     compatible = "goodix,gt7986u";
> >>>   }
> >>> }
> >>>
> >>> Should I consider merging both YAML files into a single one to fix this?
> >>
> >> And there is no difference. I don't understand the problem.
> >>
> > I'm sorry for the confusion regarding your comment
> > 
> > "And there is no difference." 
> > 
> > Are you implying that the issue we are encountering is no different from
> 
> I don't understand the issue.
> 

Sorry for my poor English expression.

> > 'adi,adxl313.yaml'? Or are you suggesting that the gt7986u device should
> > be treated as the same entity for both I2C and SPI interfaces?
> 
> I told you what to do - extend existing binding. I gave you example how
> one binding is for both I2C and SPI devices.
> 

Ack, thank you for the clarification and the example provided.

> > 
> > Original error messages: https://lore.kernel.org/all/CAL_Jsq+QfTtRj_JCqXzktQ49H8VUnztVuaBjvvkg3fwEHniUHw@mail.gmail.com/
> > 
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - goodix,gt7986u
> > 
> > This is already documented in goodix,gt7375p.yaml. Now linux-next has warnings:
> > 
> > /builds/robherring/linux-dt/Documentation/devicetree/bindings/input/goodix,gt7986u.example.dtb:
> > touchscreen@0: compatible: 'oneOf' conditional failed, one must be
> > fixed:
> >         ['goodix,gt7986u'] is too short
> >         'goodix,gt7375p' was expected
> >         from schema $id:
> > 
> > I understand this error message to mean the same chip model is redundantly declared
> > in two separate files.
> 
> That was old error, which I fixed by reverting your patches.
>
> > 
> > Is my understanding incorrect? Could you provide more explicit guidance?
> > 
> > Note that the 'gt7986u' uses different drivers and has distinct device property
> > for its I2C and SPI interfaces.
> 
> Drivers don't matter. We talk about bindings here. One device, one
> binding, one compatible. Regardless of the bus.
> 

Okay, really appreciate your guidance.

Best regards,
Charles