[PATCH v3 0/4] Input: hynitron-cst816x: axes and gesture keys

[PATCH v3 0/4] Input: hynitron-cst816x: axes and gesture keys

[Daniel Golle]

This series improves the hynitron-cst816x touchscreen driver. It makes
the driver respect the common touchscreen axis properties, which the
binding now describes along with the rest of touchscreen.yaml, and it
keeps gesture keys from getting stuck.

The gesture key is reported with the controller's touch flag as its
value, so the key stays pressed once the controller stops reporting
the gesture code. The controller has no lift event to offer, so the
keys are now released on the first report carrying no gesture code, or
40 ms after the last one that carried a code, whichever comes first.

Changes since v2:
- the binding describes the whole of touchscreen.yaml through
  unevaluatedProperties (Krzysztof Kozlowski)
- the panel follower patch is dropped. It serves a panel that
  registers a drm_panel, while a MIPI DBI display driven by
  panel-mipi-dbi owns the DRM device itself and registers none, so on
  such a board the lookup defers for ever and the touchscreen never
  probes.
- the release timer is stopped with timer_shutdown_sync() in the
  devres action (Sashiko AI review)

Changes since v1:
- the binding allows touchscreen-min-x/y, touchscreen-size-x/y and
  touchscreen-fuzz-x/y as well, which touchscreen_parse_properties()
  already honours; the 240x240 default is unchanged
- the binding allows the 'panel' phandle, and patch 5 is new: it
  follows the panel's power sequence
- the binding commit message carries a justification (Conor Dooley)
- keycodes above KEY_MAX are rejected, which keeps the gesture key
  lookup inside the input core's key bitmap (Sashiko AI review)
- the timed release is skipped when the interrupt handler re-armed the
  timer while the timer callback waited for the lock (Sashiko AI
  review)
- the Sashiko AI review also asked about additionalProperties next to
  a $ref, which stays, as this binding allows a subset of
  touchscreen.yaml (writing-schema.rst)

Link: https://lore.kernel.org/r/cover.1790851590.git.daniel@makrotopia.org
Link: https://lore.kernel.org/r/cover.1790944534.git.daniel@makrotopia.org

Daniel Golle (4):
  dt-bindings: input: touchscreen: hynitron,cst816x: axes and panel
  Input: hynitron-cst816x: respect touchscreen DT properties
  Input: hynitron-cst816x: release gesture keys
  Input: hynitron-cst816x: time out gesture key release

 .../input/touchscreen/hynitron,cst816x.yaml   |   6 +-
 drivers/input/touchscreen/hynitron-cst816x.c  | 122 ++++++++++++++----
 2 files changed, 103 insertions(+), 25 deletions(-)


base-commit: 6474fa070f2b8013b4b87350b775b8c3be6e8aac
-- 
2.56.0

[PATCH v3 1/4] dt-bindings: input: touchscreen: hynitron,cst816x: axes and panel

[Daniel Golle]

The chip hands out coordinates in its own frame and knows nothing of
the panel it is glued to, so how it is oriented against that panel,
which coordinate range the assembly reports and how noisy it is are
properties of the board. Reference touchscreen.yaml and let its
properties through, among them the 'panel' phandle naming the panel a
controller is mounted on, which asks for the two to be power sequenced
together.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v3: describe the whole of touchscreen.yaml with unevaluatedProperties
v2:
- allow the min, size and fuzz properties as well (Sashiko AI review)
- allow the 'panel' phandle
- justify the change in the commit message (Conor Dooley)

 .../bindings/input/touchscreen/hynitron,cst816x.yaml        | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
index 72d4da636881e..72b1bbff6e57b 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
@@ -41,7 +41,10 @@ required:
   - reg
   - interrupts
 
-additionalProperties: false
+unevaluatedProperties: false
+
+allOf:
+  - $ref: touchscreen.yaml#
 
 examples:
   - |
@@ -59,6 +62,7 @@ examples:
             reset-gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
             linux,keycodes = <KEY_UP>, <KEY_DOWN>, <KEY_LEFT>, <KEY_RIGHT>,
                              <BTN_TOOL_TRIPLETAP>;
+            touchscreen-swapped-x-y;
         };
     };
 
-- 
2.56.0

[PATCH v3 2/4] Input: hynitron-cst816x: respect touchscreen DT properties

[Daniel Golle]

Parse common touchscreen properties and use touchscreen_report_pos()
instead of reporting raw absolute position.

This makes the driver respect the touchscreen-min-x/y,
touchscreen-size-x/y, touchscreen-fuzz-x/y, touchscreen-inverted-x/y
and touchscreen-swapped-x-y properties.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2:
- name all the properties the parser honours, no functional change

 drivers/input/touchscreen/hynitron-cst816x.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
index 47d9cd7412d1b..865c5a767ba01 100644
--- a/drivers/input/touchscreen/hynitron-cst816x.c
+++ b/drivers/input/touchscreen/hynitron-cst816x.c
@@ -11,6 +11,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/i2c.h>
 #include <linux/input.h>
+#include <linux/input/touchscreen.h>
 #include <linux/unaligned.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
@@ -31,6 +32,7 @@ struct cst816x_priv {
 	struct input_dev *input;
 	unsigned int keycode[CST816X_NUM_KEYS];
 	unsigned int keycodemax;
+	struct touchscreen_properties prop;
 };
 
 static int cst816x_parse_keycodes(struct device *dev, struct cst816x_priv *priv)
@@ -142,6 +144,7 @@ static int cst816x_register_input(struct cst816x_priv *priv)
 	input_set_abs_params(priv->input, ABS_X, 0, 240, 0, 0);
 	input_set_abs_params(priv->input, ABS_Y, 0, 240, 0, 0);
 	input_set_capability(priv->input, EV_KEY, BTN_TOUCH);
+	touchscreen_parse_properties(priv->input, false, &priv->prop);
 
 	priv->input->keycode = priv->keycode;
 	priv->input->keycodesize = sizeof(priv->keycode[0]);
@@ -173,8 +176,8 @@ static irqreturn_t cst816x_irq_cb(int irq, void *cookie)
 	if (!cst816x_process_touch(priv, &tch))
 		return IRQ_HANDLED;
 
-	input_report_abs(priv->input, ABS_X, tch.abs_x);
-	input_report_abs(priv->input, ABS_Y, tch.abs_y);
+	touchscreen_report_pos(priv->input, &priv->prop,
+			       tch.abs_x, tch.abs_y, false);
 
 	if (tch.gest)
 		input_report_key(priv->input,
-- 
2.56.0

[PATCH v3 3/4] Input: hynitron-cst816x: release gesture keys

[Daniel Golle]

The gesture key is reported with the value of the touch flag, so it
stays pressed once the controller stops reporting the gesture code
while the finger is still down. Report the press on its own and
release the keys the input core still holds down on the first report
that carries no gesture code. Keycodes above KEY_MAX are rejected,
as that release looks the key state up in the input core's key bitmap.

Fixes: c87a819bec86 ("Input: add driver for Hynitron CST816x series")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v2:
- reject keycodes above KEY_MAX and publish keycodemax only once the
  whole array validates (Sashiko AI review)

 drivers/input/touchscreen/hynitron-cst816x.c | 67 ++++++++++++++------
 1 file changed, 46 insertions(+), 21 deletions(-)

diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
index 865c5a767ba01..725c8bd4c1d0e 100644
--- a/drivers/input/touchscreen/hynitron-cst816x.c
+++ b/drivers/input/touchscreen/hynitron-cst816x.c
@@ -39,28 +39,40 @@ static int cst816x_parse_keycodes(struct device *dev, struct cst816x_priv *priv)
 {
 	int count;
 	int error;
+	int i;
 
-	if (device_property_present(dev, "linux,keycodes")) {
-		count = device_property_count_u32(dev, "linux,keycodes");
-		if (count < 0) {
-			error = count;
-			dev_err(dev, "failed to count keys: %d\n", error);
-			return error;
-		} else if (count > ARRAY_SIZE(priv->keycode)) {
-			dev_err(dev, "too many keys defined: %d\n", count);
-			return -EINVAL;
-		}
-		priv->keycodemax = count;
-
-		error = device_property_read_u32_array(dev, "linux,keycodes",
-						       priv->keycode,
-						       priv->keycodemax);
-		if (error) {
-			dev_err(dev, "failed to read keycodes: %d\n", error);
-			return error;
-		}
+	if (!device_property_present(dev, "linux,keycodes"))
+		return 0;
+
+	count = device_property_count_u32(dev, "linux,keycodes");
+	if (count < 0) {
+		error = count;
+		dev_err(dev, "failed to count keys: %d\n", error);
+		return error;
+	}
+
+	if (count > ARRAY_SIZE(priv->keycode)) {
+		dev_err(dev, "too many keys defined: %d\n", count);
+		return -EINVAL;
+	}
+
+	error = device_property_read_u32_array(dev, "linux,keycodes",
+					       priv->keycode, count);
+	if (error) {
+		dev_err(dev, "failed to read keycodes: %d\n", error);
+		return error;
 	}
 
+	for (i = 0; i < count; i++) {
+		if (priv->keycode[i] <= KEY_MAX)
+			continue;
+
+		dev_err(dev, "keycode %u out of range\n", priv->keycode[i]);
+		return -EINVAL;
+	}
+
+	priv->keycodemax = count;
+
 	return 0;
 }
 
@@ -168,6 +180,18 @@ static void cst816x_reset(struct cst816x_priv *priv)
 	msleep(100);
 }
 
+static void cst816x_release_keys(struct cst816x_priv *priv)
+{
+	unsigned int i;
+
+	for (i = 0; i < priv->keycodemax; i++) {
+		if (!test_bit(priv->keycode[i], priv->input->key))
+			continue;
+
+		input_report_key(priv->input, priv->keycode[i], 0);
+	}
+}
+
 static irqreturn_t cst816x_irq_cb(int irq, void *cookie)
 {
 	struct cst816x_priv *priv = cookie;
@@ -181,8 +205,9 @@ static irqreturn_t cst816x_irq_cb(int irq, void *cookie)
 
 	if (tch.gest)
 		input_report_key(priv->input,
-				 priv->keycode[cst816x_gest_idx(tch.gest)],
-				 tch.active);
+				 priv->keycode[cst816x_gest_idx(tch.gest)], 1);
+	else
+		cst816x_release_keys(priv);
 
 	input_report_key(priv->input, BTN_TOUCH, tch.active);
 
-- 
2.56.0

[PATCH v3 4/4] Input: hynitron-cst816x: time out gesture key release

[Daniel Golle]

The controller reports a gesture code once and can then stay quiet
until the next touch, which leaves the gesture key pressed for as long
as the panel is left alone. Release the gesture keys 40 ms after the
last report that carried a gesture code, so that the release comes
from whichever happens first, the timeout or a report without a
gesture code. The 40 ms matches TSC200X_PENUP_TIME_MS, chosen there
for the same want of a lift event from the controller, and has to stay
above the interval between consecutive reports of a held gesture.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
v3: use timer_shutdown_sync() in the devres teardown action
    (Sashiko AI review)
v2: skip the release when the interrupt handler re-armed the timer
    while the timer callback waited for the lock (Sashiko AI review)

 drivers/input/touchscreen/hynitron-cst816x.c | 52 ++++++++++++++++++--
 1 file changed, 49 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
index 725c8bd4c1d0e..bd84581cf2b6d 100644
--- a/drivers/input/touchscreen/hynitron-cst816x.c
+++ b/drivers/input/touchscreen/hynitron-cst816x.c
@@ -15,9 +15,12 @@
 #include <linux/unaligned.h>
 #include <linux/interrupt.h>
 #include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/timer.h>
 
 #define CST816X_RD_REG		0x01
 #define CST816X_NUM_KEYS	5
+#define CST816X_GEST_HOLD_MS	40
 
 struct cst816x_touch {
 	u8 gest;
@@ -32,6 +35,9 @@ struct cst816x_priv {
 	struct input_dev *input;
 	unsigned int keycode[CST816X_NUM_KEYS];
 	unsigned int keycodemax;
+	struct timer_list release_timer;
+	/* lock keeps the timeout out of the middle of a report */
+	spinlock_t lock;
 	struct touchscreen_properties prop;
 };
 
@@ -180,8 +186,9 @@ static void cst816x_reset(struct cst816x_priv *priv)
 	msleep(100);
 }
 
-static void cst816x_release_keys(struct cst816x_priv *priv)
+static bool cst816x_release_keys(struct cst816x_priv *priv)
 {
+	bool released = false;
 	unsigned int i;
 
 	for (i = 0; i < priv->keycodemax; i++) {
@@ -189,33 +196,66 @@ static void cst816x_release_keys(struct cst816x_priv *priv)
 			continue;
 
 		input_report_key(priv->input, priv->keycode[i], 0);
+		released = true;
 	}
+
+	return released;
+}
+
+static void cst816x_release_timeout(struct timer_list *t)
+{
+	struct cst816x_priv *priv = timer_container_of(priv, t, release_timer);
+	unsigned long flags;
+
+	spin_lock_irqsave(&priv->lock, flags);
+
+	/* a re-arm while this waited for the lock supersedes the timeout */
+	if (!timer_pending(&priv->release_timer) && cst816x_release_keys(priv))
+		input_sync(priv->input);
+
+	spin_unlock_irqrestore(&priv->lock, flags);
 }
 
 static irqreturn_t cst816x_irq_cb(int irq, void *cookie)
 {
 	struct cst816x_priv *priv = cookie;
 	struct cst816x_touch tch;
+	unsigned long flags;
 
 	if (!cst816x_process_touch(priv, &tch))
 		return IRQ_HANDLED;
 
+	spin_lock_irqsave(&priv->lock, flags);
+
 	touchscreen_report_pos(priv->input, &priv->prop,
 			       tch.abs_x, tch.abs_y, false);
 
-	if (tch.gest)
+	if (tch.gest) {
 		input_report_key(priv->input,
 				 priv->keycode[cst816x_gest_idx(tch.gest)], 1);
-	else
+		mod_timer(&priv->release_timer,
+			  jiffies + msecs_to_jiffies(CST816X_GEST_HOLD_MS));
+	} else {
+		timer_delete(&priv->release_timer);
 		cst816x_release_keys(priv);
+	}
 
 	input_report_key(priv->input, BTN_TOUCH, tch.active);
 
 	input_sync(priv->input);
 
+	spin_unlock_irqrestore(&priv->lock, flags);
+
 	return IRQ_HANDLED;
 }
 
+static void cst816x_timer_stop(void *data)
+{
+	struct cst816x_priv *priv = data;
+
+	timer_shutdown_sync(&priv->release_timer);
+}
+
 static int cst816x_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -227,6 +267,8 @@ static int cst816x_probe(struct i2c_client *client)
 		return -ENOMEM;
 
 	priv->client = client;
+	spin_lock_init(&priv->lock);
+	timer_setup(&priv->release_timer, cst816x_release_timeout, 0);
 
 	priv->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
 	if (IS_ERR(priv->reset))
@@ -244,6 +286,10 @@ static int cst816x_probe(struct i2c_client *client)
 	if (error)
 		return dev_err_probe(dev, error, "input register failed\n");
 
+	error = devm_add_action_or_reset(dev, cst816x_timer_stop, priv);
+	if (error)
+		return error;
+
 	error = devm_request_threaded_irq(dev, client->irq,
 					  NULL, cst816x_irq_cb, IRQF_ONESHOT,
 					  dev_driver_string(dev), priv);
-- 
2.56.0

Re: [PATCH v3 3/4] Input: hynitron-cst816x: release gesture keys

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] input: TOCTOU race in cst816x_release_keys() due to lockless double-read
--

commit 0b6af957c2d032170de5ed337fe23bb294db578d
Author: Daniel Golle <daniel@makrotopia.org>

Input: hynitron-cst816x: release gesture keys

This commit fixes an issue where gesture keys stay pressed when the
gesture stops but the finger remains down. It adds logic to explicitly
release keys held by the input core on the first non-gesture report.

> diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
> --- a/drivers/input/touchscreen/hynitron-cst816x.c
> +++ b/drivers/input/touchscreen/hynitron-cst816x.c

[ ... ]

> @@ -168,6 +180,18 @@ static void cst816x_reset(struct cst816x_priv *priv)
>  	msleep(100);
>  }
>  
> +static void cst816x_release_keys(struct cst816x_priv *priv)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < priv->keycodemax; i++) {
> +		if (!test_bit(priv->keycode[i], priv->input->key))
> +			continue;
> +
> +		input_report_key(priv->input, priv->keycode[i], 0);

[Severity: Medium]
Is there a race condition here due to reading priv->keycode array elements
twice?

Because the priv->keycode array is shared with the input core and
cst816x_release_keys() does not hold the event_lock, a concurrent
EVIOCSKEYCODE ioctl could modify the array between the test_bit() check
and the input_report_key() call.

If this occurs, could the driver spuriously release a completely different
keycode than the one it just tested?

Would it be safer to read the array element into a local variable first to
ensure the exact same keycode is both evaluated and reported?

> +	}
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1791103511.git.daniel@makrotopia.org?part=3

Re: [PATCH v3 1/4] dt-bindings: input: touchscreen: hynitron,cst816x: axes and panel

[Krzysztof Kozlowski]

On 04/10/2026 11:34, Daniel Golle wrote:
> The chip hands out coordinates in its own frame and knows nothing of
> the panel it is glued to, so how it is oriented against that panel,
> which coordinate range the assembly reports and how noisy it is are
> properties of the board. Reference touchscreen.yaml and let its
> properties through, among them the 'panel' phandle naming the panel a
> controller is mounted on, which asks for the two to be power sequenced
> together.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
> v3: describe the whole of touchscreen.yaml with unevaluatedProperties


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>



Best regards,
Krzysztof