rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/3] rust: i2c: Add basic I2C driver abstractions
@ 2025-08-20 15:14 Igor Korotin
  2025-08-20 15:19 ` [PATCH v4 1/3] rust: i2c: add basic I2C device and " Igor Korotin
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Igor Korotin @ 2025-08-20 15:14 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wolfram Sang
  Cc: Boqun Feng, Daniel Almeida, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar, Asahi Lina,
	Wedson Almeida Filho, Alex Hung, Tamir Duberstein, Xiangfei Ding,
	linux-kernel, rust-for-linux, linux-i2c

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


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.43.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v4 1/3] rust: i2c: add basic I2C device and driver abstractions
  2025-08-20 15:14 [PATCH v4 0/3] rust: i2c: Add basic I2C driver abstractions Igor Korotin
@ 2025-08-20 15:19 ` Igor Korotin
  2025-08-27 18:37   ` Daniel Almeida
  2025-08-20 15:21 ` [PATCH v4 2/3] rust: i2c: add manual I2C device creation abstractions Igor Korotin
  2025-08-20 15:23 ` [PATCH v4 3/3] samples: rust: add Rust I2C sample driver Igor Korotin
  2 siblings, 1 reply; 7+ messages in thread
From: Igor Korotin @ 2025-08-20 15:19 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wolfram Sang
  Cc: Boqun Feng, Daniel Almeida, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar, Asahi Lina,
	Wedson Almeida Filho, Alex Hung, Tamir Duberstein, Xiangfei Ding,
	linux-kernel, rust-for-linux, linux-i2c

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              | 396 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 4 files changed, 407 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..f5e8c4bc1b7d
--- /dev/null
+++ b/rust/kernel/i2c.rs
@@ -0,0 +1,396 @@
+// 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::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"
+        );
+
+        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::Core>>() };
+
+        T::shutdown(idev);
+    }
+
+    /// 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 `idev.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 crate::device_id::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>) {
+///     }
+/// }
+///```
+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 when
+    fn shutdown(dev: &I2cClient<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 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 crate::types::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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 2/3] rust: i2c: add manual I2C device creation abstractions
  2025-08-20 15:14 [PATCH v4 0/3] rust: i2c: Add basic I2C driver abstractions Igor Korotin
  2025-08-20 15:19 ` [PATCH v4 1/3] rust: i2c: add basic I2C device and " Igor Korotin
@ 2025-08-20 15:21 ` Igor Korotin
  2025-08-27 19:23   ` Daniel Almeida
  2025-08-20 15:23 ` [PATCH v4 3/3] samples: rust: add Rust I2C sample driver Igor Korotin
  2 siblings, 1 reply; 7+ messages in thread
From: Igor Korotin @ 2025-08-20 15:21 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wolfram Sang
  Cc: Boqun Feng, Daniel Almeida, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar, Asahi Lina,
	Wedson Almeida Filho, Alex Hung, Tamir Duberstein, Xiangfei Ding,
	linux-kernel, rust-for-linux, linux-i2c

In addition to the basic I2C device support, added 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 | 175 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 174 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index f5e8c4bc1b7d..851d20ec65b5 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -13,7 +13,10 @@
     types::Opaque,
 };
 
-use core::{marker::PhantomData, ptr::NonNull};
+use core::{
+    marker::PhantomData,
+    ptr::{from_ref, NonNull},
+};
 
 /// An I2C device id table.
 #[repr(transparent)]
@@ -316,6 +319,134 @@ fn shutdown(dev: &I2cClient<device::Core>) {
     }
 }
 
+/// 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()
+    }
+
+    /// Gets pointer to an `i2c_adapter` by index.
+    pub fn get(index: i32) -> Result<&'static 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.cast::<Self>().as_ref() })
+    }
+}
+
+impl<Ctx: device::DeviceContext> Drop for I2cAdapter<Ctx> {
+    fn drop(&mut self) {
+        // SAFETY: This `I2cAdapter` was obtained from `i2c_get_adapter`,
+        // and calling `i2c_put_adapter` exactly once will correctly release
+        // the reference count in the I2C core. It is safe to call from any context
+        unsafe { bindings::i2c_put_adapter(self.as_raw()) }
+    }
+}
+
+// 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::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 I2cAdapter<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_adapter`.
+        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 &I2cAdapter<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_adapter(dev.as_raw()).is_null() } {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: We've just verified that the type of `dev` equals to
+        // `bindings::i2c_adapter_type`, hence `dev` must be embedded in a valid
+        // `struct i2c_adapter` as guaranteed by the corresponding C code.
+        let idev = unsafe { container_of!(dev.as_raw(), bindings::i2c_adapter, dev) };
+
+        // SAFETY: `idev` is a valid pointer to a `struct i2c_adapter`.
+        Ok(unsafe { &*idev.cast() })
+    }
+}
+
+/// 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 {
+        from_ref(&self.0)
+    }
+}
+
 /// The i2c client representation.
 ///
 /// This structure represents the Rust abstraction for a C `struct i2c_client`. The
@@ -394,3 +525,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` does not expose any methods or fields that need synchronization.
+unsafe impl Sync for Registration {}
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v4 3/3] samples: rust: add Rust I2C sample driver
  2025-08-20 15:14 [PATCH v4 0/3] rust: i2c: Add basic I2C driver abstractions Igor Korotin
  2025-08-20 15:19 ` [PATCH v4 1/3] rust: i2c: add basic I2C device and " Igor Korotin
  2025-08-20 15:21 ` [PATCH v4 2/3] rust: i2c: add manual I2C device creation abstractions Igor Korotin
@ 2025-08-20 15:23 ` Igor Korotin
  2025-08-27 19:38   ` Daniel Almeida
  2 siblings, 1 reply; 7+ messages in thread
From: Igor Korotin @ 2025-08-20 15:23 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Wolfram Sang
  Cc: Boqun Feng, Daniel Almeida, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar, Asahi Lina,
	Wedson Almeida Filho, Alex Hung, Tamir Duberstein, Xiangfei Ding,
	linux-kernel, rust-for-linux, linux-i2c

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 | 128 ++++++++++++++++++++++++++++++++
 4 files changed, 141 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..6dfc299d5aea
--- /dev/null
+++ b/samples/rust/rust_driver_i2c.rs
@@ -0,0 +1,128 @@
+// 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, Normal},
+    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 {
+    pdev: 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(pdev: &i2c::I2cClient<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::I2cClient<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");
+    }
+}
+
+// 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.
+type Ops<T> = kernel::i2c::Adapter<T>;
+
+#[pin_data]
+struct DriverModule {
+    #[pin]
+    _driver: kernel::driver::Registration<Ops<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::<Normal>::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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 1/3] rust: i2c: add basic I2C device and driver abstractions
  2025-08-20 15:19 ` [PATCH v4 1/3] rust: i2c: add basic I2C device and " Igor Korotin
@ 2025-08-27 18:37   ` Daniel Almeida
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Almeida @ 2025-08-27 18:37 UTC (permalink / raw)
  To: Igor Korotin
  Cc: Miguel Ojeda, Alex Gaynor, Wolfram Sang, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar,
	Asahi Lina, Wedson Almeida Filho, Alex Hung, Tamir Duberstein,
	Xiangfei Ding, linux-kernel, rust-for-linux, linux-i2c

Hi Igor,

> On 20 Aug 2025, at 12:19, Igor Korotin <igor.korotin.linux@gmail.com> 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>
> ---
> MAINTAINERS                     |   8 +
> rust/bindings/bindings_helper.h |   1 +
> rust/kernel/i2c.rs              | 396 ++++++++++++++++++++++++++++++++
> rust/kernel/lib.rs              |   2 +
> 4 files changed, 407 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..f5e8c4bc1b7d
> --- /dev/null
> +++ b/rust/kernel/i2c.rs
> @@ -0,0 +1,396 @@
> +// 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::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”.

The generated error is cryptic, so we should add as much information as possible.

> +
> +        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()));

I wonder if these should be private member functions?

> +
> +        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::Core>>() };
> +
> +        T::shutdown(idev);
> +    }
> +
> +    /// 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> {

Again, perhaps a private member function? I’m trying to simplify the syntax here.

> +        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 `idev.as_raw()`.

I don’t see an “idev” in scope? Perhaps you meant “dev” ?

> +        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 crate::device_id::RawDeviceIdIndex>::index(id)))

Do we really need to use the fully-qualified syntax here?

> +    }
> +}
> +
> +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>) {
> +///     }
> +/// }
> +///```
> +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 when
> +    fn shutdown(dev: &I2cClient<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 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 crate::types::AlwaysRefCounted for I2cClient {

I’d import this.

> +    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
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 2/3] rust: i2c: add manual I2C device creation abstractions
  2025-08-20 15:21 ` [PATCH v4 2/3] rust: i2c: add manual I2C device creation abstractions Igor Korotin
@ 2025-08-27 19:23   ` Daniel Almeida
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Almeida @ 2025-08-27 19:23 UTC (permalink / raw)
  To: Igor Korotin
  Cc: Miguel Ojeda, Alex Gaynor, Wolfram Sang, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar,
	Asahi Lina, Wedson Almeida Filho, Alex Hung, Tamir Duberstein,
	Xiangfei Ding, linux-kernel, rust-for-linux, linux-i2c

Hi Igor,

> On 20 Aug 2025, at 12:21, Igor Korotin <igor.korotin.linux@gmail.com> wrote:
> 
> In addition to the basic I2C device support, added rust abstractions
> upon `i2c_new_client_device`/`i2c_unregister_device` C functions.

Can you use imperative voice here?

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

Like this ^

> 
> * `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 | 175 ++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 174 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
> index f5e8c4bc1b7d..851d20ec65b5 100644
> --- a/rust/kernel/i2c.rs
> +++ b/rust/kernel/i2c.rs
> @@ -13,7 +13,10 @@
>     types::Opaque,
> };
> 
> -use core::{marker::PhantomData, ptr::NonNull};
> +use core::{
> +    marker::PhantomData,
> +    ptr::{from_ref, NonNull},
> +};
> 
> /// An I2C device id table.
> #[repr(transparent)]
> @@ -316,6 +319,134 @@ fn shutdown(dev: &I2cClient<device::Core>) {
>     }
> }
> 
> +/// 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()
> +    }
> +
> +    /// Gets pointer to an `i2c_adapter` by index.
> +    pub fn get(index: i32) -> Result<&'static Self> {

Hmm, perhaps I am misunderstanding what is going on, but I don’t think
’static is the right thing to have here.

Looking at i2c_get_adapter, it relies on: 

a) adap->nr actually being in i2c_adapter_idr, but even if it is, it may eventually be removed.
b) acquiring a refcount on adapter->dev,

Also, when this goes out of scope the refcount acquired above has to be
decremented, so this has to return an owned type and not a reference.

We should probably simply return ARef<I2cAdapter> here, or more succinctly,
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.cast::<Self>().as_ref() })
> +    }
> +}
> +
> +impl<Ctx: device::DeviceContext> Drop for I2cAdapter<Ctx> {
> +    fn drop(&mut self) {
> +        // SAFETY: This `I2cAdapter` was obtained from `i2c_get_adapter`,

Where? Note that drop() is called when a T goes out of scope, not when a &T
does so. Specially, you should not expect drop() to be called for T if
there’s a &’static T laying around.

> +        // and calling `i2c_put_adapter` exactly once will correctly release
> +        // the reference count in the I2C core. It is safe to call from any context
> +        unsafe { bindings::i2c_put_adapter(self.as_raw()) }
> +    }

Again, barring some misunderstanding on my end, remove this whole Drop impl in
favor of ARef<I2cAdapter>.

> +}
> +
> +// 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::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) }
> +    }
> +}

Shouldn’t these be `i2c_{get/put}_adapter` ?

> +
> +impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for I2cAdapter<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_adapter`.
> +        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 &I2cAdapter<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_adapter(dev.as_raw()).is_null() } {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: We've just verified that the type of `dev` equals to
> +        // `bindings::i2c_adapter_type`, hence `dev` must be embedded in a valid
> +        // `struct i2c_adapter` as guaranteed by the corresponding C code.
> +        let idev = unsafe { container_of!(dev.as_raw(), bindings::i2c_adapter, dev) };
> +
> +        // SAFETY: `idev` is a valid pointer to a `struct i2c_adapter`.
> +        Ok(unsafe { &*idev.cast() })
> +    }
> +}
> +
> +/// 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.

Nit: instead of `board-info` you can just say [`I2cBoardInfo`]. This will look
better in the docs.
 
> +    #[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
> @@ -394,3 +525,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` does not expose any methods or fields that need synchronization.

Can you mention that `Registration` offers no interior mutability instead?

I think we should all align on the safety comments for Sync.

> +unsafe impl Sync for Registration {}
> -- 
> 2.43.0
> 
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v4 3/3] samples: rust: add Rust I2C sample driver
  2025-08-20 15:23 ` [PATCH v4 3/3] samples: rust: add Rust I2C sample driver Igor Korotin
@ 2025-08-27 19:38   ` Daniel Almeida
  0 siblings, 0 replies; 7+ messages in thread
From: Daniel Almeida @ 2025-08-27 19:38 UTC (permalink / raw)
  To: Igor Korotin
  Cc: Miguel Ojeda, Alex Gaynor, Wolfram Sang, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman, Viresh Kumar,
	Asahi Lina, Wedson Almeida Filho, Alex Hung, Tamir Duberstein,
	Xiangfei Ding, linux-kernel, rust-for-linux, linux-i2c

Hi Igor,

> On 20 Aug 2025, at 12:23, Igor Korotin <igor.korotin.linux@gmail.com> wrote:
> 
> 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 | 128 ++++++++++++++++++++++++++++++++
> 4 files changed, 141 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..6dfc299d5aea
> --- /dev/null
> +++ b/samples/rust/rust_driver_i2c.rs
> @@ -0,0 +1,128 @@
> +// 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`.

Blank here.

> +//! 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`.

Blank here

> +//! - 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, Normal},
> +    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 {
> +    pdev: ARef<i2c::I2cClient>,

FYI: the pdev nomenclature is still here.

> +}
> +
> +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::I2cClient<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::I2cClient<Core>) {
> +        dev_dbg!(pdev.as_ref(), "Shutdown Rust I2C driver sample.\n");

This should probably be dev_info!()

I know that in general people want drivers to be quiet but:

a) this a sample driver that is probably the only way to test the abstractions
for the time being, so we must know whether probe() and shudown() are working
by inspecting these traces.

b) Unless something changed recently, there is no way to get dev_dbg() to print
in Rust for now, as there is no support for dynamic debug. Andrew was working
on it recently [0], but I don't think the patch was accepted yet.

> +    }
> +}
> +
> +impl Drop for SampleDriver {
> +    fn drop(&mut self) {
> +        dev_dbg!(self.pdev.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.
> +type Ops<T> = kernel::i2c::Adapter<T>;

I am not sure I understand. How is a type alias related to module_i2c_driver at
all?

Do all drivers need to introduce the alias above?

> +
> +#[pin_data]
> +struct DriverModule {
> +    #[pin]
> +    _driver: kernel::driver::Registration<Ops<SampleDriver>>,
> +    _reg: i2c::Registration,
> +}

I was expecting this to be ARef of something, most likely I2cClient?

This is where my knowledge of i2c drivers start to fall short, but others will
probably chime in :)

> +
> +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::<Normal>::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
> 
> 

[0]: https://lore.kernel.org/rust-for-linux/20250611202952.1670168-1-andrewjballance@gmail.com/


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-08-27 19:39 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-20 15:14 [PATCH v4 0/3] rust: i2c: Add basic I2C driver abstractions Igor Korotin
2025-08-20 15:19 ` [PATCH v4 1/3] rust: i2c: add basic I2C device and " Igor Korotin
2025-08-27 18:37   ` Daniel Almeida
2025-08-20 15:21 ` [PATCH v4 2/3] rust: i2c: add manual I2C device creation abstractions Igor Korotin
2025-08-27 19:23   ` Daniel Almeida
2025-08-20 15:23 ` [PATCH v4 3/3] samples: rust: add Rust I2C sample driver Igor Korotin
2025-08-27 19:38   ` Daniel Almeida

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).