Re: [PATCH V2 1/2] dt-bindings: input: Introduce Himax HID-over-SPI device

[Rob Herring]

On Tue, Sep 19, 2023 at 10:49:42AM +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 pin to disable
> and enable interrupt when suspend/resume. An optional power control
> pin if target board needed. Panel id pins for identify panel is also
> an option.
> 
> Additional optional arguments:
> ic-det-delay-ms and ic-resume-delay-ms are using to solve runtime
> conditions.
> 
> This patch also add maintainer of this driver.
> 
> Signed-off-by: Tylor Yang <tylor_yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> ---
>  .../bindings/input/himax,hid-over-spi.yaml    | 109 ++++++++++++++++++
>  MAINTAINERS                                   |   6 +
>  2 files changed, 115 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
> new file mode 100644
> index 000000000000..3ee3a89842ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
> @@ -0,0 +1,109 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/himax,hid-over-spi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Himax TDDI devices using SPI to send HID packets
> +
> +maintainers:
> +  - Tylor Yang <tylor_yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> +
> +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-over-spi

Doesn't look like a specific device. Compatibles are generally based on 
part numbers.

'over-spi' is redundant as the parent would be a spi controller.

> +
> +  reg:
> +    maxItems: 1
> +

> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0

These are for child nodes, but you don't have any.

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  himax,rst-gpio:
> +    maxItems: 1
> +    description: Reset device, active low signal.

Use standard reset-gpios.

(-gpio is deprecated)

> +
> +  himax,irq-gpio:
> +    maxItems: 1
> +    description: Interrupt request, active low signal.

You have the interrupt already, why do you need this?

> +
> +  himax,3v3-gpio:
> +    maxItems: 1
> +    description: GPIO to control 3.3V power supply.

This should be a regulator supply. Then use gpio-regulator if it happens 
to be GPIO controlled.

> +
> +  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.

These should be implied by the compatible. Unless they are board 
specific and not device specific.

Rob