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:

quoted

On Wed, May 29, 2024 at 11:48:05AM +0200, Arnd Bergmann wrote:

quoted

quoted

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

quoted

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