From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: Re: [PATCH v3 2/4] serdev: add rust private data to serdev_device Date: Mon, 16 Mar 2026 12:17:35 +1000 Message-ID: In-Reply-To: <2026031402-absence-graph-af5d@gregkh> References: <20260313-rust_serdev-v3-0-c9a3af214f7f@posteo.de> <2026031402-absence-graph-af5d@gregkh> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review ```c + void *rust_private_data; ``` Adding a `void *` field to `struct serdev_device` purely for Rust abstracti= on use is somewhat controversial. This adds overhead to every `serdev_devic= e` in the system, even when Rust is not used.=20 **Concern:** Why not use the existing `dev_set_drvdata`/`dev_get_drvdata` m= echanism for this? The patch series uses `dev_set_drvdata` for the driver's= own data (`T`) and then needs this extra field for `PrivateData` (the comp= letion/error pair). This suggests the abstraction design could be reworked = to avoid needing two separate private data fields =E2=80=94 for example, by= wrapping the driver data and the completion together into a single struct = stored via `dev_set_drvdata`. The kdoc alignment changes are good cleanup (adding missing colons on `@wri= te_comp:` and `@write_lock:`). **Initialization concern:** The new `rust_private_data` field is never expl= icitly initialized in the C code that allocates `serdev_device`. It will be= zero-initialized if the struct is allocated with `kzalloc`, but this shoul= d be documented or verified. --- Generated by Claude Code Patch Reviewer