Re: [PATCH v4 2/2] Input: add MStar MSG2638 touchscreen driver
From: Vincent Knecht <hidden>
Date: 2021-02-22 20:52:59
Also in:
linux-devicetree, linux-input
Le samedi 20 février 2021 à 15:23 -0800, Dmitry Torokhov a écrit :
Hi Vincent,
Hi Dmitry, thank you for the review !
On Wed, Feb 10, 2021 at 06:33:52PM +0100, Vincent Knecht wrote:quoted
+ + for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) { + p = &touch_event.pkt[i]; + /* Ignore non-pressed finger data */ + if (p->xy_hi == 0xFF && p->x_low == 0xFF && p->y_low == 0xFF) + continue; + + coord.x = (((p->xy_hi & 0xF0) << 4) | p->x_low) * msg2638->prop.max_x / TPD_WIDTH; + coord.y = (((p->xy_hi & 0x0F) << 8) | p->y_low) * msg2638->prop.max_y / TPD_HEIGHT; + msg2638_report_finger(msg2638, i, &coord);We do not scale the coordinates in the kernel. Rather we provide resolution, if known, and min/max coordinates reported by the hardware, and let userspace handle the rest.
Ok, will remove scaling... I was able to test it's ok by setting
touchscreen-size-{x,y} = <2048>;
quoted
+static int __maybe_unused msg2638_suspend(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct msg2638_ts_data *msg2638 = i2c_get_clientdata(client); + + mutex_lock(&msg2638->input_dev->mutex); + + if (input_device_enabled(msg2638->input_dev)) + msg2638_stop(msg2638);I believe that you should power down the device only if it is not configures as wakeup source. In fact (and I think most drivers are wrong in this), you may want to power up the device if it is a wakeup source and it does not have any users.
I don't know much on this subject ; from downstream code, it seems the touchscreen supports "wakeup gestures" (like "double click", up/down/left/right directions and some characters/letters) and apparently an irq-gpio which would be used to wakeup. I don't handle that currently, is it required ?