Re: [PATCH] rust: i2c: remove TryFrom<&Device> implementation for &I2cClient

[Igor Korotin]

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