[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ZiFWjE++/i8+lDJa@shell.armlinux.org.uk>
Date: Thu, 18 Apr 2024 18:21:16 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Florian Fainelli <florian.fainelli@...adcom.com>
Cc: Florian Fainelli <f.fainelli@...il.com>, Andrew Lunn <andrew@...n.ch>,
Vladimir Oltean <olteanv@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
netdev@...r.kernel.org
Subject: Re: [PATCH net-next] net: dsa: bcm_sf2: provide own phylink MAC
operations
On Thu, Apr 18, 2024 at 09:52:02AM -0700, Florian Fainelli wrote:
> On 4/16/24 20:13, Florian Fainelli wrote:
> >
> >
> > On 4/16/2024 11:16 AM, Russell King (Oracle) wrote:
> > > On Tue, Apr 16, 2024 at 10:44:38AM -0700, Florian Fainelli wrote:
> > > > On 4/16/24 03:19, Russell King (Oracle) wrote:
> > > > > Convert bcm_sf2 to provide its own phylink MAC operations, thus
> > > > > avoiding the shim layer in DSA's port.c
> > > > >
> > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@...linux.org.uk>
> > > >
> > > > Acked-by: Florian Fainelli <florian.fainelli@...adcom.com>
> > > > Tested-by: Florian Fainelli <florian.fainelli@...adcom.com>
> > >
> > > Great, thanks for testing.
> > >
> > > (Unrelated to this patch... so please don't delay applying based on
> > > ongoing discussion!)
> > >
> > > The other Broadcom driver, b53, isn't going to be as simple - I believe
> > > it uses a mixture of the .adjust_link method for shared ports, and
> > > .phylink_mac_* for user ports. That makes it very awkward now, given
> > > the check that was added (and suggested by Vladimir) to check for the
> > > legacy methods if dsa_switch's .phylink_mac_ops is populated.
> > >
> > > Is there any scope for converting b53 to use only phylink methods for
> > > everything, thus eliminating the .adjust_link callback?
> >
> > It is on the TODO list for sure, and there might be a window later this
> > week to actually work on removing the adjust_link callback once and for
> > all.
>
> This is what I came up with so far:
>
> https://github.com/ffainelli/linux/commits/b53-phylink
>
> Will test later today on the various devices I have.
A few comments...
In "net: dsa: b53: Configure RGMII for 531x5 and MII for 5325", the
commit description says about adding to b53_mac_config(), but the
code change modifies b53_phylink_mac_link_up(). I would think that
the former (as mentioned in the commit message) was the better
place for this rather than in b53_phylink_mac_link_up().
However, I see you move this to b53_phylink_mac_prepare() in
"net: dsa: b53: Move MII/RGMII configuration to mac_prepare" but
I'm not sure why you've chosen mac_prepare() over mac_config().
FYI, the order in which methods at the MAC and PCS are called on a
reconfiguration is:
mac_prepare()
if changing pcs && oldpcs:
oldpcs::pcs_disable()
if newpcs:
newpcs::pre_config()
mac_config()
if newpcs:
newpcs::post_config()
if changing pcs:
newpcs::pcs_enable()
newpcs::pcs_config()
newpcs::pcs_an_restart()
mac_finish()
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists