linux-kernel - Re: [PATCH v3 1/4] dt-bindings: input: Introduce Himax HID-over-SPI device

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 for Android: free password hash cracker in your pocket

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

Message-ID: <9fd1c6cf-ae4d-499b-bd62-e74b580cd29c@linaro.org>
Date:   Tue, 17 Oct 2023 18:58:43 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Conor Dooley <conor@...nel.org>,
        Tylor Yang <tylor_yang@...ax.corp-partner.google.com>
Cc:     dmitry.torokhov@...il.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
        jikos@...nel.org, benjamin.tissoires@...hat.com,
        linux-input@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        poyuan_chang@...ax.corp-partner.google.com,
        jingyliang@...omium.org, hbarnor@...omium.org, wuxy23@...ovo.com,
        luolm1@...ovo.com, poyu_hung@...ax.corp-partner.google.com
Subject: Re: [PATCH v3 1/4] dt-bindings: input: Introduce Himax HID-over-SPI
 device

On 17/10/2023 15:59, Conor Dooley wrote:
> Yo,
> 
> On Tue, Oct 17, 2023 at 05:18:57PM +0800, Tylor Yang wrote:
>> The Himax HID-over-SPI framework support for Himax touchscreen ICs
>> that report HID packet through SPI bus. The driver core need reset
>>  pin to meet reset timing spec. of IC. An interrupt to disable
>> and enable interrupt when suspend/resume. Two optional power control
>>  if target board needed.
>>
>> Signed-off-by: Tylor Yang <tylor_yang@...ax.corp-partner.google.com>
>> ---
>>  .../devicetree/bindings/input/himax,hid.yaml  | 123 ++++++++++++++++++
>>  MAINTAINERS                                   |   6 +
>>  2 files changed, 129 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/input/himax,hid.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/input/himax,hid.yaml b/Documentation/devicetree/bindings/input/himax,hid.yaml
>> new file mode 100644
>> index 000000000000..9ba86fe1b7da
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/himax,hid.yaml
>> @@ -0,0 +1,123 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/himax,hid.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Himax TDDI devices using SPI to send HID packets
>> +
>> +maintainers:
>> +  - Tylor Yang <tylor_yang@...ax.corp-partner.google.com>
>> +
>> +description: |
>> +  Support the Himax TDDI devices which using SPI interface to acquire
>> +  HID packets from the device. The device needs to be initialized using
>> +  Himax protocol before it start sending HID packets.
>> +
>> +properties:
>> +  compatible:
>> +    const: himax,hid
> 
> This compatible seems far too generic. Why are there not device specific
> compatibles for each TDDI device?

Which was pointed out by Rob in v2, so his feedback was ignored.

> 
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  reset:
>> +    maxItems: 1
>> +    description: Reset device, active low signal.

No, come one, read feedback from Rob.

>> +
>> +  vccd-supply:
>> +    description:
>> +      Optional regulator for the 1.8V voltage.
>> +
>> +  vcca-supply:
>> +    description:
>> +      Optional regulator for the analog 3.3V voltage.
>> +
>> +  himax,id-gpios:
>> +    maxItems: 8
>> +    description: GPIOs to read physical Panel ID. Optional.
>> +
>> +  spi-cpha: true
>> +  spi-cpol: true
> 
>> +  himax,ic-det-delay-ms:
>> +    description:
>> +      Due to TDDI properties, the TPIC detection timing must after the
>> +      display panel initialized. This property is used to specify the
>> +      delay time when TPIC detection and display panel initialization
>> +      timing are overlapped. How much milliseconds to delay before TPIC
>> +      detection start.
>> +
>> +  himax,ic-resume-delay-ms:
>> +    description:
>> +      Due to TDDI properties, the TPIC resume timing must after the
>> +      display panel resumed. This property is used to specify the
>> +      delay time when TPIC resume and display panel resume
>> +      timing are overlapped. How much milliseconds to delay before TPIC
>> +      resume start.
> 

No improvements here...

You must implement all feedback from v2. Not pieces of it.

Best regards,
Krzysztof