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

Noted. Will take a look. Thanks for the review

Best Regards
Igor