LinuxLists.cc - [PATCH 1/3] HID: wacom: Move Graphire raport header check.

2012-05-15 18:32:51

Subject: [PATCH 1/3] HID: wacom: Move Graphire raport header check.

That check is valid only for Wacom Graphire, as the device raports always
start with 0x03. Intuos4 WL high-speed raports begin with 0x04, so
the check would be filtering out valid reports.

Signed-off-by: Przemo Firszt <[email protected]>
---
drivers/hid/hid-wacom.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 29372ed..4fc4eeb 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -578,13 +578,15 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
hidinput = list_entry(hdev->inputs.next, struct hid_input, list);
input = hidinput->input;

- /* Check if this is a tablet report */
- if (data[0] != 0x03)
- return 0;
-
switch (hdev->product) {
case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
- return wacom_gr_parse_report(hdev, wdata, input, data);
+ if (data[0] == 0x03) {
+ return wacom_gr_parse_report(hdev, wdata, input, data);
+ } else {
+ hid_err(hdev, "Unknown report: %d,%d size:%d\n",
+ data[0], data[1], size);
+ return 0;
+ }
break;
case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
i = 1;
--
1.7.10.1

2012-05-15 18:32:57

by Przemo Firszt

[permalink] [raw]

Subject: [PATCH 2/3] HID: wacom: Add speed setting for Intuos4 WL

Add option to change reporting speed for Intuos4 WL. The option is only
internal to the module, but it will be extended to allow control over sysfs,
as it is already implemented for Graphire.

Signed-off-by: Przemo Firszt <[email protected]>
---
drivers/hid/hid-wacom.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 4fc4eeb..5f6ce70 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -228,16 +228,25 @@ static int wacom_ac_get_property(struct power_supply *psy,
return ret;
}

-static void wacom_set_features(struct hid_device *hdev)
+static void wacom_set_features(struct hid_device *hdev, u8 speed)
{
+ struct wacom_data *wdata = hid_get_drvdata(hdev);
int ret;
__u8 rep_data[2];

- /*set high speed, tablet mode*/
+ if (speed == 1)
+ wdata->features &= ~0x20;
+ else
+ wdata->features |= 0x20;
+
rep_data[0] = 0x03;
- rep_data[1] = 0x20;
+ rep_data[1] = wdata->features;
+
ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
HID_FEATURE_REPORT);
+ if (ret >= 0)
+ wdata->high_speed = speed;
+
return;
}

@@ -719,7 +728,7 @@ static int wacom_probe(struct hid_device *hdev,
case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
wdata->features = 0;
- wacom_set_features(hdev);
+ wacom_set_features(hdev, 1);
ret = wacom_initialize_leds(hdev);
if (ret) {
hid_warn(hdev,
--
1.7.10.1

2012-05-15 18:33:00

by Przemo Firszt

[permalink] [raw]

Subject: [PATCH 3/3] HID: wacom: Unify speed setting

This patch unifies speed setting for both supported tablets. Functionality
of "wacom_poke" (used only by Graphire) is now in "wacom_set_features".
Reporting speed for both tablets can be changed by somethinkg like:

echo 1 > /sys/class/bluetooth/hci0/hci0:1/{device No}/speed

Accepted values:
0 - low speed,
1 - high speed.

The way of changing reporting speed is the same for Graphire and Intuos4 WL.

Signed-off-by: Przemo Firszt <[email protected]>
---
drivers/hid/hid-wacom.c | 102 ++++++++++++++++++++++-------------------------
1 file changed, 47 insertions(+), 55 deletions(-)

diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 5f6ce70..fe23a1e 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -231,45 +231,12 @@ static int wacom_ac_get_property(struct power_supply *psy,
static void wacom_set_features(struct hid_device *hdev, u8 speed)
{
struct wacom_data *wdata = hid_get_drvdata(hdev);
- int ret;
- __u8 rep_data[2];
-
- if (speed == 1)
- wdata->features &= ~0x20;
- else
- wdata->features |= 0x20;
-
- rep_data[0] = 0x03;
- rep_data[1] = wdata->features;
-
- ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
- HID_FEATURE_REPORT);
- if (ret >= 0)
- wdata->high_speed = speed;
-
- return;
-}
-
-static void wacom_poke(struct hid_device *hdev, u8 speed)
-{
- struct wacom_data *wdata = hid_get_drvdata(hdev);
int limit, ret;
- char rep_data[2];
-
- rep_data[0] = 0x03 ; rep_data[1] = 0x00;
- limit = 3;
- do {
- ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
- HID_FEATURE_REPORT);
- } while (ret < 0 && limit-- > 0);
-
- if (ret >= 0) {
- if (speed == 0)
- rep_data[0] = 0x05;
- else
- rep_data[0] = 0x06;
+ __u8 rep_data[2];

- rep_data[1] = 0x00;
+ switch (hdev->product) {
+ case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
+ rep_data[0] = 0x03 ; rep_data[1] = 0x00;
limit = 3;
do {
ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
@@ -277,17 +244,47 @@ static void wacom_poke(struct hid_device *hdev, u8 speed)
} while (ret < 0 && limit-- > 0);

if (ret >= 0) {
- wdata->high_speed = speed;
- return;
+ if (speed == 0)
+ rep_data[0] = 0x05;
+ else
+ rep_data[0] = 0x06;
+
+ rep_data[1] = 0x00;
+ limit = 3;
+ do {
+ ret = hdev->hid_output_raw_report(hdev,
+ rep_data, 2, HID_FEATURE_REPORT);
+ } while (ret < 0 && limit-- > 0);
+
+ if (ret >= 0) {
+ wdata->high_speed = speed;
+ return;
+ }
}
+
+ /*
+ * Note that if the raw queries fail, it's not a hard failure
+ * and it is safe to continue
+ */
+ hid_warn(hdev, "failed to poke device, command %d, err %d\n",
+ rep_data[0], ret);
+ break;
+ case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
+ if (speed == 1)
+ wdata->features &= ~0x20;
+ else
+ wdata->features |= 0x20;
+
+ rep_data[0] = 0x03;
+ rep_data[1] = wdata->features;
+
+ ret = hdev->hid_output_raw_report(hdev, rep_data, 2,
+ HID_FEATURE_REPORT);
+ if (ret >= 0)
+ wdata->high_speed = speed;
+ break;
}

- /*
- * Note that if the raw queries fail, it's not a hard failure and it
- * is safe to continue
- */
- hid_warn(hdev, "failed to poke device, command %d, err %d\n",
- rep_data[0], ret);
return;
}

@@ -311,7 +308,7 @@ static ssize_t wacom_store_speed(struct device *dev,
return -EINVAL;

if (new_speed == 0 || new_speed == 1) {
- wacom_poke(hdev, new_speed);
+ wacom_set_features(hdev, new_speed);
return strnlen(buf, PAGE_SIZE);
} else
return -EINVAL;
@@ -720,22 +717,17 @@ static int wacom_probe(struct hid_device *hdev,
hid_warn(hdev,
"can't create sysfs speed attribute err: %d\n", ret);

- switch (hdev->product) {
- case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
- /* Set Wacom mode 2 with high reporting speed */
- wacom_poke(hdev, 1);
- break;
- case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
+ wdata->features = 0;
+ wacom_set_features(hdev, 1);
+
+ if (hdev->product == USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) {
sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
- wdata->features = 0;
- wacom_set_features(hdev, 1);
ret = wacom_initialize_leds(hdev);
if (ret) {
hid_warn(hdev,
"can't create led attribute, err: %d\n", ret);
goto destroy_leds;
}
- break;
}

wdata->battery.properties = wacom_battery_props;
--
1.7.10.1

2012-05-15 18:33:31

by Przemo Firszt

[permalink] [raw]

Subject: [PATCH 3/3] HID: wacom: unify speed setting

2012-05-15 18:36:43

by Przemo Firszt

[permalink] [raw]

Subject: Re: [PATCH 3/3] HID: wacom: unify speed setting

Dnia 2012-05-15, wto o godzinie 19:32 +0100, Przemo Firszt pisze:
Eeeeh, that patch shouldn't go - the one with capital "U" is correct.
Sorry for the noise....
--
Przemo Firszt <[email protected]>

2012-05-16 16:20:05

by Ping Cheng

[permalink] [raw]

Subject: Re: [PATCH 1/3] HID: wacom: Move Graphire raport header check.

On Tue, May 15, 2012 at 11:32 AM, Przemo Firszt <[email protected]> wrote:
> That check is valid only for Wacom Graphire, as the device raports always
> start with 0x03. Intuos4 WL high-speed raports begin with 0x04, so
> the check would be filtering out valid reports.
>
> Signed-off-by: Przemo Firszt <[email protected]>

Acked-by: Ping Cheng <[email protected]> for the series.

Ping

> ---
> ?drivers/hid/hid-wacom.c | ? 12 +++++++-----
> ?1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
> index 29372ed..4fc4eeb 100644
> --- a/drivers/hid/hid-wacom.c
> +++ b/drivers/hid/hid-wacom.c
> @@ -578,13 +578,15 @@ static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
> ? ? ? ?hidinput = list_entry(hdev->inputs.next, struct hid_input, list);
> ? ? ? ?input = hidinput->input;
>
> - ? ? ? /* Check if this is a tablet report */
> - ? ? ? if (data[0] != 0x03)
> - ? ? ? ? ? ? ? return 0;
> -
> ? ? ? ?switch (hdev->product) {
> ? ? ? ?case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
> - ? ? ? ? ? ? ? return wacom_gr_parse_report(hdev, wdata, input, data);
> + ? ? ? ? ? ? ? if (data[0] == 0x03) {
> + ? ? ? ? ? ? ? ? ? ? ? return wacom_gr_parse_report(hdev, wdata, input, data);
> + ? ? ? ? ? ? ? } else {
> + ? ? ? ? ? ? ? ? ? ? ? hid_err(hdev, "Unknown report: %d,%d size:%d\n",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? data[0], data[1], size);
> + ? ? ? ? ? ? ? ? ? ? ? return 0;
> + ? ? ? ? ? ? ? }
> ? ? ? ? ? ? ? ?break;
> ? ? ? ?case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
> ? ? ? ? ? ? ? ?i = 1;
> --
> 1.7.10.1
>

2012-05-16 19:43:44

by Jiri Kosina

[permalink] [raw]

Subject: Re: [PATCH 1/3] HID: wacom: Move Graphire raport header check.

On Wed, 16 May 2012, Ping Cheng wrote:

> On Tue, May 15, 2012 at 11:32 AM, Przemo Firszt <[email protected]> wrote:
> > That check is valid only for Wacom Graphire, as the device raports always
> > start with 0x03. Intuos4 WL high-speed raports begin with 0x04, so
> > the check would be filtering out valid reports.
> >
> > Signed-off-by: Przemo Firszt <[email protected]>
>
> Acked-by: Ping Cheng <[email protected]> for the series.

I have applied the series. Thanks to both of you,

--
Jiri Kosina
SUSE Labs

2012-05-17 07:33:50

by Przemo Firszt

[permalink] [raw]

Subject: Re: [PATCH 1/3] HID: wacom: Move Graphire raport header check.

Dnia 16 Maja 2012, 8:43 pm, ?r, Jiri Kosina napisa?(a):
> On Wed, 16 May 2012, Ping Cheng wrote:
> [..]
>> Acked-by: Ping Cheng <[email protected]> for the series.
>
> I have applied the series. Thanks to both of you,
Thanks!
--
Regards,
Przemo Firszt