Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752592AbdG1PPc (ORCPT ); Fri, 28 Jul 2017 11:15:32 -0400 Received: from mail-ua0-f196.google.com ([209.85.217.196]:36400 "EHLO mail-ua0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751975AbdG1PPa (ORCPT ); Fri, 28 Jul 2017 11:15:30 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170728131826.180483-1-arnd@arndb.de> <6C9C978F-447D-42A5-9245-894577F2944D@gmail.com> From: Jason Gerecke Date: Fri, 28 Jul 2017 08:15:28 -0700 Message-ID: Subject: Re: [PATCH] HID: wacom: add USB_HID dependency To: Arnd Bergmann Cc: Jiri Kosina , Jason Gerecke , Benjamin Tissoires , "open list:HID CORE LAYER" , Linux Kernel Mailing List Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2154 Lines: 54 On Fri, Jul 28, 2017 at 7:29 AM, Arnd Bergmann wrote: > On Fri, Jul 28, 2017 at 4:24 PM, Jason Gerecke wrote: >> On Fri, Jul 28, 2017 at 7:18 AM, Arnd Bergmann wrote: >>> On Fri, Jul 28, 2017 at 4:07 PM, Jason Gerecke 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