Re: [PATCH v4 3/3] samples: rust: add Rust I2C sample driver

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

Hello Daniel

On 8/27/2025 8:38 PM, Daniel Almeida wrote:
>> +    }
>> +}
>> +
>> +impl Drop for SampleDriver {
>> +    fn drop(&mut self) {
>> +        dev_dbg!(self.pdev.as_ref(), "Remove Rust I2C driver sample.\n");
>> +    }
>> +}
>> +
>> +// NOTE: The code below is expanded macro module_i2c_driver. It is not used here
>> +//       because we need to manually create an I2C client in `init()`. The macro
>> +//       hides `init()`, so to demo client creation on adapter SAMPLE_I2C_ADAPTER_INDEX
>> +//       we expand it by hand.
>> +type Ops<T> = kernel::i2c::Adapter<T>;
> 
> I am not sure I understand. How is a type alias related to module_i2c_driver at
> all?
> 
> Do all drivers need to introduce the alias above?
> 
>> +
>> +#[pin_data]
>> +struct DriverModule {
>> +    #[pin]
>> +    _driver: kernel::driver::Registration<Ops<SampleDriver>>,
>> +    _reg: i2c::Registration,
>> +}
> 
> I was expecting this to be ARef of something, most likely I2cClient?
> 
> This is where my knowledge of i2c drivers start to fall short, but others will
> probably chime in :)

You're right to mention this. This rust_driver_i2c is not the standard 
way of handling I2C devices. The idea was suggested by Danilo in the 
review of Patch v2: this driver merges an I2C driver sample with manual 
I2C device creation. The module creates a new I2cClient in its init 
function, and this new I2cClient is then probed by the SampleDriver.

In a normal driver it should be different. For example, let’s say 
there’s a platform device — the probe function for this platform device 
would create an I2cClient, and an appropriate I2C driver would then 
probe it.
Thanks for the review.

Best Regards
Igor