diff mbox series

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

Message ID 20210305163834.70924-3-giulio.benetti@benettiengineering.com
State New
Headers show
Series None | expand

Commit Message

Giulio Benetti March 5, 2021, 4:38 p.m. UTC
From: Giulio Benetti <giulio.benetti@micronovasrl.com>

This adds device tree bindings for the Hycon HY46XX touchscreen series.

Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
---
 .../input/touchscreen/hycon,hy46xx.yaml       | 130 ++++++++++++++++++
 MAINTAINERS                                   |   6 +
 2 files changed, 136 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml

Comments

Rob Herring March 6, 2021, 7:41 p.m. UTC | #1
On Fri, Mar 05, 2021 at 05:38:33PM +0100, Giulio Benetti wrote:
> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
> 
> This adds device tree bindings for the Hycon HY46XX touchscreen series.
> 
> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
> ---
>  .../input/touchscreen/hycon,hy46xx.yaml       | 130 ++++++++++++++++++
>  MAINTAINERS                                   |   6 +
>  2 files changed, 136 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..cddd5e5bae92
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
> @@ -0,0 +1,130 @@
> +# 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: hyconon HY46XX series touchscreen controller Bindings
> +
> +description: |
> +             There are 6 variants of the chip for various touch panel sizes and coverl len 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@micronovasrl.com>
> +
> +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
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  vcc-supply: true
> +
> +  threshold:

Needs a vendor prefix here and all the custom properties.

One of the touchscreen properties doesn't already address this?

> +    description: Allows setting the sensitivity in the range from 0 to 255.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +
> +  proximity-sensor-switch:

Why a switch rather than enable or disable?

> +    description: Allows enabling or disabling the Proximity Sensor.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 1

Is this a hardware config or something a user would want to change at 
runtime? The latter shouldn't be in DT.

What does not present mean? Unless there is a need for keeping the 
default setting, then this could be boolean instead.

Same questions for the rest of the properties.

> +
> +  glove-enable:
> +    description: Allows enabling or disabling glove setting.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 1
> +
> +  report-speed:
> +    description: Allows setting the report speed(i.e 0x64 => 100Hz).
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 255
> +
> +  power-noise-enable:
> +    description: Allows enabling or disabling power noise filter.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 1
> +
> +  filter-data:
> +    description: Allows setting the filtering data before reporting touch
> +                 in the range from 0 to 5.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 5
> +
> +  gain:
> +    description: Allows setting the sensitivity distance in the range from 0 to 5.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 5
> +
> +  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 {
> +        compatible = "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 d92f85ca831d..3f83daf6b2bf 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -8245,6 +8245,12 @@ S:	Maintained
>  F:	mm/hwpoison-inject.c
>  F:	mm/memory-failure.c
>  
> +HYCON HY46XX TOUCHSCREEN SUPPORT
> +M:	Giulio Benetti <giulio.benetti@micronovasrl.com>
> +L:	linux-input@vger.kernel.org
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
> +
>  HYGON PROCESSOR SUPPORT
>  M:	Pu Wen <puwen@hygon.cn>
>  L:	linux-kernel@vger.kernel.org
> -- 
> 2.25.1
>
Giulio Benetti April 1, 2021, 6:37 p.m. UTC | #2
Hello Rob, All,

Il 06/03/2021 20:41, Rob Herring ha scritto:
> On Fri, Mar 05, 2021 at 05:38:33PM +0100, Giulio Benetti wrote:
>> From: Giulio Benetti <giulio.benetti@micronovasrl.com>
>>
>> This adds device tree bindings for the Hycon HY46XX touchscreen series.
>>
>> Signed-off-by: Giulio Benetti <giulio.benetti@micronovasrl.com>
>> ---
>>   .../input/touchscreen/hycon,hy46xx.yaml       | 130 ++++++++++++++++++
>>   MAINTAINERS                                   |   6 +
>>   2 files changed, 136 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..cddd5e5bae92
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
>> @@ -0,0 +1,130 @@
>> +# 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: hyconon HY46XX series touchscreen controller Bindings
>> +
>> +description: |
>> +             There are 6 variants of the chip for various touch panel sizes and coverl len 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@micronovasrl.com>
>> +
>> +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
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  reset-gpios:
>> +    maxItems: 1
>> +
>> +  vcc-supply: true
>> +
>> +  threshold:
> 
> Needs a vendor prefix here and all the custom properties.

Sorry but I can't understand this ^^^, I've checked a lot of other yaml
files but couldn't find something related to this. Can you help me by
pointing me an example?

> 
> One of the touchscreen properties doesn't already address this?
> 
>> +    description: Allows setting the sensitivity in the range from 0 to 255.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 255
>> +
>> +  proximity-sensor-switch:
> 
> Why a switch rather than enable or disable?

I've changed its name into enable.

> 
>> +    description: Allows enabling or disabling the Proximity Sensor.
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    minimum: 0
>> +    maximum: 1
> 
> Is this a hardware config or something a user would want to change at
> runtime? The latter shouldn't be in DT.

It's a hardware config that enable the 11th touch point, but I didn't
handle it inside the driver, so it's better that I remove it for the
moment.

> What does not present mean? Unless there is a need for keeping the
> default setting, then this could be boolean instead.

I've changed all "enable" to boolean and rely on default of the
controller.

Does this sound ok for you so I can send a v2 patchset?

Thank you
Best regards
Giulio Benetti April 1, 2021, 10:24 p.m. UTC | #3
Il 01/04/2021 20:37, Giulio Benetti ha scritto:
>>
>> Needs a vendor prefix here and all the custom properties.
> 
> Sorry but I can't understand this ^^^, I've checked a lot of other yaml
> files but couldn't find something related to this. Can you help me by
> pointing me an example?
> 

Found in example-schema.yaml, sorry for the noise
Giulio Benetti April 1, 2021, 11:03 p.m. UTC | #4
This patchset adds Hycon vendor, HY46XX touchscreen controller driver
and its .yaml binding.

---
V1->V2:
* changed authorship and SoBs to @benettiengineering.com domain
* fixed vendor commit log according to Jonathan Neuschäfer's suggestion
* fixed hy46xx bindings according to Rob Herring's suggestions
* fixed hy46xx driver according to Dmitry Torokhov's suggestions
further details are listed in single patches
---

Giulio Benetti (3):
  dt-bindings: Add Hycon Technology vendor prefix
  dt-bindings: touchscreen: Add HY46XX bindings
  Input: add driver for the Hycon HY46XX touchpanel series

 .../input/touchscreen/hycon,hy46xx.yaml       | 120 ++++
 .../devicetree/bindings/vendor-prefixes.yaml  |   2 +
 MAINTAINERS                                   |   7 +
 drivers/input/touchscreen/Kconfig             |  12 +
 drivers/input/touchscreen/Makefile            |   1 +
 drivers/input/touchscreen/hycon-hy46xx.c      | 591 ++++++++++++++++++
 6 files changed, 733 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
 create mode 100644 drivers/input/touchscreen/hycon-hy46xx.c
Jonathan Neuschäfer April 2, 2021, 8:36 a.m. UTC | #5
On Fri, Apr 02, 2021 at 01:03:57AM +0200, Giulio Benetti wrote:
> This adds device tree bindings for the Hycon HY46XX touchscreen series.

> 

> Signed-off-by: Giulio Benetti <giulio.benetti@benettiengineering.com>

> ---

> 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

> ---

>  .../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..71a1dbabcd4f

> --- /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


hycon -> Hycon        (because it's a name)
Bindings -> bindings  (because it's just a regular word)

> +

> +description: |

> +             There are 6 variants of the chip for various touch panel sizes and coverl len material


"coverl len material" looks like a typo. What does it mean?

> +              Glass: 0.3mm--4.0mm

> +              PET/PMMA: 0.2mm--2.0mm

> +               HY4613(B)-N048  < 6"

> +               HY4614(B)-N068  7" .. 10.1"


According to the datasheet I was able to find[1], HY4613-N048 supports
touch panel sizes smaller than 5.3". Did this change in newer
datasheets?

What does the (B) part of the part number mean?


[1]: https://datasheetspdf.com/pdf/1297773/HYCON/HY4613-N048/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"


The description block seems unusually far indented. I'm not a YAML
expert, but according to the yamllint tool, it would work with much less
indentation:

description: |
  There are 6 variants of the chip for various touch panel sizes and coverl len material
   Glass: 0.3mm--4.0mm
[...]


> +  hycon,glove-enable:

> +    type: boolean

> +    description: Allows enabling or disabling glove setting.


Small nit: Due to the way boolean properties work in DT, you can't
really use the property to disable the glove setting (in order to
disable the setting, you would explicitly not use the property).
Perhaps:

+    description: Allows enabling the glove setting.

I don't really know :)


> +

> +  hycon,report-speed:

> +    description: Allows setting the report speed(i.e 0x64 => 100Hz).

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

> +    minimum: 0

> +    maximum: 255


Please add a space before the opening parenthesis:

+    description: Allows setting the report speed (i.e 0x64 => 100Hz).

Or perhaps like this:

+    description: Allows setting the report speed in Hertz.



Thanks,
Jonathan Neuschäfer
diff mbox series

Patch

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..cddd5e5bae92
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
@@ -0,0 +1,130 @@ 
+# 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: hyconon HY46XX series touchscreen controller Bindings
+
+description: |
+             There are 6 variants of the chip for various touch panel sizes and coverl len 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@micronovasrl.com>
+
+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
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  reset-gpios:
+    maxItems: 1
+
+  vcc-supply: true
+
+  threshold:
+    description: Allows setting the sensitivity in the range from 0 to 255.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 255
+
+  proximity-sensor-switch:
+    description: Allows enabling or disabling the Proximity Sensor.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1
+
+  glove-enable:
+    description: Allows enabling or disabling glove setting.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1
+
+  report-speed:
+    description: Allows setting the report speed(i.e 0x64 => 100Hz).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 255
+
+  power-noise-enable:
+    description: Allows enabling or disabling power noise filter.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 1
+
+  filter-data:
+    description: Allows setting the filtering data before reporting touch
+                 in the range from 0 to 5.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 5
+
+  gain:
+    description: Allows setting the sensitivity distance in the range from 0 to 5.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 5
+
+  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 {
+        compatible = "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 d92f85ca831d..3f83daf6b2bf 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8245,6 +8245,12 @@  S:	Maintained
 F:	mm/hwpoison-inject.c
 F:	mm/memory-failure.c
 
+HYCON HY46XX TOUCHSCREEN SUPPORT
+M:	Giulio Benetti <giulio.benetti@micronovasrl.com>
+L:	linux-input@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/input/touchscreen/hycon,hy46xx.yaml
+
 HYGON PROCESSOR SUPPORT
 M:	Pu Wen <puwen@hygon.cn>
 L:	linux-kernel@vger.kernel.org