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@xxxxxxx>
>> Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
>> Cc: Henrik Rydberg <rydberg@xxxxxxxxxxx>
>> Cc: Olivier Sobrie <olivier@xxxxxxxxx>
>> Cc: Philipp Puschmann <pp@xxxxxxxxx>
>> To: linux-input@xxxxxxxxxxxxxxx
>> ---
>> 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 ?
--
Best regards,
Marek Vasut
