From: "Danilo Krummrich" <dakr@kernel.org>
To: "Igor Korotin" <igor.korotin@linux.dev>
Cc: <jic23@kernel.org>, <lars@metafoo.de>,
<linux-iio@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
<linux-i2c@vger.kernel.org>, <andi.shyti@kernel.org>,
<wsa+renesas@sang-engineering.com>, <ojeda@kernel.org>,
<branstj@gmail.com>,
"Muchamad Coirul Anwar" <muchamadcoirulanwar@gmail.com>
Subject: Re: [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient
Date: Sat, 11 Jul 2026 14:05:05 +0200 [thread overview]
Message-ID: <DJVQ852J7SOH.26YBIJTQ9B66G@kernel.org> (raw)
In-Reply-To: <178376430529.16552.7043868463863908196@linux.dev>
On Sat Jul 11, 2026 at 12:05 PM CEST, Igor Korotin wrote:
> Thanks for reworking this to use `Io` as agreed -- the direction is right.
> But I think the fix is incomplete, and it points at something I'd like
> Danilo's take on.
The reason this should go through the generic I/O backend infrastructure is that
we want this to be able take advantage of the register!() framework.
You are right that currently fallible I/O is not supported. However, this should
easily be fixable by rebasing on top of the latest I/O work [1], plus the
adjustments in [2].
With this, the posted code becomes something like this:
pub struct I2cBackend;
pub struct I2cView<'a, T: ?Sized> {
client: &'a I2cClient<Bound>,
ptr: *mut T,
}
impl<T: ?Sized> Copy for I2cView<'_, T> {}
impl<T: ?Sized> Clone for I2cView<'_, T> { ... }
impl IoBackend for I2cBackend {
type View<'a, T: ?Sized + KnownSize> = I2cView<'a, T>;
fn as_ptr<'a, T: ?Sized + KnownSize>(view: Self::View<'a, T>) -> *mut T {
view.ptr // fake pointer for the register offset
}
unsafe fn project_view<'a, T, U>(
view: Self::View<'a, T>,
ptr: *mut U,
) -> Self::View<'a, U> {
I2cView { client: view.client, ptr }
}
}
impl FallibleIoCapable<u8> for I2cBackend {
fn io_try_read<'a>(view: I2cView<'a, u8>) -> Result<u8> {
...
view.client.smbus_read_byte_data(offset)
}
fn io_try_write<'a>(view: I2cView<'a, u8>, value: u8) -> Result {
...
view.client.smbus_write_byte_data(offset, value)
}
}
[1] https://lore.kernel.org/driver-core/20260706-io_projection-v6-0-72cd5d055d54@garyguo.net/
[2] FallibleIoCapable:
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 7c9f7b85bca3..d97f58c968e7 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -275,6 +276,36 @@ pub trait IoCapable<T>: IoBackend {
fn io_write<'a>(view: Self::View<'a, T>, value: T);
}
+/// Fallible counterpart of [`IoCapable`] for I/O backends where operations can fail at the
+/// transport level (e.g. I2C, SPI).
+///
+/// Infallible backends ([`IoCapable`] implementors) get this for free via blanket implementation.
+/// Fallible-only backends implement this trait directly without implementing [`IoCapable`]; the
+/// infallible [`Io::read`], [`Io::write`], and [`Io::update`] methods will then be unavailable,
+/// enforcing that callers use the `try_*` variants instead.
+pub trait FallibleIoCapable<T>: IoBackend {
+ /// Performs an I/O read of type `T` at `view` and returns the result, or an error if the
+ /// transport-level operation fails.
+ fn io_try_read<'a>(view: Self::View<'a, T>) -> Result<T>;
+
+ /// Performs an I/O write of `value` at `view`, or returns an error if the transport-level
+ /// operation fails.
+ fn io_try_write<'a>(view: Self::View<'a, T>, value: T) -> Result;
+}
+
+impl<B: IoCapable<T>, T> FallibleIoCapable<T> for B {
+ #[inline(always)]
+ fn io_try_read<'a>(view: Self::View<'a, T>) -> Result<T> {
+ Ok(Self::io_read(view))
+ }
+
+ #[inline(always)]
+ fn io_try_write<'a>(view: Self::View<'a, T>, value: T) -> Result {
+ Self::io_write(view, value);
+ Ok(())
+ }
+}
+
/// Trait indicating that an I/O backend supports memory copy operations.
pub trait IoCopyable: IoBackend {
/// Copy contents of `view` to `buffer`.
@@ -644,7 +675,7 @@ fn copy_to_slice(self, data: &mut [u8])
fn try_read8(self, offset: usize) -> Result<u8>
where
usize: IoLoc<Self::Target, u8, IoType = u8>,
- Self::Backend: IoCapable<u8>,
+ Self::Backend: FallibleIoCapable<u8>,
{
self.try_read(offset)
}
@@ -654,7 +685,7 @@ fn try_read8(self, offset: usize) -> Result<u8>
fn try_read16(self, offset: usize) -> Result<u16>
where
usize: IoLoc<Self::Target, u16, IoType = u16>,
- Self::Backend: IoCapable<u16>,
+ Self::Backend: FallibleIoCapable<u16>,
{
self.try_read(offset)
}
@@ -664,7 +695,7 @@ fn try_read16(self, offset: usize) -> Result<u16>
fn try_read32(self, offset: usize) -> Result<u32>
where
usize: IoLoc<Self::Target, u32, IoType = u32>,
- Self::Backend: IoCapable<u32>,
+ Self::Backend: FallibleIoCapable<u32>,
{
self.try_read(offset)
}
@@ -674,7 +705,7 @@ fn try_read32(self, offset: usize) -> Result<u32>
fn try_read64(self, offset: usize) -> Result<u64>
where
usize: IoLoc<Self::Target, u64, IoType = u64>,
- Self::Backend: IoCapable<u64>,
+ Self::Backend: FallibleIoCapable<u64>,
{
self.try_read(offset)
}
@@ -684,7 +715,7 @@ fn try_read64(self, offset: usize) -> Result<u64>
fn try_write8(self, value: u8, offset: usize) -> Result
where
usize: IoLoc<Self::Target, u8, IoType = u8>,
- Self::Backend: IoCapable<u8>,
+ Self::Backend: FallibleIoCapable<u8>,
{
self.try_write(offset, value)
}
@@ -694,7 +725,7 @@ fn try_write8(self, value: u8, offset: usize) -> Result
fn try_write16(self, value: u16, offset: usize) -> Result
where
usize: IoLoc<Self::Target, u16, IoType = u16>,
- Self::Backend: IoCapable<u16>,
+ Self::Backend: FallibleIoCapable<u16>,
{
self.try_write(offset, value)
}
@@ -704,7 +735,7 @@ fn try_write16(self, value: u16, offset: usize) -> Result
fn try_write32(self, value: u32, offset: usize) -> Result
where
usize: IoLoc<Self::Target, u32, IoType = u32>,
- Self::Backend: IoCapable<u32>,
+ Self::Backend: FallibleIoCapable<u32>,
{
self.try_write(offset, value)
}
@@ -714,7 +745,7 @@ fn try_write32(self, value: u32, offset: usize) -> Result
fn try_write64(self, value: u64, offset: usize) -> Result
where
usize: IoLoc<Self::Target, u64, IoType = u64>,
- Self::Backend: IoCapable<u64>,
+ Self::Backend: FallibleIoCapable<u64>,
{
self.try_write(offset, value)
}
@@ -826,10 +857,10 @@ fn write64(self, value: u64, offset: usize)
fn try_read<T, L>(self, location: L) -> Result<T>
where
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: FallibleIoCapable<L::IoType>,
{
let view = io_view::<Self, L::IoType>(self, location.offset())?;
- Ok(Self::Backend::io_read(view).into())
+ Ok(Self::Backend::io_try_read(view)?.into())
}
/// Generic fallible write with runtime bounds check.
@@ -859,12 +890,11 @@ fn try_read<T, L>(self, location: L) -> Result<T>
fn try_write<T, L>(self, location: L, value: T) -> Result
where
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: FallibleIoCapable<L::IoType>,
{
let view = io_view::<Self, L::IoType>(self, location.offset())?;
let io_value = value.into();
- Self::Backend::io_write(view, io_value);
- Ok(())
+ Self::Backend::io_try_write(view, io_value)
}
/// Generic fallible write of a fully-located register value.
@@ -904,7 +934,7 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
where
L: IoLoc<Self::Target, T>,
V: LocatedRegister<Self::Target, Location = L, Value = T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: FallibleIoCapable<L::IoType>,
{
let (location, value) = value.into_io_op();
@@ -937,16 +967,14 @@ fn try_write_reg<T, L, V>(self, value: V) -> Result
fn try_update<T, L, F>(self, location: L, f: F) -> Result
where
L: IoLoc<Self::Target, T>,
- Self::Backend: IoCapable<L::IoType>,
+ Self::Backend: FallibleIoCapable<L::IoType>,
F: FnOnce(T) -> T,
{
let view = io_view::<Self, L::IoType>(self, location.offset())?;
- let value: T = Self::Backend::io_read(view).into();
+ let value: T = Self::Backend::io_try_read(view)?.into();
let io_value = f(value).into();
- Self::Backend::io_write(view, io_value);
-
- Ok(())
+ Self::Backend::io_try_write(view, io_value)
}
/// Generic infallible read with compile-time bounds check.
next prev parent reply other threads:[~2026-07-11 12:05 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 15:15 [RFC PATCH v4 0/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
2026-07-07 15:15 ` [RFC PATCH v4 1/3] i2c: rust: implement SMBus read abstraction via kernel::io::Io for I2cClient Muchamad Coirul Anwar
2026-07-11 10:05 ` Igor Korotin
2026-07-11 12:05 ` Danilo Krummrich [this message]
2026-07-11 12:08 ` Danilo Krummrich
2026-07-07 15:15 ` [RFC PATCH v4 2/3] rust: add minimal IIO subsystem abstractions Muchamad Coirul Anwar
2026-07-11 12:12 ` Danilo Krummrich
2026-07-07 15:15 ` [RFC PATCH v4 3/3] iio: position: add Rust driver for ams AS5600 Muchamad Coirul Anwar
2026-07-08 10:36 ` [RFC PATCH v4 0/3] " Miguel Ojeda
2026-07-08 12:37 ` Muchamad Coirul Anwar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DJVQ852J7SOH.26YBIJTQ9B66G@kernel.org \
--to=dakr@kernel.org \
--cc=andi.shyti@kernel.org \
--cc=branstj@gmail.com \
--cc=igor.korotin@linux.dev \
--cc=jic23@kernel.org \
--cc=lars@metafoo.de \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-iio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=muchamadcoirulanwar@gmail.com \
--cc=ojeda@kernel.org \
--cc=wsa+renesas@sang-engineering.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox