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 v2 2/4] serdev: add private data to serdev_device Date: Mon, 09 Mar 2026 08:21:39 +1000 Message-ID: In-Reply-To: <934ca1e6-dfee-411e-890a-083dcfc9f6b1@infradead.org> References: <20260306-rust_serdev-v2-0-e9b23b42b255@posteo.de> <934ca1e6-dfee-411e-890a-083dcfc9f6b1@infradead.org> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit MIME-Version: 1.0 Patch Review ```c + * @private_data Private data for the device driver. */ struct serdev_device { ... + void *private_data; }; ``` **Missing colon in kernel-doc**: The `@private_data` line is missing the colon separator used by all other fields. Should be `@private_data:`. **Design concern**: `struct serdev_device` already has driver data via `dev_get_drvdata()`/`dev_set_drvdata()` on the embedded `struct device`. Adding a second `private_data` field is unusual. The commit message explains this is needed because the Rust abstraction uses `drvdata` for the driver type `T`, and needs a separate field for internal `PrivateData`. This is a C ABI change visible to all serdev users and should be discussed with the serdev maintainer (Rob Herring). Consider whether the Rust side could instead embed its `PrivateData` alongside the driver type in a wrapper struct stored in the standard `drvdata`, avoiding the C-side change entirely. --- --- Generated by Claude Code Patch Reviewer