Message-Id: <DFO5EYZN84GY.3EAJUY3UASFCJ@nvidia.com>
Date: Wed, 14 Jan 2026 16:54:04 +0900
From: "Alexandre Courbot" <acourbot@...dia.com>
To: "Matthew Maurer" <mmaurer@...gle.com>, "Alexandre Courbot"
 <acourbot@...dia.com>
Cc: "Miguel Ojeda" <ojeda@...nel.org>, "Boqun Feng" <boqun.feng@...il.com>,
 "Gary Guo" <gary@...yguo.net>, Björn Roy Baron
 <bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
 Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
 "Trevor Gross" <tmgross@...ch.edu>, "Danilo Krummrich" <dakr@...nel.org>,
 "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
 <rafael@...nel.org>, <linux-kernel@...r.kernel.org>,
 <rust-for-linux@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] rust: Add soc_device support

On Sat Dec 27, 2025 at 4:38 AM JST, Matthew Maurer wrote:
> On Fri, Dec 26, 2025 at 11:26 AM Matthew Maurer <mmaurer@...gle.com> wrote:
>>
>> On Tue, Dec 16, 2025 at 6:31 PM Alexandre Courbot <acourbot@...dia.com> wrote:
>> >
>> > On Wed Dec 17, 2025 at 4:24 AM JST, Matthew Maurer wrote:
>> > > Allow SoC drivers in Rust to present metadata about their devices to
>> > > userspace through /sys/devices/socX and other drivers to identify their
>> > > properties through `soc_device_match`.
>> > >
>> > > Signed-off-by: Matthew Maurer <mmaurer@...gle.com>
>> > > ---
>> > >  MAINTAINERS                     |   1 +
>> > >  rust/bindings/bindings_helper.h |   1 +
>> > >  rust/kernel/lib.rs              |   2 +
>> > >  rust/kernel/soc.rs              | 135 ++++++++++++++++++++++++++++++++++++++++
>> > >  4 files changed, 139 insertions(+)
>> > >
>> > > diff --git a/MAINTAINERS b/MAINTAINERS
>> > > index c5a7cda26c600e49c7ab0d547306d3281333f672..4ff01fb0f1bda27002094113c0bf9d074d28fdb6 100644
>> > > --- a/MAINTAINERS
>> > > +++ b/MAINTAINERS
>> > > @@ -7700,6 +7700,7 @@ F:      rust/kernel/devres.rs
>> > >  F:   rust/kernel/driver.rs
>> > >  F:   rust/kernel/faux.rs
>> > >  F:   rust/kernel/platform.rs
>> > > +F:   rust/kernel/soc.rs
>> > >  F:   samples/rust/rust_debugfs.rs
>> > >  F:   samples/rust/rust_debugfs_scoped.rs
>> > >  F:   samples/rust/rust_driver_platform.rs
>> > > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
>> > > index a067038b4b422b4256f4a2b75fe644d47e6e82c8..9fdf76ca630e00715503e2a3a809bedc895697fd 100644
>> > > --- a/rust/bindings/bindings_helper.h
>> > > +++ b/rust/bindings/bindings_helper.h
>> > > @@ -80,6 +80,7 @@
>> > >  #include <linux/sched.h>
>> > >  #include <linux/security.h>
>> > >  #include <linux/slab.h>
>> > > +#include <linux/sys_soc.h>
>> > >  #include <linux/task_work.h>
>> > >  #include <linux/tracepoint.h>
>> > >  #include <linux/usb.h>
>> > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> > > index f812cf12004286962985a068665443dc22c389a2..6d637e2fed1b605e2dfc2e7b2247179439a90ba9 100644
>> > > --- a/rust/kernel/lib.rs
>> > > +++ b/rust/kernel/lib.rs
>> > > @@ -138,6 +138,8 @@
>> > >  pub mod seq_file;
>> > >  pub mod sizes;
>> > >  pub mod slice;
>> > > +#[cfg(CONFIG_SOC_BUS)]
>> > > +pub mod soc;
>> > >  mod static_assert;
>> > >  #[doc(hidden)]
>> > >  pub mod std_vendor;
>> > > diff --git a/rust/kernel/soc.rs b/rust/kernel/soc.rs
>> > > new file mode 100644
>> > > index 0000000000000000000000000000000000000000..0d6a36c83cb67ef20dc1e3d3995752f36e25ac9f
>> > > --- /dev/null
>> > > +++ b/rust/kernel/soc.rs
>> > > @@ -0,0 +1,135 @@
>> > > +// SPDX-License-Identifier: GPL-2.0
>> > > +
>> > > +// Copyright (C) 2025 Google LLC.
>> > > +
>> > > +//! SoC Driver Abstraction.
>> > > +//!
>> > > +//! C header: [`include/linux/sys_soc.h`](srctree/include/linux/sys_soc.h)
>> > > +
>> > > +use crate::{
>> > > +    bindings,
>> > > +    error,
>> > > +    prelude::*,
>> > > +    str::CString,
>> > > +    types::Opaque, //
>> > > +};
>> > > +use core::ptr::NonNull;
>> > > +
>> > > +/// Attributes for a SoC device.
>> > > +///
>> > > +/// These are both exported to userspace under /sys/devices/socX and provided to other drivers to
>> > > +/// match against via `soc_device_match` (not yet available in Rust) to enable quirks or
>> > > +/// device-specific support where necessary.
>> > > +///
>> > > +/// All fields are freeform - they have no specific formatting, just defined meanings.
>> > > +/// For example, the [`machine`](`Attributes::machine`) field could be "DB8500" or
>> > > +/// "Qualcomm Technologies, Inc. SM8560 HDK", but regardless it should identify a board or product.
>> > > +pub struct Attributes {
>> > > +    /// Should generally be a board ID or product ID. Examples
>> > > +    /// include DB8500 (ST-Ericsson) or "Qualcomm Technologies, inc. SM8560 HDK".
>> > > +    ///
>> > > +    /// If this field is not populated, the SoC infrastructure will try to populate it from
>> > > +    /// `/model` in the device tree.
>> > > +    pub machine: Option<CString>,
>> > > +    /// The broader class this SoC belongs to. Examples include ux500
>> > > +    /// (for DB8500) or Snapdragon (for SM8650).
>> >
>> > Formatting of the comments seems a bit off (also appears in other
>> > places, please reapply formatting globally to be sure).
>>
>> I have just re-run the `rustfmt` target on this commit, and see no
>> changes. Is there something specific that you think is off?
>>
>> >
>> > > +    ///
>> > > +    /// On chips with ARM firmware supporting SMCCC v1.2+, this may be a JEDEC JEP106 manufacturer
>> > > +    /// identification.
>> > > +    pub family: Option<CString>,
>> > > +    /// The manufacturing revision of the part. Frequently this is MAJOR.MINOR, but not always.
>> > > +    pub revision: Option<CString>,
>> > > +    /// Serial Number - uniquely identifies a specific SoC. If present, should be unique (buying a
>> > > +    /// replacement part should change it if present). This field cannot be matched on and is
>> > > +    /// solely present to export through /sys.
>> > > +    pub serial_number: Option<CString>,
>> > > +    /// SoC ID - identifies a specific SoC kind in question, sometimes more specifically than
>> > > +    /// `machine` if the same SoC is used in multiple products. Some devices use this to specify a
>> > > +    /// SoC name, e.g. "I.MX??", and others just print an ID number (e.g. Tegra and Qualcomm).
>> > > +    ///
>> > > +    /// On chips with ARM firmware supporting SMCCC v1.2+, this may be a JEDEC JEP106 manufacturer
>> > > +    /// identification (the family value) followed by a colon and then a 4-digit ID value.
>> > > +    pub soc_id: Option<CString>,
>> > > +}
>> > > +
>> > > +struct BuiltAttributes {
>> >
>> > Even though this struct is private, some short documentation would be
>> > helpful - actually the explanation about the relationship between
>> > `_backing` and `inner` belongs here imho instead of in the member's
>> > documentation.
>
> I don't think I should do this unless there's some kernel-specific
> style guidance I'm unaware of (I checked and couldn't find anything,
> but maybe I missed something):
> 1. These are comments, not documentation. Documentation is intended
> for a user of an interface, comments are intended for a reader of the
> code. The information in these comments should not be considered by a
> user of `BuiltAttributes`, even an internal one.
> 2. Usually the reason to provide documentation for a private structure
> is that it is available for use at a larger scope than the local
> module, even if it's not accessible outside the crate, for the
> purposes of helping developers engage with the type either through an
> IDE or a local reference. That does not apply here - this is an
> implementation detail of `Registration` and should never be used
> outside this file.
>
> That said, I don't care that deeply about where this text goes, so if
> consensus is that we'd prefer it to be in a doc comment I can put it
> there; it just seems wrong to me.

My thinking for suggesting this is that this code is open; other folks
might work on it later, who are not familiar with it.

Having the data structures properly documented is useful for them,
especially if they are related to other types in ways that are not
immediately obvious.

Powered by blists - more mailing lists