In addition to the basic I2C device support, added rust abstractions
upon `i2c_new_client_device`/`i2c_unregister_device` C functions.

Implement additional generic parameter for i2c::Device that shows if
the i2c::Device is abstraction on top of "borrowed" i2c_client struct,
or it is manually created device which is "owned" by a caller.

Implement the core abstractions needed for owned I2C devices, including:

 * `i2c::state` — a generic parameter type for i2c::Device

 * `i2c::DeviceOwned` — a wrapper around
                       `i2c::Device<Ctx, i2c::state::Owned>`

 * `i2c::I2cAdapterRef` — a safe reference around `struct i2c_adapter`

 * `i2c::I2cBoardInfo` — a safe wrapper around `struct i2c_board_info`

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 rust/kernel/i2c.rs | 184 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 179 insertions(+), 5 deletions(-)

diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 4f2f3c378153..43487fdd8597 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -10,7 +10,7 @@
     error::*,
     of,
     prelude::*,
-    types::{ForeignOwnable, Opaque},
+    types::{ARef, ForeignOwnable, Opaque},
 };
 
 use core::{
@@ -312,6 +312,102 @@ fn shutdown(dev: &Device<device::Core>) {
     }
 }
 
+/// The i2c adapter reference
+///
+/// This represents the Rust abstraction for a reference to an existing
+/// C `struct i2c_adapter`.
+///
+/// # Invariants
+///
+/// A [`I2cAdapterRef`] instance represents a valid `struct i2c_adapter` created by the C portion of
+/// the kernel.
+#[repr(transparent)]
+pub struct I2cAdapterRef(NonNull<bindings::i2c_adapter>);
+
+impl I2cAdapterRef {
+    fn as_raw(&self) -> *mut bindings::i2c_adapter {
+        self.0.as_ptr()
+    }
+
+    /// Gets pointer to an `i2c_adapter` by index.
+    pub fn get(index: i32) -> Option<Self> {
+        // SAFETY: `index` must refer to a valid I²C adapter; the kernel
+        // guarantees that `i2c_get_adapter(index)` returns either a valid
+        // pointer or NULL. `NonNull::new` guarantees the correct check.
+        let adapter = NonNull::new(unsafe { bindings::i2c_get_adapter(index) })?;
+        Some(Self(adapter))
+    }
+}
+
+impl Drop for I2cAdapterRef {
+    fn drop(&mut self) {
+        // SAFETY: This `I2cAdapterRef` was obtained from `i2c_get_adapter`,
+        // and calling `i2c_put_adapter` exactly once will correctly release
+        // the reference count in the I²C core. It is safe to call from any context
+        unsafe { bindings::i2c_put_adapter(self.as_raw()) }
+    }
+}
+
+/// The i2c board info representation
+///
+/// This structure represents the Rust abstraction for a C `struct i2c_board_info` structure,
+/// which is used for manual I2C client creation.
+#[repr(transparent)]
+pub struct I2cBoardInfo(bindings::i2c_board_info);
+
+impl I2cBoardInfo {
+    const I2C_TYPE_SIZE: usize = 20;
+    /// Create a new board‐info for a kernel driver.
+    #[inline(always)]
+    pub const fn new(type_: &'static CStr, addr: u16) -> Self {
+        build_assert!(
+            type_.len_with_nul() <= Self::I2C_TYPE_SIZE,
+            "Type exceeds 20 bytes"
+        );
+        let src = type_.as_bytes_with_nul();
+        // Replace with `bindings::acpi_device_id::default()` once stabilized for `const`.
+        // SAFETY: FFI type is valid to be zero-initialized.
+        let mut i2c_board_info: bindings::i2c_board_info = unsafe { core::mem::zeroed() };
+        let mut i: usize = 0;
+        while i < src.len() {
+            i2c_board_info.type_[i] = src[i];
+            i += 1;
+        }
+
+        i2c_board_info.addr = addr;
+        Self(i2c_board_info)
+    }
+
+    fn as_raw(&self) -> *const bindings::i2c_board_info {
+        &self.0 as *const _
+    }
+}
+
+/// Marker trait for the state of a bus specific device.
+pub trait DeviceState: private::Sealed {}
+
+/// State module which aggregates existing Device States.
+pub mod state {
+    /// The [`Borrowed`] state is the state of a bus specific device when it was not
+    /// manually created using `DeviceOwned::new`
+    pub struct Borrowed;
+
+    /// The [`Owned`] state is the state of a bus specific device when it was
+    /// manually created using `DeviceOwned::new` and thus will be automatically
+    /// unregistered when the corresponding `DeviceOwned` is dropped
+    pub struct Owned;
+}
+
+mod private {
+    pub trait Sealed {}
+
+    impl Sealed for super::state::Borrowed {}
+    impl Sealed for super::state::Owned {}
+}
+
+impl DeviceState for state::Borrowed {}
+impl DeviceState for state::Owned {}
+
 /// The i2c client representation.
 ///
 /// This structure represents the Rust abstraction for a C `struct i2c_client`. The
@@ -323,15 +419,19 @@ fn shutdown(dev: &Device<device::Core>) {
 /// A [`Device`] instance represents a valid `struct i2c_client` created by the C portion of
 /// the kernel.
 #[repr(transparent)]
-pub struct Device<Ctx: device::DeviceContext = device::Normal>(
+pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
     Opaque<bindings::i2c_client>,
     PhantomData<Ctx>,
+    PhantomData<State>,
 );
 
-impl<Ctx: device::DeviceContext> Device<Ctx> {
+impl<Ctx: device::DeviceContext, State: DeviceState> Device<Ctx, State> {
     fn as_raw(&self) -> *mut bindings::i2c_client {
         self.0.get()
     }
+    fn from_raw(raw: *mut bindings::i2c_client) -> &'static Self {
+        unsafe { &*raw.cast::<Device<Ctx, State>>() }
+    }
 }
 
 // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
@@ -340,7 +440,9 @@ fn as_raw(&self) -> *mut bindings::i2c_client {
 kernel::impl_device_context_into_aref!(Device);
 
 // SAFETY: Instances of `Device` are always reference-counted.
-unsafe impl crate::types::AlwaysRefCounted for Device {
+unsafe impl<Ctx: device::DeviceContext, State: DeviceState> crate::types::AlwaysRefCounted
+    for Device<Ctx, State>
+{
     fn inc_ref(&self) {
         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
         unsafe { bindings::get_device(self.as_ref().as_raw()) };
@@ -352,7 +454,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
     }
 }
 
-impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+impl<Ctx: device::DeviceContext, State: DeviceState> AsRef<device::Device<Ctx>>
+    for Device<Ctx, State>
+{
     fn as_ref(&self) -> &device::Device<Ctx> {
         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
         // `struct i2c_client`.
@@ -389,3 +493,73 @@ unsafe impl Send for Device {}
 // SAFETY: `Device` can be shared among threads because all methods of `Device`
 // (i.e. `Device<Normal>) are thread safe.
 unsafe impl Sync for Device {}
+
+/// The representation of reference counted pointer to a manually created i2c client.
+///
+/// This structure represents the Rust wrapper upon i2c::Device with the i2c::state::Owned state
+#[repr(transparent)]
+pub struct DeviceOwned<Ctx: device::DeviceContext + 'static = device::Normal>(
+    ARef<Device<Ctx, state::Owned>>,
+);
+
+/// The main purpose of the DeviceOwned wrapper is to automatically
+/// take care of i2c client created by i2c_new_client_device.
+///
+/// The example of usage:
+///
+/// ```
+/// use kernel::{c_str, device::Core, i2c, prelude::*};
+///
+/// struct Context {
+///     _owned: i2c::DeviceOwned<Core>,
+/// }
+///
+/// const BOARD_INFO: i2c::I2cBoardInfo = i2c::I2cBoardInfo::new(c_str!("rust_driver_i2c"), 0x30);
+///
+/// impl Context {
+///     fn init(_module: &'static ThisModule) -> Result<Self> {
+///
+///         let adapter = i2c::I2cAdapterRef::get(0)
+///             .ok_or(EINVAL)?;
+///
+///         let device = i2c::DeviceOwned::<Core>::new(&adapter, &BOARD_INFO)
+///             .ok_or(EINVAL)?;
+///
+///         Ok(Self { _owned: device })
+///     }
+/// }
+///
+/// impl Drop for Context {
+///     fn drop(&mut self) {
+///
+///     }
+/// }
+///
+/// ```
+impl<Ctx: device::DeviceContext> DeviceOwned<Ctx> {
+    fn as_raw_client(&self) -> *mut bindings::i2c_client {
+        self.0.as_raw()
+    }
+
+    /// The C `i2c_new_client_device` function wrapper for manual I2C client creation.
+    pub fn new(i2c_adapter: &I2cAdapterRef, i2c_board_info: &I2cBoardInfo) -> Option<Self> {
+        // SAFETY: the kernel guarantees that `i2c_new_client_device()` returns either a valid
+        // pointer or NULL. `NonNull::new` guarantees the correct check.
+        let raw_dev = NonNull::new(unsafe {
+            bindings::i2c_new_client_device(i2c_adapter.as_raw(), i2c_board_info.as_raw())
+        })?;
+
+        let dev = Device::<Ctx, state::Owned>::from_raw(raw_dev.as_ptr());
+
+        Some(Self(dev.into()))
+    }
+}
+
+impl<Ctx: device::DeviceContext> Drop for DeviceOwned<Ctx> {
+    fn drop(&mut self) {
+        unsafe { bindings::i2c_unregister_device(self.as_raw_client()) }
+    }
+}
+
+// SAFETY: A `Device` is always reference-counted and can be released from any thread.
+unsafe impl<Ctx: device::DeviceContext> Send for DeviceOwned<Ctx> {}
-- 
2.43.0

On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>      Opaque<bindings::i2c_client>,
>      PhantomData<Ctx>,
> +    PhantomData<State>,
>  );

I see what you're doing here, but I think you're thinking this way too
complicated.

I recommend not to reuse the Device type to register a new I2C client device,
it's adding too much complexity without any real value.

You also don't want the DeviceContext types for a device registration, since the
registration will never have any other DeviceContext than device::Normal (see
also my comment on the sample module).

DeviceContext types are only useful for &Device (i.e. references) given out for
a specific scope, such as probe(), remove(), etc.

The only thing you really want to do is to register a new I2C client device, get
a i2c::Registration instance and call i2c_unregister_device() when the
i2c::Registration is dropped.

This is exactly the same use-case as we have in the auxiliary bus. I highly
recommend looking at what auxiliary::Registration does [1].

Also note that if you want a reference to the device in the i2c::Registration,
you can also add a i2c::Registration::device() method that returns an
&i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299

On 7/4/25 20:54, Danilo Krummrich wrote:
> On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
>> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>>      Opaque<bindings::i2c_client>,
>>      PhantomData<Ctx>,
>> +    PhantomData<State>,
>>  );
> 
> I see what you're doing here, but I think you're thinking this way too
> complicated.
> 
> I recommend not to reuse the Device type to register a new I2C client device,
> it's adding too much complexity without any real value.
> 
> You also don't want the DeviceContext types for a device registration, since the
> registration will never have any other DeviceContext than device::Normal (see
> also my comment on the sample module).
> 
> DeviceContext types are only useful for &Device (i.e. references) given out for
> a specific scope, such as probe(), remove(), etc.
> 
> The only thing you really want to do is to register a new I2C client device, get
> a i2c::Registration instance and call i2c_unregister_device() when the
> i2c::Registration is dropped.
> 
> This is exactly the same use-case as we have in the auxiliary bus. I highly
> recommend looking at what auxiliary::Registration does [1].
> 
> Also note that if you want a reference to the device in the i2c::Registration,
> you can also add a i2c::Registration::device() method that returns an
> &i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299

I took a quick look at the auxiliary Registration abstraction and I see
that it is not applicable for I2C subsystem. The issue here is that I2C
C code doesn't provide with an API that can registers an I2C client from
already existing `struct i2c_client`.

All the APIs provided require `i2c_adapter` with some other inputs
depending on a specific API, they return `i2c_client` being allocated
and registered by the Kernel C code.

Since I'm not controlling initial object allocation, I need somehow to
mark created i2c_client to be dropped automatically and that's why I
implemented `Borrowed/Owned` generic parameter along with this
`DeviceOwned(Device<Ctx, state::Owned>)`. One of the main purposes of
this Borrowed/Owned to prevent accidental casting of `i2c_client` structs.

Alternatively, it could be an implementation of `unregister` method that
user should call explicitly to de-register manually created
`i2c_client`, but as far as I understand this is not Rust "way".

Best Regards
Igor

On Mon, Jul 07, 2025 at 12:20:15PM +0100, Igor Korotin wrote:
> 
> 
> On 7/4/25 20:54, Danilo Krummrich wrote:
> > On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
> >> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
> >> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
> >>      Opaque<bindings::i2c_client>,
> >>      PhantomData<Ctx>,
> >> +    PhantomData<State>,
> >>  );
> > 
> > I see what you're doing here, but I think you're thinking this way too
> > complicated.
> > 
> > I recommend not to reuse the Device type to register a new I2C client device,
> > it's adding too much complexity without any real value.
> > 
> > You also don't want the DeviceContext types for a device registration, since the
> > registration will never have any other DeviceContext than device::Normal (see
> > also my comment on the sample module).
> > 
> > DeviceContext types are only useful for &Device (i.e. references) given out for
> > a specific scope, such as probe(), remove(), etc.
> > 
> > The only thing you really want to do is to register a new I2C client device, get
> > a i2c::Registration instance and call i2c_unregister_device() when the
> > i2c::Registration is dropped.
> > 
> > This is exactly the same use-case as we have in the auxiliary bus. I highly
> > recommend looking at what auxiliary::Registration does [1].
> > 
> > Also note that if you want a reference to the device in the i2c::Registration,
> > you can also add a i2c::Registration::device() method that returns an
> > &i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.
> > 
> > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299
> 
> I took a quick look at the auxiliary Registration abstraction and I see
> that it is not applicable for I2C subsystem. The issue here is that I2C
> C code doesn't provide with an API that can registers an I2C client from
> already existing `struct i2c_client`.

I don't see why the following wouldn't work:

	struct Registration(NonNull<bindings::i2c_client>);

	impl Registration {
	   pub fn new(adp: &I2cAdapterRef, info: &I2cBoardInfo) -> Result<Self> {
	      // SAFETY: [...]
	      let cli = unsafe { bindings::i2c_new_client_device(adp.as_raw(), info.as_raw()) };
	
	      // Handle ERR_PTR()
	
	      Self(NonNull::new(cli))
	   }
	}
	
	impl Drop for Registration {
	   fn drop(&mut self) {
	      // SAFETY: [...]
	      unsafe { bindings::i2c_unregister_device(self.as_ptr()) };
	   }
	}

And in you sample driver you can still the exactly the same as you did before:

	struct SampleDriver {
	   _reg: i2c::Registration,
	}
	
	impl kernel::Module for SampleDriver {
	    fn init(_module: &'static ThisModule) -> Result<Self> {
	        let adapter = i2c::I2cAdapterRef::get(0).ok_or(EINVAL)?;
	
	        let reg = i2c::Registration::new(&adapter, &BOARD_INFO)?;
	
	        Ok(Self { _reg: reg })
	    }
	}

Note that you can also combine you two sample drivers into one by doing the
above *and* register a and I2C driver that probes against your device
registration. :)

On 7/7/25 13:02, Danilo Krummrich wrote:
> On Mon, Jul 07, 2025 at 12:20:15PM +0100, Igor Korotin wrote:
>>
>>
>> On 7/4/25 20:54, Danilo Krummrich wrote:
>>> On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
>>>> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>>>> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>>>>      Opaque<bindings::i2c_client>,
>>>>      PhantomData<Ctx>,
>>>> +    PhantomData<State>,
>>>>  );
>>>
>>> I see what you're doing here, but I think you're thinking this way too
>>> complicated.
>>>
>>> I recommend not to reuse the Device type to register a new I2C client device,
>>> it's adding too much complexity without any real value.
>>>
>>> You also don't want the DeviceContext types for a device registration, since the
>>> registration will never have any other DeviceContext than device::Normal (see
>>> also my comment on the sample module).
>>>
>>> DeviceContext types are only useful for &Device (i.e. references) given out for
>>> a specific scope, such as probe(), remove(), etc.
>>>
>>> The only thing you really want to do is to register a new I2C client device, get
>>> a i2c::Registration instance and call i2c_unregister_device() when the
>>> i2c::Registration is dropped.
>>>
>>> This is exactly the same use-case as we have in the auxiliary bus. I highly
>>> recommend looking at what auxiliary::Registration does [1].
>>>
>>> Also note that if you want a reference to the device in the i2c::Registration,
>>> you can also add a i2c::Registration::device() method that returns an
>>> &i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.
>>>
>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299
>>
>> I took a quick look at the auxiliary Registration abstraction and I see
>> that it is not applicable for I2C subsystem. The issue here is that I2C
>> C code doesn't provide with an API that can registers an I2C client from
>> already existing `struct i2c_client`.
> 
> I don't see why the following wouldn't work:
> 
> 	struct Registration(NonNull<bindings::i2c_client>);
> 
> 	impl Registration {
> 	   pub fn new(adp: &I2cAdapterRef, info: &I2cBoardInfo) -> Result<Self> {
> 	      // SAFETY: [...]
> 	      let cli = unsafe { bindings::i2c_new_client_device(adp.as_raw(), info.as_raw()) };
> 	
> 	      // Handle ERR_PTR()
> 	
> 	      Self(NonNull::new(cli))
> 	   }
> 	}
> 	
> 	impl Drop for Registration {
> 	   fn drop(&mut self) {
> 	      // SAFETY: [...]
> 	      unsafe { bindings::i2c_unregister_device(self.as_ptr()) };
> 	   }
> 	}
> 
> And in you sample driver you can still the exactly the same as you did before:
> 
> 	struct SampleDriver {
> 	   _reg: i2c::Registration,
> 	}
> 	
> 	impl kernel::Module for SampleDriver {
> 	    fn init(_module: &'static ThisModule) -> Result<Self> {
> 	        let adapter = i2c::I2cAdapterRef::get(0).ok_or(EINVAL)?;
> 	
> 	        let reg = i2c::Registration::new(&adapter, &BOARD_INFO)?;
> 	
> 	        Ok(Self { _reg: reg })
> 	    }
> 	}
> 

Ok, I think I've caught the idea. In general I worried that if one has
link to an i2c::Device which is a transparent representation of `struct
i2c_client` he could somehow cast that `i2c::Device` to
`Registration(NonNull<bindings::i2c_client>)`. After some experiments, I
found out that this is not possible due to scope of the
`i2c::Device::as_raw()`. So the simple NonNull implementation is as safe
as my "super-puper secured" DeviceOwned implementation.
Thanks
Igor

On 7/7/25 4:31 PM, Igor Korotin wrote:
> On 7/7/25 13:02, Danilo Krummrich wrote:
>> On Mon, Jul 07, 2025 at 12:20:15PM +0100, Igor Korotin wrote:
>>>
>>>
>>> On 7/4/25 20:54, Danilo Krummrich wrote:
>>>> On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
>>>>> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>>>>> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>>>>>       Opaque<bindings::i2c_client>,
>>>>>       PhantomData<Ctx>,
>>>>> +    PhantomData<State>,
>>>>>   );
>>>>
>>>> I see what you're doing here, but I think you're thinking this way too
>>>> complicated.
>>>>
>>>> I recommend not to reuse the Device type to register a new I2C client device,
>>>> it's adding too much complexity without any real value.
>>>>
>>>> You also don't want the DeviceContext types for a device registration, since the
>>>> registration will never have any other DeviceContext than device::Normal (see
>>>> also my comment on the sample module).
>>>>
>>>> DeviceContext types are only useful for &Device (i.e. references) given out for
>>>> a specific scope, such as probe(), remove(), etc.
>>>>
>>>> The only thing you really want to do is to register a new I2C client device, get
>>>> a i2c::Registration instance and call i2c_unregister_device() when the
>>>> i2c::Registration is dropped.
>>>>
>>>> This is exactly the same use-case as we have in the auxiliary bus. I highly
>>>> recommend looking at what auxiliary::Registration does [1].
>>>>
>>>> Also note that if you want a reference to the device in the i2c::Registration,
>>>> you can also add a i2c::Registration::device() method that returns an
>>>> &i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.
>>>>
>>>> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299
>>>
>>> I took a quick look at the auxiliary Registration abstraction and I see
>>> that it is not applicable for I2C subsystem. The issue here is that I2C
>>> C code doesn't provide with an API that can registers an I2C client from
>>> already existing `struct i2c_client`.
>>
>> I don't see why the following wouldn't work:
>>
>> 	struct Registration(NonNull<bindings::i2c_client>);
>>
>> 	impl Registration {
>> 	   pub fn new(adp: &I2cAdapterRef, info: &I2cBoardInfo) -> Result<Self> {
>> 	      // SAFETY: [...]
>> 	      let cli = unsafe { bindings::i2c_new_client_device(adp.as_raw(), info.as_raw()) };
>> 	
>> 	      // Handle ERR_PTR()
>> 	
>> 	      Self(NonNull::new(cli))
>> 	   }
>> 	}
>> 	
>> 	impl Drop for Registration {
>> 	   fn drop(&mut self) {
>> 	      // SAFETY: [...]
>> 	      unsafe { bindings::i2c_unregister_device(self.as_ptr()) };
>> 	   }
>> 	}
>>
>> And in you sample driver you can still the exactly the same as you did before:
>>
>> 	struct SampleDriver {
>> 	   _reg: i2c::Registration,
>> 	}
>> 	
>> 	impl kernel::Module for SampleDriver {
>> 	    fn init(_module: &'static ThisModule) -> Result<Self> {
>> 	        let adapter = i2c::I2cAdapterRef::get(0).ok_or(EINVAL)?;
>> 	
>> 	        let reg = i2c::Registration::new(&adapter, &BOARD_INFO)?;
>> 	
>> 	        Ok(Self { _reg: reg })
>> 	    }
>> 	}
>>
> 
> Ok, I think I've caught the idea. In general I worried that if one has
> link to an i2c::Device which is a transparent representation of `struct
> i2c_client` he could somehow cast that `i2c::Device` to
> `Registration(NonNull<bindings::i2c_client>)`. After some experiments, I
> found out that this is not possible due to scope of the
> `i2c::Device::as_raw()`. So the simple NonNull implementation is as safe
> as my "super-puper secured" DeviceOwned implementation.

Well, with unsafe and raw pointer casts you can basically break anything,
including this design. Don't worry about it. The important part is that the
design does not allow "illegal" operations while sticking to safe code.

On 7/4/25 20:54, Danilo Krummrich wrote:
> On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
>> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>>      Opaque<bindings::i2c_client>,
>>      PhantomData<Ctx>,
>> +    PhantomData<State>,
>>  );
> 
> I see what you're doing here, but I think you're thinking this way too
> complicated.
> 
> I recommend not to reuse the Device type to register a new I2C client device,
> it's adding too much complexity without any real value.
> 
> You also don't want the DeviceContext types for a device registration, since the
> registration will never have any other DeviceContext than device::Normal (see
> also my comment on the sample module).
> 
> DeviceContext types are only useful for &Device (i.e. references) given out for
> a specific scope, such as probe(), remove(), etc.
> 
> The only thing you really want to do is to register a new I2C client device, get
> a i2c::Registration instance and call i2c_unregister_device() when the
> i2c::Registration is dropped.
> 
> This is exactly the same use-case as we have in the auxiliary bus. I highly
> recommend looking at what auxiliary::Registration does [1].
> 
> Also note that if you want a reference to the device in the i2c::Registration,
> you can also add a i2c::Registration::device() method that returns an
> &i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299

Noted. Will take a look. Thanks for the review

Best Regards
Igor