Message-ID: <CAKgT0Uei=6GABwke2vv0D-dY=03uSnkVN4KnKuDR_DNfem2tWg@mail.gmail.com>
Date: Tue, 22 Apr 2025 08:30:10 -0700
From: Alexander Duyck <alexander.duyck@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Joakim Zhang <qiangqing.zhang@....com>, netdev@...r.kernel.org, 
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net] net: phylink: fix suspend/resume with WoL enabled and
 link down

On Tue, Apr 22, 2025 at 2:51 AM Russell King (Oracle)
<linux@...linux.org.uk> wrote:
>
> On Thu, Apr 17, 2025 at 12:49:07PM -0700, Alexander Duyck wrote:

...

> > Sorry, mentioning it didn't occur to me as I have been dealing with
> > the "rolling start" since the beginning. In mac_prepare I deal with
> > this. Specifically the code in mac_prepare will check to see if the
> > link state is currently up with the desired configuration already or
> > not. If it is, it sets a flag that will keep us from doing any changes
> > that would be destructive to the link. If the link is down it
> > basically clears the way for a full reinitialization.
>
> I would much rather avoid any of the "setup" calls (that means
> mac_prepare(), mac_config(), mac_finish(), pcs_config() etc) and
> mac_link_up() if we're going to add support for "rolling start" to
> phylink.

It would be fine by me, at least in the case where the link is already
up and in the correct mode. For the other cases we would still want to
do this in case there is some incomplete setup or something like that.
We would essentially just pull the block out of mac_prepare and place
it wherever is needed to get things up and running.

> That basically means that the MAC needs to be fully configured to
> process packets before phylink_start() or phylink_resume() is called.
>
> This, however, makes me wonder why you'd even want to use phylink in
> this situation, as phylink will be doing virtually nothing for fbnic.

It wasn't that we had wanted to initially, it was more that we were
told that we must use it during the early driver review. I couldn't
really argue against it as our setup fits the model since we have a
MAC/PCS/FEC/PMA/PMD/AN and such. A good example is that I am pretty
certain we will end up using the XPCS driver eventually, however to do
so we must update it as it supports more than XLGMII which is why
somebody added those speeds without adding the correct interfaces.
There are 25G, 50G, and 100G modes that are likely supported if I am
not mistaken. Can't blame them though as that is essentially the state
we are in right now for fbnic. However I was holding off on enabling
the PCS until we can get the MAC pieces sorted out.

For us to fit we are going to have to expand things quite a bit as we
need to add support for higher speeds, QSFP, QSFP-DD, FEC, BMC, and
multi-host type behavior at a minimum. I had more-or-less assumed
there was a desire to push the interface support up to 100G or more
and that was one motivation for pushing us into phylink. By pushing us
in it was a way to get there with us as the lead test/development
vehicle since we are one of the first high speed NICs to actually
expose most of the hardware and not hide it behind firmware.

That said,  I have come to see some of the advantages for us as well.
Things like the QSFP support seems like it should be a light lift as I
just have to add support for basic SFF-8636, which isn't too
dissimilar to SFF-8472, and the rest seems to mostly fall in place
with the device picking up the interface mode from the QSFP module as
there isn't much needed for a DA cable.

Powered by blists - more mailing lists