Re: [RFC PATCH v1] rust: net::phy support to C45 registers access

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

On 31.05.24 15:58, FUJITA Tomonori wrote:
> Hi,
> 
> On Thu, 30 May 2024 08:05:50 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
>>> 802.3 C22 defines 16 through 31 as vendor specific. Most of the
>>> #defines appear to come from the S390 driver. We probably should not
>>> add them in the Rust API when there meaning is not clearly
>>> defined. However, a driver should be able to use them, e.g, by
>>> defining their own vendor specific name to a given value.
>>
>> @Fujita, you could fix this by adding a `VendorSpecific(T)` enum variant
>> and add a generic parameter to `C22`. Then the user has the option to
> 
> I'm not sure. You meant something like:
> 
> pub enum C22<T> {
>     Bmcr,
>     Bmsr,
>     VendorSpecific(T),
> }
> 
> ?

Yes.

> Then it's not handy to use the defined registers?
> 
> let ret = phy.read(reg::C22::<u8>::Bmcr)?;
> 
> I prefer:
> 
> let ret = phy.read(reg::C22::Bmcr)?;

Hmm I guess that it probably can't infer a default type. In that case,
you can also try if a struct approach for C22 is better. So:

    pub struct C22(u16); // choose the correct size

    impl C22 {
        pub fn new(reg: u16) -> Self;

        pub const BMCR: Self = Self::new(uapi::MII_BMCR);
        // ...
    }

And then a specific driver can declare its own constants.

---
Cheers,
Benno