[PATCH v2 0/6] Small fixes to the cyttsp5 touchscreen driver

[PATCH v2 0/6] Small fixes to the cyttsp5 touchscreen driver

[Maximilian Weigand]

While working on some intermittent module-loading problems of the
cyttsp5 module on the Pine64 PineNote it was found that the device tree
example of the cypress,tt21000 was in error regarding the interrupt
type (IRQ_TYPE_EDGE_FALLING should be used instead of IRQ_TYPE_LEVEL_LOW).

This lead to the proper implementation of device sleep states, which is
required to ensure proper functioning of the touchscreen after resume
when the correct interrupt type IRQ_TYPE_FALLING_EDGE is used. Sleep and
wakeup commands to the touchscreen were derived from the GPL-2 android
driver by Cypress Semiconductor (copyright note for Cypress
Semiconductor is already in the current driver).

The first two patches fix small issues with the code found during
development of the suspend functionality.

Changes in v2:
- fix subject lines
- fix 'unused variable' errors reported by the kernel test robot
- clean up commit message of patch 2

Maximilian Weigand (6):
  Input: cyttsp5 - fix array length
  Input: cyttsp5 - remove unused code
  dt-bindings: input: cypress,tt21000 - fix interrupt type in dts
    example
  Input: cyttsp5 - properly initialize the device as a pm wakeup device
  dt-bindings: input: cypress,tt21000 - add wakeup-source entry to
    documentation
  Input: cyttsp5 - implement proper sleep and wakeup procedures

 .../input/touchscreen/cypress,tt21000.yaml    |   4 +-
 drivers/input/touchscreen/cyttsp5.c           | 133 +++++++++++++++++-
 2 files changed, 130 insertions(+), 7 deletions(-)


base-commit: 457391b0380335d5e9a5babdec90ac53928b23b4
--
2.39.2

[PATCH v2 1/6] Input: cyttsp5 - fix array length

[Maximilian Weigand]

The cmd array should be initialized with the proper command size and not
with the actual command value that is sent to the touchscreen.

Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/input/touchscreen/cyttsp5.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 30102cb80fac..3c9d07218f48 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -560,7 +560,7 @@ static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts)
 static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts)
 {
 	int rc;
-	u8 cmd[HID_OUTPUT_BL_LAUNCH_APP];
+	u8 cmd[HID_OUTPUT_BL_LAUNCH_APP_SIZE];
 	u16 crc;
 
 	put_unaligned_le16(HID_OUTPUT_BL_LAUNCH_APP_SIZE, cmd);
-- 
2.39.2

[PATCH v2 2/6] Input: cyttsp5 - remove unused code

[Maximilian Weigand]

The removed lines are remnants of the vendor driver and are not used in
the upstream driver.

Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/input/touchscreen/cyttsp5.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 3c9d07218f48..55abf568bdf6 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -601,12 +601,7 @@ static int cyttsp5_get_hid_descriptor(struct cyttsp5 *ts,
 				      struct cyttsp5_hid_desc *desc)
 {
 	struct device *dev = ts->dev;
-	__le16 hid_desc_register = cpu_to_le16(HID_DESC_REG);
 	int rc;
-	u8 cmd[2];
-
-	/* Set HID descriptor register */
-	memcpy(cmd, &hid_desc_register, sizeof(hid_desc_register));
 
 	rc = cyttsp5_write(ts, HID_DESC_REG, NULL, 0);
 	if (rc) {
-- 
2.39.2

[PATCH v2 3/6] dt-bindings: input: cypress,tt21000 - fix interrupt type in dts example

[Maximilian Weigand]

Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
probing issues with the device for the current driver (encountered on
the Pine64 PineNote). Basically the interrupt would be triggered before
certain commands were sent to the device, leading to a race between the
device responding fast enough and the irq handler fetching a data frame
from it. Actually all devices currently using the driver already use a
falling edge trigger.

Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/input/touchscreen/cypress,tt21000.yaml  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
index 1959ec394768..a77203c78d6e 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
@@ -83,7 +83,7 @@ examples:
             pinctrl-names = "default";
             pinctrl-0 = <&tp_reset_ds203>;
             interrupt-parent = <&pio>;
-            interrupts = <1 5 IRQ_TYPE_LEVEL_LOW>;
+            interrupts = <1 5 IRQ_TYPE_EDGE_FALLING>;
             reset-gpios = <&pio 7 1 GPIO_ACTIVE_LOW>;
             vdd-supply = <&reg_touch>;
 
-- 
2.39.2

[PATCH v2 4/6] Input: cyttsp5 - properly initialize the device as a pm wakeup device

[Maximilian Weigand]

When used as a wakeup source the driver should be properly registered
with the pm system using device_init_wakeup.

Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/input/touchscreen/cyttsp5.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 55abf568bdf6..f701125357f0 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -830,6 +830,9 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
 		return error;
 	}
 
+	if (device_property_read_bool(dev, "wakeup-source"))
+		device_init_wakeup(dev, true);
+
 	error = cyttsp5_startup(ts);
 	if (error) {
 		dev_err(ts->dev, "Fail initial startup r=%d\n", error);
-- 
2.39.2

[PATCH v2 5/6] dt-bindings: input: cypress,tt21000 - add wakeup-source entry to documentation

[Maximilian Weigand]

The touchscreen can be used to wake up systems from sleep and therefore
the wakeup-source entry should be included in the documentation.

Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
---
 .../devicetree/bindings/input/touchscreen/cypress,tt21000.yaml  | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
index a77203c78d6e..e2da13b7991d 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/cypress,tt21000.yaml
@@ -40,6 +40,8 @@ properties:
   linux,keycodes:
     description: EV_ABS specific event code generated by the axis.
 
+  wakeup-source: true
+
 patternProperties:
   "^button@[0-9]+$":
     type: object
-- 
2.39.2

[PATCH v2 6/6] Input: cyttsp5 - implement proper sleep and wakeup procedures

[Maximilian Weigand]

The touchscreen can be put into a deep sleep state that prevents it from
emitting touch irqs. Put the touchscreen into deep sleep during suspend
if it is not marked as a wakeup source.

This also fixes a problem with the touchscreen getting unresponsive after
system resume because it pulled the interrupt line low during sleep in
response to a touch event, thereby effectively disabling the interrupt
handling (which triggers on the falling edge).

Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
Reviewed-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/input/touchscreen/cyttsp5.c | 125 +++++++++++++++++++++++++++-
 1 file changed, 124 insertions(+), 1 deletion(-)

diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index f701125357f0..54b1c9512e4d 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -43,6 +43,7 @@
 #define HID_DESC_REG				0x1
 #define HID_INPUT_REG				0x3
 #define HID_OUTPUT_REG				0x4
+#define HID_COMMAND_REG             0x5
 
 #define REPORT_ID_TOUCH				0x1
 #define REPORT_ID_BTN				0x3
@@ -68,6 +69,7 @@
 #define HID_APP_OUTPUT_REPORT_ID		0x2F
 #define HID_BL_RESPONSE_REPORT_ID		0x30
 #define HID_BL_OUTPUT_REPORT_ID			0x40
+#define HID_RESPONSE_REPORT_ID      0xF0
 
 #define HID_OUTPUT_RESPONSE_REPORT_OFFSET	2
 #define HID_OUTPUT_RESPONSE_CMD_OFFSET		4
@@ -78,9 +80,15 @@
 #define HID_SYSINFO_BTN_MASK			GENMASK(7, 0)
 #define HID_SYSINFO_MAX_BTN			8
 
+#define HID_CMD_SET_POWER           0x8
+
+#define HID_POWER_ON                0x0
+#define HID_POWER_SLEEP             0x1
+
 #define CY_HID_OUTPUT_TIMEOUT_MS		200
 #define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT_MS	3000
 #define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT_MS	4000
+#define CY_HID_SET_POWER_TIMEOUT		500
 
 /* maximum number of concurrent tracks */
 #define TOUCH_REPORT_SIZE			10
@@ -100,6 +108,14 @@
 #define TOUCH_REPORT_USAGE_PG_MIN		0xFF010063
 #define TOUCH_COL_USAGE_PG			0x000D0022
 
+#define SET_CMD_LOW(byte, bits) \
+	((byte) = (((byte) & 0xF0) | ((bits) & 0x0F)))
+#define SET_CMD_HIGH(byte, bits)\
+	((byte) = (((byte) & 0x0F) | ((bits) & 0xF0)))
+#define SET_CMD_OPCODE(byte, opcode) SET_CMD_LOW(byte, opcode)
+#define SET_CMD_REPORT_TYPE(byte, type) SET_CMD_HIGH(byte, ((type) << 4))
+#define SET_CMD_REPORT_ID(byte, id) SET_CMD_LOW(byte, id)
+
 /* System Information interface definitions */
 struct cyttsp5_sensing_conf_data_dev {
 	u8 electrodes_x;
@@ -180,6 +196,7 @@ struct cyttsp5_hid_desc {
 struct cyttsp5 {
 	struct device *dev;
 	struct completion cmd_done;
+	struct completion cmd_command_done;
 	struct cyttsp5_sysinfo sysinfo;
 	struct cyttsp5_hid_desc hid_desc;
 	u8 cmd_buf[CYTTSP5_PREALLOCATED_CMD_BUFFER];
@@ -192,6 +209,7 @@ struct cyttsp5 {
 	struct regmap *regmap;
 	struct touchscreen_properties prop;
 	struct regulator *vdd;
+	bool is_wakeup_source;
 };
 
 /*
@@ -557,6 +575,82 @@ static int cyttsp5_hid_output_get_sysinfo(struct cyttsp5 *ts)
 	return cyttsp5_get_sysinfo_regs(ts);
 }
 
+static int cyttsp5_enter_sleep(struct cyttsp5 *ts)
+{
+	int rc;
+	u8 cmd[2];
+
+	memset(cmd, 0, sizeof(cmd));
+
+	SET_CMD_REPORT_TYPE(cmd[0], 0);
+	SET_CMD_REPORT_ID(cmd[0], HID_POWER_SLEEP);
+	SET_CMD_OPCODE(cmd[1], HID_CMD_SET_POWER);
+
+	rc = cyttsp5_write(ts, HID_COMMAND_REG, cmd, 2);
+	if (rc) {
+		dev_err(ts->dev, "Failed to write command %d", rc);
+		return rc;
+	}
+
+	rc = wait_for_completion_interruptible_timeout(&ts->cmd_command_done,
+				msecs_to_jiffies(CY_HID_SET_POWER_TIMEOUT));
+	if (rc <= 0) {
+		dev_err(ts->dev, "HID output cmd execution timed out\n");
+		rc = -ETIMEDOUT;
+		return rc;
+	}
+
+	/* validate */
+	if ((ts->response_buf[2] != HID_RESPONSE_REPORT_ID)
+			|| ((ts->response_buf[3] & 0x3) != HID_POWER_SLEEP)
+			|| ((ts->response_buf[4] & 0xF) != HID_CMD_SET_POWER)) {
+		rc = -EINVAL;
+		dev_err(ts->dev, "Validation of the sleep response failed\n");
+		return rc;
+	}
+
+	return 0;
+
+}
+
+static int cyttsp5_wakeup(struct cyttsp5 *ts)
+{
+	int rc;
+	u8 cmd[2];
+
+	memset(cmd, 0, sizeof(cmd));
+
+	SET_CMD_REPORT_TYPE(cmd[0], 0);
+	SET_CMD_REPORT_ID(cmd[0], HID_POWER_ON);
+	SET_CMD_OPCODE(cmd[1], HID_CMD_SET_POWER);
+
+	rc = cyttsp5_write(ts, HID_COMMAND_REG, cmd, 2);
+	if (rc) {
+		dev_err(ts->dev, "Failed to write command %d", rc);
+		return rc;
+	}
+
+	rc = wait_for_completion_interruptible_timeout(&ts->cmd_command_done,
+				msecs_to_jiffies(CY_HID_SET_POWER_TIMEOUT));
+	if (rc <= 0) {
+		dev_err(ts->dev, "HID output cmd execution timed out\n");
+		rc = -ETIMEDOUT;
+		return rc;
+	}
+
+	/* validate */
+	if ((ts->response_buf[2] != HID_RESPONSE_REPORT_ID)
+			|| ((ts->response_buf[3] & 0x3) != HID_POWER_ON)
+			|| ((ts->response_buf[4] & 0xF) != HID_CMD_SET_POWER)) {
+		rc = -EINVAL;
+		dev_err(ts->dev, "Validation of the sleep response failed\n");
+		return rc;
+	}
+
+	return 0;
+
+}
+
 static int cyttsp5_hid_output_bl_launch_app(struct cyttsp5 *ts)
 {
 	int rc;
@@ -670,6 +764,10 @@ static irqreturn_t cyttsp5_handle_irq(int irq, void *handle)
 	case HID_BTN_REPORT_ID:
 		cyttsp5_btn_attention(ts->dev);
 		break;
+	case HID_RESPONSE_REPORT_ID:
+		memcpy(ts->response_buf, ts->input_buf, size);
+		complete(&ts->cmd_command_done);
+		break;
 	default:
 		/* It is not an input but a command response */
 		memcpy(ts->response_buf, ts->input_buf, size);
@@ -784,6 +882,7 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
 	dev_set_drvdata(dev, ts);
 
 	init_completion(&ts->cmd_done);
+	init_completion(&ts->cmd_command_done);
 
 	/* Power up the device */
 	ts->vdd = devm_regulator_get(dev, "vdd");
@@ -830,8 +929,11 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
 		return error;
 	}
 
-	if (device_property_read_bool(dev, "wakeup-source"))
+	if (device_property_read_bool(dev, "wakeup-source")) {
 		device_init_wakeup(dev, true);
+		ts->is_wakeup_source = true;
+	} else
+		ts->is_wakeup_source = false;
 
 	error = cyttsp5_startup(ts);
 	if (error) {
@@ -884,6 +986,27 @@ static const struct i2c_device_id cyttsp5_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
 
+static int __maybe_unused cyttsp5_suspend(struct device *dev)
+{
+	struct cyttsp5 *ts = dev_get_drvdata(dev);
+
+	if (!ts->is_wakeup_source)
+		cyttsp5_enter_sleep(ts);
+	return 0;
+}
+
+static int __maybe_unused cyttsp5_resume(struct device *dev)
+{
+	struct cyttsp5 *ts = dev_get_drvdata(dev);
+
+	if (!ts->is_wakeup_source)
+		cyttsp5_wakeup(ts);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(cyttsp5_pm, cyttsp5_suspend, cyttsp5_resume);
+
 static struct i2c_driver cyttsp5_i2c_driver = {
 	.driver = {
 		.name = CYTTSP5_NAME,
-- 
2.39.2

Re: [PATCH v2 4/6] Input: cyttsp5 - properly initialize the device as a pm wakeup device

[Dmitry Torokhov]

Hi Maximilian,

On Mon, May 01, 2023 at 01:30:08PM +0200, Maximilian Weigand wrote:
> When used as a wakeup source the driver should be properly registered
> with the pm system using device_init_wakeup.

This is an I2C device and I2C core already handles setting up a device
as a wakeup source, this patch is not needed as far as I can tell.

> 
> Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
> Reviewed-by: Alistair Francis <alistair@alistair23.me>
> ---
>  drivers/input/touchscreen/cyttsp5.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
> index 55abf568bdf6..f701125357f0 100644
> --- a/drivers/input/touchscreen/cyttsp5.c
> +++ b/drivers/input/touchscreen/cyttsp5.c
> @@ -830,6 +830,9 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
>  		return error;
>  	}
>  
> +	if (device_property_read_bool(dev, "wakeup-source"))
> +		device_init_wakeup(dev, true);
> +
>  	error = cyttsp5_startup(ts);
>  	if (error) {
>  		dev_err(ts->dev, "Fail initial startup r=%d\n", error);
> -- 
> 2.39.2
> 

Thanks.

-- 
Dmitry

Re: [PATCH v2 1/6] Input: cyttsp5 - fix array length

[Dmitry Torokhov]

On Mon, May 01, 2023 at 01:30:05PM +0200, Maximilian Weigand wrote:
> The cmd array should be initialized with the proper command size and not
> with the actual command value that is sent to the touchscreen.
> 
> Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
> Reviewed-by: Alistair Francis <alistair@alistair23.me>

Applied, thank you.

-- 
Dmitry

Re: [PATCH v2 2/6] Input: cyttsp5 - remove unused code

[Dmitry Torokhov]

On Mon, May 01, 2023 at 01:30:06PM +0200, Maximilian Weigand wrote:
> The removed lines are remnants of the vendor driver and are not used in
> the upstream driver.
> 
> Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
> Reviewed-by: Alistair Francis <alistair@alistair23.me>

Applied, thank you.

-- 
Dmitry

Re: [PATCH v2 6/6] Input: cyttsp5 - implement proper sleep and wakeup procedures

[Dmitry Torokhov]

On Mon, May 01, 2023 at 01:30:10PM +0200, Maximilian Weigand wrote:
>  struct cyttsp5 {
>  	struct device *dev;
>  	struct completion cmd_done;
> +	struct completion cmd_command_done;

Why do we need separate comletion? Do you observe some additional
traffic from the controller when powering it off and on?

> +static int __maybe_unused cyttsp5_suspend(struct device *dev)
> +{
> +	struct cyttsp5 *ts = dev_get_drvdata(dev);
> +
> +	if (!ts->is_wakeup_source)

I believe the idiomatic way to check this is to call
device_may_wakeup().

> +		cyttsp5_enter_sleep(ts);
> +	return 0;
> +}

Thanks.

-- 
Dmitry

Re: [PATCH v2 3/6] dt-bindings: input: cypress,tt21000 - fix interrupt type in dts example

[Dmitry Torokhov]

On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
> probing issues with the device for the current driver (encountered on
> the Pine64 PineNote). Basically the interrupt would be triggered before
> certain commands were sent to the device, leading to a race between the
> device responding fast enough and the irq handler fetching a data frame
> from it. Actually all devices currently using the driver already use a
> falling edge trigger.

I'd prefer we adjusted the driver to handle level interrupts properly.

Thanks.

-- 
Dmitry

Re: [PATCH v2 5/6] dt-bindings: input: cypress,tt21000 - add wakeup-source entry to documentation

[Dmitry Torokhov]

On Mon, May 01, 2023 at 01:30:09PM +0200, Maximilian Weigand wrote:
> The touchscreen can be used to wake up systems from sleep and therefore
> the wakeup-source entry should be included in the documentation.
> 
> Signed-off-by: Maximilian Weigand <mweigand@mweigand.net>
> Reviewed-by: Alistair Francis <alistair@alistair23.me>
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Applied, thank you.

-- 
Dmitry

Re: [PATCH v2 6/6] Input: cyttsp5 - implement proper sleep and wakeup procedures

[Maximilian Weigand]

Hi,

On 02.05.23 02:22, Dmitry Torokhov wrote:
> On Mon, May 01, 2023 at 01:30:10PM +0200, Maximilian Weigand wrote:
>>  struct cyttsp5 {
>>  	struct device *dev;
>>  	struct completion cmd_done;
>> +	struct completion cmd_command_done;
> 
> Why do we need separate comletion? Do you observe some additional
> traffic from the controller when powering it off and on?

I checked and indeed there is no overlap in the different command types,
so one completion will work. I will reformat correspondingly.

>> +static int __maybe_unused cyttsp5_suspend(struct device *dev)
>> +{
>> +	struct cyttsp5 *ts = dev_get_drvdata(dev);
>> +
>> +	if (!ts->is_wakeup_source)
> 
> I believe the idiomatic way to check this is to call
> device_may_wakeup().

Thanks for pointing that out. I will fix that, too.

Thanks for the feedback and best regards

Maximilian

Re: [PATCH v2 3/6] dt-bindings: input: cypress,tt21000 - fix interrupt type in dts example

[Maximilian Weigand]

Hi,

On 02.05.23 02:24, Dmitry Torokhov wrote:
> On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
>> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
>> probing issues with the device for the current driver (encountered on
>> the Pine64 PineNote). Basically the interrupt would be triggered before
>> certain commands were sent to the device, leading to a race between the
>> device responding fast enough and the irq handler fetching a data frame
>> from it. Actually all devices currently using the driver already use a
>> falling edge trigger.
> 
> I'd prefer we adjusted the driver to handle level interrupts properly.

Ok, I will have a look at that. Just to be clear: The driver should work
only with level interrupts, or should it optimally support both level
and falling edge triggers?

Thanks and best regards

Maximilian

Re: [PATCH v2 3/6] dt-bindings: input: cypress,tt21000 - fix interrupt type in dts example

[Dmitry Torokhov]

On Tue, May 02, 2023 at 04:23:54PM +0200, Maximilian Weigand wrote:
> Hi,
> 
> On 02.05.23 02:24, Dmitry Torokhov wrote:
> > On Mon, May 01, 2023 at 01:30:07PM +0200, Maximilian Weigand wrote:
> >> Triggering the interrupt of the IRQ_TYPE_LEVEL_LOW type can lead to
> >> probing issues with the device for the current driver (encountered on
> >> the Pine64 PineNote). Basically the interrupt would be triggered before
> >> certain commands were sent to the device, leading to a race between the
> >> device responding fast enough and the irq handler fetching a data frame
> >> from it. Actually all devices currently using the driver already use a
> >> falling edge trigger.
> > 
> > I'd prefer we adjusted the driver to handle level interrupts properly.
> 
> Ok, I will have a look at that. Just to be clear: The driver should work
> only with level interrupts, or should it optimally support both level
> and falling edge triggers?

Optimally a driver would work well with both.

Thanks.

-- 
Dmitry