[PATCH v2 2/4] rust: nvmem: Add an abstraction for nvmem providers

Link Mauve posted 4 patches 5 days, 17 hours ago
[PATCH v2 2/4] rust: nvmem: Add an abstraction for nvmem providers
Posted by Link Mauve 5 days, 17 hours ago
This is my very first Rust abstraction in the kernel, I took inspiration
from various other abstractions that had already been written.

I only implemented enough to rewrite a very simple driver I wrote in the
past, in order to test the API and make sure it’s at least as ergonomic
as the C version, without any unsafe at all.

Signed-off-by: Link Mauve <linkmauve@linkmauve.fr>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/lib.rs              |   2 +
 rust/kernel/nvmem.rs            | 163 ++++++++++++++++++++++++++++++++
 3 files changed, 166 insertions(+)
 create mode 100644 rust/kernel/nvmem.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index a067038b4b42..522a76b2e294 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -65,6 +65,7 @@
 #include <linux/mdio.h>
 #include <linux/mm.h>
 #include <linux/miscdevice.h>
+#include <linux/nvmem-provider.h>
 #include <linux/of_device.h>
 #include <linux/pci.h>
 #include <linux/phy.h>
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 696f62f85eb5..97f3fc1e8e12 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -118,6 +118,8 @@
 #[cfg(CONFIG_NET)]
 pub mod net;
 pub mod num;
+#[cfg(CONFIG_NVMEM)]
+pub mod nvmem;
 pub mod of;
 #[cfg(CONFIG_PM_OPP)]
 pub mod opp;
diff --git a/rust/kernel/nvmem.rs b/rust/kernel/nvmem.rs
new file mode 100644
index 000000000000..4b81fd65c9a7
--- /dev/null
+++ b/rust/kernel/nvmem.rs
@@ -0,0 +1,163 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! nvmem framework provider.
+//!
+//! Copyright (C) 2026 Link Mauve <linkmauve@linkmauve.fr>
+
+use crate::build_error;
+use crate::device::Device;
+use crate::error::{from_result, VTABLE_DEFAULT_ERROR};
+use crate::prelude::*;
+use core::marker::PhantomData;
+use macros::vtable;
+
+/// The possible types for a nvmem provider.
+#[derive(Default)]
+#[repr(u32)]
+pub enum Type {
+    /// The type of memory is unknown.
+    #[default]
+    Unknown = bindings::nvmem_type_NVMEM_TYPE_UNKNOWN,
+
+    /// Electrically erasable programmable ROM.
+    Eeprom = bindings::nvmem_type_NVMEM_TYPE_EEPROM,
+
+    /// One-time programmable memory.
+    Otp = bindings::nvmem_type_NVMEM_TYPE_OTP,
+
+    /// This memory is backed by a battery.
+    BatteryBacked = bindings::nvmem_type_NVMEM_TYPE_BATTERY_BACKED,
+
+    /// Ferroelectric RAM.
+    Fram = bindings::nvmem_type_NVMEM_TYPE_FRAM,
+}
+
+/// nvmem configuration.
+///
+/// Rust abstraction for the C `struct nvmem_config`.
+#[derive(Default)]
+#[repr(transparent)]
+pub struct NvmemConfig<T: NvmemProvider + Default> {
+    inner: bindings::nvmem_config,
+    _p: PhantomData<T>,
+}
+
+impl<T: NvmemProvider + Default> NvmemConfig<T> {
+    /// NvmemConfig's read callback.
+    ///
+    /// SAFETY: Called from C. Inputs must be valid pointers.
+    extern "C" fn reg_read(
+        context: *mut c_void,
+        offset: u32,
+        val: *mut c_void,
+        bytes: usize,
+    ) -> i32 {
+        from_result(|| {
+            let context = context.cast::<T::Priv>();
+            // SAFETY: context is a valid T::Priv as set in Device::nvmem_register().
+            let context = unsafe { &*context };
+            let val = val.cast::<u8>();
+            // SAFETY: val should be non-null, and allocated for bytes bytes.
+            let data = unsafe { core::slice::from_raw_parts_mut(val, bytes) };
+            T::read(context, offset, data).map(|()| 0)
+        })
+    }
+
+    /// NvmemConfig's write callback.
+    ///
+    /// SAFETY: Called from C. Inputs must be valid pointers.
+    extern "C" fn reg_write(
+        context: *mut c_void,
+        offset: u32,
+        // TODO: Change val from void* to const void* in the C API!
+        val: *mut c_void,
+        bytes: usize,
+    ) -> i32 {
+        from_result(|| {
+            let context = context.cast::<T::Priv>();
+            // SAFETY: context is a valid T::Priv as set in Device::nvmem_register().
+            let context = unsafe { &*context };
+            let val = val.cast::<u8>().cast_const();
+            // SAFETY: val should be non-null, and allocated for bytes bytes.
+            let data = unsafe { core::slice::from_raw_parts(val, bytes) };
+            T::write(context, offset, data).map(|()| 0)
+        })
+    }
+
+    /// Optional name.
+    pub fn with_name(mut self, name: &CStr) -> Self {
+        self.inner.name = name.as_char_ptr();
+        self
+    }
+
+    /// Type of the nvmem storage
+    pub fn with_type(mut self, type_: Type) -> Self {
+        self.inner.type_ = type_ as u32;
+        self
+    }
+
+    /// Device is read-only.
+    pub fn with_read_only(mut self, read_only: bool) -> Self {
+        self.inner.read_only = read_only;
+        self
+    }
+
+    /// Device is accessibly to root only.
+    pub fn with_root_only(mut self, root_only: bool) -> Self {
+        self.inner.root_only = root_only;
+        self
+    }
+
+    /// Device size.
+    pub fn with_size(mut self, size: i32) -> Self {
+        self.inner.size = size;
+        self
+    }
+
+    /// Minimum read/write access granularity.
+    pub fn with_word_size(mut self, word_size: i32) -> Self {
+        self.inner.word_size = word_size;
+        self
+    }
+
+    /// Minimum read/write access stride.
+    pub fn with_stride(mut self, stride: i32) -> Self {
+        self.inner.stride = stride;
+        self
+    }
+}
+
+impl Device {
+    /// Register a managed nvmem provider on the given device.
+    pub fn nvmem_register<T>(&self, mut config: NvmemConfig<T>, priv_: &T::Priv)
+    where
+        T: NvmemProvider + Default,
+    {
+        // FIXME: The last cast to mut indicates some unsoundness here.
+        config.inner.priv_ = core::ptr::from_ref(priv_).cast::<c_void>().cast_mut();
+        config.inner.dev = self.as_raw();
+        config.inner.reg_read = Some(NvmemConfig::<T>::reg_read);
+        config.inner.reg_write = Some(NvmemConfig::<T>::reg_write);
+        // SAFETY: Both self and config can’t be null here, and should have the correct type.
+        unsafe { bindings::devm_nvmem_register(self.as_raw(), &config.inner) };
+    }
+}
+
+/// Helper trait to define the callbacks of a nvmem provider.
+#[vtable]
+pub trait NvmemProvider {
+    /// The type passed into the context for read and write functions.
+    type Priv;
+
+    /// Callback to read data.
+    #[inline]
+    fn read(_context: &Self::Priv, _offset: u32, _data: &mut [u8]) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Callback to write data.
+    #[inline]
+    fn write(_context: &Self::Priv, _offset: u32, _data: &[u8]) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
-- 
2.52.0

Re: [PATCH v2 2/4] rust: nvmem: Add an abstraction for nvmem providers
Posted by Danilo Krummrich 5 days, 6 hours ago
On Wed Feb 4, 2026 at 5:04 AM CET, Link Mauve wrote:
> +impl Device {
> +    /// Register a managed nvmem provider on the given device.
> +    pub fn nvmem_register<T>(&self, mut config: NvmemConfig<T>, priv_: &T::Priv)
> +    where
> +        T: NvmemProvider + Default,
> +    {
> +        // FIXME: The last cast to mut indicates some unsoundness here.
> +        config.inner.priv_ = core::ptr::from_ref(priv_).cast::<c_void>().cast_mut();
> +        config.inner.dev = self.as_raw();
> +        config.inner.reg_read = Some(NvmemConfig::<T>::reg_read);
> +        config.inner.reg_write = Some(NvmemConfig::<T>::reg_write);
> +        // SAFETY: Both self and config can’t be null here, and should have the correct type.
> +        unsafe { bindings::devm_nvmem_register(self.as_raw(), &config.inner) };
> +    }
> +}

This should not be a method on the generic device type. Typically we use a
Registration struct for this, i.e. this would become
nvmem::Registration::register().
Re: [PATCH v2 2/4] rust: nvmem: Add an abstraction for nvmem providers
Posted by Link Mauve 4 days, 8 hours ago
On Wed, Feb 04, 2026 at 04:22:16PM +0100, Danilo Krummrich wrote:
> On Wed Feb 4, 2026 at 5:04 AM CET, Link Mauve wrote:
> > +impl Device {
> > +    /// Register a managed nvmem provider on the given device.
> > +    pub fn nvmem_register<T>(&self, mut config: NvmemConfig<T>, priv_: &T::Priv)
> > +    where
> > +        T: NvmemProvider + Default,
> > +    {
> > +        // FIXME: The last cast to mut indicates some unsoundness here.
> > +        config.inner.priv_ = core::ptr::from_ref(priv_).cast::<c_void>().cast_mut();
> > +        config.inner.dev = self.as_raw();
> > +        config.inner.reg_read = Some(NvmemConfig::<T>::reg_read);
> > +        config.inner.reg_write = Some(NvmemConfig::<T>::reg_write);
> > +        // SAFETY: Both self and config can’t be null here, and should have the correct type.
> > +        unsafe { bindings::devm_nvmem_register(self.as_raw(), &config.inner) };
> > +    }
> > +}
> 
> This should not be a method on the generic device type. Typically we use a
> Registration struct for this, i.e. this would become
> nvmem::Registration::register().

Should I also switch to the nvmem_register()/nvmem_unregister() API
instead of the devm_nvmem_register() API, so that the unregister can
happen in the Drop impl instead of being managed by the kernel?

-- 
Link Mauve
Re: [PATCH v2 2/4] rust: nvmem: Add an abstraction for nvmem providers
Posted by Danilo Krummrich 4 days, 8 hours ago
On Thu Feb 5, 2026 at 1:48 PM CET, Link Mauve wrote:
> On Wed, Feb 04, 2026 at 04:22:16PM +0100, Danilo Krummrich wrote:
>> On Wed Feb 4, 2026 at 5:04 AM CET, Link Mauve wrote:
>> > +impl Device {
>> > +    /// Register a managed nvmem provider on the given device.
>> > +    pub fn nvmem_register<T>(&self, mut config: NvmemConfig<T>, priv_: &T::Priv)
>> > +    where
>> > +        T: NvmemProvider + Default,
>> > +    {
>> > +        // FIXME: The last cast to mut indicates some unsoundness here.
>> > +        config.inner.priv_ = core::ptr::from_ref(priv_).cast::<c_void>().cast_mut();
>> > +        config.inner.dev = self.as_raw();
>> > +        config.inner.reg_read = Some(NvmemConfig::<T>::reg_read);
>> > +        config.inner.reg_write = Some(NvmemConfig::<T>::reg_write);
>> > +        // SAFETY: Both self and config can’t be null here, and should have the correct type.
>> > +        unsafe { bindings::devm_nvmem_register(self.as_raw(), &config.inner) };
>> > +    }
>> > +}
>> 
>> This should not be a method on the generic device type. Typically we use a
>> Registration struct for this, i.e. this would become
>> nvmem::Registration::register().
>
> Should I also switch to the nvmem_register()/nvmem_unregister() API
> instead of the devm_nvmem_register() API, so that the unregister can
> happen in the Drop impl instead of being managed by the kernel?

No, ensuring unregistration when the bus device is unbound is the correct thing
to do.

We typically support two patterns:

	impl Registration {
		fn new(dev: &Device<Bound>) -> Result<Devres<Registration>>;
		fn register(dev: &Device<Bound>) -> Result;
	}

Registration::new() still ensures unregistration when the bus device is unbound,
but also allows you to unregister before that happens.

Registration::register() is eqivalent to devm_nvmem_register().

You don't have to implement both, you can just pick the one you need for your
driver. I think in the case of nvmem you probably only every need register().