[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zmcwlt6Kfpt09tKi@fedora>
Date: Mon, 10 Jun 2024 18:57:58 +0200
From: José Expósito <jose.exposito89@...il.com>
To: Arnd Bergmann <arnd@...db.de>
Cc: Arnd Bergmann <arnd@...nel.org>, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>,
Rahul Rameshbabu <rrameshbabu@...dia.com>,
Fabio Baltieri <fabiobaltieri@...omium.org>,
Ivan Gorinov <linux-kernel@...imeter.info>,
Johannes Roith <johannes@...-linux.rocks>,
linux-input@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] HID: uclogic: avoid linking common code into multiple
modules
On Mon, Jun 10, 2024 at 08:24:51AM +0200, Arnd Bergmann wrote:
> On Sat, Jun 8, 2024, at 20:28, José Expósito wrote:
> > On Wed, May 29, 2024 at 11:48:05AM +0200, Arnd Bergmann wrote:
>
> >> @@ -154,10 +152,8 @@ obj-$(CONFIG_HID_WINWING) += hid-winwing.o
> >> obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
> >> obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o
> >>
> >> -hid-uclogic-test-objs := hid-uclogic-rdesc.o \
> >> - hid-uclogic-params.o \
> >> - hid-uclogic-rdesc-test.o
> >> -obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic-test.o
> >> +hid-uclogic-test-objs := hid-uclogic-rdesc-test.o
> >> +obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic-test.o hid-uclogic-params.o hid-uclogic-params.o
> >>
> >> obj-$(CONFIG_USB_HID) += usbhid/
> >> obj-$(CONFIG_USB_MOUSE) += usbhid/
> >
> > I tested your patch with:
> >
> > hid-uclogic-objs := hid-uclogic-core.o \
> > hid-uclogic-rdesc.o \
> > hid-uclogic-params.o
> > obj-$(CONFIG_HID_UCLOGIC) += hid-uclogic.o
> > [...]
> > hid-uclogic-test-objs := hid-uclogic-rdesc-test.o
> > obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic.o hid-uclogic-test.o
> >
> > And I think it is a bit more clear and it looks like it does the trick
> > removing the warning.
>
> Right, that seems fine.
>
> > Also, with that change only "EXPORT_SYMBOL_GPL(uclogic_rdesc_template_apply);"
> > is required. The other EXPORT_SYMBOL_GPL can be removed.
> >
> > However, I'm not sure about what are the best practices using EXPORT_SYMBOL_GPL
> > and if it should be used for each function/data in the .h file. Maybe that's
> > why you added them.
>
> No, having only the single export is better here, you should
> have as few of them as possible. I did picked the more complicated
> approach as I wasn't sure if loading the entire driver from the
> test module caused any problems. Let's use your simpler patch
> then.
>
> Arnd
Turns out that, since the last time I checked the KUnit docs,
we have "EXPORT_SYMBOL_IF_KUNIT" available now.
I think we can use it and your final patch, without the MODULE_*
changes, could look like:
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index ce71b53ea6c5..e40f1ddebbb7 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -154,10 +154,8 @@ obj-$(CONFIG_HID_WINWING) += hid-winwing.o
obj-$(CONFIG_HID_SENSOR_HUB) += hid-sensor-hub.o
obj-$(CONFIG_HID_SENSOR_CUSTOM_SENSOR) += hid-sensor-custom.o
-hid-uclogic-test-objs := hid-uclogic-rdesc.o \
- hid-uclogic-params.o \
- hid-uclogic-rdesc-test.o
-obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic-test.o
+hid-uclogic-test-objs := hid-uclogic-rdesc-test.o
+obj-$(CONFIG_HID_KUNIT_TEST) += hid-uclogic.o hid-uclogic-test.o
obj-$(CONFIG_USB_HID) += usbhid/
obj-$(CONFIG_USB_MOUSE) += usbhid/
diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index b6dfdf6356a6..6c7a90417569 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c
@@ -17,6 +17,7 @@
#include "hid-uclogic-rdesc.h"
#include <linux/slab.h>
#include <asm/unaligned.h>
+#include <kunit/visibility.h>
/* Fixed WP4030U report descriptor */
__u8 uclogic_rdesc_wp4030u_fixed_arr[] = {
@@ -1242,3 +1243,4 @@ __u8 *uclogic_rdesc_template_apply(const __u8 *template_ptr,
return rdesc_ptr;
}
+EXPORT_SYMBOL_IF_KUNIT(uclogic_rdesc_template_apply);
I hope that helps,
Jose
Powered by blists - more mailing lists