* Re: [git pull] Input updates for v6.8-rc7
From: pr-tracker-bot @ 2024-03-08 22:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <Zeqhkk0HJwEIUZ0D@google.com>
The pull request you sent on Thu, 7 Mar 2024 21:40:31 -0800:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.8-rc7
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7a4f31c7765e63e3219eac1e822ab16df09f318c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: input: atmel,captouch: convert bindings to YAML
From: Dmitry Torokhov @ 2024-03-09 0:34 UTC (permalink / raw)
To: Dharma Balasubiramani
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, nicolas.ferre,
alexandre.belloni, claudiu.beznea, linux-input, devicetree,
linux-arm-kernel, linux-kernel, hari.prasathge
In-Reply-To: <20240209064755.47516-1-dharma.b@microchip.com>
On Fri, Feb 09, 2024 at 12:17:55PM +0530, Dharma Balasubiramani wrote:
> Convert the Atmel capacitive touchscreen bindings to YAML format.
>
> Signed-off-by: Dharma Balasubiramani <dharma.b@microchip.com>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH 3/3] HID: surface-hid: kbd: Convert to platform remove callback returning void
From: Maximilian Luz @ 2024-03-09 0:53 UTC (permalink / raw)
To: Uwe Kleine-König, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, platform-driver-x86, kernel
In-Reply-To: <05d0d6ef781ebd6124a36f70cda1f90331799dc7.1709747164.git.u.kleine-koenig@pengutronix.de>
On 3/6/24 18:50, Uwe Kleine-König wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is ignored (apart
> from emitting a warning) and this typically results in resource leaks.
>
> To improve here there is a quest to make the remove callback return
> void. In the first step of this quest all drivers are converted to
> .remove_new(), which already returns void. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
>
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Looks good to me.
Reviewed-by: Maximilian Luz <luzmaximilian@gmail.com>
> ---
> drivers/hid/surface-hid/surface_kbd.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/surface-hid/surface_kbd.c b/drivers/hid/surface-hid/surface_kbd.c
> index 4fbce201db6a..8c0cbb2deb11 100644
> --- a/drivers/hid/surface-hid/surface_kbd.c
> +++ b/drivers/hid/surface-hid/surface_kbd.c
> @@ -271,10 +271,9 @@ static int surface_kbd_probe(struct platform_device *pdev)
> return surface_hid_device_add(shid);
> }
>
> -static int surface_kbd_remove(struct platform_device *pdev)
> +static void surface_kbd_remove(struct platform_device *pdev)
> {
> surface_hid_device_destroy(platform_get_drvdata(pdev));
> - return 0;
> }
>
> static const struct acpi_device_id surface_kbd_match[] = {
> @@ -285,7 +284,7 @@ MODULE_DEVICE_TABLE(acpi, surface_kbd_match);
>
> static struct platform_driver surface_kbd_driver = {
> .probe = surface_kbd_probe,
> - .remove = surface_kbd_remove,
> + .remove_new = surface_kbd_remove,
> .driver = {
> .name = "surface_keyboard",
> .acpi_match_table = surface_kbd_match,
^ permalink raw reply
* Re: [PATCH 0/2] HID: samsung: Object code size reduction and neatening
From: Joe Perches @ 2024-03-09 7:02 UTC (permalink / raw)
To: linux-input
Cc: Sandeep C S, Junwan Cho, Jitender Sajwan, Jiri Kosina,
Benjamin Tissoires, linux-kernel
In-Reply-To: <deebac6b6d06a6cd49ac09606a07576b84e35894.camel@perches.com>
On Mon, 2024-02-12 at 10:09 -0800, Joe Perches wrote:
> On Wed, 2024-01-31 at 12:21 -0800, Joe Perches wrote:
> > Reduce object size and neatening
> >
> > Joe Perches (2):
> > HID: samsung: Reduce code size
> > HID: samsung: Object size reduction and neatening
> >
> > drivers/hid/hid-samsung.c | 652 +++++++++++++++++---------------------
> > 1 file changed, 283 insertions(+), 369 deletions(-)
>
> Any comment from the samsung folk on these?
Ping?
^ permalink raw reply
* [dtor-input:next] BUILD SUCCESS 1099a04ccc9b4d2b9d5dcd33d9bb9c333e147e8d
From: kernel test robot @ 2024-03-09 13:43 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 1099a04ccc9b4d2b9d5dcd33d9bb9c333e147e8d dt-bindings: input: atmel,captouch: convert bindings to YAML
elapsed time: 725m
configs tested: 155
configs skipped: 2
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc defconfig gcc
arc randconfig-001-20240309 gcc
arc randconfig-002-20240309 gcc
arm allmodconfig gcc
arm allnoconfig clang
arm allyesconfig gcc
arm clps711x_defconfig clang
arm defconfig clang
arm milbeaut_m10v_defconfig clang
arm mps2_defconfig clang
arm netwinder_defconfig gcc
arm randconfig-002-20240309 gcc
arm randconfig-004-20240309 gcc
arm spear6xx_defconfig clang
arm sunxi_defconfig gcc
arm u8500_defconfig gcc
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 allyesconfig clang
arm64 defconfig gcc
arm64 randconfig-001-20240309 gcc
csky alldefconfig gcc
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240309 gcc
csky randconfig-002-20240309 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20240309 clang
i386 buildonly-randconfig-002-20240309 clang
i386 buildonly-randconfig-003-20240309 gcc
i386 buildonly-randconfig-004-20240309 gcc
i386 buildonly-randconfig-005-20240309 gcc
i386 buildonly-randconfig-006-20240309 gcc
i386 defconfig clang
i386 randconfig-001-20240309 gcc
i386 randconfig-002-20240309 gcc
i386 randconfig-003-20240309 clang
i386 randconfig-004-20240309 clang
i386 randconfig-005-20240309 clang
i386 randconfig-006-20240309 gcc
i386 randconfig-011-20240309 gcc
i386 randconfig-012-20240309 gcc
i386 randconfig-013-20240309 clang
i386 randconfig-014-20240309 clang
i386 randconfig-015-20240309 clang
i386 randconfig-016-20240309 clang
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240309 gcc
loongarch randconfig-002-20240309 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips ip28_defconfig gcc
mips malta_defconfig gcc
mips maltaaprp_defconfig clang
mips rb532_defconfig clang
nios2 10m50_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240309 gcc
nios2 randconfig-002-20240309 gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc randconfig-001-20240309 gcc
parisc randconfig-002-20240309 gcc
parisc64 defconfig gcc
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc makalu_defconfig clang
powerpc mpc866_ads_defconfig clang
powerpc mvme5100_defconfig gcc
powerpc ppc40x_defconfig clang
powerpc randconfig-001-20240309 gcc
powerpc64 randconfig-001-20240309 gcc
powerpc64 randconfig-002-20240309 gcc
riscv allmodconfig clang
riscv allnoconfig gcc
riscv allyesconfig clang
riscv defconfig clang
riscv randconfig-001-20240309 gcc
riscv randconfig-002-20240309 gcc
riscv rv32_defconfig clang
s390 allmodconfig clang
s390 allnoconfig clang
s390 allyesconfig gcc
s390 defconfig clang
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh defconfig gcc
sh randconfig-001-20240309 gcc
sh randconfig-002-20240309 gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240309 gcc
sparc64 randconfig-002-20240309 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig gcc
um defconfig clang
um i386_defconfig gcc
um x86_64_defconfig clang
x86_64 allnoconfig clang
x86_64 allyesconfig clang
x86_64 buildonly-randconfig-001-20240309 clang
x86_64 buildonly-randconfig-003-20240309 clang
x86_64 buildonly-randconfig-006-20240309 clang
x86_64 defconfig gcc
x86_64 randconfig-002-20240309 clang
x86_64 randconfig-003-20240309 clang
x86_64 randconfig-005-20240309 clang
x86_64 randconfig-011-20240309 clang
x86_64 randconfig-012-20240309 clang
x86_64 randconfig-013-20240309 clang
x86_64 randconfig-076-20240309 clang
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa randconfig-001-20240309 gcc
xtensa randconfig-002-20240309 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [syzbot] [mm?] [input?] [usb?] INFO: rcu detected stall in asm_exc_page_fault
From: syzbot @ 2024-03-09 13:56 UTC (permalink / raw)
To: akpm, linux-input, linux-kernel, linux-mm, linux-usb,
pasha.tatashin, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: 90d35da658da Linux 6.8-rc7
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=122f6f6a180000
kernel config: https://syzkaller.appspot.com/x/.config?x=119d08814b43915b
dashboard link: https://syzkaller.appspot.com/bug?extid=360faf5c01a5be55581d
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=124056de180000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/fb2c1adf4ec3/disk-90d35da6.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/09c5b88a8ceb/vmlinux-90d35da6.xz
kernel image: https://storage.googleapis.com/syzbot-assets/5e5cbc312e49/bzImage-90d35da6.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+360faf5c01a5be55581d@syzkaller.appspotmail.com
rcu: INFO: rcu_preempt detected expedited stalls on CPUs/tasks: {
1-....
} 4831 jiffies s: 1849 root: 0x2/.
rcu: blocking rcu_node structures (internal RCU debug):
Sending NMI from CPU 0 to CPUs 1:
kthread+0x2ef/0x390 kernel/kthread.c:388
NMI backtrace for cpu 1
CPU: 1 PID: 5232 Comm: syz-executor.3 Not tainted 6.8.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
RIP: 0010:format_decode+0x546/0x1bb0
Code: 85 96 01 00 00 45 84 ff 0f 84 8d 01 00 00 48 bb 00 ff ff ff 00 ff ff ff 48 8b 44 24 20 42 0f b6 04 30 84 c0 0f 85 4d 10 00 00 <48> 8b 54 24 48 48 21 da 48 8b 44 24 28 42 0f b6 04 30 84 c0 48 8d
RSP: 0000:ffffc900001efa20 EFLAGS: 00000046
RAX: 0000000000000000 RBX: ffffff00ffffff00 RCX: ffff8880219e0000
RDX: ffff8880219e0000 RSI: 0000000000000025 RDI: 0000000000000000
RBP: ffffc900001efb10 R08: ffffffff8b57a4c8 R09: ffffffff8b57a1aa
R10: 0000000000000002 R11: ffff8880219e0000 R12: ffffffff8bab75e6
R13: ffffffff8bab75e6 R14: dffffc0000000000 R15: 0000000000000025
FS: 0000555555c82480(0000) GS:ffff8880b9500000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f74dc087056 CR3: 0000000021bc6000 CR4: 00000000003506f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
<NMI>
</NMI>
<IRQ>
vsnprintf+0x14f/0x1da0 lib/vsprintf.c:2776
sprintf+0xda/0x120 lib/vsprintf.c:3028
print_time kernel/printk/printk.c:1324 [inline]
info_print_prefix+0x16b/0x310 kernel/printk/printk.c:1350
record_print_text kernel/printk/printk.c:1399 [inline]
printk_get_next_message+0x408/0xce0 kernel/printk/printk.c:2828
console_emit_next_record kernel/printk/printk.c:2868 [inline]
console_flush_all+0x42d/0xec0 kernel/printk/printk.c:2967
console_unlock+0x13b/0x4d0 kernel/printk/printk.c:3036
vprintk_emit+0x508/0x720 kernel/printk/printk.c:2303
_printk+0xd5/0x120 kernel/printk/printk.c:2328
printk_stack_address arch/x86/kernel/dumpstack.c:72 [inline]
show_trace_log_lvl+0x438/0x520 arch/x86/kernel/dumpstack.c:285
sched_show_task+0x50c/0x6d0 kernel/sched/core.c:9171
show_state_filter+0x19e/0x270 kernel/sched/core.c:9216
kbd_keycode drivers/tty/vt/keyboard.c:1524 [inline]
kbd_event+0x30fa/0x4910 drivers/tty/vt/keyboard.c:1543
input_to_handler drivers/input/input.c:132 [inline]
input_pass_values+0x945/0x1200 drivers/input/input.c:161
input_event_dispose drivers/input/input.c:378 [inline]
input_handle_event drivers/input/input.c:406 [inline]
input_repeat_key+0x3fd/0x6c0 drivers/input/input.c:2263
call_timer_fn+0x17e/0x600 kernel/time/timer.c:1700
expire_timers kernel/time/timer.c:1751 [inline]
__run_timers+0x621/0x830 kernel/time/timer.c:2038
run_timer_softirq+0x67/0xf0 kernel/time/timer.c:2051
__do_softirq+0x2bb/0x942 kernel/softirq.c:553
invoke_softirq kernel/softirq.c:427 [inline]
__irq_exit_rcu+0xf1/0x1c0 kernel/softirq.c:632
irq_exit_rcu+0x9/0x30 kernel/softirq.c:644
sysvec_apic_timer_interrupt+0x97/0xb0 arch/x86/kernel/apic/apic.c:1076
</IRQ>
<TASK>
asm_sysvec_apic_timer_interrupt+0x1a/0x20 arch/x86/include/asm/idtentry.h:649
RIP: 0010:page_table_check_set+0x58/0x700 mm/page_table_check.c:109
Code: 95 ff 85 ed 0f 84 5f 03 00 00 49 bf 00 00 00 00 00 fc ff df 48 c1 e3 06 48 bd 00 00 00 00 00 ea ff ff 48 8d 3c 2b 48 89 3c 24 <e8> 33 e9 ff ff 49 89 c6 4c 8d 64 2b 08 4c 89 e5 48 c1 ed 03 42 80
RSP: 0000:ffffc90004d0f650 EFLAGS: 00000202
RAX: 0000000000000000 RBX: 0000000001c8ae80 RCX: ffff8880219e0000
RDX: ffff8880219e0000 RSI: 0000000000000001 RDI: ffffea0001c8ae80
RBP: ffffea0000000000 R08: ffffffff81fdf590 R09: 1ffffffff1f0880d
R10: dffffc0000000000 R11: fffffbfff1f0880e R12: 0000000000000000
R13: 0000000000000001 R14: 00000000722ba025 R15: dffffc0000000000
__page_table_check_ptes_set+0x220/0x280 mm/page_table_check.c:196
page_table_check_ptes_set include/linux/page_table_check.h:74 [inline]
set_ptes include/linux/pgtable.h:241 [inline]
set_pte_range+0x885/0x8b0 mm/memory.c:4549
filemap_map_order0_folio mm/filemap.c:3513 [inline]
filemap_map_pages+0xee2/0x1830 mm/filemap.c:3559
do_fault_around mm/memory.c:4716 [inline]
do_read_fault mm/memory.c:4749 [inline]
do_fault mm/memory.c:4888 [inline]
do_pte_missing mm/memory.c:3745 [inline]
handle_pte_fault mm/memory.c:5164 [inline]
__handle_mm_fault+0x485d/0x72d0 mm/memory.c:5305
handle_mm_fault+0x27e/0x770 mm/memory.c:5470
do_user_addr_fault arch/x86/mm/fault.c:1355 [inline]
handle_page_fault arch/x86/mm/fault.c:1498 [inline]
exc_page_fault+0x456/0x870 arch/x86/mm/fault.c:1554
asm_exc_page_fault+0x26/0x30 arch/x86/include/asm/idtentry.h:570
RIP: 0033:0x7f74dc087080
Code: Unable to access opcode bytes at 0x7f74dc087056.
RSP: 002b:00007ffe028d3bb8 EFLAGS: 00010246
RAX: 00007f74dcdfb9d0 RBX: 00007f74dcdfb6c0 RCX: 00007f74dc07de67
RDX: 0000000000000003 RSI: 0000000000020000 RDI: 00007f74dcdfb6c0
RBP: 0000000000000000 R08: 00000000ffffffff R09: 0000000000000000
R10: 0000000000021000 R11: 0000000000000206 R12: 00007ffe028d3e60
R13: ffffffffffffffc0 R14: 0000000000001000 R15: 0000000000000000
</TASK>
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
</TASK>
task:kworker/u4:0 state:I stack:24400 pid:11 tgid:11 ppid:2 flags:0x00004000
Workqueue: 0x0 (events_unbound)
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5400 [inline]
__schedule+0x177f/0x49a0 kernel/sched/core.c:6727
__schedule_loop kernel/sched/core.c:6802 [inline]
schedule+0x149/0x260 kernel/sched/core.c:6817
worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
kthread+0x2ef/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
</TASK>
task:kworker/u4:1 state:I stack:23344 pid:12 tgid:12 ppid:2 flags:0x00004000
Workqueue: 0x0 (bat_events)
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5400 [inline]
__schedule+0x177f/0x49a0 kernel/sched/core.c:6727
__schedule_loop kernel/sched/core.c:6802 [inline]
schedule+0x149/0x260 kernel/sched/core.c:6817
worker_thread+0xc26/0x1000 kernel/workqueue.c:2802
kthread+0x2ef/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
</TASK>
task:kworker/R-mm_pe state:I stack:28752 pid:13 tgid:13 ppid:2 flags:0x00004000
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5400 [inline]
__schedule+0x177f/0x49a0 kernel/sched/core.c:6727
__schedule_loop kernel/sched/core.c:6802 [inline]
schedule+0x149/0x260 kernel/sched/core.c:6817
rescuer_thread+0xc45/0xda0 kernel/workqueue.c:2937
kthread+0x2ef/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
</TASK>
task:rcu_tasks_kthre state:I stack:27448 pid:14 tgid:14 ppid:2 flags:0x00004000
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5400 [inline]
__schedule+0x177f/0x49a0 kernel/sched/core.c:6727
__schedule_loop kernel/sched/core.c:6802 [inline]
schedule+0x149/0x260 kernel/sched/core.c:6817
rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
kthread+0x2ef/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
</TASK>
task:rcu_tasks_trace state:I stack:27144 pid:15 tgid:15 ppid:2 flags:0x00004000
Call Trace:
<TASK>
context_switch kernel/sched/core.c:5400 [inline]
__schedule+0x177f/0x49a0 kernel/sched/core.c:6727
__schedule_loop kernel/sched/core.c:6802 [inline]
schedule+0x149/0x260 kernel/sched/core.c:6817
rcu_tasks_one_gp+0x7f5/0xda0 kernel/rcu/tasks.h:578
rcu_tasks_kthread+0x186/0x1b0 kernel/rcu/tasks.h:625
kthread+0x2ef/0x390 kernel/kthread.c:388
ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:147
ret_from_fork_asm+0x1b/0x30 arch/x86/entry/entry_64.S:243
</TASK>
task:ksoftirqd/0
---
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 2/3] HID: hid-sensor-custom: Convert to platform remove callback returning void
From: Jonathan Cameron @ 2024-03-09 18:40 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Jiri Kosina, Benjamin Tissoires, Srinivas Pandruvada, linux-input,
linux-iio, kernel
In-Reply-To: <f4c8334ea1548d911862ede881ab0d90a408c156.1709747164.git.u.kleine-koenig@pengutronix.de>
On Wed, 6 Mar 2024 18:50:49 +0100
Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote:
> The .remove() callback for a platform driver returns an int which makes
> many driver authors wrongly assume it's possible to do error handling by
> returning an error code. However the value returned is ignored (apart
> from emitting a warning) and this typically results in resource leaks.
>
> To improve here there is a quest to make the remove callback return
> void. In the first step of this quest all drivers are converted to
> .remove_new(), which already returns void. Eventually after all drivers
> are converted, .remove_new() will be renamed to .remove().
>
> Trivially convert this driver from always returning zero in the remove
> callback to the void returning variant.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
^ permalink raw reply
* Re: [PATCH 0/2] map Fn + R key on newer Yogas and Legions
From: Dmitry Torokhov @ 2024-03-10 1:39 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Ike Panhc, Hans de Goede, Philipp Jungkamp, Gergo Koteles,
platform-driver-x86, linux-kernel, linux-input
In-Reply-To: <170895405312.2243.4199399921923370447.b4-ty@linux.intel.com>
On Mon, Feb 26, 2024 at 03:27:33PM +0200, Ilpo Järvinen wrote:
> On Tue, 20 Feb 2024 04:39:34 +0100, Gergo Koteles wrote:
>
> > This patch series adds a new KEY_FN_R input event code and map the
> > Fn + R key to it in the ideapad-laptop driver.
> >
> > It affects two WMI keycodes and I couldn't try the 0x0a, but I couldn't
> > find any indication that the refresh rate toggle should not be Fn + R.
> >
> > Regards,
> > Gergo
> >
> > [...]
>
>
> Thank you for your contribution, it has been applied to my local
> review-ilpo branch. Note it will show up in the public
> platform-drivers-x86/review-ilpo branch only once I've pushed my
> local branch there, which might take a while.
>
> The list of commits applied:
> [1/2] Input: allocate keycode for Fn + R
> commit: 4e45fa464aeef4e803412b5dcce73aad48c94b0e
I am sorry for the delay, but instead of defining a generic name we should define
a proper keycode for concrete action even if nothing is printed on a
particular key on a particular device.
Please drop this patch.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v5] platform/x86: add lenovo wmi camera button driver
From: Dmitry Torokhov @ 2024-03-10 4:52 UTC (permalink / raw)
To: Armin Wolf
Cc: 艾超, hdegoede, ilpo.jarvinen, linux-kernel,
platform-driver-x86, linux-input
In-Reply-To: <c07dedf3-175c-4748-b6d3-e1effe9cad41@gmx.de>
On Fri, Mar 08, 2024 at 05:15:48PM +0100, Armin Wolf wrote:
> Am 08.03.24 um 09:40 schrieb 艾超:
>
> > Hi
> >
> > thanks for your help.
> >
> > > Is there a way to determine the current state of the camera switch
> > without having
> > > to wait for an WMI event?
> >
> > There is no other way to detemine the current state of the camera
> > switch without
> >
> > a WMI event.
> >
> >
> > > If its not possible to determine the current state of the camera
> > switch, then your
> > > driver has to defer the initialization of the input device until it
> > knows the current
> > > state of the camera switch. Otherwise the initial switch state
> > reported by the
> > > input device might be wrong.
> >
> > > In this case, that means your driver must initialize the input
> > device when receiving
> > > a valid WMI event for the first time.
> >
> > > Basically when your driver receives a WMI event, it has to check if
> > the input device
> > > is already initialized. If this is not the case, then the input
> > device is initialized.
> > > Please protect this check and the input device initialization with a
> > mutex, since WMI
> > > event handling is multithreaded.
> >
> >
> > This Camera is a UVC device, and the Carema device is already
> > initialized before the WMI
> >
> > event . Whether the camera switch is on or off, we can used lsusb to
> > check the Camera.
It depends on which drivers are built-in and which are modules. The fact
that on one distribution WMI happens to enumerate after PCI/USB is just
a stroke of luck.
> >
> > This wmi-camera driver only need to report the current state of the
> > camera switch.
> >
> >
> > Thanks.
> >
> > Ai Chao
> >
> I see, so userspace will notice when the camera is disabled. Since the camera disappears from
> the USB bus when the switch is activated, i wonder if SW_CAMERA_LENS_COVER is the right thing
> to use in this case after all.
>
> I CCed the maintainer of the input subsystem so that he can maybe provide some advise.
> I for example would, taking the above information above the UVC device into account, say that
> KEY_CAMERA_ACCESS_ENABLE/KEY_CAMERA_ACCESS_DISABLE is more suitable. Then userspace has to
> keep track of the camera state (through lsusb for example).
KEY_CAMERA_ACCESS_ENABLE/KEY_CAMERA_ACCESS_DISABLE is supposed to affect
all cameras connected to the system (HUTRR72: "The proposal is to add a
set of new Usage IDs to the Consumer Page (0x0C) to control programmatic
access to *all* camera devices connected to a computer." - emphasis
mine).
SW_CAMERA_LENS_COVER is useful when camera is always present but may be
covered.
If I understand this correctly the camera disappears from the bus and
reappears again when enabled. In this case why do you need the key event
at all? Are you planning on having a "stub" for the camera so that
userspace believes that the camera is always present but may be not
operational?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] input: make input_class constant
From: Dmitry Torokhov @ 2024-03-10 5:15 UTC (permalink / raw)
To: Ricardo B. Marliere; +Cc: linux-input, linux-kernel, Greg Kroah-Hartman
In-Reply-To: <20240305-class_cleanup-input-v1-1-0c3d950c25db@marliere.net>
On Tue, Mar 05, 2024 at 04:42:13PM -0300, Ricardo B. Marliere wrote:
> Since commit 43a7206b0963 ("driver core: class: make class_register() take
> a const *"), the driver core allows for struct class to be in read-only
> memory, so move the input_class structure to be declared at build time
> placing it into read-only memory, instead of having to be dynamically
> allocated at boot time.
>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
Applied, thank you.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 0/3] Imagis touch keys and FIELD_GET cleanup
From: Dmitry Torokhov @ 2024-03-10 5:32 UTC (permalink / raw)
To: Duje Mihanović
Cc: Markuss Broks, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Karel Balej, ~postmarketos/upstreaming, phone-devel, devicetree,
linux-input, linux-kernel
In-Reply-To: <20240306-b4-imagis-keys-v3-0-2c429afa8420@skole.hr>
On Wed, Mar 06, 2024 at 03:40:05PM +0100, Duje Mihanović wrote:
> Tiny series to clean up the field extraction and add touch key support.
> This version is based on the next branch of Dmitry's input tree.
>
> Signed-off-by: Duje Mihanović <duje.mihanovic@skole.hr>
> ---
> Changes in v3:
> - Rebase on input/next
> - Add changelog to binding patch
> - Fix binding constraint
> - Allow changing keycodes in userspace as in 872e57abd171 ("Input:
> tm2-touchkey - allow changing keycodes from userspace")
> - Allow up to 5 keycodes (the key status field has 5 bits)
> - Link to v2: https://lore.kernel.org/r/20240120-b4-imagis-keys-v2-0-d7fc16f2e106@skole.hr
>
> Changes in v2:
> - Fix compile error
> - Add FIELD_GET patch
> - Allow specifying custom keycodes
> - Link to v1: https://lore.kernel.org/20231112194124.24916-1-duje.mihanovic@skole.hr
>
> ---
> Duje Mihanović (3):
> input: touchscreen: imagis: use FIELD_GET where applicable
> dt-bindings: input: imagis: Document touch keys
> input: touchscreen: imagis: Add touch key support
>
> .../input/touchscreen/imagis,ist3038c.yaml | 19 +++++++--
> drivers/input/touchscreen/imagis.c | 46 ++++++++++++++++------
> 2 files changed, 50 insertions(+), 15 deletions(-)
Applied the lot, thank you.
--
Dmitry
^ permalink raw reply
* [PATCH v2 0/2] map Fn + R key on newer Lenovo Yogas and Legions
From: Gergo Koteles @ 2024-03-10 11:31 UTC (permalink / raw)
To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov
Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles
Hi All,
This patch series adds a new KEY_REFRESH_RATE_TOGGLE input event code
and maps the Fn + R key to it in the ideapad-laptop driver.
It affects two WMI keycodes. I couldn't try the 0x0a.
Regards,
Gergo
Changes in v2:
- use KEY_REFRESH_RATE_TOGGLE instead of KEY_FN_R
[1]: https://lore.kernel.org/all/cover.1708399689.git.soyer@irl.hu/
Gergo Koteles (2):
Input: allocate keycode for Display refresh rate toggle
platform/x86: ideapad-laptop: map Fn + R key to
KEY_REFRESH_RATE_TOGGLE
drivers/platform/x86/ideapad-laptop.c | 4 ++--
include/uapi/linux/input-event-codes.h | 1 +
2 files changed, 3 insertions(+), 2 deletions(-)
base-commit: b401b621758e46812da61fa58a67c3fd8d91de0d
prerequisite-patch-id: 66a1ec71f181c6468a8226430af7ee917f40138a
--
2.44.0
^ permalink raw reply
* [PATCH v2 2/2] platform/x86: ideapad-laptop: map Fn + R key to KEY_REFRESH_RATE_TOGGLE
From: Gergo Koteles @ 2024-03-10 11:31 UTC (permalink / raw)
To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov
Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles
In-Reply-To: <cover.1710065750.git.soyer@irl.hu>
Newer Lenovo Yogas and Legions with 60Hz/90Hz displays send a wmi event
when Fn + R is pressed. This is intended for use to switch between the
two refresh rates.
The Fn + R key was incorrectly assigned to KEY_DISPLAYTOGGLE because it
is used to toggle the display on and off.
Map Fn + R key to the KEY_REFRESH_RATE_TOGGLE event code.
This commit depends on "platform/x86: ideapad-laptop: support Fn+R
dual-function key"
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
drivers/platform/x86/ideapad-laptop.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/x86/ideapad-laptop.c b/drivers/platform/x86/ideapad-laptop.c
index 4c130957f80d..901849810ce2 100644
--- a/drivers/platform/x86/ideapad-laptop.c
+++ b/drivers/platform/x86/ideapad-laptop.c
@@ -1092,7 +1092,7 @@ static const struct key_entry ideapad_keymap[] = {
{ KE_KEY, 0x0e | IDEAPAD_WMI_KEY, { KEY_PICKUP_PHONE } },
{ KE_KEY, 0x0f | IDEAPAD_WMI_KEY, { KEY_HANGUP_PHONE } },
/* Refresh Rate Toggle (Fn+R) */
- { KE_KEY, 0x10 | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
+ { KE_KEY, 0x10 | IDEAPAD_WMI_KEY, { KEY_REFRESH_RATE_TOGGLE } },
/* Dark mode toggle */
{ KE_KEY, 0x13 | IDEAPAD_WMI_KEY, { KEY_PROG1 } },
/* Sound profile switch */
@@ -1102,7 +1102,7 @@ static const struct key_entry ideapad_keymap[] = {
/* Lenovo Support */
{ KE_KEY, 0x27 | IDEAPAD_WMI_KEY, { KEY_HELP } },
/* Refresh Rate Toggle */
- { KE_KEY, 0x0a | IDEAPAD_WMI_KEY, { KEY_DISPLAYTOGGLE } },
+ { KE_KEY, 0x0a | IDEAPAD_WMI_KEY, { KEY_REFRESH_RATE_TOGGLE } },
{ KE_END },
};
--
2.44.0
^ permalink raw reply related
* [PATCH v2 1/2] Input: allocate keycode for Display refresh rate toggle
From: Gergo Koteles @ 2024-03-10 11:31 UTC (permalink / raw)
To: Ike Panhc, Hans de Goede, Ilpo Järvinen, Dmitry Torokhov
Cc: platform-driver-x86, linux-kernel, linux-input, Gergo Koteles
In-Reply-To: <cover.1710065750.git.soyer@irl.hu>
Newer Lenovo Yogas and Legions with 60Hz/90Hz displays send a wmi event
when Fn + R is pressed. This is intended for use to switch between the
two refresh rates.
Allocate a new KEY_REFRESH_RATE_TOGGLE keycode for it.
Signed-off-by: Gergo Koteles <soyer@irl.hu>
---
include/uapi/linux/input-event-codes.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
index 022a520e31fc..03edf2ccdf6c 100644
--- a/include/uapi/linux/input-event-codes.h
+++ b/include/uapi/linux/input-event-codes.h
@@ -602,6 +602,7 @@
#define KEY_ALS_TOGGLE 0x230 /* Ambient light sensor */
#define KEY_ROTATE_LOCK_TOGGLE 0x231 /* Display rotation lock */
+#define KEY_REFRESH_RATE_TOGGLE 0x232 /* Display refresh rate toggle */
#define KEY_BUTTONCONFIG 0x240 /* AL Button Configuration */
#define KEY_TASKMANAGER 0x241 /* AL Task/Project Manager */
--
2.44.0
^ permalink raw reply related
* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Karel Balej @ 2024-03-10 11:35 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, devicetree, linux-kernel, linux-input,
Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <ZeZxI_spu4vwxrs7@google.com>
Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
> > Dmitry,
> >
> > Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
> > > On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
> > > > From: Karel Balej <balejk@matfyz.cz>
> > > >
> > > > Marvell 88PM886 PMIC provides onkey among other things. Add client
> > > > driver to handle it. The driver currently only provides a basic support
> > > > omitting additional functions found in the vendor version, such as long
> > > > onkey and GPIO integration.
> > > >
> > > > Signed-off-by: Karel Balej <balejk@matfyz.cz>
> > > > ---
> > > >
> > > > Notes:
> > > > RFC v3:
> > > > - Drop wakeup-source.
> > > > RFC v2:
> > > > - Address Dmitry's feedback:
> > > > - Sort includes alphabetically.
> > > > - Drop onkey->irq.
> > > > - ret -> err in irq_handler and no initialization.
> > > > - Break long lines and other formatting.
> > > > - Do not clobber platform_get_irq error.
> > > > - Do not set device parent manually.
> > > > - Use input_set_capability.
> > > > - Use the wakeup-source DT property.
> > > > - Drop of_match_table.
> > >
> > > I only said that you should not be using of_match_ptr(), but you still
> > > need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
> > > proper module loading support.
> >
> > I removed of_match_table because I no longer need compatible for this --
> > there are no device tree properties and the driver is being instantiated
> > by the MFD driver.
> >
> > Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
> > compiled as module? If that is the case, given what I write above, am I
> > correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
> > to use here?
>
> Yes, if uevent generated for the device is "platform:<name>" then
> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
> sets it up (OF modalias or platform), but you should be able to check
> the format looking at the "uevent" attribute for your device in sysfs
> (/sys/devices/bus/platform/...).
The uevent is indeed platform.
But since there is only one device, perhaps having a device table is
superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
fitting?
Although I don't understand why this is even necessary when the driver
name is such and the module is registered using
`module_platform_driver`...
Thank you, best regards,
K. B.
^ permalink raw reply
* [PATCH v2] HID: nintendo: use ida for LED player id
From: Martino Fontana @ 2024-03-10 18:01 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana, Ryan McClelland
Previously, the leds pattern would just increment with every controller
connected. This wouldn't take into consideration when controllers are
disconnected. The same controller could be connected and disconnected
with the pattern increasing player count each time.
This patch changes it by using an ID allocator in order to assign the
player id, the same way hid-playstation does.
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
Changes for v2:
ida_free now frees the correct id, instead of an id that got moduloed.
drivers/hid/hid-nintendo.c | 27 +++++++++++++++------------
1 file changed, 15 insertions(+), 12 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index ccc4032fb2b0..6ab4c2ec4a5d 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -34,6 +34,7 @@
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/hid.h>
+#include <linux/idr.h>
#include <linux/input.h>
#include <linux/jiffies.h>
#include <linux/leds.h>
@@ -569,6 +570,7 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
struct joycon_ctlr {
struct hid_device *hdev;
struct input_dev *input;
+ u32 player_id;
struct led_classdev leds[JC_NUM_LEDS]; /* player leds */
struct led_classdev home_led;
enum joycon_ctlr_state ctlr_state;
@@ -2283,7 +2285,8 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
return ret;
}
-static DEFINE_SPINLOCK(joycon_input_num_spinlock);
+static DEFINE_IDA(nintendo_player_id_allocator);
+
static int joycon_leds_create(struct joycon_ctlr *ctlr)
{
struct hid_device *hdev = ctlr->hdev;
@@ -2294,20 +2297,19 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
char *name;
int ret;
int i;
- unsigned long flags;
int player_led_pattern;
- static int input_num;
-
- /*
- * Set the player leds based on controller number
- * Because there is no standard concept of "player number", the pattern
- * number will simply increase by 1 every time a controller is connected.
- */
- spin_lock_irqsave(&joycon_input_num_spinlock, flags);
- player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
- spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
/* configure the player LEDs */
+ ctlr->player_id = U32_MAX;
+ ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
+ if (ret < 0) {
+ hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
+ goto home_led;
+ }
+ ctlr->player_id = ret;
+ player_led_pattern = ret % JC_NUM_LED_PATTERNS;
+ hid_info(ctlr->hdev, "assigned player %d led pattern", player_led_pattern + 1);
+
for (i = 0; i < JC_NUM_LEDS; i++) {
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
d_name,
@@ -2789,6 +2791,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
spin_unlock_irqrestore(&ctlr->lock, flags);
destroy_workqueue(ctlr->rumble_queue);
+ ida_free(&nintendo_player_id_allocator, ctlr->player_id);
hid_hw_close(hdev);
hid_hw_stop(hdev);
--
2.43.1
^ permalink raw reply related
* [dtor-input:next] BUILD SUCCESS 326b35a8d7fa38a0ef982eaf3e221738cf59aee8
From: kernel test robot @ 2024-03-10 18:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 326b35a8d7fa38a0ef982eaf3e221738cf59aee8 Input: imagis - add touch key support
elapsed time: 727m
configs tested: 212
configs skipped: 3
The following configs have been built successfully.
More configs may be tested in the coming days.
tested configs:
alpha allnoconfig gcc
alpha allyesconfig gcc
alpha defconfig gcc
arc allmodconfig gcc
arc allnoconfig gcc
arc allyesconfig gcc
arc axs101_defconfig gcc
arc defconfig gcc
arc randconfig-001-20240310 gcc
arc randconfig-002-20240310 gcc
arc vdk_hs38_defconfig gcc
arm alldefconfig gcc
arm allmodconfig gcc
arm allnoconfig clang
arm allyesconfig gcc
arm defconfig clang
arm footbridge_defconfig clang
arm jornada720_defconfig clang
arm mmp2_defconfig gcc
arm randconfig-001-20240310 gcc
arm randconfig-002-20240310 gcc
arm randconfig-003-20240310 gcc
arm randconfig-004-20240310 gcc
arm sama7_defconfig clang
arm shmobile_defconfig gcc
arm64 allmodconfig clang
arm64 allnoconfig gcc
arm64 allyesconfig clang
arm64 defconfig gcc
arm64 randconfig-001-20240310 clang
arm64 randconfig-002-20240310 gcc
arm64 randconfig-003-20240310 clang
arm64 randconfig-004-20240310 clang
csky allmodconfig gcc
csky allnoconfig gcc
csky allyesconfig gcc
csky defconfig gcc
csky randconfig-001-20240310 gcc
csky randconfig-002-20240310 gcc
hexagon allmodconfig clang
hexagon allnoconfig clang
hexagon allyesconfig clang
hexagon defconfig clang
hexagon randconfig-001-20240310 clang
hexagon randconfig-002-20240310 clang
i386 allmodconfig gcc
i386 allnoconfig gcc
i386 allyesconfig gcc
i386 buildonly-randconfig-001-20240310 clang
i386 buildonly-randconfig-002-20240310 clang
i386 buildonly-randconfig-003-20240310 clang
i386 buildonly-randconfig-004-20240310 clang
i386 buildonly-randconfig-005-20240310 gcc
i386 buildonly-randconfig-006-20240310 gcc
i386 defconfig clang
i386 randconfig-001-20240310 gcc
i386 randconfig-002-20240310 gcc
i386 randconfig-003-20240310 clang
i386 randconfig-004-20240310 gcc
i386 randconfig-005-20240310 gcc
i386 randconfig-006-20240310 gcc
i386 randconfig-011-20240310 clang
i386 randconfig-012-20240310 clang
i386 randconfig-013-20240310 clang
i386 randconfig-014-20240310 clang
i386 randconfig-015-20240310 clang
i386 randconfig-016-20240310 gcc
loongarch allmodconfig gcc
loongarch allnoconfig gcc
loongarch defconfig gcc
loongarch randconfig-001-20240310 gcc
loongarch randconfig-002-20240310 gcc
m68k allmodconfig gcc
m68k allnoconfig gcc
m68k allyesconfig gcc
m68k apollo_defconfig gcc
m68k atari_defconfig gcc
m68k defconfig gcc
m68k m5208evb_defconfig gcc
m68k m5249evb_defconfig gcc
m68k multi_defconfig gcc
m68k mvme16x_defconfig gcc
m68k stmark2_defconfig gcc
m68k virt_defconfig gcc
microblaze allmodconfig gcc
microblaze allnoconfig gcc
microblaze allyesconfig gcc
microblaze defconfig gcc
mips allnoconfig gcc
mips allyesconfig gcc
mips cu1000-neo_defconfig gcc
mips decstation_r4k_defconfig gcc
mips ip22_defconfig gcc
nios2 allmodconfig gcc
nios2 allnoconfig gcc
nios2 allyesconfig gcc
nios2 defconfig gcc
nios2 randconfig-001-20240310 gcc
nios2 randconfig-002-20240310 gcc
openrisc allnoconfig gcc
openrisc allyesconfig gcc
openrisc defconfig gcc
openrisc virt_defconfig gcc
parisc allmodconfig gcc
parisc allnoconfig gcc
parisc allyesconfig gcc
parisc defconfig gcc
parisc generic-64bit_defconfig gcc
parisc randconfig-001-20240310 gcc
parisc randconfig-002-20240310 gcc
parisc64 defconfig gcc
powerpc adder875_defconfig gcc
powerpc akebono_defconfig clang
powerpc allmodconfig gcc
powerpc allnoconfig gcc
powerpc allyesconfig clang
powerpc canyonlands_defconfig clang
powerpc ep8248e_defconfig gcc
powerpc kilauea_defconfig clang
powerpc klondike_defconfig gcc
powerpc mgcoge_defconfig clang
powerpc mpc8313_rdb_defconfig gcc
powerpc mpc834x_itxgp_defconfig clang
powerpc mvme5100_defconfig gcc
powerpc ppa8548_defconfig gcc
powerpc ppc64_defconfig clang
powerpc randconfig-001-20240310 gcc
powerpc randconfig-002-20240310 clang
powerpc randconfig-003-20240310 clang
powerpc socrates_defconfig gcc
powerpc tqm5200_defconfig gcc
powerpc64 randconfig-001-20240310 gcc
powerpc64 randconfig-002-20240310 gcc
powerpc64 randconfig-003-20240310 clang
riscv allmodconfig clang
riscv allnoconfig gcc
riscv allyesconfig clang
riscv defconfig clang
riscv randconfig-001-20240310 gcc
riscv randconfig-002-20240310 clang
riscv rv32_defconfig clang
s390 allmodconfig clang
s390 allnoconfig clang
s390 allyesconfig gcc
s390 defconfig clang
s390 randconfig-001-20240310 clang
s390 randconfig-002-20240310 clang
sh allmodconfig gcc
sh allnoconfig gcc
sh allyesconfig gcc
sh apsh4a3a_defconfig gcc
sh apsh4ad0a_defconfig gcc
sh defconfig gcc
sh espt_defconfig gcc
sh kfr2r09-romimage_defconfig gcc
sh randconfig-001-20240310 gcc
sh randconfig-002-20240310 gcc
sh rsk7264_defconfig gcc
sh se7705_defconfig gcc
sh se7750_defconfig gcc
sh sh03_defconfig gcc
sh sh2007_defconfig gcc
sh shmin_defconfig gcc
sparc allmodconfig gcc
sparc allnoconfig gcc
sparc defconfig gcc
sparc64 allmodconfig gcc
sparc64 allyesconfig gcc
sparc64 defconfig gcc
sparc64 randconfig-001-20240310 gcc
sparc64 randconfig-002-20240310 gcc
um allmodconfig clang
um allnoconfig clang
um allyesconfig gcc
um defconfig clang
um i386_defconfig gcc
um randconfig-001-20240310 gcc
um randconfig-002-20240310 gcc
um x86_64_defconfig clang
x86_64 allnoconfig clang
x86_64 allyesconfig clang
x86_64 buildonly-randconfig-001-20240310 clang
x86_64 buildonly-randconfig-002-20240310 gcc
x86_64 buildonly-randconfig-003-20240310 clang
x86_64 buildonly-randconfig-004-20240310 gcc
x86_64 buildonly-randconfig-005-20240310 clang
x86_64 buildonly-randconfig-006-20240310 gcc
x86_64 defconfig gcc
x86_64 randconfig-001-20240310 clang
x86_64 randconfig-002-20240310 gcc
x86_64 randconfig-003-20240310 gcc
x86_64 randconfig-004-20240310 gcc
x86_64 randconfig-005-20240310 clang
x86_64 randconfig-006-20240310 gcc
x86_64 randconfig-011-20240310 gcc
x86_64 randconfig-012-20240310 clang
x86_64 randconfig-013-20240310 clang
x86_64 randconfig-014-20240310 gcc
x86_64 randconfig-015-20240310 clang
x86_64 randconfig-016-20240310 gcc
x86_64 randconfig-071-20240310 gcc
x86_64 randconfig-072-20240310 gcc
x86_64 randconfig-073-20240310 gcc
x86_64 randconfig-074-20240310 gcc
x86_64 randconfig-075-20240310 clang
x86_64 randconfig-076-20240310 gcc
x86_64 rhel-8.3-func gcc
x86_64 rhel-8.3-rust clang
x86_64 rhel-8.3 gcc
xtensa allnoconfig gcc
xtensa randconfig-001-20240310 gcc
xtensa randconfig-002-20240310 gcc
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v9 1/5] firmware: cs_dsp: Add write sequencer interface
From: Jeff LaBundy @ 2024-03-10 19:12 UTC (permalink / raw)
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree,
Charles Keepax
In-Reply-To: <20240308222421.188858-2-jogletre@opensource.cirrus.com>
Hi James,
On Fri, Mar 08, 2024 at 10:24:17PM +0000, James Ogletree wrote:
> A write sequencer is a sequence of register addresses
> and values executed by some Cirrus DSPs following
> power state transitions.
>
> Add support for Cirrus drivers to update or add to a
> write sequencer present in firmware.
>
> Reviewed-by: Charles Keepax <ckeepax@opensource.cirrus.com>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
Truly excellent work here; thanks again for continuing to lead a
productive review. I think the overall design of the driver and
the way it is organized are solid now.
Apologies for not getting you feedback sooner; I have some minor
comments below and in a few other spots throughout the series. I
think it is ready after these last few points are addressed.
> ---
> drivers/firmware/cirrus/cs_dsp.c | 263 +++++++++++++++++++++++++
> include/linux/firmware/cirrus/cs_dsp.h | 28 +++
> 2 files changed, 291 insertions(+)
>
> diff --git a/drivers/firmware/cirrus/cs_dsp.c b/drivers/firmware/cirrus/cs_dsp.c
> index 79d4254d1f9b..90cd56d70c49 100644
> --- a/drivers/firmware/cirrus/cs_dsp.c
> +++ b/drivers/firmware/cirrus/cs_dsp.c
> @@ -275,6 +275,12 @@
> #define HALO_MPU_VIO_ERR_SRC_MASK 0x00007fff
> #define HALO_MPU_VIO_ERR_SRC_SHIFT 0
>
> +/*
> + * Write Sequence
> + */
> +#define WSEQ_OP_MAX_WORDS 3
> +#define WSEQ_END_OF_SCRIPT 0xFFFFFF
> +
> struct cs_dsp_ops {
> bool (*validate_version)(struct cs_dsp *dsp, unsigned int version);
> unsigned int (*parse_sizes)(struct cs_dsp *dsp,
> @@ -3339,6 +3345,263 @@ int cs_dsp_chunk_read(struct cs_dsp_chunk *ch, int nbits)
> }
> EXPORT_SYMBOL_NS_GPL(cs_dsp_chunk_read, FW_CS_DSP);
>
> +
> +struct cs_dsp_wseq_op {
> + struct list_head list;
> + u32 address;
> + u32 data;
> + u16 offset;
> + u8 operation;
> +};
> +
> +static int cs_dsp_populate_wseq(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq)
> +{
> + struct cs_dsp_wseq_op *op = NULL;
> + struct cs_dsp_chunk ch;
> + u8 *words;
> + int ret;
> +
> + if (!wseq->ctl) {
> + cs_dsp_err(dsp, "No control for write sequence\n");
> + return -EINVAL;
> + }
> +
> + words = kzalloc(wseq->ctl->len, GFP_KERNEL);
> + if (!words)
> + return -ENOMEM;
> +
> + ret = cs_dsp_coeff_read_ctrl(wseq->ctl, 0, words, wseq->ctl->len);
> + if (ret) {
> + cs_dsp_err(dsp, "Failed to read %s: %d\n", wseq->ctl->subname, ret);
> + goto err_free;
> + }
> +
> + INIT_LIST_HEAD(&wseq->ops);
> +
> + ch = cs_dsp_chunk(words, wseq->ctl->len);
> +
> + while (!cs_dsp_chunk_end(&ch)) {
> + op = devm_kzalloc(dsp->dev, sizeof(*op), GFP_KERNEL);
> + if (!op) {
> + ret = -ENOMEM;
> + goto err_free;
> + }
> +
> + op->offset = cs_dsp_chunk_bytes(&ch);
> + op->operation = cs_dsp_chunk_read(&ch, 8);
> +
> + switch (op->operation) {
> + case CS_DSP_WSEQ_END:
> + op->data = WSEQ_END_OF_SCRIPT;
> + break;
> + case CS_DSP_WSEQ_UNLOCK:
> + op->data = cs_dsp_chunk_read(&ch, 16);
> + break;
> + case CS_DSP_WSEQ_ADDR8:
> + op->address = cs_dsp_chunk_read(&ch, 8);
> + op->data = cs_dsp_chunk_read(&ch, 32);
> + break;
> + case CS_DSP_WSEQ_H16:
> + case CS_DSP_WSEQ_L16:
> + op->address = cs_dsp_chunk_read(&ch, 24);
> + op->data = cs_dsp_chunk_read(&ch, 16);
> + break;
> + case CS_DSP_WSEQ_FULL:
> + op->address = cs_dsp_chunk_read(&ch, 32);
> + op->data = cs_dsp_chunk_read(&ch, 32);
> + break;
> + default:
> + ret = -EINVAL;
> + cs_dsp_err(dsp, "Unsupported op: %X\n", op->operation);
> + goto err_free;
> + }
> +
> + list_add_tail(&op->list, &wseq->ops);
> +
> + if (op->operation == CS_DSP_WSEQ_END)
> + break;
> + }
> +
> + if (op && op->operation != CS_DSP_WSEQ_END) {
> + cs_dsp_err(dsp, "Write sequence missing end terminator\n");
> + ret = -ENOENT;
> + }
> +
> +err_free:
> + kfree(words);
> +
> + return ret;
> +}
> +
> +/**
> + * cs_dsp_wseq_init() - Initialize write sequences contained within the loaded DSP firmware
> + * @dsp: Pointer to DSP structure
> + * @wseqs: List of write sequences to initialize
> + * @num_wseqs: Number of write sequences to initialize
> + *
> + * Return: Zero for success, a negative number on error.
> + */
> +int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs)
> +{
> + int i, ret = 0;
> +
> + lockdep_assert_held(&dsp->pwr_lock);
> +
> + for (i = 0; i < num_wseqs; i++) {
> + ret = cs_dsp_populate_wseq(dsp, &wseqs[i]);
> + if (ret)
> + break;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_init, FW_CS_DSP);
> +
> +static struct cs_dsp_wseq_op *cs_dsp_wseq_find_op(u32 addr, u8 op_code,
> + struct list_head *wseq_ops)
> +{
> + struct cs_dsp_wseq_op *op;
> +
> + list_for_each_entry(op, wseq_ops, list) {
> + if (op->operation == op_code && op->address == addr)
> + return op;
> + }
> +
> + return NULL;
> +}
> +
> +/**
> + * cs_dsp_wseq_write() - Add or update an entry in a write sequence
> + * @dsp: Pointer to a DSP structure
> + * @wseq: Write sequence to write to
> + * @addr: Address of the register to be written to
> + * @data: Data to be written
> + * @op_code: The type of operation of the new entry
> + * @update: If true, searches for the first entry in the write sequence with
> + * the same address and op_code, and replaces it. If false, creates a new entry
> + * at the tail.
> + *
> + * This function formats register address and value pairs into the format
> + * required for write sequence entries, and either updates or adds the
> + * new entry into the write sequence.
> + *
> + * If update is set to true and no matching entry is found, it will add a new entry.
> + *
> + * Return: Zero for success, a negative number on error.
> + */
> +int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> + u32 addr, u32 data, u8 op_code, bool update)
> +{
> + struct cs_dsp_wseq_op *op_end, *op_new = NULL;
> + u32 words[WSEQ_OP_MAX_WORDS];
> + struct cs_dsp_chunk ch;
Nit: 'chunk' is probably a more descriptive variable name; 'ch' is ambiguous
and too easily confused with other common things (e.g. channel).
> + int new_op_size, ret;
> +
> + if (update)
> + op_new = cs_dsp_wseq_find_op(addr, op_code, &wseq->ops);
> +
> + /* If entry to update is not found, treat it as a new operation */
> + if (!op_new) {
> + op_end = cs_dsp_wseq_find_op(0, CS_DSP_WSEQ_END, &wseq->ops);
> + if (!op_end) {
> + cs_dsp_err(dsp, "Missing write sequence list terminator\n");
> + return -EINVAL;
> + }
> +
> + op_new = devm_kzalloc(dsp->dev, sizeof(*op_new), GFP_KERNEL);
> + if (!op_new)
> + return -ENOMEM;
> +
> + op_new->operation = op_code;
> + op_new->address = addr;
> + op_new->offset = op_end->offset;
> + update = false;
> + }
> +
> + op_new->data = data;
> +
> + ch = cs_dsp_chunk(words, sizeof(words));
> + cs_dsp_chunk_write(&ch, 8, op_new->operation);
Nit: maybe a newline here?
> + switch (op_code) {
> + case CS_DSP_WSEQ_FULL:
> + cs_dsp_chunk_write(&ch, 32, op_new->address);
> + cs_dsp_chunk_write(&ch, 32, op_new->data);
> + break;
> + case CS_DSP_WSEQ_L16:
> + case CS_DSP_WSEQ_H16:
> + cs_dsp_chunk_write(&ch, 24, op_new->address);
> + cs_dsp_chunk_write(&ch, 16, op_new->data);
> + break;
> + default:
> + ret = -EINVAL;
> + cs_dsp_err(dsp, "Op code not supported: %X\n", op_code);
> + goto op_new_free;
There is no need to drop down and call devm_kfree() here; it's sufficient
to simply return -EINVAL. The devres core will free any instances of
cs_dsp_wseq_op when the driver is unbound.
I imagine you're calling devm_kfree() to protect against the case where
the driver successfully probes, but the contents of the firmware are found
to be invalid later. In that case, yes, this newly allocated memory will
persist throughout the length of the driver's life.
That's an error condition though; if it happens, the customer will surely
remove the module, correct the .wmfw or .bin file, then insert the module
again. All we need to do here is make sure that memory does not leak after
the module is removed, and device-managed allocation already handles this.
> + }
> +
> + new_op_size = cs_dsp_chunk_bytes(&ch);
> +
> + if (!update) {
> + if (wseq->ctl->len - op_end->offset < new_op_size) {
> + cs_dsp_err(dsp, "Not enough memory in write sequence for entry\n");
> + ret = -ENOMEM;
> + goto op_new_free;
Same here. Maybe you want to return -E2BIG here so the caller can tell the
difference between there not being enough memory available to allocate a
new instance of cs_dsp_wseq_op, vs. one having already been allocated, but
not big enough.
> + }
> +
> + op_end->offset += new_op_size;
> +
> + ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_end->offset / sizeof(u32),
> + &op_end->data, sizeof(u32));
> + if (ret)
> + goto op_new_free;
And here; just return ret.
> +
> + list_add_tail(&op_new->list, &op_end->list);
> + }
> +
> + ret = cs_dsp_coeff_write_ctrl(wseq->ctl, op_new->offset / sizeof(u32),
> + words, new_op_size);
> + if (ret)
> + goto op_new_free;
> +
> + return 0;
Here too. This clean-up avoids the rather unsightly design pattern of placing
a tear-down path after the function's primary return path.
> +
> +op_new_free:
> + devm_kfree(dsp->dev, op_new);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_write, FW_CS_DSP);
> +
> +/**
> + * cs_dsp_wseq_multi_write() - Add or update multiple entries in the write sequence
> + * @dsp: Pointer to a DSP structure
> + * @wseq: Write sequence to write to
> + * @reg_seq: List of address-data pairs
> + * @num_regs: Number of address-data pairs
> + * @op_code: The types of operations of the new entries
> + * @update: If true, searches for the first entry in the write sequence with the same
> + * address and op code, and replaces it. If false, creates a new entry at the tail.
> + *
> + * This function calls cs_dsp_wseq_write() for multiple address-data pairs.
> + *
> + * Return: Zero for success, a negative number on error.
> + */
> +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> + const struct reg_sequence *reg_seq, int num_regs,
> + u8 op_code, bool update)
> +{
> + int ret, i;
> +
> + for (i = 0; i < num_regs; i++) {
> + ret = cs_dsp_wseq_write(dsp, wseq, reg_seq[i].reg,
> + reg_seq[i].def, update, op_code);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
This is absolutely a nit-pick, but in cs_dsp_wseq_init(), you use the pattern
of breaking on error and always returning ret; here you return on error. Both
are functionally equivalent but it would be nice to be consistent in terms of
style.
If you do opt to match cs_dsp_wseq_multi_write() to cs_dsp_wseq_init(), I do
think you'll need to initialize ret to zero here as well to avoid a compiler
warning for the num_regs = 0 case.
> +}
> +EXPORT_SYMBOL_NS_GPL(cs_dsp_wseq_multi_write, FW_CS_DSP);
> +
> MODULE_DESCRIPTION("Cirrus Logic DSP Support");
> MODULE_AUTHOR("Simon Trimmer <simont@opensource.cirrus.com>");
> MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/firmware/cirrus/cs_dsp.h b/include/linux/firmware/cirrus/cs_dsp.h
> index 29cd11d5a3cf..cfeab75772f6 100644
> --- a/include/linux/firmware/cirrus/cs_dsp.h
> +++ b/include/linux/firmware/cirrus/cs_dsp.h
> @@ -42,6 +42,16 @@
> #define CS_DSP_ACKED_CTL_MIN_VALUE 0
> #define CS_DSP_ACKED_CTL_MAX_VALUE 0xFFFFFF
>
> +/*
> + * Write sequence operation codes
> + */
> +#define CS_DSP_WSEQ_FULL 0x00
> +#define CS_DSP_WSEQ_ADDR8 0x02
> +#define CS_DSP_WSEQ_L16 0x04
> +#define CS_DSP_WSEQ_H16 0x05
> +#define CS_DSP_WSEQ_UNLOCK 0xFD
> +#define CS_DSP_WSEQ_END 0xFF
> +
> /**
> * struct cs_dsp_region - Describes a logical memory region in DSP address space
> * @type: Memory region type
> @@ -255,6 +265,24 @@ struct cs_dsp_alg_region *cs_dsp_find_alg_region(struct cs_dsp *dsp,
>
> const char *cs_dsp_mem_region_name(unsigned int type);
>
> +/**
> + * struct cs_dsp_wseq - Describes a write sequence
> + * @name: Name of cs_dsp control
> + * @ctl: Write sequence cs_dsp control
> + * @ops: Operations contained within this write sequence
> + */
> +struct cs_dsp_wseq {
> + struct cs_dsp_coeff_ctl *ctl;
> + struct list_head ops;
> +};
> +
> +int cs_dsp_wseq_init(struct cs_dsp *dsp, struct cs_dsp_wseq *wseqs, unsigned int num_wseqs);
> +int cs_dsp_wseq_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq, u32 addr, u32 data,
> + u8 op_code, bool update);
> +int cs_dsp_wseq_multi_write(struct cs_dsp *dsp, struct cs_dsp_wseq *wseq,
> + const struct reg_sequence *reg_seq, int num_regs,
> + u8 op_code, bool update);
> +
> /**
> * struct cs_dsp_chunk - Describes a buffer holding data formatted for the DSP
> * @data: Pointer to underlying buffer memory
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [PATCH v9 2/5] dt-bindings: input: cirrus,cs40l50: Add initial DT binding
From: Jeff LaBundy @ 2024-03-10 19:29 UTC (permalink / raw)
To: James Ogletree
Cc: dmitry.torokhov, robh+dt, krzysztof.kozlowski+dt, conor+dt, lee,
broonie, patches, linux-sound, linux-input, devicetree,
Krzysztof Kozlowski
In-Reply-To: <20240308222421.188858-3-jogletre@opensource.cirrus.com>
Hi James,
On Fri, Mar 08, 2024 at 10:24:18PM +0000, James Ogletree wrote:
> The CS40L50 is a haptic driver with waveform memory,
> integrated DSP, and closed-loop algorithms.
>
> Add a YAML DT binding document for this device.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: James Ogletree <jogletre@opensource.cirrus.com>
> ---
> .../bindings/input/cirrus,cs40l50.yaml | 70 +++++++++++++++++++
> MAINTAINERS | 8 +++
> 2 files changed, 78 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> new file mode 100644
> index 000000000000..6a5bdafed56b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> @@ -0,0 +1,70 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cirrus,cs40l50.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus Logic CS40L50 Advanced Haptic Driver
> +
> +maintainers:
> + - James Ogletree <james.ogletree@cirrus.com>
Nit: this email address and the one in MAINTAINERS don't match the one
you're using to sign off. There is no requirement that they do; I just
wanted to check whether this was intentional.
> +
> +description:
> + CS40L50 is a haptic driver with waveform memory,
> + integrated DSP, and closed-loop algorithms.
> +
> +properties:
> + compatible:
> + enum:
> + - cirrus,cs40l50
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + reset-gpios:
> + maxItems: 1
> +
> + va-supply:
> + description: Power supply for internal analog circuits.
> +
> + vp-supply:
> + description: Power supply for always-on circuits.
> +
> + vio-supply:
> + description: Power supply for digital input/output.
> +
> + vamp-supply:
> + description: Power supply for the Class D amplifier.
Does L50 support external boost mode? If not, it will always be shorted
directly to VBST on the board, and there is no reason to describe it in
the binding.
If external boost mode is supported, then I recommend extending support
for it in the driver. Perhaps some additional registers must be set if
this supply is present.
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - reset-gpios
> + - vp-supply
Making VP a required supply is likely inconvenient for customers; 99% of
them connect it to a battery, and end up tying this property to a dummy
regulator to keep the driver from bleating.
Only for a wall-powered case would VP be tied to something like a 3.3-V
switching supply, and I imagine those cases are rare. It seems that VP
should be optional.
> + - vio-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
Nit: most device trees tend to use 8-column indentation as with kernel code.
> +
> + haptic-driver@34 {
> + compatible = "cirrus,cs40l50";
> + reg = <0x34>;
> + interrupt-parent = <&gpio>;
> + interrupts = <113 IRQ_TYPE_LEVEL_LOW>;
> + reset-gpios = <&gpio 112 GPIO_ACTIVE_LOW>;
> + vp-supply = <&vreg>;
> + vio-supply = <&vreg>;
Showing VP and VIO tied to the same supply is not a valid example; VP
typically connects to a battery, and VIO is likely a 1.8-V supply. Their
voltage ranges do not overlap, and hence they cannot be shared. I also
suspect there are sequencing restrictions between them as well.
> + };
> + };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dd5de540ec0b..b71017a187f8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4933,6 +4933,14 @@ F: sound/pci/hda/cs*
> F: sound/pci/hda/hda_cs_dsp_ctl.*
> F: sound/soc/codecs/cs*
>
> +CIRRUS LOGIC HAPTIC DRIVERS
> +M: James Ogletree <james.ogletree@cirrus.com>
> +M: Fred Treven <fred.treven@cirrus.com>
> +M: Ben Bright <ben.bright@cirrus.com>
> +L: patches@opensource.cirrus.com
> +S: Supported
> +F: Documentation/devicetree/bindings/input/cirrus,cs40l50.yaml
> +
> CIRRUS LOGIC DSP FIRMWARE DRIVER
> M: Simon Trimmer <simont@opensource.cirrus.com>
> M: Charles Keepax <ckeepax@opensource.cirrus.com>
> --
> 2.25.1
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Krzysztof Kozlowski @ 2024-03-10 20:35 UTC (permalink / raw)
To: Karel Balej, Dmitry Torokhov
Cc: Lee Jones, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, devicetree, linux-kernel, linux-input,
Duje Mihanović, ~postmarketos/upstreaming, phone-devel
In-Reply-To: <CZQ1EP61IDOC.1PPYGMIOINGND@gimli.ms.mff.cuni.cz>
On 10/03/2024 12:35, Karel Balej wrote:
> Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
>> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
>>> Dmitry,
>>>
>>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
>>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
>>>>> From: Karel Balej <balejk@matfyz.cz>
>>>>>
>>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
>>>>> driver to handle it. The driver currently only provides a basic support
>>>>> omitting additional functions found in the vendor version, such as long
>>>>> onkey and GPIO integration.
>>>>>
>>>>> Signed-off-by: Karel Balej <balejk@matfyz.cz>
>>>>> ---
>>>>>
>>>>> Notes:
>>>>> RFC v3:
>>>>> - Drop wakeup-source.
>>>>> RFC v2:
>>>>> - Address Dmitry's feedback:
>>>>> - Sort includes alphabetically.
>>>>> - Drop onkey->irq.
>>>>> - ret -> err in irq_handler and no initialization.
>>>>> - Break long lines and other formatting.
>>>>> - Do not clobber platform_get_irq error.
>>>>> - Do not set device parent manually.
>>>>> - Use input_set_capability.
>>>>> - Use the wakeup-source DT property.
>>>>> - Drop of_match_table.
>>>>
>>>> I only said that you should not be using of_match_ptr(), but you still
>>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
>>>> proper module loading support.
>>>
>>> I removed of_match_table because I no longer need compatible for this --
>>> there are no device tree properties and the driver is being instantiated
>>> by the MFD driver.
>>>
>>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
>>> compiled as module? If that is the case, given what I write above, am I
>>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
>>> to use here?
>>
>> Yes, if uevent generated for the device is "platform:<name>" then
>> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
>> sets it up (OF modalias or platform), but you should be able to check
>> the format looking at the "uevent" attribute for your device in sysfs
>> (/sys/devices/bus/platform/...).
>
> The uevent is indeed platform.
>
> But since there is only one device, perhaps having a device table is
> superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
> fitting?
Adding aliases for standard IDs and standard cases is almost never
correct. If you need module alias, it means your ID table is wrong (or
missing, which is usually wrong).
>
> Although I don't understand why this is even necessary when the driver
> name is such and the module is registered using
> `module_platform_driver`...
ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.
Just run `modinfo`.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2] HID: nintendo: use ida for LED player id
From: Silvan Jegen @ 2024-03-10 20:58 UTC (permalink / raw)
To: Martino Fontana
Cc: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel,
Ryan McClelland
In-Reply-To: <20240310180322.25508-2-tinozzo123@gmail.com>
Heyhey
One question below.
Martino Fontana <tinozzo123@gmail.com> wrote:
> Previously, the leds pattern would just increment with every controller
> connected. This wouldn't take into consideration when controllers are
> disconnected. The same controller could be connected and disconnected
> with the pattern increasing player count each time.
>
> This patch changes it by using an ID allocator in order to assign the
> player id, the same way hid-playstation does.
>
> Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
> Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
> ---
> Changes for v2:
>
> ida_free now frees the correct id, instead of an id that got moduloed.
>
> drivers/hid/hid-nintendo.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
> index ccc4032fb2b0..6ab4c2ec4a5d 100644
> --- a/drivers/hid/hid-nintendo.c
> +++ b/drivers/hid/hid-nintendo.c
> @@ -34,6 +34,7 @@
> #include <linux/device.h>
> #include <linux/kernel.h>
> #include <linux/hid.h>
> +#include <linux/idr.h>
> #include <linux/input.h>
> #include <linux/jiffies.h>
> #include <linux/leds.h>
> @@ -569,6 +570,7 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
> struct joycon_ctlr {
> struct hid_device *hdev;
> struct input_dev *input;
> + u32 player_id;
> struct led_classdev leds[JC_NUM_LEDS]; /* player leds */
> struct led_classdev home_led;
> enum joycon_ctlr_state ctlr_state;
> @@ -2283,7 +2285,8 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
> return ret;
> }
>
> -static DEFINE_SPINLOCK(joycon_input_num_spinlock);
> +static DEFINE_IDA(nintendo_player_id_allocator);
> +
> static int joycon_leds_create(struct joycon_ctlr *ctlr)
> {
> struct hid_device *hdev = ctlr->hdev;
> @@ -2294,20 +2297,19 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
> char *name;
> int ret;
> int i;
> - unsigned long flags;
> int player_led_pattern;
> - static int input_num;
> -
> - /*
> - * Set the player leds based on controller number
> - * Because there is no standard concept of "player number", the pattern
> - * number will simply increase by 1 every time a controller is connected.
> - */
> - spin_lock_irqsave(&joycon_input_num_spinlock, flags);
> - player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
> - spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
>
> /* configure the player LEDs */
> + ctlr->player_id = U32_MAX;
> + ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
> + if (ret < 0) {
> + hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
> + goto home_led;
> + }
> + ctlr->player_id = ret;
> + player_led_pattern = ret % JC_NUM_LED_PATTERNS;
> + hid_info(ctlr->hdev, "assigned player %d led pattern", player_led_pattern + 1);
> +
> for (i = 0; i < JC_NUM_LEDS; i++) {
> name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
> d_name,
> @@ -2789,6 +2791,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
> spin_unlock_irqrestore(&ctlr->lock, flags);
>
> destroy_workqueue(ctlr->rumble_queue);
> + ida_free(&nintendo_player_id_allocator, ctlr->player_id);
The PlayStation driver also destroys the allocator on module exit. Do
we not have to do the same in this module?
Cheers,
Silvan
> hid_hw_close(hdev);
> hid_hw_stop(hdev);
^ permalink raw reply
* Re: [PATCH v2 1/2] Input: allocate keycode for Display refresh rate toggle
From: Dmitry Torokhov @ 2024-03-10 21:34 UTC (permalink / raw)
To: Gergo Koteles
Cc: Ike Panhc, Hans de Goede, Ilpo Järvinen, platform-driver-x86,
linux-kernel, linux-input
In-Reply-To: <15a5d08c84cf4d7b820de34ebbcf8ae2502fb3ca.1710065750.git.soyer@irl.hu>
On Sun, Mar 10, 2024 at 12:31:41PM +0100, Gergo Koteles wrote:
> Newer Lenovo Yogas and Legions with 60Hz/90Hz displays send a wmi event
> when Fn + R is pressed. This is intended for use to switch between the
> two refresh rates.
>
> Allocate a new KEY_REFRESH_RATE_TOGGLE keycode for it.
>
> Signed-off-by: Gergo Koteles <soyer@irl.hu>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> include/uapi/linux/input-event-codes.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 022a520e31fc..03edf2ccdf6c 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -602,6 +602,7 @@
>
> #define KEY_ALS_TOGGLE 0x230 /* Ambient light sensor */
> #define KEY_ROTATE_LOCK_TOGGLE 0x231 /* Display rotation lock */
> +#define KEY_REFRESH_RATE_TOGGLE 0x232 /* Display refresh rate toggle */
>
> #define KEY_BUTTONCONFIG 0x240 /* AL Button Configuration */
> #define KEY_TASKMANAGER 0x241 /* AL Task/Project Manager */
> --
> 2.44.0
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: nintendo: use ida for LED player id
From: Martino Fontana @ 2024-03-10 21:39 UTC (permalink / raw)
To: Silvan Jegen
Cc: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel,
Ryan McClelland
In-Reply-To: <2TCU3VNKI7TD9.2PWW1CG9ZHYBA@homearch.localdomain>
Hi
> The PlayStation driver also destroys the allocator on module exit. Do
> we not have to do the same in this module?
I forgot to do that...
If everything else's good, I'll send a v3 that fixes that.
Cheers,
Martino
^ permalink raw reply
* Re: [RFC PATCH v3 4/5] input: add onkey driver for Marvell 88PM886 PMIC
From: Dmitry Torokhov @ 2024-03-10 21:48 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Karel Balej, Lee Jones, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Liam Girdwood, Mark Brown, devicetree, linux-kernel,
linux-input, Duje Mihanović, ~postmarketos/upstreaming,
phone-devel
In-Reply-To: <3601a374-4161-40e1-8a80-9bbfdae5bd8a@linaro.org>
On Sun, Mar 10, 2024 at 09:35:36PM +0100, Krzysztof Kozlowski wrote:
> On 10/03/2024 12:35, Karel Balej wrote:
> > Dmitry Torokhov, 2024-03-04T17:10:59-08:00:
> >> On Mon, Mar 04, 2024 at 09:28:45PM +0100, Karel Balej wrote:
> >>> Dmitry,
> >>>
> >>> Dmitry Torokhov, 2024-03-03T12:39:46-08:00:
> >>>> On Sun, Mar 03, 2024 at 11:04:25AM +0100, Karel Balej wrote:
> >>>>> From: Karel Balej <balejk@matfyz.cz>
> >>>>>
> >>>>> Marvell 88PM886 PMIC provides onkey among other things. Add client
> >>>>> driver to handle it. The driver currently only provides a basic support
> >>>>> omitting additional functions found in the vendor version, such as long
> >>>>> onkey and GPIO integration.
> >>>>>
> >>>>> Signed-off-by: Karel Balej <balejk@matfyz.cz>
> >>>>> ---
> >>>>>
> >>>>> Notes:
> >>>>> RFC v3:
> >>>>> - Drop wakeup-source.
> >>>>> RFC v2:
> >>>>> - Address Dmitry's feedback:
> >>>>> - Sort includes alphabetically.
> >>>>> - Drop onkey->irq.
> >>>>> - ret -> err in irq_handler and no initialization.
> >>>>> - Break long lines and other formatting.
> >>>>> - Do not clobber platform_get_irq error.
> >>>>> - Do not set device parent manually.
> >>>>> - Use input_set_capability.
> >>>>> - Use the wakeup-source DT property.
> >>>>> - Drop of_match_table.
> >>>>
> >>>> I only said that you should not be using of_match_ptr(), but you still
> >>>> need to have of_match_table set and have MODULE_DEVICE_TABLE() for the
> >>>> proper module loading support.
> >>>
> >>> I removed of_match_table because I no longer need compatible for this --
> >>> there are no device tree properties and the driver is being instantiated
> >>> by the MFD driver.
> >>>
> >>> Is the MODULE_DEVICE_TABLE() entry needed for the driver to probe when
> >>> compiled as module? If that is the case, given what I write above, am I
> >>> correct that MODULE_DEVICE_TABLE(platform,...) would be the right thing
> >>> to use here?
> >>
> >> Yes, if uevent generated for the device is "platform:<name>" then
> >> MODULE_DEVICE_TABLE(platform,...) will suffice. I am not sure how MFD
> >> sets it up (OF modalias or platform), but you should be able to check
> >> the format looking at the "uevent" attribute for your device in sysfs
> >> (/sys/devices/bus/platform/...).
> >
> > The uevent is indeed platform.
> >
> > But since there is only one device, perhaps having a device table is
> > superfluous and using `MODULE_ALIAS("platform:88pm886-onkey")` is more
> > fitting?
>
> Adding aliases for standard IDs and standard cases is almost never
> correct. If you need module alias, it means your ID table is wrong (or
> missing, which is usually wrong).
>
> >
> > Although I don't understand why this is even necessary when the driver
> > name is such and the module is registered using
> > `module_platform_driver`...
>
> ID table and MODULE_DEVICE_TABLE() are necessary for modprobe to work.
> Just run `modinfo`.
MODULE_DEVICE_TABLE() and MODULE_ALIAS() reduce to the same thing, but I
agree that we should not try to be too clever and simply use the ID
table.
Thanks.
--
Dmitry
^ permalink raw reply
* [PATCH v3] HID: nintendo: use ida for LED player id
From: Martino Fontana @ 2024-03-10 22:12 UTC (permalink / raw)
To: djogorchock, jikos, benjamin.tissoires, linux-input, linux-kernel
Cc: Martino Fontana, Ryan McClelland
Previously, the leds pattern would just increment with every controller
connected. This wouldn't take into consideration when controllers are
disconnected. The same controller could be connected and disconnected
with the pattern increasing player count each time.
This patch changes it by using an ID allocator in order to assign the
player id, the same way hid-playstation does.
Signed-off-by: Martino Fontana <tinozzo123@gmail.com>
Signed-off-by: Ryan McClelland <rymcclel@gmail.com>
---
Changes for v2:
ida_free now frees the correct id, instead of an id that got moduloed.
Changes for v3:
Destroy allocator when removing driver.
drivers/hid/hid-nintendo.c | 41 ++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 997c3a1adaca..8a8ae7881469 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -28,6 +28,7 @@
#include <linux/device.h>
#include <linux/kernel.h>
#include <linux/hid.h>
+#include <linux/idr.h>
#include <linux/input.h>
#include <linux/jiffies.h>
#include <linux/leds.h>
@@ -427,6 +428,7 @@ static const enum led_brightness joycon_player_led_patterns[JC_NUM_LED_PATTERNS]
struct joycon_ctlr {
struct hid_device *hdev;
struct input_dev *input;
+ u32 player_id;
struct led_classdev leds[JC_NUM_LEDS]; /* player leds */
struct led_classdev home_led;
enum joycon_ctlr_state ctlr_state;
@@ -1930,7 +1932,8 @@ static int joycon_home_led_brightness_set(struct led_classdev *led,
return ret;
}
-static DEFINE_SPINLOCK(joycon_input_num_spinlock);
+static DEFINE_IDA(nintendo_player_id_allocator);
+
static int joycon_leds_create(struct joycon_ctlr *ctlr)
{
struct hid_device *hdev = ctlr->hdev;
@@ -1941,20 +1944,19 @@ static int joycon_leds_create(struct joycon_ctlr *ctlr)
char *name;
int ret;
int i;
- unsigned long flags;
int player_led_pattern;
- static int input_num;
-
- /*
- * Set the player leds based on controller number
- * Because there is no standard concept of "player number", the pattern
- * number will simply increase by 1 every time a controller is connected.
- */
- spin_lock_irqsave(&joycon_input_num_spinlock, flags);
- player_led_pattern = input_num++ % JC_NUM_LED_PATTERNS;
- spin_unlock_irqrestore(&joycon_input_num_spinlock, flags);
/* configure the player LEDs */
+ ctlr->player_id = U32_MAX;
+ ret = ida_alloc(&nintendo_player_id_allocator, GFP_KERNEL);
+ if (ret < 0) {
+ hid_warn(hdev, "Failed to allocate player ID, skipping; ret=%d\n", ret);
+ goto home_led;
+ }
+ ctlr->player_id = ret;
+ player_led_pattern = ret % JC_NUM_LED_PATTERNS;
+ hid_info(ctlr->hdev, "assigned player %d led pattern", player_led_pattern + 1);
+
for (i = 0; i < JC_NUM_LEDS; i++) {
name = devm_kasprintf(dev, GFP_KERNEL, "%s:%s:%s",
d_name,
@@ -2423,6 +2425,7 @@ static void nintendo_hid_remove(struct hid_device *hdev)
spin_unlock_irqrestore(&ctlr->lock, flags);
destroy_workqueue(ctlr->rumble_queue);
+ ida_free(&nintendo_player_id_allocator, ctlr->player_id);
hid_hw_close(hdev);
hid_hw_stop(hdev);
@@ -2468,7 +2471,19 @@ static struct hid_driver nintendo_hid_driver = {
.resume = nintendo_hid_resume,
#endif
};
-module_hid_driver(nintendo_hid_driver);
+static int __init nintendo_init(void)
+{
+ return hid_register_driver(&nintendo_hid_driver);
+}
+
+static void __exit nintendo_exit(void)
+{
+ hid_unregister_driver(&nintendo_hid_driver);
+ ida_destroy(&nintendo_player_id_allocator);
+}
+
+module_init(nintendo_init);
+module_exit(nintendo_exit);
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Daniel J. Ogorchock <djogorchock@gmail.com>");
--
2.43.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