Re: [PATCH net v2] net: phy: phy_ethtool_ksettings_set: Don't discard phy_start_aneg's return

On Mon, Nov 08, 2021 at 07:01:23PM +0100, Heiner Kallweit wrote:

On 08.11.2021 18:57, Andrew Lunn wrote:

quoted

quoted

It is BCM53125. Currently, you can set "mdix auto|off|on" which does
not take any effect.  The chip will do what is its default depending
on copper autonegotiation.

I am adding support for setting "mdix auto|off". I want the thing to error on "mdix on".
Where would I add that check?

/* MDI or MDI-X status/control - if MDI/MDI_X/AUTO is set then
 * the driver is required to renegotiate link
 */
#define ETH_TP_MDI_INVALID	0x00 /* status: unknown; control: unsupported */
#define ETH_TP_MDI		0x01 /* status: MDI;     control: force MDI */
#define ETH_TP_MDI_X		0x02 /* status: MDI-X;   control: force MDI-X */
#define ETH_TP_MDI_AUTO		0x03 /*                  control: auto-select */

So there are three valid settings. And you are saying you only want to
implement two of them? If the hardware can do all three, you should
implement all three.

If we would like to support PHY's that don't support all MDI modes then
supposedly this would require to add ETHTOOL_LINK_MODE bits for the
MDI modes. Then we could use the generic mechanism to check the bits in
the "supported" bitmap.

We'll have to add more stuff to phylink to avoid MACs masking these
bits... ETHTOOL_LINK_MODE seems to be becoming a disorganised dumping
ground for random stuff. :(

Also, what would these bits in the advertising bitmap mean?

Finally, how do we handle the lack of these bits for existing PHYs
that already implement MDI modes?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!