linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Randy Dunlap <rdunlap@infradead.org>
To: Thorsten Leemhuis <linux@leemhuis.info>,
	Jonathan Denose <jdenose@google.com>,
	Jiri Kosina <jikos@kernel.org>,
	Benjamin Tissoires <bentiss@kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Henrik Rydberg <rydberg@bitmath.org>
Cc: linux-input@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, Angela Czubak <aczubak@google.com>,
	Sean O'Brien <seobrien@google.com>,
	Lucas GISSOT <lucas.gissot.pro@gmail.com>
Subject: Re: [PATCH v3 04/11] HID: haptic: introduce hid_haptic_device
Date: Fri, 10 Oct 2025 11:55:11 -0700	[thread overview]
Message-ID: <f2243a9b-e032-416b-aef8-958198ff3955@infradead.org> (raw)
In-Reply-To: <1cd7fb11-0569-4032-905c-f887f3e0dd4c@leemhuis.info>

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.

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/
 


  reply	other threads:[~2025-10-10 18:55 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-18 23:08 [PATCH v3 00/11] HID: Implement haptic touchpad support Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 01/11] HID: add haptics page defines Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 02/11] Input: add FF_HAPTIC effect type Jonathan Denose
2025-09-14  1:02   ` Dmitry Torokhov
2025-08-18 23:08 ` [PATCH v3 03/11] Input: add INPUT_PROP_HAPTIC_TOUCHPAD Jonathan Denose
2025-08-19  4:19   ` Randy Dunlap
2025-09-14  1:03   ` Dmitry Torokhov
2025-10-24  3:30   ` Peter Hutterer
2025-08-18 23:08 ` [PATCH v3 04/11] HID: haptic: introduce hid_haptic_device Jonathan Denose
2025-10-09 14:43   ` Thorsten Leemhuis
2025-10-10  4:50     ` Randy Dunlap
2025-10-10  6:48       ` Thorsten Leemhuis
2025-10-10 18:55         ` Randy Dunlap [this message]
2025-10-10 20:30           ` Jonathan Denose
2025-10-10 21:09             ` Lucas GISSOT
2025-10-10 22:51             ` Randy Dunlap
2025-10-13 13:19               ` Jonathan Denose
2025-10-13 14:29                 ` Benjamin Tissoires
2025-10-13 20:59                   ` Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 05/11] HID: input: allow mapping of haptic output Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 06/11] HID: haptic: initialize haptic device Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 07/11] HID: input: calculate resolution for pressure Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 08/11] HID: haptic: add functions handling events Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 09/11] Input: MT - add INPUT_MT_TOTAL_FORCE flags Jonathan Denose
2025-09-14  1:04   ` Dmitry Torokhov
2025-08-18 23:08 ` [PATCH v3 10/11] HID: haptic: add hid_haptic_switch_mode Jonathan Denose
2025-08-18 23:08 ` [PATCH v3 11/11] HID: multitouch: add haptic multitouch support Jonathan Denose
2025-09-02 21:30 ` [PATCH v3 00/11] HID: Implement haptic touchpad support Jonathan Denose
2025-09-04 10:02   ` Benjamin Tissoires
2025-09-12 15:28     ` Jiri Kosina
2025-09-14  1:05       ` Dmitry Torokhov
2025-09-15 13:41 ` Benjamin Tissoires

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f2243a9b-e032-416b-aef8-958198ff3955@infradead.org \
    --to=rdunlap@infradead.org \
    --cc=aczubak@google.com \
    --cc=bentiss@kernel.org \
    --cc=corbet@lwn.net \
    --cc=dmitry.torokhov@gmail.com \
    --cc=jdenose@google.com \
    --cc=jikos@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@leemhuis.info \
    --cc=lucas.gissot.pro@gmail.com \
    --cc=rydberg@bitmath.org \
    --cc=seobrien@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).