Hi Henrik,
I know this patchset is under Linus tree and I've acked it already.
But, I am trying to catch up with the fast moving MT train and to make
sure the train is moving to my destination :).
More comments in line.
Ping
On Sat, Sep 4, 2010 at 6:43 AM, Henrik Rydberg <rydberg@xxxxxxxxxxx> wrote:
> Add support for the Bamboo Touch trackpad, and make it work with
> both the Synaptics X Driver and the Multitouch X Driver. The device
> uses MT slots internally, so the choice of protocol is a given.
>
> Signed-off-by: Henrik Rydberg <rydberg@xxxxxxxxxxx>
> ---
> drivers/input/tablet/wacom_wac.c | 88 ++++++++++++++++++++++++++++++++++++++
> drivers/input/tablet/wacom_wac.h | 3 +
> 2 files changed, 91 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index ca18b4d..97295aa 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -855,6 +855,54 @@ static int wacom_tpc_irq(struct wacom_wac *wacom, size_t len)
> return retval;
> }
>
> +static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
> +{
> + static int trkid;
> + struct input_dev *input = wacom->input;
> + unsigned char *data = wacom->data;
> + int sp = 0, sx = 0, sy = 0, count = 0;
> + int i;
> +
> + if (len != WACOM_PKGLEN_BBTOUCH)
> + return 0;
> +
> + for (i = 0; i < 2; i++) {
> + int p = data[9 * i + 2];
> + input_mt_slot(input, i);
> + if (p) {
> + int x = get_unaligned_be16(&data[9 * i + 3]) & 0x7ff;
> + int y = get_unaligned_be16(&data[9 * i + 5]) & 0x7ff;
> + input_report_abs(input, ABS_MT_PRESSURE, p);
> + input_report_abs(input, ABS_MT_POSITION_X, x);
> + input_report_abs(input, ABS_MT_POSITION_Y, y);
> + if (wacom->id[i] < 0)
> + wacom->id[i] = trkid++ & MAX_TRACKING_ID;
Why do we need an arbitrary MAX_TRACKING_ID when the device can tell
us how many IDs we can have and it tracks the individual fingers for
us? In this case, there are only two ID/fingers and the ID can be
retrieved from the raw data. I must be missing something in the
defintion of MAX_TRACKING_ID.
> + if (!count++)
> + sp = p, sx = x, sy = y;
> + } else {
> + wacom->id[i] = -1;
> + }
> + input_report_abs(input, ABS_MT_TRACKING_ID, wacom->id[i]);
> + }
> +
> + input_report_key(input, BTN_TOUCH, count > 0);
> + input_report_key(input, BTN_TOOL_FINGER, count == 1);
> + input_report_key(input, BTN_TOOL_DOUBLETAP, count == 2);
> +
> + input_report_abs(input, ABS_PRESSURE, sp);
> + input_report_abs(input, ABS_X, sx);
> + input_report_abs(input, ABS_Y, sy);
> +
> + input_report_key(input, BTN_LEFT, (data[1] & 0x08) != 0);
> + input_report_key(input, BTN_FORWARD, (data[1] & 0x04) != 0);
> + input_report_key(input, BTN_BACK, (data[1] & 0x02) != 0);
> + input_report_key(input, BTN_RIGHT, (data[1] & 0x01) != 0);
This implementation impairs the value of those buttons since I know a
lot of users want them configurable. If we can not or do not pass a
generic set to the userland, we will need to make it configurable in
the kernel driver as Dmitry suggested (if I remember it correctly).
Which approach, in the userland or the kernel, do you like, Henrik?
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
