linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Support exporting SoC info from Rust
@ 2025-12-12 23:14 Matthew Maurer
  2025-12-12 23:14 ` [PATCH 1/2] rust: Add soc_device support Matthew Maurer
  2025-12-12 23:14 ` [PATCH 2/2] rust: Add SoC Driver Sample Matthew Maurer
  0 siblings, 2 replies; 5+ messages in thread
From: Matthew Maurer @ 2025-12-12 23:14 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

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 second patch is a sample demonstrating usage, and can be dropped
without issue.

Signed-off-by: Matthew Maurer <mmaurer@google.com>
---
Matthew Maurer (2):
      rust: Add soc_device support
      rust: Add SoC Driver Sample

 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/soc.rs              | 137 ++++++++++++++++++++++++++++++++++++++++
 samples/rust/Kconfig            |  11 ++++
 samples/rust/Makefile           |   1 +
 samples/rust/rust_soc.rs        |  74 ++++++++++++++++++++++
 7 files changed, 228 insertions(+)
---
base-commit: 008d3547aae5bc86fac3eda317489169c3fda112
change-id: 20251029-soc-bindings-9b0731bcdbed

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


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

* [PATCH 1/2] rust: Add soc_device support
  2025-12-12 23:14 [PATCH 0/2] Support exporting SoC info from Rust Matthew Maurer
@ 2025-12-12 23:14 ` Matthew Maurer
  2025-12-15 11:11   ` Danilo Krummrich
  2025-12-12 23:14 ` [PATCH 2/2] rust: Add SoC Driver Sample Matthew Maurer
  1 sibling, 1 reply; 5+ messages in thread
From: Matthew Maurer @ 2025-12-12 23:14 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

Adds the ability to register SoC devices.

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              | 137 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 141 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..b8412751a5ca8839e588cf5bd52f2e6a7f33d457
--- /dev/null
+++ b/rust/kernel/soc.rs
@@ -0,0 +1,137 @@
+// 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;
+use crate::error;
+use crate::prelude::*;
+use crate::str::CString;
+use core::marker::PhantomPinned;
+use core::ptr::addr_of;
+
+/// Attributes for a SoC device
+pub struct DeviceAttribute {
+    /// Machine
+    pub machine: Option<CString>,
+    /// Family
+    pub family: Option<CString>,
+    /// Revision
+    pub revision: Option<CString>,
+    /// Serial Number
+    pub serial_number: Option<CString>,
+    /// SoC ID
+    pub soc_id: Option<CString>,
+}
+
+// SAFETY: We provide no operations through `&BuiltDeviceAttribute`
+unsafe impl Sync for BuiltDeviceAttribute {}
+
+// SAFETY: All pointers are normal allocations, not thread-specific
+unsafe impl Send for BuiltDeviceAttribute {}
+
+#[pin_data]
+struct BuiltDeviceAttribute {
+    #[pin]
+    backing: DeviceAttribute,
+    inner: bindings::soc_device_attribute,
+    // Since `inner` has pointers to `backing`, we are !Unpin
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+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 BuiltDeviceAttribute {
+    fn as_mut_ptr(&self) -> *mut bindings::soc_device_attribute {
+        core::ptr::from_ref(&self.inner).cast_mut()
+    }
+}
+
+impl DeviceAttribute {
+    fn build(self) -> impl PinInit<BuiltDeviceAttribute> {
+        pin_init!(BuiltDeviceAttribute {
+            inner: 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,
+            _pin: PhantomPinned,
+        })
+    }
+}
+
+// SAFETY: We provide no operations through &Device
+unsafe impl Sync for Device {}
+
+// SAFETY: Device holds a pointer to a `soc_device`, which may be sent to any thread.
+unsafe impl Send for Device {}
+
+/// A registered soc device
+#[repr(transparent)]
+pub struct Device(*mut bindings::soc_device);
+
+impl Device {
+    /// # Safety
+    /// * `attr` must be pinned
+    /// * `attr` must be valid for reads during the function call
+    /// * If a device is returned (e.g. no error), `attr` must remain valid for reads until the
+    ///   returned `Device` is dropped.
+    unsafe fn register(attr: *const BuiltDeviceAttribute) -> Result<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. By caller
+            // invariant, `attr` is pinned, so the pinned data won't move. By caller invariant,
+            // `attr` is valid during this call. 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()) })?;
+        Ok(Device(raw_soc))
+    }
+}
+
+#[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 DeviceRegistration {
+    #[pin]
+    attr: BuiltDeviceAttribute,
+    soc_dev: Device,
+    // Since Device transitively points to the contents of attr, we are !Unpin
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+#[pinned_drop]
+impl PinnedDrop for DeviceRegistration {
+    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.0) }
+    }
+}
+
+impl DeviceRegistration {
+    /// Register a new SoC device
+    pub fn register(attr: DeviceAttribute) -> 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 { Device::register(addr_of!((*this.as_ptr()).attr))? },
+                    _pin: PhantomPinned,
+        }? Error)
+    }
+}

-- 
2.52.0.305.g3fc767764a-goog


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

* [PATCH 2/2] rust: Add SoC Driver Sample
  2025-12-12 23:14 [PATCH 0/2] Support exporting SoC info from Rust Matthew Maurer
  2025-12-12 23:14 ` [PATCH 1/2] rust: Add soc_device support Matthew Maurer
@ 2025-12-12 23:14 ` Matthew Maurer
  2025-12-13  7:06   ` Dirk Behme
  1 sibling, 1 reply; 5+ messages in thread
From: Matthew Maurer @ 2025-12-12 23:14 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 | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 87 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..ca0294819f100fa7b713c6b41c5b8a9b3906b863
--- /dev/null
+++ b/samples/rust/rust_soc.rs
@@ -0,0 +1,74 @@
+// 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::DeviceRegistration,
+}
+
+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!("Rust Machine"))?;
+            let family = CString::try_from(c_str!("Rust Family"))?;
+            let revision = CString::try_from(c_str!("Rust Revision"))?;
+            let serial_number = CString::try_from(c_str!("Rust Serial Number"))?;
+            let soc_id = CString::try_from(c_str!("Rust SoC ID"))?;
+
+            let attr = soc::DeviceAttribute {
+                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::DeviceRegistration::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] 5+ messages in thread

* Re: [PATCH 2/2] rust: Add SoC Driver Sample
  2025-12-12 23:14 ` [PATCH 2/2] rust: Add SoC Driver Sample Matthew Maurer
@ 2025-12-13  7:06   ` Dirk Behme
  0 siblings, 0 replies; 5+ messages in thread
From: Dirk Behme @ 2025-12-13  7:06 UTC (permalink / raw)
  To: Matthew Maurer, 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

On 13.12.25 00:14, Matthew Maurer wrote:
> 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 | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 87 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..ca0294819f100fa7b713c6b41c5b8a9b3906b863
> --- /dev/null
> +++ b/samples/rust/rust_soc.rs
> @@ -0,0 +1,74 @@
> +// 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,
> +};

Should this be formatted according to

https://docs.kernel.org/rust/coding-guidelines.html#imports

as well?

Thanks

Dirk

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

* Re: [PATCH 1/2] rust: Add soc_device support
  2025-12-12 23:14 ` [PATCH 1/2] rust: Add soc_device support Matthew Maurer
@ 2025-12-15 11:11   ` Danilo Krummrich
  0 siblings, 0 replies; 5+ messages in thread
From: Danilo Krummrich @ 2025-12-15 11:11 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 Sat Dec 13, 2025 at 12:14 AM CET, Matthew Maurer wrote:
> Adds the ability to register SoC devices.

Please use imperative mood and add at least one sentence for motivation.

> diff --git a/rust/kernel/soc.rs b/rust/kernel/soc.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..b8412751a5ca8839e588cf5bd52f2e6a7f33d457
> --- /dev/null
> +++ b/rust/kernel/soc.rs
> @@ -0,0 +1,137 @@
> +// 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;
> +use crate::error;
> +use crate::prelude::*;
> +use crate::str::CString;
> +use core::marker::PhantomPinned;
> +use core::ptr::addr_of;

Please use kernel vertical style [1].

[1] https://docs.kernel.org/rust/coding-guidelines.html#imports

> +
> +/// Attributes for a SoC device

NIT: Please end with a period.

Also, can we slightly expand the documentation please? I mean, it is fairly
obvious what it is, but maybe one brief sentence what those attributes are about
does not hurt. :)

> +pub struct DeviceAttribute {
> +    /// Machine
> +    pub machine: Option<CString>,
> +    /// Family
> +    pub family: Option<CString>,
> +    /// Revision
> +    pub revision: Option<CString>,
> +    /// Serial Number
> +    pub serial_number: Option<CString>,
> +    /// SoC ID
> +    pub soc_id: Option<CString>,

Please also expand on the documentation of the fields, only repeating the name
of the field does not seem overly useful.

For instance, the revision field could point out what kind of revision, and
whether the revision format is specific to the spcific SoC device, etc.

> +}
> +
> +// SAFETY: We provide no operations through `&BuiltDeviceAttribute`
> +unsafe impl Sync for BuiltDeviceAttribute {}
> +
> +// SAFETY: All pointers are normal allocations, not thread-specific
> +unsafe impl Send for BuiltDeviceAttribute {}

Here and in a few more places below, please end with a period.

> +#[pin_data]
> +struct BuiltDeviceAttribute {
> +    #[pin]
> +    backing: DeviceAttribute,
> +    inner: bindings::soc_device_attribute,
> +    // Since `inner` has pointers to `backing`, we are !Unpin
> +    #[pin]
> +    _pin: PhantomPinned,

It's not strictly required in this case, but I think it would be better to make
inner an Opaque<bindings::soc_device_attribute> and drop the PhantomPinned in
return.

> +}
> +
> +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 BuiltDeviceAttribute {
> +    fn as_mut_ptr(&self) -> *mut bindings::soc_device_attribute {
> +        core::ptr::from_ref(&self.inner).cast_mut()
> +    }
> +}
> +
> +impl DeviceAttribute {
> +    fn build(self) -> impl PinInit<BuiltDeviceAttribute> {
> +        pin_init!(BuiltDeviceAttribute {
> +            inner: bindings::soc_device_attribute {

You can use Opaque::new() here.

> +                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,
> +            _pin: PhantomPinned,
> +        })
> +    }
> +}
> +
> +// SAFETY: We provide no operations through &Device
> +unsafe impl Sync for Device {}
> +
> +// SAFETY: Device holds a pointer to a `soc_device`, which may be sent to any thread.
> +unsafe impl Send for Device {}
> +
> +/// A registered soc device
> +#[repr(transparent)]
> +pub struct Device(*mut bindings::soc_device);
> +
> +impl Device {
> +    /// # Safety
> +    /// * `attr` must be pinned
> +    /// * `attr` must be valid for reads during the function call
> +    /// * If a device is returned (e.g. no error), `attr` must remain valid for reads until the
> +    ///   returned `Device` is dropped.
> +    unsafe fn register(attr: *const BuiltDeviceAttribute) -> Result<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. By caller
> +            // invariant, `attr` is pinned, so the pinned data won't move. By caller invariant,
> +            // `attr` is valid during this call. 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()) })?;
> +        Ok(Device(raw_soc))
> +    }
> +}

I think the Device structure is neither used in the sample nor in your qcom
socinfo driver, and I don't see it being used in the near future either.

The only thing it does is to provide an unsafe register() function that calls
soc_device_register() which should rather be called by Registration::new()
instead. Hence, let's drop the Device struct entirely.

When moving the struct soc_device pointer to Registration, please use NonNull
instead of a raw pointer.

> +#[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 DeviceRegistration {

For consistency (DRM, auxiliary, i2c, PWM, etc.), please call this just Registration.

> +    #[pin]
> +    attr: BuiltDeviceAttribute,
> +    soc_dev: Device,
> +    // Since Device transitively points to the contents of attr, we are !Unpin
> +    #[pin]
> +    _pin: PhantomPinned,
> +}
> +
> +#[pinned_drop]
> +impl PinnedDrop for DeviceRegistration {
> +    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.0) }
> +    }
> +}
> +
> +impl DeviceRegistration {
> +    /// Register a new SoC device
> +    pub fn register(attr: DeviceAttribute) -> 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 { Device::register(addr_of!((*this.as_ptr()).attr))? },

Please prefer &raw.

> +                    _pin: PhantomPinned,
> +        }? Error)
> +    }
> +}
>
> -- 
> 2.52.0.305.g3fc767764a-goog


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

end of thread, other threads:[~2025-12-15 11:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-12 23:14 [PATCH 0/2] Support exporting SoC info from Rust Matthew Maurer
2025-12-12 23:14 ` [PATCH 1/2] rust: Add soc_device support Matthew Maurer
2025-12-15 11:11   ` Danilo Krummrich
2025-12-12 23:14 ` [PATCH 2/2] rust: Add SoC Driver Sample Matthew Maurer
2025-12-13  7:06   ` Dirk Behme

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).