Re: [PATCH 1/4] dt-bindings: input: touchscreen: Add Z2 controller

[Krzysztof Kozlowski <krzk@kernel.org>]

On 27/11/2024 09:46, Krzysztof Kozlowski wrote:
> On Tue, Nov 26, 2024 at 09:47:59PM +0100, Sasha Finkelstein wrote:
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - apple,j293-touchbar
>> +          - apple,j493-touchbar
>> +      - const: apple,z2-touchbar
>> +      - const: apple,z2-multitouch
> 
> What is the meaning of these two last compatibles in the list? What are
> these devices?


Previous Rob's comment apply here as well. If z2 is protocol, then
multitouch and touchbar do not feel appropriate, unless these are some
subsets of the protocol. But as in other cases no one knows here what's
there inside, so avoid making generic compatibles. Just
apple,j293-touchbar and 493+293. That's the recommendation we keep
insisting on almost always.

As Rob explained: protocol does not matter in terms of compatible. We do
not have devices like "analog,j293-spi" (and there is clear NAK when
people post them, with one or two exceptions).

> 
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  cs-gpios:
>> +    maxItems: 1
>> +    description: |
> 
> Do not need '|' unless you need to preserve formatting.
> 
>> +      J293 has a hardware quirk where the CS line is unusable and has
>> +      to the be driven by a GPIO pin instead
>> +
>> +  firmware-name:
>> +    maxItems: 1
>> +
>> +  label:
>> +    maxItems: 1
> 
> Why is this needed? I think it is not part of common touchscreen schema.
> Drop, devices do not need labels - node name and unit address identify
> it. If this is needed for something else, then come with generic
> property matching all touchscreens.

This is v1 so I did not expect previous talks, but now I dig them out
and there was conclusion: your compatible defines label. You do not have
two of same devices in the DTS to justify it. Drop the property.

Best regards,
Krzysztof