Small cleanup to get rid of exports of the lowlevel hid drivers and to make
them const.
To: Hans de Goede <[email protected]>
To: Jiri Kosina <[email protected]>
To: Benjamin Tissoires <[email protected]>
To: David Rheinsberg <[email protected]>
To: Marcel Holtmann <[email protected]>
To: Johan Hedberg <[email protected]>
To: Luiz Augusto von Dentz <[email protected]>
To: "David S. Miller" <[email protected]>
To: Eric Dumazet <[email protected]>
To: Jakub Kicinski <[email protected]>
To: Paolo Abeni <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
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 <[email protected]>
As there are no external users this implementation detail does not need
to be exported.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
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
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 <[email protected]>
---
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
Nothing is nor should be modifying these structs so mark them as const.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
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
As the last user was removed we can delete this function.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
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
As there are no external users this implementation detail does not need
to be exported.
Signed-off-by: Thomas Weißschuh <[email protected]>
---
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
Hi
On Thu, 22 Dec 2022 at 06:10, Thomas Weißschuh <[email protected]> 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 <[email protected]>
Thanks
David
On Thu, Dec 22, 2022 at 6:16 AM Thomas Weißschuh <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>:
> On Thu, Dec 22, 2022 at 6:16 AM Thomas Weißschuh <[email protected]> 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 <[email protected]>
>> ---
>> 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 <[email protected]> wrote:
>
>
> Dec 22, 2022 16:13:06 Benjamin Tissoires <[email protected]>:
>
> > On Thu, Dec 22, 2022 at 6:16 AM Thomas Weißschuh <[email protected]> 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 <[email protected]>
> >> ---
> >> 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 <[email protected]>
for the series.
Regards,
Hans
>
> To: Hans de Goede <[email protected]>
> To: Jiri Kosina <[email protected]>
> To: Benjamin Tissoires <[email protected]>
> To: David Rheinsberg <[email protected]>
> To: Marcel Holtmann <[email protected]>
> To: Johan Hedberg <[email protected]>
> To: Luiz Augusto von Dentz <[email protected]>
> To: "David S. Miller" <[email protected]>
> To: Eric Dumazet <[email protected]>
> To: Jakub Kicinski <[email protected]>
> To: Paolo Abeni <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> ---
> 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 <[email protected]>
> To: Jiri Kosina <[email protected]>
> To: Benjamin Tissoires <[email protected]>
> To: David Rheinsberg <[email protected]>
> To: Marcel Holtmann <[email protected]>
> To: Johan Hedberg <[email protected]>
> To: Luiz Augusto von Dentz <[email protected]>
> To: "David S. Miller" <[email protected]>
> To: Eric Dumazet <[email protected]>
> To: Jakub Kicinski <[email protected]>
> To: Paolo Abeni <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
>
> ---
> 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