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

[PATCH v5 0/2] 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.

To: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Michael Srba <Michael.Srba@seznam.cz>
To: Linus Walleij <linus.walleij@linaro.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: linux-input@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Nikita Travkin <nikita@trvn.ru>

Changes in v5:
- Switch to using device_property_ (Dmitry)
- Link to v4: https://lore.kernel.org/r/20230801-zinitix-tkey-v4-0-b85526c5a474@trvn.ru

Changes in v4:
- The series was partially applied, these patches dropped.
- Link to v3: https://lore.kernel.org/r/20220106072840.36851-1-nikita@trvn.ru

---
Nikita Travkin (2):
      dt-bindings: input: zinitix: Document touch-keys support
      input: zinitix: Add touchkey support

 .../bindings/input/touchscreen/zinitix,bt400.yaml  | 10 ++++
 drivers/input/touchscreen/zinitix.c                | 63 ++++++++++++++++++++--
 2 files changed, 70 insertions(+), 3 deletions(-)
---
base-commit: 797012914d2d031430268fe512af0ccd7d8e46ef
change-id: 20230801-zinitix-tkey-5a3023bc304c

Best regards,
-- 
Nikita Travkin <nikita@trvn.ru>

[PATCH v5 1/2] dt-bindings: input: zinitix: Document touch-keys support

[Nikita Travkin]

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

Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 .../devicetree/bindings/input/touchscreen/zinitix,bt400.yaml   | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
index b1507463a03e..3f663ce3e44e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/zinitix,bt400.yaml
@@ -16,6 +16,7 @@ maintainers:
 
 allOf:
   - $ref: touchscreen.yaml#
+  - $ref: ../input.yaml#
 
 properties:
   $nodename:
@@ -79,6 +80,15 @@ 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.
+      If the touch-keys are enabled, controller firmware will assign some
+      touch sense lines to those keys.
+    minItems: 1
+    maxItems: 8
+
   touchscreen-size-x: true
   touchscreen-size-y: true
   touchscreen-fuzz-x: true

-- 
2.45.2

Re: [PATCH v5 1/2] dt-bindings: input: zinitix: Document touch-keys support

[Dmitry Torokhov]

On Wed, Jul 17, 2024 at 06:55:33PM +0500, Nikita Travkin wrote:
> In some configurations the touch controller can support the touch-keys.
> Document the linux,keycodes property that enables those keys and
> specifies the keycodes that should be used to report the key events.
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>

Applied, thank you.

-- 
Dmitry

[PATCH v5 2/2] input: 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.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Nikita Travkin <nikita@trvn.ru>
---
 drivers/input/touchscreen/zinitix.c | 63 +++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
index 1b4807ba4624..1df93c96f6bf 100644
--- a/drivers/input/touchscreen/zinitix.c
+++ b/drivers/input/touchscreen/zinitix.c
@@ -10,6 +10,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/property.h>
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 
@@ -119,6 +120,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 +148,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 +199,7 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	struct i2c_client *client = bt541->client;
 	int i;
 	int error;
+	u16 int_flags;
 
 	error = zinitix_write_cmd(client, ZINITIX_SWRESET_CMD);
 	if (error) {
@@ -225,6 +230,11 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	if (error)
 		return error;
 
+	error = zinitix_write_u16(client, ZINITIX_BUTTON_SUPPORTED_NUM,
+				  bt541->num_keycodes);
+	if (error)
+		return error;
+
 	error = zinitix_write_u16(client, ZINITIX_INITIAL_TOUCH_MODE,
 				  bt541->zinitix_mode);
 	if (error)
@@ -235,9 +245,11 @@ static int zinitix_init_touch(struct bt541_ts_data *bt541)
 	if (error)
 		return error;
 
-	error = zinitix_write_u16(client, ZINITIX_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, ZINITIX_INT_ENABLE_FLAG, int_flags);
 	if (error)
 		return error;
 
@@ -350,12 +362,22 @@ static void zinitix_report_finger(struct bt541_ts_data *bt541, int slot,
 	}
 }
 
+static void zinitix_report_keys(struct bt541_ts_data *bt541, u16 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;
 	struct i2c_client *client = bt541->client;
 	struct touch_event touch_event;
 	unsigned long finger_mask;
+	__le16 icon_events;
 	int error;
 	int i;
 
@@ -368,6 +390,17 @@ static irqreturn_t zinitix_ts_irq_handler(int irq, void *bt541_handler)
 		goto out;
 	}
 
+	if (le16_to_cpu(touch_event.status) & BIT_ICON_EVENT) {
+		error = zinitix_read_data(bt541->client, ZINITIX_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, le16_to_cpu(icon_events));
+	}
+
 	finger_mask = touch_event.finger_mask;
 	for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
 		const struct point_coord *p = &touch_event.point_coord[i];
@@ -453,6 +486,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) {
@@ -470,6 +504,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);
@@ -534,6 +576,21 @@ static int zinitix_ts_probe(struct i2c_client *client)
 		return error;
 	}
 
+	bt541->num_keycodes = device_property_count_u32(&client->dev, "linux,keycodes");
+	if (bt541->num_keycodes > ARRAY_SIZE(bt541->keycodes)) {
+		dev_err(&client->dev, "too many keys defined (%d)\n", bt541->num_keycodes);
+		return -EINVAL;
+	}
+
+	error = device_property_read_u32_array(&client->dev, "linux,keycodes",
+					       bt541->keycodes,
+					       bt541->num_keycodes);
+	if (error) {
+		dev_err(&client->dev,
+			"Unable to parse \"linux,keycodes\" property: %d\n", error);
+		return error;
+	}
+
 	error = zinitix_init_input_dev(bt541);
 	if (error) {
 		dev_err(&client->dev,

-- 
2.45.2

Re: [PATCH v5 2/2] input: zinitix: Add touchkey support

[Dmitry Torokhov]

On Wed, Jul 17, 2024 at 06:55:34PM +0500, Nikita Travkin 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.
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Nikita Travkin <nikita@trvn.ru>

Applied, thank you. However:

> -->  
> +	if (le16_to_cpu(touch_event.status) & BIT_ICON_EVENT) {
> +		error = zinitix_read_data(bt541->client, ZINITIX_ICON_STATUS_REG,
> +					  &icon_events, sizeof(icon_events));
> +		if (error) {
> +			dev_err(&client->dev, "Failed to read icon events\n");
> +			goto out;
> +		}

I wonder, would it make sense (and be more efficient) to issue a single
read of size sizeof(struct touch_event) + sizeof(icon_events) and the
parse the data based on touch_event.status?

Thanks.

-- 
Dmitry

Re: [PATCH v5 2/2] input: zinitix: Add touchkey support

[Nikita Travkin]

Dmitry Torokhov писал(а) 20.07.2024 05:29:
> On Wed, Jul 17, 2024 at 06:55:34PM +0500, Nikita Travkin 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.
>>
>> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Nikita Travkin <nikita@trvn.ru>
> 
> Applied, thank you. However:
> 
>> -->
>> +	if (le16_to_cpu(touch_event.status) & BIT_ICON_EVENT) {
>> +		error = zinitix_read_data(bt541->client, ZINITIX_ICON_STATUS_REG,
>> +					  &icon_events, sizeof(icon_events));
>> +		if (error) {
>> +			dev_err(&client->dev, "Failed to read icon events\n");
>> +			goto out;
>> +		}
> 
> I wonder, would it make sense (and be more efficient) to issue a single
> read of size sizeof(struct touch_event) + sizeof(icon_events) and the
> parse the data based on touch_event.status?

Maybe, but I would be really hesitant to such a change: Original driver
also makes a dedicated read for the "icon" data and per my understanding,
those "register reads" may also not be really "register" based but rather
kind of "command" based, where controller will start streaming the data
based on the request for the specific "register". In this case i'd prefer
to not accidentally confuse the touch firmware by over-reading the data,
if its somehow firmware-version-defined.

Thanks for giving it a look and picking this up!
Nikita

> 
> Thanks.