lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAMCVhVPJR0LvZF0nzn5QOK-STbPoaHmv4qKw+ibteW79Svr8FQ@mail.gmail.com>
Date: Mon, 13 Oct 2025 15:59:55 -0500
From: Jonathan Denose <jdenose@...gle.com>
To: Benjamin Tissoires <bentiss@...nel.org>
Cc: Randy Dunlap <rdunlap@...radead.org>, Thorsten Leemhuis <linux@...mhuis.info>, 
	Jiri Kosina <jikos@...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 Mon, Oct 13, 2025 at 9:29 AM Benjamin Tissoires <bentiss@...nel.org> wrote:
>
> On Oct 13 2025, Jonathan Denose wrote:
> > 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.
>
> One way around it is to declare a stub struct haptic_operations and let
> hid-haptic.ko fill in the function pointers when it loads and remove
> them when it unloads. But it can be a little bit tedious, especially
> because making it properly working involves RCUs (we don't want to have
> mutexes everywhere).
>
> > >
> > 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.
>
> I'd still prefer hid-haptic to be tristate, just because
> input_ff_memless is tristate as well. Right now distributions don't
> support the FF bits, so enforcing this into the kernel seems a little
> bit harsh on them and difficult to debug for early adopters.
>
> That being said, that build failure is pretty bad. So please send a
> (temporary) fix ASAP. If making it boolean solves the issue, then yes,
> send a boolean fix and then we can revisit it. But right now the urgency
> is to fix that.
>
> And if making it proper tristate is too hard, then we can leave with
> bool.
>
> Cheers,
> Benjamin

Changing to bool instead of tristate resolved the build error.

Submitted the patch:
https://lore.kernel.org/linux-input/20251013-hid-haptic-kconfig-fix-v1-1-b1ad90732625@google.com/
-- 
Jonathan

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ