* [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient
@ 2026-10-19 15:23 Nicolás Antinori
2026-10-21 10:38 ` Igor Korotin
2026-10-21 12:32 ` Igor Korotin
0 siblings, 2 replies; 5+ messages in thread
From: Nicolás Antinori @ 2026-10-19 15:23 UTC (permalink / raw)
To: Igor Korotin, Danilo Krummrich, Georgios Androutsopoulos
Cc: Nicolás Antinori, Alexandre Courbot, Alice Ryhl,
Andreas Hindborg, Benno Lossin, Björn Roy Baron, Boqun Feng,
Brigham Campbell, Daniel Almeida, Gary Guo, Jori Koolstra,
Miguel Ojeda, Onur Özkan, Shuah Khan, Tamir Duberstein,
Trevor Gross, linux-kernel, rust-for-linux, linux-kernel-mentees
Remove the `TryFrom<&device::Device<Ctx>>` implementation for
`&I2cClient<Ctx>` since it has had no users since its introduction.
Fixes: 57c5bd9aee94 ("rust: i2c: add basic I2C device and driver abstractions")
Reported-by: Georgios Androutsopoulos <georgeandrout13@gmail.com>
Closes: https://github.com/Rust-for-Linux/linux/issues/1264
Signed-off-by: Nicolás Antinori <nico.antinori.7@gmail.com>
---
To make sure that there were no remaining users, I checked for usages of
the `try_from` method and the `I2cClient` struct using rust analyzer,
activated the modules that were using `I2cClient`, removed the
implementation and compiled the kernel (did not trust entirely in rust
analyzer because it does not find usages that depend on this
implementation like the `try_into()` automatic implementation).
I also checked for usages in all major versions in most recent versions
until the introduction of the trait implementation (from 7.3-rc7 to
6.19), rust-next and linux-next to find the last usage and mark the
commit in the `Fixes:` tag.
rust/kernel/i2c.rs | 21 ---------------------
1 file changed, 21 deletions(-)
diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
index 0487bae811fb..b4ff25c830fc 100644
--- a/rust/kernel/i2c.rs
+++ b/rust/kernel/i2c.rs
@@ -5,7 +5,6 @@
// I2C Driver abstractions.
use crate::{
acpi,
- container_of,
device,
device_id::{
RawDeviceId,
@@ -517,26 +516,6 @@ fn as_ref(&self) -> &device::Device<Ctx> {
}
}
-impl<Ctx: device::DeviceContext> TryFrom<&device::Device<Ctx>> for &I2cClient<Ctx> {
- type Error = kernel::error::Error;
-
- fn try_from(dev: &device::Device<Ctx>) -> Result<Self, Self::Error> {
- // SAFETY: By the type invariant of `Device`, `dev.as_raw()` is a valid pointer to a
- // `struct device`.
- if unsafe { bindings::i2c_verify_client(dev.as_raw()).is_null() } {
- return Err(EINVAL);
- }
-
- // SAFETY: We've just verified that the type of `dev` equals to
- // `bindings::i2c_client_type`, hence `dev` must be embedded in a valid
- // `struct i2c_client` as guaranteed by the corresponding C code.
- let idev = unsafe { container_of!(dev.as_raw(), bindings::i2c_client, dev) };
-
- // SAFETY: `idev` is a valid pointer to a `struct i2c_client`.
- Ok(unsafe { &*idev.cast() })
- }
-}
-
// SAFETY: A `I2cClient` is always reference-counted and can be released from any thread.
unsafe impl Send for I2cClient {}
--
2.47.3
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient
2026-10-19 15:23 [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient Nicolás Antinori
@ 2026-10-21 10:38 ` Igor Korotin
2026-10-21 12:32 ` Igor Korotin
1 sibling, 0 replies; 5+ messages in thread
From: Igor Korotin @ 2026-10-21 10:38 UTC (permalink / raw)
To: Nicolás Antinori, Danilo Krummrich, Georgios Androutsopoulos
Cc: Alexandre Courbot, Alice Ryhl, Andreas Hindborg, Benno Lossin,
Björn Roy Baron, Boqun Feng, Brigham Campbell,
Daniel Almeida, Gary Guo, Jori Koolstra, Miguel Ojeda,
Onur Özkan, Shuah Khan, Tamir Duberstein, Trevor Gross,
linux-kernel, rust-for-linux, linux-kernel-mentees
Hi Nicolás,
Checked -- confirmed no other users in the tree, and the Fixes tag is
correct. Agreed removal is the right call here over a lifetime fix,
given the new driver model has no use for this upcast anyway.
Acked-by: Igor Korotin <igor.korotin@linux.dev>
I'll take this through rust-i2c-next.
Cheers
Igor
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient
2026-10-19 15:23 [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient Nicolás Antinori
2026-10-21 10:38 ` Igor Korotin
@ 2026-10-21 12:32 ` Igor Korotin
2026-10-21 12:44 ` Gary Guo
1 sibling, 1 reply; 5+ messages in thread
From: Igor Korotin @ 2026-10-21 12:32 UTC (permalink / raw)
To: Danilo Krummrich, Nicolás Antinori, Georgios Androutsopoulos
Cc: Muchamad Coirul Anwar, Jonathan Cameron, Alexandre Courbot,
Alice Ryhl, Andreas Hindborg, Benno Lossin, Björn Roy Baron,
Boqun Feng, Brigham Campbell, Daniel Almeida, Gary Guo,
Jori Koolstra, Miguel Ojeda, Onur Özkan, Shuah Khan,
Tamir Duberstein, Trevor Gross, linux-kernel, rust-for-linux,
linux-kernel-mentees
Hi all,
Want to flag something before this lands -- I gave this an Acked-by
already, but while reviewing Muchamad's concurrent AS5600/IIO series
(Cc'd) I hit a real conflict with it.
His IioDriver::read_raw is called by the generic IIO core, which only
ever hands the driver a generic &Device<Bound> -- it has no way to know
the device is I2C specifically. To do SMBus I/O there, the driver needs
to recover &I2cClient<Bound> from that generic reference, which is
exactly what this TryFrom did.
I checked whether storing a handle from probe() instead avoids the need
-- it doesn't. AlwaysRefCounted is only implemented for I2cClient
(Normal context), not I2cClient<Bound>, so there's nothing you can hold
onto from probe() that still proves "currently bound" later. And there's
no way to get back from Normal to Bound (Deref only goes the other
way). As_raw() on both Device and I2cClient is private to kernel::i2c,
so a driver can't reimplement the conversion itself either.
So removing this outright leaves no path for any bus-agnostic subsystem
callback (IIO being the concrete case, but this'll recur for SPI, HID,
etc.) to ever recover a bus-specific handle after probe. Danilo, you
mentioned a fix tying the lifetimes together properly
(impl<'a, Ctx> TryFrom<&'a Device<Ctx>> for &'a I2cClient<Ctx>) as an
alternative to removal -- given this, I think that's the right call
instead of deleting it. It closes the actual soundness hole (output
lifetime no longer independent of input) without removing the only way
generic callbacks can get back to a specific bus type.
I haven't pushed Nicolás's patch to rust-i2c-next yet -- holding off
until this is settled.
Cheers
Igor
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient
2026-10-21 12:32 ` Igor Korotin
@ 2026-10-21 12:44 ` Gary Guo
2026-10-21 16:46 ` Igor Korotin
0 siblings, 1 reply; 5+ messages in thread
From: Gary Guo @ 2026-10-21 12:44 UTC (permalink / raw)
To: Igor Korotin, Danilo Krummrich, Nicolás Antinori,
Georgios Androutsopoulos
Cc: Muchamad Coirul Anwar, Jonathan Cameron, Alexandre Courbot,
Alice Ryhl, Andreas Hindborg, Benno Lossin, Björn Roy Baron,
Boqun Feng, Brigham Campbell, Daniel Almeida, Gary Guo,
Jori Koolstra, Miguel Ojeda, Onur Özkan, Shuah Khan,
Tamir Duberstein, Trevor Gross, linux-kernel, rust-for-linux,
linux-kernel-mentees
On Wed Oct 21, 2026 at 1:32 PM BST, Igor Korotin wrote:
> Hi all,
>
> Want to flag something before this lands -- I gave this an Acked-by
> already, but while reviewing Muchamad's concurrent AS5600/IIO series
> (Cc'd) I hit a real conflict with it.
>
> His IioDriver::read_raw is called by the generic IIO core, which only
> ever hands the driver a generic &Device<Bound>
That'd be a subsystem design issue.
In the latest device driver lifecycle model design, class device must not mess
around with parent bus device. It should use its own mechanism to carry around
private data, which is initialized when creating the class device registration.
The callback bridges should simply retrieve that registered data and hand it to
the driver callback. The driver can decide to put whatever it needs there, and
this include a `&Device<Bound>` of the bus device (hint -- you probably don't
need it, it should just be a reference to whatever bus device resoruce the
callback needs).
The need for conversion is a strong indication that it's not aligned with the
device driver lifetime design.
Best,
Gary
> -- it has no way to know
> the device is I2C specifically. To do SMBus I/O there, the driver needs
> to recover &I2cClient<Bound> from that generic reference, which is
> exactly what this TryFrom did.
>
> I checked whether storing a handle from probe() instead avoids the need
> -- it doesn't. AlwaysRefCounted is only implemented for I2cClient
> (Normal context), not I2cClient<Bound>, so there's nothing you can hold
> onto from probe() that still proves "currently bound" later. And there's
> no way to get back from Normal to Bound (Deref only goes the other
> way). As_raw() on both Device and I2cClient is private to kernel::i2c,
> so a driver can't reimplement the conversion itself either.
>
> So removing this outright leaves no path for any bus-agnostic subsystem
> callback (IIO being the concrete case, but this'll recur for SPI, HID,
> etc.) to ever recover a bus-specific handle after probe. Danilo, you
> mentioned a fix tying the lifetimes together properly
> (impl<'a, Ctx> TryFrom<&'a Device<Ctx>> for &'a I2cClient<Ctx>) as an
> alternative to removal -- given this, I think that's the right call
> instead of deleting it. It closes the actual soundness hole (output
> lifetime no longer independent of input) without removing the only way
> generic callbacks can get back to a specific bus type.
>
> I haven't pushed Nicolás's patch to rust-i2c-next yet -- holding off
> until this is settled.
>
> Cheers
> Igor
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient
2026-10-21 12:44 ` Gary Guo
@ 2026-10-21 16:46 ` Igor Korotin
0 siblings, 0 replies; 5+ messages in thread
From: Igor Korotin @ 2026-10-21 16:46 UTC (permalink / raw)
To: Gary Guo, Danilo Krummrich, Nicolás Antinori,
Georgios Androutsopoulos
Cc: Muchamad Coirul Anwar, Jonathan Cameron, Alexandre Courbot,
Alice Ryhl, Andreas Hindborg, Benno Lossin, Björn Roy Baron,
Boqun Feng, Brigham Campbell, Daniel Almeida, Jori Koolstra,
Miguel Ojeda, Onur Özkan, Shuah Khan, Tamir Duberstein,
Trevor Gross, linux-kernel, rust-for-linux, linux-kernel-mentees
Hi Gary,
Understood. Point accepted.
The case I was worried about has an alternative resolution that doesn't
need anything from kernel::i2c. Proceeding with this patch as-is.
Acked-by: Igor Korotin <igor.korotin@linux.dev>
Cheers
Igor
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-21 16:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19 15:23 [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient Nicolás Antinori
2026-10-21 10:38 ` Igor Korotin
2026-10-21 12:32 ` Igor Korotin
2026-10-21 12:44 ` Gary Guo
2026-10-21 16:46 ` Igor Korotin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®