| Date | Mon, 18 Jul 2022 17:12:23 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH net-next v3 10/47] net: phylink: Adjust link settings based on rate adaptation |
| |
On Fri, Jul 15, 2022 at 05:59:17PM -0400, Sean Anderson wrote: > If the phy is configured to use pause-based rate adaptation, ensure that > the link is full duplex with pause frame reception enabled. Note that these > settings may be overridden by ethtool. > > Signed-off-by: Sean Anderson <sean.anderson@seco.com> > --- > > Changes in v3: > - New > > drivers/net/phy/phylink.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c > index 7fa21941878e..7f65413aa778 100644 > --- a/drivers/net/phy/phylink.c > +++ b/drivers/net/phy/phylink.c > @@ -1445,6 +1445,10 @@ static void phylink_phy_change(struct phy_device *phydev, bool up) > pl->phy_state.speed = phy_interface_speed(phydev->interface, > phydev->speed); > pl->phy_state.duplex = phydev->duplex; > + if (phydev->rate_adaptation == RATE_ADAPT_PAUSE) { > + pl->phy_state.duplex = DUPLEX_FULL; > + rx_pause = true; > + }
I really don't like this - as I've pointed out in my previous email, the reporting in the kernel message log for "Link is Up" will be incorrect if you force the phy_state here like this. If the media side link has been negotiated to be half duplex, we should state that in the "Link is Up" message.
It's only the PCS and MAC that care about this, so this should be dealt with when calling into the PCS and MAC's link_up() method.
The problem we have are the legacy drivers (of which mv88e6xxx and mtk_eth_soc are the only two I'm aware of) that make use of the state->speed and state->duplex when configuring stuff. We could've been down to just mv88e6xxx had the DSA and mv88e6xxx patches been sorted out, but sadly that's now going to be some time off due to reviewer failure.
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
|