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().
> + 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.
--
Dmitry
