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

[Conor Dooley]

Hey,


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.

Runtime conditions? Aren't thєse properties of the panel & therefore
fixed? If they were runtime conditions, then setting them statically in
your DT is not going to work, right?

> 
> This patch also add maintainer of this driver.
> 
> Signed-off-by: Tylor Yang <tylor_yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>

It looks like you deleted all of the properties from the previous
submission of these changes. I don't really understand that, it kinda
feels just like appeasement, as you must have needed those properties
to do the firmware loading etc. How are you filling the gap those
properties have left, when you still only have a single compatible
string in thㄟs binding? Is there a way to do runtime detection of which
chip you're dealing with that you are now using?

Confused,
Conor.

> ---
>  .../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
> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  himax,rst-gpio:
> +    maxItems: 1
> +    description: Reset device, active low signal.
> +
> +  himax,irq-gpio:
> +    maxItems: 1
> +    description: Interrupt request, active low signal.
> +
> +  himax,3v3-gpio:
> +    maxItems: 1
> +    description: GPIO to control 3.3V power supply.
> +
> +  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.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - himax,rst-gpio
> +  - himax,irq-gpio
> +
> +unevaluatedProperties: false
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        touchscreen@0 {
> +            compatible = "himax,hid-over-spi";
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            reg = <0x0>;
> +            interrupt-parent = <&gpio1>;
> +            interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
> +            pinctrl-0 = <&touch_pins>;
> +            pinctrl-names = "default";
> +
> +            spi-max-frequency = <12500000>;
> +            spi-cpha;
> +            spi-cpol;
> +
> +            himax,rst-gpio = <&gpio1 8 GPIO_ACTIVE_LOW>;
> +            himax,irq-gpio = <&gpio1 7 GPIO_ACTIVE_LOW>;
> +            himax,3v3-gpio = <&gpio1 6 GPIO_ACTIVE_HIGH>;
> +            himax,ic-det-delay-ms = <500>;
> +            himax,ic-resume-delay-ms = <100>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index bf0f54c24f81..452701261bec 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -9323,6 +9323,12 @@ L:	linux-kernel@xxxxxxxxxxxxxxx
>  S:	Maintained
>  F:	drivers/misc/hisi_hikey_usb.c
>  
> +HIMAX HID OVER SPI TOUCHSCREEN SUPPORT
> +M:	Tylor Yang <tylor_yang@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> +L:	linux-input@xxxxxxxxxxxxxxx
> +S:	Supported
> +F:	Documentation/devicetree/bindings/input/himax,hid-over-spi.yaml
> +
>  HIMAX HX83112B TOUCHSCREEN SUPPORT
>  M:	Job Noorman <job@xxxxxxxxxxxx>
>  L:	linux-input@xxxxxxxxxxxxxxx
> -- 
> 2.25.1
>

Attachment: signature.asc
Description: PGP signature