Re: Regression: supported_interfaces filling enforcement

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Tue, Jul 25, 2023 at 12:58:31PM +0200, Linux regression tracking (Thorsten Leemhuis) wrote:
> [CCing the regression list, as it should be in the loop for regressions:
> https://docs.kernel.org/admin-guide/reporting-regressions.html]
> 
> On 13.07.23 17:08, Russell King (Oracle) wrote:
> > On Mon, Jul 10, 2023 at 03:35:56PM +0300, Vladimir Oltean wrote:
> >> On Tue, Jul 04, 2023 at 05:28:47PM +0300, Sergei Antonov wrote:
> >>> This commit seems to break the mv88e6060 dsa driver:
> >>> de5c9bf40c4582729f64f66d9cf4920d50beb897    "net: phylink: require
> >>> supported_interfaces to be filled"
> >>>
> >>> The driver does not fill 'supported_interfaces'. What is the proper
> >>> way to fix it? I managed to fix it by the following quick code.
> >>> Comments? Recommendations?
> >>
> >> Ok, it seems that commit de5c9bf40c45 ("net: phylink: require
> >> supported_interfaces to be filled") was based on a miscalculation.
> > 
> > Yes, it seems so. I'm not great with dealing with legacy stuff - which
> > is something I've stated time and time again when drivers fall behind
> > with phylink development. There's only so much that I can hold in my
> > head, and I can't runtime test the legacy stuff.
> > 
> > I suspect two other DSA drivers are also broken by this:
> > 
> > drivers/net/dsa/dsa_loop.c
> > drivers/net/dsa/realtek/rtl8366rb.c
> > 
> > based upon:
> > 
> > $ grep -lr dsa_switch_ops drivers/net/dsa | xargs grep -L '\.phylink_get_caps.*=' | xargs grep -L '\.adjust_link'
> 
> What happened to this regression? From here it looks like things
> stalled, but I might have missed something, hence allow me to ask:
> 
> Is this still happening? Is anyone still working on fixing this?

I think the discussion got side-tracked into whether mv88e6060 should
be merged into mv88e6xxx, and then just petered out with no further
patch(es) - plus I was on holiday so obviously wasn't paying much
attention.

I suppose the sane thing to do would be to fix all drivers in one
go - maybe something like this:

-	if (ds->ops->phylink_get_caps)
+	if (ds->ops->phylink_get_caps) {
 		ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);
+	} else {
+		/* For legacy drivers */
+		__set_bit(PHY_INTERFACE_MODE_INTERNAL,
+			  &dp->pl_config.supported_interfaces);
+		__set_bit(PHY_INTERFACE_MODE_GMII,
+			  &dp->pl_config.supported_interfaces);
+	}

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