Re: [PATCH net-next v4 1/4] rust: core abstractions for network PHY drivers

[Benno Lossin <benno.lossin@proton.me>]

On 17.10.23 09:32, FUJITA Tomonori wrote:
> On Tue, 17 Oct 2023 07:06:38 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
>>> Secondly, What's the difference between read() and write(), where you
>>> think that read() is &self write() is &mut self.
>>
>> This is just the standard Rust way of using mutability. For reading one
>> uses `&self` and for writing `&mut self`. The only thing that is special
>> here is the stats that are updated. But I thought that it still could fit
>> Rust by the following pattern:
>> ```rust
>>       pub struct TrackingReader {
>>           buf: [u8; 64],
>>           num_of_reads: Mutex<usize>,
>>       }
>>
>>       impl TrackingReader {
>>           pub fn read(&self, idx: usize) -> u8 {
>>               *self.num_of_reads.lock() += 1;
>>               self.buf[idx]
>>           }
>>       }
>>
>> ```
>>
>> And after taking a look at `mdiobus_read` I indeed found a mutex.
> 
> Yes, both read() and write() update the stats with mdiobus's lock.
> 
> 
>>> read() is reading from hardware register. write() is writing a value
>>> to hardware register. Both updates the object that phy_device points
>>> to?
>>
>> Indeed, I was just going with the standard way of suggesting `&self`
>> for reads, there are of course exceptions where `&mut self` would make
>> sense. That being said in this case both options are sound, since
>> the C side locks a mutex.
> 
> I see. I use &mut self for both read() and write().

I would recommend documenting this somewhere (why `read` is `&mut`), since
that is a bit unusual (why restrict something more than necessary?).

-- 
Cheers,
Benno