diff mbox series

[V2,07/10] input: touchscreen: ili210x: Rework the touchscreen sample processing

Message ID 20181220204305.28807-8-marex@denx.de (mailing list archive)
State Superseded
Headers show
Series input: touchscreen: ili210x: Add ILI2511 support | expand

Commit Message

Marek Vasut Dec. 20, 2018, 8:43 p.m. UTC
Get rid of the packed structures for representing data as that does not
apply to other similar Ilitek touchscreens. Instead, implement a function
which parses the data and reports touch events and coordinates.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Henrik Rydberg <rydberg@bitmath.org>
Cc: Olivier Sobrie <olivier@sobrie.be>
Cc: Philipp Puschmann <pp@emlix.com>
To: linux-input@vger.kernel.org
---
V2: Replace touchdata.status with touchdata[0] and move it into
    ili210x_report_events() in anticipation of ILI251x specific changes,
    and to to synchronize the patch with changes in
    "input: touchscreen: ili210x: Convert to devm_ functions"
---
 drivers/input/touchscreen/ili210x.c | 59 +++++++++++++++++------------
 1 file changed, 34 insertions(+), 25 deletions(-)

Comments

Dmitry Torokhov Dec. 21, 2018, 1:52 a.m. UTC | #1
On Thu, Dec 20, 2018 at 09:43:02PM +0100, Marek Vasut wrote:
> Get rid of the packed structures for representing data as that does not
> apply to other similar Ilitek touchscreens. Instead, implement a function
> which parses the data and reports touch events and coordinates.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Henrik Rydberg <rydberg@bitmath.org>
> Cc: Olivier Sobrie <olivier@sobrie.be>
> Cc: Philipp Puschmann <pp@emlix.com>
> To: linux-input@vger.kernel.org
> ---
> V2: Replace touchdata.status with touchdata[0] and move it into
>     ili210x_report_events() in anticipation of ILI251x specific changes,
>     and to to synchronize the patch with changes in
>     "input: touchscreen: ili210x: Convert to devm_ functions"
> ---
>  drivers/input/touchscreen/ili210x.c | 59 +++++++++++++++++------------
>  1 file changed, 34 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> index a84ca555829e..ab0e6c9a614c 100644
> --- a/drivers/input/touchscreen/ili210x.c
> +++ b/drivers/input/touchscreen/ili210x.c
> @@ -16,20 +16,11 @@
>  #define REG_FIRMWARE_VERSION	0x40
>  #define REG_CALIBRATE		0xcc
>  
> -struct finger {
> +struct panel_info {
>  	u8 x_low;
>  	u8 x_high;
>  	u8 y_low;
>  	u8 y_high;
> -} __packed;
> -
> -struct touchdata {
> -	u8 status;
> -	struct finger finger[MAX_TOUCHES];
> -} __packed;
> -
> -struct panel_info {
> -	struct finger finger_max;
>  	u8 xchannel_num;
>  	u8 ychannel_num;
>  } __packed;
> @@ -74,25 +65,40 @@ static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
>  	return 0;
>  }
>  
> -static void ili210x_report_events(struct input_dev *input,
> -				  const struct touchdata *touchdata)
> +static bool ili210x_touchdata_to_coords(struct ili210x *priv, u8 *touchdata,
> +					unsigned int finger,
> +					unsigned int *x, unsigned int *y)
> +{
> +	u8 x_low, x_high, y_low, y_high;
> +
> +	if (finger >= MAX_TOUCHES)
> +		return false;
> +
> +	if (touchdata[0] & BIT(finger))
> +		return false;
> +
> +	x_high = touchdata[1 + (finger * 4) + 0];
> +	x_low = touchdata[1 + (finger * 4) + 1];

Sounds like get_unaligned_be16().

> +	y_high = touchdata[1 + (finger * 4) + 2];
> +	y_low = touchdata[1 + (finger * 4) + 3];
> +	*x = x_low | (x_high << 8);
> +	*y = y_low | (y_high << 8);
> +
> +	return true;
> +}
> +
> +static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
>  {
>  	int i;
>  	bool touch;
>  	unsigned int x, y;
> -	const struct finger *finger;
>  
>  	for (i = 0; i < MAX_TOUCHES; i++) {
>  		input_mt_slot(input, i);
>  
> -		finger = &touchdata->finger[i];
> -
> -		touch = touchdata->status & (1 << i);
> +		touch = ili210x_touchdata_to_coords(priv, touchdata, i, &x, &y);
>  		input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
>  		if (touch) {
> -			x = finger->x_low | (finger->x_high << 8);
> -			y = finger->y_low | (finger->y_high << 8);
> -
>  			input_report_abs(input, ABS_MT_POSITION_X, x);
>  			input_report_abs(input, ABS_MT_POSITION_Y, y);
>  		}
> @@ -100,6 +106,8 @@ static void ili210x_report_events(struct input_dev *input,
>  
>  	input_mt_report_pointer_emulation(input, false);
>  	input_sync(input);
> +
> +	return touchdata[0] & 0xf3;
>  }
>  
>  static void ili210x_work(struct work_struct *work)
> @@ -107,20 +115,21 @@ static void ili210x_work(struct work_struct *work)
>  	struct ili210x *priv = container_of(work, struct ili210x,
>  					    dwork.work);
>  	struct i2c_client *client = priv->client;
> -	struct touchdata touchdata;
> +	u8 touchdata[1 + 4 * MAX_TOUCHES];

Would it make sense to make the buffer part of struct ili210x and mark
it ____cacheline_aligned so that we can use dmasafe I2C APIs?

> +	bool touch;
>  	int error;
>  
>  	error = ili210x_read_reg(client, REG_TOUCHDATA,
> -				 &touchdata, sizeof(touchdata));
> +				 touchdata, sizeof(touchdata));
>  	if (error) {
>  		dev_err(&client->dev,
>  			"Unable to get touchdata, err = %d\n", error);
>  		return;
>  	}
>  
> -	ili210x_report_events(priv->input, &touchdata);
> +	touch = ili210x_report_events(priv, touchdata);
>  
> -	if (touchdata.status & 0xf3)
> +	if (touch)
>  		schedule_delayed_work(&priv->dwork,
>  				      msecs_to_jiffies(priv->poll_period));
>  }
> @@ -216,8 +225,8 @@ static int ili210x_i2c_probe(struct i2c_client *client,
>  		return error;
>  	}
>  
> -	xmax = panel.finger_max.x_low | (panel.finger_max.x_high << 8);
> -	ymax = panel.finger_max.y_low | (panel.finger_max.y_high << 8);
> +	xmax = panel.x_low | (panel.x_high << 8);
> +	ymax = panel.y_low | (panel.y_high << 8);
>  
>  	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>  	input = devm_input_allocate_device(dev);
> -- 
> 2.19.2
> 

Thanks.
Marek Vasut Dec. 21, 2018, 2:25 a.m. UTC | #2
On 12/21/2018 02:52 AM, Dmitry Torokhov wrote:
> On Thu, Dec 20, 2018 at 09:43:02PM +0100, Marek Vasut wrote:
>> Get rid of the packed structures for representing data as that does not
>> apply to other similar Ilitek touchscreens. Instead, implement a function
>> which parses the data and reports touch events and coordinates.
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Henrik Rydberg <rydberg@bitmath.org>
>> Cc: Olivier Sobrie <olivier@sobrie.be>
>> Cc: Philipp Puschmann <pp@emlix.com>
>> To: linux-input@vger.kernel.org
>> ---
>> V2: Replace touchdata.status with touchdata[0] and move it into
>>     ili210x_report_events() in anticipation of ILI251x specific changes,
>>     and to to synchronize the patch with changes in
>>     "input: touchscreen: ili210x: Convert to devm_ functions"
>> ---
>>  drivers/input/touchscreen/ili210x.c | 59 +++++++++++++++++------------
>>  1 file changed, 34 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
>> index a84ca555829e..ab0e6c9a614c 100644
>> --- a/drivers/input/touchscreen/ili210x.c
>> +++ b/drivers/input/touchscreen/ili210x.c
>> @@ -16,20 +16,11 @@
>>  #define REG_FIRMWARE_VERSION	0x40
>>  #define REG_CALIBRATE		0xcc
>>  
>> -struct finger {
>> +struct panel_info {
>>  	u8 x_low;
>>  	u8 x_high;
>>  	u8 y_low;
>>  	u8 y_high;
>> -} __packed;
>> -
>> -struct touchdata {
>> -	u8 status;
>> -	struct finger finger[MAX_TOUCHES];
>> -} __packed;
>> -
>> -struct panel_info {
>> -	struct finger finger_max;
>>  	u8 xchannel_num;
>>  	u8 ychannel_num;
>>  } __packed;
>> @@ -74,25 +65,40 @@ static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
>>  	return 0;
>>  }
>>  
>> -static void ili210x_report_events(struct input_dev *input,
>> -				  const struct touchdata *touchdata)
>> +static bool ili210x_touchdata_to_coords(struct ili210x *priv, u8 *touchdata,
>> +					unsigned int finger,
>> +					unsigned int *x, unsigned int *y)
>> +{
>> +	u8 x_low, x_high, y_low, y_high;
>> +
>> +	if (finger >= MAX_TOUCHES)
>> +		return false;
>> +
>> +	if (touchdata[0] & BIT(finger))
>> +		return false;
>> +
>> +	x_high = touchdata[1 + (finger * 4) + 0];
>> +	x_low = touchdata[1 + (finger * 4) + 1];
> 
> Sounds like get_unaligned_be16().

Fixed

>> +	y_high = touchdata[1 + (finger * 4) + 2];
>> +	y_low = touchdata[1 + (finger * 4) + 3];
>> +	*x = x_low | (x_high << 8);
>> +	*y = y_low | (y_high << 8);
>> +
>> +	return true;
>> +}
>> +
>> +static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
>>  {
>>  	int i;
>>  	bool touch;
>>  	unsigned int x, y;
>> -	const struct finger *finger;
>>  
>>  	for (i = 0; i < MAX_TOUCHES; i++) {
>>  		input_mt_slot(input, i);
>>  
>> -		finger = &touchdata->finger[i];
>> -
>> -		touch = touchdata->status & (1 << i);
>> +		touch = ili210x_touchdata_to_coords(priv, touchdata, i, &x, &y);
>>  		input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
>>  		if (touch) {
>> -			x = finger->x_low | (finger->x_high << 8);
>> -			y = finger->y_low | (finger->y_high << 8);
>> -
>>  			input_report_abs(input, ABS_MT_POSITION_X, x);
>>  			input_report_abs(input, ABS_MT_POSITION_Y, y);
>>  		}
>> @@ -100,6 +106,8 @@ static void ili210x_report_events(struct input_dev *input,
>>  
>>  	input_mt_report_pointer_emulation(input, false);
>>  	input_sync(input);
>> +
>> +	return touchdata[0] & 0xf3;
>>  }
>>  
>>  static void ili210x_work(struct work_struct *work)
>> @@ -107,20 +115,21 @@ static void ili210x_work(struct work_struct *work)
>>  	struct ili210x *priv = container_of(work, struct ili210x,
>>  					    dwork.work);
>>  	struct i2c_client *client = priv->client;
>> -	struct touchdata touchdata;
>> +	u8 touchdata[1 + 4 * MAX_TOUCHES];
> 
> Would it make sense to make the buffer part of struct ili210x and mark
> it ____cacheline_aligned so that we can use dmasafe I2C APIs?

Given how few (up to 32) bytes are transferred from the device, I'm not
sure. Would it ?
Dmitry Torokhov Dec. 21, 2018, 8:32 a.m. UTC | #3
On Fri, Dec 21, 2018 at 03:25:45AM +0100, Marek Vasut wrote:
> On 12/21/2018 02:52 AM, Dmitry Torokhov wrote:
> > On Thu, Dec 20, 2018 at 09:43:02PM +0100, Marek Vasut wrote:
> >> Get rid of the packed structures for representing data as that does not
> >> apply to other similar Ilitek touchscreens. Instead, implement a function
> >> which parses the data and reports touch events and coordinates.
> >>
> >> Signed-off-by: Marek Vasut <marex@denx.de>
> >> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> >> Cc: Henrik Rydberg <rydberg@bitmath.org>
> >> Cc: Olivier Sobrie <olivier@sobrie.be>
> >> Cc: Philipp Puschmann <pp@emlix.com>
> >> To: linux-input@vger.kernel.org
> >> ---
> >> V2: Replace touchdata.status with touchdata[0] and move it into
> >>     ili210x_report_events() in anticipation of ILI251x specific changes,
> >>     and to to synchronize the patch with changes in
> >>     "input: touchscreen: ili210x: Convert to devm_ functions"
> >> ---
> >>  drivers/input/touchscreen/ili210x.c | 59 +++++++++++++++++------------
> >>  1 file changed, 34 insertions(+), 25 deletions(-)
> >>
> >> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
> >> index a84ca555829e..ab0e6c9a614c 100644
> >> --- a/drivers/input/touchscreen/ili210x.c
> >> +++ b/drivers/input/touchscreen/ili210x.c
> >> @@ -16,20 +16,11 @@
> >>  #define REG_FIRMWARE_VERSION	0x40
> >>  #define REG_CALIBRATE		0xcc
> >>  
> >> -struct finger {
> >> +struct panel_info {
> >>  	u8 x_low;
> >>  	u8 x_high;
> >>  	u8 y_low;
> >>  	u8 y_high;
> >> -} __packed;
> >> -
> >> -struct touchdata {
> >> -	u8 status;
> >> -	struct finger finger[MAX_TOUCHES];
> >> -} __packed;
> >> -
> >> -struct panel_info {
> >> -	struct finger finger_max;
> >>  	u8 xchannel_num;
> >>  	u8 ychannel_num;
> >>  } __packed;
> >> @@ -74,25 +65,40 @@ static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
> >>  	return 0;
> >>  }
> >>  
> >> -static void ili210x_report_events(struct input_dev *input,
> >> -				  const struct touchdata *touchdata)
> >> +static bool ili210x_touchdata_to_coords(struct ili210x *priv, u8 *touchdata,
> >> +					unsigned int finger,
> >> +					unsigned int *x, unsigned int *y)
> >> +{
> >> +	u8 x_low, x_high, y_low, y_high;
> >> +
> >> +	if (finger >= MAX_TOUCHES)
> >> +		return false;
> >> +
> >> +	if (touchdata[0] & BIT(finger))
> >> +		return false;
> >> +
> >> +	x_high = touchdata[1 + (finger * 4) + 0];
> >> +	x_low = touchdata[1 + (finger * 4) + 1];
> > 
> > Sounds like get_unaligned_be16().
> 
> Fixed
> 
> >> +	y_high = touchdata[1 + (finger * 4) + 2];
> >> +	y_low = touchdata[1 + (finger * 4) + 3];
> >> +	*x = x_low | (x_high << 8);
> >> +	*y = y_low | (y_high << 8);
> >> +
> >> +	return true;
> >> +}
> >> +
> >> +static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
> >>  {
> >>  	int i;
> >>  	bool touch;
> >>  	unsigned int x, y;
> >> -	const struct finger *finger;
> >>  
> >>  	for (i = 0; i < MAX_TOUCHES; i++) {
> >>  		input_mt_slot(input, i);
> >>  
> >> -		finger = &touchdata->finger[i];
> >> -
> >> -		touch = touchdata->status & (1 << i);
> >> +		touch = ili210x_touchdata_to_coords(priv, touchdata, i, &x, &y);
> >>  		input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
> >>  		if (touch) {
> >> -			x = finger->x_low | (finger->x_high << 8);
> >> -			y = finger->y_low | (finger->y_high << 8);
> >> -
> >>  			input_report_abs(input, ABS_MT_POSITION_X, x);
> >>  			input_report_abs(input, ABS_MT_POSITION_Y, y);
> >>  		}
> >> @@ -100,6 +106,8 @@ static void ili210x_report_events(struct input_dev *input,
> >>  
> >>  	input_mt_report_pointer_emulation(input, false);
> >>  	input_sync(input);
> >> +
> >> +	return touchdata[0] & 0xf3;
> >>  }
> >>  
> >>  static void ili210x_work(struct work_struct *work)
> >> @@ -107,20 +115,21 @@ static void ili210x_work(struct work_struct *work)
> >>  	struct ili210x *priv = container_of(work, struct ili210x,
> >>  					    dwork.work);
> >>  	struct i2c_client *client = priv->client;
> >> -	struct touchdata touchdata;
> >> +	u8 touchdata[1 + 4 * MAX_TOUCHES];
> > 
> > Would it make sense to make the buffer part of struct ili210x and mark
> > it ____cacheline_aligned so that we can use dmasafe I2C APIs?
> 
> Given how few (up to 32) bytes are transferred from the device, I'm not
> sure. Would it ?

It is really up to you. We do not have many I2C masters doing DMA I
think, but they do exist.

Thanks.
Marek Vasut Dec. 21, 2018, 8:46 p.m. UTC | #4
On 12/21/2018 09:32 AM, Dmitry Torokhov wrote:
> On Fri, Dec 21, 2018 at 03:25:45AM +0100, Marek Vasut wrote:
>> On 12/21/2018 02:52 AM, Dmitry Torokhov wrote:
>>> On Thu, Dec 20, 2018 at 09:43:02PM +0100, Marek Vasut wrote:
>>>> Get rid of the packed structures for representing data as that does not
>>>> apply to other similar Ilitek touchscreens. Instead, implement a function
>>>> which parses the data and reports touch events and coordinates.
>>>>
>>>> Signed-off-by: Marek Vasut <marex@denx.de>
>>>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>>>> Cc: Henrik Rydberg <rydberg@bitmath.org>
>>>> Cc: Olivier Sobrie <olivier@sobrie.be>
>>>> Cc: Philipp Puschmann <pp@emlix.com>
>>>> To: linux-input@vger.kernel.org
>>>> ---
>>>> V2: Replace touchdata.status with touchdata[0] and move it into
>>>>     ili210x_report_events() in anticipation of ILI251x specific changes,
>>>>     and to to synchronize the patch with changes in
>>>>     "input: touchscreen: ili210x: Convert to devm_ functions"
>>>> ---
>>>>  drivers/input/touchscreen/ili210x.c | 59 +++++++++++++++++------------
>>>>  1 file changed, 34 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
>>>> index a84ca555829e..ab0e6c9a614c 100644
>>>> --- a/drivers/input/touchscreen/ili210x.c
>>>> +++ b/drivers/input/touchscreen/ili210x.c
>>>> @@ -16,20 +16,11 @@
>>>>  #define REG_FIRMWARE_VERSION	0x40
>>>>  #define REG_CALIBRATE		0xcc
>>>>  
>>>> -struct finger {
>>>> +struct panel_info {
>>>>  	u8 x_low;
>>>>  	u8 x_high;
>>>>  	u8 y_low;
>>>>  	u8 y_high;
>>>> -} __packed;
>>>> -
>>>> -struct touchdata {
>>>> -	u8 status;
>>>> -	struct finger finger[MAX_TOUCHES];
>>>> -} __packed;
>>>> -
>>>> -struct panel_info {
>>>> -	struct finger finger_max;
>>>>  	u8 xchannel_num;
>>>>  	u8 ychannel_num;
>>>>  } __packed;
>>>> @@ -74,25 +65,40 @@ static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -static void ili210x_report_events(struct input_dev *input,
>>>> -				  const struct touchdata *touchdata)
>>>> +static bool ili210x_touchdata_to_coords(struct ili210x *priv, u8 *touchdata,
>>>> +					unsigned int finger,
>>>> +					unsigned int *x, unsigned int *y)
>>>> +{
>>>> +	u8 x_low, x_high, y_low, y_high;
>>>> +
>>>> +	if (finger >= MAX_TOUCHES)
>>>> +		return false;
>>>> +
>>>> +	if (touchdata[0] & BIT(finger))
>>>> +		return false;
>>>> +
>>>> +	x_high = touchdata[1 + (finger * 4) + 0];
>>>> +	x_low = touchdata[1 + (finger * 4) + 1];
>>>
>>> Sounds like get_unaligned_be16().
>>
>> Fixed
>>
>>>> +	y_high = touchdata[1 + (finger * 4) + 2];
>>>> +	y_low = touchdata[1 + (finger * 4) + 3];
>>>> +	*x = x_low | (x_high << 8);
>>>> +	*y = y_low | (y_high << 8);
>>>> +
>>>> +	return true;
>>>> +}
>>>> +
>>>> +static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
>>>>  {
>>>>  	int i;
>>>>  	bool touch;
>>>>  	unsigned int x, y;
>>>> -	const struct finger *finger;
>>>>  
>>>>  	for (i = 0; i < MAX_TOUCHES; i++) {
>>>>  		input_mt_slot(input, i);
>>>>  
>>>> -		finger = &touchdata->finger[i];
>>>> -
>>>> -		touch = touchdata->status & (1 << i);
>>>> +		touch = ili210x_touchdata_to_coords(priv, touchdata, i, &x, &y);
>>>>  		input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
>>>>  		if (touch) {
>>>> -			x = finger->x_low | (finger->x_high << 8);
>>>> -			y = finger->y_low | (finger->y_high << 8);
>>>> -
>>>>  			input_report_abs(input, ABS_MT_POSITION_X, x);
>>>>  			input_report_abs(input, ABS_MT_POSITION_Y, y);
>>>>  		}
>>>> @@ -100,6 +106,8 @@ static void ili210x_report_events(struct input_dev *input,
>>>>  
>>>>  	input_mt_report_pointer_emulation(input, false);
>>>>  	input_sync(input);
>>>> +
>>>> +	return touchdata[0] & 0xf3;
>>>>  }
>>>>  
>>>>  static void ili210x_work(struct work_struct *work)
>>>> @@ -107,20 +115,21 @@ static void ili210x_work(struct work_struct *work)
>>>>  	struct ili210x *priv = container_of(work, struct ili210x,
>>>>  					    dwork.work);
>>>>  	struct i2c_client *client = priv->client;
>>>> -	struct touchdata touchdata;
>>>> +	u8 touchdata[1 + 4 * MAX_TOUCHES];
>>>
>>> Would it make sense to make the buffer part of struct ili210x and mark
>>> it ____cacheline_aligned so that we can use dmasafe I2C APIs?
>>
>> Given how few (up to 32) bytes are transferred from the device, I'm not
>> sure. Would it ?
> 
> It is really up to you. We do not have many I2C masters doing DMA I
> think, but they do exist.

The iMX6 one I use does not do I2C DMA, so I guess we can leave this as
is. Plus, I think the overhead of setting up DMA for 31 byte transfer is
not worth it. However, if someone proves me wrong by actually testing it
on hardware that can do I2C DMA, good :)
diff mbox series

Patch

diff --git a/drivers/input/touchscreen/ili210x.c b/drivers/input/touchscreen/ili210x.c
index a84ca555829e..ab0e6c9a614c 100644
--- a/drivers/input/touchscreen/ili210x.c
+++ b/drivers/input/touchscreen/ili210x.c
@@ -16,20 +16,11 @@ 
 #define REG_FIRMWARE_VERSION	0x40
 #define REG_CALIBRATE		0xcc
 
-struct finger {
+struct panel_info {
 	u8 x_low;
 	u8 x_high;
 	u8 y_low;
 	u8 y_high;
-} __packed;
-
-struct touchdata {
-	u8 status;
-	struct finger finger[MAX_TOUCHES];
-} __packed;
-
-struct panel_info {
-	struct finger finger_max;
 	u8 xchannel_num;
 	u8 ychannel_num;
 } __packed;
@@ -74,25 +65,40 @@  static int ili210x_read_reg(struct i2c_client *client, u8 reg, void *buf,
 	return 0;
 }
 
-static void ili210x_report_events(struct input_dev *input,
-				  const struct touchdata *touchdata)
+static bool ili210x_touchdata_to_coords(struct ili210x *priv, u8 *touchdata,
+					unsigned int finger,
+					unsigned int *x, unsigned int *y)
+{
+	u8 x_low, x_high, y_low, y_high;
+
+	if (finger >= MAX_TOUCHES)
+		return false;
+
+	if (touchdata[0] & BIT(finger))
+		return false;
+
+	x_high = touchdata[1 + (finger * 4) + 0];
+	x_low = touchdata[1 + (finger * 4) + 1];
+	y_high = touchdata[1 + (finger * 4) + 2];
+	y_low = touchdata[1 + (finger * 4) + 3];
+	*x = x_low | (x_high << 8);
+	*y = y_low | (y_high << 8);
+
+	return true;
+}
+
+static bool ili210x_report_events(struct ili210x *priv, u8 *touchdata)
 {
 	int i;
 	bool touch;
 	unsigned int x, y;
-	const struct finger *finger;
 
 	for (i = 0; i < MAX_TOUCHES; i++) {
 		input_mt_slot(input, i);
 
-		finger = &touchdata->finger[i];
-
-		touch = touchdata->status & (1 << i);
+		touch = ili210x_touchdata_to_coords(priv, touchdata, i, &x, &y);
 		input_mt_report_slot_state(input, MT_TOOL_FINGER, touch);
 		if (touch) {
-			x = finger->x_low | (finger->x_high << 8);
-			y = finger->y_low | (finger->y_high << 8);
-
 			input_report_abs(input, ABS_MT_POSITION_X, x);
 			input_report_abs(input, ABS_MT_POSITION_Y, y);
 		}
@@ -100,6 +106,8 @@  static void ili210x_report_events(struct input_dev *input,
 
 	input_mt_report_pointer_emulation(input, false);
 	input_sync(input);
+
+	return touchdata[0] & 0xf3;
 }
 
 static void ili210x_work(struct work_struct *work)
@@ -107,20 +115,21 @@  static void ili210x_work(struct work_struct *work)
 	struct ili210x *priv = container_of(work, struct ili210x,
 					    dwork.work);
 	struct i2c_client *client = priv->client;
-	struct touchdata touchdata;
+	u8 touchdata[1 + 4 * MAX_TOUCHES];
+	bool touch;
 	int error;
 
 	error = ili210x_read_reg(client, REG_TOUCHDATA,
-				 &touchdata, sizeof(touchdata));
+				 touchdata, sizeof(touchdata));
 	if (error) {
 		dev_err(&client->dev,
 			"Unable to get touchdata, err = %d\n", error);
 		return;
 	}
 
-	ili210x_report_events(priv->input, &touchdata);
+	touch = ili210x_report_events(priv, touchdata);
 
-	if (touchdata.status & 0xf3)
+	if (touch)
 		schedule_delayed_work(&priv->dwork,
 				      msecs_to_jiffies(priv->poll_period));
 }
@@ -216,8 +225,8 @@  static int ili210x_i2c_probe(struct i2c_client *client,
 		return error;
 	}
 
-	xmax = panel.finger_max.x_low | (panel.finger_max.x_high << 8);
-	ymax = panel.finger_max.y_low | (panel.finger_max.y_high << 8);
+	xmax = panel.x_low | (panel.x_high << 8);
+	ymax = panel.y_low | (panel.y_high << 8);
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	input = devm_input_allocate_device(dev);