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


Groups > linux.kernel > #1442576 > unrolled thread

[PATCH v2 00/30] HID: wacom: cleanup/EKR/LED

Started byBenjamin Tissoires <benjamin.tissoires@redhat.com>
First post2016-07-13 18:10 +0200
Last post2016-07-13 23:40 +0200
Articles 20 on this page of 29 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH v2 00/30] HID: wacom: cleanup/EKR/LED Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:10 +0200
    [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2 Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:10 +0200
      Re: [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for  INTUOSHT and INTUOSHT2 Ping Cheng <pinglinux@gmail.com> - 2016-07-15 06:20 +0200
        Re: [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for  INTUOSHT and INTUOSHT2 Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-18 17:00 +0200
    [PATCH v2 11/30] HID: wacom: use devres to allocate driver data Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:10 +0200
    [PATCH v2 13/30] HID: wacom: leds: dynamically allocate LED groups Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:10 +0200
    [PATCH v2 28/30] HID: wacom: power_supply: mark the type as USB Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 23/30] HID: wacom: leds: use the ledclass instead of custom made sysfs files Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 24/30] HID: wacom: leds: actually release the LEDs on disconnect Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 22/30] HID: wacom: EKR: attach the power_supply on first connection Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 30/30] HID: wacom: power_supply: provide the actual model_name Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 05/30] HID: wacom: use one work queue per task Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 18/30] HID: wacom: EKR: use devres groups to manage resources Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 15/30] HID: wacom: EKR: have the wacom resources dynamically allocated Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 10/30] HID: wacom: use devm_kasprintf for allocating the name of the remote Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 19/30] HID: wacom: EKR: have one array of struct remotes instead of many arrays Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 04/30] HID: wacom: untie leds from inputs Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 16/30] HID: wacom: rework fail path in probe() and parse_and_register() Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 27/30] HID: wacom: leds: handle Cintiq 24HD leds buttons Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 25/30] HID: wacom: leds: handle the switch of the LEDs directly in the kernel Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 26/30] HID: wacom: leds: make sure Cintiq 21UX2 and 24HD control the right LEDs Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 06/30] HID: wacom: switch battery to devres Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 12/30] HID: wacom: devres manage the shared data too Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 08/30] HID: wacom: put the managed resources in a group Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 29/30] HID: wacom: power_supply: remove ac information Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 21/30] HID: wacom: EKR: have one power_supply per remote Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 14/30] HID: wacom: EKR: add a worker to add/remove resources on addition/removal Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    [PATCH v2 20/30] HID: wacom: EKR: allocate one input node per remote Benjamin Tissoires <benjamin.tissoires@redhat.com> - 2016-07-13 18:20 +0200
    Re: [PATCH v2 00/30] HID: wacom: cleanup/EKR/LED Aaron Armstrong Skomra <skomra@gmail.com> - 2016-07-13 23:40 +0200

Page 1 of 2  [1] 2  Next page →


#1442576 — [PATCH v2 00/30] HID: wacom: cleanup/EKR/LED

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:10 +0200
Subject[PATCH v2 00/30] HID: wacom: cleanup/EKR/LED
Message-ID<rUuTv-5nh-3@gated-at.bofh.it>
Hi,

So this is a v2 of my summer cleanup of the wacom driver.
I fixed the remarks from everybody I think, and it should be in a better shape
now.
I removed the patch that changed the LED banks ordering as libwacom exports it
that way. I also added 3 extra patches for the power_supply to be a little bit
more user friendly in gnome-control-center (well, upowerd).

Thanks for double testing on the Cintiq 21UX2 and the 24HD as I could only
compare the raw events to what was expected, and nothing is better than actual
testing with real hardware.

Cheers,
Benjamin

Benjamin Tissoires (30):
  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: handle the switch of the LEDs directly in the kernel
  HID: wacom: leds: make sure Cintiq 21UX2 and 24HD control the right
    LEDs
  HID: wacom: leds: handle Cintiq 24HD leds buttons
  HID: wacom: power_supply: mark the type as USB
  HID: wacom: power_supply: remove ac information
  HID: wacom: power_supply: provide the actual model_name

 Documentation/ABI/testing/sysfs-driver-wacom |    5 +
 drivers/hid/Kconfig                          |    1 +
 drivers/hid/wacom.h                          |   96 ++-
 drivers/hid/wacom_sys.c                      | 1104 ++++++++++++++++++--------
 drivers/hid/wacom_wac.c                      |  254 ++++--
 drivers/hid/wacom_wac.h                      |   19 +-
 6 files changed, 1058 insertions(+), 421 deletions(-)

-- 
2.5.5

[toc] | [next] | [standalone]


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

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:10 +0200
Subject[PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2
Message-ID<rUuTw-5nh-31@gated-at.bofh.it>
In reply to#1442576
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>
---

No changes in v2
---
 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]


#1443931 — Re: [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2

FromPing Cheng <pinglinux@gmail.com>
Date2016-07-15 06:20 +0200
SubjectRe: [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2
Message-ID<rV2Lv-28X-1@gated-at.bofh.it>
In reply to#1442578
On Wed, Jul 13, 2016 at 9:05 AM, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> The type is never set but we check for it in wacom_wireless_irq().

Type was assigned in wacom_wireless_work [1] before we moved code
around. It somehow failed to get into wacom_parse_and_register. The
value was only assigned once on stylus interface. It was unnecessary
to assign it again on touch interface since it is a shared value.
Maybe that was the reason it missed its "flight"?

[1] https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/commit/drivers/hid/wacom_sys.c?id=2546dacd3e0e48c40bbb99caf01455f1ade9bb24

> It looks like this is a big hack from the beginning, so fill in the gap
> only.

Yeah, wireless is a dynamic connection. We can not tell which tablet
model is going to be connected before it is paired. But, the dongle is
always the same. That's the tricky part.

> Untested.

Aaron tested it. So, your code is safe ;).

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

The whole set is

Acked-by: Ping Cheng <pingc@wacom.com>

Thanks for your effort!

Ping

> ---
>
> No changes in v2
> ---
>  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]


#1445573 — Re: [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-18 17:00 +0200
SubjectRe: [PATCH v2 02/30] HID: wacom: store the type in wacom->shared for INTUOSHT and INTUOSHT2
Message-ID<rWibv-7xJ-15@gated-at.bofh.it>
In reply to#1443931
On Jul 14 2016 or thereabouts, Ping Cheng wrote:
> On Wed, Jul 13, 2016 at 9:05 AM, Benjamin Tissoires
> <benjamin.tissoires@redhat.com> wrote:
> > The type is never set but we check for it in wacom_wireless_irq().
> 
> Type was assigned in wacom_wireless_work [1] before we moved code
> around. It somehow failed to get into wacom_parse_and_register. The
> value was only assigned once on stylus interface. It was unnecessary
> to assign it again on touch interface since it is a shared value.
> Maybe that was the reason it missed its "flight"?

Yeah, sorry I have seen that my patch removed it a while ago and that no
one actually reported it as an issue.

> 
> [1] https://git.kernel.org/cgit/linux/kernel/git/dtor/input.git/commit/drivers/hid/wacom_sys.c?id=2546dacd3e0e48c40bbb99caf01455f1ade9bb24
> 
> > It looks like this is a big hack from the beginning, so fill in the gap
> > only.
> 
> Yeah, wireless is a dynamic connection. We can not tell which tablet
> model is going to be connected before it is paired. But, the dongle is
> always the same. That's the tricky part.
> 
> > Untested.
> 
> Aaron tested it. So, your code is safe ;).
> 
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
> 
> The whole set is
> 
> Acked-by: Ping Cheng <pingc@wacom.com>
> 

Thanks!

Cheers,
Benjamin

> Thanks for your effort!
> 
> Ping
> 
> > ---
> >
> > No changes in v2
> > ---
> >  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]


#1442580 — [PATCH v2 11/30] HID: wacom: use devres to allocate driver data

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:10 +0200
Subject[PATCH v2 11/30] HID: wacom: use devres to allocate driver data
Message-ID<rUuTx-5nh-43@gated-at.bofh.it>
In reply to#1442576
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>
---

No changes in v2
---
 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]


#1442581 — [PATCH v2 13/30] HID: wacom: leds: dynamically allocate LED groups

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:10 +0200
Subject[PATCH v2 13/30] HID: wacom: leds: dynamically allocate LED groups
Message-ID<rUuTx-5nh-35@gated-at.bofh.it>
In reply to#1442576
We need to add an action to ensure wacom->led.groups is null when
wacom_led_control() gets called after the resources has been freed.

This also prevents to send a LED command when there is no support
from the device.

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

No changes in v2
---
 drivers/hid/wacom.h     |  8 +++--
 drivers/hid/wacom_sys.c | 85 +++++++++++++++++++++++++++++++++++++++++--------
 drivers/hid/wacom_wac.c |  2 +-
 3 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index bcfeb51..8ac1eb8 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -110,6 +110,10 @@ enum wacom_worker {
 	WACOM_WORKER_BATTERY,
 };
 
+struct wacom_group_leds {
+	u8 select; /* status led selector (0..3) */
+};
+
 struct wacom {
 	struct usb_device *usbdev;
 	struct usb_interface *intf;
@@ -118,8 +122,8 @@ struct wacom {
 	struct mutex lock;
 	struct work_struct wireless_work;
 	struct work_struct battery_work;
-	struct wacom_led {
-		u8 select[5]; /* status led selector (0..3) */
+	struct wacom_leds {
+		struct wacom_group_leds *groups;
 		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 */
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index d0f5764..4d4c737 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -647,6 +647,9 @@ static int wacom_led_control(struct wacom *wacom)
 	unsigned char report_id = WAC_CMD_LED_CONTROL;
 	int buf_size = 9;
 
+	if (!wacom->led.groups)
+		return -ENOTSUPP;
+
 	if (wacom->wacom_wac.pid) { /* wireless connected */
 		report_id = WAC_CMD_WL_LED_CONTROL;
 		buf_size = 13;
@@ -662,7 +665,7 @@ static int wacom_led_control(struct wacom *wacom)
 		 * one of four values:
 		 *    0 = Low; 1 = Medium; 2 = High; 3 = Off
 		 */
-		int ring_led = wacom->led.select[0] & 0x03;
+		int ring_led = wacom->led.groups[0].select & 0x03;
 		int ring_lum = (((wacom->led.llv & 0x60) >> 5) - 1) & 0x03;
 		int crop_lum = 0;
 		unsigned char led_bits = (crop_lum << 4) | (ring_lum << 2) | (ring_led);
@@ -677,11 +680,11 @@ static int wacom_led_control(struct wacom *wacom)
 			buf[1] = led_bits;
 	}
 	else {
-		int led = wacom->led.select[0] | 0x4;
+		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.select[1] << 4) | 0x40;
+			led |= (wacom->led.groups[1].select << 4) | 0x40;
 
 		buf[0] = report_id;
 		buf[1] = led;
@@ -753,7 +756,7 @@ static ssize_t wacom_led_select_store(struct device *dev, int set_id,
 
 	mutex_lock(&wacom->lock);
 
-	wacom->led.select[set_id] = id & 0x3;
+	wacom->led.groups[set_id].select = id & 0x3;
 	err = wacom_led_control(wacom);
 
 	mutex_unlock(&wacom->lock);
@@ -773,7 +776,7 @@ 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);			\
 	return scnprintf(buf, PAGE_SIZE, "%d\n",			\
-			 wacom->led.select[SET_ID]);			\
+			 wacom->led.groups[SET_ID].select);		\
 }									\
 static DEVICE_ATTR(status_led##SET_ID##_select, DEV_ATTR_RW_PERM,	\
 		    wacom_led##SET_ID##_select_show,			\
@@ -955,6 +958,35 @@ static int wacom_devm_sysfs_create_group(struct wacom *wacom,
 	return 0;
 }
 
+static void wacom_led_groups_release(void *data)
+{
+	struct wacom *wacom = data;
+
+	wacom->led.groups = NULL;
+}
+
+static int wacom_led_groups_allocate(struct wacom *wacom, int count)
+{
+	struct wacom_group_leds *groups;
+	int error;
+
+	groups = devm_kzalloc(&wacom->hdev->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);
+	if (error)
+		return error;
+
+	wacom->led.groups = groups;
+
+	return 0;
+}
+
 static int wacom_initialize_leds(struct wacom *wacom)
 {
 	int error;
@@ -968,23 +1000,34 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOS4:
 	case INTUOS4WL:
 	case INTUOS4L:
-		wacom->led.select[0] = 0;
-		wacom->led.select[1] = 0;
 		wacom->led.llv = 10;
 		wacom->led.hlv = 20;
 		wacom->led.img_lum = 10;
+
+		error = wacom_led_groups_allocate(wacom, 1);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+
 		error = wacom_devm_sysfs_create_group(wacom,
 						      &intuos4_led_attr_group);
 		break;
 
 	case WACOM_24HD:
 	case WACOM_21UX2:
-		wacom->led.select[0] = 0;
-		wacom->led.select[1] = 0;
 		wacom->led.llv = 0;
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
+		error = wacom_led_groups_allocate(wacom, 2);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+
 		error = wacom_devm_sysfs_create_group(wacom,
 						      &cintiq_led_attr_group);
 		break;
@@ -995,16 +1038,30 @@ static int wacom_initialize_leds(struct wacom *wacom)
 	case INTUOSPS:
 	case INTUOSPM:
 	case INTUOSPL:
-		wacom->led.select[0] = 0;
-		wacom->led.select[1] = 0;
 		wacom->led.llv = 32;
 		wacom->led.hlv = 0;
 		wacom->led.img_lum = 0;
 
+		error = wacom_led_groups_allocate(wacom, 1);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+
 		error = wacom_devm_sysfs_create_group(wacom,
 						      &intuos5_led_attr_group);
 		break;
 
+	case REMOTE:
+		error = wacom_led_groups_allocate(wacom, 5);
+		if (error) {
+			hid_err(wacom->hdev,
+				"cannot create leds err: %d\n", error);
+			return error;
+		}
+		return 0;
+
 	default:
 		return 0;
 	}
@@ -1204,7 +1261,7 @@ static ssize_t wacom_show_remote_mode(struct kobject *kobj,
 	struct wacom *wacom = hid_get_drvdata(hdev);
 	u8 mode;
 
-	mode = wacom->led.select[index];
+	mode = wacom->led.groups[index].select;
 	if (mode >= 0 && mode < 3)
 		return snprintf(buf, PAGE_SIZE, "%d\n", mode);
 	else
@@ -1272,7 +1329,7 @@ void wacom_remote_destroy_attr_group(struct wacom *wacom, __u32 serial)
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		if (wacom_wac->serial[i] == serial) {
 			wacom_wac->serial[i] = 0;
-			wacom->led.select[i] = WACOM_STATUS_UNKNOWN;
+			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 			if (wacom->remote_group[i].name) {
 				sysfs_remove_group(wacom->remote_dir,
 						   &wacom->remote_group[i]);
@@ -1369,7 +1426,7 @@ static int wacom_initialize_remote(struct wacom *wacom)
 	}
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		wacom->led.select[i] = WACOM_STATUS_UNKNOWN;
+		wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 		wacom_wac->serial[i] = 0;
 	}
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 3e2c903..932d3ee 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -808,7 +808,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		if (wacom_wac->serial[i] == serial)
-			wacom->led.select[i] = touch_ring_mode;
+			wacom->led.groups[i].select = touch_ring_mode;
 	}
 
 	if (!wacom->battery &&
-- 
2.5.5

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


#1442584 — [PATCH v2 28/30] HID: wacom: power_supply: mark the type as USB

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 28/30] HID: wacom: power_supply: mark the type as USB
Message-ID<rUv3c-5rv-9@gated-at.bofh.it>
In reply to#1442576
When upowerd detects a new device, it tries to map this new device to
an input to guess its kind. It works OK for wired tablets when the
wireless module and its battery are attached, but not so well when
connected over wireless.
In that case, the battery is attached to the wireless HID node, not
the Pen or Pad HID node. So there is no input node as a parent of the
reported battery, which means it will be showed as a computer battery
in gnome-control-center.

If we set the power supply type to USB, upowerd has a heuristic that
detects "wacom_" in the name of the power_supply, and set the type to
tablet. So it's now clear that the reported battery of from a tablet.
(see https://cgit.freedesktop.org/upower/tree/src/linux/up-device-supply.c)

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

New in v2
---
 drivers/hid/wacom_sys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 7e83352..792708b 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1429,7 +1429,7 @@ static int __wacom_initialize_battery(struct wacom *wacom,
 	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->type = POWER_SUPPLY_TYPE_USB;
 	bat_desc->use_for_apm = 0;
 
 	ac_desc->properties = wacom_ac_props;
-- 
2.5.5

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


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

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 23/30] HID: wacom: leds: use the ledclass instead of custom made sysfs files
Message-ID<rUv3b-5rv-3@gated-at.bofh.it>
In reply to#1442576
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>
---

Changes in v2:
- renamed the LED to be: intput123::wacom-0.1 (dropped the "led_")
- add LED_HW_PLUGGABLE flag
- make sure we do not call wacom_led_control() if the device has
  been removed
---
 Documentation/ABI/testing/sysfs-driver-wacom |   5 +
 drivers/hid/wacom.h                          |  19 +++
 drivers/hid/wacom_sys.c                      | 195 +++++++++++++++++++++++++--
 3 files changed, 209 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..c5d518d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -647,6 +647,9 @@ static int wacom_led_control(struct wacom *wacom)
 	unsigned char report_id = WAC_CMD_LED_CONTROL;
 	int buf_size = 9;
 
+	if (!hid_get_drvdata(wacom->hdev))
+		return -ENODEV;
+
 	if (!wacom->led.groups)
 		return -ENOTSUPP;
 
@@ -966,31 +969,194 @@ 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-%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.flags = LED_HW_PLUGGABLE;
+	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 +1176,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 +1197,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 +1215,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 +1229,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 +2156,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]


#1442587 — [PATCH v2 24/30] HID: wacom: leds: actually release the LEDs on disconnect

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 24/30] HID: wacom: leds: actually release the LEDs on disconnect
Message-ID<rUv3c-5rv-21@gated-at.bofh.it>
In reply to#1442576
There is a bug (?) in devm_led_classdev_register() in which its increments
the refcount of the parent. If the parent is an input device, that means
the ref count never reaches 0 when devm_input_device_release() gets called.
This means that the LEDs and all the devres resources attached to the
input device are not released.

Manually force the release of the group so that the leds are released once
we are done using them.

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

No changes in v2
---
 drivers/hid/wacom.h     |  1 +
 drivers/hid/wacom_sys.c | 27 ++++++++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 0c498c4..9159dd3 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -130,6 +130,7 @@ struct wacom_group_leds {
 	u8 select; /* status led selector (0..3) */
 	struct wacom_led *leds;
 	unsigned int count;
+	struct device *dev;
 };
 
 struct wacom_battery {
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index c5d518d..3a651e2 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1068,6 +1068,13 @@ static int wacom_led_register_one(struct device *dev, struct wacom *wacom,
 	return 0;
 }
 
+static void wacom_led_groups_release_one(void *data)
+{
+	struct wacom_group_leds *group = data;
+
+	devres_release_group(group->dev, group);
+}
+
 static int wacom_led_groups_alloc_and_register_one(struct device *dev,
 						   struct wacom *wacom,
 						   int group_id, int count,
@@ -1098,7 +1105,25 @@ static int wacom_led_groups_alloc_and_register_one(struct device *dev,
 			goto err;
 	}
 
-	devres_remove_group(dev, &wacom->led.groups[group_id]);
+	wacom->led.groups[group_id].dev = dev;
+
+	devres_close_group(dev, &wacom->led.groups[group_id]);
+
+	/*
+	 * There is a bug (?) in devm_led_classdev_register() in which its
+	 * increments the refcount of the parent. If the parent is an input
+	 * device, that means the ref count never reaches 0 when
+	 * devm_input_device_release() gets called.
+	 * This means that the LEDs are still there after disconnect.
+	 * Manually force the release of the group so that the leds are released
+	 * once we are done using them.
+	 */
+	error = devm_add_action_or_reset(&wacom->hdev->dev,
+					 wacom_led_groups_release_one,
+					 &wacom->led.groups[group_id]);
+	if (error)
+		return error;
+
 	return 0;
 
 err:
-- 
2.5.5

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


#1442589 — [PATCH v2 22/30] HID: wacom: EKR: attach the power_supply on first connection

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 22/30] HID: wacom: EKR: attach the power_supply on first connection
Message-ID<rUv3c-5rv-25@gated-at.bofh.it>
In reply to#1442576
Or Gnome complains about an empty battery.

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

No changes in v2
---
 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]


#1442590 — [PATCH v2 30/30] HID: wacom: power_supply: provide the actual model_name

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 30/30] HID: wacom: power_supply: provide the actual model_name
Message-ID<rUv3c-5rv-15@gated-at.bofh.it>
In reply to#1442576
Instead of displaying a generic "tablet", now g-c-c shows a pretty
"Wacom Intuos Pro S (WL)".

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

New in v2
---
 drivers/hid/wacom.h     |  1 +
 drivers/hid/wacom_sys.c | 11 +++++++++++
 drivers/hid/wacom_wac.h |  1 +
 3 files changed, 13 insertions(+)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 26a8a05..b4800ea 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -134,6 +134,7 @@ struct wacom_group_leds {
 };
 
 struct wacom_battery {
+	struct wacom *wacom;
 	struct power_supply_desc bat_desc;
 	struct power_supply *battery;
 	char bat_name[WACOM_NAME_MAX];
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index d8f3d3d..edde881 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1336,6 +1336,7 @@ static int wacom_initialize_leds(struct wacom *wacom)
 }
 
 static enum power_supply_property wacom_battery_props[] = {
+	POWER_SUPPLY_PROP_MODEL_NAME,
 	POWER_SUPPLY_PROP_PRESENT,
 	POWER_SUPPLY_PROP_STATUS,
 	POWER_SUPPLY_PROP_SCOPE,
@@ -1350,6 +1351,9 @@ static int wacom_battery_get_property(struct power_supply *psy,
 	int ret = 0;
 
 	switch (psp) {
+		case POWER_SUPPLY_PROP_MODEL_NAME:
+			val->strval = battery->wacom->wacom_wac.name;
+			break;
 		case POWER_SUPPLY_PROP_PRESENT:
 			val->intval = battery->bat_connected;
 			break;
@@ -1392,6 +1396,8 @@ static int __wacom_initialize_battery(struct wacom *wacom,
 	if (!devres_open_group(dev, bat_desc, GFP_KERNEL))
 		return -ENOMEM;
 
+	battery->wacom = wacom;
+
 	n = atomic_inc_return(&battery_no) - 1;
 
 	bat_desc->properties = wacom_battery_props;
@@ -1863,6 +1869,9 @@ static void wacom_update_name(struct wacom *wacom, const char *suffix)
 		strlcpy(name, features->name, sizeof(name));
 	}
 
+	snprintf(wacom_wac->name, sizeof(wacom_wac->name), "%s%s",
+		 name, suffix);
+
 	/* Append the device type to the name */
 	snprintf(wacom_wac->pen_name, sizeof(wacom_wac->pen_name),
 		"%s%s Pen", name, suffix);
@@ -2097,6 +2106,8 @@ static void wacom_wireless_work(struct work_struct *work)
 				goto fail;
 		}
 
+		strlcpy(wacom_wac->name, wacom_wac1->name,
+			sizeof(wacom_wac->name));
 		error = wacom_initialize_battery(wacom);
 		if (error)
 			goto fail;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 8a8974c..745e2c9 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -226,6 +226,7 @@ struct wacom_remote_data {
 };
 
 struct wacom_wac {
+	char name[WACOM_NAME_MAX];
 	char pen_name[WACOM_NAME_MAX];
 	char touch_name[WACOM_NAME_MAX];
 	char pad_name[WACOM_NAME_MAX];
-- 
2.5.5

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


#1442591 — [PATCH v2 05/30] HID: wacom: use one work queue per task

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 05/30] HID: wacom: use one work queue per task
Message-ID<rUv3c-5rv-11@gated-at.bofh.it>
In reply to#1442576
Looks like the battery hijacked the wireless worker. That's not fair so
use a work queue per task.

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

No changes in v2
---
 drivers/hid/wacom.h     | 21 ++++++++++++++++++---
 drivers/hid/wacom_sys.c | 10 ++++++----
 drivers/hid/wacom_wac.c | 13 +++++--------
 3 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 4681a65..a968fbb 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -105,13 +105,19 @@
 #define USB_VENDOR_ID_WACOM	0x056a
 #define USB_VENDOR_ID_LENOVO	0x17ef
 
+enum wacom_worker {
+	WACOM_WORKER_WIRELESS,
+	WACOM_WORKER_BATTERY,
+};
+
 struct wacom {
 	struct usb_device *usbdev;
 	struct usb_interface *intf;
 	struct wacom_wac wacom_wac;
 	struct hid_device *hdev;
 	struct mutex lock;
-	struct work_struct work;
+	struct work_struct wireless_work;
+	struct work_struct battery_work;
 	struct wacom_led {
 		u8 select[5]; /* status led selector (0..3) */
 		u8 llv;       /* status led brightness no button (1..127) */
@@ -127,10 +133,19 @@ struct wacom {
 	struct attribute_group remote_group[5];
 };
 
-static inline void wacom_schedule_work(struct wacom_wac *wacom_wac)
+static inline void wacom_schedule_work(struct wacom_wac *wacom_wac,
+				       enum wacom_worker which)
 {
 	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
-	schedule_work(&wacom->work);
+
+	switch (which) {
+	case WACOM_WORKER_WIRELESS:
+		schedule_work(&wacom->wireless_work);
+		break;
+	case WACOM_WORKER_BATTERY:
+		schedule_work(&wacom->battery_work);
+		break;
+	}
 }
 
 extern const struct hid_device_id wacom_ids[];
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index af2801d..54f0260 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1527,7 +1527,7 @@ static void wacom_calculate_res(struct wacom_features *features)
 
 void wacom_battery_work(struct work_struct *work)
 {
-	struct wacom *wacom = container_of(work, struct wacom, work);
+	struct wacom *wacom = container_of(work, struct wacom, battery_work);
 
 	if ((wacom->wacom_wac.features.quirks & WACOM_QUIRK_BATTERY) &&
 	     !wacom->battery) {
@@ -1743,7 +1743,7 @@ fail_allocate_inputs:
 
 static void wacom_wireless_work(struct work_struct *work)
 {
-	struct wacom *wacom = container_of(work, struct wacom, work);
+	struct wacom *wacom = container_of(work, struct wacom, wireless_work);
 	struct usb_device *usbdev = wacom->usbdev;
 	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
 	struct hid_device *hdev1, *hdev2;
@@ -1871,7 +1871,8 @@ static int wacom_probe(struct hid_device *hdev,
 	wacom->usbdev = dev;
 	wacom->intf = intf;
 	mutex_init(&wacom->lock);
-	INIT_WORK(&wacom->work, wacom_wireless_work);
+	INIT_WORK(&wacom->wireless_work, wacom_wireless_work);
+	INIT_WORK(&wacom->battery_work, wacom_battery_work);
 
 	/* ask for the report descriptor to be loaded by HID */
 	error = hid_parse(hdev);
@@ -1912,7 +1913,8 @@ static void wacom_remove(struct hid_device *hdev)
 
 	hid_hw_stop(hdev);
 
-	cancel_work_sync(&wacom->work);
+	cancel_work_sync(&wacom->wireless_work);
+	cancel_work_sync(&wacom->battery_work);
 	kobject_put(wacom->remote_dir);
 	wacom_destroy_leds(wacom);
 	wacom_clean_inputs(wacom);
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index d6746ca..c7707b2 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -814,8 +814,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 	if (!wacom->battery &&
 	    !(features->quirks & WACOM_QUIRK_BATTERY)) {
 		features->quirks |= WACOM_QUIRK_BATTERY;
-		INIT_WORK(&wacom->work, wacom_battery_work);
-		wacom_schedule_work(wacom_wac);
+		wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
 	}
 
 	wacom_notify_battery(wacom_wac, bat_percent, bat_charging, 1,
@@ -2150,7 +2149,7 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
 		charging = !!(data[5] & 0x80);
 		if (wacom->pid != pid) {
 			wacom->pid = pid;
-			wacom_schedule_work(wacom);
+			wacom_schedule_work(wacom, WACOM_WORKER_WIRELESS);
 		}
 
 		if (w->battery)
@@ -2159,7 +2158,7 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
 	} else if (wacom->pid != 0) {
 		/* disconnected while previously connected */
 		wacom->pid = 0;
-		wacom_schedule_work(wacom);
+		wacom_schedule_work(wacom, WACOM_WORKER_WIRELESS);
 		wacom_notify_battery(wacom, 0, 0, 0, 0);
 	}
 
@@ -2194,15 +2193,13 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
 		if (!wacom->battery &&
 		    !(features->quirks & WACOM_QUIRK_BATTERY)) {
 			features->quirks |= WACOM_QUIRK_BATTERY;
-			INIT_WORK(&wacom->work, wacom_battery_work);
-			wacom_schedule_work(wacom_wac);
+			wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
 		}
 	}
 	else if ((features->quirks & WACOM_QUIRK_BATTERY) &&
 		 wacom->battery) {
 		features->quirks &= ~WACOM_QUIRK_BATTERY;
-		INIT_WORK(&wacom->work, wacom_battery_work);
-		wacom_schedule_work(wacom_wac);
+		wacom_schedule_work(wacom_wac, WACOM_WORKER_BATTERY);
 		wacom_notify_battery(wacom_wac, 0, 0, 0, 0);
 	}
 	return 0;
-- 
2.5.5

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


#1442592 — [PATCH v2 18/30] HID: wacom: EKR: use devres groups to manage resources

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 18/30] HID: wacom: EKR: use devres groups to manage resources
Message-ID<rUv3c-5rv-23@gated-at.bofh.it>
In reply to#1442576
This will be useful when each remote will be assigned its own input device.
We won't need to unregister each input and sysfs and other elements one
at a time.

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

No changes in v2
---
 drivers/hid/wacom_sys.c | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 27ca147..9121e72 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -934,8 +934,9 @@ static void wacom_devm_sysfs_group_release(struct device *dev, void *res)
 	sysfs_remove_group(kobj, devres->group);
 }
 
-static int wacom_devm_sysfs_create_group(struct wacom *wacom,
-					 struct attribute_group *group)
+static int __wacom_devm_sysfs_create_group(struct wacom *wacom,
+					   struct kobject *root,
+					   struct attribute_group *group)
 {
 	struct wacom_sysfs_group_devres *devres;
 	int error;
@@ -947,7 +948,7 @@ static int wacom_devm_sysfs_create_group(struct wacom *wacom,
 		return -ENOMEM;
 
 	devres->group = group;
-	devres->root = &wacom->hdev->dev.kobj;
+	devres->root = root;
 
 	error = sysfs_create_group(devres->root, group);
 	if (error)
@@ -958,6 +959,13 @@ static int wacom_devm_sysfs_create_group(struct wacom *wacom,
 	return 0;
 }
 
+static int wacom_devm_sysfs_create_group(struct wacom *wacom,
+					 struct attribute_group *group)
+{
+	return __wacom_devm_sysfs_create_group(wacom, &wacom->hdev->dev.kobj,
+					       group);
+}
+
 static void wacom_led_groups_release(void *data)
 {
 	struct wacom *wacom = data;
@@ -1306,9 +1314,10 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 	if (!remote->remote_group[index].name)
 		return -ENOMEM;
 
-	error = sysfs_create_group(remote->remote_dir,
-				   &remote->remote_group[index]);
+	error = __wacom_devm_sysfs_create_group(wacom, remote->remote_dir,
+						&remote->remote_group[index]);
 	if (error) {
+		remote->remote_group[index].name = NULL;
 		hid_err(wacom->hdev,
 			"cannot create sysfs group err: %d\n", error);
 		return error;
@@ -1317,15 +1326,6 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 	return 0;
 }
 
-static void wacom_remote_destroy_attr_group(struct wacom *wacom, unsigned int i)
-{
-	struct wacom_remote *remote = wacom->remote;
-
-	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)
 {
 	const size_t buf_size = 2;
@@ -1906,11 +1906,13 @@ static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
 	u32 serial = remote->serial[index];
 	int i;
 
-	wacom_remote_destroy_attr_group(wacom, index);
+	if (remote->remote_group[index].name)
+		devres_release_group(&wacom->hdev->dev, &remote->serial[index]);
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		if (remote->serial[i] == serial) {
 			remote->serial[i] = 0;
+			remote->remote_group[i].name = NULL;
 			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 		}
 	}
@@ -1920,6 +1922,7 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 				   unsigned int index)
 {
 	struct wacom_remote *remote = wacom->remote;
+	struct device *dev = &wacom->hdev->dev;
 	int error, k;
 
 	/* A remote can pair more than once with an EKR,
@@ -1935,13 +1938,22 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 		return 0;
 	}
 
+	if (!devres_open_group(dev, &remote->serial[index], GFP_KERNEL))
+		return -ENOMEM;
+
 	error = wacom_remote_create_attr_group(wacom, serial, index);
 	if (error)
-		return error;
+		goto fail;
 
 	remote->serial[index] = serial;
 
+	devres_close_group(dev, &remote->serial[index]);
 	return 0;
+
+fail:
+	devres_release_group(dev, &remote->serial[index]);
+	remote->serial[index] = 0;
+	return error;
 }
 
 static void wacom_remote_work(struct work_struct *work)
-- 
2.5.5

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


#1442593 — [PATCH v2 15/30] HID: wacom: EKR: have the wacom resources dynamically allocated

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 15/30] HID: wacom: EKR: have the wacom resources dynamically allocated
Message-ID<rUv3c-5rv-29@gated-at.bofh.it>
In reply to#1442576
If we want to have one input device per remote, it's better to have our
own struct wacom_remote which is dynamically allocated.

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

Changes in v2:
- fixed spin_unlock_irqrestore() according to previous patch
---
 drivers/hid/wacom.h     |  13 +++--
 drivers/hid/wacom_sys.c | 133 ++++++++++++++++++++++++++++--------------------
 drivers/hid/wacom_wac.c |  12 +++--
 drivers/hid/wacom_wac.h |   2 +-
 4 files changed, 94 insertions(+), 66 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 1797e4b..110ea67 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -116,6 +116,14 @@ struct wacom_group_leds {
 	u8 select; /* status led selector (0..3) */
 };
 
+struct wacom_remote {
+	spinlock_t remote_lock;
+	struct kfifo remote_fifo;
+	struct kobject *remote_dir;
+	struct attribute_group remote_group[WACOM_MAX_REMOTES];
+	__u32 serial[WACOM_MAX_REMOTES];
+};
+
 struct wacom {
 	struct usb_device *usbdev;
 	struct usb_interface *intf;
@@ -125,8 +133,7 @@ struct wacom {
 	struct work_struct wireless_work;
 	struct work_struct battery_work;
 	struct work_struct remote_work;
-	spinlock_t remote_lock;
-	struct kfifo remote_fifo;
+	struct wacom_remote *remote;
 	struct wacom_leds {
 		struct wacom_group_leds *groups;
 		u8 llv;       /* status led brightness no button (1..127) */
@@ -137,8 +144,6 @@ struct wacom {
 	struct power_supply *ac;
 	struct power_supply_desc battery_desc;
 	struct power_supply_desc ac_desc;
-	struct kobject *remote_dir;
-	struct attribute_group remote_group[5];
 	bool resources;
 };
 
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 7e01117..64acb71 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1298,18 +1298,18 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 					  int index)
 {
 	int error = 0;
-	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct wacom_remote *remote = wacom->remote;
 
-	wacom_wac->serial[index] = serial;
+	remote->serial[index] = serial;
 
-	wacom->remote_group[index].name = devm_kasprintf(&wacom->hdev->dev,
-							 GFP_KERNEL,
-							 "%d", serial);
-	if (!wacom->remote_group[index].name)
+	remote->remote_group[index].name = devm_kasprintf(&wacom->hdev->dev,
+							  GFP_KERNEL,
+							  "%d", serial);
+	if (!remote->remote_group[index].name)
 		return -ENOMEM;
 
-	error = sysfs_create_group(wacom->remote_dir,
-				   &wacom->remote_group[index]);
+	error = sysfs_create_group(remote->remote_dir,
+				   &remote->remote_group[index]);
 	if (error) {
 		hid_err(wacom->hdev,
 			"cannot create sysfs group err: %d\n", error);
@@ -1321,22 +1321,22 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 
 static void wacom_remote_destroy_attr_group(struct wacom *wacom, __u32 serial)
 {
-	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct wacom_remote *remote = wacom->remote;
 	int i;
 
 	if (!serial)
 		return;
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		if (wacom_wac->serial[i] == serial) {
-			wacom_wac->serial[i] = 0;
+		if (remote->serial[i] == serial) {
+			remote->serial[i] = 0;
 			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
-			if (wacom->remote_group[i].name) {
-				sysfs_remove_group(wacom->remote_dir,
-						   &wacom->remote_group[i]);
+			if (remote->remote_group[i].name) {
+				sysfs_remove_group(remote->remote_dir,
+						   &remote->remote_group[i]);
 				devm_kfree(&wacom->hdev->dev,
-					   (char *)wacom->remote_group[i].name);
-				wacom->remote_group[i].name = NULL;
+					  (char *)remote->remote_group[i].name);
+				remote->remote_group[i].name = NULL;
 			}
 		}
 	}
@@ -1398,27 +1398,57 @@ static const struct attribute *remote_unpair_attrs[] = {
 	NULL
 };
 
-static int wacom_initialize_remote(struct wacom *wacom)
+static void wacom_remotes_destroy(void *data)
+{
+	struct wacom *wacom = data;
+	struct wacom_remote *remote = wacom->remote;
+
+	if (!remote)
+		return;
+
+	kobject_put(remote->remote_dir);
+	kfifo_free(&remote->remote_fifo);
+	wacom->remote = NULL;
+}
+
+static int wacom_initialize_remotes(struct wacom *wacom)
 {
 	int error = 0;
-	struct wacom_wac *wacom_wac = &(wacom->wacom_wac);
+	struct wacom_remote *remote;
 	int i;
 
 	if (wacom->wacom_wac.features.type != REMOTE)
 		return 0;
 
-	wacom->remote_group[0] = remote0_serial_group;
-	wacom->remote_group[1] = remote1_serial_group;
-	wacom->remote_group[2] = remote2_serial_group;
-	wacom->remote_group[3] = remote3_serial_group;
-	wacom->remote_group[4] = remote4_serial_group;
+	remote = devm_kzalloc(&wacom->hdev->dev, sizeof(*wacom->remote),
+			      GFP_KERNEL);
+	if (!remote)
+		return -ENOMEM;
 
-	wacom->remote_dir = kobject_create_and_add("wacom_remote",
-						   &wacom->hdev->dev.kobj);
-	if (!wacom->remote_dir)
+	wacom->remote = remote;
+
+	spin_lock_init(&remote->remote_lock);
+
+	error = kfifo_alloc(&remote->remote_fifo,
+			5 * sizeof(struct wacom_remote_data),
+			GFP_KERNEL);
+	if (error) {
+		hid_err(wacom->hdev, "failed allocating remote_fifo\n");
 		return -ENOMEM;
+	}
 
-	error = sysfs_create_files(wacom->remote_dir, remote_unpair_attrs);
+	remote->remote_group[0] = remote0_serial_group;
+	remote->remote_group[1] = remote1_serial_group;
+	remote->remote_group[2] = remote2_serial_group;
+	remote->remote_group[3] = remote3_serial_group;
+	remote->remote_group[4] = remote4_serial_group;
+
+	remote->remote_dir = kobject_create_and_add("wacom_remote",
+						    &wacom->hdev->dev.kobj);
+	if (!remote->remote_dir)
+		return -ENOMEM;
+
+	error = sysfs_create_files(remote->remote_dir, remote_unpair_attrs);
 
 	if (error) {
 		hid_err(wacom->hdev,
@@ -1428,9 +1458,14 @@ static int wacom_initialize_remote(struct wacom *wacom)
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
-		wacom_wac->serial[i] = 0;
+		remote->serial[i] = 0;
 	}
 
+	error = devm_add_action_or_reset(&wacom->hdev->dev,
+					 wacom_remotes_destroy, wacom);
+	if (error)
+		return error;
+
 	return 0;
 }
 
@@ -1740,7 +1775,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		if (error)
 			goto fail_leds;
 
-		error = wacom_initialize_remote(wacom);
+		error = wacom_initialize_remotes(wacom);
 		if (error)
 			goto fail_remote;
 	}
@@ -1792,7 +1827,6 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 fail_quirks:
 	hid_hw_stop(hdev);
 fail_hw_start:
-	kobject_put(wacom->remote_dir);
 fail_remote:
 fail_leds:
 fail_register_inputs:
@@ -1893,58 +1927,58 @@ fail:
 static void wacom_remote_work(struct work_struct *work)
 {
 	struct wacom *wacom = container_of(work, struct wacom, remote_work);
-	struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+	struct wacom_remote *remote = wacom->remote;
 	struct wacom_remote_data data;
 	unsigned long flags;
 	unsigned int count;
 	u32 serial;
 	int i, k;
 
-	spin_lock_irqsave(&wacom->remote_lock, flags);
+	spin_lock_irqsave(&remote->remote_lock, flags);
 
-	count = kfifo_out(&wacom->remote_fifo, &data, sizeof(data));
+	count = kfifo_out(&remote->remote_fifo, &data, sizeof(data));
 
 	if (count != sizeof(data)) {
 		hid_err(wacom->hdev,
 			"workitem triggered without status available\n");
-		spin_unlock_irqrestore(&wacom->remote_lock, flags);
+		spin_unlock_irqrestore(&remote->remote_lock, flags);
 		return;
 	}
 
-	if (!kfifo_is_empty(&wacom->remote_fifo))
+	if (!kfifo_is_empty(&remote->remote_fifo))
 		wacom_schedule_work(&wacom->wacom_wac, WACOM_WORKER_REMOTE);
 
-	spin_unlock_irqrestore(&wacom->remote_lock, flags);
+	spin_unlock_irqrestore(&remote->remote_lock, flags);
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		serial = data.remote[i].serial;
 		if (data.remote[i].connected) {
 
-			if (wacom_wac->serial[i] == serial)
+			if (remote->serial[i] == serial)
 				continue;
 
-			if (wacom_wac->serial[i]) {
+			if (remote->serial[i]) {
 				wacom_remote_destroy_attr_group(wacom,
-							wacom_wac->serial[i]);
+							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 (wacom_wac->serial[k] == serial)
+				if (remote->serial[k] == serial)
 					break;
 			}
 
 			if (k < WACOM_MAX_REMOTES) {
-				wacom_wac->serial[i] = serial;
+				remote->serial[i] = serial;
 				continue;
 			}
 			wacom_remote_create_attr_group(wacom, serial, i);
 
-		} else if (wacom_wac->serial[i]) {
+		} else if (remote->serial[i]) {
 			wacom_remote_destroy_attr_group(wacom,
-							wacom_wac->serial[i]);
+							remote->serial[i]);
 		}
 	}
 }
@@ -1992,16 +2026,6 @@ static int wacom_probe(struct hid_device *hdev,
 	INIT_WORK(&wacom->wireless_work, wacom_wireless_work);
 	INIT_WORK(&wacom->battery_work, wacom_battery_work);
 	INIT_WORK(&wacom->remote_work, wacom_remote_work);
-	spin_lock_init(&wacom->remote_lock);
-
-	if (kfifo_alloc(&wacom->remote_fifo,
-			5 * sizeof(struct wacom_remote_data),
-			GFP_KERNEL)) {
-		dev_err(&hdev->dev,
-			"%s:failed allocating remote_fifo\n", __func__);
-		error = -ENOMEM;
-		goto fail_type;
-	}
 
 	/* ask for the report descriptor to be loaded by HID */
 	error = hid_parse(hdev);
@@ -2025,7 +2049,6 @@ static int wacom_probe(struct hid_device *hdev,
 	return 0;
 
 fail_parse:
-	kfifo_free(&wacom->remote_fifo);
 fail_type:
 	hid_set_drvdata(hdev, NULL);
 	return error;
@@ -2045,8 +2068,6 @@ static void wacom_remove(struct hid_device *hdev)
 	cancel_work_sync(&wacom->wireless_work);
 	cancel_work_sync(&wacom->battery_work);
 	cancel_work_sync(&wacom->remote_work);
-	kfifo_free(&wacom->remote_fifo);
-	kobject_put(wacom->remote_dir);
 	if (hdev->bus == BUS_BLUETOOTH)
 		device_remove_file(&hdev->dev, &dev_attr_speed);
 
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index d549929..ce1089f 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -753,6 +753,7 @@ 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 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;
@@ -807,7 +808,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 	touch_ring_mode = (data[11] & 0xC0) >> 6;
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		if (wacom_wac->serial[i] == serial)
+		if (remote->serial[i] == serial)
 			wacom->led.groups[i].select = touch_ring_mode;
 	}
 
@@ -827,6 +828,7 @@ static void wacom_remote_status_irq(struct wacom_wac *wacom_wac, size_t len)
 {
 	struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
 	unsigned char *data = wacom_wac->data;
+	struct wacom_remote *remote = wacom->remote;
 	struct wacom_remote_data remote_data;
 	unsigned long flags;
 	int i, ret;
@@ -845,16 +847,16 @@ static void wacom_remote_status_irq(struct wacom_wac *wacom_wac, size_t len)
 		remote_data.remote[i].connected = connected;
 	}
 
-	spin_lock_irqsave(&wacom->remote_lock, flags);
+	spin_lock_irqsave(&remote->remote_lock, flags);
 
-	ret = kfifo_in(&wacom->remote_fifo, &remote_data, sizeof(remote_data));
+	ret = kfifo_in(&remote->remote_fifo, &remote_data, sizeof(remote_data));
 	if (ret != sizeof(remote_data)) {
-		spin_unlock_irqrestore(&wacom->remote_lock, flags);
+		spin_unlock_irqrestore(&remote->remote_lock, flags);
 		hid_err(wacom->hdev, "Can't queue Remote status event.\n");
 		return;
 	}
 
-	spin_unlock_irqrestore(&wacom->remote_lock, flags);
+	spin_unlock_irqrestore(&remote->remote_lock, flags);
 
 	wacom_schedule_work(wacom_wac, WACOM_WORKER_REMOTE);
 }
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index c2c65b8..6be6cae 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -234,7 +234,7 @@ struct wacom_wac {
 	unsigned char data[WACOM_PKGLEN_MAX];
 	int tool[2];
 	int id[2];
-	__u32 serial[5];
+	__u32 serial[2];
 	bool reporting_data;
 	struct wacom_features features;
 	struct wacom_shared *shared;
-- 
2.5.5

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


#1442595 — [PATCH v2 10/30] HID: wacom: use devm_kasprintf for allocating the name of the remote

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 10/30] HID: wacom: use devm_kasprintf for allocating the name of the remote
Message-ID<rUv3d-5rv-39@gated-at.bofh.it>
In reply to#1442576
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>
---

No changes in v2
---
 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]


#1442596 — [PATCH v2 19/30] HID: wacom: EKR: have one array of struct remotes instead of many arrays

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 19/30] HID: wacom: EKR: have one array of struct remotes instead of many arrays
Message-ID<rUv3c-5rv-35@gated-at.bofh.it>
In reply to#1442576
No functional changes, just a prep patch for the one after.

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

No changes in v2
---
 drivers/hid/wacom.h     |  6 ++++--
 drivers/hid/wacom_sys.c | 53 +++++++++++++++++++++++++------------------------
 drivers/hid/wacom_wac.c |  2 +-
 3 files changed, 32 insertions(+), 29 deletions(-)

diff --git a/drivers/hid/wacom.h b/drivers/hid/wacom.h
index 110ea67..6b8df67 100644
--- a/drivers/hid/wacom.h
+++ b/drivers/hid/wacom.h
@@ -120,8 +120,10 @@ struct wacom_remote {
 	spinlock_t remote_lock;
 	struct kfifo remote_fifo;
 	struct kobject *remote_dir;
-	struct attribute_group remote_group[WACOM_MAX_REMOTES];
-	__u32 serial[WACOM_MAX_REMOTES];
+	struct {
+		struct attribute_group group;
+		u32 serial;
+	} remotes[WACOM_MAX_REMOTES];
 };
 
 struct wacom {
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 9121e72..c3b2692 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1308,16 +1308,16 @@ static int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial,
 	int error = 0;
 	struct wacom_remote *remote = wacom->remote;
 
-	remote->remote_group[index].name = devm_kasprintf(&wacom->hdev->dev,
+	remote->remotes[index].group.name = devm_kasprintf(&wacom->hdev->dev,
 							  GFP_KERNEL,
 							  "%d", serial);
-	if (!remote->remote_group[index].name)
+	if (!remote->remotes[index].group.name)
 		return -ENOMEM;
 
 	error = __wacom_devm_sysfs_create_group(wacom, remote->remote_dir,
-						&remote->remote_group[index]);
+						&remote->remotes[index].group);
 	if (error) {
-		remote->remote_group[index].name = NULL;
+		remote->remotes[index].group.name = NULL;
 		hid_err(wacom->hdev,
 			"cannot create sysfs group err: %d\n", error);
 		return error;
@@ -1421,11 +1421,11 @@ static int wacom_initialize_remotes(struct wacom *wacom)
 		return -ENOMEM;
 	}
 
-	remote->remote_group[0] = remote0_serial_group;
-	remote->remote_group[1] = remote1_serial_group;
-	remote->remote_group[2] = remote2_serial_group;
-	remote->remote_group[3] = remote3_serial_group;
-	remote->remote_group[4] = remote4_serial_group;
+	remote->remotes[0].group = remote0_serial_group;
+	remote->remotes[1].group = remote1_serial_group;
+	remote->remotes[2].group = remote2_serial_group;
+	remote->remotes[3].group = remote3_serial_group;
+	remote->remotes[4].group = remote4_serial_group;
 
 	remote->remote_dir = kobject_create_and_add("wacom_remote",
 						    &wacom->hdev->dev.kobj);
@@ -1442,7 +1442,7 @@ static int wacom_initialize_remotes(struct wacom *wacom)
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
 		wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
-		remote->serial[i] = 0;
+		remote->remotes[i].serial = 0;
 	}
 
 	error = devm_add_action_or_reset(&wacom->hdev->dev,
@@ -1903,16 +1903,17 @@ fail:
 static void wacom_remote_destroy_one(struct wacom *wacom, unsigned int index)
 {
 	struct wacom_remote *remote = wacom->remote;
-	u32 serial = remote->serial[index];
+	u32 serial = remote->remotes[index].serial;
 	int i;
 
-	if (remote->remote_group[index].name)
-		devres_release_group(&wacom->hdev->dev, &remote->serial[index]);
+	if (remote->remotes[index].group.name)
+		devres_release_group(&wacom->hdev->dev,
+				     &remote->remotes[index]);
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		if (remote->serial[i] == serial) {
-			remote->serial[i] = 0;
-			remote->remote_group[i].name = NULL;
+		if (remote->remotes[i].serial == serial) {
+			remote->remotes[i].serial = 0;
+			remote->remotes[i].group.name = NULL;
 			wacom->led.groups[i].select = WACOM_STATUS_UNKNOWN;
 		}
 	}
@@ -1929,30 +1930,30 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,
 	 * check to make sure this serial isn't already paired.
 	 */
 	for (k = 0; k < WACOM_MAX_REMOTES; k++) {
-		if (remote->serial[k] == serial)
+		if (remote->remotes[k].serial == serial)
 			break;
 	}
 
 	if (k < WACOM_MAX_REMOTES) {
-		remote->serial[index] = serial;
+		remote->remotes[index].serial = serial;
 		return 0;
 	}
 
-	if (!devres_open_group(dev, &remote->serial[index], GFP_KERNEL))
+	if (!devres_open_group(dev, &remote->remotes[index], GFP_KERNEL))
 		return -ENOMEM;
 
 	error = wacom_remote_create_attr_group(wacom, serial, index);
 	if (error)
 		goto fail;
 
-	remote->serial[index] = serial;
+	remote->remotes[index].serial = serial;
 
-	devres_close_group(dev, &remote->serial[index]);
+	devres_close_group(dev, &remote->remotes[index]);
 	return 0;
 
 fail:
-	devres_release_group(dev, &remote->serial[index]);
-	remote->serial[index] = 0;
+	devres_release_group(dev, &remote->remotes[index]);
+	remote->remotes[index].serial = 0;
 	return error;
 }
 
@@ -1986,15 +1987,15 @@ static void wacom_remote_work(struct work_struct *work)
 		serial = data.remote[i].serial;
 		if (data.remote[i].connected) {
 
-			if (remote->serial[i] == serial)
+			if (remote->remotes[i].serial == serial)
 				continue;
 
-			if (remote->serial[i])
+			if (remote->remotes[i].serial)
 				wacom_remote_destroy_one(wacom, i);
 
 			wacom_remote_create_one(wacom, serial, i);
 
-		} else if (remote->serial[i]) {
+		} else if (remote->remotes[i].serial) {
 			wacom_remote_destroy_one(wacom, i);
 		}
 	}
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index ce1089f..aee0761 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -808,7 +808,7 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)
 	touch_ring_mode = (data[11] & 0xC0) >> 6;
 
 	for (i = 0; i < WACOM_MAX_REMOTES; i++) {
-		if (remote->serial[i] == serial)
+		if (remote->remotes[i].serial == serial)
 			wacom->led.groups[i].select = touch_ring_mode;
 	}
 
-- 
2.5.5

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


#1442597 — [PATCH v2 04/30] HID: wacom: untie leds from inputs

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 04/30] HID: wacom: untie leds from inputs
Message-ID<rUv3d-5rv-45@gated-at.bofh.it>
In reply to#1442576
Like remotes, LEDs should be handled by themself, not magically behind
the inputs as they have a complete different life.

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

No changes in v2
---
 drivers/hid/wacom_sys.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 5dd2640..af2801d 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -1403,7 +1403,6 @@ static void wacom_clean_inputs(struct wacom *wacom)
 	wacom->wacom_wac.pen_registered = false;
 	wacom->wacom_wac.touch_registered = false;
 	wacom->wacom_wac.pad_registered = false;
-	wacom_destroy_leds(wacom);
 }
 
 static int wacom_allocate_inputs(struct wacom *wacom)
@@ -1475,18 +1474,10 @@ static int wacom_register_inputs(struct wacom *wacom)
 		if (error)
 			goto fail_register_pad_input;
 		wacom_wac->pad_registered = true;
-
-		error = wacom_initialize_leds(wacom);
-		if (error)
-			goto fail_leds;
 	}
 
 	return 0;
 
-fail_leds:
-	input_unregister_device(pad_input_dev);
-	pad_input_dev = NULL;
-	wacom_wac->pad_registered = false;
 fail_register_pad_input:
 	if (touch_input_dev)
 		input_unregister_device(touch_input_dev);
@@ -1680,6 +1671,10 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
 		goto fail_register_inputs;
 
 	if (wacom->wacom_wac.features.device_type & WACOM_DEVICETYPE_PAD) {
+		error = wacom_initialize_leds(wacom);
+		if (error)
+			goto fail_leds;
+
 		error = wacom_initialize_remote(wacom);
 		if (error)
 			goto fail_remote;
@@ -1732,6 +1727,8 @@ fail_quirks:
 fail_hw_start:
 	kobject_put(wacom->remote_dir);
 fail_remote:
+	wacom_destroy_leds(wacom);
+fail_leds:
 	wacom_clean_inputs(wacom);
 fail_register_inputs:
 	wacom_destroy_battery(wacom);
@@ -1765,12 +1762,14 @@ 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_clean_inputs(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);
 
 	if (wacom_wac->pid == 0) {
@@ -1825,7 +1824,9 @@ static void wacom_wireless_work(struct work_struct *work)
 	return;
 
 fail:
+	wacom_destroy_leds(wacom1);
 	wacom_clean_inputs(wacom1);
+	wacom_destroy_leds(wacom2);
 	wacom_clean_inputs(wacom2);
 	return;
 }
@@ -1913,6 +1914,7 @@ static void wacom_remove(struct hid_device *hdev)
 
 	cancel_work_sync(&wacom->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);
-- 
2.5.5

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


#1442598 — [PATCH v2 16/30] HID: wacom: rework fail path in probe() and parse_and_register()

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 16/30] HID: wacom: rework fail path in probe() and parse_and_register()
Message-ID<rUv3d-5rv-47@gated-at.bofh.it>
In reply to#1442576
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>
---

No changes in v2
---
 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]


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

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 27/30] HID: wacom: leds: handle Cintiq 24HD leds buttons
Message-ID<rUv3d-5rv-51@gated-at.bofh.it>
In reply to#1442576
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>
---

Changes in v2:
- amended to use the old bank ordering
- removed the pr_err debug left over
---
 drivers/hid/wacom_wac.c | 38 +++++++++++++++++++++++++++++++++++---
 1 file changed, 35 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a9693d5..0914667 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2763,18 +2763,47 @@ 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;
+
+	/*
+	 * 24HD has LED group 1 to the left and LED group 0 to the right.
+	 * So group 0 matches the second half of the buttons and thus the mask
+	 * needs to be shifted.
+	 */
+	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)
 {
 	int button_per_group;
 
 	/*
-	 * 24HD and 21UX2 have LED group 1 to the left and LED group 0
+	 * 21UX2 has LED group 1 to the left and LED group 0
 	 * to the right. We need to reverse the group to match this
 	 * historical behavior.
 	 */
-	if (wacom->wacom_wac.features.type == WACOM_24HD ||
-	    wacom->wacom_wac.features.type == WACOM_21UX2)
+	if (wacom->wacom_wac.features.type == WACOM_21UX2)
 		group = 1 - group;
 
 	button_per_group = button_count/wacom->led.count;
@@ -2789,6 +2818,9 @@ 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;
 
-- 
2.5.5

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


#1442600 — [PATCH v2 25/30] HID: wacom: leds: handle the switch of the LEDs directly in the kernel

FromBenjamin Tissoires <benjamin.tissoires@redhat.com>
Date2016-07-13 18:20 +0200
Subject[PATCH v2 25/30] HID: wacom: leds: handle the switch of the LEDs directly in the kernel
Message-ID<rUv3d-5rv-43@gated-at.bofh.it>
In reply to#1442576
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>
---

Changes in v2:
- added missing Kconfig LEDS_TRIGGERS
---
 drivers/hid/Kconfig     |  1 +
 drivers/hid/wacom.h     |  4 ++++
 drivers/hid/wacom_sys.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++---
 drivers/hid/wacom_wac.c | 53 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 118 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 78ac481..bdacb55 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -862,6 +862,7 @@ config HID_WACOM
 	select POWER_SUPPLY
 	select NEW_LEDS
 	select LEDS_CLASS
+	select LEDS_TRIGGERS
 	help
 	  Say Y here if you want to use the USB or BT version of the Wacom Intuos
 	  or Graphire tablet.
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 3a651e2..7e83352 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -969,7 +969,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;
 
@@ -1042,6 +1042,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;
@@ -1051,10 +1062,12 @@ static int wacom_led_register_one(struct device *dev, struct wacom *wacom,
 	led->cdev.max_brightness = LED_FULL;
 	led->cdev.flags = LED_HW_PLUGGABLE;
 	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) {
@@ -1131,6 +1144,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 1c882bb..00e0c80 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2763,11 +2763,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]


Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel


csiph-web