Re: [PATCH v2 2/4] rust: i2c: add manual I2C device creation abstractions

[Igor Korotin <igor.korotin.linux@gmail.com>]

On 7/4/25 20:54, Danilo Krummrich wrote:
> On Fri, Jul 04, 2025 at 04:39:12PM +0100, Igor Korotin wrote:
>> -pub struct Device<Ctx: device::DeviceContext = device::Normal>(
>> +pub struct Device<Ctx: device::DeviceContext = device::Normal, State: DeviceState = state::Borrowed>(
>>      Opaque<bindings::i2c_client>,
>>      PhantomData<Ctx>,
>> +    PhantomData<State>,
>>  );
> 
> I see what you're doing here, but I think you're thinking this way too
> complicated.
> 
> I recommend not to reuse the Device type to register a new I2C client device,
> it's adding too much complexity without any real value.
> 
> You also don't want the DeviceContext types for a device registration, since the
> registration will never have any other DeviceContext than device::Normal (see
> also my comment on the sample module).
> 
> DeviceContext types are only useful for &Device (i.e. references) given out for
> a specific scope, such as probe(), remove(), etc.
> 
> The only thing you really want to do is to register a new I2C client device, get
> a i2c::Registration instance and call i2c_unregister_device() when the
> i2c::Registration is dropped.
> 
> This is exactly the same use-case as we have in the auxiliary bus. I highly
> recommend looking at what auxiliary::Registration does [1].
> 
> Also note that if you want a reference to the device in the i2c::Registration,
> you can also add a i2c::Registration::device() method that returns an
> &i2c::Device, which through into() you can obtain an ARef<i2c::Device> from.
> 
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/rust/kernel/auxiliary.rs?h=v6.16-rc4#n299

I took a quick look at the auxiliary Registration abstraction and I see
that it is not applicable for I2C subsystem. The issue here is that I2C
C code doesn't provide with an API that can registers an I2C client from
already existing `struct i2c_client`.

All the APIs provided require `i2c_adapter` with some other inputs
depending on a specific API, they return `i2c_client` being allocated
and registered by the Kernel C code.

Since I'm not controlling initial object allocation, I need somehow to
mark created i2c_client to be dropped automatically and that's why I
implemented `Borrowed/Owned` generic parameter along with this
`DeviceOwned(Device<Ctx, state::Owned>)`. One of the main purposes of
this Borrowed/Owned to prevent accidental casting of `i2c_client` structs.

Alternatively, it could be an implementation of `unregister` method that
user should call explicitly to de-register manually created
`i2c_client`, but as far as I understand this is not Rust "way".

Best Regards
Igor