| Message ID | 20221222-hid-v1-0-f4a6c35487a5@weissschuh.net |
|---|---|
| Headers | show |
| Series | HID: remove some unneeded exported symbols from hid.h | expand |
Hi On Thu, 22 Dec 2022 at 06:10, Thomas Weißschuh <linux@weissschuh.net> wrote: > Small cleanup to get rid of exports of the lowlevel hid drivers and to make > them const. [...] > Thomas Weißschuh (8): > HID: letsketch: Use hid_is_usb() > HID: usbhid: Make hid_is_usb() non-inline > HID: Remove unused function hid_is_using_ll_driver() > HID: Unexport struct usb_hid_driver > HID: Unexport struct uhid_hid_driver > HID: Unexport struct hidp_hid_driver > HID: Unexport struct i2c_hid_ll_driver > HID: Make lowlevel driver structs const Yeah, it makes sense to avoid exposing the structs. Reviewed-by: David Rheinsberg <david.rheinsberg@gmail.com> Thanks David
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 > >> >
Hi, On 12/22/22 06:10, Thomas Weißschuh wrote: > Small cleanup to get rid of exports of the lowlevel hid drivers and to make > them const. Thanks, the entire series looks good to me: Reviewed-by: Hans de Goede <hdegoede@redhat.com> for the series. Regards, Hans > > To: Hans de Goede <hdegoede@redhat.com> > To: Jiri Kosina <jikos@kernel.org> > To: Benjamin Tissoires <benjamin.tissoires@redhat.com> > To: David Rheinsberg <david.rheinsberg@gmail.com> > To: Marcel Holtmann <marcel@holtmann.org> > To: Johan Hedberg <johan.hedberg@gmail.com> > To: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > To: "David S. Miller" <davem@davemloft.net> > To: Eric Dumazet <edumazet@google.com> > To: Jakub Kicinski <kuba@kernel.org> > To: Paolo Abeni <pabeni@redhat.com> > Cc: linux-input@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-usb@vger.kernel.org > Cc: linux-bluetooth@vger.kernel.org > Cc: netdev@vger.kernel.org > > --- > Thomas Weißschuh (8): > HID: letsketch: Use hid_is_usb() > HID: usbhid: Make hid_is_usb() non-inline > HID: Remove unused function hid_is_using_ll_driver() > HID: Unexport struct usb_hid_driver > HID: Unexport struct uhid_hid_driver > HID: Unexport struct hidp_hid_driver > HID: Unexport struct i2c_hid_ll_driver > HID: Make lowlevel driver structs const > > drivers/hid/hid-letsketch.c | 2 +- > drivers/hid/i2c-hid/i2c-hid-core.c | 3 +-- > drivers/hid/uhid.c | 3 +-- > drivers/hid/usbhid/hid-core.c | 9 +++++++-- > include/linux/hid.h | 18 ++---------------- > net/bluetooth/hidp/core.c | 3 +-- > 6 files changed, 13 insertions(+), 25 deletions(-) > --- > base-commit: d264dd3bbbd16b56239e889023fbe49413a58eaf > change-id: 20221222-hid-b9551f9fa236 > > Best regards,
On Thu, 22 Dec 2022, Thomas Weißschuh wrote: > Small cleanup to get rid of exports of the lowlevel hid drivers and to make > them const. > > To: Hans de Goede <hdegoede@redhat.com> > To: Jiri Kosina <jikos@kernel.org> > To: Benjamin Tissoires <benjamin.tissoires@redhat.com> > To: David Rheinsberg <david.rheinsberg@gmail.com> > To: Marcel Holtmann <marcel@holtmann.org> > To: Johan Hedberg <johan.hedberg@gmail.com> > To: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > To: "David S. Miller" <davem@davemloft.net> > To: Eric Dumazet <edumazet@google.com> > To: Jakub Kicinski <kuba@kernel.org> > To: Paolo Abeni <pabeni@redhat.com> > Cc: linux-input@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Cc: linux-usb@vger.kernel.org > Cc: linux-bluetooth@vger.kernel.org > Cc: netdev@vger.kernel.org > > --- > Thomas Weißschuh (8): > HID: letsketch: Use hid_is_usb() > HID: usbhid: Make hid_is_usb() non-inline > HID: Remove unused function hid_is_using_ll_driver() > HID: Unexport struct usb_hid_driver > HID: Unexport struct uhid_hid_driver > HID: Unexport struct hidp_hid_driver > HID: Unexport struct i2c_hid_ll_driver > HID: Make lowlevel driver structs const > > drivers/hid/hid-letsketch.c | 2 +- > drivers/hid/i2c-hid/i2c-hid-core.c | 3 +-- > drivers/hid/uhid.c | 3 +-- > drivers/hid/usbhid/hid-core.c | 9 +++++++-- > include/linux/hid.h | 18 ++---------------- > net/bluetooth/hidp/core.c | 3 +-- > 6 files changed, 13 insertions(+), 25 deletions(-) Applied to hid.git#for-6.3/hid-core. Thanks,
Small cleanup to get rid of exports of the lowlevel hid drivers and to make them const. To: Hans de Goede <hdegoede@redhat.com> To: Jiri Kosina <jikos@kernel.org> To: Benjamin Tissoires <benjamin.tissoires@redhat.com> To: David Rheinsberg <david.rheinsberg@gmail.com> To: Marcel Holtmann <marcel@holtmann.org> To: Johan Hedberg <johan.hedberg@gmail.com> To: Luiz Augusto von Dentz <luiz.dentz@gmail.com> To: "David S. Miller" <davem@davemloft.net> To: Eric Dumazet <edumazet@google.com> To: Jakub Kicinski <kuba@kernel.org> To: Paolo Abeni <pabeni@redhat.com> Cc: linux-input@vger.kernel.org Cc: linux-kernel@vger.kernel.org Cc: linux-usb@vger.kernel.org Cc: linux-bluetooth@vger.kernel.org Cc: netdev@vger.kernel.org --- Thomas Weißschuh (8): HID: letsketch: Use hid_is_usb() HID: usbhid: Make hid_is_usb() non-inline HID: Remove unused function hid_is_using_ll_driver() HID: Unexport struct usb_hid_driver HID: Unexport struct uhid_hid_driver HID: Unexport struct hidp_hid_driver HID: Unexport struct i2c_hid_ll_driver HID: Make lowlevel driver structs const drivers/hid/hid-letsketch.c | 2 +- drivers/hid/i2c-hid/i2c-hid-core.c | 3 +-- drivers/hid/uhid.c | 3 +-- drivers/hid/usbhid/hid-core.c | 9 +++++++-- include/linux/hid.h | 18 ++---------------- net/bluetooth/hidp/core.c | 3 +-- 6 files changed, 13 insertions(+), 25 deletions(-) --- base-commit: d264dd3bbbd16b56239e889023fbe49413a58eaf change-id: 20221222-hid-b9551f9fa236 Best regards,