[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMCVhVMO4y9P=Y3SWvY6BvA1sUK2o=Gn6Hk2UpaueNN=6CNHZQ@mail.gmail.com>
Date: Mon, 13 Oct 2025 08:19:33 -0500
From: Jonathan Denose <jdenose@...gle.com>
To: Randy Dunlap <rdunlap@...radead.org>
Cc: Thorsten Leemhuis <linux@...mhuis.info>, Jiri Kosina <jikos@...nel.org>,
Benjamin Tissoires <bentiss@...nel.org>, Dmitry Torokhov <dmitry.torokhov@...il.com>,
Jonathan Corbet <corbet@....net>, Henrik Rydberg <rydberg@...math.org>, linux-input@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
Angela Czubak <aczubak@...gle.com>, "Sean O'Brien" <seobrien@...gle.com>,
Lucas GISSOT <lucas.gissot.pro@...il.com>
Subject: Re: [PATCH v3 04/11] HID: haptic: introduce hid_haptic_device
On Fri, Oct 10, 2025 at 5:52 PM Randy Dunlap <rdunlap@...radead.org> wrote:
>
>
>
> On 10/10/25 1:30 PM, Jonathan Denose wrote:
> > Hi all,
> >
> > Thanks for looking into this.
> >
> > On Fri, Oct 10, 2025 at 1:55 PM Randy Dunlap <rdunlap@...radead.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@...gle.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.)
>
> More correctly, any .o that is listed as being built as part of
> hid.o should is going to be controlled by CONFIG_HID and should
> not its own have a Kconfig symbol at all.
>
> E.g. here:
> hid-y := hid-core.o hid-input.o hid-quirks.o
> hid-core, hid-input, and hid-quirks don't have their own
> Kconfig symbols.
>
>
>
> >> 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?
>
> I only know of two possibilities though there may be more.
>
> a. use request_module(true, "hid-haptic");
>
> This would probably be used in the hid core somewhere, afterthe device matching is done.
>
> b. use udev. When a device that needs hid-multitouch is
> discovered, have udev load both hid-haptic and hid-multitouch.
>
>
> I see that hid-haptic.h is written so that it has stubs for
> when CONFIG_HID_HAPTIC is not enabled. Can hid-multitouch
> operate (in a reduced capacity) when HID_HAPTIC is not enabled?
> So that they are separate modules and hid-multitouch is not
> dependent on hid-haptic?
>
> There is probably a use case for hid-multitouch without having
> hid-haptic loaded since hid-multitouch existed without having
> hid-haptic around at all.
>
> It seems that you want both of them loaded. And then Lucas
> has reported a build error when HID_HAPTIC=m and
> HID_MULTITOUCH=y, so either (like Lucas said) HID_HAPTIC
> should be bool, not tristate; or in Kconfig,
> HID_MULTITOUCH should depend on HID_HAPTIC, which would not
> allow the problem config that Lucas reported.
> But that forces devices that want HID_MULTITOUCH to also
> have HID_HAPTIC loaded, even though they may not need it.
>
The idea behind these patches was that hid-haptic would depend on
hid-multitouch but not the other way around. I am fine changing
HID_HAPTIC to bool. That's what I had it as initially, but I was asked
to change it.
If everyone else is fine with that, I can send out a patch.
> --
> ~Randy
>
--
Jonathan
Powered by blists - more mailing lists