diff mbox series

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

Message ID 20210401230358.2468618-3-giulio.benetti@benettiengineering.com (mailing list archive)
State Superseded
Headers show
Series Input: add Hycon HY46XX Touchscreen controller | expand

Commit Message

Giulio Benetti April 1, 2021, 11:03 p.m. UTC
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

Comments

Jonathan Neuschäfer April 2, 2021, 8:36 a.m. UTC | #1
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
Giulio Benetti April 2, 2021, 3:17 p.m. UTC | #2
Hi Jonathan,

thank you for reviewing,

On 4/2/21 10:36 AM, Jonathan Neuschäfer wrote:
> 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)

ok

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

yes

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

Yes, there is a newer document called "HY46XX Application Notes
V4.0", here is the screenshot of models:
https://pasteboard.co/JVtOMcO.png

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

It's an upgraded version even if hardware is fully compatible, so (B) 
means with and without "B".

> 
> [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
> [...]

Ah yes, I've started from edt,ft5x06.yaml and there it was indented that 
way. Anyway I've changed it.

> 
>> +  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 :)

Ah yes, it's true. If not specified it's simply not enabled, so I use 
your suggested form.

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

Oh yes! I didn't realize it was exactly in Hertz, 0x64 is 100, not only 
equivalent to 100Hz, so every step is 1 Hz

> 
> Thanks,
> Jonathan Neuschäfer
> 

Thanks again!
Best regards
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..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
+
+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@benettiengineering.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
+
+  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 or disabling glove setting.
+
+  hycon,report-speed:
+    description: Allows setting the report speed(i.e 0x64 => 100Hz).
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 255
+
+  hycon,power-noise-enable:
+    type: boolean
+    description: Allows enabling or disabling power noise filter.
+
+  hycon,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
+
+  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 {
+        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 6e91994b8d3b..5e9cc7e610ce 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8242,6 +8242,12 @@  S:	Maintained
 F:	mm/hwpoison-inject.c
 F:	mm/memory-failure.c
 
+HYCON HY46XX TOUCHSCREEN SUPPORT
+M:	Giulio Benetti <giulio.benetti@benettiengineering.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