Re: [PATCH] rust: i2c: avoid locking when calling I2cAdapter::inc_ref

["Nicolás Antinori" <nico.antinori.7@gmail.com>]

Hello Gary,

On Wed Jun 17, 2026 at 11:12 AM -03, Gary Guo wrote:
> On Mon Jun 15, 2026 at 9:10 PM BST, Nicolás Antinori wrote:
>> diff --git a/rust/kernel/i2c.rs b/rust/kernel/i2c.rs
>> index 624b971ca8b0..d89c42691dfe 100644
>> --- a/rust/kernel/i2c.rs
>> +++ b/rust/kernel/i2c.rs
>> @@ -426,8 +426,11 @@ pub fn get(index: i32) -> Result<ARef<Self>> {
>>  // SAFETY: Instances of `I2cAdapter` are always reference-counted.
>>  unsafe impl AlwaysRefCounted for I2cAdapter {
>>      fn inc_ref(&self) {
>> -        // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>> -        unsafe { bindings::i2c_get_adapter(self.index()) };
>> +        // SAFETY: The existence of a shared reference guarantees that the refcounts are non-zero.
>> +        unsafe {
>> +            bindings::__module_get((*self.as_raw()).owner);
>> +            bindings::get_device(&raw mut (*self.as_raw()).dev);
>
> Instead of open coding this sequence, it would be better to add a C API that
> does exactly this (getting another reference from existing one).

I based this solution on the logic used in I2cClient (where only
get_device is needed) and by verifying which counters the C
implementation (i2c_get_adapter) increments.

The i2c_get_adapter function in i2c-core-base.c performs two increments
(module increment is done by calling try_module_get, but in this case,
because inc_ref operates on an already live instance, unconditionally 
incrementing the count with `__module_get` should be safe).

If I understand correctly, the idea would be to introduce a helper 
function on the C side, for example:
void i2c_adapter_increment(struct i2c_adap *adap);

We would perform the increments there and call it from Rust. Using this 
in inc_ref would be safe because the existence of &self guarantees that 
we already have a valid, live instance.

Is there any concern regarding this function beign exposed to the C
side? To use it safely in C, callers would have to ensure that *adap
points to a valid instance.

>
> That said, is there an actual user that needs this, or are we just implementing
> AlwaysRefCounted preemptively?

The only user I could find at the moment is the example driver
(impl platform::Driver for SampleDriver ..) in
samples/rust/rust_i2c_client.rs.

Implementing AlwaysRefCounted is required for types wrapped in ARef,
which is an smart pointer that handles custom reference counting.
Since I2cAdapter::get returns an ARef<I2cAdapter> upon success, the 
trait must be implemented.

Thank you!

>
> Best,
> Gary
>
>> +        }
>>      }
>>
>>      unsafe fn dec_ref(obj: NonNull<Self>) {
>> --
>> 2.47.3