[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20221104142549.gdgolb6uljq3b7kc@skbuf>
Date: Fri, 4 Nov 2022 14:25:50 +0000
From: Vladimir Oltean <vladimir.oltean@....com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
CC: Florian Fainelli <f.fainelli@...il.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
Claudiu Manoil <claudiu.manoil@....com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
"UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Sean Anderson <sean.anderson@...o.com>,
Colin Foster <colin.foster@...advantage.com>,
Andrew Lunn <andrew@...n.ch>,
Vivien Didelot <vivien.didelot@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net-next 4/4] net: dsa: remove phylink_validate() method
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().
Powered by blists - more mailing lists