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: <26f64e48-4fd3-4e0f-b7c5-e77abeee391a@lunn.ch>
Date: Tue, 16 Apr 2024 14:38:11 +0200
From: Andrew Lunn <andrew@...n.ch>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: 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

> > In summary, the C API is a bit of a mess.
> > 
> > For the Rust API we have two sensible choices:
> > 
> > 1) It is the same mess as the C API, so hopefully one day we will fix
> >    both at the same time.
> > 
> > 2) We define a different API which correctly separate C45 bus access
> >    from C45 registers.
> > 
> > How you currently defined the Rust API is neither of these.
> 
> Which do your prefer?
> 
> If I correctly understand the original driver code, C45 bus protocol
> is used. Adding functions for C45 bus protocol read/write would be
> enough for this driver, I guess.

Now i've read more of the patches, i can see that the MDIO bus master
is C45 only. At least, that is all that is implemented in the
driver. So for this combination of MAC and PHY, forcing C45 register
access using C45 bus protocol will work.

However, can you combine this PHY with some other MDIO bus master,
which does not support C45? Then C45 over C22 would need to be used?
Looking at the data sheet i found online, there is no suggestion it
does support C22 bus protocol. All the diagrams/tables only show C45
bus protocol.

So this PHY is a special case. So you can use wrapper methods which
force C45 bus protocol, and ignore phylib support for performing C45
over C22 when needed. However, while doing this:

1: Clearly document that these helpers are not generic, they force C45
   register access using C45 bus protocol, and should only by used PHY
   drivers which know the PHY device does not support C45 over C22

2: Think about naming. At some point we are going to add the generic
   helpers for accessing C45 registers which leave the core to decide
   if to perform a C45 bus protocol access or C45 over C22. Those
   generic helpers need to have the natural name for accessing a C45
   register since 99% of drivers will be using them. The helpers you
   add now need to use a less common name.

	Andrew