linux-kernel - Re: [PATCH 2/2] Input: ili210x

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20191104183702.8894-1-TheSven73@gmail.com>
Date:   Mon,  4 Nov 2019 13:37:02 -0500
From:   Sven Van Asbroeck <thesven73@...il.com>
To:     Marek Vasut <marex@...x.de>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     Adam Ford <aford173@...il.com>, linux-input@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] Input: ili210x - add ILI2117 support

Ok, so here are my test results on an ili211x :

Using Marek's patch:
https://patchwork.kernel.org/patch/10836651/#22811657
It works fine.

Using Dmitry's patch:
https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git/log/?h=ili2xxx-touchscreen
Does not work at all - the driver even enters an infinite loop.

I tracked this down to two separate issues:
1. the ili211x does not have a touchdata register; but the driver tries to 
	read from one.
2. the ili211x should never poll - otherwise it will read all zeros, which
	passes the crc check (!), then results in ten finger touches all
	at (x=0, y=0).

The patch at the end of this e-mail addresses these two issues. When it is
applied, the ili211x works fine.

Of course, this does not address the issue Marek saw with Dmitry's patch
	on the ili251x.

Sven

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index 7a9c46b8a079..f51a3a19075f 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -36,7 +36,7 @@ struct ili2xxx_chip {
 	int (*get_touch_data)(struct i2c_client *client, u8 *data);
 	bool (*parse_touch_data)(const u8 *data, unsigned int finger,
 				 unsigned int *x, unsigned int *y);
-	bool (*continue_polling)(const u8 *data);
+	bool (*continue_polling)(const u8 *data, bool has_touch);
 	unsigned int max_touches;
 };
 
@@ -96,7 +96,7 @@ static bool ili210x_touchdata_to_coords(const u8 *touchdata,
 	return true;
 }
 
-static bool ili210x_check_continue_polling(const u8 *data)
+static bool ili210x_check_continue_polling(const u8 *data, bool has_touch)
 {
 	return data[0] & 0xf3;
 }
@@ -111,14 +111,19 @@ static const struct ili2xxx_chip ili210x_chip = {
 
 static int ili211x_read_touch_data(struct i2c_client *client, u8 *data)
 {
+	struct i2c_msg msg = {
+		.addr	= client->addr,
+		.flags	= I2C_M_RD,
+		.len	= ILI211X_DATA_SIZE,
+		.buf	= data,
+	};
 	s16 sum = 0;
 	int i;
-	int error;
 
-	error = ili210x_read_reg(client, REG_TOUCHDATA,
-				 data, ILI211X_DATA_SIZE);
-	if (error)
-		return error;
+	if (i2c_transfer(client->adapter, &msg, 1) != 1) {
+		dev_err(&client->dev, "i2c transfer failed\n");
+		return -EIO;
+	}
 
 	/* This chip uses custom checksum at the end of data */
 	for (i = 0; i <= ILI211X_DATA_SIZE - 2; i++)
@@ -152,7 +157,7 @@ static bool ili211x_touchdata_to_coords(const u8 *touchdata,
 	return true;
 }
 
-static bool ili2xxx_decline_polling(const u8 *data)
+static bool ili2xxx_decline_polling(const u8 *data, bool has_touch)
 {
 	return false;
 }
@@ -216,11 +221,16 @@ static bool ili251x_touchdata_to_coords(const u8 *touchdata,
 	return true;
 }
 
+static bool ili251x_check_continue_polling(const u8 *data, bool has_touch)
+{
+	return has_touch;
+}
+
 static const struct ili2xxx_chip ili251x_chip = {
 	.read_reg		= ili251x_read_reg,
 	.get_touch_data		= ili251x_read_touch_data,
 	.parse_touch_data	= ili251x_touchdata_to_coords,
-	.continue_polling	= ili2xxx_decline_polling,
+	.continue_polling	= ili251x_check_continue_polling,
 	.max_touches		= 10,
 };
 
@@ -268,7 +278,7 @@ static irqreturn_t ili210x_irq(int irq, void *irq_data)
 		}
 
 		touch = ili210x_report_events(priv, touchdata);
-		keep_polling = touch || chip->continue_polling(touchdata);
+		keep_polling = chip->continue_polling(touchdata, touch);
 		if (keep_polling)
 			msleep(ILI2XXX_POLL_PERIOD);
 	} while (!priv->stop && keep_polling);
-- 
2.17.1