linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sven Van Asbroeck <thesven73@gmail.com>
To: Marek Vasut <marex@denx.de>, Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Adam Ford <aford173@gmail.com>,
	linux-input@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] Input: ili210x - add ILI2117 support
Date: Mon,  4 Nov 2019 13:37:02 -0500	[thread overview]
Message-ID: <20191104183702.8894-1-TheSven73@gmail.com> (raw)
In-Reply-To: <20191104070116.GM57214@dtor-ws>

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

  parent reply	other threads:[~2019-11-04 18:37 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20190302141704.32547-1-marex@denx.de>
2019-11-01 20:48 ` [PATCH 2/2] Input: ili210x - add ILI2117 support Sven Van Asbroeck
2019-11-03 23:55   ` Adam Ford
2019-11-04  0:16     ` Marek Vasut
2019-11-04  7:02       ` Dmitry Torokhov
2019-11-04  7:01   ` Dmitry Torokhov
2019-11-04  9:13     ` Marek Vasut
2019-11-04 13:49     ` Sven Van Asbroeck
2019-11-04 14:19       ` Adam Ford
2019-11-04 18:36         ` Dmitry Torokhov
2019-11-04 18:37     ` Sven Van Asbroeck [this message]
2019-11-04 21:28       ` Adam Ford
2019-11-04 21:43         ` Sven Van Asbroeck
2019-11-04 23:25           ` Adam Ford
2019-11-04 23:36             ` Dmitry Torokhov
2019-11-04 23:40               ` Adam Ford
2019-11-05  2:04                 ` Sven Van Asbroeck
2019-11-05 13:27                   ` Adam Ford
2019-11-05 15:26               ` Sven Van Asbroeck
2019-11-05 15:29               ` Sven Van Asbroeck
2019-11-05 15:53                 ` Sven Van Asbroeck
2019-11-11 18:16                 ` Dmitry Torokhov
2019-11-11 18:43                   ` Rob Herring
2019-11-12  0:19                     ` Dmitry Torokhov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191104183702.8894-1-TheSven73@gmail.com \
    --to=thesven73@gmail.com \
    --cc=aford173@gmail.com \
    --cc=dmitry.torokhov@gmail.com \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marex@denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).