netdev - Re: [PATCH v3 1/3] net: phy: bcm54811: New link mode for BroadR-Reach

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-next>] [day] [month] [year] [list]

Message-ID: <b5c6b65b-d4be-4ebc-a529-679d42e56c39@lunn.ch>
Date: Wed, 22 May 2024 16:05:32 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Kamil Horák, 2N <kamilh@...s.com>
Cc: florian.fainelli@...adcom.com, bcm-kernel-feedback-list@...adcom.com,
	hkallweit1@...il.com, netdev <netdev@...r.kernel.org>
Subject: Re: [PATCH v3 1/3] net: phy: bcm54811: New link mode for BroadR-Reach

On Wed, May 22, 2024 at 09:34:05AM +0200, Kamil Horák, 2N wrote:
> 
> On 5/6/24 21:14, Andrew Lunn wrote:
> > On Mon, May 06, 2024 at 04:40:13PM +0200, Kamil Horák - 2N wrote:
> > > Introduce new link modes necessary for the BroadR-Reach mode on
> > > bcm5481x PHY by Broadcom and new PHY tunable to choose between
> > > normal (IEEE) ethernet and BroadR-Reach modes of the PHY.
> > I would of split this into two patches. The reason being, we need the
> > new link mode. But do we need the tunable? Why don't i just use the
> > link mode to select it?
> > 
> > ethtool -s eth42 advertise 1BR10
> 
> Tried to find a way to do the link mode selection this way but the
> advertised modes are only applicable when there is auto-negotiation, which
> is only partially the case of BCM54811: it only has auto-negotiation in IEEE
> mode.
> Thus, to avoid choosing between BroadR-Reach and IEEE mode using the PHY
> Tunable, we would need something else and I am already running out of
> ideas...
> Is there any other possibility?
> 
> In addition, we would have to check for incompatible link modes selected to
> advertise (cannot choose one BRR and one IEEE mode to advertise), or perhaps
> the BRR modes would take precedence, if there is any BRR mode selected to
> advertise, IEEE modes would be ignored.

So it sounds like multiple problems.

1) Passing a specific link mode when not using auto-neg. The current
API is:

ethtool -s eth42 autoneg off speed 10 duplex full

Maybe we want to extend that with

ethtool -s eth42 autoneg off speed 10 duplex full linkmode 1BR10

or just

ethtool -s eth42 autoneg off linkmode 1BR10

You can probably add a new member to ethtool_link_ksettings to pass it
to phylib. From there, it probably needs putting into a phy_device
member, next to speed and duplex. The PHY driver can then use it to
configure the hardware.

2) Invalid combinations of link modes when auto-neg is
enabled. Probably the first question to answer is, is this specific to
this PHY, or generic across all PHYs which support BR and IEEE
modes. If it is generic, we can add tests in
phy_ethtool_ksettings_set() to return EINVAL. If this is specific to
this PHY, it gets messy. When phylib call phy_start_aneg() to
configure the hardware, it does not expect it to return an error. We
might need to add an additional op to phy_driver to allow the PHY
driver to validate settings when phy_ethtool_ksettings_set() is
called. This would help solve a similar problem with a new mediatek
PHY which is broken with forced modes.

    Andrew