Re: [PATCH net-next 4/4] net: dsa: remove phylink_validate() method

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

On Fri, Nov 04, 2022 at 02:25:50PM +0000, Vladimir Oltean wrote:
> On Fri, Nov 04, 2022 at 02:01:15PM +0000, Russell King (Oracle) wrote:
> > On Fri, Nov 04, 2022 at 01:32:48PM +0000, Vladimir Oltean wrote:
> > > On Fri, Nov 04, 2022 at 11:35:08AM +0000, Russell King (Oracle) wrote:
> > > > On Fri, Nov 04, 2022 at 11:24:44AM +0000, Russell King (Oracle) wrote:
> > > > > There is one remaining issue that needs to be properly addressed,
> > > > > which is the bcm_sf2 driver, which is basically buggy. The recent
> > > > > kernel build bot reports reminded me of this.
> > > > > 
> > > > > I've tried talking to Florian about it, and didn't make much progress,
> > > > > so I'm carrying a patch in my tree which at least makes what is
> > > > > provided to phylink correct.
> > > > > 
> > > > > See
> > > > > http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=63d77c1f9db167fd74994860a4a899df5c957aab
> > > > > and all the FIXME comments in there.
> > > > > 
> > > > > This driver really needs to be fixed before we kill DSA's
> > > > > phylink_validate method (although doing so doesn't change anything
> > > > > in mainline, but will remove my reminder that bcm_sf2 is still
> > > > > technically broken.)
> > > > 
> > > > Here's the corrected patch, along with a bit more commentry about the
> > > > problems that I had kicking around in another commit.
> > > 
> > > The inconsistencies in the sf2 driver seem valid - I don't know why/if
> > > the hardware doesn't support flow control on MoCA, internal ports and
> > > (some but not all?!) RGMII modes. I hope Florian can make some clarifications.
> > > 
> > > However, I don't exactly understand your choice of fixing this
> > > inconsistency (by providing a phylink_validate method). Why don't you
> > > simply set MAC_ASYM_PAUSE | MAC_SYM_PAUSE in config->mac_capabilities
> > > from within bcm_sf2_sw_get_caps(), only if we know this is an xMII port
> > > (and not for MoCA and internal PHYs)? Then, phylink_generic_validate()
> > > would know to exclude the "pause" link modes, right?
> > 
> > bcm_sf2_sw_get_caps() doesn't have visibility of which interface mode
> > will be used.
> 
> Update your tree, commit 4d2f6dde4daa ("net: dsa: bcm_sf2: Have PHYLINK
> configure CPU/IMP port(s)") has appeared in net-next and now the check
> in mac_link_up() is for phy_interface_is_rgmii().

Great, one less fixme. Still a couple remaining open.

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