netdev - Re: [PATCH net-next v1 2/4] rust: net::phy support C45 helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f908e54a-b0e6-49d5-b4ff-768072755a78@lunn.ch>
Date: Wed, 17 Apr 2024 00:30:29 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: FUJITA Tomonori <fujita.tomonori@...il.com>, netdev@...r.kernel.org,
	rust-for-linux@...r.kernel.org, tmgross@...ch.edu
Subject: Re: [PATCH net-next v1 2/4] rust: net::phy support C45 helpers

> I think we could also do a more rusty solution. For example we could
> have these generic functions for `phy::Device`:
> 
>      fn read_register<R: Register>(&mut self, which: R::Index) -> Result<R::Value>;
>      fn write_register<R: Register>(&mut self, which: R::Index, value: R::Value) -> Result;
> 
> That way we can support many different registers without polluting the
> namespace. We can then have a `C45` register and a `C22` register and a
> `C45OrC22` (maybe we should use `C45_OrC22` instead, since I can read
> that better, but let's bikeshed when we have the actual patch).
> 
> Calling those functions would look like this:
> 
>      let value = dev.read_register::<C45>(reg1)?;
>      dev.write_register::<C45>(reg2, value)?;

I don't know how well that will work out in practice. The addressing
schemes for C22 and C45 are different.

C22 simply has 32 registers, numbered 0-31.

C45 has 32 MDIO manageable devices (MMD) each with 65536 registers.

All of the 32 C22 registers have well defined names, which are listed
in mii.h. Ideally we want to keep the same names. The most of the MMD
also have defined names, listed in mdio.h. Many of the registers are
also named in mdio.h, although vendors do tend to add more vendor
proprietary registers.

Your R::Index would need to be a single value for C22 and a tuple of
two values for C45.

There is nothing like `C45OrC22`. A register is either in C22
namespace, or in the C45 namespace.

Where it gets interesting is that there are two methods to access the
C45 register namespace. The PHY driver should not care about this, it
is the MDIO bus driver and the PHYLIB core which handles the two
access mechanisms.

       Andrew