2014-07-24 18:14:27

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Hi Dmitry,

this is the second series I told you about for wacom.ko. This series also have
a good number of removed lines of code. \o/

The first patch is Jason's one that I finally decided to take with me. His
previous submission still applied correctly even after the moving of the files
(git is definitively awesome).

The second one is a patch I sent earlier and forgot to include in the v2 of
the first series. It might have been dropped during my many rebases. So here
he is.

The rest is for one part enhancing the battery reporting system (to make it
equal to the one in hid-wacom, and even slightly better). The other part
is the actual merge of hid-wacom into wacom which gives the same user space API
for bluetooth and USB devices, fixes the pad-in-a-separate-input-dev, and
fixes the missing tools not supported in the previous implementation of
hid-wacom for Intuos 4 BT.

Hi Jason,

I took your patch through this series, I hope you don't mind,

Hi Przemo,

Normally, this series contains all the bits of hid-wacom (except the custom
LED/OLED API). Please tell me if I am missing anything and if you like the
change.

Cheers,
Benjamin


Benjamin Tissoires (9):
Input - wacom: put a flag when the led are initialized
Input - wacom: enhance Wireless Receiver battery reporting
Input - wacom: use a uniq name for the battery device
Input - wacom: register an ac power supply for wireless devices
Input - wacom: prepare the driver to include BT devices
Input - wacom: handle Graphire BT tablets in wacom.ko
Input - wacom: handle Intuos 4 BT in wacom.ko
Input - wacom: add copyright note and bump version to 2.0
HID: remove hid-wacom Bluetooth driver

Jason Gerecke (1):
Input - wacom: Support up to 2048 pressure levels with ISDv4

drivers/hid/Kconfig | 10 +-
drivers/hid/Makefile | 3 +-
drivers/hid/hid-core.c | 2 -
drivers/hid/hid-wacom.c | 973 ------------------------------------------------
drivers/hid/wacom.h | 11 +
drivers/hid/wacom_sys.c | 217 ++++++++++-
drivers/hid/wacom_wac.c | 195 +++++++++-
drivers/hid/wacom_wac.h | 10 +
8 files changed, 415 insertions(+), 1006 deletions(-)
delete mode 100644 drivers/hid/hid-wacom.c

--
2.0.1


2014-07-24 18:14:43

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 02/10] Input - wacom: put a flag when the led are initialized

This solves a bug with the wireless receiver:
- at plug, the wireless receiver does not know which Wacom device it is
connected to, so it does not actually creates all the LEDs
- when the tablet connects, wacom->wacom_wac.features.type is set to the
proper device so that wacom_wac can understand the packets
- when the receiver is unplugged, it detects that a LED should have been
created (based on wacom->wacom_wac.features.type) and tries to remove
it: crash when removing the sysfs group.

Side effect, we can now safely call several times wacom_destroy_leds().

Signed-off-by: Benjamin Tissoires <[email protected]>
Acked-by: Ping Cheng <[email protected]>
---

no changes in v2

drivers/hid/wacom.h | 1 +
drivers/hid/wacom_sys.c | 6 ++++++
2 files changed, 7 insertions(+)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index dd67b7d..a678f82 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -118,6 +118,7 @@ struct wacom {
u8 hlv; /* status led brightness button pressed (1..127) */
u8 img_lum; /* OLED matrix display brightness */
} led;
+ bool led_initialized;
struct power_supply battery;
};

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 9dbb6dd..c857b3d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -729,12 +729,18 @@ static int wacom_initialize_leds(struct wacom *wacom)
return error;
}
wacom_led_control(wacom);
+ wacom->led_initialized = true;

return 0;
}

static void wacom_destroy_leds(struct wacom *wacom)
{
+ if (!wacom->led_initialized)
+ return;
+
+ wacom->led_initialized = false;
+
switch (wacom->wacom_wac.features.type) {
case INTUOS4S:
case INTUOS4:
--
2.0.1

2014-07-24 18:14:54

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 06/10] Input - wacom: prepare the driver to include BT devices

Now that wacom is a hid driver, there is no point in having a separate
driver for bluetooth devices.
This patch prepares the common paths of Bluetooth devices in the
common wacom driver.
It also adds the sysfs file "speed" used by Bluetooth devices.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

new in v2

drivers/hid/wacom_sys.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++---
drivers/hid/wacom_wac.h | 2 ++
2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index d0d06b8..add76ec 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -262,6 +262,12 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
return error < 0 ? error : 0;
}

+static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
+ struct wacom_features *features)
+{
+ return 0;
+}
+
/*
* Switch the tablet into its most-capable mode. Wacom tablets are
* typically configured to power-up in a mode which sends mouse-like
@@ -272,6 +278,9 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
static int wacom_query_tablet_data(struct hid_device *hdev,
struct wacom_features *features)
{
+ if (hdev->bus == BUS_BLUETOOTH)
+ return wacom_bt_query_tablet_data(hdev, 1, features);
+
if (features->device_type == BTN_TOOL_FINGER) {
if (features->type > TABLETPC) {
/* MT Tablet PC touch */
@@ -890,6 +899,38 @@ static void wacom_destroy_battery(struct wacom *wacom)
}
}

+static ssize_t wacom_show_speed(struct device *dev,
+ struct device_attribute
+ *attr, char *buf)
+{
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct wacom *wacom = hid_get_drvdata(hdev);
+
+ return snprintf(buf, PAGE_SIZE, "%i\n", wacom->wacom_wac.bt_high_speed);
+}
+
+static ssize_t wacom_store_speed(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = container_of(dev, struct hid_device, dev);
+ struct wacom *wacom = hid_get_drvdata(hdev);
+ int new_speed;
+
+ if (sscanf(buf, "%1d", &new_speed ) != 1)
+ return -EINVAL;
+
+ if (new_speed == 0 || new_speed == 1) {
+ wacom_bt_query_tablet_data(hdev, new_speed,
+ &wacom->wacom_wac.features);
+ return strnlen(buf, PAGE_SIZE);
+ } else
+ return -EINVAL;
+}
+
+static DEVICE_ATTR(speed, S_IRUGO | S_IWUSR | S_IWGRP,
+ wacom_show_speed, wacom_store_speed);
+
static struct input_dev *wacom_allocate_input(struct wacom *wacom)
{
struct input_dev *input_dev;
@@ -1210,6 +1251,9 @@ static int wacom_probe(struct hid_device *hdev,
features->y_max = 4096;
}

+ if (hdev->bus == BUS_BLUETOOTH)
+ features->quirks |= WACOM_QUIRK_BATTERY;
+
wacom_setup_device_quirks(features);

/* set unit to "100th of a mm" for devices not reported by HID */
@@ -1241,10 +1285,25 @@ static int wacom_probe(struct hid_device *hdev,
if (error)
goto fail2;

+ if (!(features->quirks & WACOM_QUIRK_MONITOR) &&
+ (features->quirks & WACOM_QUIRK_BATTERY)) {
+ error = wacom_initialize_battery(wacom);
+ if (error)
+ goto fail3;
+ }
+
if (!(features->quirks & WACOM_QUIRK_NO_INPUT)) {
error = wacom_register_inputs(wacom);
if (error)
- goto fail3;
+ goto fail4;
+ }
+
+ if (hdev->bus == BUS_BLUETOOTH) {
+ error = device_create_file(&hdev->dev, &dev_attr_speed);
+ if (error)
+ hid_warn(hdev,
+ "can't create sysfs speed attribute err: %d\n",
+ error);
}

/* Note that if query fails it is not a hard failure */
@@ -1254,7 +1313,7 @@ static int wacom_probe(struct hid_device *hdev,
error = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
if (error) {
hid_err(hdev, "hw start failed\n");
- goto fail4;
+ goto fail5;
}

if (features->quirks & WACOM_QUIRK_MONITOR)
@@ -1267,7 +1326,10 @@ static int wacom_probe(struct hid_device *hdev,

return 0;

- fail4: wacom_unregister_inputs(wacom);
+ fail5: if (hdev->bus == BUS_BLUETOOTH)
+ device_remove_file(&hdev->dev, &dev_attr_speed);
+ wacom_unregister_inputs(wacom);
+ fail4: wacom_destroy_battery(wacom);
fail3: wacom_destroy_leds(wacom);
fail2: wacom_remove_shared_data(wacom_wac);
fail1: kfree(wacom);
@@ -1283,6 +1345,8 @@ static void wacom_remove(struct hid_device *hdev)

cancel_work_sync(&wacom->work);
wacom_unregister_inputs(wacom);
+ if (hdev->bus == BUS_BLUETOOTH)
+ device_remove_file(&hdev->dev, &dev_attr_speed);
wacom_destroy_battery(wacom);
wacom_destroy_leds(wacom);
wacom_remove_shared_data(&wacom->wacom_wac);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 3433a0e..3aa55d9 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -169,6 +169,8 @@ struct wacom_wac {
int num_contacts_left;
int bat_charging;
int ps_connected;
+ __u8 bt_features;
+ __u8 bt_high_speed;
};

#endif
--
2.0.1

2014-07-24 18:14:55

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 05/10] Input - wacom: register an ac power supply for wireless devices

This is used by HID Bluetooth devices but also add some more information
to the USB Wireless Receiver.
We are just porting the bits from hid-wacom.c to the common driver here.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

new in v2

drivers/hid/wacom.h | 1 +
drivers/hid/wacom_sys.c | 58 ++++++++++++++++++++++++++++++++++++++++++++-----
drivers/hid/wacom_wac.h | 1 +
3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 99516ba..2b5562b 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -120,6 +120,7 @@ struct wacom {
} led;
bool led_initialized;
struct power_supply battery;
+ struct power_supply ac;
};

static inline void wacom_schedule_work(struct wacom_wac *wacom_wac)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index eb8705d..d0d06b8 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -774,6 +774,12 @@ static enum power_supply_property wacom_battery_props[] = {
POWER_SUPPLY_PROP_CAPACITY
};

+static enum power_supply_property wacom_ac_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_ONLINE,
+ POWER_SUPPLY_PROP_SCOPE,
+};
+
static int wacom_battery_get_property(struct power_supply *psy,
enum power_supply_property psp,
union power_supply_propval *val)
@@ -806,14 +812,38 @@ static int wacom_battery_get_property(struct power_supply *psy,
return ret;
}

+static int wacom_ac_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct wacom *wacom = container_of(psy, struct wacom, ac);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ /* fall through */
+ case POWER_SUPPLY_PROP_ONLINE:
+ val->intval = wacom->wacom_wac.ps_connected;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
static int wacom_initialize_battery(struct wacom *wacom)
{
- int error = 0;
static atomic_t battery_no = ATOMIC_INIT(0);
+ int error;
unsigned long n;

if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
n = atomic_inc_return(&battery_no) - 1;
+
wacom->battery.properties = wacom_battery_props;
wacom->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
wacom->battery.get_property = wacom_battery_get_property;
@@ -822,15 +852,31 @@ static int wacom_initialize_battery(struct wacom *wacom)
wacom->battery.type = POWER_SUPPLY_TYPE_BATTERY;
wacom->battery.use_for_apm = 0;

+ wacom->ac.properties = wacom_ac_props;
+ wacom->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
+ wacom->ac.get_property = wacom_ac_get_property;
+ sprintf(wacom->wacom_wac.ac_name, "wacom_ac_%ld", n);
+ wacom->ac.name = wacom->wacom_wac.ac_name;
+ wacom->ac.type = POWER_SUPPLY_TYPE_MAINS;
+ wacom->ac.use_for_apm = 0;
+
error = power_supply_register(&wacom->hdev->dev,
&wacom->battery);
+ if (error)
+ return error;
+
+ power_supply_powers(&wacom->battery, &wacom->hdev->dev);
+
+ error = power_supply_register(&wacom->hdev->dev, &wacom->ac);
+ if (error) {
+ power_supply_unregister(&wacom->battery);
+ return error;
+ }

- if (!error)
- power_supply_powers(&wacom->battery,
- &wacom->hdev->dev);
+ power_supply_powers(&wacom->ac, &wacom->hdev->dev);
}

- return error;
+ return 0;
}

static void wacom_destroy_battery(struct wacom *wacom)
@@ -839,6 +885,8 @@ static void wacom_destroy_battery(struct wacom *wacom)
wacom->battery.dev) {
power_supply_unregister(&wacom->battery);
wacom->battery.dev = NULL;
+ power_supply_unregister(&wacom->ac);
+ wacom->ac.dev = NULL;
}
}

diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 6cdf707..3433a0e 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -155,6 +155,7 @@ struct wacom_wac {
char name[WACOM_NAME_MAX];
char pad_name[WACOM_NAME_MAX];
char bat_name[WACOM_NAME_MAX];
+ char ac_name[WACOM_NAME_MAX];
unsigned char data[WACOM_PKGLEN_MAX];
int tool[2];
int id[2];
--
2.0.1

2014-07-24 18:15:08

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 08/10] Input - wacom: handle Intuos 4 BT in wacom.ko

A good point of this change is that now, the Intuos4 bluetooth can handle
the different tools (artpen, airbrush, mice), and we get a common interface
between USB and BT for accessing the LEDs/OLEDs.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

no or little changes in v2

drivers/hid/hid-core.c | 1 -
drivers/hid/hid-wacom.c | 1 -
drivers/hid/wacom_sys.c | 16 +++++++++++
drivers/hid/wacom_wac.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++
drivers/hid/wacom_wac.h | 1 +
5 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 4db6804..1d35e6c 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1920,7 +1920,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_3_PRO) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_DUAL_BOX_PRO) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_5_PRO) },
- { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_5_8_INCH) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_12_1_INCH) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_Q_PAD) },
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 967c457..db2d07d 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -952,7 +952,6 @@ static void wacom_remove(struct hid_device *hdev)
}

static const struct hid_device_id wacom_devices[] = {
- { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },

{ }
};
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 8492177..8c4c76a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -305,6 +305,20 @@ static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
hid_warn(hdev, "failed to poke device, command %d, err %d\n",
rep_data[0], ret);
break;
+ case INTUOS4WL:
+ if (speed == 1)
+ wacom->wacom_wac.bt_features &= ~0x20;
+ else
+ wacom->wacom_wac.bt_features |= 0x20;
+
+ rep_data[0] = 0x03;
+ rep_data[1] = wacom->wacom_wac.bt_features;
+
+ ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret >= 0)
+ wacom->wacom_wac.bt_high_speed = speed;
+ break;
}

return 0;
@@ -729,6 +743,7 @@ static int wacom_initialize_leds(struct wacom *wacom)
switch (wacom->wacom_wac.features.type) {
case INTUOS4S:
case INTUOS4:
+ case INTUOS4WL:
case INTUOS4L:
wacom->led.select[0] = 0;
wacom->led.select[1] = 0;
@@ -795,6 +810,7 @@ static void wacom_destroy_leds(struct wacom *wacom)
switch (wacom->wacom_wac.features.type) {
case INTUOS4S:
case INTUOS4:
+ case INTUOS4WL:
case INTUOS4L:
sysfs_remove_group(&wacom->hdev->dev.kobj,
&intuos4_led_attr_group);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 06bc600..1bcee15 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -34,6 +34,9 @@
8th value means AC online and show 100% capacity */
static unsigned short batcap_gr[8] = { 1, 15, 25, 35, 50, 70, 100, 100 };

+/*percent of battery capacity for Intuos4 WL, AC has a separate bit*/
+static unsigned short batcap_i4[8] = { 1, 15, 30, 45, 60, 70, 85, 100 };
+
static int wacom_penpartner_irq(struct wacom_wac *wacom)
{
unsigned char *data = wacom->data;
@@ -953,6 +956,58 @@ static int int_dist(int x1, int y1, int x2, int y2)
return int_sqrt(x*x + y*y);
}

+static void wacom_intuos_bt_process_data(struct wacom_wac *wacom,
+ unsigned char *data)
+{
+ memcpy(wacom->data, data, 10);
+ wacom_intuos_irq(wacom);
+
+ input_sync(wacom->input);
+ if (wacom->pad_input)
+ input_sync(wacom->pad_input);
+}
+
+static int wacom_intuos_bt_irq(struct wacom_wac *wacom, size_t len)
+{
+ unsigned char data[WACOM_PKGLEN_MAX];
+ int i = 1;
+ unsigned power_raw, battery_capacity, bat_charging, ps_connected;
+
+ memcpy(data, wacom->data, len);
+
+ switch (data[0]) {
+ case 0x04:
+ wacom_intuos_bt_process_data(wacom, data + i);
+ i += 10;
+ /* fall through */
+ case 0x03:
+ wacom_intuos_bt_process_data(wacom, data + i);
+ i += 10;
+ wacom_intuos_bt_process_data(wacom, data + i);
+ i += 10;
+ power_raw = data[i];
+ bat_charging = (power_raw & 0x08) ? 1 : 0;
+ ps_connected = (power_raw & 0x10) ? 1 : 0;
+ battery_capacity = batcap_i4[power_raw & 0x07];
+ if ((wacom->battery_capacity != battery_capacity) ||
+ (wacom->bat_charging != bat_charging) ||
+ (wacom->ps_connected != ps_connected)) {
+ wacom->battery_capacity = battery_capacity;
+ wacom->bat_charging = bat_charging;
+ wacom->ps_connected = ps_connected;
+ wacom_notify_battery(wacom);
+ }
+
+ break;
+ default:
+ dev_dbg(wacom->input->dev.parent,
+ "Unknown report: %d,%d size:%li\n",
+ data[0], data[1], len);
+ return 0;
+ }
+ return 0;
+}
+
static int wacom_24hdt_irq(struct wacom_wac *wacom)
{
struct input_dev *input = wacom->input;
@@ -1512,6 +1567,10 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
sync = wacom_intuos_irq(wacom_wac);
break;

+ case INTUOS4WL:
+ sync = wacom_intuos_bt_irq(wacom_wac, len);
+ break;
+
case WACOM_24HDT:
sync = wacom_24hdt_irq(wacom_wac);
break;
@@ -1803,6 +1862,7 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
break;

case INTUOS4:
+ case INTUOS4WL:
case INTUOS4L:
case INTUOS4S:
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
@@ -2051,6 +2111,15 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
input_set_abs_params(input_dev, ABS_WHEEL, 0, 71, 0, 0);
break;

+ case INTUOS4WL:
+ /*
+ * For Bluetooth devices, the udev rule does not work correctly
+ * for pads unless we add a stylus capability, which forces
+ * ID_INPUT_TABLET to be set.
+ */
+ __set_bit(BTN_STYLUS, input_dev->keybit);
+ /* fall through */
+
case INTUOS4:
case INTUOS4L:
__set_bit(BTN_7, input_dev->keybit);
@@ -2265,6 +2334,9 @@ static const struct wacom_features wacom_features_0xBB =
static const struct wacom_features wacom_features_0xBC =
{ "Wacom Intuos4 WL", 40640, 25400, 2047, 63,
INTUOS4, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
+static const struct wacom_features wacom_features_0xBD =
+ { "Wacom Intuos4 WL", 40640, 25400, 2047, 63,
+ INTUOS4WL, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES };
static const struct wacom_features wacom_features_0x26 =
{ "Wacom Intuos5 touch S", 31496, 19685, 2047, 63,
INTUOS5S, WACOM_INTUOS3_RES, WACOM_INTUOS3_RES, .touch_max = 16 };
@@ -2581,6 +2653,7 @@ const struct hid_device_id wacom_ids[] = {
{ USB_DEVICE_WACOM(0xBA) },
{ USB_DEVICE_WACOM(0xBB) },
{ USB_DEVICE_WACOM(0xBC) },
+ { BT_DEVICE_WACOM(0xBD) },
{ USB_DEVICE_WACOM(0xC0) },
{ USB_DEVICE_WACOM(0xC2) },
{ USB_DEVICE_WACOM(0xC4) },
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 0f3df34..5039357 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -86,6 +86,7 @@ enum {
INTUOS3L,
INTUOS4S,
INTUOS4,
+ INTUOS4WL,
INTUOS4L,
INTUOS5S,
INTUOS5,
--
2.0.1

2014-07-24 18:15:21

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 10/10] HID: remove hid-wacom Bluetooth driver

Bluetooth Wacom tablets are now handled by the regular wacom.ko driver.
Remove the now useless hid-wacom driver.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

no changes in v2

drivers/hid/Kconfig | 10 +-
drivers/hid/Makefile | 3 +-
drivers/hid/hid-wacom.c | 971 ------------------------------------------------
3 files changed, 2 insertions(+), 982 deletions(-)
delete mode 100644 drivers/hid/hid-wacom.c

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 5a7517b..5586b61 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -737,21 +737,13 @@ config THRUSTMASTER_FF
Rumble Force or Force Feedback Wheel.

config HID_WACOM
- tristate "Wacom Bluetooth devices support"
- depends on HID
- depends on LEDS_CLASS
- select POWER_SUPPLY
- ---help---
- Support for Wacom Graphire Bluetooth and Intuos4 WL tablets.
-
-config HID_USB_WACOM
tristate "Wacom Intuos/Graphire tablet support (USB)"
depends on HID
select POWER_SUPPLY
select NEW_LEDS
select LEDS_CLASS
help
- Say Y here if you want to use the USB version of the Wacom Intuos
+ Say Y here if you want to use the USB or BT version of the Wacom Intuos
or Graphire tablet.

To compile this driver as a module, choose M here: the
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index a9e9f48..ccb3c80 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -114,10 +114,9 @@ obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
obj-$(CONFIG_HID_XINMO) += hid-xinmo.o
obj-$(CONFIG_HID_ZEROPLUS) += hid-zpff.o
obj-$(CONFIG_HID_ZYDACRON) += hid-zydacron.o
-obj-$(CONFIG_HID_WACOM) += hid-wacom.o

wacom-objs := wacom_wac.o wacom_sys.o
-obj-$(CONFIG_HID_USB_WACOM) += wacom.o
+obj-$(CONFIG_HID_WACOM) += wacom.o
obj-$(CONFIG_HID_WALTOP) += hid-waltop.o
obj-$(CONFIG_HID_WIIMOTE) += hid-wiimote.o
obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
deleted file mode 100644
index db2d07d..0000000
--- a/drivers/hid/hid-wacom.c
+++ /dev/null
@@ -1,971 +0,0 @@
-/*
- * Bluetooth Wacom Tablet support
- *
- * Copyright (c) 1999 Andreas Gal
- * Copyright (c) 2000-2005 Vojtech Pavlik <[email protected]>
- * Copyright (c) 2005 Michael Haboustak <[email protected]> for Concept2, Inc
- * Copyright (c) 2006-2007 Jiri Kosina
- * Copyright (c) 2008 Jiri Slaby <[email protected]>
- * Copyright (c) 2006 Andrew Zabolotny <[email protected]>
- * Copyright (c) 2009 Bastien Nocera <[email protected]>
- * Copyright (c) 2011 Przemysław Firszt <[email protected]>
- */
-
-/*
- * This program is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the Free
- * Software Foundation; either version 2 of the License, or (at your option)
- * any later version.
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/device.h>
-#include <linux/hid.h>
-#include <linux/module.h>
-#include <linux/leds.h>
-#include <linux/slab.h>
-#include <linux/power_supply.h>
-
-#include "hid-ids.h"
-
-#define PAD_DEVICE_ID 0x0F
-
-#define WAC_CMD_LED_CONTROL 0x20
-#define WAC_CMD_ICON_START_STOP 0x21
-#define WAC_CMD_ICON_TRANSFER 0x26
-
-struct wacom_data {
- __u16 tool;
- __u16 butstate;
- __u8 whlstate;
- __u8 features;
- __u32 id;
- __u32 serial;
- unsigned char high_speed;
- __u8 battery_capacity;
- __u8 power_raw;
- __u8 ps_connected;
- __u8 bat_charging;
- struct power_supply battery;
- struct power_supply ac;
- __u8 led_selector;
- struct led_classdev *leds[4];
-};
-
-/*percent of battery capacity for Graphire
- 8th value means AC online and show 100% capacity */
-static unsigned short batcap_gr[8] = { 1, 15, 25, 35, 50, 70, 100, 100 };
-/*percent of battery capacity for Intuos4 WL, AC has a separate bit*/
-static unsigned short batcap_i4[8] = { 1, 15, 30, 45, 60, 70, 85, 100 };
-
-static enum power_supply_property wacom_battery_props[] = {
- POWER_SUPPLY_PROP_PRESENT,
- POWER_SUPPLY_PROP_CAPACITY,
- POWER_SUPPLY_PROP_SCOPE,
- POWER_SUPPLY_PROP_STATUS,
-};
-
-static enum power_supply_property wacom_ac_props[] = {
- POWER_SUPPLY_PROP_PRESENT,
- POWER_SUPPLY_PROP_ONLINE,
- POWER_SUPPLY_PROP_SCOPE,
-};
-
-static void wacom_scramble(__u8 *image)
-{
- __u16 mask;
- __u16 s1;
- __u16 s2;
- __u16 r1 ;
- __u16 r2 ;
- __u16 r;
- __u8 buf[256];
- int i, w, x, y, z;
-
- for (x = 0; x < 32; x++) {
- for (y = 0; y < 8; y++)
- buf[(8 * x) + (7 - y)] = image[(8 * x) + y];
- }
-
- /* Change 76543210 into GECA6420 as required by Intuos4 WL
- * HGFEDCBA HFDB7531
- */
- for (x = 0; x < 4; x++) {
- for (y = 0; y < 4; y++) {
- for (z = 0; z < 8; z++) {
- mask = 0x0001;
- r1 = 0;
- r2 = 0;
- i = (x << 6) + (y << 4) + z;
- s1 = buf[i];
- s2 = buf[i+8];
- for (w = 0; w < 8; w++) {
- r1 |= (s1 & mask);
- r2 |= (s2 & mask);
- s1 <<= 1;
- s2 <<= 1;
- mask <<= 2;
- }
- r = r1 | (r2 << 1);
- i = (x << 6) + (y << 4) + (z << 1);
- image[i] = 0xFF & r;
- image[i+1] = (0xFF00 & r) >> 8;
- }
- }
- }
-}
-
-static void wacom_set_image(struct hid_device *hdev, const char *image,
- __u8 icon_no)
-{
- __u8 rep_data[68];
- __u8 p[256];
- int ret, i, j;
-
- for (i = 0; i < 256; i++)
- p[i] = image[i];
-
- rep_data[0] = WAC_CMD_ICON_START_STOP;
- rep_data[1] = 0;
- ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
- HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
- if (ret < 0)
- goto err;
-
- rep_data[0] = WAC_CMD_ICON_TRANSFER;
- rep_data[1] = icon_no & 0x07;
-
- wacom_scramble(p);
-
- for (i = 0; i < 4; i++) {
- for (j = 0; j < 64; j++)
- rep_data[j + 3] = p[(i << 6) + j];
-
- rep_data[2] = i;
- ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 67,
- HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
- }
-
- rep_data[0] = WAC_CMD_ICON_START_STOP;
- rep_data[1] = 0;
-
- ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
- HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
-
-err:
- return;
-}
-
-static void wacom_leds_set_brightness(struct led_classdev *led_dev,
- enum led_brightness value)
-{
- struct device *dev = led_dev->dev->parent;
- struct hid_device *hdev;
- struct wacom_data *wdata;
- unsigned char *buf;
- __u8 led = 0;
- int i;
-
- hdev = container_of(dev, struct hid_device, dev);
- wdata = hid_get_drvdata(hdev);
- for (i = 0; i < 4; ++i) {
- if (wdata->leds[i] == led_dev)
- wdata->led_selector = i;
- }
-
- led = wdata->led_selector | 0x04;
- buf = kzalloc(9, GFP_KERNEL);
- if (buf) {
- buf[0] = WAC_CMD_LED_CONTROL;
- buf[1] = led;
- buf[2] = value >> 2;
- buf[3] = value;
- /* use fixed brightness for OLEDs */
- buf[4] = 0x08;
- hid_hw_raw_request(hdev, buf[0], buf, 9, HID_FEATURE_REPORT,
- HID_REQ_SET_REPORT);
- kfree(buf);
- }
-
- return;
-}
-
-static enum led_brightness wacom_leds_get_brightness(struct led_classdev *led_dev)
-{
- struct wacom_data *wdata;
- struct device *dev = led_dev->dev->parent;
- int value = 0;
- int i;
-
- wdata = hid_get_drvdata(container_of(dev, struct hid_device, dev));
-
- for (i = 0; i < 4; ++i) {
- if (wdata->leds[i] == led_dev) {
- value = wdata->leds[i]->brightness;
- break;
- }
- }
-
- return value;
-}
-
-
-static int wacom_initialize_leds(struct hid_device *hdev)
-{
- struct wacom_data *wdata = hid_get_drvdata(hdev);
- struct led_classdev *led;
- struct device *dev = &hdev->dev;
- size_t namesz = strlen(dev_name(dev)) + 12;
- char *name;
- int i, ret;
-
- wdata->led_selector = 0;
-
- for (i = 0; i < 4; i++) {
- led = kzalloc(sizeof(struct led_classdev) + namesz, GFP_KERNEL);
- if (!led) {
- hid_warn(hdev,
- "can't allocate memory for LED selector\n");
- ret = -ENOMEM;
- goto err;
- }
-
- name = (void *)&led[1];
- snprintf(name, namesz, "%s:selector:%d", dev_name(dev), i);
- led->name = name;
- led->brightness = 0;
- led->max_brightness = 127;
- led->brightness_get = wacom_leds_get_brightness;
- led->brightness_set = wacom_leds_set_brightness;
-
- wdata->leds[i] = led;
-
- ret = led_classdev_register(dev, wdata->leds[i]);
-
- if (ret) {
- wdata->leds[i] = NULL;
- kfree(led);
- hid_warn(hdev, "can't register LED\n");
- goto err;
- }
- }
-
-err:
- return ret;
-}
-
-static void wacom_destroy_leds(struct hid_device *hdev)
-{
- struct wacom_data *wdata = hid_get_drvdata(hdev);
- struct led_classdev *led;
- int i;
-
- for (i = 0; i < 4; ++i) {
- if (wdata->leds[i]) {
- led = wdata->leds[i];
- wdata->leds[i] = NULL;
- led_classdev_unregister(led);
- kfree(led);
- }
- }
-
-}
-
-static int wacom_battery_get_property(struct power_supply *psy,
- enum power_supply_property psp,
- union power_supply_propval *val)
-{
- struct wacom_data *wdata = container_of(psy,
- struct wacom_data, battery);
- int ret = 0;
-
- switch (psp) {
- case POWER_SUPPLY_PROP_PRESENT:
- val->intval = 1;
- break;
- case POWER_SUPPLY_PROP_SCOPE:
- val->intval = POWER_SUPPLY_SCOPE_DEVICE;
- break;
- case POWER_SUPPLY_PROP_CAPACITY:
- val->intval = wdata->battery_capacity;
- break;
- case POWER_SUPPLY_PROP_STATUS:
- if (wdata->bat_charging)
- val->intval = POWER_SUPPLY_STATUS_CHARGING;
- else
- if (wdata->battery_capacity == 100 && wdata->ps_connected)
- val->intval = POWER_SUPPLY_STATUS_FULL;
- else
- val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
- break;
- default:
- ret = -EINVAL;
- break;
- }
- return ret;
-}
-
-static int wacom_ac_get_property(struct power_supply *psy,
- enum power_supply_property psp,
- union power_supply_propval *val)
-{
- struct wacom_data *wdata = container_of(psy, struct wacom_data, ac);
- int ret = 0;
-
- switch (psp) {
- case POWER_SUPPLY_PROP_PRESENT:
- /* fall through */
- case POWER_SUPPLY_PROP_ONLINE:
- val->intval = wdata->ps_connected;
- break;
- case POWER_SUPPLY_PROP_SCOPE:
- val->intval = POWER_SUPPLY_SCOPE_DEVICE;
- break;
- default:
- ret = -EINVAL;
- break;
- }
- return ret;
-}
-
-static void wacom_set_features(struct hid_device *hdev, u8 speed)
-{
- struct wacom_data *wdata = hid_get_drvdata(hdev);
- int limit, ret;
- __u8 rep_data[2];
-
- switch (hdev->product) {
- case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
- rep_data[0] = 0x03 ; rep_data[1] = 0x00;
- limit = 3;
- do {
- ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
- HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
- } while (ret < 0 && limit-- > 0);
-
- if (ret >= 0) {
- if (speed == 0)
- rep_data[0] = 0x05;
- else
- rep_data[0] = 0x06;
-
- rep_data[1] = 0x00;
- limit = 3;
- do {
- ret = hid_hw_raw_request(hdev, rep_data[0],
- rep_data, 2, HID_FEATURE_REPORT,
- HID_REQ_SET_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 = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
- HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
- if (ret >= 0)
- wdata->high_speed = speed;
- break;
- }
-
- return;
-}
-
-static ssize_t wacom_show_speed(struct device *dev,
- struct device_attribute
- *attr, char *buf)
-{
- struct wacom_data *wdata = dev_get_drvdata(dev);
-
- return snprintf(buf, PAGE_SIZE, "%i\n", wdata->high_speed);
-}
-
-static ssize_t wacom_store_speed(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct hid_device *hdev = container_of(dev, struct hid_device, dev);
- int new_speed;
-
- if (sscanf(buf, "%1d", &new_speed ) != 1)
- return -EINVAL;
-
- if (new_speed == 0 || new_speed == 1) {
- wacom_set_features(hdev, new_speed);
- return strnlen(buf, PAGE_SIZE);
- } else
- return -EINVAL;
-}
-
-static DEVICE_ATTR(speed, S_IRUGO | S_IWUSR | S_IWGRP,
- wacom_show_speed, wacom_store_speed);
-
-#define WACOM_STORE(OLED_ID) \
-static ssize_t wacom_oled##OLED_ID##_store(struct device *dev, \
- struct device_attribute *attr, \
- const char *buf, size_t count) \
-{ \
- struct hid_device *hdev = container_of(dev, struct hid_device, \
- dev); \
- \
- if (count != 256) \
- return -EINVAL; \
- \
- wacom_set_image(hdev, buf, OLED_ID); \
- \
- return count; \
-} \
- \
-static DEVICE_ATTR(oled##OLED_ID##_img, S_IWUSR | S_IWGRP, NULL, \
- wacom_oled##OLED_ID##_store)
-
-WACOM_STORE(0);
-WACOM_STORE(1);
-WACOM_STORE(2);
-WACOM_STORE(3);
-WACOM_STORE(4);
-WACOM_STORE(5);
-WACOM_STORE(6);
-WACOM_STORE(7);
-
-static int wacom_gr_parse_report(struct hid_device *hdev,
- struct wacom_data *wdata,
- struct input_dev *input, unsigned char *data)
-{
- int tool, x, y, rw;
-
- tool = 0;
- /* Get X & Y positions */
- x = le16_to_cpu(*(__le16 *) &data[2]);
- y = le16_to_cpu(*(__le16 *) &data[4]);
-
- /* Get current tool identifier */
- if (data[1] & 0x90) { /* If pen is in the in/active area */
- switch ((data[1] >> 5) & 3) {
- case 0: /* Pen */
- tool = BTN_TOOL_PEN;
- break;
-
- case 1: /* Rubber */
- tool = BTN_TOOL_RUBBER;
- break;
-
- case 2: /* Mouse with wheel */
- case 3: /* Mouse without wheel */
- tool = BTN_TOOL_MOUSE;
- break;
- }
-
- /* Reset tool if out of active tablet area */
- if (!(data[1] & 0x10))
- tool = 0;
- }
-
- /* If tool changed, notify input subsystem */
- if (wdata->tool != tool) {
- if (wdata->tool) {
- /* Completely reset old tool state */
- if (wdata->tool == BTN_TOOL_MOUSE) {
- input_report_key(input, BTN_LEFT, 0);
- input_report_key(input, BTN_RIGHT, 0);
- input_report_key(input, BTN_MIDDLE, 0);
- input_report_abs(input, ABS_DISTANCE,
- input_abs_get_max(input, ABS_DISTANCE));
- } else {
- input_report_key(input, BTN_TOUCH, 0);
- input_report_key(input, BTN_STYLUS, 0);
- input_report_key(input, BTN_STYLUS2, 0);
- input_report_abs(input, ABS_PRESSURE, 0);
- }
- input_report_key(input, wdata->tool, 0);
- input_sync(input);
- }
- wdata->tool = tool;
- if (tool)
- input_report_key(input, tool, 1);
- }
-
- if (tool) {
- input_report_abs(input, ABS_X, x);
- input_report_abs(input, ABS_Y, y);
-
- switch ((data[1] >> 5) & 3) {
- case 2: /* Mouse with wheel */
- input_report_key(input, BTN_MIDDLE, data[1] & 0x04);
- rw = (data[6] & 0x01) ? -1 :
- (data[6] & 0x02) ? 1 : 0;
- input_report_rel(input, REL_WHEEL, rw);
- /* fall through */
-
- case 3: /* Mouse without wheel */
- input_report_key(input, BTN_LEFT, data[1] & 0x01);
- input_report_key(input, BTN_RIGHT, data[1] & 0x02);
- /* Compute distance between mouse and tablet */
- rw = 44 - (data[6] >> 2);
- if (rw < 0)
- rw = 0;
- else if (rw > 31)
- rw = 31;
- input_report_abs(input, ABS_DISTANCE, rw);
- break;
-
- default:
- input_report_abs(input, ABS_PRESSURE,
- data[6] | (((__u16) (data[1] & 0x08)) << 5));
- input_report_key(input, BTN_TOUCH, data[1] & 0x01);
- input_report_key(input, BTN_STYLUS, data[1] & 0x02);
- input_report_key(input, BTN_STYLUS2, (tool == BTN_TOOL_PEN) && data[1] & 0x04);
- break;
- }
-
- input_sync(input);
- }
-
- /* Report the state of the two buttons at the top of the tablet
- * as two extra fingerpad keys (buttons 4 & 5). */
- rw = data[7] & 0x03;
- if (rw != wdata->butstate) {
- wdata->butstate = rw;
- input_report_key(input, BTN_0, rw & 0x02);
- input_report_key(input, BTN_1, rw & 0x01);
- input_report_key(input, BTN_TOOL_FINGER, 0xf0);
- input_event(input, EV_MSC, MSC_SERIAL, 0xf0);
- input_sync(input);
- }
-
- /* Store current battery capacity and power supply state*/
- rw = (data[7] >> 2 & 0x07);
- if (rw != wdata->power_raw) {
- wdata->power_raw = rw;
- wdata->battery_capacity = batcap_gr[rw];
- if (rw == 7)
- wdata->ps_connected = 1;
- else
- wdata->ps_connected = 0;
- }
- return 1;
-}
-
-static void wacom_i4_parse_button_report(struct wacom_data *wdata,
- struct input_dev *input, unsigned char *data)
-{
- __u16 new_butstate;
- __u8 new_whlstate;
- __u8 sync = 0;
-
- new_whlstate = data[1];
- if (new_whlstate != wdata->whlstate) {
- wdata->whlstate = new_whlstate;
- if (new_whlstate & 0x80) {
- input_report_key(input, BTN_TOUCH, 1);
- input_report_abs(input, ABS_WHEEL, (new_whlstate & 0x7f));
- input_report_key(input, BTN_TOOL_FINGER, 1);
- } else {
- input_report_key(input, BTN_TOUCH, 0);
- input_report_abs(input, ABS_WHEEL, 0);
- input_report_key(input, BTN_TOOL_FINGER, 0);
- }
- sync = 1;
- }
-
- new_butstate = (data[3] << 1) | (data[2] & 0x01);
- if (new_butstate != wdata->butstate) {
- wdata->butstate = new_butstate;
- input_report_key(input, BTN_0, new_butstate & 0x001);
- input_report_key(input, BTN_1, new_butstate & 0x002);
- input_report_key(input, BTN_2, new_butstate & 0x004);
- input_report_key(input, BTN_3, new_butstate & 0x008);
- input_report_key(input, BTN_4, new_butstate & 0x010);
- input_report_key(input, BTN_5, new_butstate & 0x020);
- input_report_key(input, BTN_6, new_butstate & 0x040);
- input_report_key(input, BTN_7, new_butstate & 0x080);
- input_report_key(input, BTN_8, new_butstate & 0x100);
- input_report_key(input, BTN_TOOL_FINGER, 1);
- sync = 1;
- }
-
- if (sync) {
- input_report_abs(input, ABS_MISC, PAD_DEVICE_ID);
- input_event(input, EV_MSC, MSC_SERIAL, 0xffffffff);
- input_sync(input);
- }
-}
-
-static void wacom_i4_parse_pen_report(struct wacom_data *wdata,
- struct input_dev *input, unsigned char *data)
-{
- __u16 x, y, pressure;
- __u8 distance;
- __u8 tilt_x, tilt_y;
-
- switch (data[1]) {
- case 0x80: /* Out of proximity report */
- input_report_key(input, BTN_TOUCH, 0);
- input_report_abs(input, ABS_PRESSURE, 0);
- input_report_key(input, BTN_STYLUS, 0);
- input_report_key(input, BTN_STYLUS2, 0);
- input_report_key(input, wdata->tool, 0);
- input_report_abs(input, ABS_MISC, 0);
- input_event(input, EV_MSC, MSC_SERIAL, wdata->serial);
- wdata->tool = 0;
- input_sync(input);
- break;
- case 0xC2: /* Tool report */
- wdata->id = ((data[2] << 4) | (data[3] >> 4) |
- ((data[7] & 0x0f) << 20) |
- ((data[8] & 0xf0) << 12));
- wdata->serial = ((data[3] & 0x0f) << 28) +
- (data[4] << 20) + (data[5] << 12) +
- (data[6] << 4) + (data[7] >> 4);
-
- switch (wdata->id) {
- case 0x100802:
- wdata->tool = BTN_TOOL_PEN;
- break;
- case 0x10080A:
- wdata->tool = BTN_TOOL_RUBBER;
- break;
- }
- break;
- default: /* Position/pressure report */
- x = data[2] << 9 | data[3] << 1 | ((data[9] & 0x02) >> 1);
- y = data[4] << 9 | data[5] << 1 | (data[9] & 0x01);
- pressure = (data[6] << 3) | ((data[7] & 0xC0) >> 5)
- | (data[1] & 0x01);
- distance = (data[9] >> 2) & 0x3f;
- tilt_x = ((data[7] << 1) & 0x7e) | (data[8] >> 7);
- tilt_y = data[8] & 0x7f;
-
- input_report_key(input, BTN_TOUCH, pressure > 1);
-
- input_report_key(input, BTN_STYLUS, data[1] & 0x02);
- input_report_key(input, BTN_STYLUS2, data[1] & 0x04);
- input_report_key(input, wdata->tool, 1);
- input_report_abs(input, ABS_X, x);
- input_report_abs(input, ABS_Y, y);
- input_report_abs(input, ABS_PRESSURE, pressure);
- input_report_abs(input, ABS_DISTANCE, distance);
- input_report_abs(input, ABS_TILT_X, tilt_x);
- input_report_abs(input, ABS_TILT_Y, tilt_y);
- input_report_abs(input, ABS_MISC, wdata->id);
- input_event(input, EV_MSC, MSC_SERIAL, wdata->serial);
- input_report_key(input, wdata->tool, 1);
- input_sync(input);
- break;
- }
-
- return;
-}
-
-static void wacom_i4_parse_report(struct hid_device *hdev,
- struct wacom_data *wdata,
- struct input_dev *input, unsigned char *data)
-{
- switch (data[0]) {
- case 0x00: /* Empty report */
- break;
- case 0x02: /* Pen report */
- wacom_i4_parse_pen_report(wdata, input, data);
- break;
- case 0x03: /* Features Report */
- wdata->features = data[2];
- break;
- case 0x0C: /* Button report */
- wacom_i4_parse_button_report(wdata, input, data);
- break;
- default:
- hid_err(hdev, "Unknown report: %d,%d\n", data[0], data[1]);
- break;
- }
-}
-
-static int wacom_raw_event(struct hid_device *hdev, struct hid_report *report,
- u8 *raw_data, int size)
-{
- struct wacom_data *wdata = hid_get_drvdata(hdev);
- struct hid_input *hidinput;
- struct input_dev *input;
- unsigned char *data = (unsigned char *) raw_data;
- int i;
- __u8 power_raw;
-
- if (!(hdev->claimed & HID_CLAIMED_INPUT))
- return 0;
-
- hidinput = list_entry(hdev->inputs.next, struct hid_input, list);
- input = hidinput->input;
-
- switch (hdev->product) {
- case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
- 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;
-
- switch (data[0]) {
- case 0x04:
- wacom_i4_parse_report(hdev, wdata, input, data + i);
- i += 10;
- /* fall through */
- case 0x03:
- wacom_i4_parse_report(hdev, wdata, input, data + i);
- i += 10;
- wacom_i4_parse_report(hdev, wdata, input, data + i);
- power_raw = data[i+10];
- if (power_raw != wdata->power_raw) {
- wdata->power_raw = power_raw;
- wdata->battery_capacity = batcap_i4[power_raw & 0x07];
- wdata->bat_charging = (power_raw & 0x08) ? 1 : 0;
- wdata->ps_connected = (power_raw & 0x10) ? 1 : 0;
- }
-
- break;
- default:
- hid_err(hdev, "Unknown report: %d,%d size:%d\n",
- data[0], data[1], size);
- return 0;
- }
- }
- return 1;
-}
-
-static int wacom_input_mapped(struct hid_device *hdev, struct hid_input *hi,
- struct hid_field *field, struct hid_usage *usage, unsigned long **bit,
- int *max)
-{
- struct input_dev *input = hi->input;
-
- __set_bit(INPUT_PROP_POINTER, input->propbit);
-
- /* Basics */
- input->evbit[0] |= BIT(EV_KEY) | BIT(EV_ABS) | BIT(EV_REL);
-
- __set_bit(REL_WHEEL, input->relbit);
-
- __set_bit(BTN_TOOL_PEN, input->keybit);
- __set_bit(BTN_TOUCH, input->keybit);
- __set_bit(BTN_STYLUS, input->keybit);
- __set_bit(BTN_STYLUS2, input->keybit);
- __set_bit(BTN_LEFT, input->keybit);
- __set_bit(BTN_RIGHT, input->keybit);
- __set_bit(BTN_MIDDLE, input->keybit);
-
- /* Pad */
- input_set_capability(input, EV_MSC, MSC_SERIAL);
-
- __set_bit(BTN_0, input->keybit);
- __set_bit(BTN_1, input->keybit);
- __set_bit(BTN_TOOL_FINGER, input->keybit);
-
- /* Distance, rubber and mouse */
- __set_bit(BTN_TOOL_RUBBER, input->keybit);
- __set_bit(BTN_TOOL_MOUSE, input->keybit);
-
- switch (hdev->product) {
- case USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH:
- input_set_abs_params(input, ABS_X, 0, 16704, 4, 0);
- input_set_abs_params(input, ABS_Y, 0, 12064, 4, 0);
- input_set_abs_params(input, ABS_PRESSURE, 0, 511, 0, 0);
- input_set_abs_params(input, ABS_DISTANCE, 0, 32, 0, 0);
- break;
- case USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH:
- __set_bit(ABS_WHEEL, input->absbit);
- __set_bit(ABS_MISC, input->absbit);
- __set_bit(BTN_2, input->keybit);
- __set_bit(BTN_3, input->keybit);
- __set_bit(BTN_4, input->keybit);
- __set_bit(BTN_5, input->keybit);
- __set_bit(BTN_6, input->keybit);
- __set_bit(BTN_7, input->keybit);
- __set_bit(BTN_8, input->keybit);
- input_set_abs_params(input, ABS_WHEEL, 0, 71, 0, 0);
- input_set_abs_params(input, ABS_X, 0, 40640, 4, 0);
- input_set_abs_params(input, ABS_Y, 0, 25400, 4, 0);
- input_set_abs_params(input, ABS_PRESSURE, 0, 2047, 0, 0);
- input_set_abs_params(input, ABS_DISTANCE, 0, 63, 0, 0);
- input_set_abs_params(input, ABS_TILT_X, 0, 127, 0, 0);
- input_set_abs_params(input, ABS_TILT_Y, 0, 127, 0, 0);
- break;
- }
-
- return 0;
-}
-
-static int wacom_probe(struct hid_device *hdev,
- const struct hid_device_id *id)
-{
- struct wacom_data *wdata;
- int ret;
-
- wdata = kzalloc(sizeof(*wdata), GFP_KERNEL);
- if (wdata == NULL) {
- hid_err(hdev, "can't alloc wacom descriptor\n");
- return -ENOMEM;
- }
-
- hid_set_drvdata(hdev, wdata);
-
- /* Parse the HID report now */
- ret = hid_parse(hdev);
- if (ret) {
- hid_err(hdev, "parse failed\n");
- goto err_free;
- }
-
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret) {
- hid_err(hdev, "hw start failed\n");
- goto err_free;
- }
-
- ret = device_create_file(&hdev->dev, &dev_attr_speed);
- if (ret)
- hid_warn(hdev,
- "can't create sysfs speed attribute err: %d\n", ret);
-
-#define OLED_INIT(OLED_ID) \
- do { \
- ret = device_create_file(&hdev->dev, \
- &dev_attr_oled##OLED_ID##_img); \
- if (ret) \
- hid_warn(hdev, \
- "can't create sysfs oled attribute, err: %d\n", ret);\
- } while (0)
-
-OLED_INIT(0);
-OLED_INIT(1);
-OLED_INIT(2);
-OLED_INIT(3);
-OLED_INIT(4);
-OLED_INIT(5);
-OLED_INIT(6);
-OLED_INIT(7);
-
- wdata->features = 0;
- wacom_set_features(hdev, 1);
-
- if (hdev->product == USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) {
- sprintf(hdev->name, "%s", "Wacom Intuos4 WL");
- ret = wacom_initialize_leds(hdev);
- if (ret)
- hid_warn(hdev,
- "can't create led attribute, err: %d\n", ret);
- }
-
- wdata->battery.properties = wacom_battery_props;
- wdata->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
- wdata->battery.get_property = wacom_battery_get_property;
- wdata->battery.name = "wacom_battery";
- wdata->battery.type = POWER_SUPPLY_TYPE_BATTERY;
- wdata->battery.use_for_apm = 0;
-
-
- ret = power_supply_register(&hdev->dev, &wdata->battery);
- if (ret) {
- hid_err(hdev, "can't create sysfs battery attribute, err: %d\n",
- ret);
- goto err_battery;
- }
-
- power_supply_powers(&wdata->battery, &hdev->dev);
-
- wdata->ac.properties = wacom_ac_props;
- wdata->ac.num_properties = ARRAY_SIZE(wacom_ac_props);
- wdata->ac.get_property = wacom_ac_get_property;
- wdata->ac.name = "wacom_ac";
- wdata->ac.type = POWER_SUPPLY_TYPE_MAINS;
- wdata->ac.use_for_apm = 0;
-
- ret = power_supply_register(&hdev->dev, &wdata->ac);
- if (ret) {
- hid_err(hdev,
- "can't create ac battery attribute, err: %d\n", ret);
- goto err_ac;
- }
-
- power_supply_powers(&wdata->ac, &hdev->dev);
- return 0;
-
-err_ac:
- power_supply_unregister(&wdata->battery);
-err_battery:
- wacom_destroy_leds(hdev);
- device_remove_file(&hdev->dev, &dev_attr_oled0_img);
- device_remove_file(&hdev->dev, &dev_attr_oled1_img);
- device_remove_file(&hdev->dev, &dev_attr_oled2_img);
- device_remove_file(&hdev->dev, &dev_attr_oled3_img);
- device_remove_file(&hdev->dev, &dev_attr_oled4_img);
- device_remove_file(&hdev->dev, &dev_attr_oled5_img);
- device_remove_file(&hdev->dev, &dev_attr_oled6_img);
- device_remove_file(&hdev->dev, &dev_attr_oled7_img);
- device_remove_file(&hdev->dev, &dev_attr_speed);
- hid_hw_stop(hdev);
-err_free:
- kfree(wdata);
- return ret;
-}
-
-static void wacom_remove(struct hid_device *hdev)
-{
- struct wacom_data *wdata = hid_get_drvdata(hdev);
-
- wacom_destroy_leds(hdev);
- device_remove_file(&hdev->dev, &dev_attr_oled0_img);
- device_remove_file(&hdev->dev, &dev_attr_oled1_img);
- device_remove_file(&hdev->dev, &dev_attr_oled2_img);
- device_remove_file(&hdev->dev, &dev_attr_oled3_img);
- device_remove_file(&hdev->dev, &dev_attr_oled4_img);
- device_remove_file(&hdev->dev, &dev_attr_oled5_img);
- device_remove_file(&hdev->dev, &dev_attr_oled6_img);
- device_remove_file(&hdev->dev, &dev_attr_oled7_img);
- device_remove_file(&hdev->dev, &dev_attr_speed);
- hid_hw_stop(hdev);
-
- power_supply_unregister(&wdata->battery);
- power_supply_unregister(&wdata->ac);
- kfree(hid_get_drvdata(hdev));
-}
-
-static const struct hid_device_id wacom_devices[] = {
-
- { }
-};
-MODULE_DEVICE_TABLE(hid, wacom_devices);
-
-static struct hid_driver wacom_driver = {
- .name = "hid-wacom",
- .id_table = wacom_devices,
- .probe = wacom_probe,
- .remove = wacom_remove,
- .raw_event = wacom_raw_event,
- .input_mapped = wacom_input_mapped,
-};
-module_hid_driver(wacom_driver);
-
-MODULE_DESCRIPTION("Driver for Wacom Graphire Bluetooth and Wacom Intuos4 WL");
-MODULE_LICENSE("GPL");
--
2.0.1

2014-07-24 18:15:55

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 07/10] Input - wacom: handle Graphire BT tablets in wacom.ko

First, merge the Graphire BT tablet.

Signed-off-by: Benjamin Tissoires <[email protected]>
---

changes in v2:
- simplified because part of it has been splitted out in the previous commit.

drivers/hid/hid-core.c | 1 -
drivers/hid/hid-wacom.c | 1 -
drivers/hid/wacom_sys.c | 42 +++++++++++++++++++++
drivers/hid/wacom_wac.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-
drivers/hid/wacom_wac.h | 2 +
5 files changed, 140 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 7cff1c2..4db6804 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1920,7 +1920,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_3_PRO) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_DUAL_BOX_PRO) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_5_PRO) },
- { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_5_8_INCH) },
{ HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_12_1_INCH) },
diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
index 4874f4e..967c457 100644
--- a/drivers/hid/hid-wacom.c
+++ b/drivers/hid/hid-wacom.c
@@ -952,7 +952,6 @@ static void wacom_remove(struct hid_device *hdev)
}

static const struct hid_device_id wacom_devices[] = {
- { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH) },
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },

{ }
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index add76ec..8492177 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -265,6 +265,48 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
struct wacom_features *features)
{
+ struct wacom *wacom = hid_get_drvdata(hdev);
+ int limit, ret;
+ __u8 rep_data[2];
+
+ switch (features->type) {
+ case GRAPHIRE_BT:
+ rep_data[0] = 0x03 ; rep_data[1] = 0x00;
+ limit = 3;
+ do {
+ ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ } while (ret < 0 && limit-- > 0);
+
+ if (ret >= 0) {
+ if (speed == 0)
+ rep_data[0] = 0x05;
+ else
+ rep_data[0] = 0x06;
+
+ rep_data[1] = 0x00;
+ limit = 3;
+ do {
+ ret = hid_hw_raw_request(hdev, rep_data[0],
+ rep_data, 2, HID_FEATURE_REPORT,
+ HID_REQ_SET_REPORT);
+ } while (ret < 0 && limit-- > 0);
+
+ if (ret >= 0) {
+ wacom->wacom_wac.bt_high_speed = speed;
+ return 0;
+ }
+ }
+
+ /*
+ * 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;
+ }
+
return 0;
}

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2c69326..06bc600 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -30,6 +30,10 @@
*/
#define WACOM_CONTACT_AREA_SCALE 2607

+/*percent of battery capacity for Graphire
+ 8th value means AC online and show 100% capacity */
+static unsigned short batcap_gr[8] = { 1, 15, 25, 35, 50, 70, 100, 100 };
+
static int wacom_penpartner_irq(struct wacom_wac *wacom)
{
unsigned char *data = wacom->data;
@@ -263,11 +267,19 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
unsigned char *data = wacom->data;
struct input_dev *input = wacom->input;
struct input_dev *pad_input = wacom->pad_input;
+ int battery_capacity, ps_connected;
int prox;
int rw = 0;
int retval = 0;

- if (data[0] != WACOM_REPORT_PENABLED) {
+ if (features->type == GRAPHIRE_BT) {
+ if (data[0] != WACOM_REPORT_PENABLED_BT) {
+ dev_dbg(input->dev.parent,
+ "%s: received unknown report #%d\n", __func__,
+ data[0]);
+ goto exit;
+ }
+ } else if (data[0] != WACOM_REPORT_PENABLED) {
dev_dbg(input->dev.parent,
"%s: received unknown report #%d\n", __func__, data[0]);
goto exit;
@@ -301,7 +313,12 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
if (wacom->tool[0] != BTN_TOOL_MOUSE) {
- input_report_abs(input, ABS_PRESSURE, data[6] | ((data[7] & 0x03) << 8));
+ if (features->type == GRAPHIRE_BT)
+ input_report_abs(input, ABS_PRESSURE, data[6] |
+ (((__u16) (data[1] & 0x08)) << 5));
+ else
+ input_report_abs(input, ABS_PRESSURE, data[6] |
+ ((data[7] & 0x03) << 8));
input_report_key(input, BTN_TOUCH, data[1] & 0x01);
input_report_key(input, BTN_STYLUS, data[1] & 0x02);
input_report_key(input, BTN_STYLUS2, data[1] & 0x04);
@@ -312,6 +329,23 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
features->type == WACOM_MO) {
input_report_abs(input, ABS_DISTANCE, data[6] & 0x3f);
rw = (data[7] & 0x04) - (data[7] & 0x03);
+ } else if (features->type == GRAPHIRE_BT) {
+ /* Compute distance between mouse and tablet */
+ rw = 44 - (data[6] >> 2);
+ if (rw < 0)
+ rw = 0;
+ else if (rw > 31)
+ rw = 31;
+ input_report_abs(input, ABS_DISTANCE, rw);
+ if (((data[1] >> 5) & 3) == 2) {
+ /* Mouse with wheel */
+ input_report_key(input, BTN_MIDDLE,
+ data[1] & 0x04);
+ rw = (data[6] & 0x01) ? -1 :
+ (data[6] & 0x02) ? 1 : 0;
+ } else {
+ rw = 0;
+ }
} else {
input_report_abs(input, ABS_DISTANCE, data[7] & 0x3f);
rw = -(signed char)data[6];
@@ -358,6 +392,31 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
retval = 1;
}
break;
+ case GRAPHIRE_BT:
+ prox = data[7] & 0x03;
+ if (prox || wacom->id[1]) {
+ wacom->id[1] = PAD_DEVICE_ID;
+ input_report_key(pad_input, BTN_0, (data[7] & 0x02));
+ input_report_key(pad_input, BTN_1, (data[7] & 0x01));
+ if (!prox)
+ wacom->id[1] = 0;
+ input_report_abs(pad_input, ABS_MISC, wacom->id[1]);
+ retval = 1;
+ }
+ break;
+ }
+
+ /* Store current battery capacity and power supply state */
+ if (features->type == GRAPHIRE_BT) {
+ rw = (data[7] >> 2 & 0x07);
+ battery_capacity = batcap_gr[rw];
+ ps_connected = rw == 7;
+ if ((wacom->battery_capacity != battery_capacity) ||
+ (wacom->ps_connected != ps_connected)) {
+ wacom->battery_capacity = battery_capacity;
+ wacom->ps_connected = ps_connected;
+ wacom_notify_battery(wacom);
+ }
}
exit:
return retval;
@@ -1418,6 +1477,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)

case WACOM_G4:
case GRAPHIRE:
+ case GRAPHIRE_BT:
case WACOM_MO:
sync = wacom_graphire_irq(wacom_wac);
break;
@@ -1654,6 +1714,27 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
__set_bit(INPUT_PROP_POINTER, input_dev->propbit);
break;

+ case GRAPHIRE_BT:
+ __clear_bit(ABS_MISC, input_dev->absbit);
+ input_set_abs_params(input_dev, ABS_DISTANCE, 0,
+ features->distance_max,
+ 0, 0);
+
+ input_set_capability(input_dev, EV_REL, REL_WHEEL);
+
+ __set_bit(BTN_LEFT, input_dev->keybit);
+ __set_bit(BTN_RIGHT, input_dev->keybit);
+ __set_bit(BTN_MIDDLE, input_dev->keybit);
+
+ __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
+ __set_bit(BTN_TOOL_PEN, input_dev->keybit);
+ __set_bit(BTN_TOOL_MOUSE, input_dev->keybit);
+ __set_bit(BTN_STYLUS, input_dev->keybit);
+ __set_bit(BTN_STYLUS2, input_dev->keybit);
+
+ __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
+ break;
+
case WACOM_24HD:
input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
input_set_abs_params(input_dev, ABS_THROTTLE, 0, 71, 0, 0);
@@ -1848,6 +1929,11 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);

switch (features->type) {
+ case GRAPHIRE_BT:
+ __set_bit(BTN_0, input_dev->keybit);
+ __set_bit(BTN_1, input_dev->keybit);
+ break;
+
case WACOM_MO:
__set_bit(BTN_BACK, input_dev->keybit);
__set_bit(BTN_LEFT, input_dev->keybit);
@@ -2017,6 +2103,9 @@ static const struct wacom_features wacom_features_0x00 =
static const struct wacom_features wacom_features_0x10 =
{ "Wacom Graphire", 10206, 7422, 511, 63,
GRAPHIRE, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
+static const struct wacom_features wacom_features_0x81 =
+ { "Wacom Graphire BT", 16704, 12064, 511, 32,
+ GRAPHIRE_BT, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
static const struct wacom_features wacom_features_0x11 =
{ "Wacom Graphire2 4x5", 10206, 7422, 511, 63,
GRAPHIRE, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
@@ -2412,6 +2501,10 @@ static const struct wacom_features wacom_features_0x309 =
HID_DEVICE(BUS_USB, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
.driver_data = (kernel_ulong_t)&wacom_features_##prod

+#define BT_DEVICE_WACOM(prod) \
+ HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
+ .driver_data = (kernel_ulong_t)&wacom_features_##prod
+
#define USB_DEVICE_LENOVO(prod) \
HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, prod), \
.driver_data = (kernel_ulong_t)&wacom_features_##prod
@@ -2469,6 +2562,7 @@ const struct hid_device_id wacom_ids[] = {
{ USB_DEVICE_WACOM(0x69) },
{ USB_DEVICE_WACOM(0x6A) },
{ USB_DEVICE_WACOM(0x6B) },
+ { BT_DEVICE_WACOM(0x81) },
{ USB_DEVICE_WACOM(0x84) },
{ USB_DEVICE_WACOM(0x90) },
{ USB_DEVICE_WACOM(0x93) },
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 3aa55d9..0f3df34 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -46,6 +46,7 @@

/* wacom data packet report IDs */
#define WACOM_REPORT_PENABLED 2
+#define WACOM_REPORT_PENABLED_BT 3
#define WACOM_REPORT_INTUOSREAD 5
#define WACOM_REPORT_INTUOSWRITE 6
#define WACOM_REPORT_INTUOSPAD 12
@@ -73,6 +74,7 @@
enum {
PENPARTNER = 0,
GRAPHIRE,
+ GRAPHIRE_BT,
WACOM_G4,
PTU,
PL,
--
2.0.1

2014-07-24 18:15:54

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 09/10] Input - wacom: add copyright note and bump version to 2.0

Signed-off-by: Benjamin Tissoires <[email protected]>
---

new in v2:
- I thought it was interesting to bump the number of the release

drivers/hid/wacom.h | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 2b5562b..e354972 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -12,6 +12,7 @@
* Copyright (c) 2001 Frederic Lepied <[email protected]>
* Copyright (c) 2004 Panagiotis Issaris <[email protected]>
* Copyright (c) 2002-2011 Ping Cheng <[email protected]>
+ * Copyright (c) 2014 Benjamin Tissoires <[email protected]>
*
* ChangeLog:
* v0.1 (vp) - Initial release
@@ -72,6 +73,8 @@
* v1.52 (pc) - Query Wacom data upon system resume
* - add defines for features->type
* - add new devices (0x9F, 0xE2, and 0XE3)
+ * v2.00 (bt) - conversion to a HID driver
+ * - integration of the Bluetooth devices
*/

/*
--
2.0.1

2014-07-24 18:14:42

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 04/10] Input - wacom: use a uniq name for the battery device

The current implementation uses "wacom_battery" as a generic name for
batteries. This prevents us to have two Wacom devices with a battery
attached as the power system will complain about the name which is already
registered.

Use an incremental name for each battery attached.

Related bug:
https://sourceforge.net/p/linuxwacom/bugs/248/

Signed-off-by: Benjamin Tissoires <[email protected]>
---

changes in v2:
- do not use the name of the tablet (which removes the need of the check for
spaces), but use instead a simple counter to get a uniq name.

drivers/hid/wacom_sys.c | 6 +++++-
drivers/hid/wacom_wac.h | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index f0ee7ff..eb8705d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -809,12 +809,16 @@ static int wacom_battery_get_property(struct power_supply *psy,
static int wacom_initialize_battery(struct wacom *wacom)
{
int error = 0;
+ static atomic_t battery_no = ATOMIC_INIT(0);
+ unsigned long n;

if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
+ n = atomic_inc_return(&battery_no) - 1;
wacom->battery.properties = wacom_battery_props;
wacom->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
wacom->battery.get_property = wacom_battery_get_property;
- wacom->battery.name = "wacom_battery";
+ sprintf(wacom->wacom_wac.bat_name, "wacom_battery_%ld", n);
+ wacom->battery.name = wacom->wacom_wac.bat_name;
wacom->battery.type = POWER_SUPPLY_TYPE_BATTERY;
wacom->battery.use_for_apm = 0;

diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 8a042ac..6cdf707 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -154,6 +154,7 @@ struct wacom_shared {
struct wacom_wac {
char name[WACOM_NAME_MAX];
char pad_name[WACOM_NAME_MAX];
+ char bat_name[WACOM_NAME_MAX];
unsigned char data[WACOM_PKGLEN_MAX];
int tool[2];
int id[2];
--
2.0.1

2014-07-24 18:16:49

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 03/10] Input - wacom: enhance Wireless Receiver battery reporting

- Reports the current status of the battery (discharging, charging, full).
- Also notify the upower daemon when there is a change in the battery
value.
- keep the battery value as a percentage, not the raw value
- add WACOM_QUIRK_BATTERY to easily add a battery to a device (required
for Bluetooth devices)

Signed-off-by: Benjamin Tissoires <[email protected]>
---

changes in v2:
- add WACOM_QUIRK_BATTERY, which simplifies a bit the other patches

drivers/hid/wacom.h | 6 ++++++
drivers/hid/wacom_sys.c | 19 +++++++++++++++----
drivers/hid/wacom_wac.c | 22 ++++++++++++++++++----
drivers/hid/wacom_wac.h | 3 +++
4 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index a678f82..99516ba 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -128,6 +128,12 @@ static inline void wacom_schedule_work(struct wacom_wac *wacom_wac)
schedule_work(&wacom->work);
}

+static inline void wacom_notify_battery(struct wacom_wac *wacom_wac)
+{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+ power_supply_changed(&wacom->battery);
+}
+
extern const struct hid_device_id wacom_ids[];

void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len);
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index c857b3d..f0ee7ff 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -769,6 +769,7 @@ static void wacom_destroy_leds(struct wacom *wacom)
}

static enum power_supply_property wacom_battery_props[] = {
+ POWER_SUPPLY_PROP_STATUS,
POWER_SUPPLY_PROP_SCOPE,
POWER_SUPPLY_PROP_CAPACITY
};
@@ -786,7 +787,16 @@ static int wacom_battery_get_property(struct power_supply *psy,
break;
case POWER_SUPPLY_PROP_CAPACITY:
val->intval =
- wacom->wacom_wac.battery_capacity * 100 / 31;
+ wacom->wacom_wac.battery_capacity;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ if (wacom->wacom_wac.bat_charging)
+ val->intval = POWER_SUPPLY_STATUS_CHARGING;
+ else if (wacom->wacom_wac.battery_capacity == 100 &&
+ wacom->wacom_wac.ps_connected)
+ val->intval = POWER_SUPPLY_STATUS_FULL;
+ else
+ val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
break;
default:
ret = -EINVAL;
@@ -800,7 +810,7 @@ static int wacom_initialize_battery(struct wacom *wacom)
{
int error = 0;

- if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_MONITOR) {
+ if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
wacom->battery.properties = wacom_battery_props;
wacom->battery.num_properties = ARRAY_SIZE(wacom_battery_props);
wacom->battery.get_property = wacom_battery_get_property;
@@ -821,8 +831,8 @@ static int wacom_initialize_battery(struct wacom *wacom)

static void wacom_destroy_battery(struct wacom *wacom)
{
- if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_MONITOR &&
- wacom->battery.dev) {
+ if ((wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) &&
+ wacom->battery.dev) {
power_supply_unregister(&wacom->battery);
wacom->battery.dev = NULL;
}
@@ -947,6 +957,7 @@ static void wacom_wireless_work(struct work_struct *work)

if (wacom_wac->pid == 0) {
hid_info(wacom->hdev, "wireless tablet disconnected\n");
+ wacom_wac1->shared->type = 0;
} else {
const struct hid_device_id *id = wacom_ids;

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index f30083f..2c69326 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1365,7 +1365,7 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)

connected = data[1] & 0x01;
if (connected) {
- int pid, battery;
+ int pid, battery, ps_connected;

if ((wacom->shared->type == INTUOSHT) &&
wacom->shared->touch_max) {
@@ -1375,17 +1375,29 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
}

pid = get_unaligned_be16(&data[6]);
- battery = data[5] & 0x3f;
+ battery = (data[5] & 0x3f) * 100 / 31;
+ ps_connected = !!(data[5] & 0x80);
if (wacom->pid != pid) {
wacom->pid = pid;
wacom_schedule_work(wacom);
}
- wacom->battery_capacity = battery;
+
+ if (wacom->shared->type &&
+ ((battery != wacom->battery_capacity) ||
+ (ps_connected != wacom->ps_connected))) {
+ wacom->battery_capacity = battery;
+ wacom->ps_connected = ps_connected;
+ wacom->bat_charging = ps_connected &&
+ wacom->battery_capacity < 100;
+ wacom_notify_battery(wacom);
+ }
} else if (wacom->pid != 0) {
/* disconnected while previously connected */
wacom->pid = 0;
wacom_schedule_work(wacom);
wacom->battery_capacity = 0;
+ wacom->bat_charging = 0;
+ wacom->ps_connected = 0;
}

return 0;
@@ -1558,8 +1570,10 @@ void wacom_setup_device_quirks(struct wacom_features *features)
features->quirks |= WACOM_QUIRK_NO_INPUT;

/* must be monitor interface if no device_type set */
- if (!features->device_type)
+ if (!features->device_type) {
features->quirks |= WACOM_QUIRK_MONITOR;
+ features->quirks |= WACOM_QUIRK_BATTERY;
+ }
}
}

diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 4c59247..8a042ac 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -68,6 +68,7 @@
#define WACOM_QUIRK_BBTOUCH_LOWRES 0x0002
#define WACOM_QUIRK_NO_INPUT 0x0004
#define WACOM_QUIRK_MONITOR 0x0008
+#define WACOM_QUIRK_BATTERY 0x0010

enum {
PENPARTNER = 0,
@@ -164,6 +165,8 @@ struct wacom_wac {
int pid;
int battery_capacity;
int num_contacts_left;
+ int bat_charging;
+ int ps_connected;
};

#endif
--
2.0.1

2014-07-24 18:14:39

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH v2 01/10] Input - wacom: Support up to 2048 pressure levels with ISDv4

From: Jason Gerecke <[email protected]>

Signed-off-by: Jason Gerecke <[email protected]>
Rebased in drivers/hid/
Signed-off-by: Benjamin Tissoires <[email protected]>
---

changes in v2:
- rebased in drivers/hid/

drivers/hid/wacom_wac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index d62e320..f30083f 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1086,7 +1086,7 @@ static int wacom_tpc_pen(struct wacom_wac *wacom)
input_report_key(input, BTN_STYLUS2, data[1] & 0x10);
input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
- input_report_abs(input, ABS_PRESSURE, ((data[7] & 0x03) << 8) | data[6]);
+ input_report_abs(input, ABS_PRESSURE, ((data[7] & 0x07) << 8) | data[6]);
input_report_key(input, BTN_TOUCH, data[1] & 0x05);
input_report_key(input, wacom->tool[0], prox);
return 1;
--
2.0.1

2014-07-24 19:35:16

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] Input - wacom: handle Graphire BT tablets in wacom.ko

Hi Benjamin,

On Thu, Jul 24, 2014 at 02:14:02PM -0400, Benjamin Tissoires wrote:
> First, merge the Graphire BT tablet.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
>
> changes in v2:
> - simplified because part of it has been splitted out in the previous commit.
>
> drivers/hid/hid-core.c | 1 -
> drivers/hid/hid-wacom.c | 1 -
> drivers/hid/wacom_sys.c | 42 +++++++++++++++++++++
> drivers/hid/wacom_wac.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-
> drivers/hid/wacom_wac.h | 2 +
> 5 files changed, 140 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 7cff1c2..4db6804 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1920,7 +1920,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
> { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_3_PRO) },
> { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_DUAL_BOX_PRO) },
> { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_5_PRO) },
> - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },
> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_5_8_INCH) },
> { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_12_1_INCH) },
> diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
> index 4874f4e..967c457 100644
> --- a/drivers/hid/hid-wacom.c
> +++ b/drivers/hid/hid-wacom.c
> @@ -952,7 +952,6 @@ static void wacom_remove(struct hid_device *hdev)
> }
>
> static const struct hid_device_id wacom_devices[] = {
> - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },
>
> { }
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index add76ec..8492177 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -265,6 +265,48 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
> struct wacom_features *features)
> {
> + struct wacom *wacom = hid_get_drvdata(hdev);
> + int limit, ret;
> + __u8 rep_data[2];

I think just u8 in kerneli proper.

> +
> + switch (features->type) {
> + case GRAPHIRE_BT:
> + rep_data[0] = 0x03 ; rep_data[1] = 0x00;
> + limit = 3;
> + do {
> + ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + } while (ret < 0 && limit-- > 0);
> +
> + if (ret >= 0) {
> + if (speed == 0)
> + rep_data[0] = 0x05;
> + else
> + rep_data[0] = 0x06;

I am a fan of

rep_data[0] = speed == 0 ? 0x05 : 0x06;

as it takes 1 line vs 3.

> +
> + rep_data[1] = 0x00;
> + limit = 3;
> + do {
> + ret = hid_hw_raw_request(hdev, rep_data[0],
> + rep_data, 2, HID_FEATURE_REPORT,
> + HID_REQ_SET_REPORT);
> + } while (ret < 0 && limit-- > 0);
> +
> + if (ret >= 0) {
> + wacom->wacom_wac.bt_high_speed = speed;
> + return 0;
> + }
> + }
> +
> + /*
> + * 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;
> + }
> +
> return 0;
> }
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 2c69326..06bc600 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -30,6 +30,10 @@
> */
> #define WACOM_CONTACT_AREA_SCALE 2607
>
> +/*percent of battery capacity for Graphire
> + 8th value means AC online and show 100% capacity */
> +static unsigned short batcap_gr[8] = { 1, 15, 25, 35, 50, 70, 100, 100 };
> +
> static int wacom_penpartner_irq(struct wacom_wac *wacom)
> {
> unsigned char *data = wacom->data;
> @@ -263,11 +267,19 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> unsigned char *data = wacom->data;
> struct input_dev *input = wacom->input;
> struct input_dev *pad_input = wacom->pad_input;
> + int battery_capacity, ps_connected;
> int prox;
> int rw = 0;
> int retval = 0;
>
> - if (data[0] != WACOM_REPORT_PENABLED) {
> + if (features->type == GRAPHIRE_BT) {
> + if (data[0] != WACOM_REPORT_PENABLED_BT) {
> + dev_dbg(input->dev.parent,
> + "%s: received unknown report #%d\n", __func__,
> + data[0]);
> + goto exit;
> + }
> + } else if (data[0] != WACOM_REPORT_PENABLED) {
> dev_dbg(input->dev.parent,
> "%s: received unknown report #%d\n", __func__, data[0]);
> goto exit;
> @@ -301,7 +313,12 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> if (wacom->tool[0] != BTN_TOOL_MOUSE) {
> - input_report_abs(input, ABS_PRESSURE, data[6] | ((data[7] & 0x03) << 8));
> + if (features->type == GRAPHIRE_BT)
> + input_report_abs(input, ABS_PRESSURE, data[6] |
> + (((__u16) (data[1] & 0x08)) << 5));
> + else
> + input_report_abs(input, ABS_PRESSURE, data[6] |
> + ((data[7] & 0x03) << 8));
> input_report_key(input, BTN_TOUCH, data[1] & 0x01);
> input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> input_report_key(input, BTN_STYLUS2, data[1] & 0x04);
> @@ -312,6 +329,23 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> features->type == WACOM_MO) {
> input_report_abs(input, ABS_DISTANCE, data[6] & 0x3f);
> rw = (data[7] & 0x04) - (data[7] & 0x03);
> + } else if (features->type == GRAPHIRE_BT) {
> + /* Compute distance between mouse and tablet */
> + rw = 44 - (data[6] >> 2);
> + if (rw < 0)
> + rw = 0;
> + else if (rw > 31)
> + rw = 31;

rw = clamp_val(rw, 0, 31);

?

> + input_report_abs(input, ABS_DISTANCE, rw);
> + if (((data[1] >> 5) & 3) == 2) {
> + /* Mouse with wheel */
> + input_report_key(input, BTN_MIDDLE,
> + data[1] & 0x04);
> + rw = (data[6] & 0x01) ? -1 :
> + (data[6] & 0x02) ? 1 : 0;
> + } else {
> + rw = 0;
> + }
> } else {
> input_report_abs(input, ABS_DISTANCE, data[7] & 0x3f);
> rw = -(signed char)data[6];
> @@ -358,6 +392,31 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> retval = 1;
> }
> break;
> + case GRAPHIRE_BT:
> + prox = data[7] & 0x03;
> + if (prox || wacom->id[1]) {
> + wacom->id[1] = PAD_DEVICE_ID;
> + input_report_key(pad_input, BTN_0, (data[7] & 0x02));
> + input_report_key(pad_input, BTN_1, (data[7] & 0x01));
> + if (!prox)
> + wacom->id[1] = 0;
> + input_report_abs(pad_input, ABS_MISC, wacom->id[1]);
> + retval = 1;
> + }
> + break;
> + }
> +
> + /* Store current battery capacity and power supply state */
> + if (features->type == GRAPHIRE_BT) {
> + rw = (data[7] >> 2 & 0x07);
> + battery_capacity = batcap_gr[rw];
> + ps_connected = rw == 7;
> + if ((wacom->battery_capacity != battery_capacity) ||
> + (wacom->ps_connected != ps_connected)) {
> + wacom->battery_capacity = battery_capacity;
> + wacom->ps_connected = ps_connected;
> + wacom_notify_battery(wacom);
> + }
> }
> exit:
> return retval;
> @@ -1418,6 +1477,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
>
> case WACOM_G4:
> case GRAPHIRE:
> + case GRAPHIRE_BT:
> case WACOM_MO:
> sync = wacom_graphire_irq(wacom_wac);
> break;
> @@ -1654,6 +1714,27 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
> __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> break;
>
> + case GRAPHIRE_BT:
> + __clear_bit(ABS_MISC, input_dev->absbit);
> + input_set_abs_params(input_dev, ABS_DISTANCE, 0,
> + features->distance_max,
> + 0, 0);
> +
> + input_set_capability(input_dev, EV_REL, REL_WHEEL);
> +
> + __set_bit(BTN_LEFT, input_dev->keybit);
> + __set_bit(BTN_RIGHT, input_dev->keybit);
> + __set_bit(BTN_MIDDLE, input_dev->keybit);
> +
> + __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
> + __set_bit(BTN_TOOL_PEN, input_dev->keybit);
> + __set_bit(BTN_TOOL_MOUSE, input_dev->keybit);
> + __set_bit(BTN_STYLUS, input_dev->keybit);
> + __set_bit(BTN_STYLUS2, input_dev->keybit);
> +
> + __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> + break;
> +
> case WACOM_24HD:
> input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
> input_set_abs_params(input_dev, ABS_THROTTLE, 0, 71, 0, 0);
> @@ -1848,6 +1929,11 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
> input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);
>
> switch (features->type) {
> + case GRAPHIRE_BT:
> + __set_bit(BTN_0, input_dev->keybit);
> + __set_bit(BTN_1, input_dev->keybit);
> + break;
> +
> case WACOM_MO:
> __set_bit(BTN_BACK, input_dev->keybit);
> __set_bit(BTN_LEFT, input_dev->keybit);
> @@ -2017,6 +2103,9 @@ static const struct wacom_features wacom_features_0x00 =
> static const struct wacom_features wacom_features_0x10 =
> { "Wacom Graphire", 10206, 7422, 511, 63,
> GRAPHIRE, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
> +static const struct wacom_features wacom_features_0x81 =
> + { "Wacom Graphire BT", 16704, 12064, 511, 32,
> + GRAPHIRE_BT, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
> static const struct wacom_features wacom_features_0x11 =
> { "Wacom Graphire2 4x5", 10206, 7422, 511, 63,
> GRAPHIRE, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
> @@ -2412,6 +2501,10 @@ static const struct wacom_features wacom_features_0x309 =
> HID_DEVICE(BUS_USB, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
> .driver_data = (kernel_ulong_t)&wacom_features_##prod
>
> +#define BT_DEVICE_WACOM(prod) \
> + HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
> + .driver_data = (kernel_ulong_t)&wacom_features_##prod
> +
> #define USB_DEVICE_LENOVO(prod) \
> HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, prod), \
> .driver_data = (kernel_ulong_t)&wacom_features_##prod
> @@ -2469,6 +2562,7 @@ const struct hid_device_id wacom_ids[] = {
> { USB_DEVICE_WACOM(0x69) },
> { USB_DEVICE_WACOM(0x6A) },
> { USB_DEVICE_WACOM(0x6B) },
> + { BT_DEVICE_WACOM(0x81) },
> { USB_DEVICE_WACOM(0x84) },
> { USB_DEVICE_WACOM(0x90) },
> { USB_DEVICE_WACOM(0x93) },
> diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> index 3aa55d9..0f3df34 100644
> --- a/drivers/hid/wacom_wac.h
> +++ b/drivers/hid/wacom_wac.h
> @@ -46,6 +46,7 @@
>
> /* wacom data packet report IDs */
> #define WACOM_REPORT_PENABLED 2
> +#define WACOM_REPORT_PENABLED_BT 3
> #define WACOM_REPORT_INTUOSREAD 5
> #define WACOM_REPORT_INTUOSWRITE 6
> #define WACOM_REPORT_INTUOSPAD 12
> @@ -73,6 +74,7 @@
> enum {
> PENPARTNER = 0,
> GRAPHIRE,
> + GRAPHIRE_BT,
> WACOM_G4,
> PTU,
> PL,
> --
> 2.0.1
>

--
Dmitry

2014-07-24 19:44:20

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] Input - wacom: handle Graphire BT tablets in wacom.ko

On Jul 24 2014 or thereabouts, Dmitry Torokhov wrote:
> Hi Benjamin,
>
> On Thu, Jul 24, 2014 at 02:14:02PM -0400, Benjamin Tissoires wrote:
> > First, merge the Graphire BT tablet.
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
> > ---
> >
> > changes in v2:
> > - simplified because part of it has been splitted out in the previous commit.
> >
> > drivers/hid/hid-core.c | 1 -
> > drivers/hid/hid-wacom.c | 1 -
> > drivers/hid/wacom_sys.c | 42 +++++++++++++++++++++
> > drivers/hid/wacom_wac.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++-
> > drivers/hid/wacom_wac.h | 2 +
> > 5 files changed, 140 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> > index 7cff1c2..4db6804 100644
> > --- a/drivers/hid/hid-core.c
> > +++ b/drivers/hid/hid-core.c
> > @@ -1920,7 +1920,6 @@ static const struct hid_device_id hid_have_special_driver[] = {
> > { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_3_PRO) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_DUAL_BOX_PRO) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_WISEGROUP_LTD, USB_DEVICE_ID_SUPER_JOY_BOX_5_PRO) },
> > - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH) },
> > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_5_8_INCH) },
> > { HID_USB_DEVICE(USB_VENDOR_ID_WALTOP, USB_DEVICE_ID_WALTOP_SLIM_TABLET_12_1_INCH) },
> > diff --git a/drivers/hid/hid-wacom.c b/drivers/hid/hid-wacom.c
> > index 4874f4e..967c457 100644
> > --- a/drivers/hid/hid-wacom.c
> > +++ b/drivers/hid/hid-wacom.c
> > @@ -952,7 +952,6 @@ static void wacom_remove(struct hid_device *hdev)
> > }
> >
> > static const struct hid_device_id wacom_devices[] = {
> > - { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_GRAPHIRE_BLUETOOTH) },
> > { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_WACOM, USB_DEVICE_ID_WACOM_INTUOS4_BLUETOOTH) },
> >
> > { }
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index add76ec..8492177 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -265,6 +265,48 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> > static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
> > struct wacom_features *features)
> > {
> > + struct wacom *wacom = hid_get_drvdata(hdev);
> > + int limit, ret;
> > + __u8 rep_data[2];
>
> I think just u8 in kerneli proper.

OK, I will adjust.

>
> > +
> > + switch (features->type) {
> > + case GRAPHIRE_BT:
> > + rep_data[0] = 0x03 ; rep_data[1] = 0x00;
> > + limit = 3;
> > + do {
> > + ret = hid_hw_raw_request(hdev, rep_data[0], rep_data, 2,
> > + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> > + } while (ret < 0 && limit-- > 0);
> > +
> > + if (ret >= 0) {
> > + if (speed == 0)
> > + rep_data[0] = 0x05;
> > + else
> > + rep_data[0] = 0x06;
>
> I am a fan of
>
> rep_data[0] = speed == 0 ? 0x05 : 0x06;
>
> as it takes 1 line vs 3.
>

hehe

BTW, actually, I just copied/pasted the chunks from hid-wacom. But you
are right, a little cleanup is definitively better.

> > +
> > + rep_data[1] = 0x00;
> > + limit = 3;
> > + do {
> > + ret = hid_hw_raw_request(hdev, rep_data[0],
> > + rep_data, 2, HID_FEATURE_REPORT,
> > + HID_REQ_SET_REPORT);
> > + } while (ret < 0 && limit-- > 0);
> > +
> > + if (ret >= 0) {
> > + wacom->wacom_wac.bt_high_speed = speed;
> > + return 0;
> > + }
> > + }
> > +
> > + /*
> > + * 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;
> > + }
> > +
> > return 0;
> > }
> >
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index 2c69326..06bc600 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -30,6 +30,10 @@
> > */
> > #define WACOM_CONTACT_AREA_SCALE 2607
> >
> > +/*percent of battery capacity for Graphire
> > + 8th value means AC online and show 100% capacity */
> > +static unsigned short batcap_gr[8] = { 1, 15, 25, 35, 50, 70, 100, 100 };
> > +
> > static int wacom_penpartner_irq(struct wacom_wac *wacom)
> > {
> > unsigned char *data = wacom->data;
> > @@ -263,11 +267,19 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> > unsigned char *data = wacom->data;
> > struct input_dev *input = wacom->input;
> > struct input_dev *pad_input = wacom->pad_input;
> > + int battery_capacity, ps_connected;
> > int prox;
> > int rw = 0;
> > int retval = 0;
> >
> > - if (data[0] != WACOM_REPORT_PENABLED) {
> > + if (features->type == GRAPHIRE_BT) {
> > + if (data[0] != WACOM_REPORT_PENABLED_BT) {
> > + dev_dbg(input->dev.parent,
> > + "%s: received unknown report #%d\n", __func__,
> > + data[0]);
> > + goto exit;
> > + }
> > + } else if (data[0] != WACOM_REPORT_PENABLED) {
> > dev_dbg(input->dev.parent,
> > "%s: received unknown report #%d\n", __func__, data[0]);
> > goto exit;
> > @@ -301,7 +313,12 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> > input_report_abs(input, ABS_X, le16_to_cpup((__le16 *)&data[2]));
> > input_report_abs(input, ABS_Y, le16_to_cpup((__le16 *)&data[4]));
> > if (wacom->tool[0] != BTN_TOOL_MOUSE) {
> > - input_report_abs(input, ABS_PRESSURE, data[6] | ((data[7] & 0x03) << 8));
> > + if (features->type == GRAPHIRE_BT)
> > + input_report_abs(input, ABS_PRESSURE, data[6] |
> > + (((__u16) (data[1] & 0x08)) << 5));
> > + else
> > + input_report_abs(input, ABS_PRESSURE, data[6] |
> > + ((data[7] & 0x03) << 8));
> > input_report_key(input, BTN_TOUCH, data[1] & 0x01);
> > input_report_key(input, BTN_STYLUS, data[1] & 0x02);
> > input_report_key(input, BTN_STYLUS2, data[1] & 0x04);
> > @@ -312,6 +329,23 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> > features->type == WACOM_MO) {
> > input_report_abs(input, ABS_DISTANCE, data[6] & 0x3f);
> > rw = (data[7] & 0x04) - (data[7] & 0x03);
> > + } else if (features->type == GRAPHIRE_BT) {
> > + /* Compute distance between mouse and tablet */
> > + rw = 44 - (data[6] >> 2);
> > + if (rw < 0)
> > + rw = 0;
> > + else if (rw > 31)
> > + rw = 31;
>
> rw = clamp_val(rw, 0, 31);
>
> ?

could be...

I'll do the tests and resubmit later the v3. I'll wait a little in case
you have other comments on the other patches :)

Cheers,
Benjamin

>
> > + input_report_abs(input, ABS_DISTANCE, rw);
> > + if (((data[1] >> 5) & 3) == 2) {
> > + /* Mouse with wheel */
> > + input_report_key(input, BTN_MIDDLE,
> > + data[1] & 0x04);
> > + rw = (data[6] & 0x01) ? -1 :
> > + (data[6] & 0x02) ? 1 : 0;
> > + } else {
> > + rw = 0;
> > + }
> > } else {
> > input_report_abs(input, ABS_DISTANCE, data[7] & 0x3f);
> > rw = -(signed char)data[6];
> > @@ -358,6 +392,31 @@ static int wacom_graphire_irq(struct wacom_wac *wacom)
> > retval = 1;
> > }
> > break;
> > + case GRAPHIRE_BT:
> > + prox = data[7] & 0x03;
> > + if (prox || wacom->id[1]) {
> > + wacom->id[1] = PAD_DEVICE_ID;
> > + input_report_key(pad_input, BTN_0, (data[7] & 0x02));
> > + input_report_key(pad_input, BTN_1, (data[7] & 0x01));
> > + if (!prox)
> > + wacom->id[1] = 0;
> > + input_report_abs(pad_input, ABS_MISC, wacom->id[1]);
> > + retval = 1;
> > + }
> > + break;
> > + }
> > +
> > + /* Store current battery capacity and power supply state */
> > + if (features->type == GRAPHIRE_BT) {
> > + rw = (data[7] >> 2 & 0x07);
> > + battery_capacity = batcap_gr[rw];
> > + ps_connected = rw == 7;
> > + if ((wacom->battery_capacity != battery_capacity) ||
> > + (wacom->ps_connected != ps_connected)) {
> > + wacom->battery_capacity = battery_capacity;
> > + wacom->ps_connected = ps_connected;
> > + wacom_notify_battery(wacom);
> > + }
> > }
> > exit:
> > return retval;
> > @@ -1418,6 +1477,7 @@ void wacom_wac_irq(struct wacom_wac *wacom_wac, size_t len)
> >
> > case WACOM_G4:
> > case GRAPHIRE:
> > + case GRAPHIRE_BT:
> > case WACOM_MO:
> > sync = wacom_graphire_irq(wacom_wac);
> > break;
> > @@ -1654,6 +1714,27 @@ int wacom_setup_input_capabilities(struct input_dev *input_dev,
> > __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> > break;
> >
> > + case GRAPHIRE_BT:
> > + __clear_bit(ABS_MISC, input_dev->absbit);
> > + input_set_abs_params(input_dev, ABS_DISTANCE, 0,
> > + features->distance_max,
> > + 0, 0);
> > +
> > + input_set_capability(input_dev, EV_REL, REL_WHEEL);
> > +
> > + __set_bit(BTN_LEFT, input_dev->keybit);
> > + __set_bit(BTN_RIGHT, input_dev->keybit);
> > + __set_bit(BTN_MIDDLE, input_dev->keybit);
> > +
> > + __set_bit(BTN_TOOL_RUBBER, input_dev->keybit);
> > + __set_bit(BTN_TOOL_PEN, input_dev->keybit);
> > + __set_bit(BTN_TOOL_MOUSE, input_dev->keybit);
> > + __set_bit(BTN_STYLUS, input_dev->keybit);
> > + __set_bit(BTN_STYLUS2, input_dev->keybit);
> > +
> > + __set_bit(INPUT_PROP_POINTER, input_dev->propbit);
> > + break;
> > +
> > case WACOM_24HD:
> > input_set_abs_params(input_dev, ABS_Z, -900, 899, 0, 0);
> > input_set_abs_params(input_dev, ABS_THROTTLE, 0, 71, 0, 0);
> > @@ -1848,6 +1929,11 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
> > input_set_abs_params(input_dev, ABS_Y, 0, 1, 0, 0);
> >
> > switch (features->type) {
> > + case GRAPHIRE_BT:
> > + __set_bit(BTN_0, input_dev->keybit);
> > + __set_bit(BTN_1, input_dev->keybit);
> > + break;
> > +
> > case WACOM_MO:
> > __set_bit(BTN_BACK, input_dev->keybit);
> > __set_bit(BTN_LEFT, input_dev->keybit);
> > @@ -2017,6 +2103,9 @@ static const struct wacom_features wacom_features_0x00 =
> > static const struct wacom_features wacom_features_0x10 =
> > { "Wacom Graphire", 10206, 7422, 511, 63,
> > GRAPHIRE, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
> > +static const struct wacom_features wacom_features_0x81 =
> > + { "Wacom Graphire BT", 16704, 12064, 511, 32,
> > + GRAPHIRE_BT, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
> > static const struct wacom_features wacom_features_0x11 =
> > { "Wacom Graphire2 4x5", 10206, 7422, 511, 63,
> > GRAPHIRE, WACOM_GRAPHIRE_RES, WACOM_GRAPHIRE_RES };
> > @@ -2412,6 +2501,10 @@ static const struct wacom_features wacom_features_0x309 =
> > HID_DEVICE(BUS_USB, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
> > .driver_data = (kernel_ulong_t)&wacom_features_##prod
> >
> > +#define BT_DEVICE_WACOM(prod) \
> > + HID_DEVICE(BUS_BLUETOOTH, HID_GROUP_WACOM, USB_VENDOR_ID_WACOM, prod),\
> > + .driver_data = (kernel_ulong_t)&wacom_features_##prod
> > +
> > #define USB_DEVICE_LENOVO(prod) \
> > HID_USB_DEVICE(USB_VENDOR_ID_LENOVO, prod), \
> > .driver_data = (kernel_ulong_t)&wacom_features_##prod
> > @@ -2469,6 +2562,7 @@ const struct hid_device_id wacom_ids[] = {
> > { USB_DEVICE_WACOM(0x69) },
> > { USB_DEVICE_WACOM(0x6A) },
> > { USB_DEVICE_WACOM(0x6B) },
> > + { BT_DEVICE_WACOM(0x81) },
> > { USB_DEVICE_WACOM(0x84) },
> > { USB_DEVICE_WACOM(0x90) },
> > { USB_DEVICE_WACOM(0x93) },
> > diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
> > index 3aa55d9..0f3df34 100644
> > --- a/drivers/hid/wacom_wac.h
> > +++ b/drivers/hid/wacom_wac.h
> > @@ -46,6 +46,7 @@
> >
> > /* wacom data packet report IDs */
> > #define WACOM_REPORT_PENABLED 2
> > +#define WACOM_REPORT_PENABLED_BT 3
> > #define WACOM_REPORT_INTUOSREAD 5
> > #define WACOM_REPORT_INTUOSWRITE 6
> > #define WACOM_REPORT_INTUOSPAD 12
> > @@ -73,6 +74,7 @@
> > enum {
> > PENPARTNER = 0,
> > GRAPHIRE,
> > + GRAPHIRE_BT,
> > WACOM_G4,
> > PTU,
> > PL,
> > --
> > 2.0.1
> >
>
> --
> Dmitry

2014-07-24 19:58:33

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] Input - wacom: handle Graphire BT tablets in wacom.ko

On Thu, Jul 24, 2014 at 03:43:52PM -0400, Benjamin Tissoires wrote:
> On Jul 24 2014 or thereabouts, Dmitry Torokhov wrote:
> > Hi Benjamin,
> >
> > On Thu, Jul 24, 2014 at 02:14:02PM -0400, Benjamin Tissoires wrote:
> > > + } else if (features->type == GRAPHIRE_BT) {
> > > + /* Compute distance between mouse and tablet */
> > > + rw = 44 - (data[6] >> 2);
> > > + if (rw < 0)
> > > + rw = 0;
> > > + else if (rw > 31)
> > > + rw = 31;
> >
> > rw = clamp_val(rw, 0, 31);
> >
> > ?
>
> could be...
>
> I'll do the tests and resubmit later the v3. I'll wait a little in case
> you have other comments on the other patches :)

How about doing it incrementally - the patch is still sound as far as I
am concerned. I will wait a bit for other to chime in with comments or
acks before applying this series.

--
Dmitry

2014-07-24 20:12:15

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] Input - wacom: handle Graphire BT tablets in wacom.ko

On Jul 24 2014 or thereabouts, Dmitry Torokhov wrote:
> On Thu, Jul 24, 2014 at 03:43:52PM -0400, Benjamin Tissoires wrote:
> > On Jul 24 2014 or thereabouts, Dmitry Torokhov wrote:
> > > Hi Benjamin,
> > >
> > > On Thu, Jul 24, 2014 at 02:14:02PM -0400, Benjamin Tissoires wrote:
> > > > + } else if (features->type == GRAPHIRE_BT) {
> > > > + /* Compute distance between mouse and tablet */
> > > > + rw = 44 - (data[6] >> 2);
> > > > + if (rw < 0)
> > > > + rw = 0;
> > > > + else if (rw > 31)
> > > > + rw = 31;
> > >
> > > rw = clamp_val(rw, 0, 31);
> > >
> > > ?
> >
> > could be...
> >
> > I'll do the tests and resubmit later the v3. I'll wait a little in case
> > you have other comments on the other patches :)
>
> How about doing it incrementally - the patch is still sound as far as I
> am concerned. I will wait a bit for other to chime in with comments or
> acks before applying this series.
>

As you want.
Just making the exact changes you mentioned still makes the Graphire
working (what a surprise, isn't it?). So I am fine either way.

Cheers,
Benjamin

2014-07-24 20:43:42

by Ping Cheng

[permalink] [raw]
Subject: Re: [PATCH v2 07/10] Input - wacom: handle Graphire BT tablets in wacom.ko

On Thu, Jul 24, 2014 at 12:58 PM, Dmitry Torokhov
<[email protected]> wrote:
>
> On Thu, Jul 24, 2014 at 03:43:52PM -0400, Benjamin Tissoires wrote:
> > On Jul 24 2014 or thereabouts, Dmitry Torokhov wrote:
> > > Hi Benjamin,
> > >
> > > On Thu, Jul 24, 2014 at 02:14:02PM -0400, Benjamin Tissoires wrote:
> > > > + } else if (features->type == GRAPHIRE_BT) {
> > > > + /* Compute distance between mouse and tablet */
> > > > + rw = 44 - (data[6] >> 2);
> > > > + if (rw < 0)
> > > > + rw = 0;
> > > > + else if (rw > 31)
> > > > + rw = 31;
> > >
> > > rw = clamp_val(rw, 0, 31);
> > >
> > > ?
> >
> > could be...
> >
> > I'll do the tests and resubmit later the v3. I'll wait a little in case
> > you have other comments on the other patches :)
>
> How about doing it incrementally - the patch is still sound as far as I
> am concerned. I will wait a bit for other to chime in with comments or
> acks before applying this series.


Thank you Benjamin and Dmitry for your support. The whole series is:

Acked-by: Ping Cheng <[email protected]>

Cheers,

Ping

2014-07-25 12:43:12

by Przemo Firszt

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
[..]
> Hi Przemo,
Hi Benjamin,
> Normally, this series contains all the bits of hid-wacom (except the custom
> LED/OLED API). Please tell me if I am missing anything and if you like the
> change.

I can't cleanly apply your set yet to test it, so:

Acked-by: Przemo Firszt <[email protected]>

What's your plan about LED/OLED API?

> Benjamin Tissoires (9):
> Input - wacom: put a flag when the led are initialized
> Input - wacom: enhance Wireless Receiver battery reporting
> Input - wacom: use a uniq name for the battery device
> Input - wacom: register an ac power supply for wireless devices
> Input - wacom: prepare the driver to include BT devices
> Input - wacom: handle Graphire BT tablets in wacom.ko
> Input - wacom: handle Intuos 4 BT in wacom.ko
> Input - wacom: add copyright note and bump version to 2.0
> HID: remove hid-wacom Bluetooth driver
>
> Jason Gerecke (1):
> Input - wacom: Support up to 2048 pressure levels with ISDv4
>
> drivers/hid/Kconfig | 10 +-
> drivers/hid/Makefile | 3 +-
> drivers/hid/hid-core.c | 2 -
> drivers/hid/hid-wacom.c | 973 ------------------------------------------------
> drivers/hid/wacom.h | 11 +
> drivers/hid/wacom_sys.c | 217 ++++++++++-
> drivers/hid/wacom_wac.c | 195 +++++++++-
> drivers/hid/wacom_wac.h | 10 +
> 8 files changed, 415 insertions(+), 1006 deletions(-)
> delete mode 100644 drivers/hid/hid-wacom.c
>

--
Kind regards,
Przemo Firszt

2014-07-25 12:58:45

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Hi Przemo,

On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> [..]
> > Hi Przemo,
> Hi Benjamin,
> > Normally, this series contains all the bits of hid-wacom (except the custom
> > LED/OLED API). Please tell me if I am missing anything and if you like the
> > change.
>
> I can't cleanly apply your set yet to test it, so:

Hmm, you need to apply the first series I sent on July 15th, on top of
Dmitry's next branch.

http://www.spinics.net/lists/linux-input/msg32385.html

>
> Acked-by: Przemo Firszt <[email protected]>

Thanks!

>
> What's your plan about LED/OLED API?

I thought I would only preserve the current, wider used, LED/OLED API and
just drop the one in hid-wacom. This way, g-s-d will access both
bluetooth and USB the same way.

My decision was mostly guided because the support of BT in g-s-d was
only added recently (3.12 and backported to gnome 3.10 IIRC). And as
soon as these patches hit Dmitry's tree, I'll send the g-s-d patches to
fix all that.

Cheers,
Benjamin

>
> > Benjamin Tissoires (9):
> > Input - wacom: put a flag when the led are initialized
> > Input - wacom: enhance Wireless Receiver battery reporting
> > Input - wacom: use a uniq name for the battery device
> > Input - wacom: register an ac power supply for wireless devices
> > Input - wacom: prepare the driver to include BT devices
> > Input - wacom: handle Graphire BT tablets in wacom.ko
> > Input - wacom: handle Intuos 4 BT in wacom.ko
> > Input - wacom: add copyright note and bump version to 2.0
> > HID: remove hid-wacom Bluetooth driver
> >
> > Jason Gerecke (1):
> > Input - wacom: Support up to 2048 pressure levels with ISDv4
> >
> > drivers/hid/Kconfig | 10 +-
> > drivers/hid/Makefile | 3 +-
> > drivers/hid/hid-core.c | 2 -
> > drivers/hid/hid-wacom.c | 973 ------------------------------------------------
> > drivers/hid/wacom.h | 11 +
> > drivers/hid/wacom_sys.c | 217 ++++++++++-
> > drivers/hid/wacom_wac.c | 195 +++++++++-
> > drivers/hid/wacom_wac.h | 10 +
> > 8 files changed, 415 insertions(+), 1006 deletions(-)
> > delete mode 100644 drivers/hid/hid-wacom.c
> >
>
> --
> Kind regards,
> Przemo Firszt
>
>

2014-07-25 13:30:45

by Przemo Firszt

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Dnia 2014-07-25, pią o godzinie 08:58 -0400, Benjamin Tissoires pisze:
> Hi Przemo,
Hi Benjamin,
> On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> > Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> > [..]
> > > Hi Przemo,
> > Hi Benjamin,
> > > Normally, this series contains all the bits of hid-wacom (except the custom
> > > LED/OLED API). Please tell me if I am missing anything and if you like the
> > > change.
> >
> > I can't cleanly apply your set yet to test it, so:
>
> Hmm, you need to apply the first series I sent on July 15th, on top of
> Dmitry's next branch.
>
> http://www.spinics.net/lists/linux-input/msg32385.html
>
OK, thnaks!
> > Acked-by: Przemo Firszt <[email protected]>
>
> Thanks!
>
> >
> > What's your plan about LED/OLED API?
>
> I thought I would only preserve the current, wider used, LED/OLED API and
> just drop the one in hid-wacom. This way, g-s-d will access both
> bluetooth and USB the same way.
>
> My decision was mostly guided because the support of BT in g-s-d was
> only added recently (3.12 and backported to gnome 3.10 IIRC). And as
> soon as these patches hit Dmitry's tree, I'll send the g-s-d patches to
> fix all that.

If I understand you correctly we cannot have the same entry point in
sysfs for OLEDs unless we can tell userspace somehow if the tablet is
conected over USB or over bluetooth. The hardware of Intuos4 Wireless
over bluetooth allows only 1-bit images. The same hardware over USB
allows 4-bit images. Formatting of the images is also completely
different and it's not "plain". Check [1] for usb and exisitng
hid-wacom.c/wacom_scramble function for bluetooth.

If we want to make it consistent single entry on kernel level we
probably have to implement image conversion in the kernel. The user land
would always use 4-bit, plain formatted images and kernel driver would
convert them to 4-bit, wacom usb format or 1-bit wacom bluetooth format
depending on how the tablet is connected.

The downside of this approach is that the user land wouldn't have 100%
control over 1-bit images for bluetooth as kernel would have to create
them from 4-bit images.

[1] https://lkml.org/lkml/2012/9/9/80
--
Kind regards,
Przemo Firszt

2014-07-25 14:54:35

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> Dnia 2014-07-25, pią o godzinie 08:58 -0400, Benjamin Tissoires pisze:
> > Hi Przemo,
> Hi Benjamin,
> > On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> > > Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> > > [..]
> > > > Hi Przemo,
> > > Hi Benjamin,
> > > > Normally, this series contains all the bits of hid-wacom (except the custom
> > > > LED/OLED API). Please tell me if I am missing anything and if you like the
> > > > change.
> > >
> > > I can't cleanly apply your set yet to test it, so:
> >
> > Hmm, you need to apply the first series I sent on July 15th, on top of
> > Dmitry's next branch.
> >
> > http://www.spinics.net/lists/linux-input/msg32385.html
> >
> OK, thnaks!
> > > Acked-by: Przemo Firszt <[email protected]>
> >
> > Thanks!
> >
> > >
> > > What's your plan about LED/OLED API?
> >
> > I thought I would only preserve the current, wider used, LED/OLED API and
> > just drop the one in hid-wacom. This way, g-s-d will access both
> > bluetooth and USB the same way.
> >
> > My decision was mostly guided because the support of BT in g-s-d was
> > only added recently (3.12 and backported to gnome 3.10 IIRC). And as
> > soon as these patches hit Dmitry's tree, I'll send the g-s-d patches to
> > fix all that.
>
> If I understand you correctly we cannot have the same entry point in
> sysfs for OLEDs unless we can tell userspace somehow if the tablet is
> conected over USB or over bluetooth. The hardware of Intuos4 Wireless
> over bluetooth allows only 1-bit images. The same hardware over USB
> allows 4-bit images. Formatting of the images is also completely

Holy crap! I missed that. I did not noticed the 1-bit vs 4-bits
difference :(

> different and it's not "plain". Check [1] for usb and exisitng
> hid-wacom.c/wacom_scramble function for bluetooth.

Maybe I overlooked it, but I thought that in case of USB, the scrambling
is done in user space, and in case of BT, the same scrambling made in the
kernel. They looks very similar so I thought the user-space scramble for
USB would have fit. However, the 4-bits/1-bits kills that assumption.

>
> If we want to make it consistent single entry on kernel level we
> probably have to implement image conversion in the kernel. The user land
> would always use 4-bit, plain formatted images and kernel driver would
> convert them to 4-bit, wacom usb format or 1-bit wacom bluetooth format
> depending on how the tablet is connected.
>
> The downside of this approach is that the user land wouldn't have 100%
> control over 1-bit images for bluetooth as kernel would have to create
> them from 4-bit images.

The USB interface is *very* simple:
- if incoming data != 1024 -> reject
- forward everything to the device without in kernel treatment*

How about just changing the expected size to 256 in case of a BT tablet,
and let the user-space scramble in both cases and forward proper raw
data?

This way, user space will still have control over 1-bit vs 4-bits, and
the changes required in the kernel will be small enough.

My concern is that I'd like to have this series in 3.17 and I will not
have access to the hardware until next week :/
Having all of the user-spaces breakages in the same kernel release will
I think simplify the user space work.

Cheers,
Benjamin

* speaking about that: I just noticed that hid-wacom sent the
WAC_CMD_ICON_START_STOP message twice with 0 as the second parameter.
However, the USB spec tells that you have to use 1 for 'start' and 0
for 'stop'. Rather weird that this is working with both 0 in BT mode.

>
> [1] https://lkml.org/lkml/2012/9/9/80
> --
> Kind regards,
> Przemo Firszt
>
>

2014-07-25 19:26:26

by Przemo Firszt

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Dnia 2014-07-25, pią o godzinie 10:54 -0400, Benjamin Tissoires pisze:
> [..]
Hi Benjamin,
>
> > If I understand you correctly we cannot have the same entry point in
> > sysfs for OLEDs unless we can tell userspace somehow if the tablet is
> > conected over USB or over bluetooth. The hardware of Intuos4 Wireless
> > over bluetooth allows only 1-bit images. The same hardware over USB
> > allows 4-bit images. Formatting of the images is also completely
>
> Holy crap! I missed that. I did not noticed the 1-bit vs 4-bits
> difference :(
>
> > different and it's not "plain". Check [1] for usb and exisitng
> > hid-wacom.c/wacom_scramble function for bluetooth.
>
> Maybe I overlooked it, but I thought that in case of USB, the scrambling
> is done in user space, and in case of BT, the same scrambling made in the
> kernel. They looks very similar so I thought the user-space scramble for
> USB would have fit. However, the 4-bits/1-bits kills that assumption.

You're correct - USB requires scrambling in the user space, but
scrambling for USB is different than for BT. Example of USB scramble is
here: https://github.com/PrzemoF/i4oled/blob/master/src/i4oled.c#L401

> >
> > If we want to make it consistent single entry on kernel level we
> > probably have to implement image conversion in the kernel. The user land
> > would always use 4-bit, plain formatted images and kernel driver would
> > convert them to 4-bit, wacom usb format or 1-bit wacom bluetooth format
> > depending on how the tablet is connected.
> >
> > The downside of this approach is that the user land wouldn't have 100%
> > control over 1-bit images for bluetooth as kernel would have to create
> > them from 4-bit images.
>
> The USB interface is *very* simple:
> - if incoming data != 1024 -> reject
> - forward everything to the device without in kernel treatment*
>
> How about just changing the expected size to 256 in case of a BT tablet,
> and let the user-space scramble in both cases and forward proper raw
> data?
>
> This way, user space will still have control over 1-bit vs 4-bits, and
> the changes required in the kernel will be small enough.
Sounds good to me. Bit more coding in gnome, but it's a small thing.

> My concern is that I'd like to have this series in 3.17 and I will not
> have access to the hardware until next week :/
> Having all of the user-spaces breakages in the same kernel release will
> I think simplify the user space work.
I agree.

P.S. Could you send me that set of 23 patches using git-send-email that
I should apply on top of linux-next? I got it from lkml, but there is
still something messed (fails on patch 09/23). Of course, if it's not a
problem for you :-)

--
Kind regards,
Przemo Firszt

2014-07-25 19:33:22

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> Dnia 2014-07-25, pią o godzinie 10:54 -0400, Benjamin Tissoires pisze:
> > [..]
> Hi Benjamin,
> >
> > > If I understand you correctly we cannot have the same entry point in
> > > sysfs for OLEDs unless we can tell userspace somehow if the tablet is
> > > conected over USB or over bluetooth. The hardware of Intuos4 Wireless
> > > over bluetooth allows only 1-bit images. The same hardware over USB
> > > allows 4-bit images. Formatting of the images is also completely
> >
> > Holy crap! I missed that. I did not noticed the 1-bit vs 4-bits
> > difference :(
> >
> > > different and it's not "plain". Check [1] for usb and exisitng
> > > hid-wacom.c/wacom_scramble function for bluetooth.
> >
> > Maybe I overlooked it, but I thought that in case of USB, the scrambling
> > is done in user space, and in case of BT, the same scrambling made in the
> > kernel. They looks very similar so I thought the user-space scramble for
> > USB would have fit. However, the 4-bits/1-bits kills that assumption.
>
> You're correct - USB requires scrambling in the user space, but
> scrambling for USB is different than for BT. Example of USB scramble is
> here: https://github.com/PrzemoF/i4oled/blob/master/src/i4oled.c#L401
>
> > >
> > > If we want to make it consistent single entry on kernel level we
> > > probably have to implement image conversion in the kernel. The user land
> > > would always use 4-bit, plain formatted images and kernel driver would
> > > convert them to 4-bit, wacom usb format or 1-bit wacom bluetooth format
> > > depending on how the tablet is connected.
> > >
> > > The downside of this approach is that the user land wouldn't have 100%
> > > control over 1-bit images for bluetooth as kernel would have to create
> > > them from 4-bit images.
> >
> > The USB interface is *very* simple:
> > - if incoming data != 1024 -> reject
> > - forward everything to the device without in kernel treatment*
> >
> > How about just changing the expected size to 256 in case of a BT tablet,
> > and let the user-space scramble in both cases and forward proper raw
> > data?
> >
> > This way, user space will still have control over 1-bit vs 4-bits, and
> > the changes required in the kernel will be small enough.
> Sounds good to me. Bit more coding in gnome, but it's a small thing.

yeah, gnome already scramble for USB, let's have it scramble for BT too
:)

>
> > My concern is that I'd like to have this series in 3.17 and I will not
> > have access to the hardware until next week :/
> > Having all of the user-spaces breakages in the same kernel release will
> > I think simplify the user space work.
> I agree.

OK.

>
> P.S. Could you send me that set of 23 patches using git-send-email that
> I should apply on top of linux-next? I got it from lkml, but there is
> still something messed (fails on patch 09/23). Of course, if it's not a
> problem for you :-)

Sure, I can definitively do that. I also made a small patch to check for
1024 or 256 bits in OLEDs, so if you could give a try, that would be
great.

Cheers,
Benjamin

2014-07-25 22:13:41

by Przemo Firszt

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
[..]
Hi Benjamin,
I'm testing the whole series including the OLED patch that's not on the
list yet.

Hardware: 2 x Intuos4 Wireless tested on usb and bluetooth until noted
otherwise.

What works:
1. Tablet in general, pressure, tilt, buttons etc.
2. Battery reporting (including gnome). The double wireless tablet bug
is gone:

$ ls /sys/class/power_supply/
AC BAT0 wacom_ac_2 wacom_ac_3 wacom_battery_2 wacom_battery_3

3. Setting LED selector value
4. Setting LED selector brightness (default and pressed)
5. Rendering images to button displays works on usb ONLY.

$ i4oled -d /sys/bus/hid/drivers/wacom/0003\:056A\:00BC.0009/wacom_led/button0_rawimg -t Linux

On bluetooth writing image goes fine (no error), but there is nothing showing up,
so I suspect the brightness of OLED displays is not set properly.

That's the code before changes:

led = wdata->led_selector | 0x04;
buf = kzalloc(9, GFP_KERNEL);
if (buf) {
buf[0] = WAC_CMD_LED_CONTROL;
buf[1] = led;
buf[2] = value >> 2;
buf[3] = value;
/* use fixed brightness for OLEDs */
buf[4] = 0x08;
hid_hw_raw_request(hdev, buf[0], buf, 9, HID_FEATURE_REPORT,
HID_REQ_SET_REPORT);
kfree(buf);
}

I don't remember for sure, but I think the range of brightness might be different
over usb and over bluetooth.

TL;DR: the only thing that needs to be fixed is image-over-bluetooth, probably caused by not
setting or incorrect setting of OLED brightness.

--
Kind regards,
Przemo Firszt

2014-07-25 23:15:33

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2


Hi Przemo,

On Jul 25 2014 or thereabouts, Przemo Firszt wrote:
> Dnia 2014-07-24, czw o godzinie 14:13 -0400, Benjamin Tissoires pisze:
> [..]
> Hi Benjamin,
> I'm testing the whole series including the OLED patch that's not on the
> list yet.
>
> Hardware: 2 x Intuos4 Wireless tested on usb and bluetooth until noted
> otherwise.
>
> What works:
> 1. Tablet in general, pressure, tilt, buttons etc.
> 2. Battery reporting (including gnome). The double wireless tablet bug
> is gone:
>
> $ ls /sys/class/power_supply/
> AC BAT0 wacom_ac_2 wacom_ac_3 wacom_battery_2 wacom_battery_3
>
> 3. Setting LED selector value
> 4. Setting LED selector brightness (default and pressed)
> 5. Rendering images to button displays works on usb ONLY.
>
> $ i4oled -d /sys/bus/hid/drivers/wacom/0003\:056A\:00BC.0009/wacom_led/button0_rawimg -t Linux
>
> On bluetooth writing image goes fine (no error), but there is nothing showing up,
> so I suspect the brightness of OLED displays is not set properly.
>
> That's the code before changes:
>
> led = wdata->led_selector | 0x04;
> buf = kzalloc(9, GFP_KERNEL);
> if (buf) {
> buf[0] = WAC_CMD_LED_CONTROL;
> buf[1] = led;
> buf[2] = value >> 2;
> buf[3] = value;
> /* use fixed brightness for OLEDs */
> buf[4] = 0x08;
> hid_hw_raw_request(hdev, buf[0], buf, 9, HID_FEATURE_REPORT,
> HID_REQ_SET_REPORT);
> kfree(buf);
> }
>
> I don't remember for sure, but I think the range of brightness might be different
> over usb and over bluetooth.

Maybe you can try setting the sysfs file "buttons_luminance" with the
value 8 to check if this will solve the bug.

The bug might also be linked to the slight difference while setting up
the transfer of the image (WAC_CMD_ICON_START) with the value of buf[1]
set to 1 in USB, while it was 0 on bluetooth.

The weird thing is that I remembered having set the OLED (though
scrambled) with these patches applied. I guess the scrambling was due to
the 4-bit vs 1-bit. But I definitively had some results.

Anyway. Przemo, Dmitry, can we consider that this *will* be fixed by
next week, and so we can apply the series for 3.17?
I will have the hardware next week and be able to figure out the
differences between the 2 communication modes.

>
> TL;DR: the only thing that needs to be fixed is image-over-bluetooth, probably caused by not
> setting or incorrect setting of OLED brightness.

Thanks for the extensive testings. I'll send the "OLED patch that's not
on the list yet" as a 11/10 so everybody can have a look.

Cheers,
Benjamin

2014-07-25 23:20:32

by Benjamin Tissoires

[permalink] [raw]
Subject: [PATCH 11/10] Input - wacom: Check for bluetooth protocol while setting OLEDs

Bluetooth Intuos 4 use 1-bit definition while the USB ones use a 4-bits
definition. This changes the size of the raw image we receive, and thus
the kernel will only accept 1-bit images for Bluetooth and 4-bits for
USB.

Signed-off-by: Benjamin Tissoires <[email protected]>
---
drivers/hid/wacom_sys.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 3adc6ef..42f139f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -531,12 +531,14 @@ static int wacom_led_control(struct wacom *wacom)
return retval;
}

-static int wacom_led_putimage(struct wacom *wacom, int button_id, const void *img)
+static int wacom_led_putimage(struct wacom *wacom, int button_id,
+ const unsigned len, const void *img)
{
unsigned char *buf;
int i, retval;
+ const unsigned chunk_len = len / 4; /* 4 chunks are needed to be sent */

- buf = kzalloc(259, GFP_KERNEL);
+ buf = kzalloc(chunk_len + 3 , GFP_KERNEL);
if (!buf)
return -ENOMEM;

@@ -552,11 +554,11 @@ static int wacom_led_putimage(struct wacom *wacom, int button_id, const void *im
buf[1] = button_id & 0x07;
for (i = 0; i < 4; i++) {
buf[2] = i;
- memcpy(buf + 3, img + i * 256, 256);
+ memcpy(buf + 3, img + i * chunk_len, chunk_len);

retval = wacom_set_report(wacom->hdev, HID_FEATURE_REPORT,
WAC_CMD_ICON_XFER,
- buf, 259, WAC_CMD_RETRIES);
+ buf, chunk_len + 3, WAC_CMD_RETRIES);
if (retval < 0)
break;
}
@@ -657,13 +659,14 @@ static ssize_t wacom_button_image_store(struct device *dev, int button_id,
struct hid_device *hdev = container_of(dev, struct hid_device, dev);
struct wacom *wacom = hid_get_drvdata(hdev);
int err;
+ const unsigned len = hdev->bus == BUS_BLUETOOTH ? 256 : 1024;

- if (count != 1024)
+ if (count != len)
return -EINVAL;

mutex_lock(&wacom->lock);

- err = wacom_led_putimage(wacom, button_id, buf);
+ err = wacom_led_putimage(wacom, button_id, len, buf);

mutex_unlock(&wacom->lock);

--
2.0.1

2014-07-26 00:39:15

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] Input - wacom: prepare the driver to include BT devices

Hi Benjamin,

On Thu, Jul 24, 2014 at 02:14:01PM -0400, Benjamin Tissoires wrote:
> Now that wacom is a hid driver, there is no point in having a separate
> driver for bluetooth devices.
> This patch prepares the common paths of Bluetooth devices in the
> common wacom driver.
> It also adds the sysfs file "speed" used by Bluetooth devices.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
>
> new in v2
>
> drivers/hid/wacom_sys.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++---
> drivers/hid/wacom_wac.h | 2 ++
> 2 files changed, 69 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index d0d06b8..add76ec 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -262,6 +262,12 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> return error < 0 ? error : 0;
> }
>
> +static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
> + struct wacom_features *features)
> +{
> + return 0;
> +}
> +
> /*
> * Switch the tablet into its most-capable mode. Wacom tablets are
> * typically configured to power-up in a mode which sends mouse-like
> @@ -272,6 +278,9 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> static int wacom_query_tablet_data(struct hid_device *hdev,
> struct wacom_features *features)
> {
> + if (hdev->bus == BUS_BLUETOOTH)
> + return wacom_bt_query_tablet_data(hdev, 1, features);
> +
> if (features->device_type == BTN_TOOL_FINGER) {
> if (features->type > TABLETPC) {
> /* MT Tablet PC touch */
> @@ -890,6 +899,38 @@ static void wacom_destroy_battery(struct wacom *wacom)
> }
> }
>
> +static ssize_t wacom_show_speed(struct device *dev,
> + struct device_attribute
> + *attr, char *buf)
> +{
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct wacom *wacom = hid_get_drvdata(hdev);
> +
> + return snprintf(buf, PAGE_SIZE, "%i\n", wacom->wacom_wac.bt_high_speed);
> +}
> +
> +static ssize_t wacom_store_speed(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> + struct wacom *wacom = hid_get_drvdata(hdev);
> + int new_speed;
> +
> + if (sscanf(buf, "%1d", &new_speed ) != 1)

Checkpach is unhappy with ')' placement and I agree with it.

> + return -EINVAL;

kstrtou8?

> +
> + if (new_speed == 0 || new_speed == 1) {
> + wacom_bt_query_tablet_data(hdev, new_speed,
> + &wacom->wacom_wac.features);
> + return strnlen(buf, PAGE_SIZE);

This is weird. Normally you want to return count since you should refuse
input with excessive data.

> + } else
> + return -EINVAL;

Need braces on both branches.

Thanks.

--
Dmitry

2014-07-26 01:41:35

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

Hi Benjamin,

On Thu, Jul 24, 2014 at 02:13:55PM -0400, Benjamin Tissoires wrote:
> Hi Dmitry,
>
> this is the second series I told you about for wacom.ko. This series also have
> a good number of removed lines of code. \o/
>
> The first patch is Jason's one that I finally decided to take with me. His
> previous submission still applied correctly even after the moving of the files
> (git is definitively awesome).
>
> The second one is a patch I sent earlier and forgot to include in the v2 of
> the first series. It might have been dropped during my many rebases. So here
> he is.
>
> The rest is for one part enhancing the battery reporting system (to make it
> equal to the one in hid-wacom, and even slightly better). The other part
> is the actual merge of hid-wacom into wacom which gives the same user space API
> for bluetooth and USB devices, fixes the pad-in-a-separate-input-dev, and
> fixes the missing tools not supported in the previous implementation of
> hid-wacom for Intuos 4 BT.
>

I ended up taking 3.16-rc6 and applying your first series and the first
5 patches of this series to it. You should be able to see the result on
kernel.org in wacom branch.

Thanks.

--
Dmitry

2014-07-26 03:21:52

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

On Jul 25 2014 or thereabouts, Dmitry Torokhov wrote:
> Hi Benjamin,
>
> On Thu, Jul 24, 2014 at 02:13:55PM -0400, Benjamin Tissoires wrote:
> > Hi Dmitry,
> >
> > this is the second series I told you about for wacom.ko. This series also have
> > a good number of removed lines of code. \o/
> >
> > The first patch is Jason's one that I finally decided to take with me. His
> > previous submission still applied correctly even after the moving of the files
> > (git is definitively awesome).
> >
> > The second one is a patch I sent earlier and forgot to include in the v2 of
> > the first series. It might have been dropped during my many rebases. So here
> > he is.
> >
> > The rest is for one part enhancing the battery reporting system (to make it
> > equal to the one in hid-wacom, and even slightly better). The other part
> > is the actual merge of hid-wacom into wacom which gives the same user space API
> > for bluetooth and USB devices, fixes the pad-in-a-separate-input-dev, and
> > fixes the missing tools not supported in the previous implementation of
> > hid-wacom for Intuos 4 BT.
> >
>
> I ended up taking 3.16-rc6 and applying your first series and the first
> 5 patches of this series to it. You should be able to see the result on
> kernel.org in wacom branch.

Cool. The resolution of the hid-core.c conflict is in fact better than
what I proposed. It should not conflict with Jiri's pull request IMO. And
when the two branches will hit Linus' we can then un-split hid-rmi and
wacom processes.

I also noticed few differences. Some of them you obviously made, I am
fine with them BTW, and some other I think because your current next
branch already has some wacom patches queued. I guess you will handle
this just fine, as usual, but I'll try to keep an eye on it just in case
git messed up the merge and forgot one branch.

Thanks again Dmitry. And sorry for have pushed you regarding that.

Cheers,
Benjamin

2014-07-26 03:25:40

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] Input - wacom: prepare the driver to include BT devices

On Jul 25 2014 or thereabouts, Dmitry Torokhov wrote:
> Hi Benjamin,
>
> On Thu, Jul 24, 2014 at 02:14:01PM -0400, Benjamin Tissoires wrote:
> > Now that wacom is a hid driver, there is no point in having a separate
> > driver for bluetooth devices.
> > This patch prepares the common paths of Bluetooth devices in the
> > common wacom driver.
> > It also adds the sysfs file "speed" used by Bluetooth devices.
> >
> > Signed-off-by: Benjamin Tissoires <[email protected]>
> > ---
> >
> > new in v2
> >
> > drivers/hid/wacom_sys.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++---
> > drivers/hid/wacom_wac.h | 2 ++
> > 2 files changed, 69 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index d0d06b8..add76ec 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -262,6 +262,12 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> > return error < 0 ? error : 0;
> > }
> >
> > +static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
> > + struct wacom_features *features)
> > +{
> > + return 0;
> > +}
> > +
> > /*
> > * Switch the tablet into its most-capable mode. Wacom tablets are
> > * typically configured to power-up in a mode which sends mouse-like
> > @@ -272,6 +278,9 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> > static int wacom_query_tablet_data(struct hid_device *hdev,
> > struct wacom_features *features)
> > {
> > + if (hdev->bus == BUS_BLUETOOTH)
> > + return wacom_bt_query_tablet_data(hdev, 1, features);
> > +
> > if (features->device_type == BTN_TOOL_FINGER) {
> > if (features->type > TABLETPC) {
> > /* MT Tablet PC touch */
> > @@ -890,6 +899,38 @@ static void wacom_destroy_battery(struct wacom *wacom)
> > }
> > }
> >
> > +static ssize_t wacom_show_speed(struct device *dev,
> > + struct device_attribute
> > + *attr, char *buf)
> > +{
> > + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> > + struct wacom *wacom = hid_get_drvdata(hdev);
> > +
> > + return snprintf(buf, PAGE_SIZE, "%i\n", wacom->wacom_wac.bt_high_speed);
> > +}
> > +
> > +static ssize_t wacom_store_speed(struct device *dev,
> > + struct device_attribute *attr,
> > + const char *buf, size_t count)
> > +{
> > + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> > + struct wacom *wacom = hid_get_drvdata(hdev);
> > + int new_speed;
> > +
> > + if (sscanf(buf, "%1d", &new_speed ) != 1)
>
> Checkpach is unhappy with ')' placement and I agree with it.
>

ouch

> > + return -EINVAL;
>
> kstrtou8?

re-ouch

>
> > +
> > + if (new_speed == 0 || new_speed == 1) {
> > + wacom_bt_query_tablet_data(hdev, new_speed,
> > + &wacom->wacom_wac.features);
> > + return strnlen(buf, PAGE_SIZE);
>
> This is weird. Normally you want to return count since you should refuse
> input with excessive data.

indeed

>
> > + } else
> > + return -EINVAL;
>
> Need braces on both branches.
>

Grmblmbl. I should not have blinded copied the code from one driver to
one other. I will send a v3 of the rest of the series on top of your
wacom branch, at some point next week. Other people can still try to
find out other mistakes meanwhile ;)

Thanks for the review.

Cheers,
Benjamin

2014-07-26 03:56:35

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2

On Fri, Jul 25, 2014 at 11:21:27PM -0400, Benjamin Tissoires wrote:
> On Jul 25 2014 or thereabouts, Dmitry Torokhov wrote:
> > Hi Benjamin,
> >
> > On Thu, Jul 24, 2014 at 02:13:55PM -0400, Benjamin Tissoires wrote:
> > > Hi Dmitry,
> > >
> > > this is the second series I told you about for wacom.ko. This series also have
> > > a good number of removed lines of code. \o/
> > >
> > > The first patch is Jason's one that I finally decided to take with me. His
> > > previous submission still applied correctly even after the moving of the files
> > > (git is definitively awesome).
> > >
> > > The second one is a patch I sent earlier and forgot to include in the v2 of
> > > the first series. It might have been dropped during my many rebases. So here
> > > he is.
> > >
> > > The rest is for one part enhancing the battery reporting system (to make it
> > > equal to the one in hid-wacom, and even slightly better). The other part
> > > is the actual merge of hid-wacom into wacom which gives the same user space API
> > > for bluetooth and USB devices, fixes the pad-in-a-separate-input-dev, and
> > > fixes the missing tools not supported in the previous implementation of
> > > hid-wacom for Intuos 4 BT.
> > >
> >
> > I ended up taking 3.16-rc6 and applying your first series and the first
> > 5 patches of this series to it. You should be able to see the result on
> > kernel.org in wacom branch.
>
> Cool. The resolution of the hid-core.c conflict is in fact better than
> what I proposed. It should not conflict with Jiri's pull request IMO. And
> when the two branches will hit Linus' we can then un-split hid-rmi and
> wacom processes.
>
> I also noticed few differences. Some of them you obviously made, I am
> fine with them BTW, and some other I think because your current next
> branch already has some wacom patches queued. I guess you will handle
> this just fine, as usual, but I'll try to keep an eye on it just in case
> git messed up the merge and forgot one branch.
>
> Thanks again Dmitry. And sorry for have pushed you regarding that.

No worries, I need to be pushed sometimes ;)

--
Dmitry

2014-07-26 03:58:25

by Dmitry Torokhov

[permalink] [raw]
Subject: Re: [PATCH v2 06/10] Input - wacom: prepare the driver to include BT devices

On Fri, Jul 25, 2014 at 11:25:17PM -0400, Benjamin Tissoires wrote:
> On Jul 25 2014 or thereabouts, Dmitry Torokhov wrote:
> > Hi Benjamin,
> >
> > On Thu, Jul 24, 2014 at 02:14:01PM -0400, Benjamin Tissoires wrote:
> > > Now that wacom is a hid driver, there is no point in having a separate
> > > driver for bluetooth devices.
> > > This patch prepares the common paths of Bluetooth devices in the
> > > common wacom driver.
> > > It also adds the sysfs file "speed" used by Bluetooth devices.
> > >
> > > Signed-off-by: Benjamin Tissoires <[email protected]>
> > > ---
> > >
> > > new in v2
> > >
> > > drivers/hid/wacom_sys.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++---
> > > drivers/hid/wacom_wac.h | 2 ++
> > > 2 files changed, 69 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > > index d0d06b8..add76ec 100644
> > > --- a/drivers/hid/wacom_sys.c
> > > +++ b/drivers/hid/wacom_sys.c
> > > @@ -262,6 +262,12 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> > > return error < 0 ? error : 0;
> > > }
> > >
> > > +static int wacom_bt_query_tablet_data(struct hid_device *hdev, u8 speed,
> > > + struct wacom_features *features)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > /*
> > > * Switch the tablet into its most-capable mode. Wacom tablets are
> > > * typically configured to power-up in a mode which sends mouse-like
> > > @@ -272,6 +278,9 @@ static int wacom_set_device_mode(struct hid_device *hdev, int report_id,
> > > static int wacom_query_tablet_data(struct hid_device *hdev,
> > > struct wacom_features *features)
> > > {
> > > + if (hdev->bus == BUS_BLUETOOTH)
> > > + return wacom_bt_query_tablet_data(hdev, 1, features);
> > > +
> > > if (features->device_type == BTN_TOOL_FINGER) {
> > > if (features->type > TABLETPC) {
> > > /* MT Tablet PC touch */
> > > @@ -890,6 +899,38 @@ static void wacom_destroy_battery(struct wacom *wacom)
> > > }
> > > }
> > >
> > > +static ssize_t wacom_show_speed(struct device *dev,
> > > + struct device_attribute
> > > + *attr, char *buf)
> > > +{
> > > + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> > > + struct wacom *wacom = hid_get_drvdata(hdev);
> > > +
> > > + return snprintf(buf, PAGE_SIZE, "%i\n", wacom->wacom_wac.bt_high_speed);
> > > +}
> > > +
> > > +static ssize_t wacom_store_speed(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> > > + struct wacom *wacom = hid_get_drvdata(hdev);
> > > + int new_speed;
> > > +
> > > + if (sscanf(buf, "%1d", &new_speed ) != 1)
> >
> > Checkpach is unhappy with ')' placement and I agree with it.
> >
>
> ouch
>
> > > + return -EINVAL;
> >
> > kstrtou8?
>
> re-ouch
>
> >
> > > +
> > > + if (new_speed == 0 || new_speed == 1) {
> > > + wacom_bt_query_tablet_data(hdev, new_speed,
> > > + &wacom->wacom_wac.features);
> > > + return strnlen(buf, PAGE_SIZE);
> >
> > This is weird. Normally you want to return count since you should refuse
> > input with excessive data.
>
> indeed
>
> >
> > > + } else
> > > + return -EINVAL;
> >
> > Need braces on both branches.
> >
>
> Grmblmbl. I should not have blinded copied the code from one driver to
> one other. I will send a v3 of the rest of the series on top of your
> wacom branch, at some point next week. Other people can still try to
> find out other mistakes meanwhile ;)

Great, will be waiting for it and then will merge everything into next for
3.17.

Thanks.

--
Dmitry

2014-07-26 11:55:58

by Przemo Firszt

[permalink] [raw]
Subject: Re: [PATCH v2 00/10] Input - wacom: conversion to HID driver, series 2


Dnia 2014-07-25, pią o godzinie 19:15 -0400, Benjamin Tissoires pisze:
> [..]
Hi Bejnamin,
> > I don't remember for sure, but I think the range of brightness might be different
> > over usb and over bluetooth.
>
> Maybe you can try setting the sysfs file "buttons_luminance" with the
> value 8 to check if this will solve the bug.
No, that doesn't make any difference.

> The bug might also be linked to the slight difference while setting up
> the transfer of the image (WAC_CMD_ICON_START) with the value of buf[1]
> set to 1 in USB, while it was 0 on bluetooth.
>
> The weird thing is that I remembered having set the OLED (though
> scrambled) with these patches applied. I guess the scrambling was due to
> the 4-bit vs 1-bit. But I definitively had some results.
I spotted something:
hid-wacom:c (before modifications)
#define WAC_CMD_ICON_TRANSFER 0x26

wacom_sys.c: (after modifications)
#define WAC_CMD_ICON_XFER 0x23

Changing it fixes the problem. Patches to follow soon.
--
Kind regards,
Przemo Firszt

2014-07-26 12:05:42

by Przemo Firszt

[permalink] [raw]
Subject: Re: [PATCH 11/10] Input - wacom: Check for bluetooth protocol while setting OLEDs

Whole series (after including bt image fix):
Tested-By: Przemo Firszt <[email protected]>

Dnia 2014-07-25, pią o godzinie 19:20 -0400, Benjamin Tissoires pisze:
> Bluetooth Intuos 4 use 1-bit definition while the USB ones use a 4-bits
> definition. This changes the size of the raw image we receive, and thus
> the kernel will only accept 1-bit images for Bluetooth and 4-bits for
> USB.
>
> Signed-off-by: Benjamin Tissoires <[email protected]>
> ---
> drivers/hid/wacom_sys.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 3adc6ef..42f139f 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -531,12 +531,14 @@ static int wacom_led_control(struct wacom *wacom)
> return retval;
> }
>
> -static int wacom_led_putimage(struct wacom *wacom, int button_id, const void *img)
> +static int wacom_led_putimage(struct wacom *wacom, int button_id,
> + const unsigned len, const void *img)
> {
> unsigned char *buf;
> int i, retval;
> + const unsigned chunk_len = len / 4; /* 4 chunks are needed to be sent */
>
> - buf = kzalloc(259, GFP_KERNEL);
> + buf = kzalloc(chunk_len + 3 , GFP_KERNEL);
> if (!buf)
> return -ENOMEM;
>
> @@ -552,11 +554,11 @@ static int wacom_led_putimage(struct wacom *wacom, int button_id, const void *im
> buf[1] = button_id & 0x07;
> for (i = 0; i < 4; i++) {
> buf[2] = i;
> - memcpy(buf + 3, img + i * 256, 256);
> + memcpy(buf + 3, img + i * chunk_len, chunk_len);
>
> retval = wacom_set_report(wacom->hdev, HID_FEATURE_REPORT,
> WAC_CMD_ICON_XFER,
> - buf, 259, WAC_CMD_RETRIES);
> + buf, chunk_len + 3, WAC_CMD_RETRIES);
> if (retval < 0)
> break;
> }
> @@ -657,13 +659,14 @@ static ssize_t wacom_button_image_store(struct device *dev, int button_id,
> struct hid_device *hdev = container_of(dev, struct hid_device, dev);
> struct wacom *wacom = hid_get_drvdata(hdev);
> int err;
> + const unsigned len = hdev->bus == BUS_BLUETOOTH ? 256 : 1024;
>
> - if (count != 1024)
> + if (count != len)
> return -EINVAL;
>
> mutex_lock(&wacom->lock);
>
> - err = wacom_led_putimage(wacom, button_id, buf);
> + err = wacom_led_putimage(wacom, button_id, len, buf);
>
> mutex_unlock(&wacom->lock);
>

--
Kind regards,
Przemo Firszt