From: Igor Korotin <igor.korotin@linux.dev>
To: rust-for-linux@vger.kernel.org, linux-pm@vger.kernel.org,
linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Sebastian Reichel <sre@kernel.org>,
Miguel Ojeda <ojeda@kernel.org>, Gary Guo <gary@garyguo.net>,
Tamir Duberstein <tamird@kernel.org>,
Alice Ryhl <aliceryhl@google.com>, Boqun Feng <boqun@kernel.org>,
Danilo Krummrich <dakr@kernel.org>,
Bruce Robertson <brucer42@gmail.com>
Subject: Re: [RFC PATCH 1/3] rust: i2c: add SMBus byte transfer helpers
Date: Sat, 11 Jul 2026 11:34:11 +0100 [thread overview]
Message-ID: <178376605131.22511.1602274538745913609@linux.dev> (raw)
In-Reply-To: <20260708214738.25008-2-brucer42@gmail.com>
Hi Bruce,
Thanks for the patch. Same feedback I gave Muchamad on a similar patch:
I2cClient should implement the `Io` trait rather than adding standalone
SMBus methods like these -- this was already agreed on previously [1][2].
There's also an in-flight series doing exactly this rework right now:
Muchamad's "[RFC PATCH v4 1/3] i2c: rust: implement SMBus read
abstraction via kernel::io::Io for I2cClient". Worth looking at before
reworking this yourself, both to avoid landing two competing
implementations of the same thing, and because I've just raised a
problem with that approach that affects this patch too: overriding
try_read8()/try_read16() isn't enough on its own -- the generic
try_read/try_write/try_update entry points still silently swallow real
SMBus failures (NACK, arbitration loss, timeout) unless every one of
them gets the same treatment. Your smbus_update_bits() is exactly the
kind of read-modify-write that's affected (my reply on that thread,
Message-ID <178376430529.16552.7043868463863908196@linux.dev>, has the
detailed breakdown).
I'd suggest holding this patch until that discussion settles -- Cc'ing
Danilo here too, since it's ultimately a question of whether
Io/IoCapable is the right abstraction for I2C at all -- rather than
reworking it in parallel.
Cheers
Igor
[1] https://lore.kernel.org/rust-for-linux/20260131-i2c-adapter-v1-4-5a436e34cd1a@gmail.com/
[2] https://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git/commit/?h=driver-core-testing&id=121d87b28e1d9061d3aaa156c43a627d3cb5e620
next prev parent reply other threads:[~2026-07-11 10:34 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-08 21:47 [RFC PATCH 0/3] rust: power_supply class abstraction and SMB347 charger driver Bruce Robertson
2026-07-08 21:47 ` [RFC PATCH 1/3] rust: i2c: add SMBus byte transfer helpers Bruce Robertson
2026-07-11 10:34 ` Igor Korotin [this message]
2026-07-11 12:06 ` Danilo Krummrich
2026-07-08 21:47 ` [RFC PATCH 2/3] rust: power_supply: add power supply class abstraction Bruce Robertson
2026-07-09 9:04 ` Alice Ryhl
2026-07-08 21:47 ` [RFC PATCH 3/3] power: supply: add Rust SMB347 charger driver Bruce Robertson
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=178376605131.22511.1602274538745913609@linux.dev \
--to=igor.korotin@linux.dev \
--cc=aliceryhl@google.com \
--cc=boqun@kernel.org \
--cc=brucer42@gmail.com \
--cc=dakr@kernel.org \
--cc=gary@garyguo.net \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=rust-for-linux@vger.kernel.org \
--cc=sre@kernel.org \
--cc=tamird@kernel.org \
/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.