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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a0c89130-2b1c-494d-bd39-0f217e9979d5@kernel.org>
Date: Wed, 27 Nov 2024 11:33:57 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Sasha Finkelstein <fnkl.kernel@...il.com>
Cc: Hector Martin <marcan@...can.st>, Sven Peter <sven@...npeter.dev>,
 Alyssa Rosenzweig <alyssa@...enzweig.io>,
 Dmitry Torokhov <dmitry.torokhov@...il.com>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Henrik Rydberg <rydberg@...math.org>,
 asahi@...ts.linux.dev, linux-arm-kernel@...ts.infradead.org,
 linux-input@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/4] dt-bindings: input: touchscreen: Add Z2 controller

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