* [PATCH] driver:st1633: fixed multitouch incorrect coordinates @ 2019-08-19 10:08 Dixit Parmar 2019-08-22 12:08 ` Martin Kepplinger 0 siblings, 1 reply; 6+ messages in thread From: Dixit Parmar @ 2019-08-19 10:08 UTC (permalink / raw) To: dmitry.torokhov, rydberg, martink, kuninori.morimoto.gx, robh, matthias.fend, linux-input, linux-kernel Cc: Dixit Parmar From: Dixit Parmar <dixitparmar19@gmail.com> For Sitronix st1633 multi-touch controller driver the co-ordinates reported for multiple fingers were wrong. So the below mentioned bug was filed, Bugzilla Bug ID: 204561 While reading co-ordinates from specified I2C registers, the X & Y co-ordinates should be read from proper I2C address for particular finger as specified in chip specific datasheet. for single touch this logic is working fine. However, for multi-touch scenario the logic of reading data from data buffer has issues. This patch fixes the reading logic from data buffer. Previous logic: * Offset of X & Y Lower byte coordinate is increased by i no. only(by 1 Byte) for each finger. New logic: * The logic of reading X & Y Lower Byte coordinate needs to be increased by i+y for each time/finger. Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com> --- drivers/input/touchscreen/st1232.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c index 3492339..1139714 100644 --- a/drivers/input/touchscreen/st1232.c +++ b/drivers/input/touchscreen/st1232.c @@ -81,8 +81,10 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts) for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) { finger[i].is_valid = buf[i + y] >> 7; if (finger[i].is_valid) { - finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1]; - finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2]; + finger[i].x = ((buf[i + y] & 0x0070) << 4) | + buf[i + y + 1]; + finger[i].y = ((buf[i + y] & 0x0007) << 8) | + buf[i + y + 2]; /* st1232 includes a z-axis / touch strength */ if (ts->chip_info->have_z) -- 2.7.4 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] driver:st1633: fixed multitouch incorrect coordinates 2019-08-19 10:08 [PATCH] driver:st1633: fixed multitouch incorrect coordinates Dixit Parmar @ 2019-08-22 12:08 ` Martin Kepplinger 2019-10-20 8:29 ` Dixit Parmar 0 siblings, 1 reply; 6+ messages in thread From: Martin Kepplinger @ 2019-08-22 12:08 UTC (permalink / raw) To: Dixit Parmar Cc: dmitry.torokhov, rydberg, kuninori.morimoto.gx, robh, matthias.fend, linux-input, linux-kernel Am 19.08.2019 12:08 schrieb Dixit Parmar: > From: Dixit Parmar <dixitparmar19@gmail.com> > > For Sitronix st1633 multi-touch controller driver the co-ordinates > reported > for multiple fingers were wrong. > > So the below mentioned bug was filed, > Bugzilla Bug ID: 204561 > > While reading co-ordinates from specified I2C registers, the X & Y > co-ordinates should be read from proper I2C address for particular > finger as > specified in chip specific datasheet. > > for single touch this logic is working fine. However, for multi-touch > scenario the logic of reading data from data buffer has issues. > > This patch fixes the reading logic from data buffer. > > Previous logic: > * Offset of X & Y Lower byte coordinate is increased by i no. only(by 1 > Byte) > for each finger. > > New logic: > * The logic of reading X & Y Lower Byte coordinate needs to be > increased > by i+y for each time/finger. > > Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com> > --- > drivers/input/touchscreen/st1232.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/input/touchscreen/st1232.c > b/drivers/input/touchscreen/st1232.c > index 3492339..1139714 100644 > --- a/drivers/input/touchscreen/st1232.c > +++ b/drivers/input/touchscreen/st1232.c > @@ -81,8 +81,10 @@ static int st1232_ts_read_data(struct st1232_ts_data > *ts) > for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) { > finger[i].is_valid = buf[i + y] >> 7; > if (finger[i].is_valid) { > - finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1]; > - finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2]; > + finger[i].x = ((buf[i + y] & 0x0070) << 4) | > + buf[i + y + 1]; > + finger[i].y = ((buf[i + y] & 0x0007) << 8) | > + buf[i + y + 2]; Seems like you're right. It's simply +1 (for x) and +2 (for y) from the high-byte locations. Not sure how that went wrong. Thank you, Reviewed-by: Martin Kepplinger <martink@posteo.de> > > /* st1232 includes a z-axis / touch strength */ > if (ts->chip_info->have_z) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver:st1633: fixed multitouch incorrect coordinates 2019-08-22 12:08 ` Martin Kepplinger @ 2019-10-20 8:29 ` Dixit Parmar 2019-10-21 7:10 ` Martin Kepplinger 0 siblings, 1 reply; 6+ messages in thread From: Dixit Parmar @ 2019-10-20 8:29 UTC (permalink / raw) To: Martin Kepplinger Cc: dmitry.torokhov, rydberg, kuninori.morimoto.gx, robh, matthias.fend, linux-input, linux-kernel Any review comments for this? Or it should be merged? Thanks. On Thu, Aug 22, 2019 at 02:08:14PM +0200, Martin Kepplinger wrote: > Am 19.08.2019 12:08 schrieb Dixit Parmar: > > From: Dixit Parmar <dixitparmar19@gmail.com> > > > > For Sitronix st1633 multi-touch controller driver the co-ordinates > > reported > > for multiple fingers were wrong. > > > > So the below mentioned bug was filed, > > Bugzilla Bug ID: 204561 > > > > While reading co-ordinates from specified I2C registers, the X & Y > > co-ordinates should be read from proper I2C address for particular > > finger as > > specified in chip specific datasheet. > > > > for single touch this logic is working fine. However, for multi-touch > > scenario the logic of reading data from data buffer has issues. > > > > This patch fixes the reading logic from data buffer. > > > > Previous logic: > > * Offset of X & Y Lower byte coordinate is increased by i no. only(by 1 > > Byte) > > for each finger. > > > > New logic: > > * The logic of reading X & Y Lower Byte coordinate needs to be increased > > by i+y for each time/finger. > > > > Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com> > > --- > > drivers/input/touchscreen/st1232.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/input/touchscreen/st1232.c > > b/drivers/input/touchscreen/st1232.c > > index 3492339..1139714 100644 > > --- a/drivers/input/touchscreen/st1232.c > > +++ b/drivers/input/touchscreen/st1232.c > > @@ -81,8 +81,10 @@ static int st1232_ts_read_data(struct st1232_ts_data > > *ts) > > for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) { > > finger[i].is_valid = buf[i + y] >> 7; > > if (finger[i].is_valid) { > > - finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1]; > > - finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2]; > > + finger[i].x = ((buf[i + y] & 0x0070) << 4) | > > + buf[i + y + 1]; > > + finger[i].y = ((buf[i + y] & 0x0007) << 8) | > > + buf[i + y + 2]; > > Seems like you're right. It's simply +1 (for x) and +2 (for y) from the > high-byte locations. > Not sure how that went wrong. > > Thank you, > > Reviewed-by: Martin Kepplinger <martink@posteo.de> > > > > > > /* st1232 includes a z-axis / touch strength */ > > if (ts->chip_info->have_z) > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver:st1633: fixed multitouch incorrect coordinates 2019-10-20 8:29 ` Dixit Parmar @ 2019-10-21 7:10 ` Martin Kepplinger 2019-10-21 17:13 ` Dmitry Torokhov 0 siblings, 1 reply; 6+ messages in thread From: Martin Kepplinger @ 2019-10-21 7:10 UTC (permalink / raw) To: Dixit Parmar Cc: dmitry.torokhov, rydberg, kuninori.morimoto.gx, robh, matthias.fend, linux-input, linux-kernel On 20.10.19 10:29, Dixit Parmar wrote: > Any review comments for this? > Or it should be merged? > > Thanks. My comment and tag is there. This fixes multitouch and should be merged. martin > > > On Thu, Aug 22, 2019 at 02:08:14PM +0200, Martin Kepplinger wrote: >> Am 19.08.2019 12:08 schrieb Dixit Parmar: >>> From: Dixit Parmar <dixitparmar19@gmail.com> >>> >>> For Sitronix st1633 multi-touch controller driver the co-ordinates >>> reported >>> for multiple fingers were wrong. >>> >>> So the below mentioned bug was filed, >>> Bugzilla Bug ID: 204561 >>> >>> While reading co-ordinates from specified I2C registers, the X & Y >>> co-ordinates should be read from proper I2C address for particular >>> finger as >>> specified in chip specific datasheet. >>> >>> for single touch this logic is working fine. However, for multi-touch >>> scenario the logic of reading data from data buffer has issues. >>> >>> This patch fixes the reading logic from data buffer. >>> >>> Previous logic: >>> * Offset of X & Y Lower byte coordinate is increased by i no. only(by 1 >>> Byte) >>> for each finger. >>> >>> New logic: >>> * The logic of reading X & Y Lower Byte coordinate needs to be increased >>> by i+y for each time/finger. >>> >>> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com> >>> --- >>> drivers/input/touchscreen/st1232.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/input/touchscreen/st1232.c >>> b/drivers/input/touchscreen/st1232.c >>> index 3492339..1139714 100644 >>> --- a/drivers/input/touchscreen/st1232.c >>> +++ b/drivers/input/touchscreen/st1232.c >>> @@ -81,8 +81,10 @@ static int st1232_ts_read_data(struct st1232_ts_data >>> *ts) >>> for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) { >>> finger[i].is_valid = buf[i + y] >> 7; >>> if (finger[i].is_valid) { >>> - finger[i].x = ((buf[i + y] & 0x0070) << 4) | buf[i + 1]; >>> - finger[i].y = ((buf[i + y] & 0x0007) << 8) | buf[i + 2]; >>> + finger[i].x = ((buf[i + y] & 0x0070) << 4) | >>> + buf[i + y + 1]; >>> + finger[i].y = ((buf[i + y] & 0x0007) << 8) | >>> + buf[i + y + 2]; >> >> Seems like you're right. It's simply +1 (for x) and +2 (for y) from the >> high-byte locations. >> Not sure how that went wrong. >> >> Thank you, >> >> Reviewed-by: Martin Kepplinger <martink@posteo.de> >> >> >>> >>> /* st1232 includes a z-axis / touch strength */ >>> if (ts->chip_info->have_z) >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] driver:st1633: fixed multitouch incorrect coordinates 2019-10-21 7:10 ` Martin Kepplinger @ 2019-10-21 17:13 ` Dmitry Torokhov 2019-10-22 8:17 ` Martin Kepplinger 0 siblings, 1 reply; 6+ messages in thread From: Dmitry Torokhov @ 2019-10-21 17:13 UTC (permalink / raw) To: Martin Kepplinger Cc: Dixit Parmar, rydberg, kuninori.morimoto.gx, robh, matthias.fend, linux-input, linux-kernel On Mon, Oct 21, 2019 at 09:10:23AM +0200, Martin Kepplinger wrote: > On 20.10.19 10:29, Dixit Parmar wrote: > > Any review comments for this? > > Or it should be merged? > > > > Thanks. > > My comment and tag is there. This fixes multitouch and should be merged. Missed it earlier, sorry. I am applying it, but I wonder if we shoudl not do the patch below as I find the version with 2 loop variables quite confusing. Thanks. -- Dmitry Input: st1232 - simplify parsing of read buffer From: Dmitry Torokhov <dmitry.torokhov@gmail.com> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com> --- drivers/input/touchscreen/st1232.c | 50 ++++++++++++++++++------------------ 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c index 1139714e72e2..47033ef3749a 100644 --- a/drivers/input/touchscreen/st1232.c +++ b/drivers/input/touchscreen/st1232.c @@ -57,38 +57,38 @@ static int st1232_ts_read_data(struct st1232_ts_data *ts) { struct st1232_ts_finger *finger = ts->finger; struct i2c_client *client = ts->client; - struct i2c_msg msg[2]; - int error; - int i, y; u8 start_reg = ts->chip_info->start_reg; - u8 *buf = ts->read_buf; - - /* read touchscreen data */ - msg[0].addr = client->addr; - msg[0].flags = 0; - msg[0].len = 1; - msg[0].buf = &start_reg; - - msg[1].addr = ts->client->addr; - msg[1].flags = I2C_M_RD; - msg[1].len = ts->read_buf_len; - msg[1].buf = buf; + struct i2c_msg msg[] = { + { + .addr = client->addr, + .len = sizeof(start_reg), + .buf = &start_reg, + }, + { + .addr = client->addr, + .flags = I2C_M_RD, + .len = ts->read_buf_len, + .buf = ts->read_buf, + } + }; + int ret; + int i; + u8 *buf; - error = i2c_transfer(client->adapter, msg, 2); - if (error < 0) - return error; + ret = i2c_transfer(client->adapter, msg, ARRAY_SIZE(msg)); + if (ret != ARRAY_SIZE(msg)) + return ret < 0 ? ret : -EIO; - for (i = 0, y = 0; i < ts->chip_info->max_fingers; i++, y += 3) { - finger[i].is_valid = buf[i + y] >> 7; + for (i = 0; i < ts->chip_info->max_fingers; i++) { + buf = &ts->read_buf[i * 4]; + finger[i].is_valid = buf[0] >> 7; if (finger[i].is_valid) { - finger[i].x = ((buf[i + y] & 0x0070) << 4) | - buf[i + y + 1]; - finger[i].y = ((buf[i + y] & 0x0007) << 8) | - buf[i + y + 2]; + finger[i].x = ((buf[0] & 0x70) << 4) | buf[1]; + finger[i].y = ((buf[0] & 0x07) << 8) | buf[2]; /* st1232 includes a z-axis / touch strength */ if (ts->chip_info->have_z) - finger[i].t = buf[i + 6]; + finger[i].t = ts->read_buf[i + 6]; } } ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] driver:st1633: fixed multitouch incorrect coordinates 2019-10-21 17:13 ` Dmitry Torokhov @ 2019-10-22 8:17 ` Martin Kepplinger 0 siblings, 0 replies; 6+ messages in thread From: Martin Kepplinger @ 2019-10-22 8:17 UTC (permalink / raw) To: Dmitry Torokhov Cc: Dixit Parmar, rydberg, kuninori.morimoto.gx, robh, matthias.fend, linux-input, linux-kernel On 21.10.19 19:13, Dmitry Torokhov wrote: > On Mon, Oct 21, 2019 at 09:10:23AM +0200, Martin Kepplinger wrote: >> On 20.10.19 10:29, Dixit Parmar wrote: >>> Any review comments for this? >>> Or it should be merged? >>> >>> Thanks. >> >> My comment and tag is there. This fixes multitouch and should be merged. > > Missed it earlier, sorry. I am applying it, but I wonder if we shoudl > not do the patch below as I find the version with 2 loop variables quite > confusing. > > Thanks. > Makes sense to me. Acked-by: Martin Kepplinger <martink@posteo.de> thanks martin ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-22 8:18 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-19 10:08 [PATCH] driver:st1633: fixed multitouch incorrect coordinates Dixit Parmar 2019-08-22 12:08 ` Martin Kepplinger 2019-10-20 8:29 ` Dixit Parmar 2019-10-21 7:10 ` Martin Kepplinger 2019-10-21 17:13 ` Dmitry Torokhov 2019-10-22 8:17 ` Martin Kepplinger
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.