On Mon, Aug 03, 2020 at 02:47:41PM +0000, Madalin Bucur (OSS) wrote:

quoted

-----Original Message-----
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Sent: 03 August 2020 17:10
To: Madalin Bucur (OSS) <redacted>
Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh
[off-list ref]; f.fainelli@gmail.com; hkallweit1@gmail.com;
netdev@vger.kernel.org; Calvin Johnson (OSS) [off-list ref];
kuldip dwivedi [off-list ref]; Vikas Singh
[off-list ref]
Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed PHY

On Mon, Aug 03, 2020 at 11:45:55AM +0000, Madalin Bucur (OSS) wrote:

quoted

quoted

-----Original Message-----
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Sent: 03 August 2020 12:07
To: Madalin Bucur (OSS) <redacted>
Cc: Andrew Lunn <andrew@lunn.ch>; Vikas Singh
[off-list ref]; f.fainelli@gmail.com;

hkallweit1@gmail.com;

quoted

quoted

netdev@vger.kernel.org; Calvin Johnson (OSS)

[off-list ref];

quoted

quoted

kuldip dwivedi [off-list ref]; Vikas Singh
[off-list ref]
Subject: Re: [PATCH 2/2] net: phy: Associate device node with fixed

PHY

quoted

quoted

On Mon, Aug 03, 2020 at 08:33:19AM +0000, Madalin Bucur (OSS) wrote:

quoted

quoted

-----Original Message-----
From: netdev-owner@vger.kernel.org <redacted>

On

quoted

quoted

quoted

quoted

Behalf Of Andrew Lunn
Sent: 01 August 2020 18:11
To: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Cc: Vikas Singh <redacted>;

f.fainelli@gmail.com;

quoted

quoted

quoted

quoted

hkallweit1@gmail.com; netdev@vger.kernel.org; Calvin Johnson (OSS)
[off-list ref]; kuldip dwivedi
[off-list ref]; Madalin Bucur (OSS)
[off-list ref]; Vikas Singh [off-list ref]
Subject: Re: [PATCH 2/2] net: phy: Associate device node with

fixed

quoted

quoted

PHY

quoted

quoted

On Sat, Aug 01, 2020 at 10:41:32AM +0100, Russell King - ARM Linux

admin

quoted

quoted

wrote:

quoted

On Sat, Aug 01, 2020 at 09:52:52AM +0530, Vikas Singh wrote:

quoted

Hi Andrew,

Please refer to the "fman" node under
linux/arch/arm64/boot/dts/freescale/fsl-ls1046a-rdb.dts
I have two 10G ethernet interfaces out of which one is of

fixed-

quoted

quoted

link.

quoted

quoted

quoted

Please do not top post.

How does XGMII (which is a 10G only interface) work at 1G speed?

Is

quoted

quoted

quoted

quoted

quoted

what is in DT itself a hack because fixed-phy doesn't support

10G

quoted

quoted

quoted

quoted

quoted

modes?

My gut feeling is there is some hack going on here, which is why

i'm

quoted

quoted

quoted

quoted

being persistent at trying to understand what is actually going on
here.

Hi Andrew,

That platform used 1G fixed link there since there was no support

for

quoted

quoted

quoted

10G fixed link at the time. PHYlib could have tolerated 10G speed

there

quoted

quoted

quoted

With a one-liner.

That statement is false.  It is not a "one liner".  fixed-phy exposes
the settings to userspace as a Clause 22 PHY register set, and the
Clause 22 register set does not support 10G.  So, a "one liner" would
just be yet another hack.  Adding Clause 45 PHY emulation support
would be a huge task.

quoted

I understand that PHYLink is working to describe this
Better, but it was not there at that time. Adding the dependency on
PHYLink was not desirable as most of the users for the DPAA 1

platforms

quoted

quoted

quoted

were targeting kernels before the PHYLink introduction (and last

I've

quoted

quoted

quoted

looked, it's still under development, with unstable APIs so we'll
take a look at this later, when it settles).

I think you need to read Documentation/process/stable-api-nonsense.rst
particularly the section "Stable Kernel Source Interfaces".

phylink is going to be under development for quite some time to come
as requirements evolve.  For example, when support for QSFP interfaces
is eventually worked out, I suspect there will need to be some further
changes to the driver interface.  This is completely normal.

Now, as to the stability of the phylink API to drivers, it has in fact
been very stable - it has only changed over the course of this year to
support split PCS, a necessary step for DPAA2 and a few others.  It

has

quoted

quoted

been around in mainline for two years, and has been around much longer
than that, and during that time it has been in mainline, the MAC

facing

quoted

quoted

interface has not changed until recently.

So, I find your claim to be quite unreasonable.

I see you agree that there were and there will be many changes for a

while,

quoted

It's not a complaint, I know hot it works, it's just a decision based on
required effort vs features offered vs user requirements. Lately it's

been

quoted

time consuming to try to fix things in this area.

No, it hasn't been time consuming.  The only API changes as far as
drivers are concerned have been:

1. the change to the mac_link_up() prototype to move the setup of the
   final link parameters out of mac_config() - and almost all of the
   updates to users were done by me.

2. the addition of split PCS support, introducing new interfaces, has
   had minimal impact on those drivers that updated in step (1).

There have been no other changes as far as users are concerned.

Some of the difficulty with (1) has been that users of phylink appeared
initially with no proper review, and consequently they got quite a lot
wrong.  The most common error has been using state->speed, state->duplex
in mac_config() methods irrespective of the AN mode, which has _always_
since before phylink was merged into mainline, been totally unreliable.

That leads me on to the other visible "changes" for users are concerned,
which may be interpreted as interface changes, but are not; they have
all been clarifications to the documentation, to strengthen things such
as "do not use state->speed and state->duplex in mac_config() for
various specific AN modes".  Nothing has actually changed with any of
those clarifications.

For example, if in in-band mode, and mac_config() uses state->speed
and state->duplex, then it doesn't matter which version of phylink
you're using, if someone issues ethtool -s ethN ..., then state->speed
and state->duplex will be their respective UNKNOWN values, and if you're
using these in that situation, you will mis-program the MAC.

Again, that is not something that has changed.  Ever.  But the
documentation has because people just don't seem to get it, and I seemed
to be constantly repeating myself in review after review on the same
points.

So, your assertion that the phylink API is not stable is false.  It
has been remarkably stable over the two years that it has been around.
It is only natural that as the technology that a piece of code supports
evolves, so the code evolves with it.  That is exactly what has happened
this year with the two changes I mention above.

Now, if you've found it time consuming to "fix things" (unspecified what
"things" are) then I assert that what has needed to be fixed are things
that NXP have got wrong.  Such as the rtnl cockups.  Such as abusing
state->speed and state->duplex.  None of that is because the interface
is unstable - they are down to buggy implementation on NXPs part.

Essentially, what I'm saying is that your attempt to paint phylink as
being painful on the basis of interface changes is totally and utterly
wrong and is just an excuse to justify abusing the fixed-link code and
specifying things that are clearly incorrect via DT.

Thank you for the distilled phylink history, it may be easier to comprehend
with these details. I was not referring to phylink, but PHY related issues
on the DPAA 1 platforms.

Sigh.  No, you were referring to phylink.  This is what you said:

I understand that PHYLink is working to describe this
Better, but it was not there at that time. Adding the dependency on
PHYLink was not desirable as most of the users for the DPAA 1 platforms
were targeting kernels before the PHYLink introduction (and last I've
looked, it's still under development, with unstable APIs so we'll
take a look at this later, when it settles).

This discussion stems from your misconception and incorrect statements
concerning phylink, which I am correcting in this discussion.

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