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: 13+ 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-14 16:08 ` Muchamad Coirul Anwar
2026-07-11 12:08 ` Danilo Krummrich
2026-07-14 16:10 ` Muchamad Coirul Anwar
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-14 16:20 ` Muchamad Coirul Anwar
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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.