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

From: Benedikt Spranger <[email-masked]>
Date: Mon, 8 Nov 2021 16:06:53 +0100

On Mon, 8 Nov 2021 14:25:48 +0000
"Russell King (Oracle)" <[email-masked]> wrote:

> On Mon, Nov 08, 2021 at 03:18:34PM +0100, [email-masked] wrote:
> > From: Bastian Germann <[email-masked]>
> > 
> > Take the return of phy_start_aneg into account so that ethtool will
> > handle negotiation errors and not silently accept invalid input.
> 
> I don't think this description is accurate. If we get to call
> phy_start_aneg() with invalid input, then something has already
> gone wrong.
The MDI/MDIX/auto-MDIX settings are not checked before calling
phy_start_aneg(). If the PHY supports forcing MDI and auto-MDIX, but
not forcing MDIX _phy_start_aneg() returns a failure, which is silently
ignored.

> As Andrew has already explained, an error from this
> function means that something went wrong with PHY communication.
Or for MDI/MDIX setings, the PHY does not support a feature/setting.

> All validation should have happened prior to this function being
> called.
OK.
Just to be clear: The PHY driver should check the settings and return
an error before calling phy_ethtool_ksettings_set() ?

Thanks
    Bene