Re: [PATCH 1/9] rust: i2c: add basic I2C client abstraction
- From
- Rob Herring <robh@kernel.org>
- Date
- 2024-12-19 13:03:25
- Also in
- linux-arm-msm, linux-i2c, lkml, rust-for-linux
- Reply to
- Fabien Parent, 2024-12-18
On Wed, Dec 18, 2024 at 03:36:31PM -0800, Fabien Parent wrote:
quoted hunk ↗ jump to hunk
From: Fiona Behrens <redacted> Implement an abstraction to write I2C device drivers. The abstraction is pretty basic and provides just the infrastructure to probe a device from I2C/OF device_id and abstract `i2c_client`. The client will be used by the Regmap abstraction to perform I/O on the I2C bus. Signed-off-by: Fiona Behrens <redacted> Co-developed-by: Fabien Parent <redacted> Signed-off-by: Fabien Parent <redacted> --- MAINTAINERS | 1 + rust/bindings/bindings_helper.h | 1 + rust/helpers/helpers.c | 1 + rust/helpers/i2c.c | 13 ++ rust/kernel/i2c.rs | 288 ++++++++++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 2 + 6 files changed, 306 insertions(+)diff --git a/MAINTAINERS b/MAINTAINERS index 6b9e10551392c185b9314c9f94edeaf6e85af58f..961fe4ed39605bf489d1d9e473f47bccb692ff14 100644 --- a/MAINTAINERS +++ b/MAINTAINERS@@ -10796,6 +10796,7 @@ F: include/linux/i2c-smbus.h F: include/linux/i2c.h F: include/uapi/linux/i2c-*.h F: include/uapi/linux/i2c.h +F: rust/kernel/i2c.rs I2C SUBSYSTEM HOST DRIVERS M: Andi Shyti <andi.shyti@kernel.org>diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index e9fdceb568b8f94e602ee498323e5768a40a6cba..a882efb90bfc27960ef1fd5f2dc8cc40533a1c27 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h@@ -16,6 +16,7 @@ #include <linux/file.h> #include <linux/firmware.h> #include <linux/fs.h> +#include <linux/i2c.h> #include <linux/jiffies.h> #include <linux/jump_label.h> #include <linux/mdio.h>diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index 0640b7e115be1553549312dcfdf842bcae3bde1b..630e903f516ee14a51f46ff0bcc68e8f9a64021a 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c@@ -15,6 +15,7 @@ #include "device.c" #include "err.c" #include "fs.c" +#include "i2c.c" #include "io.c" #include "jump_label.c" #include "kunit.c"diff --git a/rust/helpers/i2c.c b/rust/helpers/i2c.c new file mode 100644 index 0000000000000000000000000000000000000000..8ffdc454e7597cc61909da5b3597057aeb5f7299 --- /dev/null +++ b/rust/helpers/i2c.c@@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/i2c.h> + +void *rust_helper_i2c_get_clientdata(const struct i2c_client *client) +{ + return i2c_get_clientdata(client); +} + +void rust_helper_i2c_set_clientdata(struct i2c_client *client, void *data) +{ + i2c_set_clientdata(client, data); +}diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs new file mode 100644 index 0000000000000000000000000000000000000000..efa03335e5b59e72738380e94213976b2464c25b --- /dev/null +++ b/rust/kernel/i2c.rs@@ -0,0 +1,288 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Abstractions for the I2C bus. +//! +//! C header: [`include/linux/i2c.h`](srctree/include/linux/i2c.h) + +use crate::{ + bindings, container_of, + device::Device, + device_id::{self, RawDeviceId}, + driver, + error::{to_result, Result}, + of, + prelude::*, + str::CStr, + types::{ARef, ForeignOwnable, Opaque}, + ThisModule, +}; + +/// Abstraction for `bindings::i2c_device_id`. +#[repr(transparent)] +#[derive(Clone, Copy)] +pub struct DeviceId(bindings::i2c_device_id); + +impl DeviceId { + /// Create a new device id from an I2C name. + pub const fn new(name: &CStr) -> Self { + let src = name.as_bytes_with_nul(); + // TODO: Replace with `bindings::i2c_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] as _; + i += 1; + }
You can simplify this now that char maps to u8 (in rust next).
quoted hunk ↗ jump to hunk
+ + Self(i2c) + } +} + +// SAFETY: +// * `DeviceId` is a `#[repr(transparent)` wrapper of `i2c_device_id` and does not add +// additional invariants, so it's safe to transmute to `RawType`. +// * `DRIVER_DATA_OFFSET` is the offset to the `data` field. +unsafe impl RawDeviceId for DeviceId { + type RawType = bindings::i2c_device_id; + + const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::i2c_device_id, driver_data); + + fn index(&self) -> usize { + self.0.driver_data as _ + } +} + +/// I2C [`DeviceId`] table. +pub type IdTable<T> = &'static dyn device_id::IdTable<DeviceId, T>; + +/// An adapter for the registration of I2C drivers. +#[doc(hidden)] +pub struct Adapter<T: Driver + 'static>(T); + +impl<T: Driver + 'static> driver::RegistrationOps for Adapter<T> { + type RegType = bindings::i2c_driver; + + fn register( + i2cdrv: &Opaque<Self::RegType>, + name: &'static CStr, + module: &'static ThisModule, + ) -> Result { + // SAFETY: It's safe to set the fields of `struct i2c_driver` on initialization. + unsafe { + (*i2cdrv.get()).driver.name = name.as_char_ptr(); + (*i2cdrv.get()).probe = Some(Self::probe_callback); + (*i2cdrv.get()).remove = Some(Self::remove_callback); + if let Some(t) = T::I2C_ID_TABLE { + (*i2cdrv.get()).id_table = t.as_ptr(); + } + if let Some(t) = T::OF_ID_TABLE { + (*i2cdrv.get()).driver.of_match_table = t.as_ptr(); + } + } + + // SAFETY: `i2cdrv` is guaranteed to be a valid `RegType`. + to_result(unsafe { bindings::i2c_register_driver(module.0, i2cdrv.get()) }) + } + + fn unregister(i2cdrv: &Opaque<Self::RegType>) { + // SAFETY: `i2cdrv` is guaranteed to be a valid `RegType`. + unsafe { bindings::i2c_del_driver(i2cdrv.get()) }; + } +} + +impl<T: Driver> Adapter<T> { + /// Get the [`Self::IdInfo`] that matched during probe. + fn id_info(client: &mut Client) -> Option<&'static T::IdInfo> { + let id = <Self as driver::Adapter>::id_info(client.as_ref()); + if id.is_some() { + return id; + } + + // SAFETY: `client` and `client.as_raw()` are guaranteed to be valid. + let id = unsafe { bindings::i2c_client_get_device_id(client.as_raw()) }; + if !id.is_null() { + // 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 { &*id.cast::<DeviceId>() }; + return Some(T::I2C_ID_TABLE?.info(id.index())); + }
You aren't handling the DT based matching. Rob