From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:01
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
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:06:41
wacom->remote_dir has nothing to do with inputs, so better not magically
removing it when cleaning inputs.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 25 ++++++++++++++-----------
1 file changed, 14 insertions(+), 11 deletions(-)
@@ -1705,7 +1704,7 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)if((features->type==BAMBOO_TOUCH)&&(features->device_type&WACOM_DEVICETYPE_PEN)){error=-ENODEV;-gotofail_hw_start;+gotofail_quirks;}/* pen only Bamboo neither support touch nor pad */
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:06:44
Since fd5f92b ("HID: wacom: reuse wacom_parse_and_register() in
wireless_work"), wacom->shared->type is not set.
Send the information of the battery if we have one.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_wac.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:06:46
Simplifying the error code paths.
We need to keep wacom_clean_inputs() around for now as the wireless
module is using it to dynamically remove the inputs on disconnect.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 34 ++++++++++++++++------------------
1 file changed, 16 insertions(+), 18 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:06
We started switching the driver to devres, so we should use it as much
as possible.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
@@ -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(structwacom),GFP_KERNEL);+wacom=devm_kzalloc(&hdev->dev,sizeof(structwacom),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);returnerror;}
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:12
Use our own wacom_devm_sysfs_create_group() as there is currently no
generic one. It has been requested at least twice [1][2] but has been
always rejected.
However, in the Wacom case, for the wirelessly connected devices, we need
to be able to release the created sysfs files without removing the parent
kobject.
[1] https://patchwork.kernel.org/patch/7526551/
[2] https://lkml.org/lkml/2013/3/14/728
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom.h | 1 -
drivers/hid/wacom_sys.c | 95 +++++++++++++++++++++++--------------------------
2 files changed, 45 insertions(+), 51 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:24
The wacom_remote_create_attr_group() and wacom_remote_destroy_attr_group()
functions were both allocating/destroying the sysfs groups but also
initializing the parameters for the remotes. Have proper functions
that can be called and extended.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 93 ++++++++++++++++++++++++++++---------------------
1 file changed, 53 insertions(+), 40 deletions(-)
@@ -1916,6 +1900,50 @@ fail:return;}+staticvoidwacom_remote_destroy_one(structwacom*wacom,unsignedintindex)+{+structwacom_remote*remote=wacom->remote;+u32serial=remote->serial[index];+inti;++wacom_remote_destroy_attr_group(wacom,index);++for(i=0;i<WACOM_MAX_REMOTES;i++){+if(remote->serial[i]==serial){+remote->serial[i]=0;+wacom->led.groups[i].select=WACOM_STATUS_UNKNOWN;+}+}+}++staticintwacom_remote_create_one(structwacom*wacom,u32serial,+unsignedintindex)+{+structwacom_remote*remote=wacom->remote;+interror,k;++/* A remote can pair more than once with an EKR,+*checktomakesurethisserialisn'talreadypaired.+*/+for(k=0;k<WACOM_MAX_REMOTES;k++){+if(remote->serial[k]==serial)+break;+}++if(k<WACOM_MAX_REMOTES){+remote->serial[index]=serial;+return0;+}++error=wacom_remote_create_attr_group(wacom,serial,index);+if(error)+returnerror;++remote->serial[index]=serial;++return0;+}+staticvoidwacom_remote_work(structwork_struct*work){structwacom*wacom=container_of(work,structwacom,remote_work);
@@ -1949,28 +1977,13 @@ static void wacom_remote_work(struct work_struct *work)if(remote->serial[i]==serial)continue;-if(remote->serial[i]){-wacom_remote_destroy_attr_group(wacom,-remote->serial[i]);-}--/* A remote can pair more than once with an EKR,-*checktomakesurethisserialisn'talreadypaired.-*/-for(k=0;k<WACOM_MAX_REMOTES;k++){-if(remote->serial[k]==serial)-break;-}+if(remote->serial[i])+wacom_remote_destroy_one(wacom,i);-if(k<WACOM_MAX_REMOTES){-remote->serial[i]=serial;-continue;-}-wacom_remote_create_attr_group(wacom,serial,i);+wacom_remote_create_one(wacom,serial,i);}elseif(remote->serial[i]){-wacom_remote_destroy_attr_group(wacom,-remote->serial[i]);+wacom_remote_destroy_one(wacom,i);}}}
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:32
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 <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
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 <redacted>
The whole set is
Acked-by: Ping Cheng <redacted>
Thanks for your effort!
Ping
quoted hunk
---
No changes in v2
---
drivers/hid/wacom_sys.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-18 14:54:47
On Jul 14 2016 or thereabouts, Ping Cheng wrote:
On Wed, Jul 13, 2016 at 9:05 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
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.
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.
quoted
Untested.
Aaron tested it. So, your code is safe ;).
quoted
Signed-off-by: Benjamin Tissoires <redacted>
The whole set is
Acked-by: Ping Cheng <redacted>
Thanks!
Cheers,
Benjamin
Thanks for your effort!
Ping
quoted
---
No changes in v2
---
drivers/hid/wacom_sys.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:42
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 <redacted>
---
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(-)
@@ -110,6 +110,10 @@ enum wacom_worker {WACOM_WORKER_BATTERY,};+structwacom_group_leds{+u8select;/* status led selector (0..3) */+};+structwacom{structusb_device*usbdev;structusb_interface*intf;
@@ -118,8 +122,8 @@ struct wacom {structmutexlock;structwork_structwireless_work;structwork_structbattery_work;-structwacom_led{-u8select[5];/* status led selector (0..3) */+structwacom_leds{+structwacom_group_leds*groups;u8llv;/* status led brightness no button (1..127) */u8hlv;/* status led brightness button pressed (1..127) */u8img_lum;/* OLED matrix display brightness */
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:47
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 <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:53
Looks like the battery hijacked the wireless worker. That's not fair so
use a work queue per task.
Signed-off-by: Benjamin Tissoires <redacted>
---
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(-)
@@ -105,13 +105,19 @@#define USB_VENDOR_ID_WACOM 0x056a#define USB_VENDOR_ID_LENOVO 0x17ef+enumwacom_worker{+WACOM_WORKER_WIRELESS,+WACOM_WORKER_BATTERY,+};+structwacom{structusb_device*usbdev;structusb_interface*intf;structwacom_wacwacom_wac;structhid_device*hdev;structmutexlock;-structwork_structwork;+structwork_structwireless_work;+structwork_structbattery_work;structwacom_led{u8select[5];/* status led selector (0..3) */u8llv;/* status led brightness no button (1..127) */
@@ -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);
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:07:58
wacom_release_shared_data() and wacom_remove_shared_data() are moved up
so they can be referenced in wacom_add_shared_data().
There is no point in explicitly setting wacom_wac1->shared->type to 0 in
wacom_wireless_work() (plus this would give an oops).
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 73 ++++++++++++++++++++++++++-----------------------
1 file changed, 39 insertions(+), 34 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:07
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 <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:18
wacom_remote_status_irq() sends information of addition/removal of EKR.
We want to allocate one input node per remote, so better having this
in a separate worker, not handled in the IRQ directly.
Signed-off-by: Benjamin Tissoires <redacted>
---
Changes in v2:
- removed warning: missing braces around initializer in
wacom_remote_status_irq()
- added missing spin_unlock_irqrestore()
- make wacom_remote_status_irq() returning void to not confuse the reader
as the return value is not an error code
---
drivers/hid/wacom.h | 11 +++++--
drivers/hid/wacom_sys.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++--
drivers/hid/wacom_wac.c | 53 +++++++++++++-------------------
drivers/hid/wacom_wac.h | 7 +++++
4 files changed, 113 insertions(+), 38 deletions(-)
@@ -122,6 +124,9 @@ struct wacom {structmutexlock;structwork_structwireless_work;structwork_structbattery_work;+structwork_structremote_work;+spinlock_tremote_lock;+structkfiforemote_fifo;structwacom_leds{structwacom_group_leds*groups;u8llv;/* status led brightness no button (1..127) */
@@ -1318,7 +1319,7 @@ int wacom_remote_create_attr_group(struct wacom *wacom, __u32 serial, int index)return0;}-voidwacom_remote_destroy_attr_group(structwacom*wacom,__u32serial)+staticvoidwacom_remote_destroy_attr_group(structwacom*wacom,__u32serial){structwacom_wac*wacom_wac=&wacom->wacom_wac;inti;
@@ -1889,6 +1890,65 @@ fail:return;}+staticvoidwacom_remote_work(structwork_struct*work)+{+structwacom*wacom=container_of(work,structwacom,remote_work);+structwacom_wac*wacom_wac=&wacom->wacom_wac;+structwacom_remote_datadata;+unsignedlongflags;+unsignedintcount;+u32serial;+inti,k;++spin_lock_irqsave(&wacom->remote_lock,flags);++count=kfifo_out(&wacom->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);+return;+}++if(!kfifo_is_empty(&wacom->remote_fifo))+wacom_schedule_work(&wacom->wacom_wac,WACOM_WORKER_REMOTE);++spin_unlock_irqrestore(&wacom->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)+continue;++if(wacom_wac->serial[i]){+wacom_remote_destroy_attr_group(wacom,+wacom_wac->serial[i]);+}++/* A remote can pair more than once with an EKR,+*checktomakesurethisserialisn'talreadypaired.+*/+for(k=0;k<WACOM_MAX_REMOTES;k++){+if(wacom_wac->serial[k]==serial)+break;+}++if(k<WACOM_MAX_REMOTES){+wacom_wac->serial[i]=serial;+continue;+}+wacom_remote_create_attr_group(wacom,serial,i);++}elseif(wacom_wac->serial[i]){+wacom_remote_destroy_attr_group(wacom,+wacom_wac->serial[i]);+}+}+}+staticintwacom_probe(structhid_device*hdev,conststructhid_device_id*id){
@@ -1931,6 +1991,17 @@ static int wacom_probe(struct hid_device *hdev,mutex_init(&wacom->lock);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(structwacom_remote_data),+GFP_KERNEL)){+dev_err(&hdev->dev,+"%s:failed allocating remote_fifo\n",__func__);+error=-ENOMEM;+gotofail_type;+}/* ask for the report descriptor to be loaded by HID */error=hid_parse(hdev);
@@ -1953,8 +2024,9 @@ static int wacom_probe(struct hid_device *hdev,return0;-fail_type:fail_parse:+kfifo_free(&wacom->remote_fifo);+fail_type:hid_set_drvdata(hdev,NULL);returnerror;}
@@ -823,52 +823,40 @@ static int wacom_remote_irq(struct wacom_wac *wacom_wac, size_t len)return1;}-staticintwacom_remote_status_irq(structwacom_wac*wacom_wac,size_tlen)+staticvoidwacom_remote_status_irq(structwacom_wac*wacom_wac,size_tlen){structwacom*wacom=container_of(wacom_wac,structwacom,wacom_wac);unsignedchar*data=wacom_wac->data;-inti;+structwacom_remote_dataremote_data;+unsignedlongflags;+inti,ret;if(data[0]!=WACOM_REPORT_DEVICE_LIST)-return0;+return;++memset(&remote_data,0,sizeof(structwacom_remote_data));for(i=0;i<WACOM_MAX_REMOTES;i++){intj=i*6;intserial=(data[j+6]<<16)+(data[j+5]<<8)+data[j+4];boolconnected=data[j+2];-if(connected){-intk;--if(wacom_wac->serial[i]==serial)-continue;--if(wacom_wac->serial[i]){-wacom_remote_destroy_attr_group(wacom,-wacom_wac->serial[i]);-}--/* A remote can pair more than once with an EKR,-*checktomakesurethisserialisn'talreadypaired.-*/-for(k=0;k<WACOM_MAX_REMOTES;k++){-if(wacom_wac->serial[k]==serial)-break;-}+remote_data.remote[i].serial=serial;+remote_data.remote[i].connected=connected;+}-if(k<WACOM_MAX_REMOTES){-wacom_wac->serial[i]=serial;-continue;-}-wacom_remote_create_attr_group(wacom,serial,i);+spin_lock_irqsave(&wacom->remote_lock,flags);-}elseif(wacom_wac->serial[i]){-wacom_remote_destroy_attr_group(wacom,-wacom_wac->serial[i]);-}+ret=kfifo_in(&wacom->remote_fifo,&remote_data,sizeof(remote_data));+if(ret!=sizeof(remote_data)){+spin_unlock_irqrestore(&wacom->remote_lock,flags);+hid_err(wacom->hdev,"Can't queue Remote status event.\n");+return;}-return0;+spin_unlock_irqrestore(&wacom->remote_lock,flags);++wacom_schedule_work(wacom_wac,WACOM_WORKER_REMOTE);}staticintwacom_intuos_general(structwacom_wac*wacom)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:26
No functional changes, just a prep patch for the one after.
Signed-off-by: Benjamin Tissoires <redacted>
---
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(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:32
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 <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 37 ++++++++++++++-----------------------
1 file changed, 14 insertions(+), 23 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:42
The code for 21UX2 and 24HD makes the LED group 1 on the left, and
the group 0 on the right. The buttons are ordered in the other way,
but libwacom already exports those that way. So we simply can't reassign
LED group 0 to the left buttons, and have to quirk the incoming data...
Signed-off-by: Benjamin Tissoires <redacted>
---
New in v2:
- replace: "[PATCH 25/27] HID: wacom: leds: fix ordering of LED banks"
---
drivers/hid/wacom_wac.c | 9 +++++++++
1 file changed, 9 insertions(+)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:48
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 <redacted>
---
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(-)
@@ -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
@@ -112,8 +113,23 @@ enum wacom_worker {WACOM_WORKER_REMOTE,};+structwacom;++structwacom_led{+structled_classdevcdev;+structled_triggertrigger;+structwacom*wacom;+unsignedintgroup;+unsignedintid;+u8llv;+u8hlv;+boolheld;+};+structwacom_group_leds{u8select;/* status led selector (0..3) */+structwacom_led*leds;+unsignedintcount;};structwacom_battery{
@@ -154,9 +170,12 @@ struct wacom {structwacom_remote*remote;structwacom_leds{structwacom_group_leds*groups;+unsignedintcount;u8llv;/* status led brightness no button (1..127) */u8hlv;/* status led brightness button pressed (1..127) */u8img_lum;/* OLED matrix display brightness */+u8max_llv;/* maximum brightness of LED (llv) */+u8max_hlv;/* maximum brightness of LED (hlv) */}led;structwacom_batterybattery;boolresources;
@@ -647,6 +647,9 @@ static int wacom_led_control(struct wacom *wacom)unsignedcharreport_id=WAC_CMD_LED_CONTROL;intbuf_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);}+staticenumled_brightnesswacom_leds_brightness_get(structwacom_led*led)+{+structwacom*wacom=led->wacom;++if(wacom->led.max_hlv)+returnled->hlv*LED_FULL/wacom->led.max_hlv;++if(wacom->led.max_llv)+returnled->llv*LED_FULL/wacom->led.max_llv;++/* device doesn't support brightness tuning */+returnLED_FULL;+}++staticenumled_brightness__wacom_led_brightness_get(structled_classdev*cdev)+{+structwacom_led*led=container_of(cdev,structwacom_led,cdev);+structwacom*wacom=led->wacom;++if(wacom->led.groups[led->group].select!=led->id)+returnLED_OFF;++returnwacom_leds_brightness_get(led);+}++staticintwacom_led_brightness_set(structled_classdev*cdev,+enumled_brightnessbrightness)+{+structwacom_led*led=container_of(cdev,structwacom_led,cdev);+structwacom*wacom=led->wacom;+interror;++mutex_lock(&wacom->lock);++if(!wacom->led.groups||(brightness==LED_OFF&&+wacom->led.groups[led->group].select!=led->id)){+error=0;+gotoout;+}++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);++returnerror;+}++staticvoidwacom_led_readonly_brightness_set(structled_classdev*cdev,+enumled_brightnessbrightness)+{+}++staticintwacom_led_register_one(structdevice*dev,structwacom*wacom,+structwacom_led*led,unsignedintgroup,+unsignedintid,boolread_only)+{+interror;+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;+returnerror;+}++return0;+}++staticintwacom_led_groups_alloc_and_register_one(structdevice*dev,+structwacom*wacom,+intgroup_id,intcount,+boolread_only)+{+structwacom_led*leds;+inti,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(structwacom_led)*count,GFP_KERNEL);+if(!leds){+error=-ENOMEM;+gotoerr;+}++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)+gotoerr;+}++devres_remove_group(dev,&wacom->led.groups[group_id]);+return0;++err:+devres_release_group(dev,&wacom->led.groups[group_id]);+returnerror;+}+staticvoidwacom_led_groups_release(void*data){structwacom*wacom=data;wacom->led.groups=NULL;+wacom->led.count=0;}staticintwacom_led_groups_allocate(structwacom*wacom,intcount){+structdevice*dev=&wacom->hdev->dev;structwacom_group_leds*groups;interror;-groups=devm_kzalloc(&wacom->hdev->dev,-sizeof(structwacom_group_leds)*count,+groups=devm_kzalloc(dev,sizeof(structwacom_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)returnerror;wacom->led.groups=groups;+wacom->led.count=count;++return0;+}++staticintwacom_leds_alloc_and_register(structwacom*wacom,intgroup_count,+intled_per_group,boolread_only)+{+structdevice*dev;+inti,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)+returnerror;++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)+returnerror;+}return0;}
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:08:54
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 <redacted>
---
No changes in v2
---
drivers/hid/wacom.h | 1 +
drivers/hid/wacom_sys.c | 27 ++++++++++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:09:05
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 <redacted>
---
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(-)
@@ -116,6 +116,14 @@ struct wacom_group_leds {u8select;/* status led selector (0..3) */};+structwacom_remote{+spinlock_tremote_lock;+structkfiforemote_fifo;+structkobject*remote_dir;+structattribute_groupremote_group[WACOM_MAX_REMOTES];+__u32serial[WACOM_MAX_REMOTES];+};+structwacom{structusb_device*usbdev;structusb_interface*intf;
@@ -125,8 +133,7 @@ struct wacom {structwork_structwireless_work;structwork_structbattery_work;structwork_structremote_work;-spinlock_tremote_lock;-structkfiforemote_fifo;+structwacom_remote*remote;structwacom_leds{structwacom_group_leds*groups;u8llv;/* status led brightness no button (1..127) */
@@ -1893,58 +1927,58 @@ fail:staticvoidwacom_remote_work(structwork_struct*work){structwacom*wacom=container_of(work,structwacom,remote_work);-structwacom_wac*wacom_wac=&wacom->wacom_wac;+structwacom_remote*remote=wacom->remote;structwacom_remote_datadata;unsignedlongflags;unsignedintcount;u32serial;inti,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,*checktomakesurethisserialisn'talreadypaired.*/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);-}elseif(wacom_wac->serial[i]){+}elseif(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(structwacom_remote_data),-GFP_KERNEL)){-dev_err(&hdev->dev,-"%s:failed allocating remote_fifo\n",__func__);-error=-ENOMEM;-gotofail_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,return0;fail_parse:-kfifo_free(&wacom->remote_fifo);fail_type:hid_set_drvdata(hdev,NULL);returnerror;
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:09:17
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 <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 44 ++++++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 16 deletions(-)
@@ -1920,6 +1922,7 @@ static int wacom_remote_create_one(struct wacom *wacom, u32 serial,unsignedintindex){structwacom_remote*remote=wacom->remote;+structdevice*dev=&wacom->hdev->dev;interror,k;/* A remote can pair more than once with an EKR,
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:09:30
Instead of displaying a generic "tablet", now g-c-c shows a pretty
"Wacom Intuos Pro S (WL)".
Signed-off-by: Benjamin Tissoires <redacted>
---
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(+)
@@ -1336,6 +1336,7 @@ static int wacom_initialize_leds(struct wacom *wacom)}staticenumpower_supply_propertywacom_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,intret=0;switch(psp){+casePOWER_SUPPLY_PROP_MODEL_NAME:+val->strval=battery->wacom->wacom_wac.name;+break;casePOWER_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);
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:09:41
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 <redacted>
---
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(-)
@@ -2763,11 +2763,64 @@ static void wacom_setup_numbered_buttons(struct input_dev *input_dev,__set_bit(BTN_BASE+(i-16),input_dev->keybit);}+staticboolwacom_is_led_toggled(structwacom*wacom,intbutton_count,+intmask,intgroup)+{+intbutton_per_group;++button_per_group=button_count/wacom->led.count;++returnmask&(1<<(group*button_per_group));+}++staticvoidwacom_update_led(structwacom*wacom,intbutton_count,intmask,+intgroup)+{+structwacom_led*led,*next_led;+intcur;+boolpressed;++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));+}+staticvoidwacom_report_numbered_buttons(structinput_dev*input_dev,intbutton_count,intmask){+structwacom*wacom=input_get_drvdata(input_dev);inti;+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++)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:11:04
Or Gnome complains about an empty battery.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:11:46
Thanks to devres, we can now afford to create more than one input node
without having to overload the remove/failure paths. Having one input
node per remote is something which should have been implemented from start
but the probability of having users with several remotes is quite low.
Anyway, still, better looking at the future and implement things properly.
Remote input nodes will be freed/unregistered magically as they are
created in the devres group &remote->remotes[index].
We need to open the hid node now that the remotes are dynamically
allocated.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom.h | 2 ++
drivers/hid/wacom_sys.c | 30 ++++++++++++++++++++++++++++++
drivers/hid/wacom_wac.c | 35 ++++++++++++++++++++++++++++++-----
3 files changed, 62 insertions(+), 5 deletions(-)
@@ -2762,6 +2784,9 @@ int wacom_setup_pad_input_capabilities(struct input_dev *input_dev,if(!(features->device_type&WACOM_DEVICETYPE_PAD))return-ENODEV;+if(features->type==REMOTE&&input_dev==wacom_wac->pad_input)+return-ENODEV;+input_dev->evbit[0]|=BIT_MASK(EV_KEY)|BIT_MASK(EV_ABS);/* kept for making legacy xf86-input-wacom working with the wheels */
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:12:24
Looks like upowerd is ignoring this since October 2013, so there is
no need to keep this around in the kernel.
And as mentioned in 8aaa592 (linux: Ignore ACs coming from devices) in
the upower tree, "We already have enough information on the device
battery".
Signed-off-by: Benjamin Tissoires <redacted>
---
New in v2
---
drivers/hid/wacom.h | 3 ---
drivers/hid/wacom_sys.c | 49 +------------------------------------------------
2 files changed, 1 insertion(+), 51 deletions(-)
@@ -1384,38 +1378,14 @@ static int wacom_battery_get_property(struct power_supply *psy,returnret;}-staticintwacom_ac_get_property(structpower_supply*psy,-enumpower_supply_propertypsp,-unionpower_supply_propval*val)-{-structwacom_battery*battery=power_supply_get_drvdata(psy);-intret=0;--switch(psp){-casePOWER_SUPPLY_PROP_PRESENT:-/* fall through */-casePOWER_SUPPLY_PROP_ONLINE:-val->intval=battery->ps_connected;-break;-casePOWER_SUPPLY_PROP_SCOPE:-val->intval=POWER_SUPPLY_SCOPE_DEVICE;-break;-default:-ret=-EINVAL;-break;-}-returnret;-}-staticint__wacom_initialize_battery(structwacom*wacom,structwacom_battery*battery){staticatomic_tbattery_no=ATOMIC_INIT(0);structdevice*dev=&wacom->hdev->dev;structpower_supply_configpsy_cfg={.drv_data=battery,};-structpower_supply*ps_bat,*ps_ac;+structpower_supply*ps_bat;structpower_supply_desc*bat_desc=&battery->bat_desc;-structpower_supply_desc*ac_desc=&battery->ac_desc;unsignedlongn;interror;
@@ -1432,31 +1402,15 @@ static int __wacom_initialize_battery(struct wacom *wacom,bat_desc->type=POWER_SUPPLY_TYPE_USB;bat_desc->use_for_apm=0;-ac_desc->properties=wacom_ac_props;-ac_desc->num_properties=ARRAY_SIZE(wacom_ac_props);-ac_desc->get_property=wacom_ac_get_property;-sprintf(battery->ac_name,"wacom_ac_%ld",n);-ac_desc->name=battery->ac_name;-ac_desc->type=POWER_SUPPLY_TYPE_MAINS;-ac_desc->use_for_apm=0;-ps_bat=devm_power_supply_register(dev,bat_desc,&psy_cfg);if(IS_ERR(ps_bat)){error=PTR_ERR(ps_bat);gotoerr;}-ps_ac=devm_power_supply_register(dev,ac_desc,&psy_cfg);-if(IS_ERR(ps_ac)){-error=PTR_ERR(ps_ac);-gotoerr;-}-power_supply_powers(ps_bat,&wacom->hdev->dev);-power_supply_powers(ps_ac,&wacom->hdev->dev);battery->battery=ps_bat;-battery->ac=ps_ac;devres_close_group(dev,bat_desc);return0;
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:12:34
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 <redacted>
---
New in v2
---
drivers/hid/wacom_sys.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:12:36
Previously, all the remotes attached to the same receiver would share the
same power_supply. That's not good as the remotes will constantly change
the battery information according to their own state.
To have something generic enough, we introduce struct wacom_battery
which regroups all the information we need for a battery.
Signed-off-by: Benjamin Tissoires <redacted>
---
Changes in v2:
- remove spurious 0% battery when creating the power_supply node
---
drivers/hid/wacom.h | 19 ++++++--
drivers/hid/wacom_sys.c | 123 ++++++++++++++++++++++++++----------------------
drivers/hid/wacom_wac.c | 54 ++++++++++-----------
drivers/hid/wacom_wac.h | 6 ---
4 files changed, 109 insertions(+), 93 deletions(-)
@@ -1101,27 +1101,26 @@ static int wacom_battery_get_property(struct power_supply *psy,enumpower_supply_propertypsp,unionpower_supply_propval*val){-structwacom*wacom=power_supply_get_drvdata(psy);+structwacom_battery*battery=power_supply_get_drvdata(psy);intret=0;switch(psp){casePOWER_SUPPLY_PROP_PRESENT:-val->intval=wacom->wacom_wac.bat_connected;+val->intval=battery->bat_connected;break;casePOWER_SUPPLY_PROP_SCOPE:val->intval=POWER_SUPPLY_SCOPE_DEVICE;break;casePOWER_SUPPLY_PROP_CAPACITY:-val->intval=-wacom->wacom_wac.battery_capacity;+val->intval=battery->battery_capacity;break;casePOWER_SUPPLY_PROP_STATUS:-if(wacom->wacom_wac.bat_charging)+if(battery->bat_charging)val->intval=POWER_SUPPLY_STATUS_CHARGING;-elseif(wacom->wacom_wac.battery_capacity==100&&-wacom->wacom_wac.ps_connected)+elseif(battery->battery_capacity==100&&+battery->ps_connected)val->intval=POWER_SUPPLY_STATUS_FULL;-elseif(wacom->wacom_wac.ps_connected)+elseif(battery->ps_connected)val->intval=POWER_SUPPLY_STATUS_NOT_CHARGING;elseval->intval=POWER_SUPPLY_STATUS_DISCHARGING;
@@ -1138,14 +1137,14 @@ static int wacom_ac_get_property(struct power_supply *psy,enumpower_supply_propertypsp,unionpower_supply_propval*val){-structwacom*wacom=power_supply_get_drvdata(psy);+structwacom_battery*battery=power_supply_get_drvdata(psy);intret=0;switch(psp){casePOWER_SUPPLY_PROP_PRESENT:/* fall through */casePOWER_SUPPLY_PROP_ONLINE:-val->intval=wacom->wacom_wac.ps_connected;+val->intval=battery->ps_connected;break;casePOWER_SUPPLY_PROP_SCOPE:val->intval=POWER_SUPPLY_SCOPE_DEVICE;
@@ -1157,58 +1156,56 @@ static int wacom_ac_get_property(struct power_supply *psy,returnret;}-staticintwacom_initialize_battery(structwacom*wacom)+staticint__wacom_initialize_battery(structwacom*wacom,+structwacom_battery*battery){staticatomic_tbattery_no=ATOMIC_INIT(0);structdevice*dev=&wacom->hdev->dev;-structpower_supply_configpsy_cfg={.drv_data=wacom,};-structpower_supply_desc*bat_desc=&wacom->battery_desc;+structpower_supply_configpsy_cfg={.drv_data=battery,};+structpower_supply*ps_bat,*ps_ac;+structpower_supply_desc*bat_desc=&battery->bat_desc;+structpower_supply_desc*ac_desc=&battery->ac_desc;unsignedlongn;interror;if(!devres_open_group(dev,bat_desc,GFP_KERNEL))return-ENOMEM;-if(wacom->wacom_wac.features.quirks&WACOM_QUIRK_BATTERY){-structpower_supply_desc*ac_desc=&wacom->ac_desc;-n=atomic_inc_return(&battery_no)-1;--bat_desc->properties=wacom_battery_props;-bat_desc->num_properties=ARRAY_SIZE(wacom_battery_props);-bat_desc->get_property=wacom_battery_get_property;-sprintf(wacom->wacom_wac.bat_name,"wacom_battery_%ld",n);-bat_desc->name=wacom->wacom_wac.bat_name;-bat_desc->type=POWER_SUPPLY_TYPE_BATTERY;-bat_desc->use_for_apm=0;--ac_desc->properties=wacom_ac_props;-ac_desc->num_properties=ARRAY_SIZE(wacom_ac_props);-ac_desc->get_property=wacom_ac_get_property;-sprintf(wacom->wacom_wac.ac_name,"wacom_ac_%ld",n);-ac_desc->name=wacom->wacom_wac.ac_name;-ac_desc->type=POWER_SUPPLY_TYPE_MAINS;-ac_desc->use_for_apm=0;--wacom->battery=devm_power_supply_register(dev,-&wacom->battery_desc,-&psy_cfg);-if(IS_ERR(wacom->battery)){-error=PTR_ERR(wacom->battery);-gotoerr;-}+n=atomic_inc_return(&battery_no)-1;++bat_desc->properties=wacom_battery_props;+bat_desc->num_properties=ARRAY_SIZE(wacom_battery_props);+bat_desc->get_property=wacom_battery_get_property;+sprintf(battery->bat_name,"wacom_battery_%ld",n);+bat_desc->name=battery->bat_name;+bat_desc->type=POWER_SUPPLY_TYPE_BATTERY;+bat_desc->use_for_apm=0;++ac_desc->properties=wacom_ac_props;+ac_desc->num_properties=ARRAY_SIZE(wacom_ac_props);+ac_desc->get_property=wacom_ac_get_property;+sprintf(battery->ac_name,"wacom_ac_%ld",n);+ac_desc->name=battery->ac_name;+ac_desc->type=POWER_SUPPLY_TYPE_MAINS;+ac_desc->use_for_apm=0;++ps_bat=devm_power_supply_register(dev,bat_desc,&psy_cfg);+if(IS_ERR(ps_bat)){+error=PTR_ERR(ps_bat);+gotoerr;+}-power_supply_powers(wacom->battery,&wacom->hdev->dev);+ps_ac=devm_power_supply_register(dev,ac_desc,&psy_cfg);+if(IS_ERR(ps_ac)){+error=PTR_ERR(ps_ac);+gotoerr;+}-wacom->ac=devm_power_supply_register(dev,-&wacom->ac_desc,-&psy_cfg);-if(IS_ERR(wacom->ac)){-error=PTR_ERR(wacom->ac);-gotoerr;-}+power_supply_powers(ps_bat,&wacom->hdev->dev);+power_supply_powers(ps_ac,&wacom->hdev->dev);-power_supply_powers(wacom->ac,&wacom->hdev->dev);-}+battery->battery=ps_bat;+battery->ac=ps_ac;devres_close_group(dev,bat_desc);return0;
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:13:39
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 <redacted>
---
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(-)
@@ -2763,18 +2763,47 @@ static void wacom_setup_numbered_buttons(struct input_dev *input_dev,__set_bit(BTN_BASE+(i-16),input_dev->keybit);}+staticvoidwacom_24hd_update_leds(structwacom*wacom,intmask,intgroup)+{+structwacom_led*led;+inti;+boolupdated=false;++/*+*24HDhasLEDgroup1totheleftandLEDgroup0totheright.+*Sogroup0matchesthesecondhalfofthebuttonsandthusthemask+*needstobeshifted.+*/+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;+}+}+}+staticboolwacom_is_led_toggled(structwacom*wacom,intbutton_count,intmask,intgroup){intbutton_per_group;/*-*24HDand21UX2haveLEDgroup1totheleftandLEDgroup0+*21UX2hasLEDgroup1totheleftandLEDgroup0*totheright.Weneedtoreversethegrouptomatchthis*historicalbehavior.*/-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,intcur;boolpressed;+if(wacom->wacom_wac.features.type==WACOM_24HD)+returnwacom_24hd_update_leds(wacom,mask,group);+pressed=wacom_is_led_toggled(wacom,button_count,mask,group);cur=wacom->led.groups[group].select;
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:14:28
Simplifying the error code paths.
We need to keep wacom_destroy_battery() around for now as the wireless
module and the remotes are using it to dynamically remove the battery
supply on disconnect.
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom_sys.c | 38 ++++++++++++++++++++++++--------------
1 file changed, 24 insertions(+), 14 deletions(-)
From: Benjamin Tissoires <hidden> Date: 2016-07-13 16:15:11
We currently have a complex clean_inputs() function while this can be
handled all by devres. Set a group that we can destroy in wireless_work().
Signed-off-by: Benjamin Tissoires <redacted>
---
No changes in v2
---
drivers/hid/wacom.h | 1 +
drivers/hid/wacom_sys.c | 69 +++++++++++++++++++++----------------------------
drivers/hid/wacom_wac.c | 2 +-
drivers/hid/wacom_wac.h | 3 ---
4 files changed, 32 insertions(+), 43 deletions(-)
@@ -1941,7 +1941,7 @@ static int wacom_bpt3_touch(struct wacom_wac *wacom)}/* only update touch if we actually have a touchpad and touch data changed */-if(wacom->touch_registered&&touch_changed){+if(wacom->touch_input&&touch_changed){input_mt_sync_frame(wacom->touch_input);wacom->shared->touch_down=wacom_wac_finger_count_touches(wacom);}
On Wed, Jul 13, 2016 at 9:05 AM, Benjamin Tissoires
[off-list ref] wrote:
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
Retested on the 21UX2 and the 24HD.
Tested-by Aaron Armstrong Skomra [off-list ref]
Best,
Aaron
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
Hi Jiri,
This patchset has gone through two rounds of testing/review. It is
also a necessary set to support future userland LED configuration
features.
Do you see any issues with the patches?
Cheers,
Ping
On Wed, Jul 13, 2016 at 2:36 PM, Aaron Armstrong Skomra
[off-list ref] wrote:
On Wed, Jul 13, 2016 at 9:05 AM, Benjamin Tissoires
[off-list ref] wrote:
quoted
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
Retested on the 21UX2 and the 24HD.
Tested-by Aaron Armstrong Skomra [off-list ref]
Best,
Aaron
quoted
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
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
This patchset has gone through two rounds of testing/review. It is
also a necessary set to support future userland LED configuration
features.
Do you see any issues with the patches?
Hi Ping,
vacation and merge window interfered, but this patchset is definitely on
my plate for the upcoming days, with 4.9 as a likely target (still going
through them, but I'm almost done and nothing major popped up so far).
Thanks for your patience,
--
Jiri Kosina
SUSE Labs