[PATCH 0/4] Input: hynitron-cst816x: axis properties and gesture keys

[PATCH 0/4] Input: hynitron-cst816x: axis properties and gesture keys

[Daniel Golle]

This series improves the hynitron-cst816x touchscreen driver. It makes
the driver respect the common touchscreen axis properties, which are
now documented in the binding as well, 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.

Daniel Golle (4):
  dt-bindings: input: touchscreen: hynitron,cst816x: configure axes
  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   |  8 +++
 drivers/input/touchscreen/hynitron-cst816x.c  | 71 +++++++++++++++++--
 2 files changed, 74 insertions(+), 5 deletions(-)


base-commit: 6474fa070f2b8013b4b87350b775b8c3be6e8aac
-- 
2.55.0

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

[Daniel Golle]

Reference touchscreen.yaml and allow properties
'touchscreen-inverted-x', 'touchscreen-inverted-y' and
'touchscreen-swapped-x-y'.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../bindings/input/touchscreen/hynitron,cst816x.yaml      | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
index 72d4da636881e..ba56bf61038c5 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
@@ -36,6 +36,10 @@ properties:
       - description: Slide right gesture
       - description: Long press gesture
 
+  touchscreen-inverted-x: true
+  touchscreen-inverted-y: true
+  touchscreen-swapped-x-y: true
+
 required:
   - compatible
   - reg
@@ -43,6 +47,9 @@ required:
 
 additionalProperties: false
 
+allOf:
+  - $ref: touchscreen.yaml#
+
 examples:
   - |
     #include <dt-bindings/gpio/gpio.h>
@@ -59,6 +66,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.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] dt-bindings: incorrect use of additionalProperties with $ref
--

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

dt-bindings: input: touchscreen: hynitron,cst816x: configure axes

This commit references touchscreen.yaml and allows the properties
touchscreen-inverted-x, touchscreen-inverted-y, and touchscreen-swapped-x-y.

> diff --git a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
> --- a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
> @@ -36,6 +36,10 @@ properties:
>        - description: Slide right gesture
>        - description: Long press gesture
>  
> +  touchscreen-inverted-x: true
> +  touchscreen-inverted-y: true
> +  touchscreen-swapped-x-y: true

[Severity: Low]
Looking at the properties block in
Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml, are
these redundant definitions a direct workaround for the improperly configured
additionalProperties keyword below?

> +
>  required:
>    - compatible
>    - reg
> @@ -43,6 +47,9 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - $ref: touchscreen.yaml#

[Severity: Low]
Looking at the allOf block in the same file, does the combination of
additionalProperties: false and a $ref create a constraint collision that
restricts validation exclusively to locally defined properties, breaking proper
schema inheritance?

If a valid generic touchscreen property (e.g., touchscreen-size-x) not
explicitly listed in the local schema is defined, running make dtbs_check
will incorrectly reject it. Would changing additionalProperties: false to
unevaluatedProperties: false address this when inheriting from another schema?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/d18629ede3f2fd5a08d0cbdaf82f4254e89500ed.1790851590.git.daniel@makrotopia.org?part=1

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

[Conor Dooley]

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

On Thu, Oct 01, 2026 at 11:52:31AM +0100, Daniel Golle wrote:
> Reference touchscreen.yaml and allow properties
> 'touchscreen-inverted-x', 'touchscreen-inverted-y' and
> 'touchscreen-swapped-x-y'.

This is a useless commit message, it just says what the diff does.
Please provide a justification for why this is the right thing to do.
pw-bot: changes-requested

> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  .../bindings/input/touchscreen/hynitron,cst816x.yaml      | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
> index 72d4da636881e..ba56bf61038c5 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
> +++ b/Documentation/devicetree/bindings/input/touchscreen/hynitron,cst816x.yaml
> @@ -36,6 +36,10 @@ properties:
>        - description: Slide right gesture
>        - description: Long press gesture
>  
> +  touchscreen-inverted-x: true
> +  touchscreen-inverted-y: true
> +  touchscreen-swapped-x-y: true
> +
>  required:
>    - compatible
>    - reg
> @@ -43,6 +47,9 @@ required:
>  
>  additionalProperties: false
>  
> +allOf:
> +  - $ref: touchscreen.yaml#
> +
>  examples:
>    - |
>      #include <dt-bindings/gpio/gpio.h>
> @@ -59,6 +66,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.55.0

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH 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-inverted-x,
touchscreen-inverted-y and touchscreen-swapped-x-y properties

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 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.55.0

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

Fixes: c87a819bec86 ("Input: add driver for Hynitron CST816x series")
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 drivers/input/touchscreen/hynitron-cst816x.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
index 865c5a767ba01..e2093518e8af1 100644
--- a/drivers/input/touchscreen/hynitron-cst816x.c
+++ b/drivers/input/touchscreen/hynitron-cst816x.c
@@ -168,6 +168,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 +193,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.55.0

Re: [PATCH 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: OOB read in cst816x_release_keys() due to unvalidated DT keycodes
--

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

Input: hynitron-cst816x: release gesture keys

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. This commit reports the press on its own and releases
the keys the input core still holds down when the first report without a
gesture code arrives.

> diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
> index 865c5a767ba01..e2093518e8af1 100644
> --- a/drivers/input/touchscreen/hynitron-cst816x.c
> +++ b/drivers/input/touchscreen/hynitron-cst816x.c
[ ... ]
> @@ -168,6 +168,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))

[Severity: Medium]
Can this read out of bounds on the priv->input->key bitmap?

If a keycode provided in the device tree property exceeds KEY_MAX,
input_set_capability() safely ignores it during registration, but the
invalid large value remains in the priv->keycode array.

When cst816x_release_keys() iterates over this array, it directly calls
test_bit(priv->keycode[i], priv->input->key). Since test_bit() doesn't
perform bounds checking like the input core's event handlers do, passing
an oversized keycode could result in a memory read past the end of the
bitmap structure during touch interrupts.

> +			continue;
> +
> +		input_report_key(priv->input, priv->keycode[i], 0);
> +	}
> +}
> +

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

[PATCH 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>
---
 drivers/input/touchscreen/hynitron-cst816x.c | 51 ++++++++++++++++++--
 1 file changed, 48 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
index e2093518e8af1..30f8ea314ed2c 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;
 };
 
@@ -168,8 +174,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++) {
@@ -177,33 +184,65 @@ 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);
+
+	if (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_delete_sync(&priv->release_timer);
+}
+
 static int cst816x_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -215,6 +254,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))
@@ -232,6 +273,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.55.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] input: spurious key release due to timer callback race
--

commit 84ec9fce5971ed83d872b86322e5cd6170da5358
Author: Daniel Golle <daniel@makrotopia.org>

Input: hynitron-cst816x: time out gesture key release

This commit adds a 40ms timer to release gesture keys if a new touch
event with a gesture code is not reported in time. It helps prevent
keys from remaining stuck in the pressed state when the panel stops
sending events.

> diff --git a/drivers/input/touchscreen/hynitron-cst816x.c b/drivers/input/touchscreen/hynitron-cst816x.c
> index e2093518e8af1..30f8ea314ed2c 100644
> --- a/drivers/input/touchscreen/hynitron-cst816x.c
> +++ b/drivers/input/touchscreen/hynitron-cst816x.c
[ ... ]
> @@ -177,33 +184,65 @@ static void cst816x_release_keys(struct cst816x_priv *priv)
[ ... ]
> +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);
> +
> +	if (cst816x_release_keys(priv))
> +		input_sync(priv->input);
> +
> +	spin_unlock_irqrestore(&priv->lock, flags);
> +}

[Severity: Medium]
Could this unconditionally release keys even if the timer was just
concurrently re-armed by the IRQ handler?

If the timer softirq and the threaded IRQ handler (cst816x_irq_cb) run
concurrently on different CPUs, the timer callback might spin waiting for
the lock while the IRQ handler is reporting a new key press and re-arming
the timer.

Once the IRQ handler drops the lock, the timer callback acquires it and
unconditionally executes cst816x_release_keys(), undoing the IRQ handler's
action and causing a spurious release of gesture keys.

Would it be safer to check if the timer was re-enqueued (e.g., via
timer_pending()) while waiting for the lock before releasing the keys?

>  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));

[Severity: Medium]
Does re-arming the timer here race with the timer callback executing on
another CPU?

Because the timer callback does not check if the timer was re-armed, the
newly reported key press here could be immediately reversed by the racing
timer callback once the IRQ handler releases the lock.

> +	} else {
> +		timer_delete(&priv->release_timer);
>  		cst816x_release_keys(priv);
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/d18629ede3f2fd5a08d0cbdaf82f4254e89500ed.1790851590.git.daniel@makrotopia.org?part=4