[PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[Nikita Travkin]

This series adds support for the touch-keys that can be present on some
touchscreen configurations, adds the compatible for bt532 and fixes a
small race condition bug in the driver probe function.

I also pick up the series that converts the dt bindings to yaml
initially submitted by Linus Walleij in [1].
I made some minor changes to those patches:
 - Fixed dt_schema_check error
 - Adressed the review comments from Dmitry on the original series

[1] https://lore.kernel.org/linux-input/20210625113435.2539282-1-linus.walleij@linaro.org/

Linus Walleij (2):
  dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend
  Input: zinitix - Handle proper supply names

Nikita Travkin (4):
  input: touchscreen: zinitix: Make sure the IRQ is allocated before it
    gets enabled
  input: touchscreen: zinitix: Add compatible for bt532
  dt-bindings: input: zinitix: Document touch-keys support
  input: touchscreen: zinitix: Add touchkey support

 .../input/touchscreen/zinitix,bt400.yaml      | 123 ++++++++++++++++++
 .../bindings/input/touchscreen/zinitix.txt    |  40 ------
 drivers/input/touchscreen/zinitix.c           | 101 +++++++++++---
 3 files changed, 207 insertions(+), 57 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt

-- 
2.30.2

[PATCH 1/6] input: touchscreen: zinitix: Make sure the IRQ is allocated before it gets enabled

[Nikita Travkin]

Since irq request is the last thing in the driver probe, it happens
later than the input device registration. This means that there is a
small time window where if the open method is called the driver will
attempt to enable not yet available irq.

Fix that by moving the irq request before the input device registration.

Fixes: 26822652c85e ("Input: add zinitix touchscreen driver")
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/input/touchscreen/zinitix.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index b8d901099378..1e70b8d2a8d7 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -488,6 +488,15 @@ static int zinitix_ts_probe(struct i2c_client *client)
 		return error;
 	}
 
+	error = devm_request_threaded_irq(&client->dev, client->irq,
+					  NULL, zinitix_ts_irq_handler,
+					  IRQF_ONESHOT | IRQF_NO_AUTOEN,
+					  client->name, bt541);
+	if (error) {
+		dev_err(&client->dev, "Failed to request IRQ: %d\n", error);
+		return error;
+	}
+
 	error = zinitix_init_input_dev(bt541);
 	if (error) {
 		dev_err(&client->dev,
@@ -513,15 +522,6 @@ static int zinitix_ts_probe(struct i2c_client *client)
 		return -EINVAL;
 	}
 
-	error = devm_request_threaded_irq(&client->dev, client->irq,
-					  NULL, zinitix_ts_irq_handler,
-					  IRQF_ONESHOT | IRQF_NO_AUTOEN,
-					  client->name, bt541);
-	if (error) {
-		dev_err(&client->dev, "Failed to request IRQ: %d\n", error);
-		return error;
-	}
-
 	return 0;
 }
 
-- 
2.30.2

Re: [PATCH 1/6] input: touchscreen: zinitix: Make sure the IRQ is allocated before it gets enabled

[Linus Walleij]

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:

> Since irq request is the last thing in the driver probe, it happens
> later than the input device registration. This means that there is a
> small time window where if the open method is called the driver will
> attempt to enable not yet available irq.
>
> Fix that by moving the irq request before the input device registration.
>
> Fixes: 26822652c85e ("Input: add zinitix touchscreen driver")
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>

Good catch!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 2/6] dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend

[Nikita Travkin]

From: Linus Walleij <linus.walleij@linaro.org>

This converts the Zinitix BT4xx and BT5xx touchscreen bindings to YAML, fix
them up a bit and extends them.

We list all the existing BT4xx and BT5xx components with compatible strings.
These are all similar, use the same bindings and work in similar ways.

We rename the supplies from the erroneous vdd/vddo to the actual supply
names vcca/vdd as specified on the actual component. It is long established
that supplies shall be named after the supply pin names of a component.
The confusion probably stems from that in a certain product the rails to the
component were named vdd/vddo. Drop some notes on how OS implementations should
avoid confusion by first looking for vddo, and if that exists assume the
legacy binding pair and otherwise use vcca/vdd.

Add reset-gpios as sometimes manufacturers pulls a GPIO line to the reset
line on the chip.

Add optional touchscreen-fuzz-x and touchscreen-fuzz-y properties.

Cc: Mark Brown <broonie@kernel.org>
Cc: Michael Srba <Michael.Srba@seznam.cz>
Cc: phone-devel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
[Fixed dt_schema_check error]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
This patch was previously submited here:
https://lore.kernel.org/linux-input/20210625113435.2539282-1-linus.walleij@linaro.org/

Changes since the original patch:
 - Use enum for compatible list instead of oneOf + const
---
 .../input/touchscreen/zinitix,bt400.yaml      | 115 ++++++++++++++++++
 .../bindings/input/touchscreen/zinitix.txt    |  40 ------
 2 files changed, 115 insertions(+), 40 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
 delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
new file mode 100644
index 000000000000..b4e5ba7c0b49
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/zinitix,bt400.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Zinitix BT4xx and BT5xx series touchscreen controller bindings
+
+description: The Zinitix BT4xx and BT5xx series of touchscreen controllers
+  are Korea-produced touchscreens with embedded microcontrollers. The
+  BT4xx series was produced 2010-2013 and the BT5xx series 2013-2014.
+
+maintainers:
+  - Michael Srba <Michael.Srba@seznam.cz>
+  - Linus Walleij <linus.walleij@linaro.org>
+
+allOf:
+  - $ref: touchscreen.yaml#
+
+properties:
+  $nodename:
+    pattern: "^touchscreen(@.*)?$"
+
+  compatible:
+    enum:
+      - zinitix,bt402
+      - zinitix,bt403
+      - zinitix,bt404
+      - zinitix,bt412
+      - zinitix,bt413
+      - zinitix,bt431
+      - zinitix,bt432
+      - zinitix,bt531
+      - zinitix,bt532
+      - zinitix,bt538
+      - zinitix,bt541
+      - zinitix,bt548
+      - zinitix,bt554
+      - zinitix,at100
+
+  reg:
+    description: I2C address on the I2C bus
+
+  clock-frequency:
+    description: I2C client clock frequency, defined for host when using
+      the device on the I2C bus
+    minimum: 0
+    maximum: 400000
+
+  interrupts:
+    description: Interrupt to host
+    maxItems: 1
+
+  vcca-supply:
+    description: Analog power supply regulator on the VCCA pin
+
+  vdd-supply:
+    description: Digital power supply regulator on the VDD pin.
+      In older device trees this can be the accidental name for the analog
+      supply on the VCCA pin, and in that case the deprecated vddo-supply is
+      used for the digital power supply.
+
+  vddo-supply:
+    description: Deprecated name for the digital power supply, use vdd-supply
+      as this reflects the real name of the pin. If this supply is present,
+      the vdd-supply represents VCCA instead of VDD. Implementers should first
+      check for this property, and if it is present assume that the vdd-supply
+      represents the analog supply.
+    deprecated: true
+
+  reset-gpios:
+    description: Reset line for the touchscreen, should be tagged
+      as GPIO_ACTIVE_LOW
+
+  zinitix,mode:
+    description: Mode of reporting touch points. Some modes may not work
+      with a particular ts firmware for unknown reasons. Available modes are
+      1 and 2. Mode 2 is the default and preferred.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [1, 2]
+
+  touchscreen-size-x: true
+  touchscreen-size-y: true
+  touchscreen-fuzz-x: true
+  touchscreen-fuzz-y: true
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - touchscreen-size-x
+  - touchscreen-size-y
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      touchscreen@20 {
+        compatible = "zinitix,bt541";
+        reg = <0x20>;
+        interrupt-parent = <&gpio>;
+        interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
+        vcca-supply = <&reg_vcca_tsp>;
+        vdd-supply = <&reg_vdd_tsp>;
+        touchscreen-size-x = <540>;
+        touchscreen-size-y = <960>;
+        zinitix,mode = <2>;
+      };
+    };
diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix.txt b/Documentation/devicetree/bindings/input/touchscreen/zinitix.txt
deleted file mode 100644
index 446efb9f5f55..000000000000
--- a/Documentation/devicetree/bindings/input/touchscreen/zinitix.txt
+++ /dev/null
@@ -1,40 +0,0 @@
-Device tree bindings for Zinitx BT541 touchscreen controller
-
-Required properties:
-
- - compatible		: Should be "zinitix,bt541"
- - reg			: I2C address of the chip. Should be 0x20
- - interrupts		: Interrupt to which the chip is connected
-
-Optional properties:
-
- - vdd-supply		: Analog power supply regulator on VCCA pin
- - vddo-supply		: Digital power supply regulator on VDD pin
- - zinitix,mode		: Mode of reporting touch points. Some modes may not work
-			  with a particular ts firmware for unknown reasons. Available
-			  modes are 1 and 2. Mode 2 is the default and preferred.
-
-The touchscreen-* properties are documented in touchscreen.txt in this
-directory.
-
-Example:
-
-	i2c@00000000 {
-		/* ... */
-
-		bt541@20 {
-			compatible = "zinitix,bt541";
-			reg = <0x20>;
-			interrupt-parent = <&msmgpio>;
-			interrupts = <13 IRQ_TYPE_EDGE_FALLING>;
-			pinctrl-names = "default";
-			pinctrl-0 = <&tsp_default>;
-			vdd-supply = <&reg_vdd_tsp>;
-			vddo-supply = <&pm8916_l6>;
-			touchscreen-size-x = <540>;
-			touchscreen-size-y = <960>;
-			zinitix,mode = <2>;
-		};
-
-		/* ... */
-	};
-- 
2.30.2

Re: [PATCH 2/6] dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend

[Rob Herring]

On Wed, 27 Oct 2021 23:13:46 +0500, Nikita Travkin wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> This converts the Zinitix BT4xx and BT5xx touchscreen bindings to YAML, fix
> them up a bit and extends them.
> 
> We list all the existing BT4xx and BT5xx components with compatible strings.
> These are all similar, use the same bindings and work in similar ways.
> 
> We rename the supplies from the erroneous vdd/vddo to the actual supply
> names vcca/vdd as specified on the actual component. It is long established
> that supplies shall be named after the supply pin names of a component.
> The confusion probably stems from that in a certain product the rails to the
> component were named vdd/vddo. Drop some notes on how OS implementations should
> avoid confusion by first looking for vddo, and if that exists assume the
> legacy binding pair and otherwise use vcca/vdd.
> 
> Add reset-gpios as sometimes manufacturers pulls a GPIO line to the reset
> line on the chip.
> 
> Add optional touchscreen-fuzz-x and touchscreen-fuzz-y properties.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Michael Srba <Michael.Srba@seznam.cz>
> Cc: phone-devel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> [Fixed dt_schema_check error]
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> This patch was previously submited here:
> https://lore.kernel.org/linux-input/20210625113435.2539282-1-linus.walleij@linaro.org/
> 
> Changes since the original patch:
>  - Use enum for compatible list instead of oneOf + const
> ---
>  .../input/touchscreen/zinitix,bt400.yaml      | 115 ++++++++++++++++++
>  .../bindings/input/touchscreen/zinitix.txt    |  40 ------
>  2 files changed, 115 insertions(+), 40 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
>  delete mode 100644 Documentation/devicetree/bindings/input/touchscreen/zinitix.txt
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH 2/6] dt-bindings: input/ts/zinitix: Convert to YAML, fix and extend

[Linus Walleij]

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:

> This patch was previously submited here:
> https://lore.kernel.org/linux-input/20210625113435.2539282-1-linus.walleij@linaro.org/
>
> Changes since the original patch:
>  - Use enum for compatible list instead of oneOf + const

Thanks for picking this up. I was meaning to get back to fixing up the
Zinitix driver but haven't had time.

Yours,
Linus Walleij

[PATCH 3/6] Input: zinitix - Handle proper supply names

[Nikita Travkin]

From: Linus Walleij <linus.walleij@linaro.org>

The supply names of the Zinitix touchscreen were a bit confused, the new
bindings rectifies this.

To deal with old and new devicetrees, first check if we have "vddo" and in
case that exists assume the old supply names. Else go and look for the new
ones.

We cannot just get the regulators since we would get an OK and a dummy
regulator: we need to check explicitly for the old supply name.

Use struct device *dev as a local variable instead of the I2C client since
the device is what we are actually obtaining the resources from.

Cc: Mark Brown <broonie@kernel.org>
Cc: Michael Srba <Michael.Srba@seznam.cz>
Cc: phone-devel@vger.kernel.org
Cc: devicetree@vger.kernel.org
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
[Slightly changed the legacy regulator detection]
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
This patch was previously submitted here:
https://lore.kernel.org/linux-input/20210625113435.2539282-2-linus.walleij@linaro.org/

Changes since the original patch:
 - Adress the review comments by Dmitry:
   Drop explict OF check and use of_find_property()
---
 drivers/input/touchscreen/zinitix.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index 1e70b8d2a8d7..d4e06a88a883 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -252,16 +252,27 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 
 static int zinitix_init_regulators(struct bt541_ts_data *bt541)
 {
-	struct i2c_client *client = bt541->client;
+	struct device *dev = &bt541->client->dev;
 	int error;
 
-	bt541->supplies[0].supply = "vdd";
-	bt541->supplies[1].supply = "vddo";
-	error = devm_regulator_bulk_get(&client->dev,
+	/*
+	 * Some older device trees have erroneous names for the regulators,
+	 * so check if "vddo" is present and in that case use these names
+	 * and warn. Else use the proper supply names on the component.
+	 */
+	if (of_find_property(dev->of_node, "vddo-supply", NULL)) {
+		bt541->supplies[0].supply = "vdd";
+		bt541->supplies[1].supply = "vddo";
+	} else {
+		/* Else use the proper supply names */
+		bt541->supplies[0].supply = "vcca";
+		bt541->supplies[1].supply = "vdd";
+	}
+	error = devm_regulator_bulk_get(dev,
 					ARRAY_SIZE(bt541->supplies),
 					bt541->supplies);
 	if (error < 0) {
-		dev_err(&client->dev, "Failed to get regulators: %d\n", error);
+		dev_err(dev, "Failed to get regulators: %d\n", error);
 		return error;
 	}
 
-- 
2.30.2

Re: [PATCH 3/6] Input: zinitix - Handle proper supply names

[Luca Weiss]

Hi Nikita,

On Mittwoch, 27. Oktober 2021 20:13:47 CEST Nikita Travkin wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> The supply names of the Zinitix touchscreen were a bit confused, the new
> bindings rectifies this.
> 
> To deal with old and new devicetrees, first check if we have "vddo" and in
> case that exists assume the old supply names. Else go and look for the new
> ones.
> 
> We cannot just get the regulators since we would get an OK and a dummy
> regulator: we need to check explicitly for the old supply name.
> 
> Use struct device *dev as a local variable instead of the I2C client since
> the device is what we are actually obtaining the resources from.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: Michael Srba <Michael.Srba@seznam.cz>
> Cc: phone-devel@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> [Slightly changed the legacy regulator detection]
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
> This patch was previously submitted here:
> https://lore.kernel.org/linux-input/20210625113435.2539282-2-linus.walleij@l
> inaro.org/
> 
> Changes since the original patch:
>  - Adress the review comments by Dmitry:
>    Drop explict OF check and use of_find_property()
> ---
>  drivers/input/touchscreen/zinitix.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/zinitix.c
> b/drivers/input/touchscreen/zinitix.c index 1e70b8d2a8d7..d4e06a88a883
> 100644
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -252,16 +252,27 @@ static int zinitix_init_touch(struct bt541_ts_data
> *bt541)
> 
>  static int zinitix_init_regulators(struct bt541_ts_data *bt541)
>  {
> -	struct i2c_client *client = bt541->client;
> +	struct device *dev = &bt541->client->dev;
>  	int error;
> 
> -	bt541->supplies[0].supply = "vdd";
> -	bt541->supplies[1].supply = "vddo";
> -	error = devm_regulator_bulk_get(&client->dev,
> +	/*
> +	 * Some older device trees have erroneous names for the regulators,
> +	 * so check if "vddo" is present and in that case use these names
> +	 * and warn. Else use the proper supply names on the component.
> +	 */

Nitpick, but:
"and in that case use these names and warn."
I don't see any dev_warn or anything that would 'warn'. If you send a v2 it 
might be nice to fix that.

Regards
Luca

> +	if (of_find_property(dev->of_node, "vddo-supply", NULL)) {
> +		bt541->supplies[0].supply = "vdd";
> +		bt541->supplies[1].supply = "vddo";
> +	} else {
> +		/* Else use the proper supply names */
> +		bt541->supplies[0].supply = "vcca";
> +		bt541->supplies[1].supply = "vdd";
> +	}
> +	error = devm_regulator_bulk_get(dev,
>  					ARRAY_SIZE(bt541-
>supplies),
>  					bt541->supplies);
>  	if (error < 0) {
> -		dev_err(&client->dev, "Failed to get regulators: %d\n", 
error);
> +		dev_err(dev, "Failed to get regulators: %d\n", error);
>  		return error;
>  	}

[PATCH 4/6] input: touchscreen: zinitix: Add compatible for bt532

[Nikita Travkin]

Zinitix BT532 is another touch controller that seem to implement the
same interface as an already supported BT541. Add it to the driver.

Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/input/touchscreen/zinitix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index d4e06a88a883..c67ff8be0df1 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -571,6 +571,7 @@ static SIMPLE_DEV_PM_OPS(zinitix_pm_ops, zinitix_suspend, zinitix_resume);
 
 #ifdef CONFIG_OF
 static const struct of_device_id zinitix_of_match[] = {
+	{ .compatible = "zinitix,bt532" },
 	{ .compatible = "zinitix,bt541" },
 	{ }
 };
-- 
2.30.2

Re: [PATCH 4/6] input: touchscreen: zinitix: Add compatible for bt532

[Linus Walleij]

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:

> Zinitix BT532 is another touch controller that seem to implement the
> same interface as an already supported BT541. Add it to the driver.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

[PATCH 5/6] dt-bindings: input: zinitix: Document touch-keys support

[Nikita Travkin]

In some configrations the touch controller can support the touch-keys.
Doucument the linux,keycodes property that enables those keys and
specifies the keycodes that should be used to report the key events.

Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 .../bindings/input/touchscreen/zinitix,bt400.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
index b4e5ba7c0b49..40b243c07fd4 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
@@ -79,6 +79,14 @@ properties:
     $ref: /schemas/types.yaml#/definitions/uint32
     enum: [1, 2]
 
+  linux,keycodes:
+    description: |
+      This property specifies an array of keycodes assigned to the
+      touch-keys that can be present in some touchscreen configurations.
+    $ref: /schemas/input/input.yaml#/properties/linux,keycodes
+    minItems: 1
+    maxItems: 8
+
   touchscreen-size-x: true
   touchscreen-size-y: true
   touchscreen-fuzz-x: true
-- 
2.30.2

Re: [PATCH 5/6] dt-bindings: input: zinitix: Document touch-keys support

[Luca Weiss]

Hi Nikita,

On Mittwoch, 27. Oktober 2021 20:13:49 CEST Nikita Travkin wrote:
> In some configrations the touch controller can support the touch-keys.
> Doucument the linux,keycodes property that enables those keys and
> specifies the keycodes that should be used to report the key events.
> 
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
>  .../bindings/input/touchscreen/zinitix,bt400.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> index b4e5ba7c0b49..40b243c07fd4 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> +++
> b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml @@
> -79,6 +79,14 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [1, 2]
> 
> +  linux,keycodes:
> +    description: |
> +      This property specifies an array of keycodes assigned to the
> +      touch-keys that can be present in some touchscreen configurations.
> +    $ref: /schemas/input/input.yaml#/properties/linux,keycodes
> +    minItems: 1

I think this would suggest all devices using the driver must have at least 
keycode declared which doesn't seem to be the desired behavior?

Regards,
Luca

> +    maxItems: 8
> +
>    touchscreen-size-x: true
>    touchscreen-size-y: true
>    touchscreen-fuzz-x: true

Re: [PATCH 5/6] dt-bindings: input: zinitix: Document touch-keys support

[Nikita Travkin]

Hi Luca,

Luca Weiss писал(а) 27.10.2021 23:33:
> Hi Nikita,
> 
> On Mittwoch, 27. Oktober 2021 20:13:49 CEST Nikita Travkin wrote:
>> In some configrations the touch controller can support the touch-keys.
>> Doucument the linux,keycodes property that enables those keys and
>> specifies the keycodes that should be used to report the key events.
>> 
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
>> ---
>>  .../bindings/input/touchscreen/zinitix,bt400.yaml         | 8 
>> ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git
>> a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
>> b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
>> index b4e5ba7c0b49..40b243c07fd4 100644
>> --- 
>> a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
>> +++
>> b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml 
>> @@
>> -79,6 +79,14 @@ properties:
>>      $ref: /schemas/types.yaml#/definitions/uint32
>>      enum: [1, 2]
>> 
>> +  linux,keycodes:
>> +    description: |
>> +      This property specifies an array of keycodes assigned to the
>> +      touch-keys that can be present in some touchscreen 
>> configurations.
>> +    $ref: /schemas/input/input.yaml#/properties/linux,keycodes
>> +    minItems: 1
> 
> I think this would suggest all devices using the driver must have at 
> least
> keycode declared which doesn't seem to be the desired behavior?

As I haven't added the property to the "required" list I'd read this as
"If the property is defined it must contain from one to eight elements"
so I think that *technically* the schema is correct. (Otherwise the
dt_schema_check would fail verifying the example) However this indeed
may be true that I could give a better description than "can be present
in some touchscreen configurations."

Thanks,
Nikita

> Regards,
> Luca
> 
>> +    maxItems: 8
>> +
>>    touchscreen-size-x: true
>>    touchscreen-size-y: true
>>    touchscreen-fuzz-x: true

Re: [PATCH 5/6] dt-bindings: input: zinitix: Document touch-keys support

[Rob Herring]

On Wed, Oct 27, 2021 at 11:13:49PM +0500, Nikita Travkin wrote:
> In some configrations the touch controller can support the touch-keys.
> Doucument the linux,keycodes property that enables those keys and
> specifies the keycodes that should be used to report the key events.
> 
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> ---
>  .../bindings/input/touchscreen/zinitix,bt400.yaml         | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> index b4e5ba7c0b49..40b243c07fd4 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
> @@ -79,6 +79,14 @@ properties:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      enum: [1, 2]
>  
> +  linux,keycodes:
> +    description: |
> +      This property specifies an array of keycodes assigned to the
> +      touch-keys that can be present in some touchscreen configurations.
> +    $ref: /schemas/input/input.yaml#/properties/linux,keycodes

Instead, reference input.yaml at the top-level.

> +    minItems: 1
> +    maxItems: 8
> +
>    touchscreen-size-x: true
>    touchscreen-size-y: true
>    touchscreen-fuzz-x: true
> -- 
> 2.30.2
> 
> 

[PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

[Nikita Travkin]

Zinitix touch controllers can use some of the sense lines for virtual
keys (like those found on many phones). Add support for those keys.

Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/input/touchscreen/zinitix.c | 61 +++++++++++++++++++++++++++--
 1 file changed, 58 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index c67ff8be0df1..3928cb743a1c 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -119,6 +119,7 @@
 
 #define DEFAULT_TOUCH_POINT_MODE		2
 #define MAX_SUPPORTED_FINGER_NUM		5
+#define MAX_SUPPORTED_BUTTON_NUM		8
 
 #define CHIP_ON_DELAY				15 // ms
 #define FIRMWARE_ON_DELAY			40 // ms
@@ -146,6 +147,8 @@ struct bt541_ts_data {
 	struct touchscreen_properties prop;
 	struct regulator_bulk_data supplies[2];
 	u32 zinitix_mode;
+	u32 keycodes[MAX_SUPPORTED_BUTTON_NUM];
+	int num_keycodes;
 };
 
 static int zinitix_read_data(struct i2c_client *client,
@@ -195,6 +198,7 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	struct i2c_client *client = bt541->client;
 	int i;
 	int error;
+	u16 int_flags = 0;
 
 	error = zinitix_write_cmd(client, BT541_SWRESET_CMD);
 	if (error) {
@@ -225,6 +229,11 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	if (error)
 		return error;
 
+	error = zinitix_write_u16(client, BT541_BUTTON_SUPPORTED_NUM,
+				  bt541->num_keycodes);
+	if (error)
+		return error;
+
 	error = zinitix_write_u16(client, BT541_INITIAL_TOUCH_MODE,
 				  bt541->zinitix_mode);
 	if (error)
@@ -235,9 +244,12 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG,
-				  BIT_PT_CNT_CHANGE | BIT_DOWN | BIT_MOVE |
-					BIT_UP);
+	int_flags = BIT_PT_CNT_CHANGE | BIT_DOWN | BIT_MOVE | BIT_UP;
+
+	if (bt541->num_keycodes)
+		int_flags |= BIT_ICON_EVENT;
+
+	error = zinitix_write_u16(client, BT541_INT_ENABLE_FLAG, int_flags);
 	if (error)
 		return error;
 
@@ -329,6 +341,15 @@ static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot,
 	input_report_abs(bt541->input_dev, ABS_MT_TOUCH_MAJOR, p->width);
 }
 
+static void zinitix_report_keys(struct bt541_ts_data *bt541, __le16 icon_events)
+{
+	int i;
+
+	for (i = 0; i < bt541->num_keycodes; i++)
+		input_report_key(bt541->input_dev,
+				 bt541->keycodes[i], !!(icon_events & BIT(i)));
+}
+
 static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 {
 	struct bt541_ts_data *bt541 = bt541_handler;
@@ -336,6 +357,7 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 	struct touch_event touch_event;
 	int error;
 	int i;
+	__le16 icon_events = 0;
 
 	memset(&touch_event, 0, sizeof(struct touch_event));
 
@@ -346,6 +368,17 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 		goto out;
 	}
 
+	if (touch_event.status & BIT_ICON_EVENT) {
+		error = zinitix_read_data(bt541->client, BT541_ICON_STATUS_REG,
+					  &icon_events, sizeof(icon_events));
+		if (error) {
+			dev_err(&client->dev, "Failed to read icon events\n");
+			goto out;
+		}
+
+		zinitix_report_keys(bt541, icon_events);
+	}
+
 	for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++)
 		if (touch_event.point_coord[i].sub_status & SUB_BIT_EXIST)
 			zinitix_report_finger(bt541, i,
@@ -427,6 +460,7 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
 {
 	struct input_dev *input_dev;
 	int error;
+	int i;
 
 	input_dev = devm_input_allocate_device(&bt541->client->dev);
 	if (!input_dev) {
@@ -444,6 +478,14 @@ static int zinitix_init_input_dev(struct bt541_ts_data *bt541)
 	input_dev->open = zinitix_input_open;
 	input_dev->close = zinitix_input_close;
 
+	if (bt541->num_keycodes) {
+		input_dev->keycode = bt541->keycodes;
+		input_dev->keycodemax = bt541->num_keycodes;
+		input_dev->keycodesize = sizeof(bt541->keycodes[0]);
+		for (i = 0; i < bt541->num_keycodes; i++)
+			input_set_capability(input_dev, EV_KEY, bt541->keycodes[i]);
+	}
+
 	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
 	input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
 	input_set_abs_params(input_dev, ABS_MT_WIDTH_MAJOR, 0, 255, 0, 0);
@@ -508,6 +550,19 @@ static int zinitix_ts_probe(struct i2c_client *client)
 		return error;
 	}
 
+	bt541->num_keycodes = of_property_read_variable_u32_array(
+					client->dev.of_node, "linux,keycodes",
+					bt541->keycodes, 0,
+					ARRAY_SIZE(bt541->keycodes));
+	if (bt541->num_keycodes == -EINVAL) {
+		bt541->num_keycodes = 0;
+	} else if (bt541->num_keycodes < 0) {
+		dev_err(&client->dev,
+			"Unable to parse \"linux,keycodes\" property: %d\n",
+			bt541->num_keycodes);
+		return bt541->num_keycodes;
+	}
+
 	error = zinitix_init_input_dev(bt541);
 	if (error) {
 		dev_err(&client->dev,
-- 
2.30.2

Re: [PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

[Linus Walleij]

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:

> Zinitix touch controllers can use some of the sense lines for virtual
> keys (like those found on many phones). Add support for those keys.
>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>

Nice!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

Re: [PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2126 bytes --]

Hi Nikita,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on robh/for-next hid/for-next linus/master v5.15 next-20211109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nikita-Travkin/Add-touch-keys-support-to-the-Zinitix-touch-driver/20211028-031652
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s001-20211027 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/4d4045dad42ed26b0dba61827ffddcd453601895
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nikita-Travkin/Add-touch-keys-support-to-the-Zinitix-touch-driver/20211028-031652
        git checkout 4d4045dad42ed26b0dba61827ffddcd453601895
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/input/touchscreen/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/input/touchscreen/zinitix.c:350:57: sparse: sparse: restricted __le16 degrades to integer
   drivers/input/touchscreen/zinitix.c:371:24: sparse: sparse: restricted __le16 degrades to integer

vim +350 drivers/input/touchscreen/zinitix.c

   343	
   344	static void zinitix_report_keys(struct bt541_ts_data *bt541, __le16 icon_events)
   345	{
   346		int i;
   347	
   348		for (i = 0; i < bt541->num_keycodes; i++)
   349			input_report_key(bt541->input_dev,
 > 350					 bt541->keycodes[i], !!(icon_events & BIT(i)));
   351	}
   352	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 36037 bytes --]

Re: [PATCH 6/6] input: touchscreen: zinitix: Add touchkey support

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

Hi Nikita,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on dtor-input/next]
[also build test WARNING on robh/for-next hid/for-next linus/master v5.15 next-20211109]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Nikita-Travkin/Add-touch-keys-support-to-the-Zinitix-touch-driver/20211028-031652
base:   https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
config: i386-randconfig-s001-20211027 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.4-dirty
        # https://github.com/0day-ci/linux/commit/4d4045dad42ed26b0dba61827ffddcd453601895
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Nikita-Travkin/Add-touch-keys-support-to-the-Zinitix-touch-driver/20211028-031652
        git checkout 4d4045dad42ed26b0dba61827ffddcd453601895
        # save the attached .config to linux build tree
        make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/input/touchscreen/

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
>> drivers/input/touchscreen/zinitix.c:350:57: sparse: sparse: restricted __le16 degrades to integer
   drivers/input/touchscreen/zinitix.c:371:24: sparse: sparse: restricted __le16 degrades to integer

vim +350 drivers/input/touchscreen/zinitix.c

   343	
   344	static void zinitix_report_keys(struct bt541_ts_data *bt541, __le16 icon_events)
   345	{
   346		int i;
   347	
   348		for (i = 0; i < bt541->num_keycodes; i++)
   349			input_report_key(bt541->input_dev,
 > 350					 bt541->keycodes[i], !!(icon_events & BIT(i)));
   351	}
   352	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 36037 bytes --]

Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[Linus Walleij]

Hi Nikita,

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:

> This series adds support for the touch-keys that can be present on some
> touchscreen configurations, adds the compatible for bt532 and fixes a
> small race condition bug in the driver probe function.
>
> I also pick up the series that converts the dt bindings to yaml
> initially submitted by Linus Walleij in [1].
> I made some minor changes to those patches:
>  - Fixed dt_schema_check error
>  - Adressed the review comments from Dmitry on the original series

Thanks for picking this up!

Have you notices some behaviour like surplus touch events
(like many press/release events fall through to the UI)
when using this driver? I think it might need some z fuzzing
but I am not sure.

Yours,
Linus Walleij

Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[Nikita Travkin]

Hi Linus,

Linus Walleij писал(а) 09.11.2021 09:45:
> Hi Nikita,
> 
> On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:
> 
>> This series adds support for the touch-keys that can be present on 
>> some
>> touchscreen configurations, adds the compatible for bt532 and fixes a
>> small race condition bug in the driver probe function.
>> 
>> I also pick up the series that converts the dt bindings to yaml
>> initially submitted by Linus Walleij in [1].
>> I made some minor changes to those patches:
>>  - Fixed dt_schema_check error
>>  - Adressed the review comments from Dmitry on the original series
> 
> Thanks for picking this up!
> 
> Have you notices some behaviour like surplus touch events
> (like many press/release events fall through to the UI)
> when using this driver? I think it might need some z fuzzing
> but I am not sure.
> 

On my device (8 inch tablet with BT532) I saw no problems with touch
so far. However another person with a different tablet (10 inch with 
ZT7554)
indeed says that they notice "multiplied" touches that make typing hard
so maybe that depends on controller model/firmware...

And speaking of that ZT7554: Seems like it's works with the driver
and I'd like to add the compatible for it in v2 but I'd also have to add 
it
to the bindings. Looking at how you add all other similar names for BT* 
there
does it make sense to add ZT* as well? Maybe you have some hints where 
to look
for a list of the models?

I was planning to send a v2 with all the review fixes near the end of 
the week
but I've noticed a yet another quirky issue with the touch controller:
At least on my device, for some reason enabling touchkeys changes the 
way the
controller reports the finger touch events which breaks multi-touch...
Assuming that *not* enabling the touchkeys leads to calibration being 
wrong
(controller assigns the touchkey sense lines to the touch area in that 
case)
I now have to resolve this quirk as well...

Thanks,
Nikita

> Yours,
> Linus Walleij

Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[Linus Walleij]

On Tue, Nov 9, 2021 at 4:23 PM Nikita Travkin <nikita@trvn.ru> wrote:
> [Me]
> > Have you notices some behaviour like surplus touch events
> > (like many press/release events fall through to the UI)
> > when using this driver? I think it might need some z fuzzing
> > but I am not sure.
> >
>
> On my device (8 inch tablet with BT532) I saw no problems with touch
> so far. However another person with a different tablet (10 inch with
> ZT7554)
> indeed says that they notice "multiplied" touches that make typing hard
> so maybe that depends on controller model/firmware...

It may even be depending on specimen. I saw that the vendor driver
does contain some debouncing code.

> And speaking of that ZT7554: Seems like it's works with the driver
> and I'd like to add the compatible for it in v2 but I'd also have to add
> it to the bindings. Looking at how you add all other similar names for BT*
> there does it make sense to add ZT* as well?

Yeah probably, if they are electrically very similar.

> Maybe you have some hints where
> to look for a list of the models?

I usually google ... try to find things like powerpoints with roadmaps
from the vendor and things like that. Best thing is if they answer
to mail but I don't know if Zinitix are even around anymore.

> I've noticed a yet another quirky issue with the touch controller:
> At least on my device, for some reason enabling touchkeys changes the
> way the
> controller reports the finger touch events which breaks multi-touch...
> Assuming that *not* enabling the touchkeys leads to calibration being
> wrong
> (controller assigns the touchkey sense lines to the touch area in that
> case)
> I now have to resolve this quirk as well...

Hm yeah I guess refer to the (messy) vendor driver for hints.

Yours,
Linus Walleij

Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[Linus Walleij]

Hi Nikita,

On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:

> This series adds support for the touch-keys that can be present on some
> touchscreen configurations, adds the compatible for bt532 and fixes a
> small race condition bug in the driver probe function.

This appears unaddressed since October?
I see there are just some small nits in patch 5 & 6 to fix, then
it is finished.

Do you have time to pick it up for kernel v5.17 instead?
Make sure to collect all Reviewed-by on this series.

Yours,
Linus Walleij

Re: [PATCH 0/6] Add touch-keys support to the Zinitix touch driver

[Nikita Travkin]

Linus Walleij писал(а) 05.01.2022 02:04:
> Hi Nikita,
> 
> On Wed, Oct 27, 2021 at 8:15 PM Nikita Travkin <nikita@trvn.ru> wrote:
> 
>> This series adds support for the touch-keys that can be present on some
>> touchscreen configurations, adds the compatible for bt532 and fixes a
>> small race condition bug in the driver probe function.
> 
> This appears unaddressed since October?
> I see there are just some small nits in patch 5 & 6 to fix, then
> it is finished.
> 

Hi, I was planning to include the fix for the message reporting
to the next version as well but then I got rather low on time
and could never finish that bit. As it seem to only affect my
device, there was not really much stopping me from submitting
a next version without that fix other than my "irrational
perfectionism" which I should probably learn to recognize better...

> Do you have time to pick it up for kernel v5.17 instead?
> Make sure to collect all Reviewed-by on this series.
> 

I will try to submit a new version with review fixes and
tags shortly.

Thanks,
Nikita

> Yours,
> Linus Walleij