[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: HID: remove some unneeded exported symbols from hid.h

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1913 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=706392

---Test result---

Test Summary:
CheckPatch                    PASS      6.97 seconds
GitLint                       PASS      2.49 seconds
SubjectPrefix                 FAIL      0.98 seconds
BuildKernel                   PASS      38.63 seconds
CheckAllWarning               PASS      41.68 seconds
CheckSparse                   PASS      47.37 seconds
BuildKernel32                 PASS      37.00 seconds
TestRunnerSetup               PASS      522.93 seconds
TestRunner_l2cap-tester       PASS      18.46 seconds
TestRunner_iso-tester         PASS      20.10 seconds
TestRunner_bnep-tester        PASS      6.85 seconds
TestRunner_mgmt-tester        PASS      125.54 seconds
TestRunner_rfcomm-tester      PASS      10.65 seconds
TestRunner_sco-tester         PASS      9.85 seconds
TestRunner_ioctl-tester       PASS      11.63 seconds
TestRunner_mesh-tester        PASS      8.59 seconds
TestRunner_smp-tester         PASS      9.65 seconds
TestRunner_userchan-tester    PASS      7.27 seconds
IncrementalBuild              PASS      76.47 seconds

Details
##############################
Test: SubjectPrefix - FAIL
Desc: Check subject contains "Bluetooth" prefix
Output:
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject
"Bluetooth: " prefix is not specified in the subject


---
Regards,
Linux Bluetooth

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