Re: [PATCH v3 04/11] HID: haptic: introduce hid_haptic_device

[Lucas GISSOT <lucas.gissot.pro@gmail.com>]

Hi,
I applied Randy's patch and built with CONFIG_HID_MULTITOUCH=m and
CONFIG_HID_HAPTIC=m, it builts; but there is an ld error on hid_haptic_*
functions as soon as CONFIG_HID_MULTITOUCH is changed to yes.

Is this .config not something desired, and then HID_HAPTIC should not be
a tristate (if I understood correctly), or is there another problem?

Thanks,

Lucas

On Fri, Oct 10, 2025 at 03:30:05PM -0500, Jonathan Denose wrote:
> Hi all,
> 
> Thanks for looking into this.
> 
> On Fri, Oct 10, 2025 at 1:55 PM Randy Dunlap <rdunlap@infradead.org> wrote:
> >
> > Hi,
> >
> > I think I found it... see below.
> >
> >
> > On 10/9/25 11:48 PM, Thorsten Leemhuis wrote:
> > > [Top-Posting for easier consumption]
> > >
> > > Mainly writing this mail to bring Lucas GISSOT in here, who reported the
> > > same error yesterday here:
> > > https://lore.kernel.org/all/aOgvA8Jiofcnk2xb@ARSENIURE.localdomain/
> > >
> > > Lucas there suggested:
> > > """but it seems to me that the #if IS_ENABLED(CONFIG_HID_HAPTIC) in
> > > hid-haptic.h should be replaced by IS_BUILTIN(CONFIG_HID_HAPTIC) and
> > > Kconfig updated."""
> > >
> > > And Randy: Thx for the closer investigation! It explains some of the
> > > "that feels odd, am I holding this wrong" feeling I had when reporting this.
> > >
> > > Ciao, Thorsten
> > >
> > > On 10/10/25 06:50, Randy Dunlap wrote:
> > >> On 10/9/25 7:43 AM, Thorsten Leemhuis wrote:
> > >>> On 8/19/25 01:08, Jonathan Denose wrote:
> > >>>> From: Angela Czubak <aczubak@google.com>
> > >>>>
> > >>>> Define a new structure that contains simple haptic device configuration
> > >>>> as well as current state.
> > >>>> Add functions that recognize auto trigger and manual trigger reports
> > >>>> as well as save their addresses.Hi,
> > >>>> Verify that the pressure unit is either grams or newtons.
> > >>>> Mark the input device as a haptic touchpad if the unit is correct and
> > >>>> the reports are found.
> > >>>>  [...]
> > >>>> +config HID_HAPTIC
> > >>>> +  tristate "Haptic touchpad support"
> > >>>> +  default n
> > >>>> +  help
> > >>>> +  Support for touchpads with force sensors and haptic actuators instead of a
> > >>>> +  traditional button.
> > >>>> +  Adds extra parsing and FF device for the hid multitouch driver.
> > >>>> +  It can be used for Elan 2703 haptic touchpad.
> > >>>> +
> > >>>> +  If unsure, say N.
> > >>>> +
> > >>>>  menu "Special HID drivers"
> > >>>
> > >>> I suspect this change is related to a build error I ran into today:
> > >>>
> > >>>   MODPOST Module.symvers
> > >>> ERROR: modpost: "hid_haptic_init" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> ERROR: modpost: "hid_haptic_pressure_increase" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> ERROR: modpost: "hid_haptic_check_pressure_unit" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> ERROR: modpost: "hid_haptic_input_configured" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> ERROR: modpost: "hid_haptic_input_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> ERROR: modpost: "hid_haptic_feature_mapping" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> ERROR: modpost: "hid_haptic_pressure_reset" [drivers/hid/hid-multitouch.ko] undefined!
> > >>> make[3]: *** [/home/thl/var/linux.dev/scripts/Makefile.modpost:147: Module.symvers] Error 1
> > >>>
> > >>> The config where this occurred had this:
> > >>>
> > >>> CONFIG_HID=y
> > >>> CONFIG_HID_MULTITOUCH=m
> > >>> CONFIG_HID_HAPTIC=m
> > >>>
> > >>> Changing the latter to "CONFIG_HID_HAPTIC=y" fixed the problem for me.
> > >>
> > >> Sure, but that's just covering up the problem.
> > >>> First, I get this build error:
> > >>
> > >> ERROR: modpost: missing MODULE_LICENSE() in drivers/hid/hid-haptic.o
> > >> WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/hid/hid-haptic.o
> > >>
> >
> > ISTM that tristate is incompatible with this newly added Makefile
> > line:
> >
> > +hid-$(CONFIG_HID_HAPTIC)       += hid-haptic.o
> >
> > hid-* lists files that should be builtin, not loadable modules.
> > These should all be hid-y.  AFAIK, hid-m is not useful.
> > (A maintainer can correct me as needed.)
> >
> > So adding a MODULE_LICENSE() and MODULE_DESCRIPTION() to
> > hid-haptic.c and changing drivers/hid/Makefile to use
> > obj-$(CONFIG_HID_HAPTIC_        += hid-haptic.o
> >
> > fixes the build errors for me.
> >
> > Angela, Jonathan D., is there any reason that
> > hid-haptic needs to be builtin instead of a loadable
> > module?  It's no problem for hid-multitouch.ko to call
> > into hid-haptic.ko (both as loadable modules) as long as
> > hid-haptic.ko is loaded first.
> >
> As long as hid-multitouch.ko is able to call into hid-haptic.ko I
> don't see any issues, but is there a way to enforce that when
> CONFIG_HID_HAPTIC is enabled, hid-haptic.ko will be loaded before
> hid-multitouch.ko?
> >
> > Thanks.
> >
> > ~Randy
> > ---
> > ---
> >  drivers/hid/hid-haptic.c |    3 +++
> >  1 file changed, 3 insertions(+)
> >
> > --- linux.orig/drivers/hid/hid-haptic.c
> > +++ linux/drivers/hid/hid-haptic.c
> > @@ -10,6 +10,9 @@
> >
> >  #include "hid-haptic.h"
> >
> > +MODULE_DESCRIPTION("HID haptic touchpad support");
> > +MODULE_LICENSE("GPL");
> > +
> >  void hid_haptic_feature_mapping(struct hid_device *hdev,
> >                                 struct hid_haptic_device *haptic,
> >                                 struct hid_field *field, struct hid_usage *usage)
> > ---
> >  drivers/hid/Makefile |    3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > --- linux.orig/drivers/hid/Makefile
> > +++ linux/drivers/hid/Makefile
> > @@ -4,7 +4,8 @@
> >  #
> >  hid-y                  := hid-core.o hid-input.o hid-quirks.o
> >  hid-$(CONFIG_DEBUG_FS)         += hid-debug.o
> > -hid-$(CONFIG_HID_HAPTIC)       += hid-haptic.o
> > +
> > +obj-$(CONFIG_HID_HAPTIC)       += hid-haptic.o
> >
> >  obj-$(CONFIG_HID_BPF)          += bpf/
> >
> >
> --
> Jonathan