Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1437063 > unrolled thread

[PATCH 00/27] HID: wacom: cleanup/EKR/LED

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/27] HID: wacom: cleanup/EKR/LED Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:40 +0200
    [PATCH 23/27] HID: wacom: leds: use the ledclass instead of custom made sysfs files Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 16/27] HID: wacom: rework fail path in probe() and parse_and_register() Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 17/27] HID: wacom: EKR: have proper allocator and destructor Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 11/27] HID: wacom: use devres to allocate driver data Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 07/27] HID: wacom: switch inputs to devres Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 25/27] HID: wacom: leds: fix ordering of LED banks Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 08/27] HID: wacom: put the managed resources in a group Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs directly in the kernel Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
      Re: [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs  directly in the kernel kbuild test robot <lkp@intel.com> - 2016-07-05 18:10 +0200
        Re: [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs  directly in the kernel Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 23:40 +0200
    [PATCH 02/27] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2 Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 10/27] HID: wacom: use devm_kasprintf for allocating the name of the remote Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 22/27] HID: wacom: EKR: attach the power_supply on first connection Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 01/27] HID: wacom: actually report the battery level for wireless connected Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 21/27] HID: wacom: EKR: have one power_supply per remote Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 06/27] HID: wacom: switch battery to devres Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 09/27] HID: wacom: convert LEDs to devres Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 20/27] HID: wacom: EKR: allocate one input node per remote Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 27/27] HID: wacom: leds: handle Cintiq 24HD leds buttons Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
      Re: [PATCH 27/27] HID: wacom: leds: handle Cintiq 24HD leds buttons kbuild test robot <lkp@intel.com> - 2016-07-05 18:30 +0200
    [PATCH 19/27] HID: wacom: EKR: have one array of struct remotes instead of many arrays Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 12/27] HID: wacom: devres manage the shared data too Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 18/27] HID: wacom: EKR: use devres groups to manage resources Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 13/27] HID: wacom: leds: dynamically allocate LED groups Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 15/27] HID: wacom: EKR: have the wacom resources dynamically allocated Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 14/27] HID: wacom: EKR: add a worker to add/remove resources on addition/removal Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 24/27] HID: wacom: leds: actually release the LEDs on disconnect Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200
    [PATCH 05/27] HID: wacom: use one work queue per task Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-05 16:50 +0200

Page 1 of 2  [1] 2  Next page →

#1437063 — [PATCH 00/27] HID: wacom: cleanup/EKR/LED

Hi,

Initially, I thought the series would have been short because it was driven
by 23/27 and 26/27. We wanted to have a common API for the LED in the kernel
and also let the kernel switches the LED for us so that libinput doesn't need
to have root access. The LED kernel API has kernel triggers which allows users
to enable/disable automatic LED switching.

Well, the plan did not go so well, and I ended up cleaning up the Wacom driver
one more time (Daft Punk's song complimentary). I finally made use of devres
management in this driver which allows to simplify the cleanups of the various
resources allocated here and there. I had to resort most of the time to devres
groups because of the 2 wireless receiver Wacom has. With these 2 receivers
(Intuos Pro/Bamboos and the EKR), input nodes can be created/destroyed at
will, without having the parent HID device removed.

The plan was also to provide a common API for the EKR and the rest of the
Intuos/Cintiqs. And working on this reminded me that we never finished providing
proper support of the Wacom ExpressKey Remote (one input node per remote,
one power_supply per remote).

So, here is the result of this past few weeks. I tested this on the following
devices: Intuos Pro, Bamboo 16 fg Pen/Touch, Bamboo Pad, Cintiq 13HD and
the EKR.
Peter tested an earlier series on the 21UX2 and I hope I fixed the bugs he
noted, but I would love the Wacom guys to test the series on the 21UX2 (again)
and the 24HD(T). I was only able to test those 2 devices through uhid, and
manually checked the raw commands were set properly, but an actual test would be
better.

Happy reviewing :)

Cheers,
Benjamin


Benjamin Tissoires (27):
  HID: wacom: actually report the battery level for wireless connected
  HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2
  HID: wacom: remove cleanup of wacom->remote_dir from
    wacom_clean_inputs()
  HID: wacom: untie leds from inputs
  HID: wacom: use one work queue per task
  HID: wacom: switch battery to devres
  HID: wacom: switch inputs to devres
  HID: wacom: put the managed resources in a group
  HID: wacom: convert LEDs to devres
  HID: wacom: use devm_kasprintf for allocating the name of the remote
  HID: wacom: use devres to allocate driver data
  HID: wacom: devres manage the shared data too
  HID: wacom: leds: dynamically allocate LED groups
  HID: wacom: EKR: add a worker to add/remove resources on
    addition/removal
  HID: wacom: EKR: have the wacom resources dynamically allocated
  HID: wacom: rework fail path in probe() and parse_and_register()
  HID: wacom: EKR: have proper allocator and destructor
  HID: wacom: EKR: use devres groups to manage resources
  HID: wacom: EKR: have one array of struct remotes instead of many
    arrays
  HID: wacom: EKR: allocate one input node per remote
  HID: wacom: EKR: have one power_supply per remote
  HID: wacom: EKR: attach the power_supply on first connection
  HID: wacom: leds: use the ledclass instead of custom made sysfs files
  HID: wacom: leds: actually release the LEDs on disconnect
  HID: wacom: leds: fix ordering of LED banks
  HID: wacom: leds: handle the switch of the LEDs directly in the kernel
  HID: wacom: leds: handle Cintiq 24HD leds buttons

 Documentation/ABI/testing/sysfs-driver-wacom |    5 +
 drivers/hid/wacom.h                          |   98 ++-
 drivers/hid/wacom_sys.c                      | 1104 +++++++++++++++++++-------
 drivers/hid/wacom_wac.c                      |  238 ++++--
 drivers/hid/wacom_wac.h                      |   18 +-
 5 files changed, 1073 insertions(+), 390 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]

#1437067 — [PATCH 23/27] HID: wacom: leds: use the ledclass instead of custom made sysfs files

The now obsolete sysfs files for LEDs and EKRemote are kept for backward
compatibility.
Both the EKR (read-only) and the regular Cintiqs and Intuos are now
sharing the same led API.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 Documentation/ABI/testing/sysfs-driver-wacom |   5 +
 drivers/hid/wacom.h                          |  19 +++
 drivers/hid/wacom_sys.c                      | 191 +++++++++++++++++++++++++--
 3 files changed, 205 insertions(+), 10 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-driver-wacom b/Documentation/ABI/testing/sysfs-driver-wacom
index dca4293..2aa5503 100644
--- a/Documentation/ABI/testing/sysfs-driver-wacom
+++ b/Documentation/ABI/testing/sysfs-driver-wacom
@@ -24,6 +24,7 @@ What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/wacom_led/status0_luminance
 Date:		August 2014
 Contact:	linux-input@vger.kernel.org
 Description:
+		<obsoleted by the LED class API now exported by the driver>
 		Writing to this file sets the status LED luminance (1..127)
 		when the stylus does not touch the tablet surface, and no
 		button is pressed on the stylus. This luminance level is
@@ -33,6 +34,7 @@ What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/wacom_led/status1_luminance
 Date:		August 2014
 Contact:	linux-input@vger.kernel.org
 Description:
+		<obsoleted by the LED class API now exported by the driver>
 		Writing to this file sets the status LED luminance (1..127)
 		when the stylus touches the tablet surface, or any button is
 		pressed on the stylus.
@@ -41,6 +43,7 @@ What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/wacom_led/status_led0_select
 Date:		August 2014
 Contact:	linux-input@vger.kernel.org
 Description:
+		<obsoleted by the LED class API now exported by the driver>
 		Writing to this file sets which one of the four (for Intuos 4
 		and Intuos 5) or of the right four (for Cintiq 21UX2 and Cintiq
 		24HD) status LEDs is active (0..3). The other three LEDs on the
@@ -50,6 +53,7 @@ What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/wacom_led/status_led1_select
 Date:		August 2014
 Contact:	linux-input@vger.kernel.org
 Description:
+		<obsoleted by the LED class API now exported by the driver>
 		Writing to this file sets which one of the left four (for Cintiq 21UX2
 		and Cintiq 24HD) status LEDs is active (0..3). The other three LEDs on
 		the left are always inactive.
@@ -91,6 +95,7 @@ What:		/sys/bus/hid/devices/<bus>:<vid>:<pid>.<n>/wacom_remote/<serial_number>/r
 Date:		July 2015
 Contact:	linux-input@vger.kernel.org
 Description:
+		<obsoleted by the LED class API now exported by the driver>
 		Reading from this file reports the mode status of the
 		remote as indicated by the LED lights on the device. If no
 		reports have been received from the paired device, reading
diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 768d696..0c498c4 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -91,6 +91,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/hid.h>
 #include <linux/kfifo.h>
+#include <linux/leds.h>
 #include <linux/usb/input.h>
 #include <linux/power_supply.h>
 #include <asm/unaligned.h>
@@ -112,8 +113,23 @@ enum wacom_worker {
 	WACOM_WORKER_REMOTE,
 };
 
+struct wacom;
+
+struct wacom_led {
+	struct led_classdev cdev;
+	struct led_trigger trigger;
+	struct wacom *wacom;
+	unsigned int group;
+	unsigned int id;
+	u8 llv;
+	u8 hlv;
+	bool held;
+};
+
 struct wacom_group_leds {
 	u8 select; /* status led selector (0..3) */
+	struct wacom_led *leds;
+	unsigned int count;
 };
 
 struct wacom_battery {
@@ -154,9 +170,12 @@ struct wacom {
 	struct wacom_remote *remote;
 	struct wacom_leds {
 		struct wacom_group_leds *groups;
+		unsigned int count;
 		u8 llv;       /* status led brightness no button (1..127) */
 		u8 hlv;       /* status led brightness button pressed (1..127) */
 		u8 img_lum;   /* OLED matrix display brightness */
+		u8 max_llv;   /* maximum brightness of LED (llv) */
+		u8 max_hlv;   /* maximum brightness of LED (hlv) */
 	} led;
 	struct wacom_battery battery;
 	bool resources;
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 1d79215..2354d73 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -966,31 +966,193 @@ static int wacom_devm_sysfs_create_group(struct wacom *wacom,
 					       group);
 }
 
+static enum led_brightness wacom_leds_brightness_get(struct wacom_led *led)
+{
+	struct wacom *wacom = led->wacom;
+
+	if (wacom->led.max_hlv)
+		return led->hlv * LED_FULL / wacom->led.max_hlv;
+
+	if (wacom->led.max_llv)
+		return led->llv * LED_FULL / wacom->led.max_llv;
+
+	/* device doesn't support brightness tuning */
+	return LED_FULL;
+}
+
+static enum led_brightness __wacom_led_brightness_get(struct led_classdev *cdev)
+{
+	struct wacom_led *led = container_of(cdev, struct wacom_led, cdev);
+	struct wacom *wacom = led->wacom;
+
+	if (wacom->led.groups[led->group].select != led->id)
+		return LED_OFF;
+
+	return wacom_leds_brightness_get(led);
+}
+
+static int wacom_led_brightness_set(struct led_classdev *cdev,
+				    enum led_brightness brightness)
+{
+	struct wacom_led *led = container_of(cdev, struct wacom_led, cdev);
+	struct wacom *wacom = led->wacom;
+	int error;
+
+	mutex_lock(&wacom->lock);
+
+	if (!wacom->led.groups || (brightness == LED_OFF &&
+	    wacom->led.groups[led->group].select != led->id)) {
+		error = 0;
+		goto out;
+	}
+
+	led->llv = wacom->led.llv = wacom->led.max_llv * brightness / LED_FULL;
+	led->hlv = wacom->led.hlv = wacom->led.max_hlv * brightness / LED_FULL;
+
+	wacom->led.groups[led->group].select = led->id;
+
+	error = wacom_led_control(wacom);
+
+out:
+	mutex_unlock(&wacom->lock);
+
+	return error;
+}
+
+static void wacom_led_readonly_brightness_set(struct led_classdev *cdev,
+					       enum led_brightness brightness)
+{
+}
+
+static int wacom_led_register_one(struct device *dev, struct wacom *wacom,
+				  struct wacom_led *led, unsigned int group,
+				  unsigned int id, bool read_only)
+{
+	int error;
+	char *name;
+
+	name = devm_kasprintf(dev, GFP_KERNEL,
+			      "%s::wacom-led_%d.%d",
+			      dev_name(dev),
+			      group,
+			      id);
+	if (!name)
+		return -ENOMEM;
+
+	led->group = group;
+	led->id = id;
+	led->wacom = wacom;
+	led->llv = wacom->led.llv;
+	led->hlv = wacom->led.hlv;
+	led->cdev.name = name;
+	led->cdev.max_brightness = LED_FULL;
+	led->cdev.brightness_get = __wacom_led_brightness_get;
+	if (!read_only)
+		led->cdev.brightness_set_blocking = wacom_led_brightness_set;
+	else
+		led->cdev.brightness_set = wacom_led_readonly_brightness_set;
+
+	error = devm_led_classdev_register(dev, &led->cdev);
+	if (error) {
+		hid_err(wacom->hdev,
+			"failed to register LED %s: %d\n",
+			led->cdev.name, error);
+		led->cdev.name = NULL;
+		return error;
+	}
+
+	return 0;
+}
+
+static int wacom_led_groups_alloc_and_register_one(struct device *dev,
+						   struct wacom *wacom,
+						   int group_id, int count,
+						   bool read_only)
+{
+	struct wacom_led *leds;
+	int i, error;
+
+	if (group_id >= wacom->led.count || count <= 0)
+		return -EINVAL;
+
+	if (!devres_open_group(dev, &wacom->led.groups[group_id], GFP_KERNEL))
+		return -ENOMEM;
+
+	leds = devm_kzalloc(dev, sizeof(struct wacom_led) * count, GFP_KERNEL);
+	if (!leds) {
+		error = -ENOMEM;
+		goto err;
+	}
+
+	wacom->led.groups[group_id].leds = leds;
+	wacom->led.groups[group_id].count = count;
+
+	for (i = 0; i < count; i++) {
+		error = wacom_led_register_one(dev, wacom, &leds[i],
+					       group_id, i, read_only);
+		if (error)
+			goto err;
+	}
+
+	devres_remove_group(dev, &wacom->led.groups[group_id]);
+	return 0;
+
+err:
+	devres_release_group(dev, &wacom->led.groups[group_id]);
+	return error;
+}
+
 static void wacom_led_groups_release(void *data)
 {
 	struct wacom *wacom = data;
 
 	wacom->led.groups = NULL;
+	wacom->led.count = 0;
 }
 
 static int wacom_led_groups_allocate(struct wacom *wacom, int count)
 {
+	struct device *dev = &wacom->hdev->dev;
 	struct wacom_group_leds *groups;
 	int error;
 
-	groups = devm_kzalloc(&wacom->hdev->dev,
-			      sizeof(struct wacom_group_leds) * count,
+	groups = devm_kzalloc(dev, sizeof(struct wacom_group_leds) * count,
 			      GFP_KERNEL);
 	if (!groups)
 		return -ENOMEM;
 
-	error = devm_add_action_or_reset(&wacom->hdev->dev,
-					 wacom_led_groups_release,
-					 wacom);
+	error = devm_add_action_or_reset(dev, wacom_led_groups_release, wacom);
 	if (error)
 		return error;
 
 	wacom->led.groups = groups;
+	wacom->led.count = count;
+
+	return 0;
+}
+
+static int wacom_leds_alloc_and_register(struct wacom *wacom, int group_count,
+					 int led_per_group, bool read_only)
+{
+	struct device *dev;
+	int i, error;
+
+	if (!wacom->wacom_wac.pad_input)
+		return -EINVAL;
+
+	dev = &wacom->wacom_wac.pad_input->dev;
+
+	error = wacom_led_groups_allocate(wacom, group_count);
+	if (error)
+		return error;
+
+	for (i = 0; i < group_count; i++) {
+		error = wacom_led_groups_alloc_and_register_one(dev, wacom, i,
+								led_per_group,
+								read_only);
+		if (error)
+			return error;
+	}
 
 	return 0;
 }
@@ -1010,9 +1172,11 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOS4L:
 		wacom->led.llv = 10;
 		wacom->led.hlv = 20;
+		wacom->led.max_llv = 127;
+		wacom->led.max_hlv = 127;
 		wacom->led.img_lum = 10;
 
-		error = wacom_led_groups_allocate(wacom, 1);
+		error = wacom_leds_alloc_and_register(wacom, 1, 4, false);
 		if (error) {
 			hid_err(wacom->hdev,
 				"cannot create leds err: %d\n", error);
@@ -1029,7 +1193,7 @@ static int wacom_initialize_leds(struct wacom *wacom)
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
-		error = wacom_led_groups_allocate(wacom, 2);
+		error = wacom_leds_alloc_and_register(wacom, 2, 4, false);
 		if (error) {
 			hid_err(wacom->hdev,
 				"cannot create leds err: %d\n", error);
@@ -1047,10 +1211,9 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOSPM:
 	case INTUOSPL:
 		wacom->led.llv = 32;
-		wacom->led.hlv = 0;
-		wacom->led.img_lum = 0;
+		wacom->led.max_llv = 96;
 
-		error = wacom_led_groups_allocate(wacom, 1);
+		error = wacom_leds_alloc_and_register(wacom, 1, 4, false);
 		if (error) {
 			hid_err(wacom->hdev,
 				"cannot create leds err: %d\n", error);
@@ -1062,6 +1225,8 @@ static int wacom_initialize_leds(struct wacom *wacom)
 		break;
 
 	case REMOTE:
+		wacom->led.llv = 255;
+		wacom->led.max_llv = 255;
 		error = wacom_led_groups_allocate(wacom, 5);
 		if (error) {
 			hid_err(wacom->hdev,
@@ -1987,6 +2152,12 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 	if (error)
 		goto fail;
 
+	error = wacom_led_groups_alloc_and_register_one(
+					&remote->remotes[index].input->dev,
+					wacom, index, 3, true);
+	if (error)
+		goto fail;
+
 	remote->remotes[index].registered = true;
 
 	devres_close_group(dev, &remote->remotes[index]);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437069 — [PATCH 16/27] HID: wacom: rework fail path in probe() and parse_and_register()

Thanks to devres management, we don't need to remember a lot of failure
path. One or two is enough.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 37 ++++++++++++++-----------------------
 1 file changed, 14 insertions(+), 23 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 64acb71..241acee 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1713,7 +1713,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 
 	error = wacom_allocate_inputs(wacom);
 	if (error)
-		goto fail_open_group;
+		goto fail;
 
 	/*
 	 * Bamboo Pad has a generic hid handling for the Pen, and we switch it
@@ -1726,7 +1726,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		} else if ((features->pktlen != WACOM_PKGLEN_BPAD_TOUCH) &&
 			   (features->pktlen != WACOM_PKGLEN_BPAD_TOUCH_USB)) {
 			error = -ENODEV;
-			goto fail_allocate_inputs;
+			goto fail;
 		}
 	}
 
@@ -1746,7 +1746,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 			 error ? "Ignoring" : "Assuming pen");
 
 		if (error)
-			goto fail_parsed;
+			goto fail;
 
 		features->device_type |= WACOM_DEVICETYPE_PEN;
 	}
@@ -1757,27 +1757,27 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 
 	error = wacom_add_shared_data(hdev);
 	if (error)
-		goto fail_shared_data;
+		goto fail;
 
 	if (!(features->device_type & WACOM_DEVICETYPE_WL_MONITOR) &&
 	     (features->quirks & WACOM_QUIRK_BATTERY)) {
 		error = wacom_initialize_battery(wacom);
 		if (error)
-			goto fail_battery;
+			goto fail;
 	}
 
 	error = wacom_register_inputs(wacom);
 	if (error)
-		goto fail_register_inputs;
+		goto fail;
 
 	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
 		error = wacom_initialize_leds(wacom);
 		if (error)
-			goto fail_leds;
+			goto fail;
 
 		error = wacom_initialize_remotes(wacom);
 		if (error)
-			goto fail_remote;
+			goto fail;
 	}
 
 	if (features->type == HID_GENERIC)
@@ -1787,7 +1787,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 	error = hid_hw_start(hdev, connect_mask);
 	if (error) {
 		hid_err(hdev, "hw start failed\n");
-		goto fail_hw_start;
+		goto fail;
 	}
 
 	if (!wireless) {
@@ -1826,15 +1826,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 
 fail_quirks:
 	hid_hw_stop(hdev);
-fail_hw_start:
-fail_remote:
-fail_leds:
-fail_register_inputs:
-fail_battery:
-fail_shared_data:
-fail_parsed:
-fail_allocate_inputs:
-fail_open_group:
+fail:
 	wacom_release_resources(wacom);
 	return error;
 }
@@ -2014,7 +2006,7 @@ static int wacom_probe(struct hid_device *hdev,
 
 	if (features->check_for_hid_type && features->hid_type != hdev->type) {
 		error = -ENODEV;
-		goto fail_type;
+		goto fail;
 	}
 
 	wacom_wac->hid_data.inputmode = -1;
@@ -2031,12 +2023,12 @@ static int wacom_probe(struct hid_device *hdev,
 	error = hid_parse(hdev);
 	if (error) {
 		hid_err(hdev, "parse failed\n");
-		goto fail_parse;
+		goto fail;
 	}
 
 	error = wacom_parse_and_register(wacom, false);
 	if (error)
-		goto fail_parse;
+		goto fail;
 
 	if (hdev->bus == BUS_BLUETOOTH) {
 		error = device_create_file(&hdev->dev, &dev_attr_speed);
@@ -2048,8 +2040,7 @@ static int wacom_probe(struct hid_device *hdev,
 
 	return 0;
 
-fail_parse:
-fail_type:
+fail:
 	hid_set_drvdata(hdev, NULL);
 	return error;
 }
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437070 — [PATCH 17/27] HID: wacom: EKR: have proper allocator and destructor

The wacom_remote_create_attr_group() and wacom_remote_destroy_attr_group()
functions were both allocating/destroying the sysfs groups but also
initializing the parameters for the remotes. Have proper functions
that can be called and extended.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 93 ++++++++++++++++++++++++++++---------------------
 1 file changed, 53 insertions(+), 40 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 241acee..27ca147 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1300,8 +1300,6 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 	int error = 0;
 	struct wacom_remote *remote = wacom->remote;
 
-	remote->serial[index] = serial;
-
 	remote->remote_group[index].name = devm_kasprintf(&wacom->hdev->dev,
 							  GFP_KERNEL,
 							  "%d", serial);
@@ -1319,27 +1317,13 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 	return 0;
 }
 
-static void wacom_remote_destroy_attr_group(struct wacom *wacom, __u32 serial)
+static void wacom_remote_destroy_attr_group(struct wacom *wacom, unsigned int i)
 {
 	struct wacom_remote *remote = wacom->remote;
-	int i;
 
-	if (!serial)
-		return;
-
-	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		if (remote->serial[i] == serial) {
-			remote->serial[i] = 0;
-			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
-			if (remote->remote_group[i].name) {
-				sysfs_remove_group(remote->remote_dir,
-						   &remote->remote_group[i]);
-				devm_kfree(&wacom->hdev->dev,
-					  (char *)remote->remote_group[i].name);
-				remote->remote_group[i].name = NULL;
-			}
-		}
-	}
+	sysfs_remove_group(remote->remote_dir, &remote->remote_group[i]);
+	devm_kfree(&wacom->hdev->dev, (char *)remote->remote_group[i].name);
+	remote->remote_group[i].name = NULL;
 }
 
 static int wacom_cmd_unpair_remote(struct wacom *wacom, unsigned char selector)
@@ -1916,6 +1900,50 @@ fail:
 	return;
 }
 
+static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
+{
+	struct wacom_remote *remote = wacom->remote;
+	u32 serial = remote->serial[index];
+	int i;
+
+	wacom_remote_destroy_attr_group(wacom, index);
+
+	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
+		if (remote->serial[i] == serial) {
+			remote->serial[i] = 0;
+			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
+		}
+	}
+}
+
+static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
+				   unsigned int index)
+{
+	struct wacom_remote *remote = wacom->remote;
+	int error, k;
+
+	/* A remote can pair more than once with an EKR,
+	 * check to make sure this serial isn't already paired.
+	 */
+	for (k = 0; k < WACOM_MAX_REMOTES; k++) {
+		if (remote->serial[k] == serial)
+			break;
+	}
+
+	if (k < WACOM_MAX_REMOTES) {
+		remote->serial[index] = serial;
+		return 0;
+	}
+
+	error = wacom_remote_create_attr_group(wacom, serial, index);
+	if (error)
+		return error;
+
+	remote->serial[index] = serial;
+
+	return 0;
+}
+
 static void wacom_remote_work(struct work_struct *work)
 {
 	struct wacom *wacom = container_of(work, struct wacom, remote_work);
@@ -1924,7 +1952,7 @@ static void wacom_remote_work(struct work_struct *work)
 	unsigned long flags;
 	unsigned int count;
 	u32 serial;
-	int i, k;
+	int i;
 
 	spin_lock_irqsave(&remote->remote_lock, flags);
 
@@ -1949,28 +1977,13 @@ static void wacom_remote_work(struct work_struct *work)
 			if (remote->serial[i] == serial)
 				continue;
 
-			if (remote->serial[i]) {
-				wacom_remote_destroy_attr_group(wacom,
-							remote->serial[i]);
-			}
-
-			/* A remote can pair more than once with an EKR,
-			 * check to make sure this serial isn't already paired.
-			 */
-			for (k = 0; k < WACOM_MAX_REMOTES; k++) {
-				if (remote->serial[k] == serial)
-					break;
-			}
+			if (remote->serial[i])
+				wacom_remote_destroy_one(wacom, i);
 
-			if (k < WACOM_MAX_REMOTES) {
-				remote->serial[i] = serial;
-				continue;
-			}
-			wacom_remote_create_attr_group(wacom, serial, i);
+			wacom_remote_create_one(wacom, serial, i);
 
 		} else if (remote->serial[i]) {
-			wacom_remote_destroy_attr_group(wacom,
-							remote->serial[i]);
+			wacom_remote_destroy_one(wacom, i);
 		}
 	}
 }
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437071 — [PATCH 11/27] HID: wacom: use devres to allocate driver data

We started switching the driver to devres, so we should use it as much
as possible.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index ec088c1..56d62e8 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1844,7 +1844,7 @@ static int wacom_probe(struct hid_device *hdev,
 	/* hid-core sets this quirk for the boot interface */
 	hdev->quirks &= ~HID_QUIRK_NOGET;
 
-	wacom = kzalloc(sizeof(struct wacom), GFP_KERNEL);
+	wacom = devm_kzalloc(&hdev->dev, sizeof(struct wacom), GFP_KERNEL);
 	if (!wacom)
 		return -ENOMEM;
 
@@ -1892,7 +1892,6 @@ static int wacom_probe(struct hid_device *hdev,
 
 fail_type:
 fail_parse:
-	kfree(wacom);
 	hid_set_drvdata(hdev, NULL);
 	return error;
 }
@@ -1916,7 +1915,6 @@ static void wacom_remove(struct hid_device *hdev)
 	wacom_remove_shared_data(wacom);
 
 	hid_set_drvdata(hdev, NULL);
-	kfree(wacom);
 }
 
 #ifdef CONFIG_PM
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437072 — [PATCH 07/27] HID: wacom: switch inputs to devres

Simplifying the error code paths.
We need to keep wacom_clean_inputs() around for now as the wireless
module is using it to dynamically remove the inputs on disconnect.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 34 ++++++++++++++++------------------
 1 file changed, 16 insertions(+), 18 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 61dcb87e..da2fccf 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -91,7 +91,12 @@ static void wacom_close(struct input_dev *dev)
 {
 	struct wacom *wacom = input_get_drvdata(dev);
 
-	hid_hw_close(wacom->hdev);
+	/*
+	 * wacom->hdev should never be null, but surprisingly, I had the case
+	 * once while unplugging the Wacom Wireless Receiver.
+	 */
+	if (wacom->hdev)
+		hid_hw_close(wacom->hdev);
 }
 
 /*
@@ -1370,7 +1375,7 @@ static struct input_dev *wacom_allocate_input(struct wacom *wacom)
 	struct hid_device *hdev = wacom->hdev;
 	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
 
-	input_dev = input_allocate_device();
+	input_dev = devm_input_allocate_device(&hdev->dev);
 	if (!input_dev)
 		return NULL;
 
@@ -1424,10 +1429,10 @@ static int wacom_allocate_inputs(struct wacom *wacom)
 	wacom_wac->pen_input = wacom_allocate_input(wacom);
 	wacom_wac->touch_input = wacom_allocate_input(wacom);
 	wacom_wac->pad_input = wacom_allocate_input(wacom);
-	if (!wacom_wac->pen_input || !wacom_wac->touch_input || !wacom_wac->pad_input) {
-		wacom_clean_inputs(wacom);
+	if (!wacom_wac->pen_input ||
+	    !wacom_wac->touch_input ||
+	    !wacom_wac->pad_input)
 		return -ENOMEM;
-	}
 
 	wacom_wac->pen_input->name = wacom_wac->pen_name;
 	wacom_wac->touch_input->name = wacom_wac->touch_name;
@@ -1458,7 +1463,7 @@ static int wacom_register_inputs(struct wacom *wacom)
 	} else {
 		error = input_register_device(pen_input_dev);
 		if (error)
-			goto fail_register_pen_input;
+			goto fail;
 		wacom_wac->pen_registered = true;
 	}
 
@@ -1471,7 +1476,7 @@ static int wacom_register_inputs(struct wacom *wacom)
 	} else {
 		error = input_register_device(touch_input_dev);
 		if (error)
-			goto fail_register_touch_input;
+			goto fail;
 		wacom_wac->touch_registered = true;
 	}
 
@@ -1484,23 +1489,19 @@ static int wacom_register_inputs(struct wacom *wacom)
 	} else {
 		error = input_register_device(pad_input_dev);
 		if (error)
-			goto fail_register_pad_input;
+			goto fail;
 		wacom_wac->pad_registered = true;
 	}
 
 	return 0;
 
-fail_register_pad_input:
-	if (touch_input_dev)
-		input_unregister_device(touch_input_dev);
+fail:
+	wacom_wac->pad_input = NULL;
+	wacom_wac->pad_registered = false;
 	wacom_wac->touch_input = NULL;
 	wacom_wac->touch_registered = false;
-fail_register_touch_input:
-	if (pen_input_dev)
-		input_unregister_device(pen_input_dev);
 	wacom_wac->pen_input = NULL;
 	wacom_wac->pen_registered = false;
-fail_register_pen_input:
 	return error;
 }
 
@@ -1741,14 +1742,12 @@ fail_hw_start:
 fail_remote:
 	wacom_destroy_leds(wacom);
 fail_leds:
-	wacom_clean_inputs(wacom);
 fail_register_inputs:
 fail_battery:
 	wacom_remove_shared_data(wacom);
 fail_shared_data:
 fail_parsed:
 fail_allocate_inputs:
-	wacom_clean_inputs(wacom);
 	return error;
 }
 
@@ -1928,7 +1927,6 @@ static void wacom_remove(struct hid_device *hdev)
 	cancel_work_sync(&wacom->battery_work);
 	kobject_put(wacom->remote_dir);
 	wacom_destroy_leds(wacom);
-	wacom_clean_inputs(wacom);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
 	wacom_remove_shared_data(wacom);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437073 — [PATCH 25/27] HID: wacom: leds: fix ordering of LED banks

Historically, 21UX2 and 24HD have the select groups inverted
(0 is the right LED bank, and 1 the left one).

This is not right, so fix that in the new LED API. For backward
compatibility, we keep the wacom_led sysfs ABI stable. We don't
need to care about luminance for these two devices as only the
select sysfs file gets exported (brightness is not configurable).

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index b88896c..153e453 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -683,8 +683,10 @@ static int wacom_led_control(struct wacom *wacom)
 		int led = wacom->led.groups[0].select | 0x4;
 
 		if (wacom->wacom_wac.features.type == WACOM_21UX2 ||
-		    wacom->wacom_wac.features.type == WACOM_24HD)
-			led |= (wacom->led.groups[1].select << 4) | 0x40;
+		    wacom->wacom_wac.features.type == WACOM_24HD) {
+			led <<= 4;
+			led |= wacom->led.groups[1].select | 0x04;
+		}
 
 		buf[0] = report_id;
 		buf[1] = led;
@@ -742,6 +744,19 @@ out:
 	return retval;
 }
 
+static inline int wacom_led_select_get_id(struct wacom *wacom, int set_id)
+{
+	/*
+	 * Historically, 21UX2 and 24HD have the select groups inverted
+	 * (0 is the right LED bank, and 1 the left one)
+	 */
+	if (wacom->wacom_wac.features.type == WACOM_21UX2 ||
+	    wacom->wacom_wac.features.type == WACOM_24HD)
+		return 1 - set_id;
+
+	return set_id;
+}
+
 static ssize_t wacom_led_select_store(struct device *dev, int set_id,
 				      const char *buf, size_t count)
 {
@@ -754,6 +769,8 @@ static ssize_t wacom_led_select_store(struct device *dev, int set_id,
 	if (err)
 		return err;
 
+	set_id = wacom_led_select_get_id(wacom, set_id);
+
 	mutex_lock(&wacom->lock);
 
 	wacom->led.groups[set_id].select = id & 0x3;
@@ -775,8 +792,9 @@ static ssize_t wacom_led##SET_ID##_select_show(struct device *dev,	\
 {									\
 	struct hid_device *hdev = to_hid_device(dev);\
 	struct wacom *wacom = hid_get_drvdata(hdev);			\
+	int set_id = wacom_led_select_get_id(wacom, SET_ID);		\
 	return scnprintf(buf, PAGE_SIZE, "%d\n",			\
-			 wacom->led.groups[SET_ID].select);		\
+			 wacom->led.groups[set_id].select);		\
 }									\
 static DEVICE_ATTR(status_led##SET_ID##_select, DEV_ATTR_RW_PERM,	\
 		    wacom_led##SET_ID##_select_show,			\
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437074 — [PATCH 08/27] HID: wacom: put the managed resources in a group

We currently have a complex clean_inputs() function while this can be
handled all by devres. Set a group that we can destroy in wireless_work().

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom.h     |  1 +
 drivers/hid/wacom_sys.c | 69 +++++++++++++++++++++----------------------------
 drivers/hid/wacom_wac.c |  2 +-
 drivers/hid/wacom_wac.h |  3 ---
 4 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index a968fbb..6f5555f 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -131,6 +131,7 @@ struct wacom {
 	struct power_supply_desc ac_desc;
 	struct kobject *remote_dir;
 	struct attribute_group remote_group[5];
+	bool resources;
 };
 
 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 da2fccf..08f6d1f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1394,34 +1394,6 @@ static struct input_dev *wacom_allocate_input(struct wacom *wacom)
 	return input_dev;
 }
 
-static void wacom_clean_inputs(struct wacom *wacom)
-{
-	if (wacom->wacom_wac.pen_input) {
-		if (wacom->wacom_wac.pen_registered)
-			input_unregister_device(wacom->wacom_wac.pen_input);
-		else
-			input_free_device(wacom->wacom_wac.pen_input);
-	}
-	if (wacom->wacom_wac.touch_input) {
-		if (wacom->wacom_wac.touch_registered)
-			input_unregister_device(wacom->wacom_wac.touch_input);
-		else
-			input_free_device(wacom->wacom_wac.touch_input);
-	}
-	if (wacom->wacom_wac.pad_input) {
-		if (wacom->wacom_wac.pad_registered)
-			input_unregister_device(wacom->wacom_wac.pad_input);
-		else
-			input_free_device(wacom->wacom_wac.pad_input);
-	}
-	wacom->wacom_wac.pen_input = NULL;
-	wacom->wacom_wac.touch_input = NULL;
-	wacom->wacom_wac.pad_input = NULL;
-	wacom->wacom_wac.pen_registered = false;
-	wacom->wacom_wac.touch_registered = false;
-	wacom->wacom_wac.pad_registered = false;
-}
-
 static int wacom_allocate_inputs(struct wacom *wacom)
 {
 	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
@@ -1464,7 +1436,6 @@ static int wacom_register_inputs(struct wacom *wacom)
 		error = input_register_device(pen_input_dev);
 		if (error)
 			goto fail;
-		wacom_wac->pen_registered = true;
 	}
 
 	error = wacom_setup_touch_input_capabilities(touch_input_dev, wacom_wac);
@@ -1477,7 +1448,6 @@ static int wacom_register_inputs(struct wacom *wacom)
 		error = input_register_device(touch_input_dev);
 		if (error)
 			goto fail;
-		wacom_wac->touch_registered = true;
 	}
 
 	error = wacom_setup_pad_input_capabilities(pad_input_dev, wacom_wac);
@@ -1490,18 +1460,14 @@ static int wacom_register_inputs(struct wacom *wacom)
 		error = input_register_device(pad_input_dev);
 		if (error)
 			goto fail;
-		wacom_wac->pad_registered = true;
 	}
 
 	return 0;
 
 fail:
 	wacom_wac->pad_input = NULL;
-	wacom_wac->pad_registered = false;
 	wacom_wac->touch_input = NULL;
-	wacom_wac->touch_registered = false;
 	wacom_wac->pen_input = NULL;
-	wacom_wac->pen_registered = false;
 	return error;
 }
 
@@ -1612,6 +1578,22 @@ static void wacom_update_name(struct wacom *wacom, const char *suffix)
 		"%s%s Pad", name, suffix);
 }
 
+static void wacom_release_resources(struct wacom *wacom)
+{
+	struct hid_device *hdev = wacom->hdev;
+
+	if (!wacom->resources)
+		return;
+
+	devres_release_group(&hdev->dev, wacom);
+
+	wacom->resources = false;
+
+	wacom->wacom_wac.pen_input = NULL;
+	wacom->wacom_wac.touch_input = NULL;
+	wacom->wacom_wac.pad_input = NULL;
+}
+
 static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 {
 	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
@@ -1624,9 +1606,14 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 	if (features->pktlen > WACOM_PKGLEN_MAX)
 		return -EINVAL;
 
+	if (!devres_open_group(&hdev->dev, wacom, GFP_KERNEL))
+		return -ENOMEM;
+
+	wacom->resources = true;
+
 	error = wacom_allocate_inputs(wacom);
 	if (error)
-		return error;
+		goto fail_open_group;
 
 	/*
 	 * Bamboo Pad has a generic hid handling for the Pen, and we switch it
@@ -1733,6 +1720,8 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		wacom_wac->shared->touch_input = wacom_wac->touch_input;
 	}
 
+	devres_close_group(&hdev->dev, wacom);
+
 	return 0;
 
 fail_quirks:
@@ -1748,6 +1737,8 @@ fail_battery:
 fail_shared_data:
 fail_parsed:
 fail_allocate_inputs:
+fail_open_group:
+	wacom_release_resources(wacom);
 	return error;
 }
 
@@ -1773,14 +1764,14 @@ static void wacom_wireless_work(struct work_struct *work)
 	wacom1 = hid_get_drvdata(hdev1);
 	wacom_wac1 = &(wacom1->wacom_wac);
 	wacom_destroy_leds(wacom1);
-	wacom_clean_inputs(wacom1);
+	wacom_release_resources(wacom1);
 
 	/* Touch interface */
 	hdev2 = usb_get_intfdata(usbdev->config->interface[2]);
 	wacom2 = hid_get_drvdata(hdev2);
 	wacom_wac2 = &(wacom2->wacom_wac);
 	wacom_destroy_leds(wacom2);
-	wacom_clean_inputs(wacom2);
+	wacom_release_resources(wacom2);
 
 	if (wacom_wac->pid == 0) {
 		hid_info(wacom->hdev, "wireless tablet disconnected\n");
@@ -1835,9 +1826,9 @@ static void wacom_wireless_work(struct work_struct *work)
 
 fail:
 	wacom_destroy_leds(wacom1);
-	wacom_clean_inputs(wacom1);
+	wacom_release_resources(wacom1);
 	wacom_destroy_leds(wacom2);
-	wacom_clean_inputs(wacom2);
+	wacom_release_resources(wacom2);
 	return;
 }
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index c7707b2..3e2c903 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1941,7 +1941,7 @@ static int wacom_bpt3_touch(struct wacom_wac *wacom)
 	}
 
 	/* only update touch if we actually have a touchpad and touch data changed */
-	if (wacom->touch_registered && touch_changed) {
+	if (wacom->touch_input && touch_changed) {
 		input_mt_sync_frame(wacom->touch_input);
 		wacom->shared->touch_down = wacom_wac_finger_count_touches(wacom);
 	}
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 53d1653..d5b973d 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -234,9 +234,6 @@ struct wacom_wac {
 	struct input_dev *pen_input;
 	struct input_dev *touch_input;
 	struct input_dev *pad_input;
-	bool pen_registered;
-	bool touch_registered;
-	bool pad_registered;
 	int pid;
 	int battery_capacity;
 	int num_contacts_left;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437075 — [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs directly in the kernel

The EKR switches the LED directly, and there is no point in having
userspace handling the switch it self when it's easy enough to do
in the kernel.

The other benefit is that now userspace does not need to have root access
to the LED but need only to read them with user privileges.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom.h     |  4 ++++
 drivers/hid/wacom_sys.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/hid/wacom_wac.c | 53 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 9159dd3..8f8a162 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -216,4 +216,8 @@ int wacom_wac_event(struct hid_device *hdev, struct hid_field *field,
 		struct hid_usage *usage, __s32 value);
 void wacom_wac_report(struct hid_device *hdev, struct hid_report *report);
 void wacom_battery_work(struct work_struct *work);
+enum led_brightness wacom_leds_brightness_get(struct wacom_led *led);
+struct wacom_led *wacom_led_find(struct wacom *wacom, unsigned int group,
+				 unsigned int id);
+struct wacom_led *wacom_led_next(struct wacom *wacom, struct wacom_led *cur);
 #endif
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 153e453..4a0bb6f 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -984,7 +984,7 @@ static int wacom_devm_sysfs_create_group(struct wacom *wacom,
 					       group);
 }
 
-static enum led_brightness wacom_leds_brightness_get(struct wacom_led *led)
+enum led_brightness wacom_leds_brightness_get(struct wacom_led *led)
 {
 	struct wacom *wacom = led->wacom;
 
@@ -1057,6 +1057,17 @@ static int wacom_led_register_one(struct device *dev, struct wacom *wacom,
 	if (!name)
 		return -ENOMEM;
 
+	if (!read_only) {
+		led->trigger.name = name;
+		error = devm_led_trigger_register(dev, &led->trigger);
+		if (error) {
+			hid_err(wacom->hdev,
+				"failed to register LED trigger %s: %d\n",
+				led->cdev.name, error);
+			return error;
+		}
+	}
+
 	led->group = group;
 	led->id = id;
 	led->wacom = wacom;
@@ -1065,10 +1076,12 @@ static int wacom_led_register_one(struct device *dev, struct wacom *wacom,
 	led->cdev.name = name;
 	led->cdev.max_brightness = LED_FULL;
 	led->cdev.brightness_get = __wacom_led_brightness_get;
-	if (!read_only)
+	if (!read_only) {
 		led->cdev.brightness_set_blocking = wacom_led_brightness_set;
-	else
+		led->cdev.default_trigger = led->cdev.name;
+	} else {
 		led->cdev.brightness_set = wacom_led_readonly_brightness_set;
+	}
 
 	error = devm_led_classdev_register(dev, &led->cdev);
 	if (error) {
@@ -1145,6 +1158,50 @@ err:
 	return error;
 }
 
+struct wacom_led *wacom_led_find(struct wacom *wacom, unsigned int group_id,
+				 unsigned int id)
+{
+	struct wacom_group_leds *group;
+
+	if (group_id >= wacom->led.count)
+		return NULL;
+
+	group = &wacom->led.groups[group_id];
+
+	if (!group->leds)
+		return NULL;
+
+	id %= group->count;
+
+	return &group->leds[id];
+}
+
+/**
+ * wacom_led_next: gives the next available led with a wacom trigger.
+ *
+ * returns the next available struct wacom_led which has its default trigger
+ * or the current one if none is available.
+ */
+struct wacom_led *wacom_led_next(struct wacom *wacom, struct wacom_led *cur)
+{
+	struct wacom_led *next_led;
+	int group, next;
+
+	if (!wacom || !cur)
+		return NULL;
+
+	group = cur->group;
+	next = cur->id;
+
+	do {
+		next_led = wacom_led_find(wacom, group, ++next);
+		if (!next_led || next_led == cur)
+			return next_led;
+	} while (next_led->cdev.trigger != &next_led->trigger);
+
+	return next_led;
+}
+
 static void wacom_led_groups_release(void *data)
 {
 	struct wacom *wacom = data;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 6904e9b..a2b9c09 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2764,11 +2764,64 @@ static void wacom_setup_numbered_buttons(struct input_dev *input_dev,
 		__set_bit(BTN_BASE + (i-16), input_dev->keybit);
 }
 
+static bool wacom_is_led_toggled(struct wacom *wacom, int button_count,
+				 int mask, int group)
+{
+	int button_per_group;
+
+	button_per_group = button_count/wacom->led.count;
+
+	return mask & (1 << (group * button_per_group));
+}
+
+static void wacom_update_led(struct wacom *wacom, int button_count, int mask,
+			     int group)
+{
+	struct wacom_led *led, *next_led;
+	int cur;
+	bool pressed;
+
+	pressed = wacom_is_led_toggled(wacom, button_count, mask, group);
+	cur = wacom->led.groups[group].select;
+
+	led = wacom_led_find(wacom, group, cur);
+	if (!led) {
+		hid_err(wacom->hdev, "can't find current LED %d in group %d\n",
+			cur, group);
+		return;
+	}
+
+	if (!pressed) {
+		led->held = false;
+		return;
+	}
+
+	if (led->held && pressed)
+		return;
+
+	next_led = wacom_led_next(wacom, led);
+	if (!next_led) {
+		hid_err(wacom->hdev, "can't find next LED in group %d\n",
+			group);
+		return;
+	}
+	if (next_led == led)
+		return;
+
+	next_led->held = true;
+	led_trigger_event(&next_led->trigger,
+			  wacom_leds_brightness_get(next_led));
+}
+
 static void wacom_report_numbered_buttons(struct input_dev *input_dev,
 				int button_count, int mask)
 {
+	struct wacom *wacom = input_get_drvdata(input_dev);
 	int i;
 
+	for (i = 0; i < wacom->led.count; i++)
+		wacom_update_led(wacom,  button_count, mask, i);
+
 	for (i = 0; i < button_count && i < 10; i++)
 		input_report_key(input_dev, BTN_0 + i, mask & (1 << i));
 	for (i = 10; i < button_count && i < 16; i++)
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437150 — Re: [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs directly in the kernel

[Multipart message — attachments visible in raw view] — view raw

Hi,

[auto build test ERROR on hid/for-next]
[also build test ERROR on v4.7-rc6 next-20160705]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/HID-wacom-cleanup-EKR-LED/20160705-225431
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
config: i386-randconfig-a0-201627 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/hid/wacom_sys.c: In function 'wacom_led_register_one':
>> drivers/hid/wacom_sys.c:1061:15: error: 'struct led_trigger' has no member named 'name'
      led->trigger.name = name;
                  ^
>> drivers/hid/wacom_sys.c:1062:11: error: implicit declaration of function 'devm_led_trigger_register' [-Werror=implicit-function-declaration]
      error = devm_led_trigger_register(dev, &led->trigger);
              ^~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/hid/wacom_sys.c: In function 'wacom_led_next':
>> drivers/hid/wacom_sys.c:1200:25: error: 'struct led_classdev' has no member named 'trigger'
     } while (next_led->cdev.trigger != &next_led->trigger);
                            ^
   cc1: some warnings being treated as errors

vim +1061 drivers/hid/wacom_sys.c

  1055				      group,
  1056				      id);
  1057		if (!name)
  1058			return -ENOMEM;
  1059	
  1060		if (!read_only) {
> 1061			led->trigger.name = name;
> 1062			error = devm_led_trigger_register(dev, &led->trigger);
  1063			if (error) {
  1064				hid_err(wacom->hdev,
  1065					"failed to register LED trigger %s: %d\n",
  1066					led->cdev.name, error);
  1067				return error;
  1068			}
  1069		}
  1070	
  1071		led->group = group;
  1072		led->id = id;
  1073		led->wacom = wacom;
  1074		led->llv = wacom->led.llv;
  1075		led->hlv = wacom->led.hlv;
  1076		led->cdev.name = name;
  1077		led->cdev.max_brightness = LED_FULL;
  1078		led->cdev.brightness_get = __wacom_led_brightness_get;
  1079		if (!read_only) {
  1080			led->cdev.brightness_set_blocking = wacom_led_brightness_set;
  1081			led->cdev.default_trigger = led->cdev.name;
  1082		} else {
  1083			led->cdev.brightness_set = wacom_led_readonly_brightness_set;
  1084		}
  1085	
  1086		error = devm_led_classdev_register(dev, &led->cdev);
  1087		if (error) {
  1088			hid_err(wacom->hdev,
  1089				"failed to register LED %s: %d\n",
  1090				led->cdev.name, error);
  1091			led->cdev.name = NULL;
  1092			return error;
  1093		}
  1094	
  1095		return 0;
  1096	}
  1097	
  1098	static void wacom_led_groups_release_one(void *data)
  1099	{
  1100		struct wacom_group_leds *group = data;
  1101	
  1102		devres_release_group(group->dev, group);
  1103	}
  1104	
  1105	static int wacom_led_groups_alloc_and_register_one(struct device *dev,
  1106							   struct wacom *wacom,
  1107							   int group_id, int count,
  1108							   bool read_only)
  1109	{
  1110		struct wacom_led *leds;
  1111		int i, error;
  1112	
  1113		if (group_id >= wacom->led.count || count <= 0)
  1114			return -EINVAL;
  1115	
  1116		if (!devres_open_group(dev, &wacom->led.groups[group_id], GFP_KERNEL))
  1117			return -ENOMEM;
  1118	
  1119		leds = devm_kzalloc(dev, sizeof(struct wacom_led) * count, GFP_KERNEL);
  1120		if (!leds) {
  1121			error = -ENOMEM;
  1122			goto err;
  1123		}
  1124	
  1125		wacom->led.groups[group_id].leds = leds;
  1126		wacom->led.groups[group_id].count = count;
  1127	
  1128		for (i = 0; i < count; i++) {
  1129			error = wacom_led_register_one(dev, wacom, &leds[i],
  1130						       group_id, i, read_only);
  1131			if (error)
  1132				goto err;
  1133		}
  1134	
  1135		wacom->led.groups[group_id].dev = dev;
  1136	
  1137		devres_close_group(dev, &wacom->led.groups[group_id]);
  1138	
  1139		/*
  1140		 * There is a bug (?) in devm_led_classdev_register() in which its
  1141		 * increments the refcount of the parent. If the parent is an input
  1142		 * device, that means the ref count never reaches 0 when
  1143		 * devm_input_device_release() gets called.
  1144		 * This means that the LEDs are still there after disconnect.
  1145		 * Manually force the release of the group so that the leds are released
  1146		 * once we are done using them.
  1147		 */
  1148		error = devm_add_action_or_reset(&wacom->hdev->dev,
  1149						 wacom_led_groups_release_one,
  1150						 &wacom->led.groups[group_id]);
  1151		if (error)
  1152			return error;
  1153	
  1154		return 0;
  1155	
  1156	err:
  1157		devres_release_group(dev, &wacom->led.groups[group_id]);
  1158		return error;
  1159	}
  1160	
  1161	struct wacom_led *wacom_led_find(struct wacom *wacom, unsigned int group_id,
  1162					 unsigned int id)
  1163	{
  1164		struct wacom_group_leds *group;
  1165	
  1166		if (group_id >= wacom->led.count)
  1167			return NULL;
  1168	
  1169		group = &wacom->led.groups[group_id];
  1170	
  1171		if (!group->leds)
  1172			return NULL;
  1173	
  1174		id %= group->count;
  1175	
  1176		return &group->leds[id];
  1177	}
  1178	
  1179	/**
  1180	 * wacom_led_next: gives the next available led with a wacom trigger.
  1181	 *
  1182	 * returns the next available struct wacom_led which has its default trigger
  1183	 * or the current one if none is available.
  1184	 */
  1185	struct wacom_led *wacom_led_next(struct wacom *wacom, struct wacom_led *cur)
  1186	{
  1187		struct wacom_led *next_led;
  1188		int group, next;
  1189	
  1190		if (!wacom || !cur)
  1191			return NULL;
  1192	
  1193		group = cur->group;
  1194		next = cur->id;
  1195	
  1196		do {
  1197			next_led = wacom_led_find(wacom, group, ++next);
  1198			if (!next_led || next_led == cur)
  1199				return next_led;
> 1200		} while (next_led->cdev.trigger != &next_led->trigger);
  1201	
  1202		return next_led;
  1203	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1437301 — Re: [PATCH 26/27] HID: wacom: leds: handle the switch of the LEDs directly in the kernel

Hi,

On Jul 05 2016 or thereabouts, kbuild test robot wrote:
> Hi,
> 
> [auto build test ERROR on hid/for-next]
> [also build test ERROR on v4.7-rc6 next-20160705]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Benjamin-Tissoires/HID-wacom-cleanup-EKR-LED/20160705-225431
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/jikos/hid.git for-next
> config: i386-randconfig-a0-201627 (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All errors (new ones prefixed by >>):
> 
>    drivers/hid/wacom_sys.c: In function 'wacom_led_register_one':
> >> drivers/hid/wacom_sys.c:1061:15: error: 'struct led_trigger' has no member named 'name'
>       led->trigger.name = name;

Thanks, looks like I forgot to add the Kconfig bits CONFIG_LEDS_TRIGGERS
here (and CONFIG_LEDS_CLASS in 23/27).

Cheers,
Benjamin

>                   ^
> >> drivers/hid/wacom_sys.c:1062:11: error: implicit declaration of function 'devm_led_trigger_register' [-Werror=implicit-function-declaration]
>       error = devm_led_trigger_register(dev, &led->trigger);
>               ^~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/hid/wacom_sys.c: In function 'wacom_led_next':
> >> drivers/hid/wacom_sys.c:1200:25: error: 'struct led_classdev' has no member named 'trigger'
>      } while (next_led->cdev.trigger != &next_led->trigger);
>                             ^
>    cc1: some warnings being treated as errors
> 
> vim +1061 drivers/hid/wacom_sys.c
> 
>   1055				      group,
>   1056				      id);
>   1057		if (!name)
>   1058			return -ENOMEM;
>   1059	
>   1060		if (!read_only) {
> > 1061			led->trigger.name = name;
> > 1062			error = devm_led_trigger_register(dev, &led->trigger);
>   1063			if (error) {
>   1064				hid_err(wacom->hdev,
>   1065					"failed to register LED trigger %s: %d\n",
>   1066					led->cdev.name, error);
>   1067				return error;
>   1068			}
>   1069		}
>   1070	
>   1071		led->group = group;
>   1072		led->id = id;
>   1073		led->wacom = wacom;
>   1074		led->llv = wacom->led.llv;
>   1075		led->hlv = wacom->led.hlv;
>   1076		led->cdev.name = name;
>   1077		led->cdev.max_brightness = LED_FULL;
>   1078		led->cdev.brightness_get = __wacom_led_brightness_get;
>   1079		if (!read_only) {
>   1080			led->cdev.brightness_set_blocking = wacom_led_brightness_set;
>   1081			led->cdev.default_trigger = led->cdev.name;
>   1082		} else {
>   1083			led->cdev.brightness_set = wacom_led_readonly_brightness_set;
>   1084		}
>   1085	
>   1086		error = devm_led_classdev_register(dev, &led->cdev);
>   1087		if (error) {
>   1088			hid_err(wacom->hdev,
>   1089				"failed to register LED %s: %d\n",
>   1090				led->cdev.name, error);
>   1091			led->cdev.name = NULL;
>   1092			return error;
>   1093		}
>   1094	
>   1095		return 0;
>   1096	}
>   1097	
>   1098	static void wacom_led_groups_release_one(void *data)
>   1099	{
>   1100		struct wacom_group_leds *group = data;
>   1101	
>   1102		devres_release_group(group->dev, group);
>   1103	}
>   1104	
>   1105	static int wacom_led_groups_alloc_and_register_one(struct device *dev,
>   1106							   struct wacom *wacom,
>   1107							   int group_id, int count,
>   1108							   bool read_only)
>   1109	{
>   1110		struct wacom_led *leds;
>   1111		int i, error;
>   1112	
>   1113		if (group_id >= wacom->led.count || count <= 0)
>   1114			return -EINVAL;
>   1115	
>   1116		if (!devres_open_group(dev, &wacom->led.groups[group_id], GFP_KERNEL))
>   1117			return -ENOMEM;
>   1118	
>   1119		leds = devm_kzalloc(dev, sizeof(struct wacom_led) * count, GFP_KERNEL);
>   1120		if (!leds) {
>   1121			error = -ENOMEM;
>   1122			goto err;
>   1123		}
>   1124	
>   1125		wacom->led.groups[group_id].leds = leds;
>   1126		wacom->led.groups[group_id].count = count;
>   1127	
>   1128		for (i = 0; i < count; i++) {
>   1129			error = wacom_led_register_one(dev, wacom, &leds[i],
>   1130						       group_id, i, read_only);
>   1131			if (error)
>   1132				goto err;
>   1133		}
>   1134	
>   1135		wacom->led.groups[group_id].dev = dev;
>   1136	
>   1137		devres_close_group(dev, &wacom->led.groups[group_id]);
>   1138	
>   1139		/*
>   1140		 * There is a bug (?) in devm_led_classdev_register() in which its
>   1141		 * increments the refcount of the parent. If the parent is an input
>   1142		 * device, that means the ref count never reaches 0 when
>   1143		 * devm_input_device_release() gets called.
>   1144		 * This means that the LEDs are still there after disconnect.
>   1145		 * Manually force the release of the group so that the leds are released
>   1146		 * once we are done using them.
>   1147		 */
>   1148		error = devm_add_action_or_reset(&wacom->hdev->dev,
>   1149						 wacom_led_groups_release_one,
>   1150						 &wacom->led.groups[group_id]);
>   1151		if (error)
>   1152			return error;
>   1153	
>   1154		return 0;
>   1155	
>   1156	err:
>   1157		devres_release_group(dev, &wacom->led.groups[group_id]);
>   1158		return error;
>   1159	}
>   1160	
>   1161	struct wacom_led *wacom_led_find(struct wacom *wacom, unsigned int group_id,
>   1162					 unsigned int id)
>   1163	{
>   1164		struct wacom_group_leds *group;
>   1165	
>   1166		if (group_id >= wacom->led.count)
>   1167			return NULL;
>   1168	
>   1169		group = &wacom->led.groups[group_id];
>   1170	
>   1171		if (!group->leds)
>   1172			return NULL;
>   1173	
>   1174		id %= group->count;
>   1175	
>   1176		return &group->leds[id];
>   1177	}
>   1178	
>   1179	/**
>   1180	 * wacom_led_next: gives the next available led with a wacom trigger.
>   1181	 *
>   1182	 * returns the next available struct wacom_led which has its default trigger
>   1183	 * or the current one if none is available.
>   1184	 */
>   1185	struct wacom_led *wacom_led_next(struct wacom *wacom, struct wacom_led *cur)
>   1186	{
>   1187		struct wacom_led *next_led;
>   1188		int group, next;
>   1189	
>   1190		if (!wacom || !cur)
>   1191			return NULL;
>   1192	
>   1193		group = cur->group;
>   1194		next = cur->id;
>   1195	
>   1196		do {
>   1197			next_led = wacom_led_find(wacom, group, ++next);
>   1198			if (!next_led || next_led == cur)
>   1199				return next_led;
> > 1200		} while (next_led->cdev.trigger != &next_led->trigger);
>   1201	
>   1202		return next_led;
>   1203	}
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[toc] | [prev] | [next] | [standalone]

#1437076 — [PATCH 02/27] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2

The type is never set but we check for it in wacom_wireless_irq().
It looks like this is a big hack from the beginning, so fill in the gap
only.

Untested.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 499cc82..9e283aa 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1720,9 +1720,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		error = hid_hw_open(hdev);
 
 	if ((wacom_wac->features.type == INTUOSHT ||
-	    wacom_wac->features.type == INTUOSHT2) &&
+	     wacom_wac->features.type == INTUOSHT2) &&
 	    (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)) {
-			wacom_wac->shared->touch_input = wacom_wac->touch_input;
+		wacom_wac->shared->type = wacom_wac->features.type;
+		wacom_wac->shared->touch_input = wacom_wac->touch_input;
 	}
 
 	return 0;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437077 — [PATCH 10/27] HID: wacom: use devm_kasprintf for allocating the name of the remote

The sysfs group was indeed removed by kobject_put(wacom->remote_dir) in
wacom_remove(), but the name of the group was never freed.

Also remove the misplaced kobject_put(wacom->remote_dir) in the error
path of wacom_remote_create_attr_group().

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index c08a752..ec088c1 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -25,7 +25,6 @@
 #define WAC_CMD_RETRIES		10
 #define WAC_CMD_DELETE_PAIRING	0x20
 #define WAC_CMD_UNPAIR_ALL	0xFF
-#define WAC_REMOTE_SERIAL_MAX_STRLEN	9
 
 #define DEV_ATTR_RW_PERM (S_IRUGO | S_IWUSR | S_IWGRP)
 #define DEV_ATTR_WO_PERM (S_IWUSR | S_IWGRP)
@@ -1233,23 +1232,21 @@ DEVICE_EKR_ATTR_GROUP(4);
 int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial, int index)
 {
 	int error = 0;
-	char *buf;
 	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
 
 	wacom_wac->serial[index] = serial;
 
-	buf = kzalloc(WAC_REMOTE_SERIAL_MAX_STRLEN, GFP_KERNEL);
-	if (!buf)
+	wacom->remote_group[index].name = devm_kasprintf(&wacom->hdev->dev,
+							 GFP_KERNEL,
+							 "%d", serial);
+	if (!wacom->remote_group[index].name)
 		return -ENOMEM;
-	snprintf(buf, WAC_REMOTE_SERIAL_MAX_STRLEN, "%d", serial);
-	wacom->remote_group[index].name = buf;
 
 	error = sysfs_create_group(wacom->remote_dir,
 				   &wacom->remote_group[index]);
 	if (error) {
 		hid_err(wacom->hdev,
 			"cannot create sysfs group err: %d\n", error);
-		kobject_put(wacom->remote_dir);
 		return error;
 	}
 
@@ -1271,7 +1268,8 @@ void wacom_remote_destroy_attr_group(struct wacom *wacom, __u32 serial)
 			if (wacom->remote_group[i].name) {
 				sysfs_remove_group(wacom->remote_dir,
 						   &wacom->remote_group[i]);
-				kfree(wacom->remote_group[i].name);
+				devm_kfree(&wacom->hdev->dev,
+					   (char *)wacom->remote_group[i].name);
 				wacom->remote_group[i].name = NULL;
 			}
 		}
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437078 — [PATCH 22/27] HID: wacom: EKR: attach the power_supply on first connection

Or Gnome complains about an empty battery.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_sys.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 04f5c75..1d79215 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1917,6 +1917,10 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
 	remote->remotes[index].registered = false;
 	spin_unlock_irqrestore(&remote->remote_lock, flags);
 
+	if (remote->remotes[index].battery.battery)
+		devres_release_group(&wacom->hdev->dev,
+				     &remote->remotes[index].battery.bat_desc);
+
 	if (remote->remotes[index].group.name)
 		devres_release_group(&wacom->hdev->dev,
 				     &remote->remotes[index]);
@@ -1926,6 +1930,7 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
 			remote->remotes[i].serial = 0;
 			remote->remotes[i].group.name = NULL;
 			remote->remotes[i].registered = false;
+			remote->remotes[i].battery.battery = NULL;
 			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 		}
 	}
@@ -1982,11 +1987,6 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 	if (error)
 		goto fail;
 
-	error = __wacom_initialize_battery(wacom,
-					   &remote->remotes[index].battery);
-	if (error)
-		goto fail;
-
 	remote->remotes[index].registered = true;
 
 	devres_close_group(dev, &remote->remotes[index]);
@@ -1998,6 +1998,28 @@ fail:
 	return error;
 }
 
+static int wacom_remote_attach_battery(struct wacom *wacom, int index)
+{
+	struct wacom_remote *remote = wacom->remote;
+	int error;
+
+	if (!remote->remotes[index].registered)
+		return 0;
+
+	if (remote->remotes[index].battery.battery)
+		return 0;
+
+	if (wacom->led.groups[index].select == WACOM_STATUS_UNKNOWN)
+		return 0;
+
+	error = __wacom_initialize_battery(wacom,
+					&wacom->remote->remotes[index].battery);
+	if (error)
+		return error;
+
+	return 0;
+}
+
 static void wacom_remote_work(struct work_struct *work)
 {
 	struct wacom *wacom = container_of(work, struct wacom, remote_work);
@@ -2028,8 +2050,10 @@ static void wacom_remote_work(struct work_struct *work)
 		serial = data.remote[i].serial;
 		if (data.remote[i].connected) {
 
-			if (remote->remotes[i].serial == serial)
+			if (remote->remotes[i].serial == serial) {
+				wacom_remote_attach_battery(wacom, i);
 				continue;
+			}
 
 			if (remote->remotes[i].serial)
 				wacom_remote_destroy_one(wacom, i);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437079 — [PATCH 01/27] HID: wacom: actually report the battery level for wireless connected

Since fd5f92b ("HID: wacom: reuse wacom_parse_and_register() in
wireless_work"), wacom->shared->type is not set.
Send the information of the battery if we have one.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_wac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 1eae13c..d6746ca 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2125,6 +2125,7 @@ static int wacom_bamboo_pad_irq(struct wacom_wac *wacom, size_t len)
 
 static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
 {
+	struct wacom *w = container_of(wacom, struct wacom, wacom_wac);
 	unsigned char *data = wacom->data;
 	int connected;
 
@@ -2152,7 +2153,7 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
 			wacom_schedule_work(wacom);
 		}
 
-		if (wacom->shared->type)
+		if (w->battery)
 			wacom_notify_battery(wacom, battery, charging, 1, 0);
 
 	} else if (wacom->pid != 0) {
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437080 — [PATCH 21/27] HID: wacom: EKR: have one power_supply per remote

Previously, all the remotes attached to the same receiver would share the
same power_supply. That's not good as the remotes will constantly change
the battery information according to their own state.

To have something generic enough, we introduce struct wacom_battery
which regroups all the information we need for a battery.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom.h     |  19 ++++++--
 drivers/hid/wacom_sys.c | 123 ++++++++++++++++++++++++++----------------------
 drivers/hid/wacom_wac.c |  55 ++++++++++++----------
 drivers/hid/wacom_wac.h |   6 ---
 4 files changed, 111 insertions(+), 92 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 393b5af..768d696 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -116,6 +116,19 @@ struct wacom_group_leds {
 	u8 select; /* status led selector (0..3) */
 };
 
+struct wacom_battery {
+	struct power_supply_desc bat_desc;
+	struct power_supply_desc ac_desc;
+	struct power_supply *battery;
+	struct power_supply *ac;
+	char bat_name[WACOM_NAME_MAX];
+	char ac_name[WACOM_NAME_MAX];
+	int battery_capacity;
+	int bat_charging;
+	int bat_connected;
+	int ps_connected;
+};
+
 struct wacom_remote {
 	spinlock_t remote_lock;
 	struct kfifo remote_fifo;
@@ -125,6 +138,7 @@ struct wacom_remote {
 		u32 serial;
 		struct input_dev *input;
 		bool registered;
+		struct wacom_battery battery;
 	} remotes[WACOM_MAX_REMOTES];
 };
 
@@ -144,10 +158,7 @@ struct wacom {
 		u8 hlv;       /* status led brightness button pressed (1..127) */
 		u8 img_lum;   /* OLED matrix display brightness */
 	} led;
-	struct power_supply *battery;
-	struct power_supply *ac;
-	struct power_supply_desc battery_desc;
-	struct power_supply_desc ac_desc;
+	struct wacom_battery battery;
 	bool resources;
 };
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index f2f5b4b..04f5c75 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1101,27 +1101,26 @@ static int wacom_battery_get_property(struct power_supply *psy,
 				      enum power_supply_property psp,
 				      union power_supply_propval *val)
 {
-	struct wacom *wacom = power_supply_get_drvdata(psy);
+	struct wacom_battery *battery = power_supply_get_drvdata(psy);
 	int ret = 0;
 
 	switch (psp) {
 		case POWER_SUPPLY_PROP_PRESENT:
-			val->intval = wacom->wacom_wac.bat_connected;
+			val->intval = battery->bat_connected;
 			break;
 		case POWER_SUPPLY_PROP_SCOPE:
 			val->intval = POWER_SUPPLY_SCOPE_DEVICE;
 			break;
 		case POWER_SUPPLY_PROP_CAPACITY:
-			val->intval =
-				wacom->wacom_wac.battery_capacity;
+			val->intval = battery->battery_capacity;
 			break;
 		case POWER_SUPPLY_PROP_STATUS:
-			if (wacom->wacom_wac.bat_charging)
+			if (battery->bat_charging)
 				val->intval = POWER_SUPPLY_STATUS_CHARGING;
-			else if (wacom->wacom_wac.battery_capacity == 100 &&
-				    wacom->wacom_wac.ps_connected)
+			else if (battery->battery_capacity == 100 &&
+				    battery->ps_connected)
 				val->intval = POWER_SUPPLY_STATUS_FULL;
-			else if (wacom->wacom_wac.ps_connected)
+			else if (battery->ps_connected)
 				val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
 			else
 				val->intval = POWER_SUPPLY_STATUS_DISCHARGING;
@@ -1138,14 +1137,14 @@ static int wacom_ac_get_property(struct power_supply *psy,
 				enum power_supply_property psp,
 				union power_supply_propval *val)
 {
-	struct wacom *wacom = power_supply_get_drvdata(psy);
+	struct wacom_battery *battery = power_supply_get_drvdata(psy);
 	int ret = 0;
 
 	switch (psp) {
 	case POWER_SUPPLY_PROP_PRESENT:
 		/* fall through */
 	case POWER_SUPPLY_PROP_ONLINE:
-		val->intval = wacom->wacom_wac.ps_connected;
+		val->intval = battery->ps_connected;
 		break;
 	case POWER_SUPPLY_PROP_SCOPE:
 		val->intval = POWER_SUPPLY_SCOPE_DEVICE;
@@ -1157,58 +1156,56 @@ static int wacom_ac_get_property(struct power_supply *psy,
 	return ret;
 }
 
-static int wacom_initialize_battery(struct wacom *wacom)
+static int __wacom_initialize_battery(struct wacom *wacom,
+				      struct wacom_battery *battery)
 {
 	static atomic_t battery_no = ATOMIC_INIT(0);
 	struct device *dev = &wacom->hdev->dev;
-	struct power_supply_config psy_cfg = { .drv_data = wacom, };
-	struct power_supply_desc *bat_desc = &wacom->battery_desc;
+	struct power_supply_config psy_cfg = { .drv_data = battery, };
+	struct power_supply *ps_bat, *ps_ac;
+	struct power_supply_desc *bat_desc = &battery->bat_desc;
+	struct power_supply_desc *ac_desc = &battery->ac_desc;
 	unsigned long n;
 	int error;
 
 	if (!devres_open_group(dev, bat_desc, GFP_KERNEL))
 		return -ENOMEM;
 
-	if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
-		struct power_supply_desc *ac_desc = &wacom->ac_desc;
-		n = atomic_inc_return(&battery_no) - 1;
-
-		bat_desc->properties = wacom_battery_props;
-		bat_desc->num_properties = ARRAY_SIZE(wacom_battery_props);
-		bat_desc->get_property = wacom_battery_get_property;
-		sprintf(wacom->wacom_wac.bat_name, "wacom_battery_%ld", n);
-		bat_desc->name = wacom->wacom_wac.bat_name;
-		bat_desc->type = POWER_SUPPLY_TYPE_BATTERY;
-		bat_desc->use_for_apm = 0;
-
-		ac_desc->properties = wacom_ac_props;
-		ac_desc->num_properties = ARRAY_SIZE(wacom_ac_props);
-		ac_desc->get_property = wacom_ac_get_property;
-		sprintf(wacom->wacom_wac.ac_name, "wacom_ac_%ld", n);
-		ac_desc->name = wacom->wacom_wac.ac_name;
-		ac_desc->type = POWER_SUPPLY_TYPE_MAINS;
-		ac_desc->use_for_apm = 0;
-
-		wacom->battery = devm_power_supply_register(dev,
-							   &wacom->battery_desc,
-							   &psy_cfg);
-		if (IS_ERR(wacom->battery)) {
-			error = PTR_ERR(wacom->battery);
-			goto err;
-		}
+	n = atomic_inc_return(&battery_no) - 1;
+
+	bat_desc->properties = wacom_battery_props;
+	bat_desc->num_properties = ARRAY_SIZE(wacom_battery_props);
+	bat_desc->get_property = wacom_battery_get_property;
+	sprintf(battery->bat_name, "wacom_battery_%ld", n);
+	bat_desc->name = battery->bat_name;
+	bat_desc->type = POWER_SUPPLY_TYPE_BATTERY;
+	bat_desc->use_for_apm = 0;
+
+	ac_desc->properties = wacom_ac_props;
+	ac_desc->num_properties = ARRAY_SIZE(wacom_ac_props);
+	ac_desc->get_property = wacom_ac_get_property;
+	sprintf(battery->ac_name, "wacom_ac_%ld", n);
+	ac_desc->name = battery->ac_name;
+	ac_desc->type = POWER_SUPPLY_TYPE_MAINS;
+	ac_desc->use_for_apm = 0;
+
+	ps_bat = devm_power_supply_register(dev, bat_desc, &psy_cfg);
+	if (IS_ERR(ps_bat)) {
+		error = PTR_ERR(ps_bat);
+		goto err;
+	}
 
-		power_supply_powers(wacom->battery, &wacom->hdev->dev);
+	ps_ac = devm_power_supply_register(dev, ac_desc, &psy_cfg);
+	if (IS_ERR(ps_ac)) {
+		error = PTR_ERR(ps_ac);
+		goto err;
+	}
 
-		wacom->ac = devm_power_supply_register(dev,
-						       &wacom->ac_desc,
-						       &psy_cfg);
-		if (IS_ERR(wacom->ac)) {
-			error = PTR_ERR(wacom->ac);
-			goto err;
-		}
+	power_supply_powers(ps_bat, &wacom->hdev->dev);
+	power_supply_powers(ps_ac, &wacom->hdev->dev);
 
-		power_supply_powers(wacom->ac, &wacom->hdev->dev);
-	}
+	battery->battery = ps_bat;
+	battery->ac = ps_ac;
 
 	devres_close_group(dev, bat_desc);
 	return 0;
@@ -1218,12 +1215,21 @@ err:
 	return error;
 }
 
+static int wacom_initialize_battery(struct wacom *wacom)
+{
+	if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY)
+		return __wacom_initialize_battery(wacom, &wacom->battery);
+
+	return 0;
+}
+
 static void wacom_destroy_battery(struct wacom *wacom)
 {
-	if (wacom->battery) {
-		devres_release_group(&wacom->hdev->dev, &wacom->battery_desc);
-		wacom->battery = NULL;
-		wacom->ac = NULL;
+	if (wacom->battery.battery) {
+		devres_release_group(&wacom->hdev->dev,
+				     &wacom->battery.bat_desc);
+		wacom->battery.battery = NULL;
+		wacom->battery.ac = NULL;
 	}
 }
 
@@ -1593,11 +1599,11 @@ void wacom_battery_work(struct work_struct *work)
 	struct wacom *wacom = container_of(work, struct wacom, battery_work);
 
 	if ((wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) &&
-	     !wacom->battery) {
+	     !wacom->battery.battery) {
 		wacom_initialize_battery(wacom);
 	}
 	else if (!(wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) &&
-		 wacom->battery) {
+		 wacom->battery.battery) {
 		wacom_destroy_battery(wacom);
 	}
 }
@@ -1976,6 +1982,11 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 	if (error)
 		goto fail;
 
+	error = __wacom_initialize_battery(wacom,
+					   &remote->remotes[index].battery);
+	if (error)
+		goto fail;
+
 	remote->remotes[index].registered = true;
 
 	devres_close_group(dev, &remote->remotes[index]);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index d1210d0..6904e9b 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -48,25 +48,37 @@ static unsigned short batcap_gr[8] = { 1, 15, 25, 35, 50, 70, 100, 100 };
  */
 static unsigned short batcap_i4[8] = { 1, 15, 30, 45, 60, 70, 85, 100 };
 
+static void __wacom_notify_battery(struct wacom_battery *battery,
+				   int bat_capacity, bool bat_charging,
+				   bool bat_connected, bool ps_connected)
+{
+	bool changed = battery->battery_capacity != bat_capacity  ||
+		       battery->bat_charging     != bat_charging  ||
+		       battery->bat_connected    != bat_connected ||
+		       battery->ps_connected     != ps_connected;
+
+	if (changed) {
+		battery->battery_capacity = bat_capacity;
+		battery->bat_charging = bat_charging;
+		battery->bat_connected = bat_connected;
+		battery->ps_connected = ps_connected;
+
+		if (battery->battery)
+			power_supply_changed(battery->battery);
+	}
+}
+
 static void wacom_notify_battery(struct wacom_wac *wacom_wac,
 	int bat_capacity, bool bat_charging, bool bat_connected,
 	bool ps_connected)
 {
 	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
-	bool changed = wacom_wac->battery_capacity != bat_capacity  ||
-		       wacom_wac->bat_charging     != bat_charging  ||
-		       wacom_wac->bat_connected    != bat_connected ||
-		       wacom_wac->ps_connected     != ps_connected;
 
-	if (changed) {
-		wacom_wac->battery_capacity = bat_capacity;
-		wacom_wac->bat_charging = bat_charging;
-		wacom_wac->bat_connected = bat_connected;
-		wacom_wac->ps_connected = ps_connected;
+	if (!wacom->battery.battery)
+		return;
 
-		if (wacom->battery)
-			power_supply_changed(wacom->battery);
-	}
+	__wacom_notify_battery(&wacom->battery, bat_capacity, bat_charging,
+			       bat_connected, ps_connected);
 }
 
 static int wacom_penpartner_irq(struct wacom_wac *wacom)
@@ -754,7 +766,6 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 	struct input_dev *input;
 	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
 	struct wacom_remote *remote = wacom->remote;
-	struct wacom_features *features = &wacom_wac->features;
 	int bat_charging, bat_percent, touch_ring_mode;
 	__u32 serial;
 	int i, index = -1;
@@ -829,14 +840,8 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 			wacom->led.groups[i].select = touch_ring_mode;
 	}
 
-	if (!wacom->battery &&
-	    !(features->quirks & WACOM_QUIRK_BATTERY)) {
-		features->quirks |= WACOM_QUIRK_BATTERY;
-		wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
-	}
-
-	wacom_notify_battery(wacom_wac, bat_percent, bat_charging, 1,
-			     bat_charging);
+	__wacom_notify_battery(&remote->remotes[index].battery, bat_percent,
+				bat_charging, 1, bat_charging);
 
 out:
 	spin_unlock_irqrestore(&remote->remote_lock, flags);
@@ -2132,7 +2137,6 @@ static int wacom_bamboo_pad_irq(struct wacom_wac *wacom, size_t len)
 
 static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
 {
-	struct wacom *w = container_of(wacom, struct wacom, wacom_wac);
 	unsigned char *data = wacom->data;
 	int connected;
 
@@ -2160,8 +2164,7 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
 			wacom_schedule_work(wacom, WACOM_WORKER_WIRELESS);
 		}
 
-		if (w->battery)
-			wacom_notify_battery(wacom, battery, charging, 1, 0);
+		wacom_notify_battery(wacom, battery, charging, 1, 0);
 
 	} else if (wacom->pid != 0) {
 		/* disconnected while previously connected */
@@ -2198,14 +2201,14 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
 		wacom_notify_battery(wacom_wac, battery, charging,
 				     battery || charging, 1);
 
-		if (!wacom->battery &&
+		if (!wacom->battery.battery &&
 		    !(features->quirks & WACOM_QUIRK_BATTERY)) {
 			features->quirks |= WACOM_QUIRK_BATTERY;
 			wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
 		}
 	}
 	else if ((features->quirks & WACOM_QUIRK_BATTERY) &&
-		 wacom->battery) {
+		 wacom->battery.battery) {
 		features->quirks &= ~WACOM_QUIRK_BATTERY;
 		wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
 		wacom_notify_battery(wacom_wac, 0, 0, 0, 0);
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 6be6cae..8a8974c 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -229,8 +229,6 @@ struct wacom_wac {
 	char pen_name[WACOM_NAME_MAX];
 	char touch_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];
@@ -242,11 +240,7 @@ struct wacom_wac {
 	struct input_dev *touch_input;
 	struct input_dev *pad_input;
 	int pid;
-	int battery_capacity;
 	int num_contacts_left;
-	int bat_charging;
-	int bat_connected;
-	int ps_connected;
 	u8 bt_features;
 	u8 bt_high_speed;
 	int mode_report;
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437081 — [PATCH 06/27] HID: wacom: switch battery to devres

Simplifying the error code paths.
We need to keep wacom_destroy_battery() around for now as the wireless
module and the remotes are using it to dynamically remove the battery
supply on disconnect.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>

# Conflicts:
#	drivers/hid/wacom_sys.c
---
 drivers/hid/wacom_sys.c | 38 ++++++++++++++++++++++++--------------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 54f0260..61dcb87e 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1082,11 +1082,16 @@ static int wacom_ac_get_property(struct power_supply *psy,
 static int wacom_initialize_battery(struct wacom *wacom)
 {
 	static atomic_t battery_no = ATOMIC_INIT(0);
+	struct device *dev = &wacom->hdev->dev;
 	struct power_supply_config psy_cfg = { .drv_data = wacom, };
+	struct power_supply_desc *bat_desc = &wacom->battery_desc;
 	unsigned long n;
+	int error;
+
+	if (!devres_open_group(dev, bat_desc, GFP_KERNEL))
+		return -ENOMEM;
 
 	if (wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) {
-		struct power_supply_desc *bat_desc = &wacom->battery_desc;
 		struct power_supply_desc *ac_desc = &wacom->ac_desc;
 		n = atomic_inc_return(&battery_no) - 1;
 
@@ -1106,33 +1111,40 @@ static int wacom_initialize_battery(struct wacom *wacom)
 		ac_desc->type = POWER_SUPPLY_TYPE_MAINS;
 		ac_desc->use_for_apm = 0;
 
-		wacom->battery = power_supply_register(&wacom->hdev->dev,
-					      &wacom->battery_desc, &psy_cfg);
-		if (IS_ERR(wacom->battery))
-			return PTR_ERR(wacom->battery);
+		wacom->battery = devm_power_supply_register(dev,
+							   &wacom->battery_desc,
+							   &psy_cfg);
+		if (IS_ERR(wacom->battery)) {
+			error = PTR_ERR(wacom->battery);
+			goto err;
+		}
 
 		power_supply_powers(wacom->battery, &wacom->hdev->dev);
 
-		wacom->ac = power_supply_register(&wacom->hdev->dev,
-						  &wacom->ac_desc,
-						  &psy_cfg);
+		wacom->ac = devm_power_supply_register(dev,
+						       &wacom->ac_desc,
+						       &psy_cfg);
 		if (IS_ERR(wacom->ac)) {
-			power_supply_unregister(wacom->battery);
-			return PTR_ERR(wacom->ac);
+			error = PTR_ERR(wacom->ac);
+			goto err;
 		}
 
 		power_supply_powers(wacom->ac, &wacom->hdev->dev);
 	}
 
+	devres_close_group(dev, bat_desc);
 	return 0;
+
+err:
+	devres_release_group(dev, bat_desc);
+	return error;
 }
 
 static void wacom_destroy_battery(struct wacom *wacom)
 {
 	if (wacom->battery) {
-		power_supply_unregister(wacom->battery);
+		devres_release_group(&wacom->hdev->dev, &wacom->battery_desc);
 		wacom->battery = NULL;
-		power_supply_unregister(wacom->ac);
 		wacom->ac = NULL;
 	}
 }
@@ -1731,7 +1743,6 @@ fail_remote:
 fail_leds:
 	wacom_clean_inputs(wacom);
 fail_register_inputs:
-	wacom_destroy_battery(wacom);
 fail_battery:
 	wacom_remove_shared_data(wacom);
 fail_shared_data:
@@ -1920,7 +1931,6 @@ static void wacom_remove(struct hid_device *hdev)
 	wacom_clean_inputs(wacom);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
-	wacom_destroy_battery(wacom);
 	wacom_remove_shared_data(wacom);
 
 	hid_set_drvdata(hdev, NULL);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437082 — [PATCH 09/27] HID: wacom: convert LEDs to devres

Use our own wacom_devm_sysfs_create_group() as there is currently no
generic one. It has been requested at least twice [1][2] but has been
always rejected.
However, in the Wacom case, for the wirelessly connected devices, we need
to be able to release the created sysfs files without removing the parent
kobject.

[1] https://patchwork.kernel.org/patch/7526551/
[2] https://lkml.org/lkml/2013/3/14/728

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom.h     |  1 -
 drivers/hid/wacom_sys.c | 95 +++++++++++++++++++++++--------------------------
 2 files changed, 45 insertions(+), 51 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 6f5555f..bcfeb51 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -124,7 +124,6 @@ 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;
 	struct power_supply *ac;
 	struct power_supply_desc battery_desc;
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 08f6d1f..c08a752 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -909,6 +909,45 @@ static struct attribute_group intuos5_led_attr_group = {
 	.attrs = intuos5_led_attrs,
 };
 
+struct wacom_sysfs_group_devres {
+	struct attribute_group *group;
+	struct kobject *root;
+};
+
+static void wacom_devm_sysfs_group_release(struct device *dev, void *res)
+{
+	struct wacom_sysfs_group_devres *devres = res;
+	struct kobject *kobj = devres->root;
+
+	dev_dbg(dev, "%s: dropping reference to %s\n",
+		__func__, devres->group->name);
+	sysfs_remove_group(kobj, devres->group);
+}
+
+static int wacom_devm_sysfs_create_group(struct wacom *wacom,
+					 struct attribute_group *group)
+{
+	struct wacom_sysfs_group_devres *devres;
+	int error;
+
+	devres = devres_alloc(wacom_devm_sysfs_group_release,
+			      sizeof(struct wacom_sysfs_group_devres),
+			      GFP_KERNEL);
+	if (!devres)
+		return -ENOMEM;
+
+	devres->group = group;
+	devres->root = &wacom->hdev->dev.kobj;
+
+	error = sysfs_create_group(devres->root, group);
+	if (error)
+		return error;
+
+	devres_add(&wacom->hdev->dev, devres);
+
+	return 0;
+}
+
 static int wacom_initialize_leds(struct wacom *wacom)
 {
 	int error;
@@ -927,8 +966,8 @@ static int wacom_initialize_leds(struct wacom *wacom)
 		wacom->led.llv = 10;
 		wacom->led.hlv = 20;
 		wacom->led.img_lum = 10;
-		error = sysfs_create_group(&wacom->hdev->dev.kobj,
-					   &intuos4_led_attr_group);
+		error = wacom_devm_sysfs_create_group(wacom,
+						      &intuos4_led_attr_group);
 		break;
 
 	case WACOM_24HD:
@@ -939,8 +978,8 @@ static int wacom_initialize_leds(struct wacom *wacom)
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
-		error = sysfs_create_group(&wacom->hdev->dev.kobj,
-					   &cintiq_led_attr_group);
+		error = wacom_devm_sysfs_create_group(wacom,
+						      &cintiq_led_attr_group);
 		break;
 
 	case INTUOS5S:
@@ -955,8 +994,8 @@ static int wacom_initialize_leds(struct wacom *wacom)
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
-		error = sysfs_create_group(&wacom->hdev->dev.kobj,
-					  &intuos5_led_attr_group);
+		error = wacom_devm_sysfs_create_group(wacom,
+						      &intuos5_led_attr_group);
 		break;
 
 	default:
@@ -969,48 +1008,10 @@ 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;
-
-	if (!(wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD))
-		return;
-
-	wacom->led_initialized = false;
-
-	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);
-		break;
-
-	case WACOM_24HD:
-	case WACOM_21UX2:
-		sysfs_remove_group(&wacom->hdev->dev.kobj,
-				   &cintiq_led_attr_group);
-		break;
-
-	case INTUOS5S:
-	case INTUOS5:
-	case INTUOS5L:
-	case INTUOSPS:
-	case INTUOSPM:
-	case INTUOSPL:
-		sysfs_remove_group(&wacom->hdev->dev.kobj,
-				   &intuos5_led_attr_group);
-		break;
-	}
-}
-
 static enum power_supply_property wacom_battery_props[] = {
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_STATUS,
@@ -1729,7 +1730,6 @@ fail_quirks:
 fail_hw_start:
 	kobject_put(wacom->remote_dir);
 fail_remote:
-	wacom_destroy_leds(wacom);
 fail_leds:
 fail_register_inputs:
 fail_battery:
@@ -1763,14 +1763,12 @@ static void wacom_wireless_work(struct work_struct *work)
 	hdev1 = usb_get_intfdata(usbdev->config->interface[1]);
 	wacom1 = hid_get_drvdata(hdev1);
 	wacom_wac1 = &(wacom1->wacom_wac);
-	wacom_destroy_leds(wacom1);
 	wacom_release_resources(wacom1);
 
 	/* Touch interface */
 	hdev2 = usb_get_intfdata(usbdev->config->interface[2]);
 	wacom2 = hid_get_drvdata(hdev2);
 	wacom_wac2 = &(wacom2->wacom_wac);
-	wacom_destroy_leds(wacom2);
 	wacom_release_resources(wacom2);
 
 	if (wacom_wac->pid == 0) {
@@ -1825,9 +1823,7 @@ static void wacom_wireless_work(struct work_struct *work)
 	return;
 
 fail:
-	wacom_destroy_leds(wacom1);
 	wacom_release_resources(wacom1);
-	wacom_destroy_leds(wacom2);
 	wacom_release_resources(wacom2);
 	return;
 }
@@ -1917,7 +1913,6 @@ static void wacom_remove(struct hid_device *hdev)
 	cancel_work_sync(&wacom->wireless_work);
 	cancel_work_sync(&wacom->battery_work);
 	kobject_put(wacom->remote_dir);
-	wacom_destroy_leds(wacom);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
 	wacom_remove_shared_data(wacom);
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437084 — [PATCH 20/27] HID: wacom: EKR: allocate one input node per remote

Thanks to devres, we can now afford to create more than one input node
without having to overload the remove/failure paths. Having one input
node per remote is something which should have been implemented from start
but the probability of having users with several remotes is quite low.
Anyway, still, better looking at the future and implement things properly.

Remote input nodes will be freed/unregistered magically as they are
created in the devres group &remote->remotes[index].

We need to open the hid node now that the remotes are dynamically
allocated.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom.h     |  2 ++
 drivers/hid/wacom_sys.c | 30 ++++++++++++++++++++++++++++++
 drivers/hid/wacom_wac.c | 35 ++++++++++++++++++++++++++++++-----
 3 files changed, 62 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 6b8df67..393b5af 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -123,6 +123,8 @@ struct wacom_remote {
 	struct {
 		struct attribute_group group;
 		u32 serial;
+		struct input_dev *input;
+		bool registered;
 	} remotes[WACOM_MAX_REMOTES];
 };
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index c3b2692..f2f5b4b 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1905,6 +1905,11 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
 	struct wacom_remote *remote = wacom->remote;
 	u32 serial = remote->remotes[index].serial;
 	int i;
+	unsigned long flags;
+
+	spin_lock_irqsave(&remote->remote_lock, flags);
+	remote->remotes[index].registered = false;
+	spin_unlock_irqrestore(&remote->remote_lock, flags);
 
 	if (remote->remotes[index].group.name)
 		devres_release_group(&wacom->hdev->dev,
@@ -1914,6 +1919,7 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
 		if (remote->remotes[i].serial == serial) {
 			remote->remotes[i].serial = 0;
 			remote->remotes[i].group.name = NULL;
+			remote->remotes[i].registered = false;
 			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 		}
 	}
@@ -1946,8 +1952,32 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 	if (error)
 		goto fail;
 
+	remote->remotes[index].input = wacom_allocate_input(wacom);
+	if (!remote->remotes[index].input) {
+		error = -ENOMEM;
+		goto fail;
+	}
+	remote->remotes[index].input->uniq = remote->remotes[index].group.name;
+	remote->remotes[index].input->name = wacom->wacom_wac.pad_name;
+
+	if (!remote->remotes[index].input->name) {
+		error = -EINVAL;
+		goto fail;
+	}
+
+	error = wacom_setup_pad_input_capabilities(remote->remotes[index].input,
+						   &wacom->wacom_wac);
+	if (error)
+		goto fail;
+
 	remote->remotes[index].serial = serial;
 
+	error = input_register_device(remote->remotes[index].input);
+	if (error)
+		goto fail;
+
+	remote->remotes[index].registered = true;
+
 	devres_close_group(dev, &remote->remotes[index]);
 	return 0;
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 1162dc5..d1210d0 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -751,23 +751,38 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
 static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 {
 	unsigned char *data = wacom_wac->data;
-	struct input_dev *input = wacom_wac->pad_input;
+	struct input_dev *input;
 	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
 	struct wacom_remote *remote = wacom->remote;
 	struct wacom_features *features = &wacom_wac->features;
 	int bat_charging, bat_percent, touch_ring_mode;
 	__u32 serial;
-	int i;
+	int i, index = -1;
+	unsigned long flags;
 
 	if (data[0] != WACOM_REPORT_REMOTE) {
-		dev_dbg(input->dev.parent,
-			"%s: received unknown report #%d", __func__, data[0]);
+		hid_dbg(wacom->hdev, "%s: received unknown report #%d",
+			__func__, data[0]);
 		return 0;
 	}
 
 	serial = data[3] + (data[4] << 8) + (data[5] << 16);
 	wacom_wac->id[0] = PAD_DEVICE_ID;
 
+	spin_lock_irqsave(&remote->remote_lock, flags);
+
+	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
+		if (remote->remotes[i].serial == serial) {
+			index = i;
+			break;
+		}
+	}
+
+	if (index < 0 || !remote->remotes[index].registered)
+		goto out;
+
+	input = remote->remotes[index].input;
+
 	input_report_key(input, BTN_0, (data[9] & 0x01));
 	input_report_key(input, BTN_1, (data[9] & 0x02));
 	input_report_key(input, BTN_2, (data[9] & 0x04));
@@ -804,6 +819,8 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 
 	input_event(input, EV_MSC, MSC_SERIAL, serial);
 
+	input_sync(input);
+
 	/*Which mode select (LED light) is currently on?*/
 	touch_ring_mode = (data[11] & 0xC0) >> 6;
 
@@ -821,7 +838,9 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 	wacom_notify_battery(wacom_wac, bat_percent, bat_charging, 1,
 			     bat_charging);
 
-	return 1;
+out:
+	spin_unlock_irqrestore(&remote->remote_lock, flags);
+	return 0;
 }
 
 static int wacom_remote_status_irq(struct wacom_wac *wacom_wac, size_t len)
@@ -2456,6 +2475,9 @@ void wacom_setup_device_quirks(struct wacom *wacom)
 			features->quirks |= WACOM_QUIRK_BATTERY;
 		}
 	}
+
+	if (features->type == REMOTE)
+		features->device_type |= WACOM_DEVICETYPE_WL_MONITOR;
 }
 
 int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
@@ -2760,6 +2782,9 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,
 	if (!(features->device_type & WACOM_DEVICETYPE_PAD))
 		return -ENODEV;
 
+	if (features->type == REMOTE && input_dev == wacom_wac->pad_input)
+		return -ENODEV;
+
 	input_dev->evbit[0] |= BIT_MASK(EV_KEY) | BIT_MASK(EV_ABS);
 
 	/* kept for making legacy xf86-input-wacom working with the wheels */
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

#1437085 — [PATCH 27/27] HID: wacom: leds: handle Cintiq 24HD leds buttons

The 24HD has 1 button per LED (first three buttons of each group).
We need a special treatment for them as it's not a uniq button that
switches between the LEDs.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
---
 drivers/hid/wacom_wac.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a2b9c09..fcf2264 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2764,6 +2764,31 @@ static void wacom_setup_numbered_buttons(struct input_dev *input_dev,
 		__set_bit(BTN_BASE + (i-16), input_dev->keybit);
 }
 
+static void wacom_24hd_update_leds(struct wacom *wacom, int mask, int group)
+{
+	struct wacom_led *led;
+	int i;
+	bool updated = false;
+
+	if (group > 0)
+		mask >>= 8;
+
+	for (i = 0; i < 3; i++) {
+		led = wacom_led_find(wacom, group, i);
+		if (!led) {
+			hid_err(wacom->hdev, "can't find LED %d in group %d\n",
+				i, group);
+			continue;
+		}
+		if (!updated && mask & BIT(i)) {
+			led->held = true;
+			led_trigger_event(&led->trigger, LED_FULL);
+		} else {
+			led->held = false;
+		}
+	}
+}
+
 static bool wacom_is_led_toggled(struct wacom *wacom, int button_count,
 				 int mask, int group)
 {
@@ -2781,7 +2806,11 @@ static void wacom_update_led(struct wacom *wacom, int button_count, int mask,
 	int cur;
 	bool pressed;
 
+	if (wacom->wacom_wac.features.type == WACOM_24HD)
+		return wacom_24hd_update_leds(wacom, mask, group);
+
 	pressed = wacom_is_led_toggled(wacom, button_count, mask, group);
+
 	cur = wacom->led.groups[group].select;
 
 	led = wacom_led_find(wacom, group, cur);
@@ -2808,6 +2837,13 @@ static void wacom_update_led(struct wacom *wacom, int button_count, int mask,
 	if (next_led == led)
 		return;
 
+	pr_err("%s group: %d led: (%d -> %d) t: %s %s:%d\n", __func__,
+		led->group,
+		led->id,
+		next_led->id,
+		next_led->trigger.name,
+		__FILE__, __LINE__);
+
 	next_led->held = true;
 	led_trigger_event(&next_led->trigger,
 			  wacom_leds_brightness_get(next_led));
-- 
2.5.5

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web