From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: rust: add basic serial device bus abstractions Date: Tue, 28 Apr 2026 14:14:44 +1000 Message-ID: In-Reply-To: <20260427-rust_serdev-v6-0-173798d5e1a3@posteo.de> References: <20260427-rust_serdev-v6-0-173798d5e1a3@posteo.de> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Overall Series Review Subject: rust: add basic serial device bus abstractions Author: Markus Probst via B4 Relay Patches: 5 Reviewed: 2026-04-28T14:14:44.163925 --- This is a v6 series (4 patches) by Markus Probst adding Rust abstractions f= or the serial device bus (serdev) to the Linux kernel. The series is well-s= tructured: a preparatory devres API change, a small C-side struct modificat= ion, the main Rust abstraction, and a sample driver. **Strengths:** - The overall architecture follows established patterns from the platform a= nd PCI Rust driver abstractions closely. - The probe synchronization design (using `Completion` to handle the race b= etween `devm_serdev_device_open` and probe completion) is a thoughtful solu= tion to a real concurrency problem. - The API surface is reasonable =E2=80=94 covering baudrate, flow control, = parity, read, and write operations. - Good use of `devres` for the abstraction's internal bookkeeping data. **Concerns:** - The `Timeout::Max` doc comment says it is "equivalent to `kernel::task::M= AX_SCHEDULE_TIMEOUT`" but it actually maps to 0, relying on the C serdev/tt= y convention that 0 means "wait forever." The doc is misleading =E2=80=94 i= t should reference the C API convention instead. - The `unwrap_or_default()` in `Timeout::into_jiffies()` silently degrades = large timeouts to 0, which happens to be correct (wait forever) by the same= C convention, but this is accidental correctness and confusing. - Adding `void *rust_private_data` to the C `struct serdev_device` is a deb= atable design choice that has no enforcement against misuse by C drivers. - A few doc typos and a broken intra-doc link. --- --- Generated by Claude Code Patch Reviewer