Messages in this thread | ![/](/images/icornerl.gif) | | From | Jason Gerecke <> | Date | Fri, 28 Jul 2017 08:15:28 -0700 | Subject | Re: [PATCH] HID: wacom: add USB_HID dependency |
| |
On Fri, Jul 28, 2017 at 7:29 AM, Arnd Bergmann <arnd@arndb.de> wrote: > On Fri, Jul 28, 2017 at 4:24 PM, Jason Gerecke <killertofu@gmail.com> wrote: >> On Fri, Jul 28, 2017 at 7:18 AM, Arnd Bergmann <arnd@arndb.de> wrote: >>> On Fri, Jul 28, 2017 at 4:07 PM, Jason Gerecke <killertofu@gmail.com> wrote: > >>> #ifdef CONFIG_USB_HID >>> extern bool hid_is_using_usb_driver(struct hid_device *hdev) >>> #else >>> static inline bool hid_is_using_usb_driver(struct hid_device *hdev) >>> { >>> return false; >>> } >>> #endif >>> >>> but is it worth it to avoid the dependency? >>> >>> Arnd >> >> I was thinking something more along the lines of the following since >> the idea of per-transport helper functions was dismissed earlier: >> >> #ifdef CONFIG_USB_HID >> if (hid_is_using_ll_driver(wacom->hdev, &usb_hid_driver)) { > > I would consider that rather ugly, a driver shouldn't really use > #ifdef like this, but you can hide stuff like this in a header. The method > I proposed also has the advantage of avoiding exporting the > usb_hid_driver object. Drivers shouldn't really need to access this, > and wacom_sys.c is the only remaining user of the export. > > Arnd
The exports and hid_is_using_ll_driver were actually introduced in the patch immediately preceding the change to wacom_sys.c which triggered this error (making it the "first", not "last" user).
That said, after reading through the patch discussion[1] again, I see that my memory is faulty: per-transport functions were *not* dismissed. Rather, a more-generic function that is fed the hid_ll_driver of interest was suggested instead. Given that these exports are liable to cause this same issue for future users, perhaps providing per-transport functions is the better option after all.
I could accept either the strict dependency you originally suggested or a modified header, but don't see much reason for the former. Checking if a HID device is using a particular transport shouldn't require that that transport even be available IMO, but that's ultimately not my call...
Jiri? Benjamin?
[1]: https://patchwork.kernel.org/patch/9815539/
Jason
| ![\](/images/icornerr.gif) |