* Re: [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-11-13 0:45 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Ilpo Järvinen, platform-driver-x86, linux-input, LKML,
Jiri Kosina, Benjamin Tissoires, Corentin Chary, Luke D . Jones,
Hans de Goede
In-Reply-To: <CAGwozwH4_5LnJ4kt-sbdwzL5Ezt74ddPOpGqvTvKiiVzkorSMQ@mail.gmail.com>
On 11/13/25 00:22, Antheas Kapenekakis wrote:
> On Wed, 12 Nov 2025 at 23:08, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>> snip
>>>> Sure, sorry if it came out this way. Dennis had two comments on the V7
>>>> version of the series.
>>>>
>>>> The first one was that asusctl has a hang bug, which he hasn't had
>>>> time to look into yet. This should have been fixed by dropping the
>>>> HID_QUIRK_INPUT_PER_APP. I retested the series and that QUIRK was a
>>>> bit of a NOOP that does not need to be added in the future.
>>> So it is supposed to not regress it now, correct?
> I missed this. Spaces. Yes, quirk input per app created around 3 more
> input devices per USB device, plus the dynamic lighting one, so you
> went from 2 to 6, and there seems to be an issue with asusctl and a
> large number of event devices, that caused some intermittent freezing
> when the user typed on certain keyboards. I removed it. Although, not
> a problem with the kernel itself, per say.
Okay, I will ask asus-linux kernel man to load this patchset then.
> Antheas
>
>>>> The second is he is concerned with dropping the 0x5d/0x5e inits. Luke
>>>> said (back in March) that it is fine to drop 0x5e because it is only
>>>>> snip
^ permalink raw reply
* Re: [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Denis Benato @ 2025-11-13 1:14 UTC (permalink / raw)
To: Antheas Kapenekakis
Cc: Ilpo Järvinen, platform-driver-x86, linux-input, LKML,
Jiri Kosina, Benjamin Tissoires, Corentin Chary, Luke D . Jones,
Hans de Goede
In-Reply-To: <CAGwozwEKqqJxxmtjJhy2MzNVhmBTMmy8xG5TZGkKJqJCgK=X5w@mail.gmail.com>
On 11/12/25 23:08, Antheas Kapenekakis wrote:
> On Wed, 12 Nov 2025 at 20:51, Denis Benato <benato.denis96@gmail.com> wrote:
>>
>> On 11/12/25 14:41, Antheas Kapenekakis wrote:
>>> On Wed, 12 Nov 2025 at 14:22, Ilpo Järvinen
>>> <ilpo.jarvinen@linux.intel.com> wrote:
>>>> On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
>>>>
>>>>> On Sat, 1 Nov 2025 at 11:47, Antheas Kapenekakis <lkml@antheas.dev> wrote:
>>>>>> This is a two part series which does the following:
>>>>>> - Clean-up init sequence
>>>>>> - Unify backlight handling to happen under asus-wmi so that all Aura
>>>>>> devices have synced brightness controls and the backlight button works
>>>>>> properly when it is on a USB laptop keyboard instead of one w/ WMI.
>>>>>>
>>>>>> For more context, see cover letter of V1. Since V5, I removed some patches
>>>>>> to make this easier to merge.
>>>>> Small bump for this.
>>>> I looked at v8 earlier but then got an impression some of Denis' comments
>>>> against v7 were not taken into account in v8, which implies there will be
>>>> delay until I've time to delve into the details (I need to understand
>>>> things pretty deeply in such a case, which does take lots of time).
>>>>
>>>> Alternatively, if Denis says v8 is acceptable, then I don't need to spend
>>>> so much time on it, but somehow I've a feeling he isn't happy with v8
>>>> but just hasn't voiced it again...
>>>>
>>>> Please do realize that ignoring reviewer feedback without a very very good
>>>> reason always risks adding delay or friction into getting things
>>>> upstreamed. Especially, when the review comes from a person who has been
>>>> around for me to learn to trust their reviews or from a maintainer of the
>>>> code in question.
>>> Sure, sorry if it came out this way. Dennis had two comments on the V7
>>> version of the series.
>>>
>>> The first one was that asusctl has a hang bug, which he hasn't had
>>> time to look into yet. This should have been fixed by dropping the
>>> HID_QUIRK_INPUT_PER_APP. I retested the series and that QUIRK was a
>>> bit of a NOOP that does not need to be added in the future.
>> So it is supposed to not regress it now, correct?
>>> The second is he is concerned with dropping the 0x5d/0x5e inits. Luke
>>> said (back in March) that it is fine to drop 0x5e because it is only
>>> used for ANIME displays. However, for 0x5d, it is hard to verify some
>>> of the older laptops because they have only been tested with 0x5d and
>>> we do not have the hardware in question to test.
>>>
>>> For this series, I re-added "initialize LED endpoint early for old
>>> NKEY keyboards" that re-adds 0x5d for the keyboards that cannot be
>>> tested again so this comment should be resolved too. With that in
>>> mind, we do end up with an additional quirk/command that may be
>>> unneeded and will remain there forever, but since it was a point of
>>> contention, it is not worth arguing over.
>>>
>>> So both comments should be resolved
>> The driver should also not late-initialize anything.
>>
>> Windows doesn't do it and the official asus application
>> can freely send LEDs changing commands to either WMI or USB
>> so I don't see any reason to do things differently [than windows]
>> and not prepare every USB endpoint to receive commands,
>> this has not been addressed unless I confused v7 and v8?
> Yes, it's been added on v8. 0x5d is init for the laptops it is
> problematic for. Not because it does not work, but because it has not
> been verified to work for those laptops.
I am not sure I am reading this right:
are you telling me that on recent models the windows driver
doesn't issue 0x5d?
>>> @Denis: can give an ack if this is the case?
>>>
>>> As for Derek's comment, he has a PR for his project where he removes
>>> the name match for Ally devices with ample time for it to be merged
>>> until kernel 6.19 is released. In addition, that specific software for
>>> full functionality relies on OOT drivers on the distros it ships with,
>>> so it is minimally affected in either case.
>> The part we are talking about depends on this driver (hid-asus)
>> and there are people on asus-linux community using inputplumber
>> for non-ally devices (the OOT driver is only for ally devices)
>> therefore it is very important to us (and various other distributions)
>> not to break that software in any way.
> This driver is only used for Ally devices. If you mean that people
> remap their keyboards using inputplumber I guess they could but I have
> not seen it.
I meant people remap keyboards using IP. I am sure there were
(and very probably still are) people doing that.
>> Weighting pros and cons of changing the name I am not sure
>> changing name brings any benefit? Or am I missing something here?
>> It's simply used by userspace so the hardware should be loading
>> regardless of the name...
> Users see the name of their devices in their settings menu. They
> should be accurate. Also, the early entry needs to be added anyway to
> prevent kicking devices.
If it's just aesthetics I don't see much reasons in changing the name.
"the early entry needs to be added anyway ...." has no meaning to me,
please rephrase. Sorry.
>> Along with IP and your tool and asusctl there is also openrgb,
>> and a newborn application for asus devices (I don't have contacts
>> with that dev nor I remember the name of the tool)
>> and I am not even that sure these are all asus-related
>> applications.
> My tool never checked for names, it briefly did for around a month
> after its creation for some devices until capability matches. Around
> 6.1 and 6.7 the kernel changed the names of most USB devices and that
> caused issues. It currently only uses name matches for VID/PID 0/0
> devices created by the kernel. Specifically, WMI and AT Keyboards. I
> am not sure there is a workaround for those. Asusctl also does not use
> names either.
But IP does, so I would like to hear confirmation from at least Derek
before the merge that there won't be future issues.
Interpret what I say here as a broad topic, not just name/PER_APP flag:
avoid changing data flow on older models...
>> Excercise EXTRA care touching this area as these are enough changes
>> to make it difficult to understand what exactly is the problem if
>> someone shows up with LEDs malfunctioning, laptop not entering sleep
>> anymore or something else entirely. Plus over time
>> ASUS has used various workarounds for windows problems
>> and I am not eager to find out what those are since there is only
>> a realistic way it's going to happen....
> These changes are not doing something extraordinary. It's just a minor cleanup.
>
>>> Moreover, that specific commit is needed for Xbox Ally devices anyway,
>>> as the kernel kicks one of the RGB endpoints because it does not
>>> register an input device (the check skipped by early return) so
>>> userspace becomes unable to control RGB on a stock kernel
>>> (hidraw/hiddev nodes are gone). For more context here, this specific
>>> endpoint implements the RGB Lamparray protocol for Windows dynamic
>>> lighting, and I think in an attempt to make it work properly in
>>> Windows, Asus made it so Windows has to first disable dynamic lighting
>>> for armoury crate RGB commands to work (the 0x5a ones over the 0xff31
>>> hid page).
>> Yes once ASUS introduces something new it sticks with that for
>> future models so it's expected more and more laptops will have
>> the same problem: I am not questioning if these patches are needed
>> as they very clearly are; I am questioning if everything that these
>> patches are doing are worth doing and aren't breaking/regressing
>> either tools or the flow of actions between the EC and these USB devices.
> Well, this series is needed to account for that. Sending the disable
> command is out of scope for now though.
Here I apologize for confusion: my comments were mostly about
older models: I absolutely don't want to break those, but if you find a way
to distinguish them from newer models that would give you more freedom with those.
No disable commands unless we find hard evidence those are strictly needed.
> Antheas
>
>>> Hopefully this clears things up
>>>
>>> Antheas
>>>
>>>>> Unrelated but I was b4ing this series on Ubuntu 24 and got BADSIG:
>>>>> DKIM/antheas.dev. Is there a reference for fixing this on my host?
>>>>> Perhaps it would help with spam
>>>> I see BADSIG very often these days from b4 (thanks to gmail expiring
>>>> things after 7 days or so, I recall hearing somewhere), I just ignore them
>>>> entirely.
>>>>
>>>> AFAIK, that has never caused any delay to any patch in pdx86 domain so if
>>>> that is what you thought is happening here, it's not the case.
>>>> If your patch does appear in the pdx86 patchwork, there's even less reason
>>>> to worry as I mostly pick patches to process using patchwork's list.
>>> Turns out I had to update my DNS records. It should be good now.
>>>
>>>> --
>>>> i.
>> snipp
>>>>>> 2.51.2
>>>>>>
>>>>>>
^ permalink raw reply
* [syzbot] [input?] [usb?] memory leak in dualshock4_get_calibration_data
From: syzbot @ 2025-11-13 4:38 UTC (permalink / raw)
To: bentiss, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 24172e0d7990 Merge tag 'arm64-fixes' of git://git.kernel.o..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=12a44692580000
kernel config: https://syzkaller.appspot.com/x/.config?x=cb128cd5cb439809
dashboard link: https://syzkaller.appspot.com/bug?extid=4f5f81e1456a1f645bf8
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1508c658580000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1051897c580000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/ded911fa4408/disk-24172e0d.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/a1f3e61cb784/vmlinux-24172e0d.xz
kernel image: https://storage.googleapis.com/syzbot-assets/b92fd0e25cb7/bzImage-24172e0d.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com
BUG: memory leak
unreferenced object 0xffff8881192f9a40 (size 64):
comm "kworker/1:0", pid 23, jiffies 4294944710
hex dump (first 32 bytes):
02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc c51b5d6b):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4983 [inline]
slab_alloc_node mm/slub.c:5288 [inline]
__kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5766
kmalloc_noprof include/linux/slab.h:957 [inline]
kzalloc_noprof include/linux/slab.h:1094 [inline]
dualshock4_get_calibration_data+0x437/0x500 drivers/hid/hid-playstation.c:1919
dualshock4_create drivers/hid/hid-playstation.c:2747 [inline]
ps_probe drivers/hid/hid-playstation.c:2845 [inline]
ps_probe+0x747/0x17d0 drivers/hid/hid-playstation.c:2821
__hid_device_probe drivers/hid/hid-core.c:2775 [inline]
hid_device_probe+0x298/0x3b0 drivers/hid/hid-core.c:2812
call_driver_probe drivers/base/dd.c:581 [inline]
really_probe+0x12f/0x430 drivers/base/dd.c:659
__driver_probe_device+0xc3/0x1a0 drivers/base/dd.c:801
driver_probe_device+0x2a/0x120 drivers/base/dd.c:831
__device_attach_driver+0x10f/0x170 drivers/base/dd.c:959
bus_for_each_drv+0xcf/0x120 drivers/base/bus.c:462
__device_attach+0xf9/0x290 drivers/base/dd.c:1031
bus_probe_device+0xcd/0xe0 drivers/base/bus.c:537
device_add+0x983/0xc80 drivers/base/core.c:3689
hid_add_device+0x140/0x250 drivers/hid/hid-core.c:2951
usbhid_probe+0x5ed/0x950 drivers/hid/usbhid/hid-core.c:1435
usb_probe_interface+0x173/0x3f0 drivers/usb/core/driver.c:396
call_driver_probe drivers/base/dd.c:581 [inline]
really_probe+0x12f/0x430 drivers/base/dd.c:659
BUG: memory leak
unreferenced object 0xffff8881192e7740 (size 64):
comm "kworker/1:0", pid 23, jiffies 4294944884
hex dump (first 32 bytes):
02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc c51b5d6b):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4983 [inline]
slab_alloc_node mm/slub.c:5288 [inline]
__kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5766
kmalloc_noprof include/linux/slab.h:957 [inline]
kzalloc_noprof include/linux/slab.h:1094 [inline]
dualshock4_get_calibration_data+0x437/0x500 drivers/hid/hid-playstation.c:1919
dualshock4_create drivers/hid/hid-playstation.c:2747 [inline]
ps_probe drivers/hid/hid-playstation.c:2845 [inline]
ps_probe+0x747/0x17d0 drivers/hid/hid-playstation.c:2821
__hid_device_probe drivers/hid/hid-core.c:2775 [inline]
hid_device_probe+0x298/0x3b0 drivers/hid/hid-core.c:2812
call_driver_probe drivers/base/dd.c:581 [inline]
really_probe+0x12f/0x430 drivers/base/dd.c:659
__driver_probe_device+0xc3/0x1a0 drivers/base/dd.c:801
driver_probe_device+0x2a/0x120 drivers/base/dd.c:831
__device_attach_driver+0x10f/0x170 drivers/base/dd.c:959
bus_for_each_drv+0xcf/0x120 drivers/base/bus.c:462
__device_attach+0xf9/0x290 drivers/base/dd.c:1031
bus_probe_device+0xcd/0xe0 drivers/base/bus.c:537
device_add+0x983/0xc80 drivers/base/core.c:3689
hid_add_device+0x140/0x250 drivers/hid/hid-core.c:2951
usbhid_probe+0x5ed/0x950 drivers/hid/usbhid/hid-core.c:1435
usb_probe_interface+0x173/0x3f0 drivers/usb/core/driver.c:396
call_driver_probe drivers/base/dd.c:581 [inline]
really_probe+0x12f/0x430 drivers/base/dd.c:659
BUG: memory leak
unreferenced object 0xffff88812484e5c0 (size 64):
comm "kworker/0:0", pid 9, jiffies 4294945059
hex dump (first 32 bytes):
02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace (crc c51b5d6b):
kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
slab_post_alloc_hook mm/slub.c:4983 [inline]
slab_alloc_node mm/slub.c:5288 [inline]
__kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5766
kmalloc_noprof include/linux/slab.h:957 [inline]
kzalloc_noprof include/linux/slab.h:1094 [inline]
dualshock4_get_calibration_data+0x437/0x500 drivers/hid/hid-playstation.c:1919
dualshock4_create drivers/hid/hid-playstation.c:2747 [inline]
ps_probe drivers/hid/hid-playstation.c:2845 [inline]
ps_probe+0x747/0x17d0 drivers/hid/hid-playstation.c:2821
__hid_device_probe drivers/hid/hid-core.c:2775 [inline]
hid_device_probe+0x298/0x3b0 drivers/hid/hid-core.c:2812
call_driver_probe drivers/base/dd.c:581 [inline]
really_probe+0x12f/0x430 drivers/base/dd.c:659
__driver_probe_device+0xc3/0x1a0 drivers/base/dd.c:801
driver_probe_device+0x2a/0x120 drivers/base/dd.c:831
__device_attach_driver+0x10f/0x170 drivers/base/dd.c:959
bus_for_each_drv+0xcf/0x120 drivers/base/bus.c:462
__device_attach+0xf9/0x290 drivers/base/dd.c:1031
bus_probe_device+0xcd/0xe0 drivers/base/bus.c:537
device_add+0x983/0xc80 drivers/base/core.c:3689
hid_add_device+0x140/0x250 drivers/hid/hid-core.c:2951
usbhid_probe+0x5ed/0x950 drivers/hid/usbhid/hid-core.c:1435
usb_probe_interface+0x173/0x3f0 drivers/usb/core/driver.c:396
call_driver_probe drivers/base/dd.c:581 [inline]
really_probe+0x12f/0x430 drivers/base/dd.c:659
connection error: failed to recv *flatrpc.ExecutorMessageRawT: EOF
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* Re: [PATCH v2 2/2] HID: multitouch: Toggle touch surface on Elan touchpad on lid event
From: kernel test robot @ 2025-11-13 4:49 UTC (permalink / raw)
To: Jonathan Denose, Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
Cc: oe-kbuild-all, linux-input, linux-kernel, Jonathan Denose
In-Reply-To: <20251111-lid-switch-notifier-v2-2-789723d78d89@google.com>
Hi Jonathan,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3a8660878839faadb4f1a6dd72c3179c1df56787]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Denose/Input-Add-lid-switch-notifier/20251112-053559
base: 3a8660878839faadb4f1a6dd72c3179c1df56787
patch link: https://lore.kernel.org/r/20251111-lid-switch-notifier-v2-2-789723d78d89%40google.com
patch subject: [PATCH v2 2/2] HID: multitouch: Toggle touch surface on Elan touchpad on lid event
config: parisc-randconfig-002-20251112 (https://download.01.org/0day-ci/archive/20251112/202511122158.oyGRoKNz-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251112/202511122158.oyGRoKNz-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511122158.oyGRoKNz-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "register_lid_notifier" [drivers/hid/hid-multitouch.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 2/2] HID: multitouch: Toggle touch surface on Elan touchpad on lid event
From: kernel test robot @ 2025-11-13 4:49 UTC (permalink / raw)
To: Jonathan Denose, Dmitry Torokhov, Jiri Kosina, Benjamin Tissoires
Cc: oe-kbuild-all, linux-input, linux-kernel, Jonathan Denose
In-Reply-To: <20251111-lid-switch-notifier-v2-2-789723d78d89@google.com>
Hi Jonathan,
kernel test robot noticed the following build errors:
[auto build test ERROR on 3a8660878839faadb4f1a6dd72c3179c1df56787]
url: https://github.com/intel-lab-lkp/linux/commits/Jonathan-Denose/Input-Add-lid-switch-notifier/20251112-053559
base: 3a8660878839faadb4f1a6dd72c3179c1df56787
patch link: https://lore.kernel.org/r/20251111-lid-switch-notifier-v2-2-789723d78d89%40google.com
patch subject: [PATCH v2 2/2] HID: multitouch: Toggle touch surface on Elan touchpad on lid event
config: xtensa-randconfig-002-20251112 (https://download.01.org/0day-ci/archive/20251112/202511122329.Sg4foDAx-lkp@intel.com/config)
compiler: xtensa-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251112/202511122329.Sg4foDAx-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511122329.Sg4foDAx-lkp@intel.com/
All errors (new ones prefixed by >>):
arch/xtensa/kernel/entry.o: in function `fast_syscall_spill_registers':
arch/xtensa/kernel/entry.S:1423:(.exception.text+0x21b): dangerous relocation: windowed longcall crosses 1GB boundary; return may fail: make_task_dead
xtensa-linux-ld: drivers/hid/hid-multitouch.o: in function `mt_touch_report.isra.24':
>> drivers/hid/hid-multitouch.c:1319: undefined reference to `register_lid_notifier'
xtensa-linux-ld: drivers/hid/hid-multitouch.o: in function `mt_touch_report.isra.24':
>> include/linux/input.h:512: undefined reference to `register_lid_notifier'
vim +1319 drivers/hid/hid-multitouch.c
2d93666e70662c Benjamin Tissoires 2011-01-11 1256
8dfe14b3b47ff8 Benjamin Tissoires 2018-07-13 1257 static void mt_touch_report(struct hid_device *hid,
8dfe14b3b47ff8 Benjamin Tissoires 2018-07-13 1258 struct mt_report_data *rdata)
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1259 {
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1260 struct mt_device *td = hid_get_drvdata(hid);
8dfe14b3b47ff8 Benjamin Tissoires 2018-07-13 1261 struct hid_report *report = rdata->report;
8dfe14b3b47ff8 Benjamin Tissoires 2018-07-13 1262 struct mt_application *app = rdata->application;
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1263 struct hid_field *field;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1264 struct input_dev *input;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1265 struct mt_usages *slot;
55746d28d66860 Hans de Goede 2017-11-22 1266 bool first_packet;
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1267 unsigned count;
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1268 int r, n;
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1269 int scantime = 0;
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1270 int contact_count = -1;
5519cab477b613 Benjamin Tissoires 2011-01-07 1271
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1272 /* sticky fingers release in progress, abort */
be6e2b5734a425 Andri Yngvason 2022-09-07 1273 if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1274 return;
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1275
01eaac7e57134c Benjamin Tissoires 2018-07-13 1276 scantime = *app->scantime;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1277 app->timestamp = mt_compute_timestamp(app, scantime);
01eaac7e57134c Benjamin Tissoires 2018-07-13 1278 if (app->raw_cc != DEFAULT_ZERO)
01eaac7e57134c Benjamin Tissoires 2018-07-13 1279 contact_count = *app->raw_cc;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1280
c2517f62dac608 Benjamin Tissoires 2013-01-31 1281 /*
c2517f62dac608 Benjamin Tissoires 2013-01-31 1282 * Includes multi-packet support where subsequent
c2517f62dac608 Benjamin Tissoires 2013-01-31 1283 * packets are sent with zero contactcount.
c2517f62dac608 Benjamin Tissoires 2013-01-31 1284 */
01eaac7e57134c Benjamin Tissoires 2018-07-13 1285 if (contact_count >= 0) {
af8dc4d0949092 Hans de Goede 2017-11-22 1286 /*
af8dc4d0949092 Hans de Goede 2017-11-22 1287 * For Win8 PTPs the first packet (td->num_received == 0) may
af8dc4d0949092 Hans de Goede 2017-11-22 1288 * have a contactcount of 0 if there only is a button event.
af8dc4d0949092 Hans de Goede 2017-11-22 1289 * We double check that this is not a continuation packet
af8dc4d0949092 Hans de Goede 2017-11-22 1290 * of a possible multi-packet frame be checking that the
af8dc4d0949092 Hans de Goede 2017-11-22 1291 * timestamp has changed.
af8dc4d0949092 Hans de Goede 2017-11-22 1292 */
3ceb3826448d1e Benjamin Tissoires 2018-07-13 1293 if ((app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1294 app->num_received == 0 &&
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1295 app->prev_scantime != scantime)
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1296 app->num_expected = contact_count;
af8dc4d0949092 Hans de Goede 2017-11-22 1297 /* A non 0 contact count always indicates a first packet */
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1298 else if (contact_count)
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1299 app->num_expected = contact_count;
7e3cc447ff8906 Benjamin Tissoires 2013-02-06 1300 }
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1301 app->prev_scantime = scantime;
c2517f62dac608 Benjamin Tissoires 2013-01-31 1302
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1303 first_packet = app->num_received == 0;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1304
01eaac7e57134c Benjamin Tissoires 2018-07-13 1305 input = report->field[0]->hidinput->input;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1306
01eaac7e57134c Benjamin Tissoires 2018-07-13 1307 list_for_each_entry(slot, &app->mt_usages, list) {
01eaac7e57134c Benjamin Tissoires 2018-07-13 1308 if (!mt_process_slot(td, input, app, slot))
01eaac7e57134c Benjamin Tissoires 2018-07-13 1309 app->num_received++;
01eaac7e57134c Benjamin Tissoires 2018-07-13 1310 }
01eaac7e57134c Benjamin Tissoires 2018-07-13 1311
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1312 for (r = 0; r < report->maxfield; r++) {
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1313 field = report->field[r];
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1314 count = field->report_count;
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1315
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1316 if (!(HID_MAIN_ITEM_VARIABLE & field->flags))
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1317 continue;
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1318
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 @1319 for (n = 0; n < count; n++)
01eaac7e57134c Benjamin Tissoires 2018-07-13 1320 mt_process_mt_event(hid, app, field,
01eaac7e57134c Benjamin Tissoires 2018-07-13 1321 &field->usage[n], field->value[n],
01eaac7e57134c Benjamin Tissoires 2018-07-13 1322 first_packet);
55978fa9dc4c57 Benjamin Tissoires 2013-01-31 1323 }
5b62efd8250d6a Benjamin Tissoires 2013-02-27 1324
f146d1c4d7eafd Benjamin Tissoires 2018-07-13 1325 if (app->num_received >= app->num_expected)
01eaac7e57134c Benjamin Tissoires 2018-07-13 1326 mt_sync_frame(td, app, input);
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1327
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1328 /*
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1329 * Windows 8 specs says 2 things:
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1330 * - once a contact has been reported, it has to be reported in each
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1331 * subsequent report
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1332 * - the report rate when fingers are present has to be at least
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1333 * the refresh rate of the screen, 60 or 120 Hz
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1334 *
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1335 * I interprete this that the specification forces a report rate of
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1336 * at least 60 Hz for a touchscreen to be certified.
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1337 * Which means that if we do not get a report whithin 16 ms, either
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1338 * something wrong happens, either the touchscreen forgets to send
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1339 * a release. Taking a reasonable margin allows to remove issues
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1340 * with USB communication or the load of the machine.
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1341 *
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1342 * Given that Win 8 devices are forced to send a release, this will
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1343 * only affect laggish machines and the ones that have a firmware
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1344 * defect.
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1345 */
3ceb3826448d1e Benjamin Tissoires 2018-07-13 1346 if (app->quirks & MT_QUIRK_STICKY_FINGERS) {
9609827458c37d Benjamin Tissoires 2017-06-15 1347 if (test_bit(MT_IO_FLAGS_PENDING_SLOTS, &td->mt_io_flags))
9609827458c37d Benjamin Tissoires 2017-06-15 1348 mod_timer(&td->release_timer,
9609827458c37d Benjamin Tissoires 2017-06-15 1349 jiffies + msecs_to_jiffies(100));
9609827458c37d Benjamin Tissoires 2017-06-15 1350 else
8fa7292fee5c52 Thomas Gleixner 2025-04-05 1351 timer_delete(&td->release_timer);
9609827458c37d Benjamin Tissoires 2017-06-15 1352 }
4f4001bc76fd1a Benjamin Tissoires 2017-06-15 1353
be6e2b5734a425 Andri Yngvason 2022-09-07 1354 clear_bit_unlock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags);
5519cab477b613 Benjamin Tissoires 2011-01-07 1355 }
5519cab477b613 Benjamin Tissoires 2011-01-07 1356
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v4 1/2] dt-bindings: input: i2c-hid: Introduce FocalTech FT8112
From: daniel_peng @ 2025-11-13 6:00 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: LKML, Daniel Peng, Conor Dooley, Krzysztof Kozlowski, Rob Herring,
devicetree
From: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
The FocalTech FT8112 touch screen chip same as Ilitek ili2901 controller
has a reset gpio. The difference is that they have different
post_gpio_reset_on_delay_ms.
FocalTech FT8112 also uses 3.3V power supply.
Signed-off-by: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
---
Changes in v4:
- Move change log of v3 under "---" correctly as below 4 items.
- Modified the subject description.
- Modified maintainers to myself of this binding file.
- Fixed the extra '>' on section of "interrupts = <15 IRQ_TYPE_LEVEL_LOW>;" and confirm command 'make dt_binding_check' correctly.
- Restored MAINTAINERS file.
.../bindings/input/focaltech,ft8112.yaml | 66 +++++++++++++++++++
1 file changed, 66 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
diff --git a/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml b/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
new file mode 100644
index 000000000000..197f30b14d45
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/focaltech,ft8112.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/focaltech,ft8112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: FocalTech FT8112 touchscreen controller
+
+maintainers:
+ - Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
+
+description:
+ Supports the FocalTech FT8112 touchscreen controller.
+ This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
+
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+
+properties:
+ compatible:
+ enum:
+ - focaltech,ft8112
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ panel: true
+
+ reset-gpios:
+ maxItems: 1
+
+ vcc33-supply: true
+
+ vccio-supply: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - vcc33-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@38 {
+ compatible = "focaltech,ft8112";
+ reg = <0x38>;
+
+ interrupt-parent = <&pio>;
+ interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
+
+ reset-gpios = <&pio 126 GPIO_ACTIVE_LOW>;
+ vcc33-supply = <&pp3300_tchscr_x>;
+ };
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH v4 2/2] HID: i2c-hid: Add FocalTech FT8112
From: daniel_peng @ 2025-11-13 6:00 UTC (permalink / raw)
To: Dmitry Torokhov, linux-input
Cc: LKML, Daniel Peng, Benjamin Tissoires, Douglas Anderson,
Jiri Kosina, Pin-yen Lin
In-Reply-To: <20251113140004.v4.1.I894dde5015f4acad94cb5bada61e5811c5142395@changeid>
From: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
Information for touchscreen model HKO/RB116AS01-2 as below:
- HID :FTSC1000
- slave address:0X38
- Interface:HID over I2C
- Touch control lC:FT8112
- I2C ID: PNP0C50
Signed-off-by: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
---
Changes in v4:
- Move change log of v3 under "---" correctly as below 1 item.
- Modified the subject to include the ID of the chip as suggestion only.
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 0215f217f6d8..b81fcc6ff49e 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -168,6 +168,13 @@ static const struct elan_i2c_hid_chip_data elan_ekth6a12nay_chip_data = {
.power_after_backlight = true,
};
+static const struct elan_i2c_hid_chip_data focaltech_ft8112_chip_data = {
+ .post_power_delay_ms = 10,
+ .post_gpio_reset_on_delay_ms = 150,
+ .hid_descriptor_address = 0x0001,
+ .main_supply_name = "vcc33",
+};
+
static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
.post_power_delay_ms = 1,
.post_gpio_reset_on_delay_ms = 200,
@@ -191,6 +198,7 @@ static const struct elan_i2c_hid_chip_data ilitek_ili2901_chip_data = {
static const struct of_device_id elan_i2c_hid_of_match[] = {
{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
{ .compatible = "elan,ekth6a12nay", .data = &elan_ekth6a12nay_chip_data },
+ { .compatible = "focaltech,ft8112", .data = &focaltech_ft8112_chip_data },
{ .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
{ .compatible = "ilitek,ili2901", .data = &ilitek_ili2901_chip_data },
{ }
--
2.34.1
^ permalink raw reply related
* Re: [syzbot] [input?] [usb?] memory leak in dualshock4_get_calibration_data
From: Oliver Neukum @ 2025-11-13 7:23 UTC (permalink / raw)
To: syzbot, bentiss, jikos, linux-input, linux-kernel, linux-usb,
syzkaller-bugs
In-Reply-To: <691560c4.a70a0220.3124cb.0019.GAE@google.com>
[-- Attachment #1: Type: text/plain, Size: 7903 bytes --]
#syz test: upstream 24172e0d7990
On 13.11.25 05:38, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: 24172e0d7990 Merge tag 'arm64-fixes' of git://git.kernel.o..
> git tree: upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=12a44692580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=cb128cd5cb439809
> dashboard link: https://syzkaller.appspot.com/bug?extid=4f5f81e1456a1f645bf8
> compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1508c658580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1051897c580000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/ded911fa4408/disk-24172e0d.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/a1f3e61cb784/vmlinux-24172e0d.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/b92fd0e25cb7/bzImage-24172e0d.xz
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+4f5f81e1456a1f645bf8@syzkaller.appspotmail.com
>
> BUG: memory leak
> unreferenced object 0xffff8881192f9a40 (size 64):
> comm "kworker/1:0", pid 23, jiffies 4294944710
> hex dump (first 32 bytes):
> 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc c51b5d6b):
> kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
> slab_post_alloc_hook mm/slub.c:4983 [inline]
> slab_alloc_node mm/slub.c:5288 [inline]
> __kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5766
> kmalloc_noprof include/linux/slab.h:957 [inline]
> kzalloc_noprof include/linux/slab.h:1094 [inline]
> dualshock4_get_calibration_data+0x437/0x500 drivers/hid/hid-playstation.c:1919
> dualshock4_create drivers/hid/hid-playstation.c:2747 [inline]
> ps_probe drivers/hid/hid-playstation.c:2845 [inline]
> ps_probe+0x747/0x17d0 drivers/hid/hid-playstation.c:2821
> __hid_device_probe drivers/hid/hid-core.c:2775 [inline]
> hid_device_probe+0x298/0x3b0 drivers/hid/hid-core.c:2812
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x12f/0x430 drivers/base/dd.c:659
> __driver_probe_device+0xc3/0x1a0 drivers/base/dd.c:801
> driver_probe_device+0x2a/0x120 drivers/base/dd.c:831
> __device_attach_driver+0x10f/0x170 drivers/base/dd.c:959
> bus_for_each_drv+0xcf/0x120 drivers/base/bus.c:462
> __device_attach+0xf9/0x290 drivers/base/dd.c:1031
> bus_probe_device+0xcd/0xe0 drivers/base/bus.c:537
> device_add+0x983/0xc80 drivers/base/core.c:3689
> hid_add_device+0x140/0x250 drivers/hid/hid-core.c:2951
> usbhid_probe+0x5ed/0x950 drivers/hid/usbhid/hid-core.c:1435
> usb_probe_interface+0x173/0x3f0 drivers/usb/core/driver.c:396
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x12f/0x430 drivers/base/dd.c:659
>
> BUG: memory leak
> unreferenced object 0xffff8881192e7740 (size 64):
> comm "kworker/1:0", pid 23, jiffies 4294944884
> hex dump (first 32 bytes):
> 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc c51b5d6b):
> kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
> slab_post_alloc_hook mm/slub.c:4983 [inline]
> slab_alloc_node mm/slub.c:5288 [inline]
> __kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5766
> kmalloc_noprof include/linux/slab.h:957 [inline]
> kzalloc_noprof include/linux/slab.h:1094 [inline]
> dualshock4_get_calibration_data+0x437/0x500 drivers/hid/hid-playstation.c:1919
> dualshock4_create drivers/hid/hid-playstation.c:2747 [inline]
> ps_probe drivers/hid/hid-playstation.c:2845 [inline]
> ps_probe+0x747/0x17d0 drivers/hid/hid-playstation.c:2821
> __hid_device_probe drivers/hid/hid-core.c:2775 [inline]
> hid_device_probe+0x298/0x3b0 drivers/hid/hid-core.c:2812
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x12f/0x430 drivers/base/dd.c:659
> __driver_probe_device+0xc3/0x1a0 drivers/base/dd.c:801
> driver_probe_device+0x2a/0x120 drivers/base/dd.c:831
> __device_attach_driver+0x10f/0x170 drivers/base/dd.c:959
> bus_for_each_drv+0xcf/0x120 drivers/base/bus.c:462
> __device_attach+0xf9/0x290 drivers/base/dd.c:1031
> bus_probe_device+0xcd/0xe0 drivers/base/bus.c:537
> device_add+0x983/0xc80 drivers/base/core.c:3689
> hid_add_device+0x140/0x250 drivers/hid/hid-core.c:2951
> usbhid_probe+0x5ed/0x950 drivers/hid/usbhid/hid-core.c:1435
> usb_probe_interface+0x173/0x3f0 drivers/usb/core/driver.c:396
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x12f/0x430 drivers/base/dd.c:659
>
> BUG: memory leak
> unreferenced object 0xffff88812484e5c0 (size 64):
> comm "kworker/0:0", pid 9, jiffies 4294945059
> hex dump (first 32 bytes):
> 02 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> backtrace (crc c51b5d6b):
> kmemleak_alloc_recursive include/linux/kmemleak.h:44 [inline]
> slab_post_alloc_hook mm/slub.c:4983 [inline]
> slab_alloc_node mm/slub.c:5288 [inline]
> __kmalloc_cache_noprof+0x3a6/0x5b0 mm/slub.c:5766
> kmalloc_noprof include/linux/slab.h:957 [inline]
> kzalloc_noprof include/linux/slab.h:1094 [inline]
> dualshock4_get_calibration_data+0x437/0x500 drivers/hid/hid-playstation.c:1919
> dualshock4_create drivers/hid/hid-playstation.c:2747 [inline]
> ps_probe drivers/hid/hid-playstation.c:2845 [inline]
> ps_probe+0x747/0x17d0 drivers/hid/hid-playstation.c:2821
> __hid_device_probe drivers/hid/hid-core.c:2775 [inline]
> hid_device_probe+0x298/0x3b0 drivers/hid/hid-core.c:2812
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x12f/0x430 drivers/base/dd.c:659
> __driver_probe_device+0xc3/0x1a0 drivers/base/dd.c:801
> driver_probe_device+0x2a/0x120 drivers/base/dd.c:831
> __device_attach_driver+0x10f/0x170 drivers/base/dd.c:959
> bus_for_each_drv+0xcf/0x120 drivers/base/bus.c:462
> __device_attach+0xf9/0x290 drivers/base/dd.c:1031
> bus_probe_device+0xcd/0xe0 drivers/base/bus.c:537
> device_add+0x983/0xc80 drivers/base/core.c:3689
> hid_add_device+0x140/0x250 drivers/hid/hid-core.c:2951
> usbhid_probe+0x5ed/0x950 drivers/hid/usbhid/hid-core.c:1435
> usb_probe_interface+0x173/0x3f0 drivers/usb/core/driver.c:396
> call_driver_probe drivers/base/dd.c:581 [inline]
> really_probe+0x12f/0x430 drivers/base/dd.c:659
>
> connection error: failed to recv *flatrpc.ExecutorMessageRawT: EOF
>
>
> ---
> This report is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this issue. See:
> https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
>
> If the report is already addressed, let syzbot know by replying with:
> #syz fix: exact-commit-title
>
> If you want syzbot to run the reproducer, reply with:
> #syz test: git://repo/address.git branch-or-commit-hash
> If you attach or paste a git patch, syzbot will apply it before testing.
>
> If you want to overwrite report's subsystems, reply with:
> #syz set subsystems: new-subsystem
> (See the list of subsystem names on the web dashboard)
>
> If the report is a duplicate of another one, reply with:
> #syz dup: exact-subject-of-another-report
>
> If you want to undo deduplication, reply with:
> #syz undup
>
[-- Attachment #2: 0001-hid-hid-playstation-fix-memleak-in-probe.patch --]
[-- Type: text/x-patch, Size: 1110 bytes --]
From 29d67f8aa4aed5d207ceff52ad9c5c675a438425 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oneukum@suse.com>
Date: Thu, 13 Nov 2025 08:03:55 +0100
Subject: [PATCH] hid: hid-playstation: fix memleak in probe
Free the buffer if we bail out in the error case of
dualshock4_get_calibration_data()
Signed-off-by: Oliver Neukum <oneukum@suse.com>
---
drivers/hid/hid-playstation.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-playstation.c b/drivers/hid/hid-playstation.c
index 63f6eb9030d1..128aa6abd10b 100644
--- a/drivers/hid/hid-playstation.c
+++ b/drivers/hid/hid-playstation.c
@@ -1942,6 +1942,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
"Failed to retrieve DualShock4 calibration info: %d\n",
ret);
ret = -EILSEQ;
+ kfree(buf);
goto transfer_failed;
} else {
break;
@@ -1959,6 +1960,7 @@ static int dualshock4_get_calibration_data(struct dualshock4 *ds4)
if (ret) {
hid_warn(hdev, "Failed to retrieve DualShock4 calibration info: %d\n", ret);
+ kfree(buf);
goto transfer_failed;
}
}
--
2.51.1
^ permalink raw reply related
* Re: [syzbot] [input?] [usb?] memory leak in dualshock4_get_calibration_data
From: syzbot @ 2025-11-13 7:38 UTC (permalink / raw)
To: bentiss, jikos, linux-input, linux-kernel, linux-usb, oneukum,
syzkaller-bugs
In-Reply-To: <148861b7-4f6b-45d9-8d52-1b21f8bb2395@suse.com>
Hello,
syzbot tried to test the proposed patch but the build/boot failed:
SYZFAIL: failed to recv rpc
SYZFAIL: failed to recv rpc
fd=3 want=4 recv=0 n=0
Warning: Permanently added '10.128.0.165' (ED25519) to the list of known hosts.
2025/11/13 07:36:48 parsed 1 programs
[ 46.199818][ T5813] cgroup: Unknown subsys name 'net'
[ 46.379023][ T5813] cgroup: Unknown subsys name 'cpuset'
[ 46.385474][ T5813] cgroup: Unknown subsys name 'rlimit'
Setting up swapspace version 1, size = 127995904 bytes
[ 54.888279][ T5813] Adding 124996k swap on ./swap-file. Priority:0 extents:1 across:124996k
[ 56.340369][ T5825] soft_limit_in_bytes is deprecated and will be removed. Please report your usecase to linux-mm@kvack.org if you depend on this functionality.
[ 56.421936][ T5832] Bluetooth: hci0: unexpected cc 0x0c03 length: 249 > 1
[ 56.429766][ T5832] Bluetooth: hci0: unexpected cc 0x1003 length: 249 > 9
[ 56.436985][ T5832] Bluetooth: hci0: unexpected cc 0x1001 length: 249 > 9
[ 56.445171][ T5832] Bluetooth: hci0: unexpected cc 0x0c23 length: 249 > 4
[ 56.452741][ T5832] Bluetooth: hci0: unexpected cc 0x0c38 length: 249 > 2
[ 56.846094][ T48] wlan0: Created IBSS using preconfigured BSSID 50:50:50:50:50:50
[ 56.854123][ T48] wlan0: Creating new IBSS network, BSSID 50:50:50:50:50:50
[ 56.864851][ T48] wlan1: Created IBSS using preconfigured BSSID 50:50:50:50:50:50
[ 56.872802][ T48] wlan1: Creating new IBSS network, BSSID 50:50:50:50:50:50
[ 57.063354][ T5895] chnl_net:caif_netlink_parms(): no params data found
[ 57.083062][ T5895] bridge0: port 1(bridge_slave_0) entered blocking state
[ 57.090193][ T5895] bridge0: port 1(bridge_slave_0) entered disabled state
[ 57.097410][ T5895] bridge_slave_0: entered allmulticast mode
[ 57.104038][ T5895] bridge_slave_0: entered promiscuous mode
[ 57.113031][ T5895] bridge0: port 2(bridge_slave_1) entered blocking state
[ 57.120155][ T5895] bridge0: port 2(bridge_slave_1) entered disabled state
[ 57.127349][ T5895] bridge_slave_1: entered allmulticast mode
[ 57.133596][ T5895] bridge_slave_1: entered promiscuous mode
[ 57.144454][ T5895] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link
[ 57.154187][ T5895] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link
[ 57.168185][ T5895] team0: Port device team_slave_0 added
[ 57.174463][ T5895] team0: Port device team_slave_1 added
[ 57.186086][ T5895] batman_adv: batadv0: Adding interface: batadv_slave_0
[ 57.193078][ T5895] batman_adv: batadv0: The MTU of interface batadv_slave_0 is too small (1500) to handle the transport of batman-adv packets. Packets going over this interface will be fragmented on layer2 which could impact the performance. Setting the MTU to 1532 would solve the problem.
[ 57.219301][ T5895] batman_adv: batadv0: Not using interface batadv_slave_0 (retrying later): interface not active
[ 57.230889][ T5895] batman_adv: batadv0: Adding interface: batadv_slave_1
[ 57.237915][ T5895] batman_adv: batadv0: The MTU of interface batadv_slave_1 is too small (1500) to handle the transport of batman-adv packets. Packets going over this interface will be fragmented on layer2 which could impact the performance. Setting the MTU to 1532 would solve the problem.
[ 57.263881][ T5895] batman_adv: batadv0: Not using interface batadv_slave_1 (retrying later): interface not active
[ 57.281848][ T5895] hsr_slave_0: entered promiscuous mode
[ 57.287619][ T5895] hsr_slave_1: entered promiscuous mode
[ 57.317718][ T5895] netdevsim netdevsim0 netdevsim0: renamed from eth0
[ 57.325494][ T5895] netdevsim netdevsim0 netdevsim1: renamed from eth1
[ 57.334404][ T5895] netdevsim netdevsim0 netdevsim2: renamed from eth2
[ 57.349057][ T5895] netdevsim netdevsim0 netdevsim3: renamed from eth3
[ 57.361932][ T5895] bridge0: port 2(bridge_slave_1) entered blocking state
[ 57.368995][ T5895] bridge0: port 2(bridge_slave_1) entered forwarding state
[ 57.376311][ T5895] bridge0: port 1(bridge_slave_0) entered blocking state
[ 57.383366][ T5895] bridge0: port 1(bridge_slave_0) entered forwarding state
[ 57.402889][ T5895] 8021q: adding VLAN 0 to HW filter on device bond0
[ 57.412746][ T48] bridge0: port 1(bridge_slave_0) entered disabled state
[ 57.420421][ T48] bridge0: port 2(bridge_slave_1) entered disabled state
[ 57.430935][ T5895] 8021q: adding VLAN 0 to HW filter on device team0
[ 57.439750][ T1005] bridge0: port 1(bridge_slave_0) entered blocking state
[ 57.446848][ T1005] bridge0: port 1(bridge_slave_0) entered forwarding state
[ 57.455906][ T1005] bridge0: port 2(bridge_slave_1) entered blocking state
[ 57.463221][ T1005] bridge0: port 2(bridge_slave_1) entered forwarding state
[ 57.480670][ T5895] hsr0: Slave B (hsr_slave_1) is not up; please bring it up to get a fully working HSR network
[ 57.523051][ T5895] 8021q: adding VLAN 0 to HW filter on device batadv0
[ 57.539669][ T5895] veth0_vlan: entered promiscuous mode
[ 57.546689][ T5895] veth1_vlan: entered promiscuous mode
[ 57.557686][ T5895] veth0_macvtap: entered promiscuous mode
[ 57.564689][ T5895] veth1_macvtap: entered promiscuous mode
[ 57.574038][ T5895] batman_adv: batadv0: Interface activated: batadv_slave_0
[ 57.583627][ T5895] batman_adv: batadv0: Interface activated: batadv_slave_1
[ 57.592625][ T1005] netdevsim netdevsim0 netdevsim0: set [1, 0] type 2 family 0 port 6081 - 0
[ 57.601383][ T1005] netdevsim netdevsim0 netdevsim1: set [1, 0] type 2 family 0 port 6081 - 0
[ 57.610479][ T1005] netdevsim netdevsim0 netdevsim2: set [1, 0] type 2 family 0 port 6081 - 0
[ 57.619249][ T1005] netdevsim netdevsim0 netdevsim3: set [1, 0] type 2 family 0 port 6081 - 0
[ 57.680555][ T1005] netdevsim netdevsim0 netdevsim3 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
[ 57.721144][ T1005] netdevsim netdevsim0 netdevsim2 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
2025/11/13 07:37:01 executed programs: 0
[ 57.769104][ T1005] netdevsim netdevsim0 netdevsim1 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
[ 57.819448][ T1005] netdevsim netdevsim0 netdevsim0 (unregistering): unset [1, 0] type 2 family 0 port 6081 - 0
[ 60.847880][ T1005] bridge_slave_1: left allmulticast mode
[ 60.856131][ T1005] bridge_slave_1: left promiscuous mode
[ 60.861839][ T1005] bridge0: port 2(bridge_slave_1) entered disabled state
[ 60.869580][ T1005] bridge_slave_0: left allmulticast mode
[ 60.875248][ T1005] bridge_slave_0: left promiscuous mode
[ 60.881523][ T1005] bridge0: port 1(bridge_slave_0) entered disabled state
[ 60.930394][ T1005] bond0 (unregistering): (slave bond_slave_0): Releasing backup interface
[ 60.940379][ T1005] bond0 (unregistering): (slave bond_slave_1): Releasing backup interface
[ 60.949636][ T1005] bond0 (unregistering): Released all slaves
[ 61.019603][ T1005] hsr_slave_0: left promiscuous mode
[ 61.025166][ T1005] hsr_slave_1: left promiscuous mode
[ 61.030851][ T1005] batman_adv: batadv0: Interface deactivated: batadv_slave_0
[ 61.038360][ T1005] batman_adv: batadv0: Removing interface: batadv_slave_0
[ 61.045838][ T1005] batman_adv: batadv0: Interface deactivated: batadv_slave_1
[ 61.053628][ T1005] batman_adv: batadv0: Removing interface: batadv_slave_1
[ 61.062820][ T1005] veth1_macvtap: left promiscuous mode
[ 61.068468][ T1005] veth0_macvtap: left promiscuous mode
[ 61.074011][ T1005] veth1_vlan: left promiscuous mode
[ 61.079293][ T1005] veth0_vlan: left promiscuous mode
[ 61.105155][ T1005] team0 (unregistering): Port device team_slave_1 removed
[ 61.113500][ T1005] team0 (unregistering): Port device team_slave_0 removed
[ 63.295265][ T5134] Bluetooth: hci0: unexpected cc 0x0c03 length: 249 > 1
[ 63.302496][ T5134] Bluetooth: hci0: unexpected cc 0x1003 length: 249 > 9
[ 63.309684][ T5134] Bluetooth: hci0: unexpected cc 0x1001 length: 249 > 9
[ 63.317008][ T5134] Bluetooth: hci0: unexpected cc 0x0c23 length: 249 > 4
[ 63.324371][ T5134] Bluetooth: hci0: unexpected cc 0x0c38 length: 249 > 2
[ 63.362995][ T5989] chnl_net:caif_netlink_parms(): no params data found
[ 63.383696][ T5989] bridge0: port 1(bridge_slave_0) entered blocking state
[ 63.390958][ T5989] bridge0: port 1(bridge_slave_0) entered disabled state
[ 63.398237][ T5989] bridge_slave_0: entered allmulticast mode
[ 63.404497][ T5989] bridge_slave_0: entered promiscuous mode
[ 63.411309][ T5989] bridge0: port 2(bridge_slave_1) entered blocking state
[ 63.418421][ T5989] bridge0: port 2(bridge_slave_1) entered disabled state
[ 63.425574][ T5989] bridge_slave_1: entered allmulticast mode
[ 63.431855][ T5989] bridge_slave_1: entered promiscuous mode
[ 63.443227][ T5989] bond0: (slave bond_slave_0): Enslaving as an active interface with an up link
[ 63.453005][ T5989] bond0: (slave bond_slave_1): Enslaving as an active interface with an up link
[ 63.466713][ T5989] team0: Port device team_slave_0 added
[ 63.472987][ T5989] team0: Port device team_slave_1 added
[ 63.482893][ T5989] batman_adv: batadv0: Adding interface: batadv_slave_0
[ 63.489894][ T5989] batman_adv: batadv0: The MTU of interface batadv_slave_0 is too small (1500) to handle the transport of batman-adv packets. Packets going over this interface will be fragmented on layer2 which could impact the performance. Setting the MTU to 1532 would solve the problem.
[ 63.515910][ T5989] batman_adv: batadv0: Not using interface batadv_slave_0 (retrying later): interface not active
[ 63.527038][ T5989] batman_adv: batadv0: Adding interface: batadv_slave_1
[ 63.534021][ T5989] batman_adv: batadv0: The MTU of interface batadv_slave_1 is too small (1500) to handle the transport of batman-adv packets. Packets going over this interface will be fragmented on layer2 which could impact the performance. Setting the MTU to 1532 would solve the problem.
[ 63.560024][ T5989] batman_adv: batadv0: Not using interface batadv_slave_1 (retrying later): interface not active
[ 63.577524][ T5989] hsr_slave_0: entered promiscuous mode
[ 63.583382][ T5989] hsr_slave_1: entered promiscuous mode
[ 63.781644][ T5989] netdevsim netdevsim0 netdevsim0: renamed from eth0
[ 63.790644][ T5989] netdevsim netdevsim0 netdevsim1: renamed from eth1
[ 63.798849][ T5989] netdevsim netdevsim0 netdevsim2: renamed from eth2
[ 63.806845][ T5989] netdevsim netdevsim0 netdevsim3: renamed from eth3
[ 63.821643][ T5989] bridge0: port 2(bridge_slave_1) entered blocking state
[ 63.828815][ T5989] bridge0: port 2(bridge_slave_1) entered forwarding state
[ 63.836059][ T5989] bridge0: port 1(bridge_slave_0) entered blocking state
[ 63.843125][ T5989] bridge0: port 1(bridge_slave_0) entered forwarding state
[ 63.865285][ T5989] 8021q: adding VLAN 0 to HW filter on device bond0
[ 63.875346][ T12] bridge0: port 1(bridge_slave_0) entered disabled state
[ 63.883832][ T12] bridge0: port 2(bridge_slave_1) entered disabled state
[ 63.894432][ T5989] 8021q: adding VLAN 0 to HW filter on device team0
[ 63.903560][ T1005] bridge0: port 1(bridge_slave_0) entered blocking state
[ 63.910672][ T1005] bridge0: port 1(bridge_slave_0) entered forwarding state
[ 63.921689][ T1005] bridge0: port 2(bridge_slave_1) entered blocking state
[ 63.928797][ T1005] bridge0: port 2(bridge_slave_1) entered forwarding state
[ 63.990298][ T5989] 8021q: adding VLAN 0 to HW filter on device batadv0
[ 64.009155][ T5989] veth0_vlan: entered promiscuous mode
[ 64.016994][ T5989] veth1_vlan: entered promiscuous mode
[ 64.032046][ T5989] veth0_macvtap: entered promiscuous mode
[ 64.039726][ T5989] veth1_macvtap: entered promiscuous mode
[ 64.051239][ T5989] batman_adv: batadv0: Interface activated: batadv_slave_0
[ 64.061759][ T5989] batman_adv: batadv0: Interface activated: batadv_slave_1
[ 64.072138][ T12] netdevsim netdevsim0 netdevsim0: set [1, 0] type 2 family 0 port 6081 - 0
[ 64.096024][ T12] netdevsim netdevsim0 netdevsim1: set [1, 0] type 2 family 0 port 6081 - 0
[ 64.113835][ T48] wlan0: Created IBSS using preconfigured BSSID 50:50:50:50:50:50
[ 64.126167][ T48] wlan0: Creating new IBSS network, BSSID 50:50:50:50:50:50
[ 64.136390][ T1005] wlan1: Created IBSS using preconfigured BSSID 50:50:50:50:50:50
SYZFAIL: failed to recv rpc
[ 64.136519][ T12] netdevsim netdevsim0 netdevsim2: set [1, 0] type 2 family 0 port 6081 - 0
[ 64.152984][ T1005] wlan1: Creating new IBSS network, BSSID 50:50:50:50:50:50
[ 64.154772][ T12] netdevsim netdevsim0 netdevsim3: set [1, 0] type 2 family 0 port 6081 - 0
fd=3 want=4 recv=0 n=0
syzkaller build log:
go env (err=<nil>)
AR='ar'
CC='gcc'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='g++'
GCCGO='gccgo'
GO111MODULE='auto'
GOAMD64='v1'
GOARCH='amd64'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/syzkaller/.cache/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/syzkaller/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4112260622=/tmp/go-build -gno-record-gcc-switches'
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMOD='/syzkaller/jobs-2/linux/gopath/src/github.com/google/syzkaller/go.mod'
GOMODCACHE='/syzkaller/jobs-2/linux/gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/syzkaller/jobs-2/linux/gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/syzkaller/.config/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.24.4'
GOWORK=''
PKG_CONFIG='pkg-config'
git status (err=<nil>)
HEAD detached at 07e030dea6e
nothing to commit, working tree clean
tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
Makefile:31: run command via tools/syz-env for best compatibility, see:
Makefile:32: https://github.com/google/syzkaller/blob/master/docs/contributing.md#using-syz-env
go list -f '{{.Stale}}' -ldflags="-s -w -X github.com/google/syzkaller/prog.GitRevision=07e030dea6e6d9ca88b75bb3be2810f47083b328 -X github.com/google/syzkaller/prog.gitRevisionDate=20251112-115923" ./sys/syz-sysgen | grep -q false || go install -ldflags="-s -w -X github.com/google/syzkaller/prog.GitRevision=07e030dea6e6d9ca88b75bb3be2810f47083b328 -X github.com/google/syzkaller/prog.gitRevisionDate=20251112-115923" ./sys/syz-sysgen
make .descriptions
tput: No value for $TERM and no -T specified
tput: No value for $TERM and no -T specified
Makefile:31: run command via tools/syz-env for best compatibility, see:
Makefile:32: https://github.com/google/syzkaller/blob/master/docs/contributing.md#using-syz-env
bin/syz-sysgen
touch .descriptions
GOOS=linux GOARCH=amd64 go build -ldflags="-s -w -X github.com/google/syzkaller/prog.GitRevision=07e030dea6e6d9ca88b75bb3be2810f47083b328 -X github.com/google/syzkaller/prog.gitRevisionDate=20251112-115923" -o ./bin/linux_amd64/syz-execprog github.com/google/syzkaller/tools/syz-execprog
mkdir -p ./bin/linux_amd64
g++ -o ./bin/linux_amd64/syz-executor executor/executor.cc \
-m64 -O2 -pthread -Wall -Werror -Wparentheses -Wunused-const-variable -Wframe-larger-than=16384 -Wno-stringop-overflow -Wno-array-bounds -Wno-format-overflow -Wno-unused-but-set-variable -Wno-unused-command-line-argument -static-pie -std=c++17 -I. -Iexecutor/_include -DGOOS_linux=1 -DGOARCH_amd64=1 \
-DHOSTGOOS_linux=1 -DGIT_REVISION=\"07e030dea6e6d9ca88b75bb3be2810f47083b328\"
/usr/bin/ld: /tmp/ccqeZpgW.o: in function `Connection::Connect(char const*, char const*)':
executor.cc:(.text._ZN10Connection7ConnectEPKcS1_[_ZN10Connection7ConnectEPKcS1_]+0x104): warning: Using 'gethostbyname' in statically linked applications requires at runtime the shared libraries from the glibc version used for linking
./tools/check-syzos.sh 2>/dev/null
Tested on:
commit: 24172e0d Merge tag 'arm64-fixes' of git://git.kernel.o..
git tree: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel config: https://syzkaller.appspot.com/x/.config?x=cb128cd5cb439809
dashboard link: https://syzkaller.appspot.com/bug?extid=4f5f81e1456a1f645bf8
compiler: gcc (Debian 12.2.0-14+deb12u1) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=16573c12580000
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: i2c-hid: Introduce FocalTech FT8112
From: Krzysztof Kozlowski @ 2025-11-13 7:56 UTC (permalink / raw)
To: daniel_peng
Cc: Dmitry Torokhov, linux-input, LKML, Conor Dooley,
Krzysztof Kozlowski, Rob Herring, devicetree
In-Reply-To: <20251113140004.v4.1.I894dde5015f4acad94cb5bada61e5811c5142395@changeid>
On Thu, Nov 13, 2025 at 02:00:55PM +0800, daniel_peng@pegatron.corp-partner.google.com wrote:
> From: Daniel Peng <Daniel_Peng@pegatron.corp-partner.google.com>
>
> The FocalTech FT8112 touch screen chip same as Ilitek ili2901 controller
So keep the device in that binding under enum. No need to create
document for every device, even if they were different but here it is
pretty obvious - same chip.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v8 00/10] HID: asus: Fix ASUS ROG Laptop's Keyboard backlight handling
From: Antheas Kapenekakis @ 2025-11-13 8:44 UTC (permalink / raw)
To: Denis Benato
Cc: Ilpo Järvinen, platform-driver-x86, linux-input, LKML,
Jiri Kosina, Benjamin Tissoires, Corentin Chary, Luke D . Jones,
Hans de Goede
In-Reply-To: <4671d267-d823-4bf7-af30-b587e67dec49@gmail.com>
On Thu, 13 Nov 2025 at 02:14, Denis Benato <benato.denis96@gmail.com> wrote:
>
>
> On 11/12/25 23:08, Antheas Kapenekakis wrote:
> > On Wed, 12 Nov 2025 at 20:51, Denis Benato <benato.denis96@gmail.com> wrote:
> >>
> >> On 11/12/25 14:41, Antheas Kapenekakis wrote:
> >>> On Wed, 12 Nov 2025 at 14:22, Ilpo Järvinen
> >>> <ilpo.jarvinen@linux.intel.com> wrote:
> >>>> On Wed, 12 Nov 2025, Antheas Kapenekakis wrote:
> >>>>
> >>>>> On Sat, 1 Nov 2025 at 11:47, Antheas Kapenekakis <lkml@antheas.dev> wrote:
> >>>>>> This is a two part series which does the following:
> >>>>>> - Clean-up init sequence
> >>>>>> - Unify backlight handling to happen under asus-wmi so that all Aura
> >>>>>> devices have synced brightness controls and the backlight button works
> >>>>>> properly when it is on a USB laptop keyboard instead of one w/ WMI.
> >>>>>>
> >>>>>> For more context, see cover letter of V1. Since V5, I removed some patches
> >>>>>> to make this easier to merge.
> >>>>> Small bump for this.
> >>>> I looked at v8 earlier but then got an impression some of Denis' comments
> >>>> against v7 were not taken into account in v8, which implies there will be
> >>>> delay until I've time to delve into the details (I need to understand
> >>>> things pretty deeply in such a case, which does take lots of time).
> >>>>
> >>>> Alternatively, if Denis says v8 is acceptable, then I don't need to spend
> >>>> so much time on it, but somehow I've a feeling he isn't happy with v8
> >>>> but just hasn't voiced it again...
> >>>>
> >>>> Please do realize that ignoring reviewer feedback without a very very good
> >>>> reason always risks adding delay or friction into getting things
> >>>> upstreamed. Especially, when the review comes from a person who has been
> >>>> around for me to learn to trust their reviews or from a maintainer of the
> >>>> code in question.
> >>> Sure, sorry if it came out this way. Dennis had two comments on the V7
> >>> version of the series.
> >>>
> >>> The first one was that asusctl has a hang bug, which he hasn't had
> >>> time to look into yet. This should have been fixed by dropping the
> >>> HID_QUIRK_INPUT_PER_APP. I retested the series and that QUIRK was a
> >>> bit of a NOOP that does not need to be added in the future.
> >> So it is supposed to not regress it now, correct?
> >>> The second is he is concerned with dropping the 0x5d/0x5e inits. Luke
> >>> said (back in March) that it is fine to drop 0x5e because it is only
> >>> used for ANIME displays. However, for 0x5d, it is hard to verify some
> >>> of the older laptops because they have only been tested with 0x5d and
> >>> we do not have the hardware in question to test.
> >>>
> >>> For this series, I re-added "initialize LED endpoint early for old
> >>> NKEY keyboards" that re-adds 0x5d for the keyboards that cannot be
> >>> tested again so this comment should be resolved too. With that in
> >>> mind, we do end up with an additional quirk/command that may be
> >>> unneeded and will remain there forever, but since it was a point of
> >>> contention, it is not worth arguing over.
> >>>
> >>> So both comments should be resolved
> >> The driver should also not late-initialize anything.
> >>
> >> Windows doesn't do it and the official asus application
> >> can freely send LEDs changing commands to either WMI or USB
> >> so I don't see any reason to do things differently [than windows]
> >> and not prepare every USB endpoint to receive commands,
> >> this has not been addressed unless I confused v7 and v8?
> > Yes, it's been added on v8. 0x5d is init for the laptops it is
> > problematic for. Not because it does not work, but because it has not
> > been verified to work for those laptops.
> I am not sure I am reading this right:
> are you telling me that on recent models the windows driver
> doesn't issue 0x5d?
Try to add spaces in your replies. This is hard to follow.
Do not conflate driver with software. 0x5a (over the application
0xff310076) has traditionally been used by a driver in Windows to
control the backlight level, as it is done in this driver. 0x5d (over
the application 0xff310079) is only used by laptops with RGB by
Armoury crate. But this driver does not do RGB. No device
functionality relies on it being sent for any device I've seen. The
device remembers its Windows settings, incl. the backlight color, in
the absence of a driver.
Laptops without RGB such as the Duo series which I would like to add
support for next only have a 0x5a endpoint. But, they are sent garbage
inits for 0x5d and 0x5e currently. This should be fixed.
Moreso, it seems that Armoury crate on the Xbox Ally series uses
exclusively 0x5a commands and if you use 0x5d it ignores them (perhaps
RGB still works though). With the previous generation, commands worked
for either report id.
> >>> @Denis: can give an ack if this is the case?
> >>>
> >>> As for Derek's comment, he has a PR for his project where he removes
> >>> the name match for Ally devices with ample time for it to be merged
> >>> until kernel 6.19 is released. In addition, that specific software for
> >>> full functionality relies on OOT drivers on the distros it ships with,
> >>> so it is minimally affected in either case.
> >> The part we are talking about depends on this driver (hid-asus)
> >> and there are people on asus-linux community using inputplumber
> >> for non-ally devices (the OOT driver is only for ally devices)
> >> therefore it is very important to us (and various other distributions)
> >> not to break that software in any way.
> > This driver is only used for Ally devices. If you mean that people
> > remap their keyboards using inputplumber I guess they could but I have
> > not seen it.
> I meant people remap keyboards using IP. I am sure there were
> (and very probably still are) people doing that.
> >> Weighting pros and cons of changing the name I am not sure
> >> changing name brings any benefit? Or am I missing something here?
> >> It's simply used by userspace so the hardware should be loading
> >> regardless of the name...
> > Users see the name of their devices in their settings menu. They
> > should be accurate. Also, the early entry needs to be added anyway to
> > prevent kicking devices.
> If it's just aesthetics I don't see much reasons in changing the name.
>
> "the early entry needs to be added anyway ...." has no meaning to me,
> please rephrase. Sorry.
Early exit-
> >> Along with IP and your tool and asusctl there is also openrgb,
> >> and a newborn application for asus devices (I don't have contacts
> >> with that dev nor I remember the name of the tool)
> >> and I am not even that sure these are all asus-related
> >> applications.
> > My tool never checked for names, it briefly did for around a month
> > after its creation for some devices until capability matches. Around
> > 6.1 and 6.7 the kernel changed the names of most USB devices and that
> > caused issues. It currently only uses name matches for VID/PID 0/0
> > devices created by the kernel. Specifically, WMI and AT Keyboards. I
> > am not sure there is a workaround for those. Asusctl also does not use
> > names either.
> But IP does, so I would like to hear confirmation from at least Derek
> before the merge that there won't be future issues.
>
> Interpret what I say here as a broad topic, not just name/PER_APP flag:
> avoid changing data flow on older models...
In [1] Derek removes the name matches
There are no other name matches concerning this driver in it.
The data flow is not changed in this series; you should go through the
patches once again if you think that. The only difference is 0x5e is
not sent, and 0x5d is not sent for newer devices.
[1] https://github.com/ShadowBlip/InputPlumber/pull/453
> >> Excercise EXTRA care touching this area as these are enough changes
> >> to make it difficult to understand what exactly is the problem if
> >> someone shows up with LEDs malfunctioning, laptop not entering sleep
> >> anymore or something else entirely. Plus over time
> >> ASUS has used various workarounds for windows problems
> >> and I am not eager to find out what those are since there is only
> >> a realistic way it's going to happen....
> > These changes are not doing something extraordinary. It's just a minor cleanup.
> >
> >>> Moreover, that specific commit is needed for Xbox Ally devices anyway,
> >>> as the kernel kicks one of the RGB endpoints because it does not
> >>> register an input device (the check skipped by early return) so
> >>> userspace becomes unable to control RGB on a stock kernel
> >>> (hidraw/hiddev nodes are gone). For more context here, this specific
> >>> endpoint implements the RGB Lamparray protocol for Windows dynamic
> >>> lighting, and I think in an attempt to make it work properly in
> >>> Windows, Asus made it so Windows has to first disable dynamic lighting
> >>> for armoury crate RGB commands to work (the 0x5a ones over the 0xff31
> >>> hid page).
> >> Yes once ASUS introduces something new it sticks with that for
> >> future models so it's expected more and more laptops will have
> >> the same problem: I am not questioning if these patches are needed
> >> as they very clearly are; I am questioning if everything that these
> >> patches are doing are worth doing and aren't breaking/regressing
> >> either tools or the flow of actions between the EC and these USB devices.
> > Well, this series is needed to account for that. Sending the disable
> > command is out of scope for now though.
> Here I apologize for confusion: my comments were mostly about
> older models: I absolutely don't want to break those, but if you find a way
> to distinguish them from newer models that would give you more freedom with those.
Yes, we know their specific PIDs, so if you see the patch that adds
the 0x5d init, it is only added for those models.
> No disable commands unless we find hard evidence those are strictly needed.
They are needed for the Xbox Ally series, but since this driver does
not do RGB it is out of scope.
> > Antheas
> >
> >>> Hopefully this clears things up
> >>>
> >>> Antheas
> >>>
> >>>>> Unrelated but I was b4ing this series on Ubuntu 24 and got BADSIG:
> >>>>> DKIM/antheas.dev. Is there a reference for fixing this on my host?
> >>>>> Perhaps it would help with spam
> >>>> I see BADSIG very often these days from b4 (thanks to gmail expiring
> >>>> things after 7 days or so, I recall hearing somewhere), I just ignore them
> >>>> entirely.
> >>>>
> >>>> AFAIK, that has never caused any delay to any patch in pdx86 domain so if
> >>>> that is what you thought is happening here, it's not the case.
> >>>> If your patch does appear in the pdx86 patchwork, there's even less reason
> >>>> to worry as I mostly pick patches to process using patchwork's list.
> >>> Turns out I had to update my DNS records. It should be good now.
> >>>
> >>>> --
> >>>> i.
> >> snipp
> >>>>>> 2.51.2
> >>>>>>
> >>>>>>
>
^ permalink raw reply
* Re: [PATCH v3] Input: rename INPUT_PROP_HAPTIC_TOUCHPAD to INPUT_PROP_PRESSUREPAD
From: Peter Hutterer @ 2025-11-13 9:20 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Benjamin Tissoires, Jonathan Denose, Jiri Kosina, Dmitry Torokhov,
Jonathan Corbet, Henrik Rydberg, linux-input, linux-kernel,
linux-doc, Angela Czubak, Sean O'Brien, Randy Dunlap
In-Reply-To: <c7popmowkdilnthufvne4rednntgae2cxqwdvmd3tkfeji24nl@yhpwili7qobt>
Dmitry - gentle ping?
Cheers,
Peter
On Thu, Nov 06, 2025 at 03:32:56PM +0100, Benjamin Tissoires wrote:
> On Nov 06 2025, Peter Hutterer wrote:
> > And expand it to encompass all pressure pads.
> >
> > Definition: "pressure pad" as used here as includes all touchpads that
> > use physical pressure to convert to click, without physical hinges. Also
> > called haptic touchpads in general parlance, Synaptics calls them
> > ForcePads.
> >
> > Most (all?) pressure pads are currently advertised as
> > INPUT_PROP_BUTTONPAD. The suggestion to identify them as pressure pads
> > by defining the resolution on ABS_MT_PRESSURE has been in the docs since
> > commit 20ccc8dd38a3 ("Documentation: input: define
> > ABS_PRESSURE/ABS_MT_PRESSURE resolution as grams") but few devices
> > provide this information.
> >
> > In userspace it's thus impossible to determine whether a device is a
> > true pressure pad (pressure equals pressure) or a normal clickpad with
> > (pressure equals finger size).
> >
> > Commit 7075ae4ac9db ("Input: add INPUT_PROP_HAPTIC_TOUCHPAD") introduces
> > INPUT_PROP_HAPTIC_TOUCHPAD but restricted it to those touchpads that
> > have support for userspace-controlled effects. Let's expand and rename
> > that definition to include all pressure pad touchpads since those that
> > do support FF effects can be identified by the presence of the
> > FF_HAPTIC bit.
> >
> > This means:
> > - clickpad: INPUT_PROP_BUTTONPAD
> > - pressurepad: INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD
> > - pressurepad with configurable haptics:
> > INPUT_PROP_BUTTONPAD + INPUT_PROP_PRESSUREPAD + FF_HAPTIC
> >
> > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
>
> Acked-by: Benjamin Tissoires <bentiss@kernel.org>
>
> Unless Jiri has any problem with it, I guess this would be fine going
> through your tree Dmitry.
>
> Cheers,
> Benjamin
>
>
> > ---
> > ftr, I picked PRESSUREPAD over Dmitry's PRESSURE_TOUCHPAD suggestion
> > because it matches better with the existing BUTTONPAD.
> >
> > Changes to v1: extra empty lines to render the lists as lists
> > Changes to v2: rename to PRESSUREPAD and rename it in the instances
> > where it's used in the code
> >
> > v1: https://lore.kernel.org/linux-input/20251030011735.GA969565@quokka/T/#u
> > v2: https://lore.kernel.org/linux-input/20251030011735.GA969565@quokka/T/#m9504de27b02d00a55d540fd9fec9aed3edd0133c
> >
> > Documentation/input/event-codes.rst | 25 ++++++++++++++++++-------
> > drivers/hid/hid-haptic.c | 2 +-
> > include/uapi/linux/input-event-codes.h | 2 +-
> > 3 files changed, 20 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/input/event-codes.rst b/Documentation/input/event-codes.rst
> > index 1ead9bb8d9c6..4424cbff251f 100644
> > --- a/Documentation/input/event-codes.rst
> > +++ b/Documentation/input/event-codes.rst
> > @@ -400,19 +400,30 @@ can report through the rotational axes (absolute and/or relative rx, ry, rz).
> > All other axes retain their meaning. A device must not mix
> > regular directional axes and accelerometer axes on the same event node.
> >
> > -INPUT_PROP_HAPTIC_TOUCHPAD
> > ---------------------------
> > +INPUT_PROP_PRESSUREPAD
> > +----------------------
> > +
> > +The INPUT_PROP_PRESSUREPAD property indicates that the device provides
> > +simulated haptic feedback (e.g. a vibrator motor situated below the surface)
> > +instead of physical haptic feedback (e.g. a hinge). This property is only set
> > +if the device:
> >
> > -The INPUT_PROP_HAPTIC_TOUCHPAD property indicates that device:
> > -- supports simple haptic auto and manual triggering
> > - can differentiate between at least 5 fingers
> > - uses correct resolution for the X/Y (units and value)
> > -- reports correct force per touch, and correct units for them (newtons or grams)
> > - follows the MT protocol type B
> >
> > +If the simulated haptic feedback is controllable by userspace the device must:
> > +
> > +- support simple haptic auto and manual triggering, and
> > +- report correct force per touch, and correct units for them (newtons or grams), and
> > +- provide the EV_FF FF_HAPTIC force feedback effect.
> > +
> > Summing up, such devices follow the MS spec for input devices in
> > -Win8 and Win8.1, and in addition support the Simple haptic controller HID table,
> > -and report correct units for the pressure.
> > +Win8 and Win8.1, and in addition may support the Simple haptic controller HID
> > +table, and report correct units for the pressure.
> > +
> > +Where applicable, this property is set in addition to INPUT_PROP_BUTTONPAD, it
> > +does not replace that property.
> >
> > Guidelines
> > ==========
> > diff --git a/drivers/hid/hid-haptic.c b/drivers/hid/hid-haptic.c
> > index aa090684c1f2..fc8a9997f815 100644
> > --- a/drivers/hid/hid-haptic.c
> > +++ b/drivers/hid/hid-haptic.c
> > @@ -86,7 +86,7 @@ int hid_haptic_input_configured(struct hid_device *hdev,
> > if (hi->application == HID_DG_TOUCHPAD) {
> > if (haptic->auto_trigger_report &&
> > haptic->manual_trigger_report) {
> > - __set_bit(INPUT_PROP_HAPTIC_TOUCHPAD, hi->input->propbit);
> > + __set_bit(INPUT_PROP_PRESSUREPAD, hi->input->propbit);
> > return 1;
> > }
> > return 0;
> > diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> > index 8ba48590bd2c..d21172c6a266 100644
> > --- a/include/uapi/linux/input-event-codes.h
> > +++ b/include/uapi/linux/input-event-codes.h
> > @@ -27,7 +27,7 @@
> > #define INPUT_PROP_TOPBUTTONPAD 0x04 /* softbuttons at top of pad */
> > #define INPUT_PROP_POINTING_STICK 0x05 /* is a pointing stick */
> > #define INPUT_PROP_ACCELEROMETER 0x06 /* has accelerometer */
> > -#define INPUT_PROP_HAPTIC_TOUCHPAD 0x07 /* is a haptic touchpad */
> > +#define INPUT_PROP_PRESSUREPAD 0x07 /* pressure triggers clicks */
> >
> > #define INPUT_PROP_MAX 0x1f
> > #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1)
> > --
> > 2.51.1
> >
^ permalink raw reply
* Re: [RFC PATCH v2 0/3] HID: Add support for multiple batteries per device
From: Benjamin Tissoires @ 2025-11-13 10:44 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
In-Reply-To: <20251113001508.713574-1-lzampier@redhat.com>
Hi Lucas,
On Nov 13 2025, Lucas Zampieri wrote:
> This RFC introduces support for multiple batteries per HID device, addressing
> a long-standing architectural limitation in the HID battery reporting subsystem.
(TBH, that cover letter screams AI :-P)
Stating a "long-standing architectural limitation" is a bit rough. The
need wasn't there or nobody took the time and effort to plug this hole
:)
General comment on this cover letter actually: it is way too detailed,
and try to iron out every corner case the AI bot thought would be fought
against.
Which leads to people actually nitpicking on the details below:
for example, I immediately thought the Wacom bit down below was wrong.
But really, all you need is multiple battery support on devices.
And this is fine. A couple of example would be good enough, and we can
move on.
>
> ## Background
>
> The current HID implementation explicitly prevents multiple batteries per device
> through an early return in hidinput_setup_battery() that enforces a single-battery
> assumption. Linux treats peripheral batteries (scope=Device) differently from system
> batteries, with desktop environments often displaying them separately or ignoring
> them entirely. However, this design doesn't account for modern multi-battery hardware patterns.
>
> ## Problem Statement
>
> Multiple battery scenarios that cannot be properly reported today:
>
> 1. Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro
> Wireless) - headset battery reported, dock battery invisible
> 2. Graphics tablets with stylus batteries (Wacom) - requires driver-specific
> workarounds
Technically, most Wacom styluses are battery-less for their pencils. The
battery issue that was related below is for a AES device, which is an
integrated wacom tablet in a laptop, so the only battery we have is on
the stylus itself. Arguably we can have several styluses, but only one
can be used at the same time, so reusing the same power supply device is
fine.
(see, I told you, that's the "someone is wrong on the internet effect")
> 3. Wireless earbuds with per-earbud batteries plus charging case
> 4. Multi-device receivers (Logitech Unifying) - requires proprietary HID++
> protocol parsing
That's an entire different reason why there is HID++, and the fact that
Solaar handles batteries itself is lack of support in the kernel for
some devices. (and because sometimes it's much simpler to update a user
space client than a kernel driver)
>
> This forces manufacturers to use proprietary protocols and vendor-specific
> software. Community projects parse USB packets directly because standard HID
> battery reporting cannot handle multi-battery scenarios.
I don't think Linux ever forced any manufacturer to use proprietary
protocols or vendor-specific software. For Logitech (or any gaming mouse
FWIW), those protocols were in place well before Linux ever had support
for them.
>
> ## Why This Matters
>
> The current limitation creates a cycle: OS lacks support, so manufacturers
> implement proprietary protocols, which makes vendor software necessary, which
> reduces pressure to fix the OS limitation. Improving HID core support for
> multiple batteries would enable standardized reporting, reduce the need for
> vendor software, improve OS integration, reduce driver duplication, and provide
> a foundation for future multi-battery devices.
This is completely BS and AI generated. There is no such cycle.
But again, the need is there, so we need to push this series forward.
The packaging of the cover letter is wrong. We are not salesmen, there
is no need to pitch this like that.
>
> ## Proposed Solution
>
> This series introduces struct hid_battery to encapsulate individual battery
> state, refactors the code to use this structure internally, and adds support
> for multiple batteries tracked in a list within struct hid_device. Batteries
> are identified by report ID. The implementation maintains full backwards
> compatibility with existing single-battery code.
That paragraph is good. Keep it please :)
>
> ## Testing
>
> Tested with split keyboard hardware (Dactyl 5x6) using custom ZMK firmware
> that implements per-side HID battery reporting. Each battery (left and right
> keyboard halves) reports independently through the power supply interface with
> distinct report IDs (0x05 and 0x06).
>
> Test firmware available on my personal fork at:
> https://github.com/zampierilucas/zmk/tree/feat/individual-hid-battery-reporting
> If this series gets merged, these changes will be proposed to upstream ZMK.
>
> HID descriptor and recording captured with hid-recorder:
>
> D: 0
> R: 162 05 01 09 06 a1 01 85 01 05 07 19 e0 29 e7 15 00 25 01 75 01 95 08 81 02 05 07 75 08 95 01 81 03 05 07 15 00 25 01 19 00 29 67 75 01 95 68 81 02 c0 05 0c 09 01 a1 01 85 02 05 0c 15 00 26 ff 0f 19 00 2a ff 0f 75 10 95 06 81 00 c0 05 84 09 05 a1 01 05 85 85 05 09 44 15 00 25 01 35 00 45 01 75 08 95 01 81 02 09 65 15 00 25 64 35 00 45 64 75 08 95 01 81 02 c0 05 84 09 05 a1 01 05 85 85 06 09 44 15 00 25 01 35 00 45 01 75 08 95 01 81 02 09 65 15 00 25 64 35 00 45 64 75 08 95 01 81 02 c0
> N: ZMK Project Dactyl 5x6
> P: usb-0000:2d:00.3-4.2/input2
> I: 3 1d50 615e
> D: 0
> E: 0.000000 3 05 00 56
> E: 0.000977 3 05 00 56
> E: 1.490974 3 06 00 52
> E: 1.491958 3 06 00 52
> E: 6.492979 3 06 00 53
> E: 6.493962 3 06 00 53
>
> The recording shows both batteries reporting with different charge levels
> (Report ID 05: 86%, Report ID 06: 82%-83%), demonstrating the multi-battery
> functionality. This can be used to verify UPower compatibility.
Same, please keep that testing note in future versions.
>
> ## Future Work: Userspace Integration
>
> As suggested by Bastien, semantic battery differentiation (e.g., "left
> earbud" vs "right earbud") requires userspace coordination, as HID
> reports typically lack role metadata.
The sad part is that HID can report a lot of things, but manufacturers
don't make a full use of it.
For example, in the split keyboard case, you could make use of the
Handedness NAry of the generic device Controls page (0x06) which can
help describing Left Hand (0x32) and Right Hand (0x33).
But the problem is more likely that no known manufacturer will follow
your lead on this, and the NAry code you put here will only be used by
your firmware.
For the bluetooth earbuds, the Logical collection Channel Left (0x161,
page Consumer Page 0x0c) and Channel Right (0x162, same page) could be
used.
>
> This will require:
> 1. systemd/hwdb entries for device-specific battery role mappings
> 2. UPower updates to enumerate and group multi-battery devices
> 3. Desktop environment changes to display batteries with meaningful labels
>
> This kernel infrastructure is a prerequisite for that userspace work.
>
> ## Request for Comments
>
> Is list-based storage appropriate or would another structure work better?
List based storage should be fine. We don't receive battery updates that
often, so if that is simpler for you, that's all right.
> Should we support usage-based identification in addition to report ID for
> devices using the same report ID?
If the question is:
- should we enforce each battery to have a different name, the answer is
yes
- should we take the report ID as the battery identifier in the name:
why not?
- should we make use of other collections to split up batteries: maybe.
If you have a device which reports multiple batteries on the same
report ID, then yes, we will need a way to split them.
But again, we are only theorically speaking here, so it's hard to
answer. I really wish we had an actual consumer device we could use as a
reference, not a custom firmware we can tune in any way we want.
> Is sequential naming (battery-N) sufficient
> or should batteries have semantic role identifiers like "main", "stylus", "dock"?
Semantic would be ideal, but at least having a unique identifier is a
requirement. That's more a question for the upower guys.
>
> To HID maintainers (Jiri Kosina, Benjamin Tissoires): Does this belong in
> hid-input.c or should it be separate?
hid-input.c seems fine to me. There are technically not a lot of
differences from the existing AFAICT, it's just that we iterate over a
list.
> Any concerns about the backwards
> compatibility approach? Meaning, should I have removed the whole
> dev->bat legacy mapping and use the new struct?
I'm not sure I follow that backwards compatibility. AFAIU that API is
kernel internal, so it should be easier to drop it entirely instead of
duplicating the data for no benefit.
>
> To power supply maintainers (Sebastian Reichel): Any issues with multiple
> power_supply devices from a single HID device?
Not a power supply maintainer, but unless upower builds a graph of the
devices it sees, I don't see why this would be a problem.
>
> Related commits:
> - c6838eeef2fb: HID: hid-input: occasionally report stylus battery
> - a608dc1c0639: HID: input: map battery system charging
> - fd2a9b29dc9c: HID: wacom: Remove AES power_supply after inactivity
What the point of those?
>
> Community projects demonstrating the need:
> - HeadsetControl: https://github.com/Sapd/HeadsetControl
> - Solaar: https://github.com/pwr-Solaar/Solaar
> - OpenRazer: https://github.com/openrazer/openrazer
>
> Lucas Zampieri (3):
> HID: input: Introduce struct hid_battery
> HID: input: Refactor battery code to use struct hid_battery
> HID: input: Add support for multiple batteries per device
>
> Changes in v2:
> - Split the monolithic v1 patch into three logical patches for easier review:
> 1. Introduce struct hid_battery (pure structure addition)
> 2. Refactor existing code to use the new structure (internal changes)
> 3. Add multi-battery support (new functionality)
> - Added detailed testing section with hardware specifics
> - Added hid-recorder output (dactyl-hid-recording.txt) demonstrating two-battery
> HID descriptor for UPower validation
> - Added "Future Work: Userspace Integration" section addressing Bastien's feedback
> about semantic battery differentiation
> - Added hardware examples with product links to commit messages (per Bastien's
> suggestion)
> - No functional changes from v1, only improved patch organization and documentation
>
> drivers/hid/hid-core.c | 4 +
> drivers/hid/hid-input.c | 196 +++++++++++++++++++++++++++-------------
> include/linux/hid.h | 42 ++++++++-
> 3 files changed, 179 insertions(+), 63 deletions(-)
>
> --
> 2.51.1
>
And now after all of this bikeshedding, I can move on to the review of
the series.
Cheers,
Benjamin
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery
From: Benjamin Tissoires @ 2025-11-13 10:47 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
In-Reply-To: <20251113001508.713574-2-lzampier@redhat.com>
On Nov 13 2025, Lucas Zampieri wrote:
> Add struct hid_battery to encapsulate battery state per HID device.
> This structure will allow tracking individual battery properties
> including capacity, min/max values, report information, and status.
>
> The structure includes a list node to enable support for multiple
> batteries per device in subsequent patches.
>
> This is a preparation step for transitioning from direct power_supply
> access to a more flexible battery management system.
>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
> include/linux/hid.h | 30 ++++++++++++++++++++++++++++++
I know Bastien asked you to split out this into a separate commit, but I
hate having a header change when noone uses it. It is painful for people
needing to backport the further changes (imagine you are fixing a CVE
without noticing it), as they also need to pull this one.
> 1 file changed, 30 insertions(+)
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index a4ddb94e3ee5..63422130de20 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -634,6 +634,36 @@ enum hid_battery_status {
> HID_BATTERY_REPORTED, /* Device sent unsolicited battery strength report */
> };
>
> +/**
> + * struct hid_battery - represents a single battery power supply
> + * @list: list node for linking into hid_device's battery list
For the first inclusion of the new struct, please drop the list field.
This should go into the last patch when you actually make use of it.
> + * @dev: pointer to the parent hid_device
> + * @ps: the power supply device
> + * @capacity: current battery capacity
> + * @min: minimum battery value
> + * @max: maximum battery value
> + * @report_type: type of report (HID_INPUT_REPORT, HID_FEATURE_REPORT)
> + * @report_id: report ID for this battery
> + * @charge_status: current charge status
> + * @status: battery status (unknown, queried, reported)
> + * @avoid_query: if true, don't query battery (wait for device reports)
> + * @ratelimit_time: time for rate limiting battery updates
> + */
> +struct hid_battery {
> + struct list_head list;
> + struct hid_device *dev;
> + struct power_supply *ps;
> + __s32 capacity;
> + __s32 min;
> + __s32 max;
> + __s32 report_type;
> + __s32 report_id;
> + __s32 charge_status;
> + enum hid_battery_status status;
> + bool avoid_query;
> + ktime_t ratelimit_time;
> +};
> +
> struct hid_driver;
> struct hid_ll_driver;
>
> --
> 2.51.1
>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH v1 3/5] Input: gpio_decoder - replace custom loop by gpiod_get_array_value_cansleep()
From: kernel test robot @ 2025-11-13 10:47 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: oe-kbuild-all, Dmitry Torokhov
In-Reply-To: <20251112191412.2088105-4-andriy.shevchenko@linux.intel.com>
Hi Andy,
kernel test robot noticed the following build errors:
[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus hid/for-next linus/master v6.18-rc5 next-20251113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/Input-gpio_decoder-make-use-of-device-properties/20251113-032111
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20251112191412.2088105-4-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 3/5] Input: gpio_decoder - replace custom loop by gpiod_get_array_value_cansleep()
config: x86_64-buildonly-randconfig-004-20251113 (https://download.01.org/0day-ci/archive/20251113/202511131832.JljMfKrW-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251113/202511131832.JljMfKrW-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511131832.JljMfKrW-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/device.h:15,
from drivers/input/misc/gpio_decoder.c:10:
drivers/input/misc/gpio_decoder.c: In function 'gpio_decoder_get_gpios_state':
>> drivers/input/misc/gpio_decoder.c:38:67: error: 'val' undeclared (first use in this function)
38 | dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
| ^~~
include/linux/dev_printk.h:110:37: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/input/misc/gpio_decoder.c:38:17: note: in expansion of macro 'dev_err'
38 | dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
| ^~~~~~~
drivers/input/misc/gpio_decoder.c:38:67: note: each undeclared identifier is reported only once for each function it appears in
38 | dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
| ^~~
include/linux/dev_printk.h:110:37: note: in definition of macro 'dev_printk_index_wrap'
110 | _p_func(dev, fmt, ##__VA_ARGS__); \
| ^~~~~~~~~~~
drivers/input/misc/gpio_decoder.c:38:17: note: in expansion of macro 'dev_err'
38 | dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
| ^~~~~~~
vim +/val +38 drivers/input/misc/gpio_decoder.c
27
28 static int gpio_decoder_get_gpios_state(struct gpio_decoder *decoder)
29 {
30 struct gpio_descs *gpios = decoder->input_gpios;
31 DECLARE_BITMAP(values, 32);
32 unsigned int size;
33 int err;
34
35 size = min(gpios->ndescs, 32U);
36 err = gpiod_get_array_value_cansleep(size, gpios->desc, gpios->info, values);
37 if (err) {
> 38 dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
39 return err;
40 }
41
42 return bitmap_read(values, 0, size);
43 }
44
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: i2c-hid: Introduce FocalTech FT8112
From: Krzysztof Kozlowski @ 2025-11-13 10:52 UTC (permalink / raw)
To: Daniel Peng
Cc: Dmitry Torokhov, linux-input, LKML, Conor Dooley,
Krzysztof Kozlowski, Rob Herring, devicetree
In-Reply-To: <CAAq2-DFJ2HZQ=p5J7wppQWYh9tqrFxNqexYXFcVB=b1ufWgmXg@mail.gmail.com>
On 13/11/2025 11:44, Daniel Peng wrote:
> Hi Krzysztof,
>
> Sorry for the confusion.
> I just refer ilitek,ili2901.yaml as example to modify correct information
> for this device. And I create document file for FocalTech FT8112 device for
> Skywalker platform only.
> Moreover, modified related interrupt and reset gpio to map for Skywaler
> platform(MT8189).
>
> I think it would be good to create document. If any, please let me know.
>
Please don't top post.
There is no need for new document, everything is identical, so just add
new compatible to the other binding.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [RFC PATCH v2 2/3] HID: input: Refactor battery code to use struct hid_battery
From: Benjamin Tissoires @ 2025-11-13 10:54 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
In-Reply-To: <20251113001508.713574-3-lzampier@redhat.com>
On Nov 13 2025, Lucas Zampieri wrote:
> Refactor the battery handling code to use the newly introduced
> struct hid_battery internally, replacing direct access to individual
> power_supply and state fields.
>
> The legacy dev->battery and dev->battery_* fields are maintained and
> synchronized for backward compatibility. This refactoring prepares
> the code for supporting multiple batteries per device in a subsequent
> patch.
>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
> drivers/hid/hid-input.c | 125 ++++++++++++++++++++++++++--------------
> 1 file changed, 83 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index e56e7de53279..0e71efea9da3 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -454,7 +454,8 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> enum power_supply_property prop,
> union power_supply_propval *val)
> {
> - struct hid_device *dev = power_supply_get_drvdata(psy);
> + struct hid_battery *bat = power_supply_get_drvdata(psy);
> + struct hid_device *dev = bat->dev;
> int value;
> int ret = 0;
>
> @@ -465,13 +466,13 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> break;
>
> case POWER_SUPPLY_PROP_CAPACITY:
> - if (dev->battery_status != HID_BATTERY_REPORTED &&
> - !dev->battery_avoid_query) {
> + if (bat->status != HID_BATTERY_REPORTED &&
> + !bat->avoid_query) {
> value = hidinput_query_battery_capacity(dev);
> if (value < 0)
> return value;
> } else {
> - value = dev->battery_capacity;
> + value = bat->capacity;
> }
>
> val->intval = value;
> @@ -482,20 +483,20 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> break;
>
> case POWER_SUPPLY_PROP_STATUS:
> - if (dev->battery_status != HID_BATTERY_REPORTED &&
> - !dev->battery_avoid_query) {
> + if (bat->status != HID_BATTERY_REPORTED &&
> + !bat->avoid_query) {
> value = hidinput_query_battery_capacity(dev);
> if (value < 0)
> return value;
>
> - dev->battery_capacity = value;
> - dev->battery_status = HID_BATTERY_QUERIED;
> + bat->capacity = value;
> + bat->status = HID_BATTERY_QUERIED;
> }
>
> - if (dev->battery_status == HID_BATTERY_UNKNOWN)
> + if (bat->status == HID_BATTERY_UNKNOWN)
> val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
> else
> - val->intval = dev->battery_charge_status;
> + val->intval = bat->charge_status;
> break;
>
> case POWER_SUPPLY_PROP_SCOPE:
> @@ -513,8 +514,9 @@ static int hidinput_get_battery_property(struct power_supply *psy,
> static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> struct hid_field *field, bool is_percentage)
> {
> + struct hid_battery *bat;
> struct power_supply_desc *psy_desc;
> - struct power_supply_config psy_cfg = { .drv_data = dev, };
> + struct power_supply_config psy_cfg;
> unsigned quirks;
> s32 min, max;
> int error;
> @@ -530,16 +532,22 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> if (quirks & HID_BATTERY_QUIRK_IGNORE)
> return 0;
>
> - psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> - if (!psy_desc)
> + bat = kzalloc(sizeof(*bat), GFP_KERNEL);
> + if (!bat)
> return -ENOMEM;
>
> + psy_desc = kzalloc(sizeof(*psy_desc), GFP_KERNEL);
> + if (!psy_desc) {
> + error = -ENOMEM;
> + goto err_free_bat;
> + }
> +
> psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> strlen(dev->uniq) ?
> dev->uniq : dev_name(&dev->dev));
> if (!psy_desc->name) {
> error = -ENOMEM;
> - goto err_free_mem;
> + goto err_free_desc;
I wonder if a devm conversion of the whole battery support handling
would not be beneficial here.
> }
>
> psy_desc->type = POWER_SUPPLY_TYPE_BATTERY;
> @@ -559,63 +567,85 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> if (quirks & HID_BATTERY_QUIRK_FEATURE)
> report_type = HID_FEATURE_REPORT;
>
> - dev->battery_min = min;
> - dev->battery_max = max;
> - dev->battery_report_type = report_type;
> - dev->battery_report_id = field->report->id;
> - dev->battery_charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
> + /* Initialize battery structure */
> + bat->dev = dev;
> + bat->min = min;
> + bat->max = max;
> + bat->report_type = report_type;
> + bat->report_id = field->report->id;
> + bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
> + bat->status = HID_BATTERY_UNKNOWN;
>
> /*
> * Stylus is normally not connected to the device and thus we
> * can't query the device and get meaningful battery strength.
> * We have to wait for the device to report it on its own.
> */
> - dev->battery_avoid_query = report_type == HID_INPUT_REPORT &&
> - field->physical == HID_DG_STYLUS;
> + bat->avoid_query = report_type == HID_INPUT_REPORT &&
> + field->physical == HID_DG_STYLUS;
>
> if (quirks & HID_BATTERY_QUIRK_AVOID_QUERY)
> - dev->battery_avoid_query = true;
> + bat->avoid_query = true;
>
> - dev->battery = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
> - if (IS_ERR(dev->battery)) {
> - error = PTR_ERR(dev->battery);
> + psy_cfg.drv_data = bat;
> + bat->ps = power_supply_register(&dev->dev, psy_desc, &psy_cfg);
> + if (IS_ERR(bat->ps)) {
> + error = PTR_ERR(bat->ps);
> hid_warn(dev, "can't register power supply: %d\n", error);
> goto err_free_name;
> }
>
> - power_supply_powers(dev->battery, &dev->dev);
> + power_supply_powers(bat->ps, &dev->dev);
> +
> + /* Maintain legacy single battery fields for backward compatibility */
> + dev->battery = bat->ps;
> + dev->battery_min = bat->min;
> + dev->battery_max = bat->max;
> + dev->battery_report_type = bat->report_type;
> + dev->battery_report_id = bat->report_id;
> + dev->battery_charge_status = bat->charge_status;
> + dev->battery_status = bat->status;
> + dev->battery_avoid_query = bat->avoid_query;
Already mentioned in the cover letter, but what's the point of keeping
those legacy fields when the exact same data is stored in bat?
> +
> return 0;
>
> err_free_name:
> kfree(psy_desc->name);
> -err_free_mem:
> +err_free_desc:
> kfree(psy_desc);
> - dev->battery = NULL;
> +err_free_bat:
> + kfree(bat);
> return error;
> }
>
> static void hidinput_cleanup_battery(struct hid_device *dev)
> {
> + struct hid_battery *bat;
> const struct power_supply_desc *psy_desc;
>
> if (!dev->battery)
> return;
>
> + bat = power_supply_get_drvdata(dev->battery);
> psy_desc = dev->battery->desc;
> power_supply_unregister(dev->battery);
> kfree(psy_desc->name);
> kfree(psy_desc);
> + kfree(bat);
> dev->battery = NULL;
> }
>
> -static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> +static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> unsigned int usage, int value)
> {
> switch (usage) {
> case HID_BAT_CHARGING:
> - dev->battery_charge_status = value ?
> - POWER_SUPPLY_STATUS_CHARGING :
> - POWER_SUPPLY_STATUS_DISCHARGING;
> + bat->charge_status = value ?
> + POWER_SUPPLY_STATUS_CHARGING :
> + POWER_SUPPLY_STATUS_DISCHARGING;
> + /* Update legacy field for backward compatibility */
> + if (bat->dev->battery == bat->ps)
> + bat->dev->battery_charge_status = bat->charge_status;
> return true;
> }
>
> @@ -625,32 +655,43 @@ static bool hidinput_update_battery_charge_status(struct hid_device *dev,
> static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> int value)
> {
> + struct hid_battery *bat;
> int capacity;
>
> if (!dev->battery)
> return;
>
> - if (hidinput_update_battery_charge_status(dev, usage, value)) {
> - power_supply_changed(dev->battery);
> + bat = power_supply_get_drvdata(dev->battery);
> +
> + if (hidinput_update_battery_charge_status(bat, usage, value)) {
> + power_supply_changed(bat->ps);
> return;
> }
>
> if ((usage & HID_USAGE_PAGE) == HID_UP_DIGITIZER && value == 0)
> return;
>
> - if (value < dev->battery_min || value > dev->battery_max)
> + if (value < bat->min || value > bat->max)
> return;
>
> capacity = hidinput_scale_battery_capacity(dev, value);
>
> - if (dev->battery_status != HID_BATTERY_REPORTED ||
> - capacity != dev->battery_capacity ||
> - ktime_after(ktime_get_coarse(), dev->battery_ratelimit_time)) {
> - dev->battery_capacity = capacity;
> - dev->battery_status = HID_BATTERY_REPORTED;
> - dev->battery_ratelimit_time =
> + if (bat->status != HID_BATTERY_REPORTED ||
> + capacity != bat->capacity ||
> + ktime_after(ktime_get_coarse(), bat->ratelimit_time)) {
> + bat->capacity = capacity;
> + bat->status = HID_BATTERY_REPORTED;
> + bat->ratelimit_time =
> ktime_add_ms(ktime_get_coarse(), 30 * 1000);
> - power_supply_changed(dev->battery);
> +
> + /* Update legacy fields for backward compatibility */
> + if (dev->battery == bat->ps) {
> + dev->battery_capacity = bat->capacity;
> + dev->battery_status = bat->status;
> + dev->battery_ratelimit_time = bat->ratelimit_time;
> + }
> +
> + power_supply_changed(bat->ps);
> }
> }
> #else /* !CONFIG_HID_BATTERY_STRENGTH */
> --
> 2.51.1
>
Cheers,
Benjamin
^ permalink raw reply
* Re: [RFC PATCH v2 3/3] HID: input: Add support for multiple batteries per device
From: Benjamin Tissoires @ 2025-11-13 11:08 UTC (permalink / raw)
To: Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
Bastien Nocera, linux-pm
In-Reply-To: <20251113001508.713574-4-lzampier@redhat.com>
On Nov 13 2025, Lucas Zampieri wrote:
> Enable HID devices to register and manage multiple batteries by
> maintaining a list of hid_battery structures, each identified by
> its report ID.
>
> The legacy dev->battery field and related fields are maintained for
> backward compatibility, pointing to the first battery in the list.
> This allows existing code to continue working unchanged while
> enabling new functionality for multi-battery devices.
>
> Example hardware that can benefit from this:
> - Gaming headsets with charging docks (e.g., SteelSeries Arctis Nova Pro
> Wireless)
> - Graphics tablets with stylus batteries (Wacom)
> - Wireless earbuds with per-earbud batteries plus charging case
> - Split keyboards with independent battery per side
>
> Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> ---
> drivers/hid/hid-core.c | 4 ++
> drivers/hid/hid-input.c | 99 +++++++++++++++++++++++++++--------------
> include/linux/hid.h | 12 ++++-
> 3 files changed, 80 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index a5b3a8ca2fcb..76d628547e9a 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2990,6 +2990,10 @@ struct hid_device *hid_allocate_device(void)
> mutex_init(&hdev->ll_open_lock);
> kref_init(&hdev->ref);
>
> +#ifdef CONFIG_HID_BATTERY_STRENGTH
> + INIT_LIST_HEAD(&hdev->batteries);
> +#endif
> +
> ret = hid_bpf_device_init(hdev);
> if (ret)
> goto out_err;
> diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
> index 0e71efea9da3..9d0be3d4ce04 100644
> --- a/drivers/hid/hid-input.c
> +++ b/drivers/hid/hid-input.c
> @@ -520,14 +520,20 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> unsigned quirks;
> s32 min, max;
> int error;
> + int battery_num = 0;
>
> - if (dev->battery)
> - return 0; /* already initialized? */
> + /* Check if battery with this report_id already exists */
> + list_for_each_entry(bat, &dev->batteries, list) {
> + if (bat->report_id == field->report->id)
> + return 0; /* already initialized */
I wonder if we should not make this test stick out a little bit more.
Something like "get_battery(field)" which would return a battery if the
field matches. This way, if we ever encounter multiple batteries
reported on the same report ID, we can always split them by logical
collection, physical, or something else.
> + battery_num++;
> + }
>
> quirks = find_battery_quirk(dev);
>
> - hid_dbg(dev, "device %x:%x:%x %d quirks %d\n",
> - dev->bus, dev->vendor, dev->product, dev->version, quirks);
> + hid_dbg(dev, "device %x:%x:%x %d quirks %d report_id %d\n",
> + dev->bus, dev->vendor, dev->product, dev->version, quirks,
> + field->report->id);
>
> if (quirks & HID_BATTERY_QUIRK_IGNORE)
> return 0;
> @@ -542,9 +548,17 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
> goto err_free_bat;
> }
>
> - psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> - strlen(dev->uniq) ?
> - dev->uniq : dev_name(&dev->dev));
> + /* Create unique name for each battery based on report ID */
> + if (battery_num == 0) {
> + psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery",
> + strlen(dev->uniq) ?
> + dev->uniq : dev_name(&dev->dev));
> + } else {
> + psy_desc->name = kasprintf(GFP_KERNEL, "hid-%s-battery-%d",
> + strlen(dev->uniq) ?
> + dev->uniq : dev_name(&dev->dev),
> + battery_num);
> + }
Not sure how much conservative you need to be here, but I would prefer
we stick to the same naming pattern whether this is the first or second
battery.
> if (!psy_desc->name) {
> error = -ENOMEM;
> goto err_free_desc;
> @@ -597,15 +611,23 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
>
> power_supply_powers(bat->ps, &dev->dev);
>
> - /* Maintain legacy single battery fields for backward compatibility */
> - dev->battery = bat->ps;
> - dev->battery_min = bat->min;
> - dev->battery_max = bat->max;
> - dev->battery_report_type = bat->report_type;
> - dev->battery_report_id = bat->report_id;
> - dev->battery_charge_status = bat->charge_status;
> - dev->battery_status = bat->status;
> - dev->battery_avoid_query = bat->avoid_query;
> + list_add_tail(&bat->list, &dev->batteries);
> +
> + /*
> + * The legacy single battery API is preserved by exposing the first
> + * discovered battery. Systems relying on a single battery view maintain
> + * unchanged behavior.
> + */
> + if (battery_num == 0) {
> + dev->battery = bat->ps;
> + dev->battery_min = bat->min;
> + dev->battery_max = bat->max;
> + dev->battery_report_type = bat->report_type;
> + dev->battery_report_id = bat->report_id;
> + dev->battery_charge_status = bat->charge_status;
> + dev->battery_status = bat->status;
> + dev->battery_avoid_query = bat->avoid_query;
> + }
Again, why keeping the old fields? Are they used anywhere?
>
> return 0;
>
> @@ -620,21 +642,33 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
>
> static void hidinput_cleanup_battery(struct hid_device *dev)
> {
> - struct hid_battery *bat;
> + struct hid_battery *bat, *next;
> const struct power_supply_desc *psy_desc;
>
> - if (!dev->battery)
> - return;
> + list_for_each_entry_safe(bat, next, &dev->batteries, list) {
> + psy_desc = bat->ps->desc;
> + power_supply_unregister(bat->ps);
> + kfree(psy_desc->name);
> + kfree(psy_desc);
> + list_del(&bat->list);
> + kfree(bat);
> + }
Really, switching to devm the batteries would greatly help not making
mistakes here.
>
> - bat = power_supply_get_drvdata(dev->battery);
> - psy_desc = dev->battery->desc;
> - power_supply_unregister(dev->battery);
> - kfree(psy_desc->name);
> - kfree(psy_desc);
> - kfree(bat);
> dev->battery = NULL;
> }
>
> +static struct hid_battery *hidinput_find_battery(struct hid_device *dev,
> + int report_id)
> +{
> + struct hid_battery *bat;
> +
> + list_for_each_entry(bat, &dev->batteries, list) {
> + if (bat->report_id == report_id)
> + return bat;
> + }
> + return NULL;
> +}
Oh, so you already have this function. So why not making use of it in
hidinput_setup_battery()?
> +
> static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> unsigned int usage, int value)
> {
> @@ -652,17 +686,16 @@ static bool hidinput_update_battery_charge_status(struct hid_battery *bat,
> return false;
> }
>
> -static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> - int value)
> +static void hidinput_update_battery(struct hid_device *dev, int report_id,
> + unsigned int usage, int value)
> {
> struct hid_battery *bat;
> int capacity;
>
> - if (!dev->battery)
> + bat = hidinput_find_battery(dev, report_id);
> + if (!bat)
> return;
>
> - bat = power_supply_get_drvdata(dev->battery);
> -
> if (hidinput_update_battery_charge_status(bat, usage, value)) {
> power_supply_changed(bat->ps);
> return;
> @@ -705,8 +738,8 @@ static void hidinput_cleanup_battery(struct hid_device *dev)
> {
> }
>
> -static void hidinput_update_battery(struct hid_device *dev, unsigned int usage,
> - int value)
> +static void hidinput_update_battery(struct hid_device *dev, int report_id,
> + unsigned int usage, int value)
Why not hidinput_update_battery(struct hid_battery *bat, unsigned int usage, int value)?
> {
> }
> #endif /* CONFIG_HID_BATTERY_STRENGTH */
> @@ -1574,7 +1607,7 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct
> return;
>
> if (usage->type == EV_PWR) {
> - hidinput_update_battery(hid, usage->hid, value);
> + hidinput_update_battery(hid, report->id, usage->hid, value);
With the suggested change in hidinput_update_battery, first query the
battery and then change it.
> return;
> }
>
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 63422130de20..a6e36835fb3c 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -700,8 +700,16 @@ struct hid_device {
> #ifdef CONFIG_HID_BATTERY_STRENGTH
> /*
> * Power supply information for HID devices which report
> - * battery strength. power_supply was successfully registered if
> - * battery is non-NULL.
> + * battery strength. Each battery is tracked separately in the
> + * batteries list.
> + */
> + struct list_head batteries; /* List of hid_battery structures */
> +
> + /*
> + * Legacy single battery support - kept for backwards compatibility.
> + * Points to the first battery in the list if any exists.
> + * power_supply was successfully registered if battery is non-NULL.
> + * DEPRECATED: New code should iterate through batteries list instead.
> */
> struct power_supply *battery;
Nah. hid_device is pure internal interface. So change all the users if
needed, but don't keep something around in the hope that others will do
the work for you.
For references there has been a very long discussion with Linus about
API changes, and the result was that any API change that introduced a
duplicate API was probably a bad design ;)
Cheers,
Benjamin
> __s32 battery_capacity;
> --
> 2.51.1
>
^ permalink raw reply
* Re: [RFC PATCH v2 1/3] HID: input: Introduce struct hid_battery
From: Bastien Nocera @ 2025-11-13 11:56 UTC (permalink / raw)
To: Benjamin Tissoires, Lucas Zampieri
Cc: linux-input, linux-kernel, Jiri Kosina, Sebastian Reichel,
linux-pm
In-Reply-To: <tuya626zqiabd6ejwbaafj4cq3nlngc2vzvvjwdl3rc5wkwhhh@2k6ehu62tviu>
On Thu, 2025-11-13 at 11:47 +0100, Benjamin Tissoires wrote:
> On Nov 13 2025, Lucas Zampieri wrote:
> > Add struct hid_battery to encapsulate battery state per HID device.
> > This structure will allow tracking individual battery properties
> > including capacity, min/max values, report information, and status.
> >
> > The structure includes a list node to enable support for multiple
> > batteries per device in subsequent patches.
> >
> > This is a preparation step for transitioning from direct
> > power_supply
> > access to a more flexible battery management system.
> >
> > Signed-off-by: Lucas Zampieri <lzampier@redhat.com>
> > ---
> > include/linux/hid.h | 30 ++++++++++++++++++++++++++++++
>
> I know Bastien asked you to split out this into a separate commit,
> but I
> hate having a header change when noone uses it. It is painful for
> people
> needing to backport the further changes (imagine you are fixing a CVE
> without noticing it), as they also need to pull this one.
I usually find that a stand-alone "this can't possibly introduce bugs"
commit to be better (and it makes it easier to concentrate on smaller
amounts of content).
For the CVE use case, I don't really understand the problem. Either you
forget to backport the commit that added the type, in which case it
just doesn't compile, or you can backport the whole series (I would
hope that there's enough metadata in the kernel git to figure that out,
isn't there?).
In any case, your subsystem, your rules, just thought I'd mention why I
asked Lucas to split this up.
Cheers
>
> > 1 file changed, 30 insertions(+)
> >
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index a4ddb94e3ee5..63422130de20 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -634,6 +634,36 @@ enum hid_battery_status {
> > HID_BATTERY_REPORTED, /* Device sent unsolicited
> > battery strength report */
> > };
> >
> > +/**
> > + * struct hid_battery - represents a single battery power supply
> > + * @list: list node for linking into hid_device's battery list
>
> For the first inclusion of the new struct, please drop the list
> field.
> This should go into the last patch when you actually make use of it.
>
> > + * @dev: pointer to the parent hid_device
> > + * @ps: the power supply device
> > + * @capacity: current battery capacity
> > + * @min: minimum battery value
> > + * @max: maximum battery value
> > + * @report_type: type of report (HID_INPUT_REPORT,
> > HID_FEATURE_REPORT)
> > + * @report_id: report ID for this battery
> > + * @charge_status: current charge status
> > + * @status: battery status (unknown, queried, reported)
> > + * @avoid_query: if true, don't query battery (wait for device
> > reports)
> > + * @ratelimit_time: time for rate limiting battery updates
> > + */
> > +struct hid_battery {
> > + struct list_head list;
> > + struct hid_device *dev;
> > + struct power_supply *ps;
> > + __s32 capacity;
> > + __s32 min;
> > + __s32 max;
> > + __s32 report_type;
> > + __s32 report_id;
> > + __s32 charge_status;
> > + enum hid_battery_status status;
> > + bool avoid_query;
> > + ktime_t ratelimit_time;
> > +};
> > +
> > struct hid_driver;
> > struct hid_ll_driver;
> >
> > --
> > 2.51.1
> >
>
> Cheers,
> Benjamin
^ permalink raw reply
* Re: [PATCH v1 3/5] Input: gpio_decoder - replace custom loop by gpiod_get_array_value_cansleep()
From: kernel test robot @ 2025-11-13 12:15 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel
Cc: llvm, oe-kbuild-all, Dmitry Torokhov
In-Reply-To: <20251112191412.2088105-4-andriy.shevchenko@linux.intel.com>
Hi Andy,
kernel test robot noticed the following build errors:
[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus hid/for-next linus/master v6.18-rc5 next-20251113]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Andy-Shevchenko/Input-gpio_decoder-make-use-of-device-properties/20251113-032111
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20251112191412.2088105-4-andriy.shevchenko%40linux.intel.com
patch subject: [PATCH v1 3/5] Input: gpio_decoder - replace custom loop by gpiod_get_array_value_cansleep()
config: x86_64-buildonly-randconfig-002-20251113 (https://download.01.org/0day-ci/archive/20251113/202511131958.6ItfY60O-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251113/202511131958.6ItfY60O-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511131958.6ItfY60O-lkp@intel.com/
All errors (new ones prefixed by >>):
>> drivers/input/misc/gpio_decoder.c:38:53: error: use of undeclared identifier 'val'
38 | dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
| ^
1 error generated.
vim +/val +38 drivers/input/misc/gpio_decoder.c
27
28 static int gpio_decoder_get_gpios_state(struct gpio_decoder *decoder)
29 {
30 struct gpio_descs *gpios = decoder->input_gpios;
31 DECLARE_BITMAP(values, 32);
32 unsigned int size;
33 int err;
34
35 size = min(gpios->ndescs, 32U);
36 err = gpiod_get_array_value_cansleep(size, gpios->desc, gpios->info, values);
37 if (err) {
> 38 dev_err(decoder->dev, "Error reading GPIO: %d\n", val);
39 return err;
40 }
41
42 return bitmap_read(values, 0, size);
43 }
44
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v7 3/6] Input: mc13783-pwrbutton: convert pdata members to array
From: Lee Jones @ 2025-11-13 14:30 UTC (permalink / raw)
To: Alexander Kurz
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Dmitry Torokhov, Dzmitry Sankouski, Griffin Kroah-Hartman,
Mathieu Dubois-Briand, Heiko Stuebner, Dr . David Alan Gilbert,
Job Sava, Alexandre Belloni, Uwe Kleine-König, devicetree,
imx, linux-arm-kernel, linux-kernel, linux-input
In-Reply-To: <20251031195718.1586-4-akurz@blala.de>
On Fri, 31 Oct 2025, Alexander Kurz wrote:
> As preparation for the extension of support for all three mc13xxx
> variants, convert the members of mc13xxx_buttons_platform_data to
> arrays to allow index access within the next commit.
>
> Signed-off-by: Alexander Kurz <akurz@blala.de>
> ---
> drivers/input/misc/mc13783-pwrbutton.c | 42 +++++++++++++-------------
> include/linux/mfd/mc13xxx.h | 8 ++---
Acked-by: Lee Jones <lee@kernel.org>
> 2 files changed, 23 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
> index 20f68aab6edf..2ee115d77b1c 100644
> --- a/drivers/input/misc/mc13783-pwrbutton.c
> +++ b/drivers/input/misc/mc13783-pwrbutton.c
> @@ -108,24 +108,24 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> - reg |= (pdata->b1on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
> - reg |= (pdata->b2on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
> - reg |= (pdata->b3on_flags & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
> + reg |= (pdata->b_on_flags[0] & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
> + reg |= (pdata->b_on_flags[1] & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
> + reg |= (pdata->b_on_flags[2] & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
>
> priv->pwr = pwr;
> priv->mc13783 = mc13783;
>
> mc13xxx_lock(mc13783);
>
> - if (pdata->b1on_flags & MC13783_BUTTON_ENABLE) {
> - priv->keymap[0] = pdata->b1on_key;
> - if (pdata->b1on_key != KEY_RESERVED)
> - __set_bit(pdata->b1on_key, pwr->keybit);
> + if (pdata->b_on_flags[0] & MC13783_BUTTON_ENABLE) {
> + priv->keymap[0] = pdata->b_on_key[0];
> + if (pdata->b_on_key[0] != KEY_RESERVED)
> + __set_bit(pdata->b_on_key[0], pwr->keybit);
>
> - if (pdata->b1on_flags & MC13783_BUTTON_POL_INVERT)
> + if (pdata->b_on_flags[0] & MC13783_BUTTON_POL_INVERT)
> priv->flags |= MC13783_PWRB_B1_POL_INVERT;
>
> - if (pdata->b1on_flags & MC13783_BUTTON_RESET_EN)
> + if (pdata->b_on_flags[0] & MC13783_BUTTON_RESET_EN)
> reg |= MC13783_POWER_CONTROL_2_ON1BRSTEN;
>
> irq = platform_get_irq_byname(pdev, "b1on");
> @@ -144,15 +144,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> priv->irq[0] = irq;
> }
>
> - if (pdata->b2on_flags & MC13783_BUTTON_ENABLE) {
> - priv->keymap[1] = pdata->b2on_key;
> - if (pdata->b2on_key != KEY_RESERVED)
> - __set_bit(pdata->b2on_key, pwr->keybit);
> + if (pdata->b_on_flags[1] & MC13783_BUTTON_ENABLE) {
> + priv->keymap[1] = pdata->b_on_key[1];
> + if (pdata->b_on_key[1] != KEY_RESERVED)
> + __set_bit(pdata->b_on_key[1], pwr->keybit);
>
> - if (pdata->b2on_flags & MC13783_BUTTON_POL_INVERT)
> + if (pdata->b_on_flags[1] & MC13783_BUTTON_POL_INVERT)
> priv->flags |= MC13783_PWRB_B2_POL_INVERT;
>
> - if (pdata->b2on_flags & MC13783_BUTTON_RESET_EN)
> + if (pdata->b_on_flags[1] & MC13783_BUTTON_RESET_EN)
> reg |= MC13783_POWER_CONTROL_2_ON2BRSTEN;
>
> irq = platform_get_irq_byname(pdev, "b2on");
> @@ -171,15 +171,15 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> priv->irq[1] = irq;
> }
>
> - if (pdata->b3on_flags & MC13783_BUTTON_ENABLE) {
> - priv->keymap[2] = pdata->b3on_key;
> - if (pdata->b3on_key != KEY_RESERVED)
> - __set_bit(pdata->b3on_key, pwr->keybit);
> + if (pdata->b_on_flags[2] & MC13783_BUTTON_ENABLE) {
> + priv->keymap[2] = pdata->b_on_key[2];
> + if (pdata->b_on_key[2] != KEY_RESERVED)
> + __set_bit(pdata->b_on_key[2], pwr->keybit);
>
> - if (pdata->b3on_flags & MC13783_BUTTON_POL_INVERT)
> + if (pdata->b_on_flags[2] & MC13783_BUTTON_POL_INVERT)
> priv->flags |= MC13783_PWRB_B3_POL_INVERT;
>
> - if (pdata->b3on_flags & MC13783_BUTTON_RESET_EN)
> + if (pdata->b_on_flags[2] & MC13783_BUTTON_RESET_EN)
> reg |= MC13783_POWER_CONTROL_2_ON3BRSTEN;
>
> irq = platform_get_irq_byname(pdev, "b3on");
> diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
> index dd46fe424a80..4437ab80fcf8 100644
> --- a/include/linux/mfd/mc13xxx.h
> +++ b/include/linux/mfd/mc13xxx.h
> @@ -181,12 +181,8 @@ struct mc13xxx_leds_platform_data {
> #define MC13783_BUTTON_RESET_EN (1 << 4)
>
> struct mc13xxx_buttons_platform_data {
> - int b1on_flags;
> - unsigned short b1on_key;
> - int b2on_flags;
> - unsigned short b2on_key;
> - int b3on_flags;
> - unsigned short b3on_key;
> + int b_on_flags[3];
> + unsigned int b_on_key[3];
> };
>
> #define MC13783_TS_ATO_FIRST false
> --
> 2.39.5
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v7 5/6] Input: mc13783-pwrbutton: add OF support and drop platform_data
From: Lee Jones @ 2025-11-13 14:30 UTC (permalink / raw)
To: Alexander Kurz
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Dmitry Torokhov, Dzmitry Sankouski, Griffin Kroah-Hartman,
Mathieu Dubois-Briand, Heiko Stuebner, Dr . David Alan Gilbert,
Job Sava, Alexandre Belloni, Uwe Kleine-König, devicetree,
imx, linux-arm-kernel, linux-kernel, linux-input
In-Reply-To: <20251031195718.1586-6-akurz@blala.de>
On Fri, 31 Oct 2025, Alexander Kurz wrote:
> Add OF support for the mc13783-pwrbutton so that it can be used with
> modern DT based systems, dropping support for platform_data.
>
> Signed-off-by: Alexander Kurz <akurz@blala.de>
> ---
> drivers/input/misc/mc13783-pwrbutton.c | 104 +++++++++++++++++++++----
> drivers/mfd/mc13xxx-core.c | 4 -
> include/linux/mfd/mc13xxx.h | 14 ----
Acked-by: Lee Jones <lee@kernel.org>
> 3 files changed, 88 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/input/misc/mc13783-pwrbutton.c b/drivers/input/misc/mc13783-pwrbutton.c
> index 08618c59197f..0fa630adff19 100644
> --- a/drivers/input/misc/mc13783-pwrbutton.c
> +++ b/drivers/input/misc/mc13783-pwrbutton.c
> @@ -27,6 +27,7 @@
> #include <linux/interrupt.h>
> #include <linux/platform_device.h>
> #include <linux/mfd/mc13783.h>
> +#include <linux/property.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
>
> @@ -41,10 +42,20 @@ struct mc13783_pwrb {
> struct mc13xxx *mc13783;
> const struct mc13xxx_button_devtype *devtype;
> int flags;
> + int b_on_flags[3];
> + unsigned int b_on_key[3];
> unsigned short keymap[3];
> int irq[3];
> };
>
> +#define MC13783_BUTTON_DBNC_0MS 0
> +#define MC13783_BUTTON_DBNC_30MS 1
> +#define MC13783_BUTTON_DBNC_150MS 2
> +#define MC13783_BUTTON_DBNC_750MS 3
> +#define MC13783_BUTTON_ENABLE (1 << 2)
> +#define MC13783_BUTTON_POL_INVERT (1 << 3)
> +#define MC13783_BUTTON_RESET_EN (1 << 4)
> +
> #define MC13783_PWRB_B1_POL_INVERT (1 << 0)
> #define MC13783_PWRB_B2_POL_INVERT (1 << 1)
> #define MC13783_PWRB_B3_POL_INVERT (1 << 2)
> @@ -88,9 +99,69 @@ static irqreturn_t button_irq(int irq, void *_priv)
> return IRQ_HANDLED;
> }
>
> +static int mc13xxx_pwrbutton_parse_properties(struct platform_device *pdev,
> + struct mc13783_pwrb *priv)
> +{
> + struct fwnode_handle *child;
> + struct device *dev = &pdev->dev;
> + struct mc13xxx_button_devtype *devtype =
> + (struct mc13xxx_button_devtype *)platform_get_device_id(pdev)->driver_data;
> +
> + struct fwnode_handle *parent __free(fwnode_handle) =
> + device_get_named_child_node(dev->parent, "buttons");
> + if (!parent)
> + return -ENODATA;
> +
> + fwnode_for_each_named_child_node(parent, child, "onkey") {
> + u32 idx;
> + u8 dbnc = MC13783_BUTTON_DBNC_30MS;
> + u16 dbnc_ms;
> +
> + if (fwnode_property_read_u32(child, "reg", &idx))
> + continue;
> +
> + if (idx > devtype->button_id_max) {
> + dev_warn(dev, "reg out of range\n");
> + continue;
> + }
> +
> + fwnode_property_read_u16(child, "debounce-delay-ms", &dbnc_ms);
> + switch (dbnc_ms) {
> + case 0:
> + dbnc = MC13783_BUTTON_DBNC_0MS;
> + break;
> + case 30:
> + dbnc = MC13783_BUTTON_DBNC_30MS;
> + break;
> + case 150:
> + dbnc = MC13783_BUTTON_DBNC_150MS;
> + break;
> + case 750:
> + dbnc = MC13783_BUTTON_DBNC_750MS;
> + break;
> + default:
> + dev_warn(dev, "invalid debounce-delay-ms value\n");
> + continue;
> + }
> +
> + if (fwnode_property_read_u32(child, "linux,code", &priv->b_on_key[idx]))
> + continue;
> +
> + if (fwnode_property_read_bool(child, "active-low"))
> + priv->b_on_flags[idx] |= MC13783_BUTTON_POL_INVERT;
> +
> + if (fwnode_property_read_bool(child, "fsl,enable-reset"))
> + priv->b_on_flags[idx] |= MC13783_BUTTON_RESET_EN;
> +
> + priv->b_on_flags[idx] |= MC13783_BUTTON_ENABLE | dbnc;
> + }
> +
> + return 0;
> +}
> +
> static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> {
> - const struct mc13xxx_buttons_platform_data *pdata;
> + struct device *dev = &pdev->dev;
> struct mc13xxx *mc13783 = dev_get_drvdata(pdev->dev.parent);
> struct mc13xxx_button_devtype *devtype =
> (struct mc13xxx_button_devtype *)pdev->id_entry->driver_data;
> @@ -100,11 +171,8 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> int reg = 0;
> int irq = 0;
>
> - pdata = dev_get_platdata(&pdev->dev);
> - if (!pdata) {
> - dev_err(&pdev->dev, "missing platform data\n");
> - return -ENODEV;
> - }
> + if (!dev->parent->of_node)
> + return -ENODATA;
>
> pwr = devm_input_allocate_device(&pdev->dev);
> if (!pwr)
> @@ -114,14 +182,18 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> - if (devtype->button_id_max < 2 && pdata->b_on_flags[2] & 0x3) {
> + err = mc13xxx_pwrbutton_parse_properties(pdev, priv);
> + if (err)
> + return err;
> +
> + if (devtype->button_id_max < 2 && priv->b_on_flags[2] & 0x3) {
> dev_err(&pdev->dev, "button not supported\n");
> return -ENODEV;
> }
>
> - reg |= (pdata->b_on_flags[0] & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
> - reg |= (pdata->b_on_flags[1] & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
> - reg |= (pdata->b_on_flags[2] & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
> + reg |= (priv->b_on_flags[0] & 0x3) << MC13783_POWER_CONTROL_2_ON1BDBNC;
> + reg |= (priv->b_on_flags[1] & 0x3) << MC13783_POWER_CONTROL_2_ON2BDBNC;
> + reg |= (priv->b_on_flags[2] & 0x3) << MC13783_POWER_CONTROL_2_ON3BDBNC;
>
> priv->pwr = pwr;
> priv->mc13783 = mc13783;
> @@ -130,17 +202,17 @@ static int mc13783_pwrbutton_probe(struct platform_device *pdev)
> mc13xxx_lock(mc13783);
>
> for (int i = 0; i < devtype->button_id_max; i++) {
> - if ((pdata->b_on_flags[i] & MC13783_BUTTON_ENABLE) == 0)
> + if ((priv->b_on_flags[i] & MC13783_BUTTON_ENABLE) == 0)
> continue;
>
> - priv->keymap[i] = pdata->b_on_key[i];
> - if (pdata->b_on_key[i] != KEY_RESERVED)
> - __set_bit(pdata->b_on_key[i], pwr->keybit);
> + priv->keymap[i] = priv->b_on_key[i];
> + if (priv->b_on_key[i] != KEY_RESERVED)
> + __set_bit(priv->b_on_key[i], pwr->keybit);
>
> - if (pdata->b_on_flags[i] & MC13783_BUTTON_POL_INVERT)
> + if (priv->b_on_flags[i] & MC13783_BUTTON_POL_INVERT)
> priv->flags |= (MC13783_PWRB_B1_POL_INVERT << i);
>
> - if (pdata->b_on_flags[i] & MC13783_BUTTON_RESET_EN)
> + if (priv->b_on_flags[i] & MC13783_BUTTON_RESET_EN)
> reg |= (MC13783_POWER_CONTROL_2_ON1BRSTEN << i);
>
> irq = platform_get_irq_byname(pdev, devtype->irq_name[i]);
> diff --git a/drivers/mfd/mc13xxx-core.c b/drivers/mfd/mc13xxx-core.c
> index c29974722704..9512136e821b 100644
> --- a/drivers/mfd/mc13xxx-core.c
> +++ b/drivers/mfd/mc13xxx-core.c
> @@ -504,10 +504,6 @@ int mc13xxx_common_init(struct device *dev)
> &pdata->regulators, sizeof(pdata->regulators));
> mc13xxx_add_subdevice_pdata(mc13xxx, "%s-led",
> pdata->leds, sizeof(*pdata->leds));
> - mc13xxx_add_subdevice_pdata_res(mc13xxx, "%s-pwrbutton",
> - pdata->buttons, sizeof(*pdata->buttons),
> - mc13xxx->variant->button_resources,
> - mc13xxx->variant->button_resources_size);
> if (mc13xxx->flags & MC13XXX_USE_CODEC)
> mc13xxx_add_subdevice_pdata(mc13xxx, "%s-codec",
> pdata->codec, sizeof(*pdata->codec));
> diff --git a/include/linux/mfd/mc13xxx.h b/include/linux/mfd/mc13xxx.h
> index 71c7d3614d4c..ac3765df341d 100644
> --- a/include/linux/mfd/mc13xxx.h
> +++ b/include/linux/mfd/mc13xxx.h
> @@ -174,19 +174,6 @@ struct mc13xxx_leds_platform_data {
> u32 led_control[MAX_LED_CONTROL_REGS];
> };
>
> -#define MC13783_BUTTON_DBNC_0MS 0
> -#define MC13783_BUTTON_DBNC_30MS 1
> -#define MC13783_BUTTON_DBNC_150MS 2
> -#define MC13783_BUTTON_DBNC_750MS 3
> -#define MC13783_BUTTON_ENABLE (1 << 2)
> -#define MC13783_BUTTON_POL_INVERT (1 << 3)
> -#define MC13783_BUTTON_RESET_EN (1 << 4)
> -
> -struct mc13xxx_buttons_platform_data {
> - int b_on_flags[3];
> - unsigned int b_on_key[3];
> -};
> -
> #define MC13783_TS_ATO_FIRST false
> #define MC13783_TS_ATO_EACH true
>
> @@ -219,7 +206,6 @@ struct mc13xxx_platform_data {
>
> struct mc13xxx_regulator_platform_data regulators;
> struct mc13xxx_leds_platform_data *leds;
> - struct mc13xxx_buttons_platform_data *buttons;
> struct mc13xxx_ts_platform_data touch;
> struct mc13xxx_codec_platform_data *codec;
> };
> --
> 2.39.5
>
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v2] Apply the quirk HID_QUIRK_ALWAYS_POLL to the Edifier QR30 (2d99:a101).
From: The-Luga @ 2025-11-13 15:45 UTC (permalink / raw)
To: Alan Stern
Cc: Terry Junge, michal.pecio, bentiss, dmitry.torokhov, jikos,
linux-input, linux-sound, linux-usb, linuxsound
In-Reply-To: <58edd03a-a7a7-40af-8228-18004dc6e737@rowland.harvard.edu>
> You might want to wait a couple of days before sending a v3 so other
> reviewers can get a chance to make comments.
Yes, I shall wait. I was just *too eager*.
> You should include the HID: tag in the subject--
> ...] HID: Apply...
>
> The patch should be sent
>
> To: the HID subsystem maintainers
> bentiss@kernel.org
> jikos@kernel.org
>
> Cc: at a minimum
> linux-input@vger.kernel.org
> linux-kernel@vger.kernel.org
>
> plus any of the others already part of this thread.
This is very helpful. Thank you!
I read the documentation, but I was really waiting for comments before
finally sending it, as I didn’t want to create noise with all the
iterations of the patch. I didn’t realize I should wait before making
a bigger final change.
> Vendor ID 0x2d99 belongs to Edifier International Limited not Jieli
> Can you change to USB_VENDOR_ID_EDIFIER instead and move to the
> alphabetically correct location?
Could you kindly tell me where this info is?
Searching the internet I only found this website with this info:
https://the-sz.com/products/usbid/index.php?v=0x2D99
I just checked the vendor from `lsusb`, and I was hesitant whether to
write Jieli or Edifier.
I also decided to write QR30 instead of Hal0. Should I add a comment
mentioning Jieli as I did with the device?
> One more thing about patch style: The Signed-off-by: line should go
> above the --- line, whereas the revision information goes below it (as
> it is here). The second --- line isn't needed (but it doesn't hurt).
>
> Look at other patch submissions in the email archives to see more
> examples of the style.
>
> Alan Stern
This is also very helpful. I should have done it in the first place. Thank you!
> Thanks for the patch!
> Regards,
> Terry Junge
It's me who is *super thankful*. I learned how to do packet capture,
hexcode/table analysis, and even wrote a script to time out the RGB
lights when idle, show a simple brightness animation (increasing and
decreasing steps before coming back to the default) when changing
volume, and synchronize it with my keyboard. I have more functionality
here than with the vendor software on those supported OS's. All thanks
to everyone who helped me debug and showed me the tools needed to do
this.
And even invited me to write my first patch. I believe this process
should be repetitive, with new people coming and going. I’m grateful
for the support and patience.
^ permalink raw reply
* [PATCH v2 0/5] Input: gpio_decoder - update driver to use modern APIs
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
Update gpio_decoder driver to use modern in-kernel APIs.
Changelog v2:
- fixed typo (LKP), dunno what I have compiled before
v1: 20251112191412.2088105-1-andriy.shevchenko@linux.intel.com
Andy Shevchenko (5):
Input: gpio_decoder - make use of device properties
Input: gpio_decoder - unify messages with help of dev_err_probe()
Input: gpio_decoder - replace custom loop by
gpiod_get_array_value_cansleep()
Input: gpio_decoder - make use of the macros from bits.h
Input: gpio_decoder - don't use "proxy" headers
drivers/input/misc/gpio_decoder.c | 72 ++++++++++++++-----------------
1 file changed, 33 insertions(+), 39 deletions(-)
--
2.50.1
^ permalink raw reply
* [PATCH v2 1/5] Input: gpio_decoder - make use of device properties
From: Andy Shevchenko @ 2025-11-13 15:44 UTC (permalink / raw)
To: Andy Shevchenko, linux-input, linux-kernel; +Cc: Dmitry Torokhov
In-Reply-To: <20251113154616.3107676-1-andriy.shevchenko@linux.intel.com>
Convert the module to be property provider agnostic and allow
it to be used on non-OF platforms.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
drivers/input/misc/gpio_decoder.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/input/misc/gpio_decoder.c b/drivers/input/misc/gpio_decoder.c
index ee668eba302f..459abc749a49 100644
--- a/drivers/input/misc/gpio_decoder.c
+++ b/drivers/input/misc/gpio_decoder.c
@@ -10,9 +10,10 @@
#include <linux/gpio/consumer.h>
#include <linux/input.h>
#include <linux/kernel.h>
+#include <linux/mod_devicetable.h>
#include <linux/module.h>
-#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/property.h>
struct gpio_decoder {
struct gpio_descs *input_gpios;
@@ -110,19 +111,17 @@ static int gpio_decoder_probe(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_OF
static const struct of_device_id gpio_decoder_of_match[] = {
{ .compatible = "gpio-decoder", },
- { },
+ { }
};
MODULE_DEVICE_TABLE(of, gpio_decoder_of_match);
-#endif
static struct platform_driver gpio_decoder_driver = {
.probe = gpio_decoder_probe,
.driver = {
.name = "gpio-decoder",
- .of_match_table = of_match_ptr(gpio_decoder_of_match),
+ .of_match_table = gpio_decoder_of_match,
}
};
module_platform_driver(gpio_decoder_driver);
--
2.50.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox