[PATCH v6 0/3] rust: i2c: Add basic I2C driver abstractions

[PATCH v6 0/3] 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::I2cClient`, `i2c::I2cAdapter`, `i2c::Driver` and 
    built on the existing `struct i2c_client`, `struct i2c_adapter` 
    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` and `I2cAdapter`, including automatic cleanup when 
    the driver unloads.

 3. Sample driver (legacy table, OF & ACPI)  
    Add `rust_driver_i2c`, a sample that:
      - creates an I2C client device using `i2c::Registration::new()`
      - binds to an I2C client via: 
        - legacy I2C-ID table, 
        - Open Firmware (device-tree) compatible strings, or
        - ACPI IDs.
      - destroyes the I2C client device on exit.

Together, these three 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 a samples showing typical usage 

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

Changelog
---------
v6:
 - Add implementation of unbind for `i2c::Driver` trait;
 - Add argument `Pin<&Self>` to `i2c::Driver::shutdown` method; 
 - Adjust usage of `i2c::Driver::shutdown` in 
   `i2c::Adapter::shutdown_callback` in `i2c::Driver` trait code 
   example and in rust_driver_i2c code;
 - Remove dummy AsRef implementation for I2cAdapter. Adjust code 
   in rust_driver_i2c;
 - Add `i2c::I2cAdapter::get_nr` method that returns I2cAdapter index;
 - Optimize unsafe sections in inc_ref/dec_ref in AlwaysRefCounted 
   for I2cAdapter implementation;
 - Remove unnecessary Drop implementation for I2cAdapter, because 
   I2cAdapter is always a reference;
 - Remove unnecessary type definition `Ops<T>` in rust_driver_i2c
 - Simplify call of `i2c::I2cAdapter::get` in `try_pin_init!` macro 
   for rust_driver_i2c
 - Link to v5: https://lore.kernel.org/rust-for-linux/20250911154717.96637-1-igor.korotin.linux@gmail.com/ 
v5:
 - Rename missed pdev variables to idev (thanks to Daniel).
 - Import `crate::device_id::RawDeviceIdIndex` and 
   `crate::types::AlwaysRefCounted` in i2c.rs.
 - Switch dev_dbg to dev_info in the sample I2C driver messages.
 - Make `I2cAdapter::get()` return `ARef<I2cAdapter>` instead of 
   `&I2cAdapter`.
 - Remove `TryFrom<device::Device<Ctx>> for I2cAdapter<Ctx>` (unused; 
   to be reintroduced in a later I2C series).
 - Remove `AsRef<device::Device<Ctx>> for I2cAdapter<Ctx>` (unused; 
   to be reintroduced in a later I2C series).
 - Add `AsRef<I2cAdapter> for I2cAdapter<Ctx>`.
 - Use i2c_get/put_adapter instead of get/put_device for 
   `AlwaysRefCounted<I2cAdapter>`.
 - Update safety comment for `unsafe impl Sync for Registration {}`.
 - Tweak comment for `I2cBoardInfo::new`.
 - Adjust build-time assertion message in `Adapter::register`.
 - Link to v4: https://lore.kernel.org/rust-for-linux/20250820151427.1812482-1-igor.korotin.linux@gmail.com/ 
v4:
 - Renamed `i2c::I2cAdapterRef` to `i2c::I2cAdapter`.
 - Renamed `i2c::Device` to `i2c::I2cClient` for consistency with 
   `i2c::I2cAdapter` and to avoid confusion with `i2c::Adapter`
 - Reworked `i2c::I2cAdapter` to be an Opaque around `i2c_adapter` struct
 - Implemented AlwaysRefCounted trait for `i2c::I2cAdapter`. 
 - Fixed numerous comment mistakes and typos all over the code, thanks 
   to Danilo and Daniel
 - Got rid of all unwrap() use-cases in i2c.rs and rust_driver_i2c.rs.
   This covers 0-day kernel panic <202508071027.8981cbd4-lkp@intel.com>
 - Removed unnecessary casts.
 - Replaced all addr_of_mut! macros to &raw mut.
 - In `i2c::Adapter::register` method build assert if all ID tables are 
   None.
 - Renamed all pdrv and pdev instances to idrv and idev respectivly 
 - Implemented an ealry return in `i2c::Adapter::i2c_id_info`
 - Added all missing Safety comments. 
 - Removed `unsafe impl<Ctx: device::DeviceContext> crate::types::AlwaysRefCounted for Device<Ctx>` 
   implementation which came to v3 from v2 by mistake.
 - Added more details regarding i2c-stub driver usage in rust_driver_i2c
   comment.
 - Changed `i2c::I2cAdapter::get` return type from `Option<Self>` to 
   `Result<&'static Self>`.
 - Added Daniel Almeida as a reviewer to the "I2C Subsystem [RUST]" entry 
   in MAINTAINERS, per his offer.
 - Link to v3: https://lore.kernel.org/rust-for-linux/20250801153742.13472-1-igor.korotin.linux@gmail.com/
v3: 
 - removed unnecessary i2c_get_clientdata and i2c_set_clientdata rust 
   helpers. Using generic accessors implemented in [1] instead.
 - Reimplemented i2c::DeviceId based on changes in [2].
 - Using from_result in i2c::Adapter::probe_callback
 - Using explicit drop() for i2c client private data in 
   `i2c::Adapter::remove_callback`
 - replaced device::Device::as_ref() with device::Device::from_raw in 
   `i2c::Device::as_ref()`. It is renamed in device::Device.
 - Build Rust I2C only if I2C is built-in
 - Reimplement overcomplicated trait i2c::DeviceOwned the same way it is 
   implemented in auxiliary [3].
 - Merge rust_device_i2c and rust_driver_i2c samples. Resulting 
   rust_driver_i2c creates pined i2c_client using i2c::Registration::new 
   and probes newly created i2c_client.
 - Created a new entry in MAINTAINERS file containing i2c.rs and 
   rust_driver_i2c.rs in it.
 - Link to v2: [4] 

 [1] https://lore.kernel.org/lkml/20250621195118.124245-3-dakr@kernel.org/
 [2] https://lore.kernel.org/rust-for-linux/20250711040947.1252162-1-fujita.tomonori@gmail.com/
 [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299
 [4] https://lore.kernel.org/rust-for-linux/20250704153332.1193214-1-igor.korotin.linux@gmail.com/ 

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                     |   9 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/i2c.rs              | 565 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 samples/rust/Kconfig            |  11 +
 samples/rust/Makefile           |   1 +
 samples/rust/rust_driver_i2c.rs | 126 +++++++
 7 files changed, 715 insertions(+)
 create mode 100644 rust/kernel/i2c.rs
 create mode 100644 samples/rust/rust_driver_i2c.rs


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.43.0

[PATCH v6 1/3] 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::I2cClient` — a safe wrapper around `struct i2c_client`

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

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

Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>
---
 MAINTAINERS                     |   8 +
 rust/bindings/bindings_helper.h |   1 +
 rust/kernel/i2c.rs              | 423 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 4 files changed, 434 insertions(+)
 create mode 100644 rust/kernel/i2c.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index fe168477caa4..c44c7ac317b1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11516,6 +11516,14 @@ F:	include/linux/i2c.h
 F:	include/uapi/linux/i2c-*.h
 F:	include/uapi/linux/i2c.h
 
+I2C SUBSYSTEM [RUST]
+M:	Igor Korotin <igor.korotin.linux@gmail.com>
+R:	Danilo Krummrich <dakr@kernel.org>
+R:	Daniel Almeida <daniel.almeida@collabora.com>
+L:	rust-for-linux@vger.kernel.org
+S:	Maintained
+F:	rust/kernel/i2c.rs
+
 I2C SUBSYSTEM HOST DRIVERS
 M:	Andi Shyti <andi.shyti@kernel.org>
 L:	linux-i2c@vger.kernel.org
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 84d60635e8a9..81796d5e16e8 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/ioport.h>
 #include <linux/jiffies.h>
 #include <linux/jump_label.h>
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
new file mode 100644
index 000000000000..c5a8a5791523
--- /dev/null
+++ b/rust/kernel/i2c.rs
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! I2C Driver subsystem
+
+// I2C Driver abstractions.
+use crate::{
+    acpi, container_of, device,
+    device_id::{RawDeviceId, RawDeviceIdIndex},
+    driver,
+    error::*,
+    of,
+    prelude::*,
+    types::{AlwaysRefCounted, Opaque},
+};
+
+use core::{marker::PhantomData, ptr::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`.
+unsafe impl RawDeviceId for DeviceId {
+    type RawType = bindings::i2c_device_id;
+}
+
+// SAFETY: `DRIVER_DATA_OFFSET` is the offset to the `driver_data` field.
+unsafe impl RawDeviceIdIndex for DeviceId {
+    const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::i2c_device_id, driver_data);
+
+    fn index(&self) -> usize {
+        self.0.driver_data
+    }
+}
+
+/// 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(
+        idrv: &Opaque<Self::RegType>,
+        name: &'static CStr,
+        module: &'static ThisModule,
+    ) -> Result {
+        build_assert!(
+            T::ACPI_ID_TABLE.is_some() || T::OF_ID_TABLE.is_some() || T::I2C_ID_TABLE.is_some(),
+            "At least one of ACPI/OF/Legacy tables must be present when registering an i2c driver"
+        );
+
+        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 {
+            (*idrv.get()).driver.name = name.as_char_ptr();
+            (*idrv.get()).probe = Some(Self::probe_callback);
+            (*idrv.get()).remove = Some(Self::remove_callback);
+            (*idrv.get()).shutdown = Some(Self::shutdown_callback);
+            (*idrv.get()).id_table = i2c_table;
+            (*idrv.get()).driver.of_match_table = of_table;
+            (*idrv.get()).driver.acpi_match_table = acpi_table;
+        }
+
+        // SAFETY: `idrv` is guaranteed to be a valid `RegType`.
+        to_result(unsafe { bindings::i2c_register_driver(module.0, idrv.get()) })
+    }
+
+    unsafe fn unregister(idrv: &Opaque<Self::RegType>) {
+        // SAFETY: `idrv` is guaranteed to be a valid `RegType`.
+        unsafe { bindings::i2c_del_driver(idrv.get()) }
+    }
+}
+
+impl<T: Driver + 'static> Adapter<T> {
+    extern "C" fn probe_callback(idev: *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: `idev` is valid for the duration of `probe_callback()`.
+        let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal>>() };
+
+        let info =
+            Self::i2c_id_info(idev).or_else(|| <Self as driver::Adapter>::id_info(idev.as_ref()));
+
+        from_result(|| {
+            let data = T::probe(idev, info)?;
+
+            idev.as_ref().set_drvdata(data);
+            Ok(0)
+        })
+    }
+
+    extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {
+        // SAFETY: `idev` is a valid pointer to a `struct i2c_client`.
+        let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal>>() };
+
+        // SAFETY: `remove_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called
+        // and stored a `Pin<KBox<T>>`.
+        drop(unsafe { idev.as_ref().drvdata_obtain::<Pin<KBox<T>>>() });
+    }
+
+    extern "C" fn shutdown_callback(idev: *mut bindings::i2c_client) {
+        // SAFETY: `shutdown_callback` is only ever called for a valid `idev`
+        let idev = unsafe { &*idev.cast::<I2cClient<device::CoreInternal>>() };
+
+        // SAFETY: `shutdown_callback` is only ever called after a successful call to
+        // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
+        // and stored a `Pin<KBox<T>>`.
+        let data = unsafe { idev.as_ref().drvdata_obtain::<Pin<KBox<T>>>() };
+
+        T::shutdown(idev, data.as_ref());
+    }
+
+    /// 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: &I2cClient) -> Option<&'static <Self as driver::Adapter>::IdInfo> {
+        let table = Self::i2c_id_table()?;
+
+        // SAFETY:
+        // - `table` has static lifetime, hence it's valid for reads
+        // - `dev` is guaranteed to be valid while it's alive, and so is `dev.as_raw()`.
+        let raw_id = unsafe { bindings::i2c_match_id(table.as_ptr(), dev.as_raw()) };
+
+        if raw_id.is_null() {
+            return None;
+        }
+
+        // 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 RawDeviceIdIndex>::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(
+///         _idev: &i2c::I2cClient<Core>,
+///         _id_info: Option<&Self::IdInfo>,
+///     ) -> Result<Pin<KBox<Self>>> {
+///         Err(ENODEV)
+///     }
+///
+///     fn shutdown(_idev: &i2c::I2cClient<Core>, this: Pin<&Self>) {
+///     }
+/// }
+///```
+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 client is added or discovered.
+    /// Implementers should attempt to initialize the client here.
+    fn probe(
+        dev: &I2cClient<device::Core>,
+        id_info: Option<&Self::IdInfo>,
+    ) -> Result<Pin<KBox<Self>>>;
+
+    /// I2C driver shutdown.
+    ///
+    /// Called by the kernel during system reboot or power-off to allow the [`Driver`] to bring the
+    /// [`Device`] into a safe state. Implementing this callback is optional.
+    ///
+    /// Typical actions include stopping transfers, disabling interrupts, or resetting the hardware
+    /// to prevent undesired behavior during shutdown.
+    ///
+    /// This callback is distinct from final resource cleanup, as the driver instance remains valid
+    /// after it returns. Any deallocation or teardown of driver-owned resources should instead be
+    /// handled in `Self::drop`.
+    fn shutdown(dev: &I2cClient<device::Core>, this: Pin<&Self>) {
+        let _ = (dev, this);
+    }
+
+    /// I2C driver unbind.
+    ///
+    /// Called when a [`Device`] is unbound from its bound [`Driver`]. Implementing this callback
+    /// is optional.
+    ///
+    /// This callback serves as a place for drivers to perform teardown operations that require a
+    /// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O
+    /// operations to gracefully tear down the device.
+    ///
+    /// Otherwise, release operations for driver resources should be performed in `Self::drop`.
+    fn unbind(dev: &I2cClient<device::Core>, this: Pin<&Self>) {
+        let _ = (dev, this);
+    }
+}
+
+/// The i2c client representation.
+///
+/// This structure represents the Rust abstraction for a C `struct i2c_client`. The
+/// implementation abstracts the usage of an existing C `struct i2c_client` that
+/// gets passed from the C side
+///
+/// # Invariants
+///
+/// A [`I2cClient`] instance represents a valid `struct i2c_client` created by the C portion of
+/// the kernel.
+#[repr(transparent)]
+pub struct I2cClient<Ctx: device::DeviceContext = device::Normal>(
+    Opaque<bindings::i2c_client>,
+    PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> I2cClient<Ctx> {
+    fn as_raw(&self) -> *mut bindings::i2c_client {
+        self.0.get()
+    }
+}
+
+// SAFETY: `I2cClient` is a transparent wrapper of a type that doesn't depend on `I2cClient`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { I2cClient });
+kernel::impl_device_context_into_aref!(I2cClient);
+
+// SAFETY: Instances of `I2cClient` are always reference-counted.
+unsafe impl AlwaysRefCounted for I2cClient {
+    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(&raw mut (*obj.as_ref().as_raw()).dev) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for I2cClient<Ctx> {
+    fn as_ref(&self) -> &device::Device<Ctx> {
+        let raw = self.as_raw();
+        // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
+        // `struct i2c_client`.
+        let dev = unsafe { &raw mut (*raw).dev };
+
+        // SAFETY: `dev` points to a valid `struct device`.
+        unsafe { device::Device::from_raw(dev) }
+    }
+}
+
+impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &I2cClient<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 idev = unsafe { container_of!(dev.as_raw(), bindings::i2c_client, dev) };
+
+        // SAFETY: `idev` is a valid pointer to a `struct i2c_client`.
+        Ok(unsafe { &*idev.cast() })
+    }
+}
+
+// SAFETY: A `I2cClient` is always reference-counted and can be released from any thread.
+unsafe impl Send for I2cClient {}
+
+// SAFETY: `I2cClient` can be shared among threads because all methods of `I2cClient`
+// (i.e. `I2cClient<Normal>) are thread safe.
+unsafe impl Sync for I2cClient {}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ed53169e795c..a5e97a9b1546 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -89,6 +89,8 @@
 pub mod firmware;
 pub mod fmt;
 pub mod fs;
+#[cfg(CONFIG_I2C = "y")]
+pub mod i2c;
 pub mod init;
 pub mod io;
 pub mod ioctl;
-- 
2.43.0

[PATCH v6 2/3] rust: i2c: add manual I2C device creation abstractions

[Igor Korotin]

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

Implement the core abstractions needed for manual creation/deletion
of I2C devices, including:

 * `i2c::Registration` — a NonNull pointer created by the function
                          `i2c_new_client_device`

 * `i2c::I2cAdapter` — a ref counted wrapper 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 | 144 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 143 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index c5a8a5791523..73858aecc131 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -13,7 +13,12 @@
     types::{AlwaysRefCounted, Opaque},
 };
 
-use core::{marker::PhantomData, ptr::NonNull};
+use core::{
+    marker::PhantomData,
+    ptr::{from_ref, NonNull},
+};
+
+use kernel::types::ARef;
 
 /// An I2C device id table.
 #[repr(transparent)]
@@ -343,6 +348,101 @@ fn unbind(dev: &I2cClient<device::Core>, this: Pin<&Self>) {
     }
 }
 
+/// The i2c adapter representation.
+///
+/// This structure represents the Rust abstraction for a C `struct i2c_adapter`. The
+/// implementation abstracts the usage of an existing C `struct i2c_adapter` that
+/// gets passed from the C side
+///
+/// # Invariants
+///
+/// A [`I2cAdapter`] instance represents a valid `struct i2c_adapter` created by the C portion of
+/// the kernel.
+#[repr(transparent)]
+pub struct I2cAdapter<Ctx: device::DeviceContext = device::Normal>(
+    Opaque<bindings::i2c_adapter>,
+    PhantomData<Ctx>,
+);
+
+impl<Ctx: device::DeviceContext> I2cAdapter<Ctx> {
+    fn as_raw(&self) -> *mut bindings::i2c_adapter {
+        self.0.get()
+    }
+}
+
+impl I2cAdapter {
+    /// Returns the I2C Adapter index.
+    #[inline]
+    pub fn get_nr(&self) -> i32 {
+        // SAFETY: `self.as_raw` is a valid pointer to a `struct i2c_adapter`.
+        unsafe { (*self.as_raw()).nr }
+    }
+    /// Gets pointer to an `i2c_adapter` by index.
+    pub fn get(index: i32) -> Result<ARef<Self>> {
+        // SAFETY: `index` must refer to a valid I2C 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) }).ok_or(ENODEV)?;
+
+        // SAFETY: `adapter` is non-null and points to a live `i2c_adapter`.
+        // `I2cAdapter` is #[repr(transparent)], so this cast is valid.
+        Ok(unsafe { (&*adapter.as_ptr().cast::<I2cAdapter<device::Normal>>()).into() })
+    }
+}
+
+// SAFETY: `I2cAdapter` is a transparent wrapper of a type that doesn't depend on `I2cAdapter`'s generic
+// argument.
+kernel::impl_device_context_deref!(unsafe { I2cAdapter });
+kernel::impl_device_context_into_aref!(I2cAdapter);
+
+// SAFETY: Instances of `I2cAdapter` are always reference-counted.
+unsafe impl crate::types::AlwaysRefCounted for I2cAdapter {
+    fn inc_ref(&self) {
+        // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
+        unsafe { bindings::i2c_get_adapter(self.get_nr()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+        unsafe { bindings::i2c_put_adapter(obj.as_ref().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 [`I2cBoardInfo`] 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 {
+        from_ref(&self.0)
+    }
+}
+
 /// The i2c client representation.
 ///
 /// This structure represents the Rust abstraction for a C `struct i2c_client`. The
@@ -421,3 +521,45 @@ unsafe impl Send for I2cClient {}
 // SAFETY: `I2cClient` can be shared among threads because all methods of `I2cClient`
 // (i.e. `I2cClient<Normal>) are thread safe.
 unsafe impl Sync for I2cClient {}
+
+/// The registration of an i2c client device.
+///
+/// This type represents the registration of a [`struct i2c_client`]. When an instance of this
+/// type is dropped, its respective i2c client device will be unregistered from the system.
+///
+/// # Invariants
+///
+/// `self.0` always holds a valid pointer to an initialized and registered
+/// [`struct i2c_client`].
+#[repr(transparent)]
+pub struct Registration(NonNull<bindings::i2c_client>);
+
+impl Registration {
+    /// The C `i2c_new_client_device` function wrapper for manual I2C client creation.
+    pub fn new(i2c_adapter: &I2cAdapter, i2c_board_info: &I2cBoardInfo) -> Result<Self> {
+        // SAFETY: the kernel guarantees that `i2c_new_client_device()` returns either a valid
+        // pointer or NULL. `from_err_ptr` separates errors. Following `NonNull::new` checks for NULL.
+        let raw_dev = from_err_ptr(unsafe {
+            bindings::i2c_new_client_device(i2c_adapter.as_raw(), i2c_board_info.as_raw())
+        })?;
+
+        let dev_ptr = NonNull::new(raw_dev).ok_or(ENODEV)?;
+
+        Ok(Self(dev_ptr))
+    }
+}
+
+impl Drop for Registration {
+    fn drop(&mut self) {
+        // SAFETY: `Drop` is only called for a valid `Registration`, which by invariant
+        // always contains a non-null pointer to an `i2c_client`.
+        unsafe { bindings::i2c_unregister_device(self.0.as_ptr()) }
+    }
+}
+
+// SAFETY: A `Registration` of a `struct i2c_client` can be released from any thread.
+unsafe impl Send for Registration {}
+
+// SAFETY: `Registration` offers no interior mutability (no mutation through &self
+// and no mutable access is exposed)
+unsafe impl Sync for Registration {}
-- 
2.43.0

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

[Igor Korotin]

Add a new `rust_driver_i2c` sample, showing how to create a new
i2c client using `i2c::Registration` and bind a driver to it
via legacy I2C-ID table.

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 | 126 ++++++++++++++++++++++++++++++++
 4 files changed, 139 insertions(+)
 create mode 100644 samples/rust/rust_driver_i2c.rs

diff --git a/MAINTAINERS b/MAINTAINERS
index c44c7ac317b1..2654a7ea0c80 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11523,6 +11523,7 @@ R:	Daniel Almeida <daniel.almeida@collabora.com>
 L:	rust-for-linux@vger.kernel.org
 S:	Maintained
 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..28dae070b365 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=y
+	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..b2d1d234b077
--- /dev/null
+++ b/samples/rust/rust_driver_i2c.rs
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Rust I2C driver sample.
+//!
+//! This module shows how to:
+//!
+//! 1. Manually create an `i2c_client` at address `SAMPLE_I2C_CLIENT_ADDR`
+//!    on the adapter with index `SAMPLE_I2C_ADAPTER_INDEX`.
+//!
+//! 2. Register a matching Rust-based I2C driver for that client.
+//!
+//! # Requirements
+//!
+//! - The target system must expose an I2C adapter at index
+//!   `SAMPLE_I2C_ADAPTER_INDEX`.
+//!
+//! - To emulate an adapter for testing, you can load the
+//!   `i2c-stub` kernel module with an option `chip_addr`
+//!   For example for this sample driver to emulate an I2C device with
+//!   an address 0x30 you can use:
+//!      `modprobe i2c-stub chip_addr=0x30`
+//!
+
+use kernel::{
+    acpi, c_str,
+    device::Core,
+    i2c, of,
+    prelude::*,
+    types::ARef,
+};
+
+const SAMPLE_I2C_CLIENT_ADDR: u16 = 0x30;
+const SAMPLE_I2C_ADAPTER_INDEX: i32 = 0;
+const BOARD_INFO: i2c::I2cBoardInfo =
+    i2c::I2cBoardInfo::new(c_str!("rust_driver_i2c"), SAMPLE_I2C_CLIENT_ADDR);
+
+struct SampleDriver {
+    idev: ARef<i2c::I2cClient>,
+}
+
+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(idev: &i2c::I2cClient<Core>, info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>> {
+        let dev = idev.as_ref();
+
+        dev_info!(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 { idev: idev.into() }, GFP_KERNEL)?;
+
+        Ok(drvdata.into())
+    }
+
+    fn shutdown(idev: &i2c::I2cClient<Core>, _this: Pin<&Self>) {
+        dev_info!(idev.as_ref(), "Shutdown Rust I2C driver sample.\n");
+    }
+}
+
+impl Drop for SampleDriver {
+    fn drop(&mut self) {
+        dev_info!(self.idev.as_ref(), "Remove Rust I2C driver sample.\n");
+    }
+}
+
+// NOTE: The code below is expanded macro module_i2c_driver. It is not used here
+//       because we need to manually create an I2C client in `init()`. The macro
+//       hides `init()`, so to demo client creation on adapter SAMPLE_I2C_ADAPTER_INDEX
+//       we expand it by hand.
+#[pin_data]
+struct DriverModule {
+    #[pin]
+    _driver: kernel::driver::Registration<i2c::Adapter<SampleDriver>>,
+    _reg: i2c::Registration,
+}
+
+impl kernel::InPlaceModule for DriverModule {
+    fn init(
+        module: &'static kernel::ThisModule,
+    ) -> impl ::pin_init::PinInit<Self, kernel::error::Error> {
+        kernel::try_pin_init!(Self {
+            _reg <- {
+                let adapter = i2c::I2cAdapter::get(SAMPLE_I2C_ADAPTER_INDEX)?;
+
+                i2c::Registration::new(&adapter, &BOARD_INFO)
+            },
+            _driver <- kernel::driver::Registration::new(<Self as kernel::ModuleMetadata>::NAME,module,),
+        })
+    }
+}
+
+kernel::prelude::module! {
+    type: DriverModule,
+    name: "rust_driver_i2c",
+    authors: ["Igor Korotin"],
+    description: "Rust I2C driver",
+    license: "GPL v2",
+}
-- 
2.43.0

Re: [PATCH v6 0/3] rust: i2c: Add basic I2C driver abstractions

[Igor Korotin]

Hi all,

Gentle ping on this series:
[PATCH v6 0/3] rust: i2c: Add basic I2C driver abstractions (2025-10-05)

Just checking if there’s any feedback or if it’s still pending review.
Thanks!

Re: [PATCH v6 2/3] rust: i2c: add manual I2C device creation abstractions

[Danilo Krummrich]

On 10/5/25 12:23 PM, Igor Korotin wrote:
> +impl Registration {
> +    /// The C `i2c_new_client_device` function wrapper for manual I2C client creation.
> +    pub fn new(i2c_adapter: &I2cAdapter, i2c_board_info: &I2cBoardInfo) -> Result<Self> {
> +        // SAFETY: the kernel guarantees that `i2c_new_client_device()` returns either a valid
> +        // pointer or NULL. `from_err_ptr` separates errors. Following `NonNull::new` checks for NULL.
> +        let raw_dev = from_err_ptr(unsafe {
> +            bindings::i2c_new_client_device(i2c_adapter.as_raw(), i2c_board_info.as_raw())
> +        })?;
> +
> +        let dev_ptr = NonNull::new(raw_dev).ok_or(ENODEV)?;
> +
> +        Ok(Self(dev_ptr))
> +    }
> +}

I wonder if we want to ensure that a Registration can't out-live the driver that
registers the I2C client device.

This should only ever be called by drivers bound to more complex devices, so if
the parent driver is unbound I don't think I2C client device registered by this
driver should be able to survive.

Hence, I think Registration::new() should return
impl PinInit<Devres<Self>, Error> instead.

Re: [PATCH v6 3/3] samples: rust: add Rust I2C sample driver

[Danilo Krummrich]

On 10/5/25 12:23 PM, Igor Korotin wrote:
> +impl Drop for SampleDriver {
> +    fn drop(&mut self) {
> +        dev_info!(self.idev.as_ref(), "Remove Rust I2C driver sample.\n");
> +    }
> +}

NIT: Please use the i2c::Driver::unbind() callback instead.

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

[Danilo Krummrich]

On 10/5/25 12:23 PM, Igor Korotin wrote:
> Implement the core abstractions needed for I2C drivers, including:
> 
> * `i2c::Driver` — the trait drivers must implement, including `probe`
> 
> * `i2c::I2cClient` — a safe wrapper around `struct i2c_client`
> 
> * `i2c::Adapter` — implements `driver::RegistrationOps` to hook into the
>   generic `driver::Registration` machinery
> 
> * `i2c::DeviceId` — a `RawDeviceIdIndex` implementation for I2C device IDs
> 
> Signed-off-by: Igor Korotin <igor.korotin.linux@gmail.com>

Acked-by: Danilo Krummrich <dakr@kernel.org>

Re: [PATCH v6 0/3] rust: i2c: Add basic I2C driver abstractions

[Danilo Krummrich]

On 10/26/25 10:38 AM, Igor Korotin wrote:
> Just checking if there’s any feedback or if it’s still pending review.
> Thanks!

One note, if taken through the I2C tree, this patch series will have a conflict
with [1] from the driver-core tree.

Igor, Wolfram: If you rather want to avoid the conflict, I can also take this
initial series through the driver-core tree.

Thanks,
Danilo

[1] https://lore.kernel.org/all/20251016125544.15559-1-dakr@kernel.org/

Re: [PATCH v6 3/3] samples: rust: add Rust I2C sample driver

[Igor Korotin]

Hello Danilo

On 10/26/2025 10:48 AM, Danilo Krummrich wrote:
> On 10/5/25 12:23 PM, Igor Korotin wrote:
>> +impl Drop for SampleDriver {
>> +    fn drop(&mut self) {
>> +        dev_info!(self.idev.as_ref(), "Remove Rust I2C driver sample.\n");
>> +    }
>> +}
> 
> NIT: Please use the i2c::Driver::unbind() callback instead.

Thanks for the feedback.
I’ll move this into the i2c::Driver::unbind() callback.

Should I send v7?

Best,
Igor

Re: [PATCH v6 0/3] rust: i2c: Add basic I2C driver abstractions

[Igor Korotin]

On 10/26/2025 10:52 AM, Danilo Krummrich wrote:
> On 10/26/25 10:38 AM, Igor Korotin wrote:
>> Just checking if there’s any feedback or if it’s still pending review.
>> Thanks!
> 
> One note, if taken through the I2C tree, this patch series will have a conflict
> with [1] from the driver-core tree.
> 
> Igor, Wolfram: If you rather want to avoid the conflict, I can also take this
> initial series through the driver-core tree.

I'm fine with either option. I can rebase and update the series based on 
[1] if needed.
> Thanks,
> Danilo
> 
> [1] https://lore.kernel.org/all/20251016125544.15559-1-dakr@kernel.org/
Thanks
Igor

Re: [PATCH v6 0/3] rust: i2c: Add basic I2C driver abstractions

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 260 bytes --]


> Igor, Wolfram: If you rather want to avoid the conflict, I can also take this
> initial series through the driver-core tree.

This is totally fine with me. You can add for this series:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thank you!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v6 3/3] samples: rust: add Rust I2C sample driver

[Danilo Krummrich]

On 10/26/25 3:06 PM, Igor Korotin wrote:
> Hello Danilo
> 
> On 10/26/2025 10:48 AM, Danilo Krummrich wrote:
>> On 10/5/25 12:23 PM, Igor Korotin wrote:
>>> +impl Drop for SampleDriver {
>>> +    fn drop(&mut self) {
>>> +        dev_info!(self.idev.as_ref(), "Remove Rust I2C driver sample.\n");
>>> +    }
>>> +}
>>
>> NIT: Please use the i2c::Driver::unbind() callback instead.
> 
> Thanks for the feedback.
> I’ll move this into the i2c::Driver::unbind() callback.
> 
> Should I send v7?

For such a minor thing I can usually do it when applying the patch, but given
that for the other patch the change is a bit more significant, I'd say please
send a v7.

Re: [PATCH v6 3/3] samples: rust: add Rust I2C sample driver

[Igor Korotin]

On 10/26/2025 3:43 PM, Danilo Krummrich wrote:
> On 10/26/25 3:06 PM, Igor Korotin wrote:
>> Hello Danilo
>>
>> On 10/26/2025 10:48 AM, Danilo Krummrich wrote:
>>> On 10/5/25 12:23 PM, Igor Korotin wrote:
>>>> +impl Drop for SampleDriver {
>>>> +    fn drop(&mut self) {
>>>> +        dev_info!(self.idev.as_ref(), "Remove Rust I2C driver sample.\n");
>>>> +    }
>>>> +}
>>>
>>> NIT: Please use the i2c::Driver::unbind() callback instead.
>>
>> Thanks for the feedback.
>> I’ll move this into the i2c::Driver::unbind() callback.
>>
>> Should I send v7?
> 
> For such a minor thing I can usually do it when applying the patch, but given
> that for the other patch the change is a bit more significant, I'd say please
> send a v7.

Just for the clarification: by "the change for the other patch" you mean 
rebase and update this patch series based on [1], right?

Thanks
Igor

[1] https://lore.kernel.org/all/20251016125544.15559-1-dakr@kernel.org/

Re: [PATCH v6 3/3] samples: rust: add Rust I2C sample driver

[Danilo Krummrich]

On 10/26/25 4:50 PM, Igor Korotin wrote:
> Just for the clarification: by "the change for the other patch" you mean rebase
> and update this patch series based on [1], right?

Yes, plus the i2c:Registration change.
> [1] https://lore.kernel.org/all/20251016125544.15559-1-dakr@kernel.org/

[PATCH v6 2/3] rust: i2c: Add basic I2C driver abstractions

[Igor Korotin]

Hello Danilo

> On 10/5/25 12:23 PM, Igor Korotin wrote:
> > +impl Registration {
> > +    /// The C `i2c_new_client_device` function wrapper for manual I2C client creation.
> > +    pub fn new(i2c_adapter: &I2cAdapter, i2c_board_info: &I2cBoardInfo) -> Result<Self> {
> > +        // SAFETY: the kernel guarantees that `i2c_new_client_device()` returns either a valid
> > +        // pointer or NULL. `from_err_ptr` separates errors. Following `NonNull::new` checks for NULL.
> > +        let raw_dev = from_err_ptr(unsafe {
> > +            bindings::i2c_new_client_device(i2c_adapter.as_raw(), i2c_board_info.as_raw())
> > +        })?;
> > +
> > +        let dev_ptr = NonNull::new(raw_dev).ok_or(ENODEV)?;
> > +
> > +        Ok(Self(dev_ptr))
> > +    }
> > +}
> 
> I wonder if we want to ensure that a Registration can't out-live the driver that
> registers the I2C client device.
> 
> This should only ever be called by drivers bound to more complex devices, so if
> the parent driver is unbound I don't think I2C client device registered by this
> driver should be able to survive.
> 
> Hence, I think Registration::new() should return
> impl PinInit<Devres<Self>, Error> instead.

Maybe I'm missing something here, but as far as I understand, Devres is bound to 
an existing device. However `Registration::new` creates new device and registers 
new i2c_client using function `i2c_new_client_device`. Created i2c_client uses 
i2c_adapter as its parent.

The driver that declares Registration doesn't own that i2c_adapter. `Registration` 
itself is not part of the new client’s managed resources, so returning 
`impl PinInit<Devres<Self>, Error>` wouldn’t make sense here.

Drop for Registration calls `i2c_unregister_client()`, which gracefully unregisters
and deallocates the i2c_client.

Cheers 
Igor

Re: [PATCH v6 2/3] rust: i2c: Add basic I2C driver abstractions

[Danilo Krummrich]

On 10/26/25 7:41 PM, Igor Korotin wrote:
> Hello Danilo
> 
>> On 10/5/25 12:23 PM, Igor Korotin wrote:
>>> +impl Registration {
>>> +    /// The C `i2c_new_client_device` function wrapper for manual I2C client creation.
>>> +    pub fn new(i2c_adapter: &I2cAdapter, i2c_board_info: &I2cBoardInfo) -> Result<Self> {
>>> +        // SAFETY: the kernel guarantees that `i2c_new_client_device()` returns either a valid
>>> +        // pointer or NULL. `from_err_ptr` separates errors. Following `NonNull::new` checks for NULL.
>>> +        let raw_dev = from_err_ptr(unsafe {
>>> +            bindings::i2c_new_client_device(i2c_adapter.as_raw(), i2c_board_info.as_raw())
>>> +        })?;
>>> +
>>> +        let dev_ptr = NonNull::new(raw_dev).ok_or(ENODEV)?;
>>> +
>>> +        Ok(Self(dev_ptr))
>>> +    }
>>> +}
>>
>> I wonder if we want to ensure that a Registration can't out-live the driver that
>> registers the I2C client device.
>>
>> This should only ever be called by drivers bound to more complex devices, so if
>> the parent driver is unbound I don't think I2C client device registered by this
>> driver should be able to survive.
>>
>> Hence, I think Registration::new() should return
>> impl PinInit<Devres<Self>, Error> instead.
> 
> Maybe I'm missing something here, but as far as I understand, Devres is bound to 
> an existing device. However `Registration::new` creates new device and registers 
> new i2c_client using function `i2c_new_client_device`. Created i2c_client uses 
> i2c_adapter as its parent.

Correct, but the question is what's the correct lifetime boundary for this
i2c:Registration.
> The driver that declares Registration doesn't own that i2c_adapter. `Registration` 
> itself is not part of the new client’s managed resources, so returning 
> `impl PinInit<Devres<Self>, Error>` wouldn’t make sense here.

It does make sense, it's just not required for safety reasons.

This is an API that should be used by drivers operating complicated devices
(DRM, NET, etc.) where there is no point in keeping an i2c::Registration alive
after the driver that registered the I2C client has been unbound itself.

For instance, a GPU driver may call this in probe() to register an I2C device
for some redriver, repeater, multiplexer, etc. So, it makes no sense to allow a
corresponding i2c::Registration to still exist beyond the GPU driver being unbound.

Hence, besides not really being necessary for safety reasons, it still seems
reasonable to enforce this for semantic reasons.

> Drop for Registration calls `i2c_unregister_client()`, which gracefully unregisters
> and deallocates the i2c_client.

Not quite, it unregisters the I2C client (which is why we call the object
Registration), but it does not necessarily free the I2C client. Someone else can
still hold a reference count of the I2C client.

Re: [PATCH v6 2/3] rust: i2c: Add basic I2C driver abstractions

[Igor Korotin]

Hello Danilo

On 10/26/2025 7:20 PM, Danilo Krummrich wrote:
> On 10/26/25 7:41 PM, Igor Korotin wrote:
>> Hello Danilo
>>
>>> On 10/5/25 12:23 PM, Igor Korotin wrote:
>>>> +impl Registration {
>>>> +    /// The C `i2c_new_client_device` function wrapper for manual I2C client creation.
>>>> +    pub fn new(i2c_adapter: &I2cAdapter, i2c_board_info: &I2cBoardInfo) -> Result<Self> {
>>>> +        // SAFETY: the kernel guarantees that `i2c_new_client_device()` returns either a valid
>>>> +        // pointer or NULL. `from_err_ptr` separates errors. Following `NonNull::new` checks for NULL.
>>>> +        let raw_dev = from_err_ptr(unsafe {
>>>> +            bindings::i2c_new_client_device(i2c_adapter.as_raw(), i2c_board_info.as_raw())
>>>> +        })?;
>>>> +
>>>> +        let dev_ptr = NonNull::new(raw_dev).ok_or(ENODEV)?;
>>>> +
>>>> +        Ok(Self(dev_ptr))
>>>> +    }
>>>> +}
>>>
>>> I wonder if we want to ensure that a Registration can't out-live the driver that
>>> registers the I2C client device.
>>>
>>> This should only ever be called by drivers bound to more complex devices, so if
>>> the parent driver is unbound I don't think I2C client device registered by this
>>> driver should be able to survive.
>>>
>>> Hence, I think Registration::new() should return
>>> impl PinInit<Devres<Self>, Error> instead.
>>
>> Maybe I'm missing something here, but as far as I understand, Devres is bound to
>> an existing device. However `Registration::new` creates new device and registers
>> new i2c_client using function `i2c_new_client_device`. Created i2c_client uses
>> i2c_adapter as its parent.
> 
> Correct, but the question is what's the correct lifetime boundary for this
> i2c:Registration.
>> The driver that declares Registration doesn't own that i2c_adapter. `Registration`
>> itself is not part of the new client’s managed resources, so returning
>> `impl PinInit<Devres<Self>, Error>` wouldn’t make sense here.
> 
> It does make sense, it's just not required for safety reasons.
> 
> This is an API that should be used by drivers operating complicated devices
> (DRM, NET, etc.) where there is no point in keeping an i2c::Registration alive
> after the driver that registered the I2C client has been unbound itself.
> 
> For instance, a GPU driver may call this in probe() to register an I2C device
> for some redriver, repeater, multiplexer, etc. So, it makes no sense to allow a
> corresponding i2c::Registration to still exist beyond the GPU driver being unbound.
> 
> Hence, besides not really being necessary for safety reasons, it still seems
> reasonable to enforce this for semantic reasons.

I might be misunderstanding your point, but as I see it, Devres cannot 
apply here
because we can't bind it to i2c_adapter. There's no guarantee that driver
owns it. Registration can't be bound to i2c_client, cause it's kind of 
chicken and egg situation.

Even if we returned impl PinInit<Self, Error>, it wouldn’t prevent other 
code from holding an additional reference to the client.
>> Drop for Registration calls `i2c_unregister_client()`, which gracefully unregisters
>> and deallocates the i2c_client.
> 
> Not quite, it unregisters the I2C client (which is why we call the object
> Registration), but it does not necessarily free the I2C client. Someone else can
> still hold a reference count of the I2C client.

You’re right, it doesn’t necessarily free the client immediately.
However, i2c_unregister_client() calls device_unregister(), which triggers
device_del() and put_device().

So while the memory may remain allocated until the last reference is 
dropped,
the device itself is already unregistered and deactivated at that point.
This aligns with the standard lifetime guarantees of the C driver model:
the structure remains valid only until its refcount reaches zero, after
which it’s released automatically.
That said, could you elaborate a bit more on what exactly you’d like to 
achieve by returning impl PinInit<Devres<Self>, Error> here?
Is the idea to explicitly tie the Registration lifetime to the parent 
device (for example, the i2c_adapter), or is this more about defining a 
general pattern for device-managed helper types in the Rust driver API?
I just want to be sure I’m following the intent of your suggestion 
correctly.

Cheers,
Igor

Re: [PATCH v6 2/3] rust: i2c: Add basic I2C driver abstractions

[Danilo Krummrich]

On Mon Oct 27, 2025 at 9:27 PM CET, Igor Korotin wrote:
> On 10/26/2025 7:20 PM, Danilo Krummrich wrote:
>> This is an API that should be used by drivers operating complicated devices
>> (DRM, NET, etc.) where there is no point in keeping an i2c::Registration alive
>> after the driver that registered the I2C client has been unbound itself.
>> 
>> For instance, a GPU driver may call this in probe() to register an I2C device
>> for some redriver, repeater, multiplexer, etc. So, it makes no sense to allow a
>> corresponding i2c::Registration to still exist beyond the GPU driver being unbound.
>> 
>> Hence, besides not really being necessary for safety reasons, it still seems
>> reasonable to enforce this for semantic reasons.
>
> I might be misunderstanding your point, but as I see it, Devres cannot 
> apply here
> because we can't bind it to i2c_adapter.

Yes, you do misunderstand.

I'm not saying to use the I2C adapter device for this, that makes no sense. In
fact, it wouldn't even work, because the underlying device can not be guaranteed
to be in a bound state.

> There's no guarantee that driver
> owns it. Registration can't be bound to i2c_client, cause it's kind of 
> chicken and egg situation.

I'm also not saying to use the the I2C client, which indeed makes even less
sense (and wouldn't work for the same reason mentioned above).

Think of the bigger picture, i.e. where is i2c:Registration::new() called from?

It's called from other drivers (e.g. DRM drivers [1] or network drivers [2])
that are bound to some bus device themselves, e.g. a platform device or a PCI
device.

This is the device that we can give to i2c:Registration::new() and use for the
internal call to devres.

With that we ensure that the i2c:Registration will be dropped, when the driver
that originally called i2c:Registration::new() is unbound; it makes no sense to
allow a driver to keep an i2c:Registration alive when it is unbound.

In fact, quite some C drivers are already doing exactly this by hand. For
instance, see [3]. Four lines after [3], raa215300_rtc_unregister_device() is
registered with devm_add_action_or_reset(), which calls i2c_unregister_device().

Having that said, I'm a bit curious now: What is your use-case for
i2c:Registration?

[1] https://elixir.bootlin.com/linux/v6.17.5/source/drivers/gpu/drm/xe/xe_i2c.c#L72
[2] https://elixir.bootlin.com/linux/v6.17.5/source/drivers/net/ethernet/intel/igb/igb_hwmon.c#L201
[3] https://elixir.bootlin.com/linux/v6.17.5/source/drivers/regulator/raa215300.c#L160

Re: [PATCH v6 2/3] rust: i2c: Add basic I2C driver abstractions

[Igor Korotin]

Hello Danilo

On 10/27/2025 10:00 PM, Danilo Krummrich wrote:
> In fact, quite some C drivers are already doing exactly this by hand. For
> instance, see [3]. Four lines after [3], raa215300_rtc_unregister_device() is
> registered with devm_add_action_or_reset(), which calls i2c_unregister_device().

Now that you’ve mentioned the parent device, I finally understand what 
you mean.
Originally, I started my Rust-for-Linux journey with the goal of 
rewriting one of
our platform drivers that has I2C children — and yes, it has a root 
platform_device.
For now, I’ve put that rewrite on hold and focused on the I2C part 
instead, which
is why I was thinking from the perspective of rust_driver_i2c.rs: it’s a 
purely
artificial sample driver that creates an I2C client for a non-existent 
physical
device using an I2C adapter. That’s why the puzzle didn’t fit together 
for me earlier.

> Having that said, I'm a bit curious now: What is your use-case for
> i2c:Registration?

As for my view of i2c::Registration, I see it as a safe wrapper around 
i2c_new_client_device that holds a pointer to the newly created I2C 
client and releases it automatically when dropped.

Thanks for the explanation, I'll drop a new version when ready

Best Regards
Igor