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

Danilo Krummrich
2026-02-05T12:57:23Z

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().