| Message ID | 20221222-hid-v1-2-f4a6c35487a5@weissschuh.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | HID: remove some unneeded exported symbols from hid.h | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | fail | "Bluetooth: " prefix is not specified in the subject |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
On Thu, Dec 22, 2022 at 6:16 AM Thomas Weißschuh <linux@weissschuh.net> wrote: > > By making hid_is_usb() a non-inline function the lowlevel usbhid driver > does not have to be exported anymore. > > Also mark the argument as const as it is not modified. > > Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > --- > drivers/hid/usbhid/hid-core.c | 6 ++++++ > include/linux/hid.h | 5 +---- > 2 files changed, 7 insertions(+), 4 deletions(-) > > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > index be4c731aaa65..54b0280d0073 100644 > --- a/drivers/hid/usbhid/hid-core.c > +++ b/drivers/hid/usbhid/hid-core.c > @@ -1334,6 +1334,12 @@ struct hid_ll_driver usb_hid_driver = { > }; > EXPORT_SYMBOL_GPL(usb_hid_driver); > > +bool hid_is_usb(const struct hid_device *hdev) > +{ > + return hdev->ll_driver == &usb_hid_driver; > +} > +EXPORT_SYMBOL_GPL(hid_is_usb); > + > static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) > { > struct usb_host_interface *interface = intf->cur_altsetting; > diff --git a/include/linux/hid.h b/include/linux/hid.h > index 8677ae38599e..e8400aa78522 100644 > --- a/include/linux/hid.h > +++ b/include/linux/hid.h > @@ -864,10 +864,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev, > return hdev->ll_driver == driver; > } > > -static inline bool hid_is_usb(struct hid_device *hdev) > -{ > - return hid_is_using_ll_driver(hdev, &usb_hid_driver); > -} > +extern bool hid_is_usb(const struct hid_device *hdev); The problem here is that CONFIG_USB_HID can be set to either m or n. In the n case, you'll end up with an undefined symbol, in the m case, it won't link too if CONFIG_HID is set to Y (and it'll be quite a mess to call it if the module is not loaded yet). Cheers, Benjamin > > > #define PM_HINT_FULLON 1<<5 > #define PM_HINT_NORMAL 1<<1 > > -- > 2.39.0 >
Dec 22, 2022 16:13:06 Benjamin Tissoires <benjamin.tissoires@redhat.com>: > On Thu, Dec 22, 2022 at 6:16 AM Thomas Weißschuh <linux@weissschuh.net> wrote: >> >> By making hid_is_usb() a non-inline function the lowlevel usbhid driver >> does not have to be exported anymore. >> >> Also mark the argument as const as it is not modified. >> >> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> >> --- >> drivers/hid/usbhid/hid-core.c | 6 ++++++ >> include/linux/hid.h | 5 +---- >> 2 files changed, 7 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c >> index be4c731aaa65..54b0280d0073 100644 >> --- a/drivers/hid/usbhid/hid-core.c >> +++ b/drivers/hid/usbhid/hid-core.c >> @@ -1334,6 +1334,12 @@ struct hid_ll_driver usb_hid_driver = { >> }; >> EXPORT_SYMBOL_GPL(usb_hid_driver); >> >> +bool hid_is_usb(const struct hid_device *hdev) >> +{ >> + return hdev->ll_driver == &usb_hid_driver; >> +} >> +EXPORT_SYMBOL_GPL(hid_is_usb); >> + >> static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) >> { >> struct usb_host_interface *interface = intf->cur_altsetting; >> diff --git a/include/linux/hid.h b/include/linux/hid.h >> index 8677ae38599e..e8400aa78522 100644 >> --- a/include/linux/hid.h >> +++ b/include/linux/hid.h >> @@ -864,10 +864,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev, >> return hdev->ll_driver == driver; >> } >> >> -static inline bool hid_is_usb(struct hid_device *hdev) >> -{ >> - return hid_is_using_ll_driver(hdev, &usb_hid_driver); >> -} >> +extern bool hid_is_usb(const struct hid_device *hdev); > > The problem here is that CONFIG_USB_HID can be set to either m or n. > In the n case, you'll end up with an undefined symbol, in the m case, > it won't link too if CONFIG_HID is set to Y (and it'll be quite a mess > to call it if the module is not loaded yet). Shouldn't we already have the same problem with the symbol usb_hid_driver itself that is defined right next to the new hid_is_usb()? Thomas >> >> #define PM_HINT_FULLON 1<<5 >> #define PM_HINT_NORMAL 1<<1 >> >> -- >> 2.39.0 >>
On Thu, Dec 22, 2022 at 10:46 PM Thomas Weißschuh <thomas@t-8ch.de> wrote: > > > Dec 22, 2022 16:13:06 Benjamin Tissoires <benjamin.tissoires@redhat.com>: > > > On Thu, Dec 22, 2022 at 6:16 AM Thomas Weißschuh <linux@weissschuh.net> wrote: > >> > >> By making hid_is_usb() a non-inline function the lowlevel usbhid driver > >> does not have to be exported anymore. > >> > >> Also mark the argument as const as it is not modified. > >> > >> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> > >> --- > >> drivers/hid/usbhid/hid-core.c | 6 ++++++ > >> include/linux/hid.h | 5 +---- > >> 2 files changed, 7 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c > >> index be4c731aaa65..54b0280d0073 100644 > >> --- a/drivers/hid/usbhid/hid-core.c > >> +++ b/drivers/hid/usbhid/hid-core.c > >> @@ -1334,6 +1334,12 @@ struct hid_ll_driver usb_hid_driver = { > >> }; > >> EXPORT_SYMBOL_GPL(usb_hid_driver); > >> > >> +bool hid_is_usb(const struct hid_device *hdev) > >> +{ > >> + return hdev->ll_driver == &usb_hid_driver; > >> +} > >> +EXPORT_SYMBOL_GPL(hid_is_usb); > >> + > >> static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) > >> { > >> struct usb_host_interface *interface = intf->cur_altsetting; > >> diff --git a/include/linux/hid.h b/include/linux/hid.h > >> index 8677ae38599e..e8400aa78522 100644 > >> --- a/include/linux/hid.h > >> +++ b/include/linux/hid.h > >> @@ -864,10 +864,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev, > >> return hdev->ll_driver == driver; > >> } > >> > >> -static inline bool hid_is_usb(struct hid_device *hdev) > >> -{ > >> - return hid_is_using_ll_driver(hdev, &usb_hid_driver); > >> -} > >> +extern bool hid_is_usb(const struct hid_device *hdev); > > > > The problem here is that CONFIG_USB_HID can be set to either m or n. > > In the n case, you'll end up with an undefined symbol, in the m case, > > it won't link too if CONFIG_HID is set to Y (and it'll be quite a mess > > to call it if the module is not loaded yet). > > Shouldn't we already have the same problem with > the symbol usb_hid_driver itself that is defined > right next to the new hid_is_usb()? Yeah, sorry, my bad. All of the callers of this function are modules which depend on CONFIG_USB_HID in the Kconfig, so we should be good. Sorry for the noise. I shouldn't do reviews at 10pm :( Cheers, Benjamin > > Thomas > > >> > >> #define PM_HINT_FULLON 1<<5 > >> #define PM_HINT_NORMAL 1<<1 > >> > >> -- > >> 2.39.0 > >> >
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c index be4c731aaa65..54b0280d0073 100644 --- a/drivers/hid/usbhid/hid-core.c +++ b/drivers/hid/usbhid/hid-core.c @@ -1334,6 +1334,12 @@ struct hid_ll_driver usb_hid_driver = { }; EXPORT_SYMBOL_GPL(usb_hid_driver); +bool hid_is_usb(const struct hid_device *hdev) +{ + return hdev->ll_driver == &usb_hid_driver; +} +EXPORT_SYMBOL_GPL(hid_is_usb); + static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *id) { struct usb_host_interface *interface = intf->cur_altsetting; diff --git a/include/linux/hid.h b/include/linux/hid.h index 8677ae38599e..e8400aa78522 100644 --- a/include/linux/hid.h +++ b/include/linux/hid.h @@ -864,10 +864,7 @@ static inline bool hid_is_using_ll_driver(struct hid_device *hdev, return hdev->ll_driver == driver; } -static inline bool hid_is_usb(struct hid_device *hdev) -{ - return hid_is_using_ll_driver(hdev, &usb_hid_driver); -} +extern bool hid_is_usb(const struct hid_device *hdev); #define PM_HINT_FULLON 1<<5 #define PM_HINT_NORMAL 1<<1
By making hid_is_usb() a non-inline function the lowlevel usbhid driver does not have to be exported anymore. Also mark the argument as const as it is not modified. Signed-off-by: Thomas Weißschuh <linux@weissschuh.net> --- drivers/hid/usbhid/hid-core.c | 6 ++++++ include/linux/hid.h | 5 +---- 2 files changed, 7 insertions(+), 4 deletions(-)