From: Igor Korotin <igor.korotin@linux.dev>
To: jic23@kernel.org, lars@metafoo.de
Cc: 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,
dakr@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 11:05:05 +0100 [thread overview]
Message-ID: <178376430529.16552.7043868463863908196@linux.dev> (raw)
In-Reply-To: <20260707151542.91997-2-muchamadcoirulanwar@gmail.com>
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.
try_read8()/try_read16() are overridden here with real errno handling,
bypassing IoCapable::io_read entirely -- necessary, since io_read's
signature (-> T, not Result<T>) can't carry a negative errno from
i2c_smbus_read_byte_data.
But that override only covers two of Io's entry points. The generic
try_read<T, L>, try_write<T, L>, try_update<T, L, F>, and try_write_reg
all still route straight through IoCapable::io_read/io_write and aren't
overridden here. Concretely:
- client.try_read::<u8, _>(offset) (as opposed to try_read8()) silently
casts a negative errno to a garbage u8 on failure -- the exact bug
try_read8() exists to avoid, reachable through a different, still-public
method on the same type.
- try_write8()/try_write16() aren't overridden at all, so any write
through them (or the generic try_write) discards the real SMBus return
value and reports Ok(()) even when the transaction failed on the bus
(NACK, arbitration loss, timeout).
- try_update() combines both problems in a single read-modify-write.
This driver only calls try_read8()/try_read16(), so AS5600 itself isn't
affected in practice. But the abstraction being introduced here would be
unsound the moment any write-capable consumer reaches for it -- and I
don't think patching try_write8/try_update one at a time is the right
fix, since it just leaves the same trap for whichever method nobody's
gotten around to overriding yet.
Danilo -- since using Io for I2C was originally your suggestion, I'd like
your read on this before we go further: IoCapable::io_read/io_write are
infallible by signature, which holds for MMIO/PCI-config (bounds-checked
implies success) but doesn't hold for a bus transaction -- I2C can
genuinely fail per-transfer regardless of address validity. Given that,
is Io/IoCapable the right abstraction for I2cClient to implement at all,
or does I2C need its own fallible-native interface rather than overriding
pieces of this one?
Cheers
Igor
next prev parent reply other threads:[~2026-07-11 10: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 [this message]
2026-07-11 12:05 ` Danilo Krummrich
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=178376430529.16552.7043868463863908196@linux.dev \
--to=igor.korotin@linux.dev \
--cc=andi.shyti@kernel.org \
--cc=branstj@gmail.com \
--cc=dakr@kernel.org \
--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