GNOME Bugzilla – Bug 734458
Support for Wacom Intuos4 OLEDs in 3.17+ kernels
Last modified: 2014-08-12 21:59:23 UTC
(brother of #734455) In kernel v3.17, the wacom.ko and hid-wacom.ko drivers will receive a deep cleaning. To easier future inclusion of new Wacom tablets, these two drivers have been merged and now rely on the hid subsystem. The counterpart of it is that the OLEDs path that we used to find under the usb node are not here anymore, but under the hid node. g-s-d needs to be updated to match this new behaviour to continue to be able to send properly the OLED commands to the devices. The Bluetooth ABI to set the OLED disappeared in favour of using the same sysfs node that the one in USB. The counterpart is that now the Bluetooth stack also has to scramble the OLED image before sending it to the device.
Created attachment 282848 [details] [review] 0001-wacom-check-for-the-new-HID-unified-kernel-ABI-when-.patch Proposed patch to enable the OLED on USB. Bluetooth will fail due to the different format. Same warning as in #734455, please do not merge/solve this until the patch series gets into Linus' tree.
Created attachment 282851 [details] [review] 0002-wacom-new-kernels-required-the-Bluetooth-oled-data-t.patch Second part of the patch series, required to properly handle the Bluetooth OLEDs. The scramble code has been copied from drivers/hid/hid-wacom.c in the v3.16 kernel tree.
Review of attachment 282848 [details] [review]: ::: plugins/wacom/gsd-wacom-oled-helper.c @@ +271,3 @@ + if (parent) { + filename = get_oled_sysfs_path (parent, button_num); + if(access (filename, F_OK) != -1 ) { g_file_test() instead. @@ +276,3 @@ + device = parent; + } else { + g_free (filename); g_clear_pointer (&filename, g_free);
Review of attachment 282851 [details] [review]: ::: plugins/wacom/gsd-wacom-oled-helper.c @@ +117,3 @@ + +static void +gsd_wacom_oled_convert_1_bit (guchar *image) I'd prefer that being done in a separate commit instead. @@ +281,3 @@ const char * const subsystems[] = { "input", NULL }; int ret = 1; + int usb = 0; gboolean?
Review of attachment 282848 [details] [review]: Could we also refactor that slightly so we know what to remove when 3.17 is old in a couple of years?
Created attachment 283139 [details] [review] 0001-wacom-move-out-get_oled_sys_path-from-main.patch new in v2: refactor the main () function as requested in comment #5
Created attachment 283140 [details] [review] 0002-wacom-move-out-1-bit-handling-of-Bluetooth-OLED-buff.patch new in v2: move out the 1-bit conversion in a separate patch (review in comment #4)
Created attachment 283141 [details] [review] 0003-wacom-check-for-the-new-HID-unified-kernel-ABI-when-.patch v2: smaller, and more comprehensive patch. Very similar to the one in bug #734455
Created attachment 283142 [details] [review] 0004-wacom-new-kernels-required-the-Bluetooth-oled-data-t.patch v2: only contains the Bluetooth scrambling now
BTW, the patch series landed in Linus' tree, so I think we can now apply the patches once the reviews are allowing them.
Review of attachment 283139 [details] [review]: Looks fine otherwise. ::: plugins/wacom/gsd-wacom-oled-helper.c @@ +220,3 @@ + filename = get_oled_sysfs_path (parent, button_num); + *type = GSD_WACOM_OLED_TYPE_USB; + } else if (g_strrstr( g_udev_device_get_property (device, "DEVPATH"), "bluetooth")) { space before the parenthesis (even if I see that the bug was already there).
Review of attachment 283140 [details] [review]: Looks good.
Review of attachment 283141 [details] [review]: Looks good otherwise. ::: plugins/wacom/gsd-wacom-oled-helper.c @@ +233,3 @@ + } + g_clear_pointer (&filename, g_free); + filename = NULL; Huh, g_clear_pointer will already set "filename" to NULL, that's the benefit of using it.
Review of attachment 283142 [details] [review]: Looks good as well.
Created attachment 283209 [details] [review] wacom: move out get_oled_sys_path () from main Kernel 3.17 introduce a new ABI for Wacom devices (the path changed). In order to properly detect this and be able to differentiate both ABI, split out the retrieval of the OLED sysfs node first.
Created attachment 283210 [details] [review] wacom: move out 1-bit handling of Bluetooth OLED buffers No functional changes: - move out 1-bit packing into its own function - convert if () else if () else in a switch/case
Created attachment 283211 [details] [review] wacom: check for the new HID unified kernel ABI when setting OLEDs In kernel v3.17 and later, the Wacom OLED sysfs attribute is located under the HID node, not the USB one. This new path is now common between USB and Bluetooth, so no special handling is required for Bluetooth to retrieve this path. Check for its availability first, and then fall back on the old path.
Created attachment 283212 [details] [review] wacom: new kernels required the Bluetooth oled data to be scrambled The unified wacom kernel module now uses the same API for OLED either on Bluetooth or USB. The choice has been made to not scramble in the kernel the incoming data to keep the USB (most used) untouched. Implement the scrambling for bluetooth in g-s-d.
Attachment 283209 [details] pushed as cf41b6e - wacom: move out get_oled_sys_path () from main Attachment 283210 [details] pushed as 9dc8cf4 - wacom: move out 1-bit handling of Bluetooth OLED buffers Attachment 283211 [details] pushed as f08b12e - wacom: check for the new HID unified kernel ABI when setting OLEDs Attachment 283212 [details] pushed as b06555c - wacom: new kernels required the Bluetooth oled data to be scrambled