Hi Jason,
Sorry to not give any comments earlier. I was kinda holding off
hoping to hear any more feedback on ABS_WHEEL2 first since it would
affect this patch.
On Fri, Oct 21, 2011 at 8:35 PM, Jason Gerecke <killertofu@xxxxxxxxx> wrote:
> Adds support for the Cintiq 24HD. There are two quirks about this
> model that haven't been seen in prior tablets. First, a second
> touch ring is present on this display; it is being exposed via the
> ABS_WHEEL2 axis. Second, three capacitive buttons at the top of
> the unit are available; though physically a touch strip, we report
> the use of these buttons with generic KEY_ events.
I've reviewed the 2nd and 3rd and I'm happy to give my Reviewed-by tag
for those.
If ABS_WHEEL2 is disliked, this patch can be amended to treat both
wheels as same ABS_WHEEL and give priority to first one in rare case
user is touching both. I know this doesn't give full feature set to
user though.
I have one it-would-be-nice comment below.
>
> Signed-off-by: Jason Gerecke <killertofu@xxxxxxxxx>
> ---
> drivers/input/tablet/wacom_wac.c | 77 +++++++++++++++++++++++++++++++++++++-
> drivers/input/tablet/wacom_wac.h | 1 +
> 2 files changed, 77 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/input/tablet/wacom_wac.c b/drivers/input/tablet/wacom_wac.c
> index 7fefd93..b830190 100644
> --- a/drivers/input/tablet/wacom_wac.c
> +++ b/drivers/input/tablet/wacom_wac.c
> @@ -452,7 +452,7 @@ static void wacom_intuos_general(struct wacom_wac *wacom)
> if ((data[1] & 0xb8) == 0xa0) {
> t = (data[6] << 2) | ((data[7] >> 6) & 3);
> if ((features->type >= INTUOS4S && features->type <= INTUOS4L) ||
> - features->type == WACOM_21UX2) {
> + features->type == WACOM_21UX2 || features->type == WACOM_24HD) {
> t = (t << 1) | (data[1] & 1);
> }
> input_report_abs(input, ABS_PRESSURE, t);
> @@ -519,6 +519,56 @@ static int wacom_intuos_irq(struct wacom_wac *wacom)
> input_report_key(input, wacom->tool[1], 0);
> input_report_abs(input, ABS_MISC, 0);
> }
> + } else if (features->type == WACOM_24HD) {
> + input_report_key(input, BTN_0, (data[6] & 0x01));
> + input_report_key(input, BTN_1, (data[6] & 0x02));
> + input_report_key(input, BTN_2, (data[6] & 0x04));
> + input_report_key(input, BTN_3, (data[6] & 0x08));
> + input_report_key(input, BTN_4, (data[6] & 0x10));
> + input_report_key(input, BTN_5, (data[6] & 0x20));
> + input_report_key(input, BTN_6, (data[6] & 0x40));
> + input_report_key(input, BTN_7, (data[6] & 0x80));
> + input_report_key(input, BTN_8, (data[8] & 0x01));
> + input_report_key(input, BTN_9, (data[8] & 0x02));
> + input_report_key(input, BTN_A, (data[8] & 0x04));
> + input_report_key(input, BTN_B, (data[8] & 0x08));
> + input_report_key(input, BTN_C, (data[8] & 0x10));
> + input_report_key(input, BTN_X, (data[8] & 0x20));
> + input_report_key(input, BTN_Y, (data[8] & 0x40));
> + input_report_key(input, BTN_Z, (data[8] & 0x80));
This function looks like it works fine and this is in line with
previous logic...but we are up to 3 packet types now for buttons and
its getting kinda hard to see packet format differences vs. userland
event sequence differences.
This is my personal preference: Use the if()'s to set a bunch of
btn_1, btn_2, abs_wheel, etc variables and afterwords do all the
input_report*()'s unconditionally to make sure those sequences remain
the same. If a specific model wasn't declared to support that key
then those functions simply returns with no harm done.
This approach helps make sure we are consistent we how events are sent
between the models.
Chris
> +
> + /*
> + * Three "buttons" are available on the 24HD which are
> + * physically implemented as a touchstrip. Each button
> + * is approximately 3 bits wide with a 2 bit spacing.
> + * The raw touchstrip bits are stored at:
> + * ((data[3] & 0x1f) << 8) | data[4])
> + */
> + input_report_key(input, KEY_PROG1, data[4] & 0x07);
> + input_report_key(input, KEY_PROG2, data[4] & 0xE0);
> + input_report_key(input, KEY_PROG3, data[3] & 0x1C);
> +
> + if (data[1] & 0x80) {
> + input_report_abs(input, ABS_WHEEL, (data[1] & 0x7f));
> + } else {
> + /* Out of proximity, clear wheel value. */
> + input_report_abs(input, ABS_WHEEL, 0);
> + }
> +
> + if (data[2] & 0x80) {
> + input_report_abs(input, ABS_WHEEL2, (data[2] & 0x7f));
> + } else {
> + /* Out of proximity, clear wheel value. */
> + input_report_abs(input, ABS_WHEEL2, 0);
> + }
> +
> + if (data[1] | data[2] | (data[3] & 0x1f) | data[4] | data[6] | data[8]) {
> + input_report_key(input, wacom->tool[1], 1);
> + input_report_abs(input, ABS_MISC, PAD_DEVICE_ID);
> + } else {
> + input_report_key(input, wacom->tool[1], 0);
> + input_report_abs(input, ABS_MISC, 0);
> + }
> } else {
> if (features->type == WACOM_21UX2) {
> input_report_key(input, BTN_0, (data[5] & 0x01));
> @@ -954,6 +1004,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> case CINTIQ:
> case WACOM_BEE:
> case WACOM_21UX2:
> + case WACOM_24HD:
> sync = wacom_intuos_irq(wacom_wac);
> break;
>
> @@ -1106,6 +1157,26 @@ void wacom_setup_input_capabilities(struct input_dev *input_dev,
> __set_bit(BTN_STYLUS2, input_dev->keybit);
> break;
>
> + case WACOM_24HD:
> + __set_bit(BTN_A, input_dev->keybit);
> + __set_bit(BTN_B, input_dev->keybit);
> + __set_bit(BTN_C, input_dev->keybit);
> + __set_bit(BTN_X, input_dev->keybit);
> + __set_bit(BTN_Y, input_dev->keybit);
> + __set_bit(BTN_Z, input_dev->keybit);
> +
> + for (i = 0; i < 10; i++)
> + __set_bit(BTN_0 + i, input_dev->keybit);
> +
> + __set_bit(KEY_PROG1, input_dev->keybit);
> + __set_bit(KEY_PROG2, input_dev->keybit);
> + __set_bit(KEY_PROG3, input_dev->keybit);
> +
> + input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
> + input_set_abs_params(input_dev, ABS_WHEEL2, 0, 71, 0, 0);
> + wacom_setup_cintiq(wacom_wac);
> + break;
> +
> case WACOM_21UX2:
> __set_bit(BTN_A, input_dev->keybit);
> __set_bit(BTN_B, input_dev->keybit);
> @@ -1403,6 +1474,9 @@ static const struct wacom_features wacom_features_0xBB =
> static const struct wacom_features wacom_features_0xBC =
> { "Wacom Intuos4 WL", WACOM_PKGLEN_INTUOS, 40840, 25400, 2047,
> 63, INTUOS4, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
> +static const struct wacom_features wacom_features_0xF4 =
> + { "Wacom Cintiq 24HD", WACOM_PKGLEN_INTUOS, 104480, 65600, 2047,
> + 63, WACOM_24HD, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
> static const struct wacom_features wacom_features_0x3F =
> { "Wacom Cintiq 21UX", WACOM_PKGLEN_INTUOS, 87200, 65600, 1023,
> 63, CINTIQ, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
> @@ -1590,6 +1664,7 @@ const struct usb_device_id wacom_ids[] = {
> { USB_DEVICE_WACOM(0xE3) },
> { USB_DEVICE_WACOM(0xE6) },
> { USB_DEVICE_WACOM(0x47) },
> + { USB_DEVICE_WACOM(0xF4) },
> { USB_DEVICE_LENOVO(0x6004) },
> { }
> };
> diff --git a/drivers/input/tablet/wacom_wac.h b/drivers/input/tablet/wacom_wac.h
> index 53eb71b..cc45d9b 100644
> --- a/drivers/input/tablet/wacom_wac.h
> +++ b/drivers/input/tablet/wacom_wac.h
> @@ -57,6 +57,7 @@ enum {
> INTUOS4S,
> INTUOS4,
> INTUOS4L,
> + WACOM_24HD,
> WACOM_21UX2,
> CINTIQ,
> WACOM_BEE,
> --
> 1.7.6
>
> --
> 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
>
--
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
