Re: [PATCH v4 2/3] dt-bindings: touchscreen: Add HY46XX bindings

From: Rob Herring
Date: Thu Apr 08 2021 - 16:21:41 EST


On Wed, Apr 07, 2021 at 07:49:08PM +0200, Giulio Benetti wrote:
> This adds device tree bindings for the Hycon HY46XX touchscreen series.
>
> Signed-off-by: Giulio Benetti <giulio.benetti@xxxxxxxxxxxxxxxxxxxxxx>
> ---
> V1->V2:
> As suggested by Rob Herring:
> * fixed $id: address
> * added "hycon," in front of every custom property
> * changed all possible property to boolean type
> * removed proximity-sensor-switch property since it's not handled in driver
> V2->V3:
> As suggested by Jonathan Neuschäfer:
> * fixed some typo
> * fixed description indentation
> * improved boolean properties descriptions
> * improved hycon,report-speed description
> V3->V4:
> * fixed binding compatible string in example as suggested by Jonathan Neuschäfer
> ---
> .../input/touchscreen/hycon,hy46xx.yaml | 120 ++++++++++++++++++
> MAINTAINERS | 6 +
> 2 files changed, 126 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml b/Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
> new file mode 100644
> index 000000000000..8860613a12ad
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
> @@ -0,0 +1,120 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/touchscreen/hycon,hy46xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Hycon HY46XX series touchscreen controller bindings
> +
> +description: |
> + There are 6 variants of the chip for various touch panel sizes and cover lens material
> + Glass: 0.3mm--4.0mm
> + PET/PMMA: 0.2mm--2.0mm
> + HY4613(B)-N048 < 6"
> + HY4614(B)-N068 7" .. 10.1"
> + HY4621-NS32 < 5"
> + HY4623-NS48 5.1" .. 7"
> + Glass: 0.3mm--8.0mm
> + PET/PMMA: 0.2mm--4.0mm
> + HY4633(B)-N048 < 6"
> + HY4635(B)-N048 < 7" .. 10.1"
> +
> +maintainers:
> + - Giulio Benetti <giulio.benetti@xxxxxxxxxxxxxxxxxxxxxx>
> +
> +allOf:
> + - $ref: touchscreen.yaml#
> +
> +properties:
> + compatible:
> + enum:
> + - hycon,hycon-hy4613
> + - hycon,hycon-hy4614
> + - hycon,hycon-hy4621
> + - hycon,hycon-hy4623
> + - hycon,hycon-hy4633
> + - hycon,hycon-hy4635

As suggested earlier, drop the 2nd 'hycon'.

> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + vcc-supply: true
> +
> + hycon,threshold:
> + description: Allows setting the sensitivity in the range from 0 to 255.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 255
> +
> + hycon,glove-enable:
> + type: boolean
> + description: Allows enabling glove setting.
> +
> + hycon,report-speed:
> + description: Allows setting the report speed in Hertz.

If in Hertz, use standard unit suffix.

> + $ref: /schemas/types.yaml#/definitions/uint32

And then you can drop this.

> + minimum: 0

0Hz doesn't seem to useful?

> + maximum: 255
> +
> + hycon,power-noise-enable:

hycon,noise-filter-enable

No one wants to enable power noise. :)

> + type: boolean
> + description: Allows enabling power noise filter.
> +
> + hycon,filter-data:
> + description: Allows setting the filtering data before reporting touch
> + in the range from 0 to 5.

This is averaging samples? Sounds like something common perhaps.

> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 5
> +
> + hycon,gain:
> + description: Allows setting the sensitivity distance in the range from 0 to 5.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 5
> +
> + hycon,edge-offset:
> + description: Allows setting the edge compensation in the range from 0 to 16.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 16
> +
> + touchscreen-size-x: true
> + touchscreen-size-y: true
> + touchscreen-fuzz-x: true
> + touchscreen-fuzz-y: true
> + touchscreen-inverted-x: true
> + touchscreen-inverted-y: true
> + touchscreen-swapped-x-y: true
> + interrupt-controller: true
> +
> +additionalProperties: false
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + hycon-hy4633@1c {

touchscreen@1c

> + compatible = "hycon,hycon-hy4633";
> + reg = <0x1c>;
> + interrupt-parent = <&gpio2>;
> + interrupts = <5 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio2 6 GPIO_ACTIVE_LOW>;
> + };
> + };
> +
> +...
> diff --git a/MAINTAINERS b/MAINTAINERS
> index c80ad735b384..d022ff09e609 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8243,6 +8243,12 @@ S: Maintained
> F: mm/hwpoison-inject.c
> F: mm/memory-failure.c
>
> +HYCON HY46XX TOUCHSCREEN SUPPORT
> +M: Giulio Benetti <giulio.benetti@xxxxxxxxxxxxxxxxxxxxxx>
> +L: linux-input@xxxxxxxxxxxxxxx
> +S: Maintained
> +F: Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
> +
> HYGON PROCESSOR SUPPORT
> M: Pu Wen <puwen@xxxxxxxx>
> L: linux-kernel@xxxxxxxxxxxxxxx
> --
> 2.25.1
>