Linux I2C development
 help / color / mirror / Atom feed
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

  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