[PATCH 1/8] HID: letsketch: Use hid_is_usb()

[PATCH 1/8] HID: letsketch: Use hid_is_usb()

[Thomas Weißschuh]

Don't open code existing functionality.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hid/hid-letsketch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/hid-letsketch.c b/drivers/hid/hid-letsketch.c
index 74d17cf518ba..97f047f18136 100644
--- a/drivers/hid/hid-letsketch.c
+++ b/drivers/hid/hid-letsketch.c
@@ -238,7 +238,7 @@ static int letsketch_probe(struct hid_device *hdev, const struct hid_device_id *
 	char buf[256];
 	int i, ret;
 
-	if (!hid_is_using_ll_driver(hdev, &usb_hid_driver))
+	if (!hid_is_usb(hdev))
 		return -ENODEV;
 
 	intf = to_usb_interface(hdev->dev.parent);

-- 
2.39.0

[PATCH 0/8] HID: remove some unneeded exported symbols from hid.h

[Thomas Weißschuh]

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,
-- 
Thomas Weißschuh <linux@weissschuh.net>

[PATCH 2/8] HID: usbhid: Make hid_is_usb() non-inline

[Thomas Weißschuh]

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);
 
 #define	PM_HINT_FULLON	1<<5
 #define PM_HINT_NORMAL	1<<1

-- 
2.39.0

[PATCH 3/8] HID: Remove unused function hid_is_using_ll_driver()

[Thomas Weißschuh]

As the last user was removed we can delete this function.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/linux/hid.h | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index e8400aa78522..7c5fce6a189e 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -858,12 +858,6 @@ extern struct hid_ll_driver hidp_hid_driver;
 extern struct hid_ll_driver uhid_hid_driver;
 extern struct hid_ll_driver usb_hid_driver;
 
-static inline bool hid_is_using_ll_driver(struct hid_device *hdev,
-		struct hid_ll_driver *driver)
-{
-	return hdev->ll_driver == driver;
-}
-
 extern bool hid_is_usb(const struct hid_device *hdev);
 
 #define	PM_HINT_FULLON	1<<5

-- 
2.39.0

[PATCH 4/8] HID: Unexport struct usb_hid_driver

[Thomas Weißschuh]

As no external users remain this implementation detail does not need to
be exported anymore.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hid/usbhid/hid-core.c | 3 +--
 include/linux/hid.h           | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 54b0280d0073..4143bab3380a 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1318,7 +1318,7 @@ static bool usbhid_may_wakeup(struct hid_device *hid)
 	return device_may_wakeup(&dev->dev);
 }
 
-struct hid_ll_driver usb_hid_driver = {
+static struct hid_ll_driver usb_hid_driver = {
 	.parse = usbhid_parse,
 	.start = usbhid_start,
 	.stop = usbhid_stop,
@@ -1332,7 +1332,6 @@ struct hid_ll_driver usb_hid_driver = {
 	.idle = usbhid_idle,
 	.may_wakeup = usbhid_may_wakeup,
 };
-EXPORT_SYMBOL_GPL(usb_hid_driver);
 
 bool hid_is_usb(const struct hid_device *hdev)
 {
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 7c5fce6a189e..170cad696541 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -856,7 +856,6 @@ struct hid_ll_driver {
 extern struct hid_ll_driver i2c_hid_ll_driver;
 extern struct hid_ll_driver hidp_hid_driver;
 extern struct hid_ll_driver uhid_hid_driver;
-extern struct hid_ll_driver usb_hid_driver;
 
 extern bool hid_is_usb(const struct hid_device *hdev);
 

-- 
2.39.0

[PATCH 5/8] HID: Unexport struct uhid_hid_driver

[Thomas Weißschuh]

As there are no external users this implementation detail does not need
to be exported.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hid/uhid.c  | 3 +--
 include/linux/hid.h | 1 -
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 2a918aeb0af1..6cec0614fc98 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -387,7 +387,7 @@ static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
 	return uhid_hid_output_raw(hid, buf, count, HID_OUTPUT_REPORT);
 }
 
-struct hid_ll_driver uhid_hid_driver = {
+static struct hid_ll_driver uhid_hid_driver = {
 	.start = uhid_hid_start,
 	.stop = uhid_hid_stop,
 	.open = uhid_hid_open,
@@ -396,7 +396,6 @@ struct hid_ll_driver uhid_hid_driver = {
 	.raw_request = uhid_hid_raw_request,
 	.output_report = uhid_hid_output_report,
 };
-EXPORT_SYMBOL_GPL(uhid_hid_driver);
 
 #ifdef CONFIG_COMPAT
 
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 170cad696541..3fcc47a9d0e8 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -855,7 +855,6 @@ struct hid_ll_driver {
 
 extern struct hid_ll_driver i2c_hid_ll_driver;
 extern struct hid_ll_driver hidp_hid_driver;
-extern struct hid_ll_driver uhid_hid_driver;
 
 extern bool hid_is_usb(const struct hid_device *hdev);
 

-- 
2.39.0

[PATCH 6/8] HID: Unexport struct hidp_hid_driver

[Thomas Weißschuh]

As there are no external users this implementation detail does not need
to be exported.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 include/linux/hid.h       | 1 -
 net/bluetooth/hidp/core.c | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/include/linux/hid.h b/include/linux/hid.h
index 3fcc47a9d0e8..21017e1ddbdb 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -854,7 +854,6 @@ struct hid_ll_driver {
 };
 
 extern struct hid_ll_driver i2c_hid_ll_driver;
-extern struct hid_ll_driver hidp_hid_driver;
 
 extern bool hid_is_usb(const struct hid_device *hdev);
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index cc20e706c639..c4a741f6ed5c 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -739,7 +739,7 @@ static void hidp_stop(struct hid_device *hid)
 	hid->claimed = 0;
 }
 
-struct hid_ll_driver hidp_hid_driver = {
+static struct hid_ll_driver hidp_hid_driver = {
 	.parse = hidp_parse,
 	.start = hidp_start,
 	.stop = hidp_stop,
@@ -748,7 +748,6 @@ struct hid_ll_driver hidp_hid_driver = {
 	.raw_request = hidp_raw_request,
 	.output_report = hidp_output_report,
 };
-EXPORT_SYMBOL_GPL(hidp_hid_driver);
 
 /* This function sets up the hid device. It does not add it
    to the HID system. That is done in hidp_add_connection(). */

-- 
2.39.0

[PATCH 7/8] HID: Unexport struct i2c_hid_ll_driver

[Thomas Weißschuh]

As there are no external users this implementation detail does not need
to be exported.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 3 +--
 include/linux/hid.h                | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index b86b62f97108..fc5a0dd4eb92 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -842,7 +842,7 @@ static void i2c_hid_close(struct hid_device *hid)
 	clear_bit(I2C_HID_STARTED, &ihid->flags);
 }
 
-struct hid_ll_driver i2c_hid_ll_driver = {
+static struct hid_ll_driver i2c_hid_ll_driver = {
 	.parse = i2c_hid_parse,
 	.start = i2c_hid_start,
 	.stop = i2c_hid_stop,
@@ -851,7 +851,6 @@ struct hid_ll_driver i2c_hid_ll_driver = {
 	.output_report = i2c_hid_output_report,
 	.raw_request = i2c_hid_raw_request,
 };
-EXPORT_SYMBOL_GPL(i2c_hid_ll_driver);
 
 static int i2c_hid_init_irq(struct i2c_client *client)
 {
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 21017e1ddbdb..60a092150bc6 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -853,8 +853,6 @@ struct hid_ll_driver {
 	bool (*may_wakeup)(struct hid_device *hdev);
 };
 
-extern struct hid_ll_driver i2c_hid_ll_driver;
-
 extern bool hid_is_usb(const struct hid_device *hdev);
 
 #define	PM_HINT_FULLON	1<<5

-- 
2.39.0

[PATCH 8/8] HID: Make lowlevel driver structs const

[Thomas Weißschuh]

Nothing is nor should be modifying these structs so mark them as const.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 drivers/hid/i2c-hid/i2c-hid-core.c | 2 +-
 drivers/hid/uhid.c                 | 2 +-
 drivers/hid/usbhid/hid-core.c      | 2 +-
 include/linux/hid.h                | 2 +-
 net/bluetooth/hidp/core.c          | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index fc5a0dd4eb92..af98ac31c8d4 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -842,7 +842,7 @@ static void i2c_hid_close(struct hid_device *hid)
 	clear_bit(I2C_HID_STARTED, &ihid->flags);
 }
 
-static struct hid_ll_driver i2c_hid_ll_driver = {
+static const struct hid_ll_driver i2c_hid_ll_driver = {
 	.parse = i2c_hid_parse,
 	.start = i2c_hid_start,
 	.stop = i2c_hid_stop,
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 6cec0614fc98..f161c95a1ad2 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -387,7 +387,7 @@ static int uhid_hid_output_report(struct hid_device *hid, __u8 *buf,
 	return uhid_hid_output_raw(hid, buf, count, HID_OUTPUT_REPORT);
 }
 
-static struct hid_ll_driver uhid_hid_driver = {
+static const struct hid_ll_driver uhid_hid_driver = {
 	.start = uhid_hid_start,
 	.stop = uhid_hid_stop,
 	.open = uhid_hid_open,
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 4143bab3380a..257dd73e37bf 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1318,7 +1318,7 @@ static bool usbhid_may_wakeup(struct hid_device *hid)
 	return device_may_wakeup(&dev->dev);
 }
 
-static struct hid_ll_driver usb_hid_driver = {
+static const struct hid_ll_driver usb_hid_driver = {
 	.parse = usbhid_parse,
 	.start = usbhid_start,
 	.stop = usbhid_stop,
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 60a092150bc6..39a374c7fbac 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -595,7 +595,7 @@ struct hid_device {							/* device report descriptor */
 	struct device dev;						/* device */
 	struct hid_driver *driver;
 
-	struct hid_ll_driver *ll_driver;
+	const struct hid_ll_driver *ll_driver;
 	struct mutex ll_open_lock;
 	unsigned int ll_open_count;
 
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index c4a741f6ed5c..bed1a7b9205c 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -739,7 +739,7 @@ static void hidp_stop(struct hid_device *hid)
 	hid->claimed = 0;
 }
 
-static struct hid_ll_driver hidp_hid_driver = {
+static const struct hid_ll_driver hidp_hid_driver = {
 	.parse = hidp_parse,
 	.start = hidp_start,
 	.stop = hidp_stop,

-- 
2.39.0

Re: [PATCH 0/8] HID: remove some unneeded exported symbols from hid.h

[David Rheinsberg]

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

Re: [PATCH 2/8] HID: usbhid: Make hid_is_usb() non-inline

[Benjamin Tissoires]

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
>

Re: [PATCH 2/8] HID: usbhid: Make hid_is_usb() non-inline

[Thomas Weißschuh]

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
>>

Re: [PATCH 2/8] HID: usbhid: Make hid_is_usb() non-inline

[Benjamin Tissoires]

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
> >>
>

Re: [PATCH 0/8] HID: remove some unneeded exported symbols from hid.h

[Hans de Goede]

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,

Re: [PATCH 0/8] HID: remove some unneeded exported symbols from hid.h

[Jiri Kosina]

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,

-- 
Jiri Kosina
SUSE Labs