* [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices
@ 2010-12-09 1:17 Ping Cheng
2010-12-09 6:42 ` Dmitry Torokhov
2010-12-09 14:51 ` Chris Bagwell
0 siblings, 2 replies; 4+ messages in thread
From: Ping Cheng @ 2010-12-09 1:17 UTC (permalink / raw)
To: linux-input; +Cc: dmitry.torokhov, Ping Cheng, Ping Cheng
Not all penabled devices support touch. The same idea is true for
touch devices. Setting up the devices according to the specific
querying results.
Signed-off-by: Ping Cheng <pingc@wacom.com>
---
drivers/input/touchscreen/wacom_w8001.c | 39 ++++++++++++++++++++++++------
1 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
index 361d00a..90b92e8 100644
--- a/drivers/input/touchscreen/wacom_w8001.c
+++ b/drivers/input/touchscreen/wacom_w8001.c
@@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
#define W8001_QUERY_PACKET 0x20
+#define W8001_CMD_STOP '0'
#define W8001_CMD_START '1'
#define W8001_CMD_QUERY '*'
#define W8001_CMD_TOUCHQUERY '%'
@@ -279,24 +280,46 @@ static int w8001_setup(struct w8001 *w8001)
struct w8001_coord coord;
int error;
- error = w8001_command(w8001, W8001_CMD_QUERY, true);
+ error = w8001_command(w8001, W8001_CMD_STOP, false);
if (error)
return error;
- parse_data(w8001->response, &coord);
+ mdelay(250); /* wait 250ms before querying the device */
- input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
- input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
- input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
- input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
- input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
+ /* penabled? */
+ error = w8001_command(w8001, W8001_CMD_QUERY, true);
+ if (!error) {
+ dev->keybit[BIT_WORD(BTN_TOOL_PEN)] |= BIT_MASK(BTN_TOOL_PEN);
+ dev->keybit[BIT_WORD(BTN_TOOL_RUBBER)] |= BIT_MASK(BTN_TOOL_RUBBER);
+ dev->keybit[BIT_WORD(BTN_STYLUS)] |= BIT_MASK(BTN_STYLUS);
+ dev->keybit[BIT_WORD(BTN_STYLUS2)] |= BIT_MASK(BTN_STYLUS2);
+ parse_data(w8001->response, &coord);
+
+ input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
+ input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
+ input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
+ if (coord.tilt_x && coord.tilt_y)
+ {
+ input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
+ input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
+ }
+ }
+ /* touch enabled? */
error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
- if (!error) {
+
+ /* Some non-touch devices may reply to the touch query. But their
+ * second byte is empty, which indicates touch is not supported.
+ */
+ if (!error && w8001->response[1]) {
struct w8001_touch_query touch;
parse_touchquery(w8001->response, &touch);
+ input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
+ input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
+ dev->keybit[BIT_WORD(BTN_TOOL_FINGER)] |= BIT_MASK(BTN_TOOL_FINGER);
+
switch (touch.sensor_id) {
case 0:
case 2:
--
1.7.2.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices
2010-12-09 1:17 [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices Ping Cheng
@ 2010-12-09 6:42 ` Dmitry Torokhov
2010-12-09 19:01 ` Ping Cheng
2010-12-09 14:51 ` Chris Bagwell
1 sibling, 1 reply; 4+ messages in thread
From: Dmitry Torokhov @ 2010-12-09 6:42 UTC (permalink / raw)
To: Ping Cheng; +Cc: linux-input, Ping Cheng
Hi Ping,
On Wed, Dec 08, 2010 at 05:17:13PM -0800, Ping Cheng wrote:
> Not all penabled devices support touch. The same idea is true for
> touch devices. Setting up the devices according to the specific
> querying results.
>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
> drivers/input/touchscreen/wacom_w8001.c | 39 ++++++++++++++++++++++++------
> 1 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 361d00a..90b92e8 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
>
> #define W8001_QUERY_PACKET 0x20
>
> +#define W8001_CMD_STOP '0'
> #define W8001_CMD_START '1'
> #define W8001_CMD_QUERY '*'
> #define W8001_CMD_TOUCHQUERY '%'
> @@ -279,24 +280,46 @@ static int w8001_setup(struct w8001 *w8001)
> struct w8001_coord coord;
> int error;
>
> - error = w8001_command(w8001, W8001_CMD_QUERY, true);
> + error = w8001_command(w8001, W8001_CMD_STOP, false);
Isn't there any response from the device to W8001_CMD_STOP?
> if (error)
> return error;
>
> - parse_data(w8001->response, &coord);
> + mdelay(250); /* wait 250ms before querying the device */
>
> - input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
> - input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> - input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
> - input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
> - input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
> + /* penabled? */
> + error = w8001_command(w8001, W8001_CMD_QUERY, true);
> + if (!error) {
> + dev->keybit[BIT_WORD(BTN_TOOL_PEN)] |= BIT_MASK(BTN_TOOL_PEN);
> + dev->keybit[BIT_WORD(BTN_TOOL_RUBBER)] |= BIT_MASK(BTN_TOOL_RUBBER);
> + dev->keybit[BIT_WORD(BTN_STYLUS)] |= BIT_MASK(BTN_STYLUS);
> + dev->keybit[BIT_WORD(BTN_STYLUS2)] |= BIT_MASK(BTN_STYLUS2);
Could we start switching to __set_bit(BTN_TOOL_PEN, dev->keybit) style?
The original way was somewhat OK when setting several buttons at once
but I feel __set_bit() form is much safer.
> + parse_data(w8001->response, &coord);
> +
> + input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> + input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
> + if (coord.tilt_x && coord.tilt_y)
> + {
Formatting: opening brace goes on the same as the statement.
> + input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
> + input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
> + }
> + }
>
> + /* touch enabled? */
> error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
> - if (!error) {
> +
> + /* Some non-touch devices may reply to the touch query. But their
> + * second byte is empty, which indicates touch is not supported.
> + */
> + if (!error && w8001->response[1]) {
> struct w8001_touch_query touch;
>
> parse_touchquery(w8001->response, &touch);
>
> + input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
> + dev->keybit[BIT_WORD(BTN_TOOL_FINGER)] |= BIT_MASK(BTN_TOOL_FINGER);
> +
> switch (touch.sensor_id) {
> case 0:
> case 2:
> --
> 1.7.2.3
>
Thanks.
--
Dmitry
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices
2010-12-09 6:42 ` Dmitry Torokhov
@ 2010-12-09 19:01 ` Ping Cheng
0 siblings, 0 replies; 4+ messages in thread
From: Ping Cheng @ 2010-12-09 19:01 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
On Wed, Dec 8, 2010 at 10:42 PM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> Hi Ping,
>
> On Wed, Dec 08, 2010 at 05:17:13PM -0800, Ping Cheng wrote:
>> Not all penabled devices support touch. The same idea is true for
>> touch devices. Setting up the devices according to the specific
>> querying results.
>>
>> Signed-off-by: Ping Cheng <pingc@wacom.com>
>> ---
>> drivers/input/touchscreen/wacom_w8001.c | 39 ++++++++++++++++++++++++------
>> 1 files changed, 31 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
>> index 361d00a..90b92e8 100644
>> --- a/drivers/input/touchscreen/wacom_w8001.c
>> +++ b/drivers/input/touchscreen/wacom_w8001.c
>> @@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
>>
>> #define W8001_QUERY_PACKET 0x20
>>
>> +#define W8001_CMD_STOP '0'
>> #define W8001_CMD_START '1'
>> #define W8001_CMD_QUERY '*'
>> #define W8001_CMD_TOUCHQUERY '%'
>> @@ -279,24 +280,46 @@ static int w8001_setup(struct w8001 *w8001)
>> struct w8001_coord coord;
>> int error;
>>
>> - error = w8001_command(w8001, W8001_CMD_QUERY, true);
>> + error = w8001_command(w8001, W8001_CMD_STOP, false);
>
> Isn't there any response from the device to W8001_CMD_STOP?
No, there is no response. We just tell the device to set ready for
QUERY command.
>> if (error)
>> return error;
>>
>> - parse_data(w8001->response, &coord);
>> + mdelay(250); /* wait 250ms before querying the device */
>>
>> - input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
>> - input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
>> - input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
>> - input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
>> - input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
>> + /* penabled? */
>> + error = w8001_command(w8001, W8001_CMD_QUERY, true);
>> + if (!error) {
>> + dev->keybit[BIT_WORD(BTN_TOOL_PEN)] |= BIT_MASK(BTN_TOOL_PEN);
>> + dev->keybit[BIT_WORD(BTN_TOOL_RUBBER)] |= BIT_MASK(BTN_TOOL_RUBBER);
>> + dev->keybit[BIT_WORD(BTN_STYLUS)] |= BIT_MASK(BTN_STYLUS);
>> + dev->keybit[BIT_WORD(BTN_STYLUS2)] |= BIT_MASK(BTN_STYLUS2);
>
> Could we start switching to __set_bit(BTN_TOOL_PEN, dev->keybit) style?
Sure, I'll make a new patch. I was following the existing code style
for consistency ......
> The original way was somewhat OK when setting several buttons at once
> but I feel __set_bit() form is much safer.
>
>> + parse_data(w8001->response, &coord);
>> +
>> + input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
>> + input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
>> + input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
>> + if (coord.tilt_x && coord.tilt_y)
>> + {
>
> Formatting: opening brace goes on the same as the statement.
Oops. I thought I've taken care of this already.
Thanks.
Ping
>> + input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
>> + input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
>> + }
>> + }
>>
>> + /* touch enabled? */
>> error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
>> - if (!error) {
>> +
>> + /* Some non-touch devices may reply to the touch query. But their
>> + * second byte is empty, which indicates touch is not supported.
>> + */
>> + if (!error && w8001->response[1]) {
>> struct w8001_touch_query touch;
>>
>> parse_touchquery(w8001->response, &touch);
>>
>> + input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
>> + input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
>> + dev->keybit[BIT_WORD(BTN_TOOL_FINGER)] |= BIT_MASK(BTN_TOOL_FINGER);
>> +
>> switch (touch.sensor_id) {
>> case 0:
>> case 2:
>> --
>> 1.7.2.3
>>
>
> Thanks.
>
> --
> Dmitry
>
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices
2010-12-09 1:17 [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices Ping Cheng
2010-12-09 6:42 ` Dmitry Torokhov
@ 2010-12-09 14:51 ` Chris Bagwell
1 sibling, 0 replies; 4+ messages in thread
From: Chris Bagwell @ 2010-12-09 14:51 UTC (permalink / raw)
To: Ping Cheng; +Cc: linux-input, dmitry.torokhov, Ping Cheng
Hi Ping,
On Wed, Dec 8, 2010 at 7:17 PM, Ping Cheng <pinglinux@gmail.com> wrote:
> Not all penabled devices support touch. The same idea is true for
> touch devices. Setting up the devices according to the specific
> querying results.
>
> Signed-off-by: Ping Cheng <pingc@wacom.com>
> ---
> drivers/input/touchscreen/wacom_w8001.c | 39 ++++++++++++++++++++++++------
> 1 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/touchscreen/wacom_w8001.c b/drivers/input/touchscreen/wacom_w8001.c
> index 361d00a..90b92e8 100644
> --- a/drivers/input/touchscreen/wacom_w8001.c
> +++ b/drivers/input/touchscreen/wacom_w8001.c
> @@ -37,6 +37,7 @@ MODULE_LICENSE("GPL");
>
> #define W8001_QUERY_PACKET 0x20
>
> +#define W8001_CMD_STOP '0'
> #define W8001_CMD_START '1'
> #define W8001_CMD_QUERY '*'
> #define W8001_CMD_TOUCHQUERY '%'
> @@ -279,24 +280,46 @@ static int w8001_setup(struct w8001 *w8001)
> struct w8001_coord coord;
> int error;
>
> - error = w8001_command(w8001, W8001_CMD_QUERY, true);
> + error = w8001_command(w8001, W8001_CMD_STOP, false);
> if (error)
> return error;
>
> - parse_data(w8001->response, &coord);
> + mdelay(250); /* wait 250ms before querying the device */
>
> - input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
> - input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> - input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
> - input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
> - input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
> + /* penabled? */
> + error = w8001_command(w8001, W8001_CMD_QUERY, true);
> + if (!error) {
> + dev->keybit[BIT_WORD(BTN_TOOL_PEN)] |= BIT_MASK(BTN_TOOL_PEN);
> + dev->keybit[BIT_WORD(BTN_TOOL_RUBBER)] |= BIT_MASK(BTN_TOOL_RUBBER);
> + dev->keybit[BIT_WORD(BTN_STYLUS)] |= BIT_MASK(BTN_STYLUS);
> + dev->keybit[BIT_WORD(BTN_STYLUS2)] |= BIT_MASK(BTN_STYLUS2);
> + parse_data(w8001->response, &coord);
> +
> + input_set_abs_params(dev, ABS_X, 0, coord.x, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, coord.y, 0, 0);
> + input_set_abs_params(dev, ABS_PRESSURE, 0, coord.pen_pressure, 0, 0);
> + if (coord.tilt_x && coord.tilt_y)
> + {
> + input_set_abs_params(dev, ABS_TILT_X, 0, coord.tilt_x, 0, 0);
> + input_set_abs_params(dev, ABS_TILT_Y, 0, coord.tilt_y, 0, 0);
> + }
> + }
>
> + /* touch enabled? */
> error = w8001_command(w8001, W8001_CMD_TOUCHQUERY, true);
> - if (!error) {
> +
> + /* Some non-touch devices may reply to the touch query. But their
> + * second byte is empty, which indicates touch is not supported.
> + */
> + if (!error && w8001->response[1]) {
> struct w8001_touch_query touch;
>
> parse_touchquery(w8001->response, &touch);
>
> + input_set_abs_params(dev, ABS_X, 0, touch.x, 0, 0);
> + input_set_abs_params(dev, ABS_Y, 0, touch.y, 0, 0);
> + dev->keybit[BIT_WORD(BTN_TOOL_FINGER)] |= BIT_MASK(BTN_TOOL_FINGER);
Sorry for such basic questions: Is this an either/or thing? Is it
either pen or touch device? Is the answer different for single vs. 2
touch devices? Doesn't this create a single input device?
If it can be combo on single input then it looks like it overrides its
shared events. Probably the easiest solution is to normalize to
single range... and I'd think that pen is higher resolution then touch
so I'd put this logic before pen logic so pens resolution wins in
combo case.
Shouldn't this also be disabling the MT events as well when touch
isn't supported?
Chris
> +
> switch (touch.sensor_id) {
> case 0:
> case 2:
> --
> 1.7.2.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-12-09 19:01 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-09 1:17 [PATCH 1/2]input - wacom_w8001: Support pen or touch only devices Ping Cheng
2010-12-09 6:42 ` Dmitry Torokhov
2010-12-09 19:01 ` Ping Cheng
2010-12-09 14:51 ` Chris Bagwell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).