* [syzbot] [serial?] KASAN: slab-use-after-free Read in stop_tty
From: syzbot @ 2026-06-01 19:31 UTC (permalink / raw)
To: gregkh, jirislaby, linux-kernel, linux-serial, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: eb3f4b7426cf Merge tag 'nfsd-7.1-2' of git://git.kernel.or..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=110a02b6580000
kernel config: https://syzkaller.appspot.com/x/.config?x=bd38685893011045
dashboard link: https://syzkaller.appspot.com/bug?extid=2932e8970a6398db95c3
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/3e8630fd2e70/disk-eb3f4b74.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/3a23f2a3eb2a/vmlinux-eb3f4b74.xz
kernel image: https://storage.googleapis.com/syzbot-assets/e102c22c34c3/bzImage-eb3f4b74.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+2932e8970a6398db95c3@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: slab-use-after-free in stop_t[ 93.746644][ T5837] BUG: KASAN: slab-use-after-free in __stop_tty drivers/tty/tty_io.c:742 [inline]
BUG: KASAN: slab-use-after-free in stop_t[ 93.746644][ T5837] BUG: KASAN: slab-use-after-free in stop_tty+0x115/0x140 drivers/tty/tty_io.c:766
Read of size 1 at addr ffff888037076480 by task syz.1.2/5837
CPU: 1 UID: 0 PID: 5837 Comm: syz.1.2 Not tainted syzkaller #0 PREEMPT_{RT,(full)}
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
print_address_description+0x55/0x1e0 mm/kasan/report.c:378
print_report+0x58/0x70 mm/kasan/report.c:482
kasan_report+0x117/0x150 mm/kasan/report.c:595
__stop_tty drivers/tty/tty_io.c:742 [inline]
stop_tty+0x115/0x140 drivers/tty/tty_io.c:766
kbd_keycode drivers/tty/vt/keyboard.c:1565 [inline]
kbd_event+0x2f91/0x42f0 drivers/tty/vt/keyboard.c:1583
input_handle_events_default+0xd4/0x1a0 drivers/input/input.c:2558
input_pass_values+0x288/0x890 drivers/input/input.c:128
input_event_dispose+0x330/0x6b0 drivers/input/input.c:342
input_inject_event+0x1d7/0x320 drivers/input/input.c:424
evdev_write+0x328/0x4c0 drivers/input/evdev.c:528
vfs_write+0x2a3/0xba0 fs/read_write.c:686
ksys_write+0x156/0x270 fs/read_write.c:740
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fdc7927ce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fdc7748c028 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007fdc794f6180 RCX: 00007fdc7927ce59
RDX: 0000000000002250 RSI: 0000200000000040 RDI: 0000000000000008
RBP: 00007fdc79312d6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fdc794f6218 R14: 00007fdc794f6180 R15: 00007fff3e71db88
</TASK>
Allocated by task 5835:
kasan_save_stack mm/kasan/common.c:57 [inline]
kasan_save_track+0x3e/0x80 mm/kasan/common.c:78
poison_kmalloc_redzone mm/kasan/common.c:398 [inline]
__kasan_kmalloc+0x93/0xb0 mm/kasan/common.c:415
kasan_kmalloc include/linux/kasan.h:263 [inline]
__kmalloc_cache_noprof+0x3a6/0x690 mm/slub.c:5420
kmalloc_noprof include/linux/slab.h:950 [inline]
kzalloc_noprof include/linux/slab.h:1188 [inline]
alloc_tty_struct+0xa6/0x7b0 drivers/tty/tty_io.c:3102
tty_init_dev+0x59/0x4d0 drivers/tty/tty_io.c:1400
tty_open_by_driver drivers/tty/tty_io.c:2073 [inline]
tty_open+0x86e/0xd80 drivers/tty/tty_io.c:2120
chrdev_open+0x4d0/0x5f0 fs/char_dev.c:411
do_dentry_open+0x83d/0x13e0 fs/open.c:947
vfs_open+0x3b/0x350 fs/open.c:1079
do_open fs/namei.c:4699 [inline]
path_openat+0x2e43/0x38a0 fs/namei.c:4858
do_file_open+0x23e/0x4a0 fs/namei.c:4887
do_sys_openat2+0x113/0x200 fs/open.c:1364
do_sys_open fs/open.c:1370 [inline]
__do_sys_openat fs/open.c:1386 [inline]
__se_sys_openat fs/open.c:1381 [inline]
__x64_sys_openat+0x138/0x170 fs/open.c:1381
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Freed by task 5800:
kasan_save_stack mm/kasan/common.c:57 [inline]
kasan_save_track+0x3e/0x80 mm/kasan/common.c:78
kasan_save_free_info+0x46/0x50 mm/kasan/generic.c:584
poison_slab_object mm/kasan/common.c:253 [inline]
__kasan_slab_free+0x5c/0x80 mm/kasan/common.c:285
kasan_slab_free include/linux/kasan.h:235 [inline]
slab_free_hook mm/slub.c:2689 [inline]
slab_free mm/slub.c:6251 [inline]
kfree+0x1c5/0x6c0 mm/slub.c:6566
process_one_work kernel/workqueue.c:3314 [inline]
process_scheduled_works+0xb5d/0x1860 kernel/workqueue.c:3397
worker_thread+0xa53/0xfc0 kernel/workqueue.c:3478
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
Last potentially related work creation:
kasan_save_stack+0x3e/0x60 mm/kasan/common.c:57
kasan_record_aux_stack+0xbd/0xd0 mm/kasan/generic.c:556
insert_work+0x3d/0x330 kernel/workqueue.c:2226
__queue_work+0xd1d/0x1090 kernel/workqueue.c:2393
queue_work_on+0x106/0x1d0 kernel/workqueue.c:2444
tty_release_struct+0xb8/0xd0 drivers/tty/tty_io.c:1692
tty_release+0xcb6/0x1710 drivers/tty/tty_io.c:1852
__fput+0x461/0xa70 fs/file_table.c:510
task_work_run+0x1d9/0x270 kernel/task_work.c:233
resume_user_mode_work include/linux/resume_user_mode.h:50 [inline]
__exit_to_user_mode_loop kernel/entry/common.c:67 [inline]
exit_to_user_mode_loop+0x193/0x680 kernel/entry/common.c:98
__exit_to_user_mode_prepare include/linux/irq-entry-common.h:207 [inline]
syscall_exit_to_user_mode_prepare include/linux/irq-entry-common.h:230 [inline]
syscall_exit_to_user_mode include/linux/entry-common.h:318 [inline]
do_syscall_64+0x353/0x580 arch/x86/entry/syscall_64.c:100
entry_SYSCALL_64_after_hwframe+0x77/0x7f
The buggy address belongs to the object at ffff888037076000
which belongs to the cache kmalloc-cg-2k of size 2048
The buggy address is located 1152 bytes inside of
freed 2048-byte region [ffff888037076000, ffff888037076800)
The buggy address belongs to the physical page:
page: refcount:0 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x37070
head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
memcg:ffff888037070811
flags: 0x80000000000040(head|node=0|zone=1)
page_type: f5(slab)
raw: 0080000000000040 ffff88813fe263c0 dead000000000100 dead000000000122
raw: 0000000000000000 0000100000080008 00000000f5000000 ffff888037070811
head: 0080000000000040 ffff88813fe263c0 dead000000000100 dead000000000122
head: 0000000000000000 0000100000080008 00000000f5000000 ffff888037070811
head: 0080000000000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000008
page dumped because: kasan: bad access detected
page_owner tracks the page as allocated
page last allocated via order 3, migratetype Unmovable, gfp_mask 0xd20c0(__GFP_IO|__GFP_FS|__GFP_NOWARN|__GFP_NORETRY|__GFP_COMP|__GFP_NOMEMALLOC), pid 5805, tgid 5803 (syz.0.1), ts 93327144778, free_ts 93319793659
set_page_owner include/linux/page_owner.h:32 [inline]
post_alloc_hook+0x22d/0x280 mm/page_alloc.c:1853
prep_new_page mm/page_alloc.c:1861 [inline]
get_page_from_freelist+0x28b2/0x2930 mm/page_alloc.c:3941
__alloc_frozen_pages_noprof+0x18d/0x380 mm/page_alloc.c:5221
alloc_slab_page mm/slub.c:3278 [inline]
allocate_slab+0x77/0x660 mm/slub.c:3467
new_slab mm/slub.c:3525 [inline]
refill_objects+0x33c/0x3d0 mm/slub.c:7272
refill_sheaf mm/slub.c:2816 [inline]
__pcs_replace_empty_main+0x373/0x720 mm/slub.c:4652
alloc_from_pcs mm/slub.c:4750 [inline]
slab_alloc_node mm/slub.c:4884 [inline]
__do_kmalloc_node mm/slub.c:5295 [inline]
__kmalloc_node_track_caller_noprof+0x60b/0x7e0 mm/slub.c:5408
kmemdup_noprof+0x2b/0x70 mm/util.c:138
kmemdup_noprof include/linux/fortify-string.h:763 [inline]
neigh_sysctl_register+0xae/0xa90 net/core/neighbour.c:3861
addrconf_sysctl_register+0xb3/0x1c0 net/ipv6/addrconf.c:7379
ipv6_add_dev+0xd64/0x1400 net/ipv6/addrconf.c:460
addrconf_notify+0x771/0x1050 net/ipv6/addrconf.c:3662
notifier_call_chain+0x1ad/0x3d0 kernel/notifier.c:85
call_netdevice_notifiers_extack net/core/dev.c:2287 [inline]
call_netdevice_notifiers net/core/dev.c:2301 [inline]
register_netdevice+0x18d5/0x1ed0 net/core/dev.c:11461
nsim_init_netdevsim drivers/net/netdevsim/netdev.c:1069 [inline]
nsim_create+0xbff/0x1170 drivers/net/netdevsim/netdev.c:1151
__nsim_dev_port_add+0x857/0xd30 drivers/net/netdevsim/dev.c:1509
page last free pid 5805 tgid 5803 stack trace:
reset_page_owner include/linux/page_owner.h:25 [inline]
__free_pages_prepare mm/page_alloc.c:1397 [inline]
__free_frozen_pages+0xfe5/0x10d0 mm/page_alloc.c:2938
__slab_free+0x252/0x2a0 mm/slub.c:5613
qlink_free mm/kasan/quarantine.c:163 [inline]
qlist_free_all+0x99/0x100 mm/kasan/quarantine.c:179
kasan_quarantine_reduce+0x148/0x160 mm/kasan/quarantine.c:286
__kasan_slab_alloc+0x22/0x80 mm/kasan/common.c:350
kasan_slab_alloc include/linux/kasan.h:253 [inline]
slab_post_alloc_hook mm/slub.c:4570 [inline]
slab_alloc_node mm/slub.c:4899 [inline]
__kmalloc_cache_noprof+0x338/0x690 mm/slub.c:5415
kmalloc_noprof include/linux/slab.h:950 [inline]
kzalloc_noprof include/linux/slab.h:1188 [inline]
kset_create lib/kobject.c:965 [inline]
kset_create_and_add+0x5a/0x180 lib/kobject.c:1008
register_queue_kobjects net/core/net-sysfs.c:2091 [inline]
netdev_register_kobject+0x1a2/0x310 net/core/net-sysfs.c:2347
register_netdevice+0x146d/0x1ed0 net/core/dev.c:11423
nsim_init_netdevsim drivers/net/netdevsim/netdev.c:1069 [inline]
nsim_create+0xbff/0x1170 drivers/net/netdevsim/netdev.c:1151
__nsim_dev_port_add+0x857/0xd30 drivers/net/netdevsim/dev.c:1509
nsim_dev_port_add_all+0x37/0xf0 drivers/net/netdevsim/dev.c:1570
nsim_dev_reload_create drivers/net/netdevsim/dev.c:1622 [inline]
nsim_dev_reload_up+0x45f/0x7c0 drivers/net/netdevsim/dev.c:1058
devlink_reload+0x501/0x8d0 net/devlink/dev.c:475
devlink_nl_reload_doit+0xaaa/0xc80 net/devlink/dev.c:585
genl_family_rcv_msg_doit+0x22a/0x330 net/netlink/genetlink.c:1114
Memory state around the buggy address:
ffff888037076380: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888037076400: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff888037076480: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff888037076500: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff888037076580: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
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 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
* [PATCH v2 3/3] serial: max310x: honour rs485 properties from per-channel DT subnode
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
In-Reply-To: <20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com>
The MAX310x DT binding pulls in /schemas/serial/rs485.yaml via its allOf
list, advertising the rs485-* properties defined there - none of which
were honoured at runtime, because the driver never called
uart_get_rs485_mode().
All channels share the parent SPI/I2C device, so uart_get_rs485_mode()
called directly on each port would read the same chip-level fwnode for
every call. Walk dev->of_node's children for the "serial@N" subnode
with matching reg, and temporarily retarget the parent device's fwnode
while uart_get_rs485_mode() runs, so each channel picks up its own
subnode's properties. Probe is serialised, so the swap is safe.
For single-channel variants (max3107, max3108), fall back to the chip's
own fwnode when no subnode is present, so existing DTs that declare
rs485 properties at the top level keep working.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
drivers/tty/serial/max310x.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index 5cb7d01e404663dc25b88bc7b4f8df61be2135ec..aee3b75fff000385a7543f099663c8a0e4a7d014 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1426,6 +1426,9 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
#endif
for (i = 0; i < devtype->nr; i++) {
+ struct fwnode_handle *saved_fwnode = dev_fwnode(dev);
+ struct device_node *port_np = NULL;
+ struct device_node *child;
unsigned int line;
line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
@@ -1435,6 +1438,40 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
}
s->p[i].port.line = line;
+ /* Locate the matching "serial@i" DT subnode, if any. */
+ for_each_available_child_of_node(dev->of_node, child) {
+ u32 reg;
+
+ if (!of_node_name_eq(child, "serial"))
+ continue;
+ if (of_property_read_u32(child, "reg", ®))
+ continue;
+ if (reg == i) {
+ port_np = child;
+ break;
+ }
+ }
+
+ /*
+ * Temporarily retarget dev's fwnode to the per-port subnode
+ * so uart_get_rs485_mode() picks up the per-port properties.
+ * For single-port variants, fall back to the chip's own
+ * fwnode so legacy DTs that declare rs485 properties at the
+ * top level keep working.
+ */
+ if (port_np) {
+ device_set_node(dev, of_fwnode_handle(port_np));
+ ret = uart_get_rs485_mode(&s->p[i].port);
+ device_set_node(dev, saved_fwnode);
+ of_node_put(port_np);
+ if (ret)
+ goto out_uart;
+ } else if (devtype->nr == 1) {
+ ret = uart_get_rs485_mode(&s->p[i].port);
+ if (ret)
+ goto out_uart;
+ }
+
/* Register port */
ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
if (ret)
--
2.47.3
^ permalink raw reply related
* [PATCH v2 2/3] dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
In-Reply-To: <20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com>
The MAX310x is a family of one- (max3107, max3108), two- (max3109) and
four-channel (max14830) UARTs. The binding pulls in
/schemas/serial/rs485.yaml at the chip level, describing a single set of
RS-485 properties - enough for the single-channel parts, but a
multi-channel chip can wire RS-485 differently on each channel.
Describe each channel as a "serial@N" subnode. Being a serial node (per
serial.yaml) the channel carries the standard rs485.yaml properties and
may also host a serial slave device. Constrain the channels per
compatible: max3107 and max3108 have none and keep RS-485 on the chip
node, max3109 has channels 0-1, and max14830 has 0-3. Chip-level rs485
properties remain accepted and are the only form for the single-channel
parts.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
.../devicetree/bindings/serial/maxim,max310x.yaml | 90 ++++++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
index 889eeaca64a027b4d9e8ec87bcf63fcc8fd9d55b..988864e7957416caea2d86c38957a894ce57c6fb 100644
--- a/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
+++ b/Documentation/devicetree/bindings/serial/maxim,max310x.yaml
@@ -40,6 +40,36 @@ properties:
minItems: 1
maxItems: 16
+ "#address-cells":
+ const: 1
+
+ "#size-cells":
+ const: 0
+
+patternProperties:
+ "^serial@[0-3]$":
+ type: object
+ description:
+ A single UART channel of a multi-channel variant. Describe the
+ channel's RS-485 wiring here with the standard properties from
+ /schemas/serial/rs485.yaml#; being a serial node, the channel may
+ also host a serial slave device. Single-channel variants have no
+ such subnode - their settings stay on the chip node.
+
+ allOf:
+ - $ref: /schemas/serial/serial.yaml#
+ - $ref: /schemas/serial/rs485.yaml#
+
+ properties:
+ reg:
+ description: UART channel number on the chip.
+ maximum: 3
+
+ required:
+ - reg
+
+ unevaluatedProperties: false
+
required:
- compatible
- reg
@@ -52,6 +82,32 @@ allOf:
- $ref: /schemas/serial/serial.yaml#
- $ref: /schemas/serial/rs485.yaml#
+ # max3107 and max3108 are single-channel parts: there are no
+ # addressable channel subnodes, so RS-485 stays on the chip node.
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - maxim,max3107
+ - maxim,max3108
+ then:
+ properties:
+ "#address-cells": false
+ "#size-cells": false
+ patternProperties:
+ "^serial@[0-3]$": false
+
+ # max3109 has two UART channels: 0 and 1.
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: maxim,max3109
+ then:
+ patternProperties:
+ "^serial@[23]$": false
+
unevaluatedProperties: false
examples:
@@ -70,5 +126,39 @@ examples:
interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
gpio-controller;
#gpio-cells = <2>;
+ rs485-rts-active-low;
+ linux,rs485-enabled-at-boot-time;
+ };
+ };
+
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ serial@0 {
+ compatible = "maxim,max14830";
+ reg = <0>;
+ spi-max-frequency = <26000000>;
+ clocks = <&xtal4m>;
+ clock-names = "xtal";
+ interrupt-parent = <&gpio3>;
+ interrupts = <7 IRQ_TYPE_LEVEL_LOW>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ serial@0 {
+ reg = <0>;
+ rs485-rts-active-low;
+ linux,rs485-enabled-at-boot-time;
+ };
+
+ serial@2 {
+ reg = <2>;
+ rs485-rts-active-low;
+ };
};
};
--
2.47.3
^ permalink raw reply related
* [PATCH v2 1/3] serial: max310x: register GPIO controller before adding UART ports
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
In-Reply-To: <20260601-b4-max310x-rs485-dt-v2-0-a105105f8e70@vaisala.com>
The MAX310x exposes four GPIOs per UART port via an in-driver
gpio_chip. devm_gpiochip_add_data() used to run after the per-port
uart_add_one_port() loop, so a device-tree consumer referencing one of
the chip's own GPIOs (for example rs485-term-gpios = <&max310x 0 ...>)
could not resolve it during port registration: the GPIO provider it
waits for is the very driver still trying to register, and the lookup
returns -EPROBE_DEFER on its own provider, deferring probe forever.
Split the per-port setup into two passes around the gpio_chip
registration:
1. Initialise per-port state - port struct fields, regmap binding,
IRQ disable, work queues. The gpio_chip callbacks dereference
s->p[i].regmap via to_max310x_port() and become callable as soon
as the chip is visible to gpiolib, so every entry must be
populated first.
2. devm_gpiochip_add_data() - register the gpio_chip.
3. Allocate a line, uart_add_one_port(), set_bit(), max310x_power().
Keeping line allocation, registration and set_bit() together
preserves the existing "bit set <=> port registered" rollback
invariant that out_uart relies on.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
drivers/tty/serial/max310x.c | 54 +++++++++++++++++++++++++++-----------------
1 file changed, 33 insertions(+), 21 deletions(-)
diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
index ac7d3f197c3a5ce3531d5607f48e21a807314021..5cb7d01e404663dc25b88bc7b4f8df61be2135ec 100644
--- a/drivers/tty/serial/max310x.c
+++ b/drivers/tty/serial/max310x.c
@@ -1364,17 +1364,12 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
dev_dbg(dev, "Reference clock set to %i Hz\n", uartclk);
+ /*
+ * Set up each port's state before registering the gpiochip,
+ * since the gpiochip callbacks will read s->p[i].regmap as
+ * soon as gpiolib exposes the controller.
+ */
for (i = 0; i < devtype->nr; i++) {
- unsigned int line;
-
- line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
- if (line == MAX310X_UART_NRMAX) {
- ret = -ERANGE;
- goto out_uart;
- }
-
- /* Initialize port data */
- s->p[i].port.line = line;
s->p[i].port.dev = dev;
s->p[i].port.irq = irq;
s->p[i].port.type = PORT_MAX310X;
@@ -1404,20 +1399,16 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
INIT_WORK(&s->p[i].md_work, max310x_md_proc);
/* Initialize queue for changing RS485 mode */
INIT_WORK(&s->p[i].rs_work, max310x_rs_proc);
-
- /* Register port */
- ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
- if (ret)
- goto out_uart;
-
- set_bit(line, max310x_lines);
-
- /* Go to suspend mode */
- max310x_power(&s->p[i].port, 0);
}
#ifdef CONFIG_GPIOLIB
- /* Setup GPIO controller */
+ /*
+ * Register the GPIO controller before adding the UART ports so
+ * that consumers referencing the chip's own GPIOs from device
+ * tree (for example rs485-term-gpios = <&max310x ...>) can
+ * resolve them at uart_add_one_port() time instead of receiving
+ * -EPROBE_DEFER from their own provider.
+ */
s->gpio.owner = THIS_MODULE;
s->gpio.parent = dev;
s->gpio.label = devtype->name;
@@ -1434,6 +1425,27 @@ static int max310x_probe(struct device *dev, const struct max310x_devtype *devty
goto out_uart;
#endif
+ for (i = 0; i < devtype->nr; i++) {
+ unsigned int line;
+
+ line = find_first_zero_bit(max310x_lines, MAX310X_UART_NRMAX);
+ if (line == MAX310X_UART_NRMAX) {
+ ret = -ERANGE;
+ goto out_uart;
+ }
+ s->p[i].port.line = line;
+
+ /* Register port */
+ ret = uart_add_one_port(&max310x_uart, &s->p[i].port);
+ if (ret)
+ goto out_uart;
+
+ set_bit(line, max310x_lines);
+
+ /* Go to suspend mode */
+ max310x_power(&s->p[i].port, 0);
+ }
+
/* Setup interrupt */
ret = devm_request_threaded_irq(dev, irq, NULL, max310x_ist,
IRQF_ONESHOT | IRQF_SHARED, dev_name(dev), s);
--
2.47.3
^ permalink raw reply related
* [PATCH v2 0/3] serial: max310x: honour per-channel DT RS485 properties
From: Tapio Reijonen @ 2026-06-01 10:25 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve
Cc: linux-kernel, linux-serial, devicetree, Tapio Reijonen
The MAX310x DT binding pulls in /schemas/serial/rs485.yaml via its allOf
list, but the driver has never actually called uart_get_rs485_mode(), so
none of the advertised rs485-* properties take effect at runtime.
This series wires per-channel RS485 DT configuration end to end:
Patch 1 reorders the probe so the gpio_chip is registered before
uart_add_one_port(). A port can then reference one of the chip's own
GPIOs (e.g. rs485-term-gpios = <&max310x ...>) without -EPROBE_DEFER
from its own provider - prerequisite for patch 3.
Patch 2 describes each UART channel as a "serial@N" subnode carrying the
standard rs485.yaml properties. The channels are constrained per
compatible: the single-channel max3107/max3108 have none (RS485 stays on
the chip node), max3109 has channels 0-1 and max14830 has 0-3. Being a
serial node, a channel may also host a serial slave device.
Patch 3 reads each channel's RS485 properties from its own subnode by
temporarily retargeting dev->fwnode while uart_get_rs485_mode() runs.
For single-channel variants, falls back to the chip's own fwnode when no
subnode is present, so existing top-level rs485 DTs keep working.
Note for maintainers: patch 3 mutates the parent SPI/I2C device's
fwnode around the uart_get_rs485_mode() call so the underlying
property/GPIO lookups resolve against the per-channel DT subnode. Probe
is serialised, so the swap is locally safe, but I'd appreciate feedback
on whether this idiom is acceptable. If a cleaner shape is preferred (a
serial_core helper that takes a fwnode directly, or one struct device
per port), I'll respin accordingly.
Tested on max14830 (SPI, 4 ports): each ttyMAXn port comes up with the
rs485 flags and delays configured in its serial@N subnode, and the
termination GPIO sourced from the MAX310x's own gpio_chip is resolved
without probe deferral.
Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
---
Changes in v2:
- dt-bindings: rename the per-port subnode "port@N" -> "serial@N" so each
channel is a proper serial node (serial.yaml) that can also host a
serial slave device; "port" is reserved for the graph binding. (Krzysztof)
- dt-bindings: constrain channels per compatible - max3107/max3108 take no
subnodes (nor #address-cells/#size-cells), max3109 allows 0-1, max14830
allows 0-3; out-of-range channels now fail dt_binding_check. (Krzysztof)
- serial: max310x: match the "serial" child node name accordingly.
- No change to patch 1; patch 3 still reads rs485 via the temporary fwnode
retarget (idiom flagged above).
- Link to v1: https://lore.kernel.org/r/20260525-b4-max310x-rs485-dt-v1-0-e6c19b4d5592@vaisala.com
---
Tapio Reijonen (3):
serial: max310x: register GPIO controller before adding UART ports
dt-bindings: serial: maxim,max310x: describe per-channel rs485 subnodes
serial: max310x: honour rs485 properties from per-channel DT subnode
.../devicetree/bindings/serial/maxim,max310x.yaml | 90 +++++++++++++++++++++
drivers/tty/serial/max310x.c | 91 +++++++++++++++++-----
2 files changed, 160 insertions(+), 21 deletions(-)
---
base-commit: 79bd2dded182b1d458b18e62684b7f82ffc682e5
change-id: 20260525-b4-max310x-rs485-dt-ebff12af9976
Best regards,
--
Tapio Reijonen <tapio.reijonen@vaisala.com>
^ permalink raw reply
* Re: [PATCH 2/3] dt-bindings: serial: maxim,max310x: allow per-port subnodes for rs485
From: Tapio Reijonen @ 2026-06-01 5:30 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Greg Kroah-Hartman, Jiri Slaby, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Hugo Villeneuve, linux-kernel, linux-serial,
devicetree
In-Reply-To: <20260530-witty-snail-of-correction-a0a6f3@quoll>
Hi,
On 5/30/26 14:26, Krzysztof Kozlowski wrote:
> On Mon, May 25, 2026 at 09:43:38AM +0000, Tapio Reijonen wrote:
>> The MAX310x is a multi-port UART (up to four ports). The existing
>> binding pulls in /schemas/serial/rs485.yaml at the top level, which
>> only describes a single port - sufficient for max3107 but ambiguous
>> for max14830 where each port can have its own RS485 wiring.
>>
>> Add a "port@N" pattern (N = 0..3) carrying rs485 properties on a
>> per-port basis. When port@N subnodes are present, the chip node also
>> needs #address-cells = <1> and #size-cells = <0>; allow both. Top-
>> level rs485 properties remain accepted for compatibility.
>>
>> Signed-off-by: Tapio Reijonen <tapio.reijonen@vaisala.com>
>> ---
>> .../devicetree/bindings/serial/maxim,max310x.yaml | 60 ++++++++++++++++++++++
>> 1 file changed, 60 insertions(+)
>
> That's a total mess now in the binding.
> 1. maxim,max3107 does not have ports, but you add there. You need to
> constrain (see writing bindings) or split.
You're right, the port count is part of the ABI and I left it
unconstrained. In v2 I'll constrain the subnodes per-compatible with
allOf/if-then in the one file: max3107 and max3108 are single-port and
take only the top-level rs485 properties (no subnodes); max3109 allows
indices 0-1 and max14830 allows 0-3. An out-of-range port will then
fail validation instead of being silently accepted.
> 2. So where do you place serial devices? Did you validate any of this?
Good point, and it made me reconsider the node name. I'll rename the
subnodes from "port@N" to "serial@N" so each channel is a proper serial
node per serial.yaml -- that matches the device class, frees the "port"
name reserved for the graph binding, and gives an attached serial slave
device a place to live under the correct channel, which "port@N" did
not. The driver match in patch 3 changes to of_node_name_eq(np,
"serial") accordingly.
I also looked at whether subnodes are the right shape at all, since the
sibling sc16is7xx (dual-UART, same driver author) does per-port config
with top-level index arrays (irda-mode-ports,
nxp,modem-control-line-ports) rather than child nodes. That idiom works
for simple per-port flags, but it doesn't extend to RS485: rs485.yaml is
a rich standard schema (several flags, the rts delays, rs485-term-gpios)
and flattening it into vendor index-arrays would mean redefining common
properties instead of referencing them. So a per-channel node that
$refs rs485.yaml seems the only clean carrier; I just had the node name
and the per-compatible constraints wrong.
On validation: the schema and the example do pass
make dt_binding_check DT_SCHEMA_FILES=.../serial/maxim,max310x.yaml
and the series was tested on max14830 hardware with per-port rs485 and
termination GPIOs sourced from the chip's own gpio-controller. But I
take the structural point -- passing the check didn't make the layout
right. I'll respin patch 2 (constraints + serial@N) and the patch 3
match for v2, with an example that exercises a multi-port variant.
>
> Best regards,
> Krzysztof
>
Thanks for the review.
Tapio
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Markus Probst @ 2026-06-01 0:13 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Markus Probst via B4 Relay, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Kari Argillander, Rafael J. Wysocki, Viresh Kumar, Boqun Feng,
David Airlie, Simona Vetter, linux-serial, linux-kernel,
rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <DIX7W3C7F1UZ.3P1LHOQSD5VMR@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2487 bytes --]
On Mon, 2026-06-01 at 00:32 +0200, Danilo Krummrich wrote:
> On Mon Jun 1, 2026 at 12:00 AM CEST, Markus Probst wrote:
> > On Sun, 2026-05-31 at 23:49 +0200, Danilo Krummrich wrote:
> > > On Sun May 31, 2026 at 9:42 PM CEST, Markus Probst wrote:
> > > > I just noticed, is it even possible to use SRCU here? Currently the mutex not
> > > > only ensures that no drvdata access happens after drvdata drop, but also that
> > > > the receive_buf waits for the probe to complete, as the drvdata hasn't been
> > > > initialized yet.
> > >
> > > Yeah, if you drop the completion, you need the mutex.
> > Is the performance impact on an mutex or on srcu + completion higher?
>
> Weighing in the completion, the mutex probably wins as it will always be
> uncontested under normal operation.
>
> > >
> > > (In case it wasn't discussed in previous versions already, there is also the
> > > option to just attach separate private data to the receive callback, which would
> > > avoid this synchonization problem in the first place.
> > >
> > > You could have serdev::Device<Core>::open(), which takes its own private data
> > > and a corresponding close(), this way you'd allow drivers to control whether
> > > they want the serial line "open" or not. You just need to make sure it is closed
> > > eventually.)
> > This would add complexity with types, as we need to ensure that write,
> > set_baudrate, set_parity etc. does not run when closed.
>
> Right, but looking at a few serdev drivers, there seem to be a few cases where
> drivers need to close and re-open.
That is something we could consider once there is a user.
I am certain it would make it more complex, not less.
>
> > > That said, I don't know what turns out to be the better approach. And maybe it
> > > simply isn't something this initial series has to tackle? I think your driver
> > > does not implement the receive callback?
> > The initial driver with only leds: no.
> >
> > That changes once the driver also takes care of hwmon (ADC sensor and
> > fan failure) and input (power button and possibly other buttons).
>
> Maybe drop it from this initial series then and revisit with the first user?
I would prefer it with this series.
Except for probe and remove there should be no overhead if receive_buf
is not implemented.
As it will be needed eventually and it is basic serdev functionality, I
don't see a reason to not include it now.
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Danilo Krummrich @ 2026-05-31 22:32 UTC (permalink / raw)
To: Markus Probst
Cc: Markus Probst via B4 Relay, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Kari Argillander, Rafael J. Wysocki, Viresh Kumar, Boqun Feng,
David Airlie, Simona Vetter, linux-serial, linux-kernel,
rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <7c30276759aaba8127275e602cb78783ec33bbe9.camel@posteo.de>
On Mon Jun 1, 2026 at 12:00 AM CEST, Markus Probst wrote:
> On Sun, 2026-05-31 at 23:49 +0200, Danilo Krummrich wrote:
>> On Sun May 31, 2026 at 9:42 PM CEST, Markus Probst wrote:
>> > I just noticed, is it even possible to use SRCU here? Currently the mutex not
>> > only ensures that no drvdata access happens after drvdata drop, but also that
>> > the receive_buf waits for the probe to complete, as the drvdata hasn't been
>> > initialized yet.
>>
>> Yeah, if you drop the completion, you need the mutex.
> Is the performance impact on an mutex or on srcu + completion higher?
Weighing in the completion, the mutex probably wins as it will always be
uncontested under normal operation.
>>
>> (In case it wasn't discussed in previous versions already, there is also the
>> option to just attach separate private data to the receive callback, which would
>> avoid this synchonization problem in the first place.
>>
>> You could have serdev::Device<Core>::open(), which takes its own private data
>> and a corresponding close(), this way you'd allow drivers to control whether
>> they want the serial line "open" or not. You just need to make sure it is closed
>> eventually.)
> This would add complexity with types, as we need to ensure that write,
> set_baudrate, set_parity etc. does not run when closed.
Right, but looking at a few serdev drivers, there seem to be a few cases where
drivers need to close and re-open.
>> That said, I don't know what turns out to be the better approach. And maybe it
>> simply isn't something this initial series has to tackle? I think your driver
>> does not implement the receive callback?
> The initial driver with only leds: no.
>
> That changes once the driver also takes care of hwmon (ADC sensor and
> fan failure) and input (power button and possibly other buttons).
Maybe drop it from this initial series then and revisit with the first user?
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Markus Probst @ 2026-05-31 22:00 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Markus Probst via B4 Relay, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Kari Argillander, Rafael J. Wysocki, Viresh Kumar, Boqun Feng,
David Airlie, Simona Vetter, linux-serial, linux-kernel,
rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <DIX6YVWRDL67.3TXDMTAMASCVZ@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 1561 bytes --]
On Sun, 2026-05-31 at 23:49 +0200, Danilo Krummrich wrote:
> On Sun May 31, 2026 at 9:42 PM CEST, Markus Probst wrote:
> > I just noticed, is it even possible to use SRCU here? Currently the mutex not
> > only ensures that no drvdata access happens after drvdata drop, but also that
> > the receive_buf waits for the probe to complete, as the drvdata hasn't been
> > initialized yet.
>
> Yeah, if you drop the completion, you need the mutex.
Is the performance impact on an mutex or on srcu + completion higher?
>
> (In case it wasn't discussed in previous versions already, there is also the
> option to just attach separate private data to the receive callback, which would
> avoid this synchonization problem in the first place.
>
> You could have serdev::Device<Core>::open(), which takes its own private data
> and a corresponding close(), this way you'd allow drivers to control whether
> they want the serial line "open" or not. You just need to make sure it is closed
> eventually.)
This would add complexity with types, as we need to ensure that write,
set_baudrate, set_parity etc. does not run when closed.
>
> That said, I don't know what turns out to be the better approach. And maybe it
> simply isn't something this initial series has to tackle? I think your driver
> does not implement the receive callback?
The initial driver with only leds: no.
That changes once the driver also takes care of hwmon (ADC sensor and
fan failure) and input (power button and possibly other buttons).
Thanks
- Markus Probst
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Danilo Krummrich @ 2026-05-31 21:49 UTC (permalink / raw)
To: Markus Probst
Cc: Markus Probst via B4 Relay, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Kari Argillander, Rafael J. Wysocki, Viresh Kumar, Boqun Feng,
David Airlie, Simona Vetter, linux-serial, linux-kernel,
rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <12fa6237168ecf6dc65ebf46118beeb5c51f3d1c.camel@posteo.de>
On Sun May 31, 2026 at 9:42 PM CEST, Markus Probst wrote:
> I just noticed, is it even possible to use SRCU here? Currently the mutex not
> only ensures that no drvdata access happens after drvdata drop, but also that
> the receive_buf waits for the probe to complete, as the drvdata hasn't been
> initialized yet.
Yeah, if you drop the completion, you need the mutex.
(In case it wasn't discussed in previous versions already, there is also the
option to just attach separate private data to the receive callback, which would
avoid this synchonization problem in the first place.
You could have serdev::Device<Core>::open(), which takes its own private data
and a corresponding close(), this way you'd allow drivers to control whether
they want the serial line "open" or not. You just need to make sure it is closed
eventually.)
That said, I don't know what turns out to be the better approach. And maybe it
simply isn't something this initial series has to tackle? I think your driver
does not implement the receive callback?
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Markus Probst @ 2026-05-31 19:42 UTC (permalink / raw)
To: Danilo Krummrich, Markus Probst via B4 Relay
Cc: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Miguel Ojeda,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Kari Argillander, Rafael J. Wysocki,
Viresh Kumar, Boqun Feng, David Airlie, Simona Vetter,
linux-serial, linux-kernel, rust-for-linux, linux-pm, driver-core,
dri-devel
In-Reply-To: <DIWEXUCNLURG.136XXDBHSBNVR@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 3963 bytes --]
On Sun, 2026-05-31 at 01:51 +0200, Danilo Krummrich wrote:
> On Sun May 31, 2026 at 12:51 AM CEST, Markus Probst via B4 Relay wrote:
> > +#[pinned_drop]
> > +impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> > + fn drop(self: Pin<&mut Self>) {
> > + let mut active = self.active.lock();
> > + if *active {
> > + // SAFETY:
> > + // - We have exclusive access to `self.driver`.
> > + // - `self.driver` is guaranteed to be initialized.
> > + unsafe { (*self.driver.get()).assume_init_drop() };
> > + *active = false;
> > + }
> > +
> > + // SAFETY: We have exclusive access to `self.open`.
> > + if unsafe { *self.open.get() } {
> > + // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> > + // `struct serdev_device`.
> > + unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
> > + }
> > + }
> > +}
>
> Just in case it came across otherwise, I did not mean to push for you to go for
> this approach. We just kept discussing it because it let to the (to me more
> interesting) discussion around the LED class device abstraction.
>
> While this approach gets us rid of an extra allocation and the rust_private_data
> pointer in struct serdev_device it also turns out a bit more convoluted.
>
> That said, both are correct, and I won't object either one; up to you and the
> serdev / tty maintainers.
>
> Please wait a bit before resending, so other people can comment on this as well.
>
> > + extern "C" fn receive_buf_callback(
> > + sdev: *mut bindings::serdev_device,
> > + buf: *const u8,
> > + length: usize,
> > + ) -> usize {
> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
>
> CoreInternal snuck back in, should be BoundInternal.
>
> > +
> > + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > + let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > + let active = private_data.active.lock();
>
> I think SRCU would be a much better fit, but the code didn't land yet, so the
> mutex seems fine for now. But I'd probably add a TODO.
I just noticed, is it even possible to use SRCU here? Currently the
mutex not only ensures that no drvdata access happens after drvdata
drop, but also that the receive_buf waits for the probe to complete, as
the drvdata hasn't been initialized yet.
(drvdata refers to PrivateData.driver in this context, not the whole
PrivateData)
Thanks
- Markus Probst
>
> > +
> > + if !*active {
> > + return length;
> > + }
> > +
> > + // SAFETY: No one has exclusive access to `private_data.driver`.
> > + let data = unsafe { &*private_data.driver.get() };
> > + // SAFETY:
> > + // - `private_data.driver` is pinned.
> > + // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> > + // hence it's guaranteed that `private_data.driver` was initialized.
> > + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > +
> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > + T::receive(sdev, data_pinned, buf)
> > + }
> > +}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Danilo Krummrich @ 2026-05-31 17:57 UTC (permalink / raw)
To: Markus Probst
Cc: Onur Özkan, Markus Probst via B4 Relay, Rob Herring,
Greg Kroah-Hartman, Jiri Slaby, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Kari Argillander, Rafael J. Wysocki, Viresh Kumar,
Boqun Feng, David Airlie, Simona Vetter, linux-serial,
linux-kernel, rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <b6191edf3dc052fa86fae12fa5746d0c1c9c45ed.camel@posteo.de>
On Sun May 31, 2026 at 6:37 PM CEST, Markus Probst wrote:
> How close? Should I rebase this patch on top of the srcu patch series?
Since it isn't blocking you, it would only make sense if SRCU makes it this
cycle, but this series does not.
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Markus Probst @ 2026-05-31 16:37 UTC (permalink / raw)
To: Onur Özkan, Danilo Krummrich
Cc: Markus Probst via B4 Relay, Rob Herring, Greg Kroah-Hartman,
Jiri Slaby, Miguel Ojeda, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Kari Argillander, Rafael J. Wysocki, Viresh Kumar, Boqun Feng,
David Airlie, Simona Vetter, linux-serial, linux-kernel,
rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <20260531065838.7352-1-work@onurozkan.dev>
[-- Attachment #1: Type: text/plain, Size: 4132 bytes --]
On Sun, 2026-05-31 at 09:58 +0300, Onur Özkan wrote:
> On Sun, 31 May 2026 01:51:08 +0200
> Danilo Krummrich <dakr@kernel.org> wrote:
>
> > On Sun May 31, 2026 at 12:51 AM CEST, Markus Probst via B4 Relay wrote:
> > > +#[pinned_drop]
> > > +impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> > > + fn drop(self: Pin<&mut Self>) {
> > > + let mut active = self.active.lock();
> > > + if *active {
> > > + // SAFETY:
> > > + // - We have exclusive access to `self.driver`.
> > > + // - `self.driver` is guaranteed to be initialized.
> > > + unsafe { (*self.driver.get()).assume_init_drop() };
> > > + *active = false;
> > > + }
> > > +
> > > + // SAFETY: We have exclusive access to `self.open`.
> > > + if unsafe { *self.open.get() } {
> > > + // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> > > + // `struct serdev_device`.
> > > + unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
> > > + }
> > > + }
> > > +}
> >
> > Just in case it came across otherwise, I did not mean to push for you to go for
> > this approach. We just kept discussing it because it let to the (to me more
> > interesting) discussion around the LED class device abstraction.
> >
> > While this approach gets us rid of an extra allocation and the rust_private_data
> > pointer in struct serdev_device it also turns out a bit more convoluted.
> >
> > That said, both are correct, and I won't object either one; up to you and the
> > serdev / tty maintainers.
> >
> > Please wait a bit before resending, so other people can comment on this as well.
> >
> > > + extern "C" fn receive_buf_callback(
> > > + sdev: *mut bindings::serdev_device,
> > > + buf: *const u8,
> > > + length: usize,
> > > + ) -> usize {
> > > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > > + // pointer to a `struct serdev_device`.
> > > + //
> > > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
> >
> > CoreInternal snuck back in, should be BoundInternal.
> >
> > > +
> > > + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > > + // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > > + let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > > + let active = private_data.active.lock();
> >
> > I think SRCU would be a much better fit, but the code didn't land yet, so the
> > mutex seems fine for now. But I'd probably add a TODO.
> >
>
> Jfyi v9 is on the list [1] and I would say we are pretty close on taking that.
How close? Should I rebase this patch on top of the srcu patch series?
Thanks
- Markus Probst
>
> Thanks,
> Onur
>
> [1]: https://lore.kernel.org/rust-for-linux/DIWEXUCNLURG.136XXDBHSBNVR@kernel.org
>
> > > +
> > > + if !*active {
> > > + return length;
> > > + }
> > > +
> > > + // SAFETY: No one has exclusive access to `private_data.driver`.
> > > + let data = unsafe { &*private_data.driver.get() };
> > > + // SAFETY:
> > > + // - `private_data.driver` is pinned.
> > > + // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> > > + // hence it's guaranteed that `private_data.driver` was initialized.
> > > + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > > +
> > > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > > +
> > > + T::receive(sdev, data_pinned, buf)
> > > + }
> > > +}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 870 bytes --]
^ permalink raw reply
* Re: [PATCHv2] serial: mxs-auart: fix probe error paths and clock handling
From: Greg Kroah-Hartman @ 2026-05-31 8:12 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-serial, Jiri Slaby, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
open list:TTY LAYER AND SERIAL DRIVERS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAKxU2N8bNT+irM-gOk9SM5DHPbcbh7zvNM7n4wF5+q4TSEMkSA@mail.gmail.com>
On Sun, May 31, 2026 at 01:00:21AM -0700, Rosen Penev wrote:
> On Sun, May 31, 2026 at 12:52 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Sat, May 30, 2026 at 12:55:03PM -0700, Rosen Penev wrote:
> > > Sashiko reported three pre-existing bugs in the mxs-auart driver:
> > >
> > > - For non-ASM9260 variants, mxs_get_clks() obtained the clock but never
> > > prepared/enabled it, leaving register accesses in probe at risk of
> > > faulting if the bootloader had gated the clock.
> > > - The error path and remove function used pdev->id instead of
> > > s->port.line to clear the auart_port[] slot. For DT-probed devices
> > > pdev->id is -1, causing an out-of-bounds write and leaving a dangling
> > > pointer in the array.
> > > - The probe error path called iounmap() while the IRQ was still
> > > registered. An interrupt during that window would dereference the
> > > unmapped membase.
> >
> > This should still be broken up into smaller pieces, don't you think?
> >
> > Whenever you have to list the different things you do in a patch, that's
> > a huge hint to do so.
> I'll do so. Although I doubt the bot will be happy either way.
What bot? Yours? If so, please don't use it.
^ permalink raw reply
* Re: [PATCHv2] serial: mxs-auart: fix probe error paths and clock handling
From: Rosen Penev @ 2026-05-31 8:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: linux-serial, Jiri Slaby, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
open list:TTY LAYER AND SERIAL DRIVERS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <2026053127-nastily-fool-88eb@gregkh>
On Sun, May 31, 2026 at 12:52 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Sat, May 30, 2026 at 12:55:03PM -0700, Rosen Penev wrote:
> > Sashiko reported three pre-existing bugs in the mxs-auart driver:
> >
> > - For non-ASM9260 variants, mxs_get_clks() obtained the clock but never
> > prepared/enabled it, leaving register accesses in probe at risk of
> > faulting if the bootloader had gated the clock.
> > - The error path and remove function used pdev->id instead of
> > s->port.line to clear the auart_port[] slot. For DT-probed devices
> > pdev->id is -1, causing an out-of-bounds write and leaving a dangling
> > pointer in the array.
> > - The probe error path called iounmap() while the IRQ was still
> > registered. An interrupt during that window would dereference the
> > unmapped membase.
>
> This should still be broken up into smaller pieces, don't you think?
>
> Whenever you have to list the different things you do in a patch, that's
> a huge hint to do so.
I'll do so. Although I doubt the bot will be happy either way.
>
> thanks,
>
> greg k-h
^ permalink raw reply
* Re: [PATCHv2] serial: mxs-auart: fix probe error paths and clock handling
From: Greg Kroah-Hartman @ 2026-05-31 7:51 UTC (permalink / raw)
To: Rosen Penev
Cc: linux-serial, Jiri Slaby, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam,
open list:TTY LAYER AND SERIAL DRIVERS,
open list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <20260530195525.5059-1-rosenp@gmail.com>
On Sat, May 30, 2026 at 12:55:03PM -0700, Rosen Penev wrote:
> Sashiko reported three pre-existing bugs in the mxs-auart driver:
>
> - For non-ASM9260 variants, mxs_get_clks() obtained the clock but never
> prepared/enabled it, leaving register accesses in probe at risk of
> faulting if the bootloader had gated the clock.
> - The error path and remove function used pdev->id instead of
> s->port.line to clear the auart_port[] slot. For DT-probed devices
> pdev->id is -1, causing an out-of-bounds write and leaving a dangling
> pointer in the array.
> - The probe error path called iounmap() while the IRQ was still
> registered. An interrupt during that window would dereference the
> unmapped membase.
This should still be broken up into smaller pieces, don't you think?
Whenever you have to list the different things you do in a patch, that's
a huge hint to do so.
thanks,
greg k-h
^ permalink raw reply
* [PATCH v2 4/4] vc_screen: replace __get_free_pages() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-31 7:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260531-b4-tty-v2-0-f7149947d4ef@kernel.org>
vcs_read() and vcs_write() allocate staging buffers with
__get_free_pages().
These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and it's a
modern way of saying "I need a page-sized buffer"
Replace use of __get_free_page() with kmalloc() and drop unused now
DEFINE_FREE(free_page_ptr ...)
Link: https://lore.kernel.org/all/700c5a5f-3128-4671-99aa-827ca73f5cdf@kernel.org
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/vt/vc_screen.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/tty/vt/vc_screen.c b/drivers/tty/vt/vc_screen.c
index 4d2d46c95fef..386c80efc672 100644
--- a/drivers/tty/vt/vc_screen.c
+++ b/drivers/tty/vt/vc_screen.c
@@ -53,8 +53,6 @@
#define HEADER_SIZE 4u
#define CON_BUF_SIZE (IS_ENABLED(CONFIG_BASE_SMALL) ? 256 : PAGE_SIZE)
-DEFINE_FREE(free_page_ptr, void *, if (_T) free_page((unsigned long)_T));
-
/*
* Our minor space:
*
@@ -371,7 +369,7 @@ vcs_read(struct file *file, char __user *buf, size_t count, loff_t *ppos)
loff_t pos;
bool viewed, attr, uni_mode;
- char *con_buf __free(free_page_ptr) = (char *)__get_free_page(GFP_KERNEL);
+ char *con_buf __free(kfree) = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!con_buf)
return -ENOMEM;
@@ -596,7 +594,7 @@ vcs_write(struct file *file, const char __user *buf, size_t count, loff_t *ppos)
if (use_unicode(inode))
return -EOPNOTSUPP;
- char *con_buf __free(free_page_ptr) = (char *)__get_free_page(GFP_KERNEL);
+ char *con_buf __free(kfree) = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!con_buf)
return -ENOMEM;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 3/4] tty: serial: men_z135_uart: replace __get_free_page() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-31 7:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260531-b4-tty-v2-0-f7149947d4ef@kernel.org>
men_z135_probe() allocates a receive staging buffer filled by the
CPU via memcpy_fromio() from the device MMIO region.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/serial/men_z135_uart.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/men_z135_uart.c b/drivers/tty/serial/men_z135_uart.c
index 6fad57fee912..9138fa29d301 100644
--- a/drivers/tty/serial/men_z135_uart.c
+++ b/drivers/tty/serial/men_z135_uart.c
@@ -16,6 +16,7 @@
#include <linux/tty_flip.h>
#include <linux/bitops.h>
#include <linux/mcb.h>
+#include <linux/slab.h>
#define MEN_Z135_MAX_PORTS 12
#define MEN_Z135_BASECLK 29491200
@@ -811,7 +812,7 @@ static int men_z135_probe(struct mcb_device *mdev,
if (!uart)
return -ENOMEM;
- uart->rxbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
+ uart->rxbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!uart->rxbuf)
return -ENOMEM;
@@ -841,7 +842,7 @@ static int men_z135_probe(struct mcb_device *mdev,
return 0;
err:
- free_page((unsigned long) uart->rxbuf);
+ kfree(uart->rxbuf);
dev_err(dev, "Failed to add UART: %d\n", err);
return err;
@@ -858,7 +859,7 @@ static void men_z135_remove(struct mcb_device *mdev)
line--;
uart_remove_one_port(&men_z135_driver, &uart->port);
- free_page((unsigned long) uart->rxbuf);
+ kfree(uart->rxbuf);
}
static const struct mcb_device_id men_z135_ids[] = {
--
2.53.0
^ permalink raw reply related
* [PATCH v2 2/4] tty: amiserial: replace get_zeroed_page() with kzalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-31 7:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260531-b4-tty-v2-0-f7149947d4ef@kernel.org>
rs_startup() allocates a transmit ring buffer that is used to buffer reads
and writes from/to serial data register.
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/amiserial.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/tty/amiserial.c b/drivers/tty/amiserial.c
index 81eaca751541..28af0fd98181 100644
--- a/drivers/tty/amiserial.c
+++ b/drivers/tty/amiserial.c
@@ -443,23 +443,23 @@ static int rs_startup(struct tty_struct *tty, struct serial_state *info)
struct tty_port *port = &info->tport;
unsigned long flags;
int retval=0;
- unsigned long page;
+ void *buffer;
- page = get_zeroed_page(GFP_KERNEL);
- if (!page)
+ buffer = kzalloc(PAGE_SIZE, GFP_KERNEL);
+ if (!buffer)
return -ENOMEM;
local_irq_save(flags);
if (tty_port_initialized(port)) {
- free_page(page);
+ kfree(buffer);
goto errout;
}
if (info->xmit.buf)
- free_page(page);
+ kfree(buffer);
else
- info->xmit.buf = (unsigned char *) page;
+ info->xmit.buf = buffer;
#ifdef SERIAL_DEBUG_OPEN
printk("starting up ttys%d ...", info->line);
@@ -537,7 +537,7 @@ static void rs_shutdown(struct tty_struct *tty, struct serial_state *info)
*/
free_irq(IRQ_AMIGA_VERTB, info);
- free_page((unsigned long)info->xmit.buf);
+ kfree(info->xmit.buf);
info->xmit.buf = NULL;
info->IER = 0;
--
2.53.0
^ permalink raw reply related
* [PATCH v2 1/4] serial: pch: replace __get_free_page() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-31 7:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
In-Reply-To: <20260531-b4-tty-v2-0-f7149947d4ef@kernel.org>
pch_uart_init_port() allocates a staging buffer for non-DMA receive path
using __get_free_page().
This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.
kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.
Replace use of __get_free_page() with kmalloc() and free_page() with
kfree().
Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
drivers/tty/serial/pch_uart.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/tty/serial/pch_uart.c b/drivers/tty/serial/pch_uart.c
index 6729d8e83c3c..07d8cdb58912 100644
--- a/drivers/tty/serial/pch_uart.c
+++ b/drivers/tty/serial/pch_uart.c
@@ -1655,7 +1655,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
if (priv == NULL)
goto init_port_alloc_err;
- rxbuf = (unsigned char *)__get_free_page(GFP_KERNEL);
+ rxbuf = kmalloc(PAGE_SIZE, GFP_KERNEL);
if (!rxbuf)
goto init_port_free_txbuf;
@@ -1728,7 +1728,7 @@ static struct eg20t_port *pch_uart_init_port(struct pci_dev *pdev,
#ifdef CONFIG_SERIAL_PCH_UART_CONSOLE
pch_uart_ports[board->line_no] = NULL;
#endif
- free_page((unsigned long)rxbuf);
+ kfree(rxbuf);
init_port_free_txbuf:
kfree(priv);
init_port_alloc_err:
@@ -1743,7 +1743,7 @@ static void pch_uart_exit_port(struct eg20t_port *priv)
snprintf(name, sizeof(name), "uart%d_regs", priv->port.line);
debugfs_lookup_and_remove(name, NULL);
uart_remove_one_port(&pch_uart_driver, &priv->port);
- free_page((unsigned long)priv->rxbuf.buf);
+ kfree(priv->rxbuf.buf);
}
static void pch_uart_pci_remove(struct pci_dev *pdev)
--
2.53.0
^ permalink raw reply related
* [PATCH v2 0/4] tty: replace __get_free_pages() with kmalloc()
From: Mike Rapoport (Microsoft) @ 2026-05-31 7:02 UTC (permalink / raw)
To: Greg Kroah-Hartman, Jiri Slaby
Cc: Mike Rapoport, linux-kernel, linux-mm, linux-serial
This is a (tiny) part of larger work of replacing page allocator calls
with kmalloc.
Nowadays the right way to say "I need a buffer" is kmalloc() rather than
ancient and ugly __get_free_pages().
---
v2 changes:
* fix placement of include <linux/slab.h> in men_z135_uart
v1: https://patch.msgid.link/20260528-b4-tty-v1-0-9da9f7aec5f2@kernel.org
---
Mike Rapoport (Microsoft) (4):
serial: pch: replace __get_free_page() with kmalloc()
tty: amiserial: replace get_zeroed_page() with kzalloc()
tty: serial: men_z135_uart: replace __get_free_page() with kmalloc()
vc_screen: replace __get_free_pages() with kmalloc()
drivers/tty/amiserial.c | 14 +++++++-------
drivers/tty/serial/men_z135_uart.c | 7 ++++---
drivers/tty/serial/pch_uart.c | 6 +++---
drivers/tty/vt/vc_screen.c | 6 ++----
4 files changed, 16 insertions(+), 17 deletions(-)
---
base-commit: 5d6919055dec134de3c40167a490f33c74c12581
change-id: 20260528-b4-tty-7c6e90f41d13
Best regards,
--
Sincerely yours,
Mike.
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Onur Özkan @ 2026-05-31 7:01 UTC (permalink / raw)
To: Onur Özkan
Cc: Danilo Krummrich, Markus Probst via B4 Relay, markus.probst,
Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Miguel Ojeda,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Kari Argillander, Rafael J. Wysocki,
Viresh Kumar, Boqun Feng, David Airlie, Simona Vetter,
linux-serial, linux-kernel, rust-for-linux, linux-pm, driver-core,
dri-devel
In-Reply-To: <20260531065838.7352-1-work@onurozkan.dev>
On Sun, 31 May 2026 09:58:34 +0300
Onur Özkan <work@onurozkan.dev> wrote:
> On Sun, 31 May 2026 01:51:08 +0200
> Danilo Krummrich <dakr@kernel.org> wrote:
>
> > On Sun May 31, 2026 at 12:51 AM CEST, Markus Probst via B4 Relay wrote:
> > > +#[pinned_drop]
> > > +impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> > > + fn drop(self: Pin<&mut Self>) {
> > > + let mut active = self.active.lock();
> > > + if *active {
> > > + // SAFETY:
> > > + // - We have exclusive access to `self.driver`.
> > > + // - `self.driver` is guaranteed to be initialized.
> > > + unsafe { (*self.driver.get()).assume_init_drop() };
> > > + *active = false;
> > > + }
> > > +
> > > + // SAFETY: We have exclusive access to `self.open`.
> > > + if unsafe { *self.open.get() } {
> > > + // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> > > + // `struct serdev_device`.
> > > + unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
> > > + }
> > > + }
> > > +}
> >
> > Just in case it came across otherwise, I did not mean to push for you to go for
> > this approach. We just kept discussing it because it let to the (to me more
> > interesting) discussion around the LED class device abstraction.
> >
> > While this approach gets us rid of an extra allocation and the rust_private_data
> > pointer in struct serdev_device it also turns out a bit more convoluted.
> >
> > That said, both are correct, and I won't object either one; up to you and the
> > serdev / tty maintainers.
> >
> > Please wait a bit before resending, so other people can comment on this as well.
> >
> > > + extern "C" fn receive_buf_callback(
> > > + sdev: *mut bindings::serdev_device,
> > > + buf: *const u8,
> > > + length: usize,
> > > + ) -> usize {
> > > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > > + // pointer to a `struct serdev_device`.
> > > + //
> > > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
> >
> > CoreInternal snuck back in, should be BoundInternal.
> >
> > > +
> > > + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > > + // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > > + let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > > + let active = private_data.active.lock();
> >
> > I think SRCU would be a much better fit, but the code didn't land yet, so the
> > mutex seems fine for now. But I'd probably add a TODO.
> >
>
> Jfyi v9 is on the list [1] and I would say we are pretty close on taking that.
>
> Thanks,
> Onur
>
> [1]: https://lore.kernel.org/rust-for-linux/DIWEXUCNLURG.136XXDBHSBNVR@kernel.org
>
I pasted the wrong link, sorry.
It should have been this instead:
https://lore.kernel.org/all/20260529134004.396743-1-work@onurozkan.dev
> > > +
> > > + if !*active {
> > > + return length;
> > > + }
> > > +
> > > + // SAFETY: No one has exclusive access to `private_data.driver`.
> > > + let data = unsafe { &*private_data.driver.get() };
> > > + // SAFETY:
> > > + // - `private_data.driver` is pinned.
> > > + // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> > > + // hence it's guaranteed that `private_data.driver` was initialized.
> > > + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > > +
> > > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > > +
> > > + T::receive(sdev, data_pinned, buf)
> > > + }
> > > +}
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Onur Özkan @ 2026-05-31 6:58 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Markus Probst via B4 Relay, markus.probst, Rob Herring,
Greg Kroah-Hartman, Jiri Slaby, Miguel Ojeda, Gary Guo,
Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
Trevor Gross, Kari Argillander, Rafael J. Wysocki, Viresh Kumar,
Boqun Feng, David Airlie, Simona Vetter, linux-serial,
linux-kernel, rust-for-linux, linux-pm, driver-core, dri-devel
In-Reply-To: <DIWEXUCNLURG.136XXDBHSBNVR@kernel.org>
On Sun, 31 May 2026 01:51:08 +0200
Danilo Krummrich <dakr@kernel.org> wrote:
> On Sun May 31, 2026 at 12:51 AM CEST, Markus Probst via B4 Relay wrote:
> > +#[pinned_drop]
> > +impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> > + fn drop(self: Pin<&mut Self>) {
> > + let mut active = self.active.lock();
> > + if *active {
> > + // SAFETY:
> > + // - We have exclusive access to `self.driver`.
> > + // - `self.driver` is guaranteed to be initialized.
> > + unsafe { (*self.driver.get()).assume_init_drop() };
> > + *active = false;
> > + }
> > +
> > + // SAFETY: We have exclusive access to `self.open`.
> > + if unsafe { *self.open.get() } {
> > + // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> > + // `struct serdev_device`.
> > + unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
> > + }
> > + }
> > +}
>
> Just in case it came across otherwise, I did not mean to push for you to go for
> this approach. We just kept discussing it because it let to the (to me more
> interesting) discussion around the LED class device abstraction.
>
> While this approach gets us rid of an extra allocation and the rust_private_data
> pointer in struct serdev_device it also turns out a bit more convoluted.
>
> That said, both are correct, and I won't object either one; up to you and the
> serdev / tty maintainers.
>
> Please wait a bit before resending, so other people can comment on this as well.
>
> > + extern "C" fn receive_buf_callback(
> > + sdev: *mut bindings::serdev_device,
> > + buf: *const u8,
> > + length: usize,
> > + ) -> usize {
> > + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> > + // pointer to a `struct serdev_device`.
> > + //
> > + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> > + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
>
> CoreInternal snuck back in, should be BoundInternal.
>
> > +
> > + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> > + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> > + // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> > + let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> > + let active = private_data.active.lock();
>
> I think SRCU would be a much better fit, but the code didn't land yet, so the
> mutex seems fine for now. But I'd probably add a TODO.
>
Jfyi v9 is on the list [1] and I would say we are pretty close on taking that.
Thanks,
Onur
[1]: https://lore.kernel.org/rust-for-linux/DIWEXUCNLURG.136XXDBHSBNVR@kernel.org
> > +
> > + if !*active {
> > + return length;
> > + }
> > +
> > + // SAFETY: No one has exclusive access to `private_data.driver`.
> > + let data = unsafe { &*private_data.driver.get() };
> > + // SAFETY:
> > + // - `private_data.driver` is pinned.
> > + // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> > + // hence it's guaranteed that `private_data.driver` was initialized.
> > + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> > +
> > + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> > + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> > +
> > + T::receive(sdev, data_pinned, buf)
> > + }
> > +}
^ permalink raw reply
* Re: [PATCH v11 1/3] rust: add basic serial device bus abstractions
From: Danilo Krummrich @ 2026-05-30 23:51 UTC (permalink / raw)
To: Markus Probst via B4 Relay
Cc: markus.probst, Rob Herring, Greg Kroah-Hartman, Jiri Slaby,
Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Kari Argillander,
Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
Simona Vetter, linux-serial, linux-kernel, rust-for-linux,
linux-pm, driver-core, dri-devel
In-Reply-To: <20260531-rust_serdev-v11-1-dee8e0d830f1@posteo.de>
On Sun May 31, 2026 at 12:51 AM CEST, Markus Probst via B4 Relay wrote:
> +#[pinned_drop]
> +impl<T: Driver> PinnedDrop for PrivateData<'_, T> {
> + fn drop(self: Pin<&mut Self>) {
> + let mut active = self.active.lock();
> + if *active {
> + // SAFETY:
> + // - We have exclusive access to `self.driver`.
> + // - `self.driver` is guaranteed to be initialized.
> + unsafe { (*self.driver.get()).assume_init_drop() };
> + *active = false;
> + }
> +
> + // SAFETY: We have exclusive access to `self.open`.
> + if unsafe { *self.open.get() } {
> + // SAFETY: `self.sdev.as_raw()` is guaranteed to be a pointer to a valid
> + // `struct serdev_device`.
> + unsafe { bindings::serdev_device_close(self.sdev.as_raw()) };
> + }
> + }
> +}
Just in case it came across otherwise, I did not mean to push for you to go for
this approach. We just kept discussing it because it let to the (to me more
interesting) discussion around the LED class device abstraction.
While this approach gets us rid of an extra allocation and the rust_private_data
pointer in struct serdev_device it also turns out a bit more convoluted.
That said, both are correct, and I won't object either one; up to you and the
serdev / tty maintainers.
Please wait a bit before resending, so other people can comment on this as well.
> + extern "C" fn receive_buf_callback(
> + sdev: *mut bindings::serdev_device,
> + buf: *const u8,
> + length: usize,
> + ) -> usize {
> + // SAFETY: The serial device bus only ever calls the receive buf callback with a valid
> + // pointer to a `struct serdev_device`.
> + //
> + // INVARIANT: `sdev` is valid for the duration of `receive_buf_callback()`.
> + let sdev = unsafe { &*sdev.cast::<Device<device::CoreInternal<'_>>>() };
CoreInternal snuck back in, should be BoundInternal.
> +
> + // SAFETY: `receive_buf_callback` is only ever called after a successful call to
> + // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
> + // and stored a `Pin<KBox<PrivateData<'_, T>>>`.
> + let private_data = unsafe { sdev.as_ref().drvdata_borrow::<PrivateData<'_, T>>() };
> + let active = private_data.active.lock();
I think SRCU would be a much better fit, but the code didn't land yet, so the
mutex seems fine for now. But I'd probably add a TODO.
> +
> + if !*active {
> + return length;
> + }
> +
> + // SAFETY: No one has exclusive access to `private_data.driver`.
> + let data = unsafe { &*private_data.driver.get() };
> + // SAFETY:
> + // - `private_data.driver` is pinned.
> + // - `receive_buf_callback` is only ever called after a successful call to `probe_callback`,
> + // hence it's guaranteed that `private_data.driver` was initialized.
> + let data_pinned = unsafe { Pin::new_unchecked(data.assume_init_ref()) };
> +
> + // SAFETY: `buf` is guaranteed to be non-null and has the size of `length`.
> + let buf = unsafe { core::slice::from_raw_parts(buf, length) };
> +
> + T::receive(sdev, data_pinned, buf)
> + }
> +}
^ permalink raw reply
* [PATCH v11 3/3] MAINTAINERS: serdev: Add self for serdev
From: Markus Probst via B4 Relay @ 2026-05-30 22:51 UTC (permalink / raw)
To: Rob Herring, Greg Kroah-Hartman, Jiri Slaby, Miguel Ojeda,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Kari Argillander,
Rafael J. Wysocki, Viresh Kumar, Boqun Feng, David Airlie,
Simona Vetter, Boqun Feng
Cc: linux-serial, linux-kernel, rust-for-linux, linux-pm, driver-core,
dri-devel, Markus Probst
In-Reply-To: <20260531-rust_serdev-v11-0-dee8e0d830f1@posteo.de>
From: Markus Probst <markus.probst@posteo.de>
Rob mentioned he needs to find someone else to maintain serdev.
Link: https://lore.kernel.org/rust-for-linux/20260430195858.GA1650658-robh@kernel.org/
Link: https://lore.kernel.org/rust-for-linux/da85ceb81f51079d4a8248a1ffde6a27d2ef24ad.camel@posteo.de/
Signed-off-by: Markus Probst <markus.probst@posteo.de>
---
MAINTAINERS | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index fa3f20f0cc4f..58db6aedf45a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24270,7 +24270,7 @@ F: drivers/iio/chemical/sps30_i2c.c
F: drivers/iio/chemical/sps30_serial.c
SERIAL DEVICE BUS
-M: Rob Herring <robh@kernel.org>
+M: Markus Probst <markus.probst@posteo.de>
L: linux-serial@vger.kernel.org
S: Maintained
F: Documentation/devicetree/bindings/serial/serial.yaml
--
2.53.0
^ 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