RE: [RFC net-next 6/8] net: phylink: Configure MAC/PCS when link is up without PHY

[Jose Abreu]

From: Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx>
Date: Jan/27/2020, 13:37:52 (UTC+00:00)

> On Mon, Jan 27, 2020 at 12:50:54PM +0000, Jose Abreu wrote:
> > From: Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx>
> > Date: Jan/27/2020, 11:46:00 (UTC+00:00)
> > 
> > > On Mon, Jan 27, 2020 at 11:38:05AM +0000, Jose Abreu wrote:
> > > > From: Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx>
> > > > Date: Jan/27/2020, 11:21:02 (UTC+00:00)
> > > > 
> > > > > On Mon, Jan 27, 2020 at 12:09:11PM +0100, Jose Abreu wrote:
> > > > > > When we don't have any real PHY driver connected and we get link up from
> > > > > > PCS we shall configure MAC and PCS for the desired speed and also
> > > > > > resolve the flow control settings from MAC side.
> > > > > 
> > > > > This is certainly the wrong place for it.  Please hold off on this patch
> > > > > for the time being.  Thanks.
> > > > 
> > > > This is actually the change that makes everything work ...
> > > > 
> > > > I need to configure PCS before Aneg is complete and then I need to 
> > > > configure MAC once Aneg is done and link is up with the outcome speed and 
> > > > flow control.
> > > 
> > > Yes, I realise that, but it comes with the expense of potentially
> > > breaking mvneta and mvpp2, where the settings are automatically
> > > passed between the PCS and MAC in hardware. I also believe DSA
> > > works around this, and I need to look at that.
> > 
> > OK so there is one alternative solution for this that's just saving the 
> > last link status on stmmac internal structure (if applicable ofc, 
> > something like an_complete is true and link is true) and then just use 
> > that info in mac_link_up() callback to configure the MAC when PCS is in 
> > use.
> 
> I'm not disagreeing that something needs to be done - the assumption
> in phylink that the MAC and PCS talk to each other is one that comes
> from the hardware which it was developed on, but is not true for all
> hardware.  The IEEE 802.3 model doesn't include that behaviour.
> 
> So please, don't try to come up with an alternative solution; this
> problem _does_ need solving in phylink, but it needs to be done in a
> way that doesn't regress the existing users.
> 
> I've already started to split the current set of MAC operations into
> a purely MAC set of operations and a set of PCS operations, but still,
> the problem of how to sensibly deal with mvneta and mvpp2 remain.
> 
> The problem is that both these use two registers to control both the
> PCS and MAC.  One is a control register, which controls what is
> advertised, whether AN is used, what is negotiated and what is forced,
> including whether the link is forced up.
> 
> The other is a status register that gives the status of the MAC -
> whether tx pause and/or rx pause is enabled, what speed and duplex the
> MAC is running at, whether the link is in sync, whether the link is up
> etc.
> 
> Essentially, the PCS and MAC are tightly integrated together in this
> hardware, so splitting this into separate PCS and MAC control blocks is
> very problematical.
> 
> As I say, this is something that needs solving.  A solution needs to be
> found, rather than having lots of drivers working around this issue in
> their own special ways, and my fear is that the more workarounds we
> have, the more the phylink core will become unmaintainable.
> 
> So please, no workarounds.

I'm not trying to rush and I do agree with you. I just thought this was a 
particular case but I did tried to fix it in this commit.

I'm not familiar with mvneta HW but thanks for the description. Indeed, 
for XPCS and stmmac they are independent but MAC still needs to know the 
speed at least. My main problem here is USXGMII. This needs to know the 
speed negotiated taking into account MAC limitations and then use it to 
configure XPCS.

So, another possible solution is just to pass phylink_link_state struct to 
mac_link_up() and mac_link_down(). From the comment on the function and 
it's usage in phylink it seems this could be a good fit ?

Another thing I could solve here (while I'm at it), is PCS that sit in a 
MII bus: Instead of having stmmac saving all that private data of XPCS I 
could just pass it to phylink and save it there ? Something like you 
proposed in ("net: axienet: Fix SGMII support") ?

---
Thanks,
Jose Miguel Abreu