public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] Support exporting SoC info from Rust
@ 2025-12-16  0:43 Matthew Maurer
  2025-12-16  0:43 ` [PATCH v2 1/3] rust: Add soc_device support Matthew Maurer
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Matthew Maurer @ 2025-12-16  0:43 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: linux-kernel, rust-for-linux, Matthew Maurer, Lee Jones

This is a fairly straightforward binding of `soc_device_register` and
`soc_device_unregister` which allows a driver to export basic info about
a SoC.

The last patch is a sample demonstrating usage, and can be dropped
without issue.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
Changes in v2:
- Switch to new import style
- Increased documentation. Some of this had to be gathered by looking at
  what is done in practice at the moment, as documentation was absent or
  did not match code.
- Remove `Device` intermediate abstraction
- Removed unnecessary pinning of `BuiltDeviceAttributes` - it only needs
  to be pinned for registration, not to exist.
- Aesthetic renames (`Attributes` pluralization, dropping `Device`,
  etc.)
- Use more representative values for attributes in the sample driver
- Fix swap of example values in the documentation for machine vs family
- Link to v1: https://lore.kernel.org/r/20251212-soc-bindings-v1-0-db51044ce805@google.com

---
Matthew Maurer (3):
      rust: Add soc_device support
      docs: ABI: sysfs-devices-soc: Fix swapped sample values
      rust: Add SoC Driver Sample

 Documentation/ABI/testing/sysfs-devices-soc |   4 +-
 MAINTAINERS                                 |   2 +
 rust/bindings/bindings_helper.h             |   1 +
 rust/kernel/lib.rs                          |   2 +
 rust/kernel/soc.rs                          | 145 ++++++++++++++++++++++++++++
 samples/rust/Kconfig                        |  11 +++
 samples/rust/Makefile                       |   1 +
 samples/rust/rust_soc.rs                    |  82 ++++++++++++++++
 8 files changed, 246 insertions(+), 2 deletions(-)
---
base-commit: 008d3547aae5bc86fac3eda317489169c3fda112
change-id: 20251029-soc-bindings-9b0731bcdbed

Best regards,
-- 
Matthew Maurer <mmaurer@google.com>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/3] rust: Add soc_device support
  2025-12-16  0:43 [PATCH v2 0/3] Support exporting SoC info from Rust Matthew Maurer
@ 2025-12-16  0:43 ` Matthew Maurer
  2025-12-16 10:20   ` Danilo Krummrich
  2025-12-16  0:43 ` [PATCH v2 2/3] docs: ABI: sysfs-devices-soc: Fix swapped sample values Matthew Maurer
  2025-12-16  0:43 ` [PATCH v2 3/3] rust: Add SoC Driver Sample Matthew Maurer
  2 siblings, 1 reply; 6+ messages in thread
From: Matthew Maurer @ 2025-12-16  0:43 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

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@google.com>
---
 MAINTAINERS                     |   1 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/soc.rs              | 145 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 149 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..fb9e461218787b5546805c0f04fadbc5e3e054a6
--- /dev/null
+++ b/rust/kernel/soc.rs
@@ -0,0 +1,145 @@
+// 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).
+    ///
+    /// 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 {
+    // While `inner` has pointers to `_backing`, it is to the interior of the `CStrings`, not
+    // `backing` itself, so it does not need to be pinned.
+    _backing: Attributes,
+    // `Opaque` makes us `!Unpin`, as the registration holds a pointer to `inner` when used.
+    inner: Opaque<bindings::soc_device_attribute>,
+}
+
+fn cstring_to_c(mcs: &Option<CString>) -> *const kernel::ffi::c_char {
+    mcs.as_ref()
+        .map(|cs| cs.as_char_ptr())
+        .unwrap_or(core::ptr::null())
+}
+
+impl BuiltAttributes {
+    fn as_mut_ptr(&self) -> *mut bindings::soc_device_attribute {
+        self.inner.get()
+    }
+}
+
+impl Attributes {
+    fn build(self) -> BuiltAttributes {
+        BuiltAttributes {
+            inner: Opaque::new(bindings::soc_device_attribute {
+                machine: cstring_to_c(&self.machine),
+                family: cstring_to_c(&self.family),
+                revision: cstring_to_c(&self.revision),
+                serial_number: cstring_to_c(&self.serial_number),
+                soc_id: cstring_to_c(&self.soc_id),
+                data: core::ptr::null(),
+                custom_attr_group: core::ptr::null(),
+            }),
+            _backing: self,
+        }
+    }
+}
+
+/// # Safety
+/// If a device is returned (e.g. no error), `attr` must remain valid for reads until the
+/// returned pointer is released through `soc_device_unregister`.
+unsafe fn register_device(attr: Pin<&BuiltAttributes>) -> Result<NonNull<bindings::soc_device>> {
+    let raw_soc =
+            // SAFETY:
+            // * The struct provided through attr is backed by pinned data next to it, so as
+            //   long as attr lives, the strings pointed to by the struct will too.
+            // * `attr` is pinned, so the pinned data won't move.
+            // * If it returns a device, and so others may try to read this data, by caller
+            //   invariant, `attr` won't be released until the device is.
+            error::from_err_ptr(unsafe { bindings::soc_device_register(attr.as_mut_ptr()) })?;
+    // `soc_device_register` should not return NULL, but it doesn't hurt to be paranoid.
+    NonNull::new(raw_soc).ok_or(EINVAL)
+}
+
+#[pin_data(PinnedDrop)]
+/// Registration handle for your soc_dev. If you let it go out of scope, your soc_dev will be
+/// unregistered.
+pub struct Registration {
+    #[pin]
+    attr: BuiltAttributes,
+    soc_dev: NonNull<bindings::soc_device>,
+}
+
+// SAFETY: We provide no operations through `&Registration`.
+unsafe impl Sync for Registration {}
+
+// SAFETY: All pointers are normal allocations, not thread-specific.
+unsafe impl Send for Registration {}
+
+#[pinned_drop]
+impl PinnedDrop for Registration {
+    fn drop(self: Pin<&mut Self>) {
+        // SAFETY: Device always contains a live pointer to a soc_device that can be unregistered
+        unsafe { bindings::soc_device_unregister(self.soc_dev.as_ptr()) }
+    }
+}
+
+impl Registration {
+    /// Register a new SoC device
+    pub fn register(attr: Attributes) -> impl PinInit<Self, Error> {
+        try_pin_init!(&this in Self {
+                    attr: attr.build(),
+                    // SAFETY: We have already initialized attr, and we are inside PinInit and Self
+                    // is !Unpin, so attr won't be moved and is valid. If it returns success, attr
+                    // will not be dropped until after our `PinnedDrop` implementation runs, so the
+                    // device will be unregistered first.
+                    soc_dev: unsafe {
+                        register_device(Pin::new_unchecked(&(*this.as_ptr()).attr))?
+                    },
+        }? Error)
+    }
+}

-- 
2.52.0.305.g3fc767764a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/3] docs: ABI: sysfs-devices-soc: Fix swapped sample values
  2025-12-16  0:43 [PATCH v2 0/3] Support exporting SoC info from Rust Matthew Maurer
  2025-12-16  0:43 ` [PATCH v2 1/3] rust: Add soc_device support Matthew Maurer
@ 2025-12-16  0:43 ` Matthew Maurer
  2025-12-16  9:22   ` Lee Jones
  2025-12-16  0:43 ` [PATCH v2 3/3] rust: Add SoC Driver Sample Matthew Maurer
  2 siblings, 1 reply; 6+ messages in thread
From: Matthew Maurer @ 2025-12-16  0:43 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: linux-kernel, rust-for-linux, Matthew Maurer, Lee Jones

The sample values for `family` and `machine` were swapped relative to
what the driver actually does, and doesn't match the field description.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 Documentation/ABI/testing/sysfs-devices-soc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-devices-soc b/Documentation/ABI/testing/sysfs-devices-soc
index 5269808ec35f8e2b18516556f886c77f5fac9401..cb6776a4afe02a76fe27ac6fc236babdc7865287 100644
--- a/Documentation/ABI/testing/sysfs-devices-soc
+++ b/Documentation/ABI/testing/sysfs-devices-soc
@@ -17,14 +17,14 @@ Date:		January 2012
 contact:	Lee Jones <lee@kernel.org>
 Description:
 		Read-only attribute common to all SoCs. Contains the SoC machine
-		name (e.g. Ux500).
+		name (e.g. DB8500).
 
 What:		/sys/devices/socX/family
 Date:		January 2012
 contact:	Lee Jones <lee@kernel.org>
 Description:
 		Read-only attribute common to all SoCs. Contains SoC family name
-		(e.g. DB8500).
+		(e.g. ux500).
 
 		On many of ARM based silicon with SMCCC v1.2+ compliant firmware
 		this will contain the JEDEC JEP106 manufacturer’s identification

-- 
2.52.0.305.g3fc767764a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/3] rust: Add SoC Driver Sample
  2025-12-16  0:43 [PATCH v2 0/3] Support exporting SoC info from Rust Matthew Maurer
  2025-12-16  0:43 ` [PATCH v2 1/3] rust: Add soc_device support Matthew Maurer
  2025-12-16  0:43 ` [PATCH v2 2/3] docs: ABI: sysfs-devices-soc: Fix swapped sample values Matthew Maurer
@ 2025-12-16  0:43 ` Matthew Maurer
  2 siblings, 0 replies; 6+ messages in thread
From: Matthew Maurer @ 2025-12-16  0:43 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki
  Cc: linux-kernel, rust-for-linux, Matthew Maurer

Shows registration of a SoC device upon receipt of a probe.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
 MAINTAINERS              |  1 +
 samples/rust/Kconfig     | 11 +++++++
 samples/rust/Makefile    |  1 +
 samples/rust/rust_soc.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 95 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 4ff01fb0f1bda27002094113c0bf9d074d28fdb6..bb2e710277cc84dd6042d4d46076e665d9f68752 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7705,6 +7705,7 @@ F:	samples/rust/rust_debugfs.rs
 F:	samples/rust/rust_debugfs_scoped.rs
 F:	samples/rust/rust_driver_platform.rs
 F:	samples/rust/rust_driver_faux.rs
+F:	samples/rust/rust_soc.rs
 
 DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
 M:	Nishanth Menon <nm@ti.com>
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 3efa51bfc8efccd91d9ee079ccd078ed1a6e8aa7..c49ab910634596aea4a1a73dac87585e084f420a 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -161,6 +161,17 @@ config SAMPLE_RUST_DRIVER_AUXILIARY
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_SOC
+	tristate "SoC Driver"
+	select SOC_BUS
+	help
+	  This option builds the Rust SoC driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_soc.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index f65885d1d62bf406b0db13121ef3e5b09829cfbc..6c0aaa58ccccfd12ef019f68ca784f6d977bc668 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_SAMPLE_RUST_DRIVER_USB)		+= rust_driver_usb.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_AUXILIARY)	+= rust_driver_auxiliary.o
 obj-$(CONFIG_SAMPLE_RUST_CONFIGFS)		+= rust_configfs.o
+obj-$(CONFIG_SAMPLE_RUST_SOC)			+= rust_soc.o
 
 rust_print-y := rust_print_main.o rust_print_events.o
 
diff --git a/samples/rust/rust_soc.rs b/samples/rust/rust_soc.rs
new file mode 100644
index 0000000000000000000000000000000000000000..696c38a2ac9d030a7dc8928fd3ef4cafc0d6ace2
--- /dev/null
+++ b/samples/rust/rust_soc.rs
@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust SoC Platform driver sample.
+
+use kernel::{
+    acpi,
+    c_str,
+    device::Core,
+    of,
+    platform,
+    prelude::*,
+    soc,
+    str::CString,
+    sync::aref::ARef, //
+};
+use pin_init::pin_init_scope;
+
+#[pin_data]
+struct SampleSocDriver {
+    pdev: ARef<platform::Device>,
+    #[pin]
+    _dev_reg: soc::Registration,
+}
+
+kernel::of_device_table!(
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SampleSocDriver as platform::Driver>::IdInfo,
+    [(of::DeviceId::new(c_str!("test,rust-device")), ())]
+);
+
+kernel::acpi_device_table!(
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <SampleSocDriver as platform::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("LNUXBEEF")), ())]
+);
+
+impl platform::Driver for SampleSocDriver {
+    type IdInfo = ();
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+
+    fn probe(
+        pdev: &platform::Device<Core>,
+        _info: Option<&Self::IdInfo>,
+    ) -> impl PinInit<Self, Error> {
+        let dev = pdev.as_ref();
+
+        dev_dbg!(dev, "Probe Rust SoC driver sample.\n");
+
+        let pdev = pdev.into();
+        pin_init_scope(move || {
+            let machine = CString::try_from(c_str!("My cool ACME15 dev board"))?;
+            let family = CString::try_from(c_str!("ACME"))?;
+            let revision = CString::try_from(c_str!("1.2"))?;
+            let serial_number = CString::try_from(c_str!("12345"))?;
+            let soc_id = CString::try_from(c_str!("ACME15"))?;
+
+            let attr = soc::Attributes {
+                machine: Some(machine),
+                family: Some(family),
+                revision: Some(revision),
+                serial_number: Some(serial_number),
+                soc_id: Some(soc_id),
+            };
+
+            Ok(try_pin_init!(SampleSocDriver {
+                pdev: pdev,
+                _dev_reg <- soc::Registration::register(attr),
+            }? Error))
+        })
+    }
+}
+
+kernel::module_platform_driver! {
+    type: SampleSocDriver,
+    name: "rust_soc",
+    authors: ["Matthew Maurer"],
+    description: "Rust SoC Driver",
+    license: "GPL",
+}

-- 
2.52.0.305.g3fc767764a-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/3] docs: ABI: sysfs-devices-soc: Fix swapped sample values
  2025-12-16  0:43 ` [PATCH v2 2/3] docs: ABI: sysfs-devices-soc: Fix swapped sample values Matthew Maurer
@ 2025-12-16  9:22   ` Lee Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Lee Jones @ 2025-12-16  9:22 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
	linux-kernel, rust-for-linux

On Tue, 16 Dec 2025, Matthew Maurer wrote:

> The sample values for `family` and `machine` were swapped relative to
> what the driver actually does, and doesn't match the field description.
> 
> Signed-off-by: Matthew Maurer <mmaurer@google.com>
> ---
>  Documentation/ABI/testing/sysfs-devices-soc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Lee Jones <lee@kernel.org>

> diff --git a/Documentation/ABI/testing/sysfs-devices-soc b/Documentation/ABI/testing/sysfs-devices-soc
> index 5269808ec35f8e2b18516556f886c77f5fac9401..cb6776a4afe02a76fe27ac6fc236babdc7865287 100644
> --- a/Documentation/ABI/testing/sysfs-devices-soc
> +++ b/Documentation/ABI/testing/sysfs-devices-soc
> @@ -17,14 +17,14 @@ Date:		January 2012
>  contact:	Lee Jones <lee@kernel.org>
>  Description:
>  		Read-only attribute common to all SoCs. Contains the SoC machine
> -		name (e.g. Ux500).
> +		name (e.g. DB8500).
>  
>  What:		/sys/devices/socX/family
>  Date:		January 2012
>  contact:	Lee Jones <lee@kernel.org>
>  Description:
>  		Read-only attribute common to all SoCs. Contains SoC family name
> -		(e.g. DB8500).
> +		(e.g. ux500).
>  
>  		On many of ARM based silicon with SMCCC v1.2+ compliant firmware
>  		this will contain the JEDEC JEP106 manufacturer’s identification

-- 
Lee Jones [李琼斯]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 1/3] rust: Add soc_device support
  2025-12-16  0:43 ` [PATCH v2 1/3] rust: Add soc_device support Matthew Maurer
@ 2025-12-16 10:20   ` Danilo Krummrich
  0 siblings, 0 replies; 6+ messages in thread
From: Danilo Krummrich @ 2025-12-16 10:20 UTC (permalink / raw)
  To: Matthew Maurer
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Greg Kroah-Hartman, Rafael J. Wysocki, linux-kernel,
	rust-for-linux

On Tue Dec 16, 2025 at 1:43 AM CET, Matthew Maurer wrote:
> +/// 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).
> +    ///
> +    /// 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>,
> +}

Thanks for expanding the documentation!

> +struct BuiltAttributes {
> +    // While `inner` has pointers to `_backing`, it is to the interior of the `CStrings`, not
> +    // `backing` itself, so it does not need to be pinned.
> +    _backing: Attributes,
> +    // `Opaque` makes us `!Unpin`, as the registration holds a pointer to `inner` when used.
> +    inner: Opaque<bindings::soc_device_attribute>,
> +}
> +
> +fn cstring_to_c(mcs: &Option<CString>) -> *const kernel::ffi::c_char {
> +    mcs.as_ref()
> +        .map(|cs| cs.as_char_ptr())
> +        .unwrap_or(core::ptr::null())
> +}
> +
> +impl BuiltAttributes {
> +    fn as_mut_ptr(&self) -> *mut bindings::soc_device_attribute {
> +        self.inner.get()
> +    }
> +}
> +
> +impl Attributes {
> +    fn build(self) -> BuiltAttributes {
> +        BuiltAttributes {
> +            inner: Opaque::new(bindings::soc_device_attribute {
> +                machine: cstring_to_c(&self.machine),
> +                family: cstring_to_c(&self.family),
> +                revision: cstring_to_c(&self.revision),
> +                serial_number: cstring_to_c(&self.serial_number),
> +                soc_id: cstring_to_c(&self.soc_id),
> +                data: core::ptr::null(),
> +                custom_attr_group: core::ptr::null(),
> +            }),
> +            _backing: self,
> +        }
> +    }
> +}
> +
> +/// # Safety
> +/// If a device is returned (e.g. no error), `attr` must remain valid for reads until the
> +/// returned pointer is released through `soc_device_unregister`.
> +unsafe fn register_device(attr: Pin<&BuiltAttributes>) -> Result<NonNull<bindings::soc_device>> {
> +    let raw_soc =
> +            // SAFETY:
> +            // * The struct provided through attr is backed by pinned data next to it, so as
> +            //   long as attr lives, the strings pointed to by the struct will too.
> +            // * `attr` is pinned, so the pinned data won't move.
> +            // * If it returns a device, and so others may try to read this data, by caller
> +            //   invariant, `attr` won't be released until the device is.
> +            error::from_err_ptr(unsafe { bindings::soc_device_register(attr.as_mut_ptr()) })?;
> +    // `soc_device_register` should not return NULL, but it doesn't hurt to be paranoid.
> +    NonNull::new(raw_soc).ok_or(EINVAL)
> +}

I think it turns out cleaner without this helper function, inlining the
contained code directly into Registration::new().

> +#[pin_data(PinnedDrop)]
> +/// Registration handle for your soc_dev. If you let it go out of scope, your soc_dev will be
> +/// unregistered.
> +pub struct Registration {
> +    #[pin]
> +    attr: BuiltAttributes,
> +    soc_dev: NonNull<bindings::soc_device>,
> +}
> +
> +// SAFETY: We provide no operations through `&Registration`.
> +unsafe impl Sync for Registration {}
> +
> +// SAFETY: All pointers are normal allocations, not thread-specific.
> +unsafe impl Send for Registration {}
> +
> +#[pinned_drop]
> +impl PinnedDrop for Registration {
> +    fn drop(self: Pin<&mut Self>) {
> +        // SAFETY: Device always contains a live pointer to a soc_device that can be unregistered
> +        unsafe { bindings::soc_device_unregister(self.soc_dev.as_ptr()) }
> +    }
> +}
> +
> +impl Registration {
> +    /// Register a new SoC device
> +    pub fn register(attr: Attributes) -> impl PinInit<Self, Error> {

Let's just call this Registration::new() please. We usually use new() if we
return a Registration object (or an initializer as in this case) and register()
if we do not return a Registration object, but rather automatically clean up the
registration silently, e.g. through devres.

> +        try_pin_init!(&this in Self {

You should be able to just access Self::attr directly, i.e. no need for &this.
(When you access attr within the code block of soc_dev it will be Self::attr and
not the attr from the function argument.)

Please find a diff [1] below.

> +                    attr: attr.build(),
> +                    // SAFETY: We have already initialized attr, and we are inside PinInit and Self
> +                    // is !Unpin, so attr won't be moved and is valid. If it returns success, attr
> +                    // will not be dropped until after our `PinnedDrop` implementation runs, so the
> +                    // device will be unregistered first.
> +                    soc_dev: unsafe {
> +                        register_device(Pin::new_unchecked(&(*this.as_ptr()).attr))?
> +                    },
> +        }? Error)
> +    }
> +}


[1]

diff --git a/rust/kernel/soc.rs b/rust/kernel/soc.rs
index fb9e46121878..242dcd09e7f5 100644
--- a/rust/kernel/soc.rs
+++ b/rust/kernel/soc.rs
@@ -89,22 +89,6 @@ fn build(self) -> BuiltAttributes {
     }
 }
 
-/// # Safety
-/// If a device is returned (e.g. no error), `attr` must remain valid for reads until the
-/// returned pointer is released through `soc_device_unregister`.
-unsafe fn register_device(attr: Pin<&BuiltAttributes>) -> Result<NonNull<bindings::soc_device>> {
-    let raw_soc =
-            // SAFETY:
-            // * The struct provided through attr is backed by pinned data next to it, so as
-            //   long as attr lives, the strings pointed to by the struct will too.
-            // * `attr` is pinned, so the pinned data won't move.
-            // * If it returns a device, and so others may try to read this data, by caller
-            //   invariant, `attr` won't be released until the device is.
-            error::from_err_ptr(unsafe { bindings::soc_device_register(attr.as_mut_ptr()) })?;
-    // `soc_device_register` should not return NULL, but it doesn't hurt to be paranoid.
-    NonNull::new(raw_soc).ok_or(EINVAL)
-}
-
 #[pin_data(PinnedDrop)]
 /// Registration handle for your soc_dev. If you let it go out of scope, your soc_dev will be
 /// unregistered.
@@ -131,14 +115,24 @@ fn drop(self: Pin<&mut Self>) {
 impl Registration {
     /// Register a new SoC device
     pub fn register(attr: Attributes) -> impl PinInit<Self, Error> {
-        try_pin_init!(&this in Self {
+        try_pin_init!(Self {
                     attr: attr.build(),
                     // SAFETY: We have already initialized attr, and we are inside PinInit and Self
                     // is !Unpin, so attr won't be moved and is valid. If it returns success, attr
                     // will not be dropped until after our `PinnedDrop` implementation runs, so the
                     // device will be unregistered first.
-                    soc_dev: unsafe {
-                        register_device(Pin::new_unchecked(&(*this.as_ptr()).attr))?
+                    soc_dev: {
+                        // SAFETY:
+                        // * The struct provided through attr is backed by pinned data next to it,
+                        //   so as long as attr lives, the strings pointed to by the struct will too.
+                        // * `attr` is pinned, so the pinned data won't move.
+                        // * If it returns a device, and so others may try to read this data, by
+                        //   caller invariant, `attr` won't be released until the device is.
+                        let raw_soc = error::from_err_ptr(unsafe {
+                            bindings::soc_device_register(attr.as_mut_ptr())
+                        })?;
+
+                        NonNull::new(raw_soc).ok_or(EINVAL)?
                     },
         }? Error)
     }


^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-12-16 10:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-16  0:43 [PATCH v2 0/3] Support exporting SoC info from Rust Matthew Maurer
2025-12-16  0:43 ` [PATCH v2 1/3] rust: Add soc_device support Matthew Maurer
2025-12-16 10:20   ` Danilo Krummrich
2025-12-16  0:43 ` [PATCH v2 2/3] docs: ABI: sysfs-devices-soc: Fix swapped sample values Matthew Maurer
2025-12-16  9:22   ` Lee Jones
2025-12-16  0:43 ` [PATCH v2 3/3] rust: Add SoC Driver Sample Matthew Maurer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox