[PATCH v2 0/4] rust: i2c: Add basic I2C driver abstractions

[PATCH v2 0/4] rust: i2c: Add basic I2C driver abstractions

[Igor Korotin]

This patch series lays the groundwork for writing Linux I2C drivers in Rust by:

 1. Core abstractions 
    Introduce `i2c::Device`, `i2c::Driver` and `i2c::Adapter` built on the
    existing `struct i2c_client` and `struct i2c_driver`, with safe Rust wrappers
    around probe, transfer, and teardown logic.

 2. Manual device creation  
    Provide an API to register an I2C device at runtime from Rust using
    `I2cBoardInfo`, including automatic cleanup when the driver unloads.

 3. Sample driver (legacy table, OF & ACPI)  
    Add `rust_driver_i2c`, a sample that binds to an I2C client via:
      - legacy I2C-ID table,
      - Open Firmware (device-tree) compatible strings, or
      - ACPI IDs.

 4. Sample for manual registration  
    Add `rust_device_i2c`, a sample demonstrating how to create an I²C device
    on a given `I2cAdapterRef`, and how to unregister it automatically.

Together, these four patches:

- Establish the essential Rust traits and types for I2C drivers.
- Enable driver binding via legacy ID table, device-tree (OF), or ACPI
- Enable manual device creation at runtime.
- Ship two samples showing typical usage: one for firmware- or table-based binding,
  and one for manual registration.

Igor Korotin (4):
  rust: i2c: add basic I2C device and driver abstractions
  rust: i2c: add manual I2C device creation abstractions
  samples: rust: add Rust I2C sample driver
  samples: rust: add Rust manual I2C device creation sample

Changelog
---------
v2:
 - Merged separated ACPI support patches since ACPI-table support is 
   merged into driver-core-next.
 - Added I2cAdapterRef and I2cBoardInfo abstractions 
 - Added DeviceState generic parameter which is used for `i2c::Device`
   as a sign if the device is created manually
 - Added `DeviceOwned` abstraction which is a safe reference to a 
   manually created `i2c::Device<Ctx, state::Owned>`. 
 - Added Rust manual I2C device creation sample
 - Link to v1: https://lore.kernel.org/rust-for-linux/20250626174623.904917-1-igor.korotin.linux@gmail.com/

 MAINTAINERS                     |   4 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/i2c.c              |  15 +
 rust/kernel/i2c.rs              | 565 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 samples/rust/Kconfig            |  24 ++
 samples/rust/Makefile           |   2 +
 samples/rust/rust_device_i2c.rs |  50 +++
 samples/rust/rust_driver_i2c.rs |  69 ++++
 10 files changed, 733 insertions(+)
 create mode 100644 rust/helpers/i2c.c
 create mode 100644 rust/kernel/i2c.rs
 create mode 100644 samples/rust/rust_device_i2c.rs
 create mode 100644 samples/rust/rust_driver_i2c.rs


base-commit: b75e1f0619bd707e027812e262af3fbce445e71a
-- 
2.43.0

[PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Igor Korotin]

Implement the core abstractions needed for I2C drivers, including:

* `i2c::Driver` — the trait drivers must implement, including `probe`

* `i2c::Device` — a safe wrapper around `struct i2c_client`

* `i2c::Adapter` — implements `driver::RegistrationOps` to hook into the
  generic `driver::Registration` machinery

* `i2c::DeviceId` — a `RawDeviceId` implementation for I2C device IDs

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/helpers.c          |   1 +
 rust/helpers/i2c.c              |  15 ++
 rust/kernel/i2c.rs              | 391 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 6 files changed, 412 insertions(+)
 create mode 100644 rust/helpers/i2c.c
 create mode 100644 rust/kernel/i2c.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 7f8ddeec3b17..688a0ff23e69 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11363,6 +11363,8 @@ F:	include/linux/i2c-smbus.h
 F:	include/linux/i2c.h
 F:	include/uapi/linux/i2c-*.h
 F:	include/uapi/linux/i2c.h
+F:	rust/helpers/i2c.c
+F:	rust/kernel/i2c.rs
 
 I2C SUBSYSTEM HOST DRIVERS
 M:	Andi Shyti <andi.shyti@kernel.org>
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7e8f22850647..efc9be4d9a6e 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -53,6 +53,7 @@
 #include <linux/file.h>
 #include <linux/firmware.h>
 #include <linux/fs.h>
+#include <linux/i2c.h>
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
 #include <linux/mdio.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 0b09bd0e3561..be34554b3fab 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -23,6 +23,7 @@
 #include "drm.c"
 #include "err.c"
 #include "fs.c"
+#include "i2c.c"
 #include "io.c"
 #include "jump_label.c"
 #include "kunit.c"
diff --git a/rust/helpers/i2c.c b/rust/helpers/i2c.c
new file mode 100644
index 000000000000..5f384f8f560e
--- /dev/null
+++ b/rust/helpers/i2c.c
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/i2c.h>
+
+void* rust_helper_i2c_get_clientdata(struct i2c_client *client)
+{
+	return i2c_get_clientdata(client);
+}
+
+void rust_helper_i2c_set_clientdata(struct i2c_client *client, void *data)
+{
+	i2c_set_clientdata(client, data);
+}
+
+
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
new file mode 100644
index 000000000000..4f2f3c378153
--- /dev/null
+++ b/rust/kernel/i2c.rs
@@ -0,0 +1,391 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! I2C Driver subsystem
+
+// I2C Driver abstractions.
+use crate::{
+    acpi, container_of, device,
+    device_id::RawDeviceId,
+    driver,
+    error::*,
+    of,
+    prelude::*,
+    types::{ForeignOwnable, Opaque},
+};
+
+use core::{
+    marker::PhantomData,
+    ptr::{addr_of_mut, NonNull},
+};
+
+/// An I2C device id table.
+#[repr(transparent)]
+#[derive(Clone, Copy)]
+pub struct DeviceId(bindings::i2c_device_id);
+
+impl DeviceId {
+    const I2C_NAME_SIZE: usize = 20;
+
+    /// Create a new device id from an I2C 'id' string.
+    #[inline(always)]
+    pub const fn new(id: &'static CStr) -> Self {
+        build_assert!(
+            id.len_with_nul() <= Self::I2C_NAME_SIZE,
+            "ID exceeds 20 bytes"
+        );
+        let src = id.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: bindings::i2c_device_id = unsafe { core::mem::zeroed() };
+        let mut i = 0;
+        while i < src.len() {
+            i2c.name[i] = src[i];
+            i += 1;
+        }
+
+        Self(i2c)
+    }
+}
+
+// SAFETY:
+// * `DeviceId` is a `#[repr(transparent)` wrapper of `i2c_device_id` and does not add
+//   additional invariants, so it's safe to transmute to `RawType`.
+// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceId for DeviceId {
+    type RawType = bindings::i2c_device_id;
+
+    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::i2c_device_id, driver_data);
+
+    fn index(&self) -> usize {
+        self.0.driver_data as _
+    }
+}
+
+/// IdTable type for I2C
+pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
+
+/// Create a I2C `IdTable` with its alias for modpost.
+#[macro_export]
+macro_rules! i2c_device_table {
+    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
+        const $table_name: $crate::device_id::IdArray<
+            $crate::i2c::DeviceId,
+            $id_info_type,
+            { $table_data.len() },
+        > = $crate::device_id::IdArray::new($table_data);
+
+        $crate::module_device_table!("i2c", $module_table_name, $table_name);
+    };
+}
+
+/// An adapter for the registration of I2C drivers.
+pub struct Adapter<T: Driver>(T);
+
+// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
+// a preceding call to `register` has been successful.
+unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
+    type RegType = bindings::i2c_driver;
+
+    unsafe fn register(
+        pdrv: &Opaque<Self::RegType>,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        let i2c_table = match T::I2C_ID_TABLE {
+            Some(table) => table.as_ptr(),
+            None => core::ptr::null(),
+        };
+
+        let of_table = match T::OF_ID_TABLE {
+            Some(table) => table.as_ptr(),
+            None => core::ptr::null(),
+        };
+
+        let acpi_table = match T::ACPI_ID_TABLE {
+            Some(table) => table.as_ptr(),
+            None => core::ptr::null(),
+        };
+
+        // SAFETY: It's safe to set the fields of `struct i2c_client` on initialization.
+        unsafe {
+            (*pdrv.get()).driver.name = name.as_char_ptr();
+            (*pdrv.get()).probe = Some(Self::probe_callback);
+            (*pdrv.get()).remove = Some(Self::remove_callback);
+            (*pdrv.get()).shutdown = Some(Self::shutdown_callback);
+            (*pdrv.get()).id_table = i2c_table;
+            (*pdrv.get()).driver.of_match_table = of_table;
+            (*pdrv.get()).driver.acpi_match_table = acpi_table;
+        }
+
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::i2c_register_driver(module.0, pdrv.get()) })
+    }
+
+    unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
+        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::i2c_del_driver(pdrv.get()) }
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    extern "C" fn probe_callback(pdev: *mut bindings::i2c_client) -> kernel::ffi::c_int {
+        // SAFETY: The I2C bus only ever calls the probe callback with a valid pointer to a
+        // `struct i2c_client`.
+        //
+        // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
+        let pdev = unsafe { &*pdev.cast::<Device<device::Core>>() };
+
+        let info =
+            Self::i2c_id_info(pdev).or_else(|| <Self as driver::Adapter>::id_info(pdev.as_ref()));
+
+        match T::probe(pdev, info) {
+            Ok(data) => {
+                unsafe { bindings::i2c_set_clientdata(pdev.as_raw(), data.into_foreign() as _) };
+            }
+            Err(err) => return Error::to_errno(err),
+        }
+
+        0
+    }
+
+    extern "C" fn remove_callback(pdev: *mut bindings::i2c_client) {
+        // SAFETY: `pdev` is a valid pointer to a `struct i2c_client`.
+        let ptr = unsafe { bindings::i2c_get_clientdata(pdev) }.cast();
+
+        // SAFETY: `remove_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
+        // `KBox<T>` pointer created through `KBox::into_foreign`.
+        let _ = unsafe { KBox::<T>::from_foreign(ptr) };
+    }
+
+    extern "C" fn shutdown_callback(pdev: *mut bindings::i2c_client) {
+        let pdev = unsafe { &*pdev.cast::<Device<device::Core>>() };
+
+        T::shutdown(pdev);
+    }
+
+    /// The [`i2c::IdTable`] of the corresponding driver.
+    fn i2c_id_table() -> Option<IdTable<<Self as driver::Adapter>::IdInfo>> {
+        T::I2C_ID_TABLE
+    }
+
+    /// Returns the driver's private data from the matching entry in the [`i2c::IdTable`], if any.
+    ///
+    /// If this returns `None`, it means there is no match with an entry in the [`i2c::IdTable`].
+    fn i2c_id_info(dev: &Device) -> Option<&'static <Self as driver::Adapter>::IdInfo> {
+        let table = Self::i2c_id_table()?;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for read,
+        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
+        let raw_id = unsafe { bindings::i2c_match_id(table.as_ptr(), dev.as_raw()) };
+
+        if raw_id.is_null() {
+            None
+        } else {
+            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct i2c_device_id` and
+            // does not add additional invariants, so it's safe to transmute.
+            let id = unsafe { &*raw_id.cast::<DeviceId>() };
+
+            Some(table.info(<DeviceId as crate::device_id::RawDeviceId>::index(id)))
+        }
+    }
+}
+
+impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
+    type IdInfo = T::IdInfo;
+
+    fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
+        T::OF_ID_TABLE
+    }
+
+    fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
+        T::ACPI_ID_TABLE
+    }
+}
+
+/// Declares a kernel module that exposes a single i2c driver.
+///
+/// # Examples
+///
+/// ```ignore
+/// kernel::module_i2c_driver! {
+///     type: MyDriver,
+///     name: "Module name",
+///     authors: ["Author name"],
+///     description: "Description",
+///     license: "GPL v2",
+/// }
+/// ```
+#[macro_export]
+macro_rules! module_i2c_driver {
+    ($($f:tt)*) => {
+        $crate::module_driver!(<T>, $crate::i2c::Adapter<T>, { $($f)* });
+    };
+}
+
+/// The i2c driver trait.
+///
+/// Drivers must implement this trait in order to get a i2c driver registered.
+///
+/// # Example
+///
+///```
+/// # use kernel::{acpi, bindings, c_str, device::Core, i2c, of};
+///
+/// struct MyDriver;
+///
+/// kernel::acpi_device_table!(
+///     ACPI_TABLE,
+///     MODULE_ACPI_TABLE,
+///     <MyDriver as i2c::Driver>::IdInfo,
+///     [
+///         (acpi::DeviceId::new(c_str!("LNUXBEEF")), ())
+///     ]
+/// );
+///
+/// kernel::i2c_device_table!(
+///     I2C_TABLE,
+///     MODULE_I2C_TABLE,
+///     <MyDriver as i2c::Driver>::IdInfo,
+///     [
+///          (i2c::DeviceId::new(c_str!("rust_driver_i2c")), ())
+///     ]
+/// );
+///
+/// kernel::of_device_table!(
+///     OF_TABLE,
+///     MODULE_OF_TABLE,
+///     <MyDriver as i2c::Driver>::IdInfo,
+///     [
+///         (of::DeviceId::new(c_str!("test,device")), ())
+///     ]
+/// );
+///
+/// impl i2c::Driver for MyDriver {
+///     type IdInfo = ();
+///     const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE);
+///     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: &i2c::Device<Core>,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> Result<Pin<KBox<Self>>> {
+///         Err(ENODEV)
+///     }
+///
+///     fn shutdown(_pdev: &i2c::Device<Core>) {
+///     }
+/// }
+///```
+pub trait Driver: Send {
+    /// The type holding information about each device id supported by the driver.
+    // TODO: Use `associated_type_defaults` once stabilized:
+    //
+    // ```
+    // type IdInfo: 'static = ();
+    // ```
+    type IdInfo: 'static;
+
+    /// The table of device ids supported by the driver.
+    const I2C_ID_TABLE: Option<IdTable<Self::IdInfo>> = None;
+
+    /// The table of OF device ids supported by the driver.
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
+
+    /// The table of ACPI device ids supported by the driver.
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
+
+    /// I2C driver probe.
+    ///
+    /// Called when a new i2c device is added or discovered.
+    /// Implementers should attempt to initialize the device here.
+    fn probe(dev: &Device<device::Core>, id_info: Option<&Self::IdInfo>)
+        -> Result<Pin<KBox<Self>>>;
+
+    /// I2C driver shutdown
+    ///
+    /// Called when
+    fn shutdown(dev: &Device<device::Core>) {
+        let _ = dev;
+    }
+}
+
+/// The i2c client representation.
+///
+/// This structure represents the Rust abstraction for a C `struct i2c_client`. The
+/// implementation abstracts the usage of an already existing C `struct i2c_client` within Rust
+/// code that we get passed from the C side.
+///
+/// # Invariants
+///
+/// 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>(
+    Opaque<bindings::i2c_client>,
+    PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> Device<Ctx> {
+    fn as_raw(&self) -> *mut bindings::i2c_client {
+        self.0.get()
+    }
+}
+
+// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { Device });
+kernel::impl_device_context_into_aref!(Device);
+
+// SAFETY: Instances of `Device` are always reference-counted.
+unsafe impl crate::types::AlwaysRefCounted for Device {
+    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()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+        unsafe { bindings::put_device(addr_of_mut!((*obj.as_ref().as_raw()).dev)) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
+    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`.
+        let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::as_ref(dev) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
+    type Error = kernel::error::Error;
+
+    fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
+        // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
+        // `struct device`.
+        if unsafe { bindings::i2c_verify_client(dev.as_raw()).is_null() } {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: We've just verified that the type of `dev` equals to
+        // `bindings::i2c_client_type`, hence `dev` must be embedded in a valid
+        // `struct i2c_client` as guaranteed by the corresponding C code.
+        let pdev = unsafe { container_of!(dev.as_raw(), bindings::i2c_client, dev) };
+
+        // SAFETY: `pdev` is a valid pointer to a `struct i2c_client`.
+        Ok(unsafe { &*pdev.cast() })
+    }
+}
+
+// SAFETY: A `Device` is always reference-counted and can be released from any thread.
+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 {}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 5bbf3627212f..ee1233e44a0f 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -79,6 +79,8 @@
 #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
 pub mod firmware;
 pub mod fs;
+#[cfg(CONFIG_I2C)]
+pub mod i2c;
 pub mod init;
 pub mod io;
 pub mod ioctl;
-- 
2.43.0

[PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Igor Korotin]

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

[PATCH v2 3/4] samples: rust: add Rust I2C sample driver

[Igor Korotin]

Add a new `rust_driver_i2c` sample, showing how to bind an I2C client
in Rust via legacy I2C-ID, ACPI ID tables or OF compatible tables.

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 MAINTAINERS                     |  1 +
 samples/rust/Kconfig            | 11 ++++++
 samples/rust/Makefile           |  1 +
 samples/rust/rust_driver_i2c.rs | 69 +++++++++++++++++++++++++++++++++
 4 files changed, 82 insertions(+)
 create mode 100644 samples/rust/rust_driver_i2c.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 688a0ff23e69..82b469b8ecb9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11365,6 +11365,7 @@ F:	include/uapi/linux/i2c-*.h
 F:	include/uapi/linux/i2c.h
 F:	rust/helpers/i2c.c
 F:	rust/kernel/i2c.rs
+F:	samples/rust/rust_driver_i2c.rs
 
 I2C SUBSYSTEM HOST DRIVERS
 M:	Andi Shyti <andi.shyti@kernel.org>
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 7f7371a004ee..55aeb12cd7f7 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -62,6 +62,17 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DRIVER_I2C
+	tristate "I2C Driver"
+	depends on I2C
+	help
+	  This option builds the Rust I2C driver sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_driver_i2c.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_PCI
 	tristate "PCI Driver"
 	depends on PCI
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index bd2faad63b4f..141d8f078248 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
+obj-$(CONFIG_SAMPLE_RUST_DRIVER_I2C)		+= rust_driver_i2c.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_FAUX)		+= rust_driver_faux.o
diff --git a/samples/rust/rust_driver_i2c.rs b/samples/rust/rust_driver_i2c.rs
new file mode 100644
index 000000000000..eee3bd774c46
--- /dev/null
+++ b/samples/rust/rust_driver_i2c.rs
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust I2C driver sample.
+
+use kernel::{acpi, c_str, device::Core, i2c, of, prelude::*, types::ARef};
+
+struct SampleDriver {
+    pdev: ARef<i2c::Device>,
+}
+
+kernel::acpi_device_table! {
+    ACPI_TABLE,
+    MODULE_ACPI_TABLE,
+    <SampleDriver as i2c::Driver>::IdInfo,
+    [(acpi::DeviceId::new(c_str!("LNUXBEEF")), 0)]
+}
+
+kernel::i2c_device_table! {
+    I2C_TABLE,
+    MODULE_I2C_TABLE,
+    <SampleDriver as i2c::Driver>::IdInfo,
+    [(i2c::DeviceId::new(c_str!("rust_driver_i2c")), 0)]
+}
+
+kernel::of_device_table! {
+    OF_TABLE,
+    MODULE_OF_TABLE,
+    <SampleDriver as i2c::Driver>::IdInfo,
+    [(of::DeviceId::new(c_str!("test,rust_driver_i2c")), 0)]
+}
+
+impl i2c::Driver for SampleDriver {
+    type IdInfo = u32;
+
+    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
+    const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE);
+    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE);
+
+    fn probe(pdev: &i2c::Device<Core>, info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
+        let dev = pdev.as_ref();
+
+        dev_dbg!(dev, "Probe Rust I2C driver sample.\n");
+
+        if let Some(info) = info {
+            dev_info!(dev, "Probed with info: '{}'.\n", info);
+        }
+
+        let drvdata = KBox::new(Self { pdev: pdev.into() }, GFP_KERNEL)?;
+
+        Ok(drvdata.into())
+    }
+    fn shutdown(pdev: &i2c::Device<Core>) {
+        dev_dbg!(pdev.as_ref(), "Shutdown Rust I2C driver sample.\n");
+    }
+}
+
+impl Drop for SampleDriver {
+    fn drop(&mut self) {
+        dev_dbg!(self.pdev.as_ref(), "Remove Rust I2C driver sample.\n");
+    }
+}
+
+kernel::module_i2c_driver! {
+    type: SampleDriver,
+    name: "rust_driver_i2c",
+    authors: ["Igor Korotin"],
+    description: "Rust I2C driver",
+    license: "GPL v2",
+}
-- 
2.43.0

[PATCH v2 4/4] samples: rust: add Rust manual I2C device creation sample

[Igor Korotin]

Add a new `rust_device_i2c` sample, showing how to create I2C device
on a certain `I2CAdapterRef` using `I2cBoardInfo`. Demonstrates
automatic unregister of such I2C device when driver is unloaded

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 MAINTAINERS                     |  1 +
 samples/rust/Kconfig            | 13 +++++++++
 samples/rust/Makefile           |  1 +
 samples/rust/rust_device_i2c.rs | 50 +++++++++++++++++++++++++++++++++
 4 files changed, 65 insertions(+)
 create mode 100644 samples/rust/rust_device_i2c.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index 82b469b8ecb9..23bab3c8e1ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11365,6 +11365,7 @@ F:	include/uapi/linux/i2c-*.h
 F:	include/uapi/linux/i2c.h
 F:	rust/helpers/i2c.c
 F:	rust/kernel/i2c.rs
+F:	samples/rust/rust_device_i2c.rs
 F:	samples/rust/rust_driver_i2c.rs
 
 I2C SUBSYSTEM HOST DRIVERS
diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index 55aeb12cd7f7..394618aaf5ef 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -62,6 +62,18 @@ config SAMPLE_RUST_DMA
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_DEVICE_I2C
+	tristate "Manual I2C Device"
+	depends on I2C && I2C_CHARDEV
+	help
+	  This option builds the Rust I2C device manual creation
+	  sample.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_device_i2c.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_DRIVER_I2C
 	tristate "I2C Driver"
 	depends on I2C
@@ -124,3 +136,4 @@ config SAMPLE_RUST_HOSTPROGS
 	  If unsure, say N.
 
 endif # SAMPLES_RUST
+
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 141d8f078248..ee830da1a9d2 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_MISC_DEVICE)		+= rust_misc_device.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
 obj-$(CONFIG_SAMPLE_RUST_DMA)			+= rust_dma.o
+obj-$(CONFIG_SAMPLE_RUST_DEVICE_I2C)		+= rust_device_i2c.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_I2C)		+= rust_driver_i2c.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PCI)		+= rust_driver_pci.o
 obj-$(CONFIG_SAMPLE_RUST_DRIVER_PLATFORM)	+= rust_driver_platform.o
diff --git a/samples/rust/rust_device_i2c.rs b/samples/rust/rust_device_i2c.rs
new file mode 100644
index 000000000000..a056736b1b97
--- /dev/null
+++ b/samples/rust/rust_device_i2c.rs
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust I2C DeviceOwned usage sample.
+//!
+//! This sample driver manually creates i2c_client using I2C board info
+//! and pointer to I2C Adapter structure.
+//!
+//! For reproduction of the scenario one should compile kernel with i2c-dev and i2c-stub
+//! modules enabled. f
+
+use kernel::{c_str, device::Core, i2c, prelude::*};
+
+struct SampleDriver {
+    _owned: i2c::DeviceOwned<Core>,
+}
+
+// SAFETY: SampleDriver contains only one field `owned: DeviceOwned<Core>`,
+// which is initialized in `init()` and dropped on module unload.
+// There is no interior mutability or concurrent access to its contents
+// (all I²C operations happen in single-threaded init/drop contexts),
+// so it is safe to share &SampleDriver across threads.
+unsafe impl Sync for SampleDriver {}
+
+const BOARD_INFO: i2c::I2cBoardInfo = i2c::I2cBoardInfo::new(c_str!("rust_driver_i2c"), 0x30);
+
+impl kernel::Module for SampleDriver {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        pr_debug!("Probe Rust I2C device sample.\n");
+
+        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 SampleDriver {
+    fn drop(&mut self) {
+        pr_debug!("Drop Rust I2C device sample.\n");
+    }
+}
+
+kernel::prelude::module! {
+    type:SampleDriver,
+    name:"rust_device_i2c",
+    authors:["Igor Korotin"],
+    description:"Rust I2C device manual creation driver ",
+    license:"GPL v2",
+}
-- 
2.43.0

Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Danilo Krummrich]

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

Re: [PATCH v2 4/4] samples: rust: add Rust manual I2C device creation sample

[Danilo Krummrich]

On Fri, Jul 04, 2025 at 04:43:41PM +0100, Igor Korotin wrote:
> +struct SampleDriver {
> +    _owned: i2c::DeviceOwned<Core>,
> +}
> +
> +const BOARD_INFO: i2c::I2cBoardInfo = i2c::I2cBoardInfo::new(c_str!("rust_driver_i2c"), 0x30);
> +
> +impl kernel::Module for SampleDriver {
> +    fn init(_module: &'static ThisModule) -> Result<Self> {
> +        pr_debug!("Probe Rust I2C device sample.\n");
> +
> +        let adapter = i2c::I2cAdapterRef::get(0).ok_or(EINVAL)?;
> +
> +        let device = i2c::DeviceOwned::<Core>::new(&adapter, &BOARD_INFO).ok_or(EINVAL)?;

This can't be device::Core, since the scope of device is not limited to I2C bus
callback. Also, device::Core dereferences to device::Bound, and device is also
not limited to scope where it can be guaranteed that the device is actually
bound.

It could be device::Normal, but as mentioned in the other thread, you're
thinking this too complicated. You really want t simple i2c::Registration type,
just like auxiliary::Registration. Please take a look at that instead.

> +        Ok(Self { _owned: device })
> +    }
> +}

Re: [PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Danilo Krummrich]

On Fri, Jul 04, 2025 at 04:36:57PM +0100, Igor Korotin wrote:
> Implement the core abstractions needed for I2C drivers, including:
> 
> * `i2c::Driver` — the trait drivers must implement, including `probe`
> 
> * `i2c::Device` — a safe wrapper around `struct i2c_client`
> 
> * `i2c::Adapter` — implements `driver::RegistrationOps` to hook into the
>   generic `driver::Registration` machinery
> 
> * `i2c::DeviceId` — a `RawDeviceId` implementation for I2C device IDs
> 
> Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
> ---
>  MAINTAINERS                     |   2 +
>  rust/bindings/bindings_helper.h |   1 +
>  rust/helpers/helpers.c          |   1 +
>  rust/helpers/i2c.c              |  15 ++
>  rust/kernel/i2c.rs              | 391 ++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   2 +
>  6 files changed, 412 insertions(+)
>  create mode 100644 rust/helpers/i2c.c
>  create mode 100644 rust/kernel/i2c.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 7f8ddeec3b17..688a0ff23e69 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11363,6 +11363,8 @@ F:	include/linux/i2c-smbus.h
>  F:	include/linux/i2c.h
>  F:	include/uapi/linux/i2c-*.h
>  F:	include/uapi/linux/i2c.h
> +F:	rust/helpers/i2c.c
> +F:	rust/kernel/i2c.rs

Is this agreed with the maintainer?

There are multiple possible options, for instance:

  1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
     Optionally, the author can be added as another maintainer or reviewer.

  2) Add a separate MAINTAINERS entry; patches still go through the same
     subsystem tree.

  3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
     tree (e.g. because the subsystem maintainers don't want to deal with it).

I usually recommend (1), which is what is proposes here, but that's of course up
to you and the I2C maintainer.

@Wolfram: In case there's no agreement yet, what's your preference of
maintainership for this?

>  I2C SUBSYSTEM HOST DRIVERS
>  M:	Andi Shyti <andi.shyti@kernel.org>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 7e8f22850647..efc9be4d9a6e 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -53,6 +53,7 @@
>  #include <linux/file.h>
>  #include <linux/firmware.h>
>  #include <linux/fs.h>
> +#include <linux/i2c.h>
>  #include <linux/jiffies.h>
>  #include <linux/jump_label.h>
>  #include <linux/mdio.h>
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 0b09bd0e3561..be34554b3fab 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -23,6 +23,7 @@
>  #include "drm.c"
>  #include "err.c"
>  #include "fs.c"
> +#include "i2c.c"
>  #include "io.c"
>  #include "jump_label.c"
>  #include "kunit.c"
> diff --git a/rust/helpers/i2c.c b/rust/helpers/i2c.c
> new file mode 100644
> index 000000000000..5f384f8f560e
> --- /dev/null
> +++ b/rust/helpers/i2c.c
> @@ -0,0 +1,15 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/i2c.h>
> +
> +void* rust_helper_i2c_get_clientdata(struct i2c_client *client)
> +{
> +	return i2c_get_clientdata(client);
> +}
> +
> +void rust_helper_i2c_set_clientdata(struct i2c_client *client, void *data)
> +{
> +	i2c_set_clientdata(client, data);
> +}

Just a note, this won't be needed in the future, I have patches in the queue
that let you use generic accessors from the generic Device type. [1]

[1] https://lore.kernel.org/lkml/20250621195118.124245-3-dakr@kernel.org/

> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> new file mode 100644
> index 000000000000..4f2f3c378153
> --- /dev/null
> +++ b/rust/kernel/i2c.rs
> @@ -0,0 +1,391 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! I2C Driver subsystem
> +
> +// I2C Driver abstractions.
> +use crate::{
> +    acpi, container_of, device,
> +    device_id::RawDeviceId,
> +    driver,
> +    error::*,
> +    of,
> +    prelude::*,
> +    types::{ForeignOwnable, Opaque},
> +};
> +
> +use core::{
> +    marker::PhantomData,
> +    ptr::{addr_of_mut, NonNull},
> +};
> +
> +/// An I2C device id table.
> +#[repr(transparent)]
> +#[derive(Clone, Copy)]
> +pub struct DeviceId(bindings::i2c_device_id);
> +
> +impl DeviceId {
> +    const I2C_NAME_SIZE: usize = 20;
> +
> +    /// Create a new device id from an I2C 'id' string.
> +    #[inline(always)]
> +    pub const fn new(id: &'static CStr) -> Self {
> +        build_assert!(
> +            id.len_with_nul() <= Self::I2C_NAME_SIZE,
> +            "ID exceeds 20 bytes"
> +        );
> +        let src = id.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: bindings::i2c_device_id = unsafe { core::mem::zeroed() };
> +        let mut i = 0;
> +        while i < src.len() {
> +            i2c.name[i] = src[i];
> +            i += 1;
> +        }
> +
> +        Self(i2c)
> +    }
> +}
> +
> +// SAFETY:
> +// * `DeviceId` is a `#[repr(transparent)` wrapper of `i2c_device_id` and does not add
> +//   additional invariants, so it's safe to transmute to `RawType`.
> +// * `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
> +unsafe impl RawDeviceId for DeviceId {
> +    type RawType = bindings::i2c_device_id;
> +
> +    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::i2c_device_id, driver_data);
> +
> +    fn index(&self) -> usize {
> +        self.0.driver_data as _
> +    }
> +}

Just a heads-up, this trait will be split up. [2]

[2] https://lore.kernel.org/lkml/20250704041003.734033-2-fujita.tomonori@gmail.com/

> +/// IdTable type for I2C
> +pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
> +
> +/// Create a I2C `IdTable` with its alias for modpost.
> +#[macro_export]
> +macro_rules! i2c_device_table {
> +    ($table_name:ident, $module_table_name:ident, $id_info_type: ty, $table_data: expr) => {
> +        const $table_name: $crate::device_id::IdArray<
> +            $crate::i2c::DeviceId,
> +            $id_info_type,
> +            { $table_data.len() },
> +        > = $crate::device_id::IdArray::new($table_data);
> +
> +        $crate::module_device_table!("i2c", $module_table_name, $table_name);
> +    };
> +}
> +
> +/// An adapter for the registration of I2C drivers.
> +pub struct Adapter<T: Driver>(T);
> +
> +// SAFETY: A call to `unregister` for a given instance of `RegType` is guaranteed to be valid if
> +// a preceding call to `register` has been successful.
> +unsafe impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> {
> +    type RegType = bindings::i2c_driver;
> +
> +    unsafe fn register(
> +        pdrv: &Opaque<Self::RegType>,
> +        name: &'static CStr,
> +        module: &'static ThisModule,
> +    ) -> Result {
> +        let i2c_table = match T::I2C_ID_TABLE {
> +            Some(table) => table.as_ptr(),
> +            None => core::ptr::null(),
> +        };
> +
> +        let of_table = match T::OF_ID_TABLE {
> +            Some(table) => table.as_ptr(),
> +            None => core::ptr::null(),
> +        };
> +
> +        let acpi_table = match T::ACPI_ID_TABLE {
> +            Some(table) => table.as_ptr(),
> +            None => core::ptr::null(),
> +        };
> +
> +        // SAFETY: It's safe to set the fields of `struct i2c_client` on initialization.
> +        unsafe {
> +            (*pdrv.get()).driver.name = name.as_char_ptr();
> +            (*pdrv.get()).probe = Some(Self::probe_callback);
> +            (*pdrv.get()).remove = Some(Self::remove_callback);
> +            (*pdrv.get()).shutdown = Some(Self::shutdown_callback);
> +            (*pdrv.get()).id_table = i2c_table;
> +            (*pdrv.get()).driver.of_match_table = of_table;
> +            (*pdrv.get()).driver.acpi_match_table = acpi_table;
> +        }
> +
> +        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
> +        to_result(unsafe { bindings::i2c_register_driver(module.0, pdrv.get()) })
> +    }
> +
> +    unsafe fn unregister(pdrv: &Opaque<Self::RegType>) {
> +        // SAFETY: `pdrv` is guaranteed to be a valid `RegType`.
> +        unsafe { bindings::i2c_del_driver(pdrv.get()) }
> +    }
> +}
> +
> +impl<T: Driver + 'static> Adapter<T> {
> +    extern "C" fn probe_callback(pdev: *mut bindings::i2c_client) -> kernel::ffi::c_int {
> +        // SAFETY: The I2C bus only ever calls the probe callback with a valid pointer to a
> +        // `struct i2c_client`.
> +        //
> +        // INVARIANT: `pdev` is valid for the duration of `probe_callback()`.
> +        let pdev = unsafe { &*pdev.cast::<Device<device::Core>>() };
> +
> +        let info =
> +            Self::i2c_id_info(pdev).or_else(|| <Self as driver::Adapter>::id_info(pdev.as_ref()));
> +
> +        match T::probe(pdev, info) {
> +            Ok(data) => {
> +                unsafe { bindings::i2c_set_clientdata(pdev.as_raw(), data.into_foreign() as _) };
> +            }
> +            Err(err) => return Error::to_errno(err),
> +        }

Better use from_result() here.

> +
> +        0
> +    }
> +
> +    extern "C" fn remove_callback(pdev: *mut bindings::i2c_client) {
> +        // SAFETY: `pdev` is a valid pointer to a `struct i2c_client`.
> +        let ptr = unsafe { bindings::i2c_get_clientdata(pdev) }.cast();
> +
> +        // SAFETY: `remove_callback` is only ever called after a successful call to
> +        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
> +        // `KBox<T>` pointer created through `KBox::into_foreign`.
> +        let _ = unsafe { KBox::<T>::from_foreign(ptr) };

I like to do that as well, but I usually get asked to use drop() instead, let's
do that here as well. :)

> +    }
> +
> +    extern "C" fn shutdown_callback(pdev: *mut bindings::i2c_client) {
> +        let pdev = unsafe { &*pdev.cast::<Device<device::Core>>() };
> +
> +        T::shutdown(pdev);
> +    }
> +
> +    /// The [`i2c::IdTable`] of the corresponding driver.
> +    fn i2c_id_table() -> Option<IdTable<<Self as driver::Adapter>::IdInfo>> {
> +        T::I2C_ID_TABLE
> +    }
> +
> +    /// Returns the driver's private data from the matching entry in the [`i2c::IdTable`], if any.
> +    ///
> +    /// If this returns `None`, it means there is no match with an entry in the [`i2c::IdTable`].
> +    fn i2c_id_info(dev: &Device) -> Option<&'static <Self as driver::Adapter>::IdInfo> {
> +        let table = Self::i2c_id_table()?;
> +
> +        // SAFETY:
> +        // - `table` has static lifetime, hence it's valid for read,
> +        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
> +        let raw_id = unsafe { bindings::i2c_match_id(table.as_ptr(), dev.as_raw()) };
> +
> +        if raw_id.is_null() {
> +            None
> +        } else {
> +            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct i2c_device_id` and

Nit: Missing ']', probably a copy-paste mistake from other busses.

> +            // does not add additional invariants, so it's safe to transmute.
> +            let id = unsafe { &*raw_id.cast::<DeviceId>() };
> +
> +            Some(table.info(<DeviceId as crate::device_id::RawDeviceId>::index(id)))
> +        }
> +    }
> +}
> +
> +impl<T: Driver + 'static> driver::Adapter for Adapter<T> {
> +    type IdInfo = T::IdInfo;
> +
> +    fn of_id_table() -> Option<of::IdTable<Self::IdInfo>> {
> +        T::OF_ID_TABLE
> +    }
> +
> +    fn acpi_id_table() -> Option<acpi::IdTable<Self::IdInfo>> {
> +        T::ACPI_ID_TABLE
> +    }
> +}
> +
> +/// Declares a kernel module that exposes a single i2c driver.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// kernel::module_i2c_driver! {
> +///     type: MyDriver,
> +///     name: "Module name",
> +///     authors: ["Author name"],
> +///     description: "Description",
> +///     license: "GPL v2",
> +/// }
> +/// ```
> +#[macro_export]
> +macro_rules! module_i2c_driver {
> +    ($($f:tt)*) => {
> +        $crate::module_driver!(<T>, $crate::i2c::Adapter<T>, { $($f)* });
> +    };
> +}
> +
> +/// The i2c driver trait.
> +///
> +/// Drivers must implement this trait in order to get a i2c driver registered.
> +///
> +/// # Example
> +///
> +///```
> +/// # use kernel::{acpi, bindings, c_str, device::Core, i2c, of};
> +///
> +/// struct MyDriver;
> +///
> +/// kernel::acpi_device_table!(
> +///     ACPI_TABLE,
> +///     MODULE_ACPI_TABLE,
> +///     <MyDriver as i2c::Driver>::IdInfo,
> +///     [
> +///         (acpi::DeviceId::new(c_str!("LNUXBEEF")), ())
> +///     ]
> +/// );
> +///
> +/// kernel::i2c_device_table!(
> +///     I2C_TABLE,
> +///     MODULE_I2C_TABLE,
> +///     <MyDriver as i2c::Driver>::IdInfo,
> +///     [
> +///          (i2c::DeviceId::new(c_str!("rust_driver_i2c")), ())
> +///     ]
> +/// );
> +///
> +/// kernel::of_device_table!(
> +///     OF_TABLE,
> +///     MODULE_OF_TABLE,
> +///     <MyDriver as i2c::Driver>::IdInfo,
> +///     [
> +///         (of::DeviceId::new(c_str!("test,device")), ())
> +///     ]
> +/// );
> +///
> +/// impl i2c::Driver for MyDriver {
> +///     type IdInfo = ();
> +///     const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE);
> +///     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: &i2c::Device<Core>,
> +///         _id_info: Option<&Self::IdInfo>,
> +///     ) -> Result<Pin<KBox<Self>>> {
> +///         Err(ENODEV)
> +///     }
> +///
> +///     fn shutdown(_pdev: &i2c::Device<Core>) {
> +///     }
> +/// }
> +///```
> +pub trait Driver: Send {
> +    /// The type holding information about each device id supported by the driver.
> +    // TODO: Use `associated_type_defaults` once stabilized:
> +    //
> +    // ```
> +    // type IdInfo: 'static = ();
> +    // ```
> +    type IdInfo: 'static;
> +
> +    /// The table of device ids supported by the driver.
> +    const I2C_ID_TABLE: Option<IdTable<Self::IdInfo>> = None;
> +
> +    /// The table of OF device ids supported by the driver.
> +    const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
> +
> +    /// The table of ACPI device ids supported by the driver.
> +    const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None;
> +
> +    /// I2C driver probe.
> +    ///
> +    /// Called when a new i2c device is added or discovered.
> +    /// Implementers should attempt to initialize the device here.
> +    fn probe(dev: &Device<device::Core>, id_info: Option<&Self::IdInfo>)
> +        -> Result<Pin<KBox<Self>>>;
> +
> +    /// I2C driver shutdown
> +    ///
> +    /// Called when

Yes? :)

> +    fn shutdown(dev: &Device<device::Core>) {
> +        let _ = dev;
> +    }
> +}
> +
> +/// The i2c client representation.
> +///
> +/// This structure represents the Rust abstraction for a C `struct i2c_client`. The
> +/// implementation abstracts the usage of an already existing C `struct i2c_client` within Rust
> +/// code that we get passed from the C side.
> +///
> +/// # Invariants
> +///
> +/// 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>(
> +    Opaque<bindings::i2c_client>,
> +    PhantomData<Ctx>,
> +);
> +
> +impl<Ctx: device::DeviceContext> Device<Ctx> {
> +    fn as_raw(&self) -> *mut bindings::i2c_client {
> +        self.0.get()
> +    }
> +}
> +
> +// SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic
> +// argument.
> +kernel::impl_device_context_deref!(unsafe { Device });
> +kernel::impl_device_context_into_aref!(Device);
> +
> +// SAFETY: Instances of `Device` are always reference-counted.
> +unsafe impl crate::types::AlwaysRefCounted for Device {
> +    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()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {
> +        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> +        unsafe { bindings::put_device(addr_of_mut!((*obj.as_ref().as_raw()).dev)) }
> +    }
> +}
> +
> +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
> +    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`.
> +        let dev = unsafe { addr_of_mut!((*self.as_raw()).dev) };
> +
> +        // SAFETY: `dev` points to a valid `struct device`.
> +        unsafe { device::Device::as_ref(dev) }
> +    }
> +}
> +
> +impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &Device<Ctx> {
> +    type Error = kernel::error::Error;
> +
> +    fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
> +        // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
> +        // `struct device`.
> +        if unsafe { bindings::i2c_verify_client(dev.as_raw()).is_null() } {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: We've just verified that the type of `dev` equals to
> +        // `bindings::i2c_client_type`, hence `dev` must be embedded in a valid
> +        // `struct i2c_client` as guaranteed by the corresponding C code.
> +        let pdev = unsafe { container_of!(dev.as_raw(), bindings::i2c_client, dev) };
> +
> +        // SAFETY: `pdev` is a valid pointer to a `struct i2c_client`.
> +        Ok(unsafe { &*pdev.cast() })
> +    }
> +}
> +
> +// SAFETY: A `Device` is always reference-counted and can be released from any thread.
> +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 {}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 5bbf3627212f..ee1233e44a0f 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -79,6 +79,8 @@
>  #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]
>  pub mod firmware;
>  pub mod fs;
> +#[cfg(CONFIG_I2C)]
> +pub mod i2c;

For now I think you have to ensure that I2C is built-in.

>  pub mod init;
>  pub mod io;
>  pub mod ioctl;
> -- 
> 2.43.0
> 

Re: [PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Igor Korotin]

On 7/4/25 21:16, Danilo Krummrich wrote:
> On Fri, Jul 04, 2025 at 04:36:57PM +0100, Igor Korotin wrote:
>> Implement the core abstractions needed for I2C drivers, including:
>>
>> * `i2c::Driver` — the trait drivers must implement, including `probe`
>>
>> * `i2c::Device` — a safe wrapper around `struct i2c_client`
>>
>> * `i2c::Adapter` — implements `driver::RegistrationOps` to hook into the
>>   generic `driver::Registration` machinery
>>
>> * `i2c::DeviceId` — a `RawDeviceId` implementation for I2C device IDs
>>
>> Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
>> ---
>>  MAINTAINERS                     |   2 +
>>  rust/bindings/bindings_helper.h |   1 +
>>  rust/helpers/helpers.c          |   1 +
>>  rust/helpers/i2c.c              |  15 ++
>>  rust/kernel/i2c.rs              | 391 ++++++++++++++++++++++++++++++++
>>  rust/kernel/lib.rs              |   2 +
>>  6 files changed, 412 insertions(+)
>>  create mode 100644 rust/helpers/i2c.c
>>  create mode 100644 rust/kernel/i2c.rs
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 7f8ddeec3b17..688a0ff23e69 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11363,6 +11363,8 @@ F:	include/linux/i2c-smbus.h
>>  F:	include/linux/i2c.h
>>  F:	include/uapi/linux/i2c-*.h
>>  F:	include/uapi/linux/i2c.h
>> +F:	rust/helpers/i2c.c
>> +F:	rust/kernel/i2c.rs
> 
> Is this agreed with the maintainer?
> 
> There are multiple possible options, for instance:
> 
>   1) Maintain the Rust abstractions as part of the existing MAINTAINERS entry.
>      Optionally, the author can be added as another maintainer or reviewer.
> 
>   2) Add a separate MAINTAINERS entry; patches still go through the same
>      subsystem tree.
> 
>   3) Add a separate MAINTAINERS entry; patches don't go through the subsystem
>      tree (e.g. because the subsystem maintainers don't want to deal with it).
> 
> I usually recommend (1), which is what is proposes here, but that's of course up
> to you and the I2C maintainer.
> 
> @Wolfram: In case there's no agreement yet, what's your preference of
> maintainership for this?
> 

Oh, that makes sense - I didn’t realize I needed the current
maintainer’s sign-off to add new files under their MAINTAINERS entry.

As for being added as a reviewer or co-maintainer, I’m not yet confident
in my Rust skills. I’m learning Rust from scratch and, given my
extensive C-kernel background, I thought I’d start by contributing
something useful to the Rust side.

>> +    }
>> +
>> +    extern "C" fn shutdown_callback(pdev: *mut bindings::i2c_client) {
>> +        let pdev = unsafe { &*pdev.cast::<Device<device::Core>>() };
>> +
>> +        T::shutdown(pdev);
>> +    }
>> +
>> +    /// The [`i2c::IdTable`] of the corresponding driver.
>> +    fn i2c_id_table() -> Option<IdTable<<Self as driver::Adapter>::IdInfo>> {
>> +        T::I2C_ID_TABLE
>> +    }
>> +
>> +    /// Returns the driver's private data from the matching entry in the [`i2c::IdTable`], if any.
>> +    ///
>> +    /// If this returns `None`, it means there is no match with an entry in the [`i2c::IdTable`].
>> +    fn i2c_id_info(dev: &Device) -> Option<&'static <Self as driver::Adapter>::IdInfo> {
>> +        let table = Self::i2c_id_table()?;
>> +
>> +        // SAFETY:
>> +        // - `table` has static lifetime, hence it's valid for read,
>> +        // - `dev` is guaranteed to be valid while it's alive, and so is `pdev.as_ref().as_raw()`.
>> +        let raw_id = unsafe { bindings::i2c_match_id(table.as_ptr(), dev.as_raw()) };
>> +
>> +        if raw_id.is_null() {
>> +            None
>> +        } else {
>> +            // SAFETY: `DeviceId` is a `#[repr(transparent)` wrapper of `struct i2c_device_id` and
> 
> Nit: Missing ']', probably a copy-paste mistake from other busses.
> 
> 
> Yes? :)
> 

Yes, a lot of the code in this patch series is copy-paste of other
chunks of the existing Rust code because these parts are not part of
generic device/driver code.

I've made the same in ACPI patch series. I have already asked in that
patch series, if I need explicitly
mention that in the code or commit messages, I'm happy to do that.

>
> Just a note, this won't be needed in the future, I have patches in the
queue
> that let you use generic accessors from the generic Device type. [1]
>
> [1] https://lore.kernel.org/lkml/20250621195118.124245-3-dakr@kernel.org/
>

>
> Just a heads-up, this trait will be split up. [2]
>
> [2]
https://lore.kernel.org/lkml/20250704041003.734033-2-fujita.tomonori@gmail.com/
>

>
> Better use from_result() here.
>

>
> I like to do that as well, but I usually get asked to use drop()
instead, let's
> do that here as well. :)
>

> 
> For now I think you have to ensure that I2C is built-in.
> 

Noted. Thanks for the review.

Best Regards
Igor

Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Igor Korotin]

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

Re: [PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Miguel Ojeda]

On Mon, Jul 7, 2025 at 12:31 PM Igor Korotin
<igor.korotin.linux@gmail.com> wrote:
>
> As for being added as a reviewer or co-maintainer, I’m not yet confident
> in my Rust skills. I’m learning Rust from scratch and, given my
> extensive C-kernel background, I thought I’d start by contributing
> something useful to the Rust side.

At the moment, for any given subsystem, it is possible that
maintainers have even less Rust experience than you do :)

In general, it never hurts to offer to help with maintenance -- it
shows you are committed to the code you want to add etc.

Cheers,
Miguel

Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Igor Korotin]

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

Re: [PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Igor Korotin]

On 7/7/25 11:47, Miguel Ojeda wrote:
> On Mon, Jul 7, 2025 at 12:31 PM Igor Korotin
> <igor.korotin.linux@gmail.com> wrote:
>>
>> As for being added as a reviewer or co-maintainer, I’m not yet confident
>> in my Rust skills. I’m learning Rust from scratch and, given my
>> extensive C-kernel background, I thought I’d start by contributing
>> something useful to the Rust side.
> 
> At the moment, for any given subsystem, it is possible that
> maintainers have even less Rust experience than you do :)
> 
> In general, it never hurts to offer to help with maintenance -- it
> shows you are committed to the code you want to add etc.
> 

Sounds reasonable from this perspective. In that case waiting for a
@Wolfram's response.

Best Regards
Igor

Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Danilo Krummrich]

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

Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Igor Korotin]

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

Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Danilo Krummrich]

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.

Re: [PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Igor Korotin]

On 7/4/25 21:16, Danilo Krummrich wrote:
>> +
>> +        0
>> +    }
>> +
>> +    extern "C" fn remove_callback(pdev: *mut bindings::i2c_client) {
>> +        // SAFETY: `pdev` is a valid pointer to a `struct i2c_client`.
>> +        let ptr = unsafe { bindings::i2c_get_clientdata(pdev) }.cast();
>> +
>> +        // SAFETY: `remove_callback` is only ever called after a successful call to
>> +        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
>> +        // `KBox<T>` pointer created through `KBox::into_foreign`.
>> +        let _ = unsafe { KBox::<T>::from_foreign(ptr) };
> 
> I like to do that as well, but I usually get asked to use drop() instead, let's
> do that here as well. :)
> 

Danilo, could you, please, explain this one a little bit more? I see the
same pattern for all the implemented subsystems so far: auxiliary,
platform, pci.
If this pattern for these subsystems is to be changed, then I'm fine.
Otherwise I'm not sure I understand the difference between these 3 and
I2C Adapter abstraction

Thanks
Igor

Re: [PATCH v2 1/4] rust: i2c: add basic I2C device and driver abstractions

[Danilo Krummrich]

On 7/10/25 4:04 PM, Igor Korotin wrote:
> On 7/4/25 21:16, Danilo Krummrich wrote:
>>> +
>>> +        0
>>> +    }
>>> +
>>> +    extern "C" fn remove_callback(pdev: *mut bindings::i2c_client) {
>>> +        // SAFETY: `pdev` is a valid pointer to a `struct i2c_client`.
>>> +        let ptr = unsafe { bindings::i2c_get_clientdata(pdev) }.cast();
>>> +
>>> +        // SAFETY: `remove_callback` is only ever called after a successful call to
>>> +        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized
>>> +        // `KBox<T>` pointer created through `KBox::into_foreign`.
>>> +        let _ = unsafe { KBox::<T>::from_foreign(ptr) };
>>
>> I like to do that as well, but I usually get asked to use drop() instead, let's
>> do that here as well. :)
>>
> 
> Danilo, could you, please, explain this one a little bit more? I see the
> same pattern for all the implemented subsystems so far: auxiliary,
> platform, pci.
> If this pattern for these subsystems is to be changed, then I'm fine.

Yes, this is because I wrote those and wasn't caught not using drop(), but `let
_ = ...` instead. :)

It was already changed by [1], which you want to rebase onto, the changes are in
driver-core-next.

[1] https://lore.kernel.org/r/20250621195118.124245-8-dakr@kernel.org