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

[Charles Wang <charles.goodix@gmail.com>]

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