All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, andrew@lunn.ch,
	f.fainelli@gmail.com, vivien.didelot@gmail.com,
	claudiu.manoil@nxp.com, alexandru.marginean@nxp.com,
	ioana.ciornei@nxp.com
Subject: Re: [PATCH net-next 5/7] net: dsa: felix: delete .phylink_mac_an_restart code
Date: Thu, 25 Jun 2020 17:53:49 +0100	[thread overview]
Message-ID: <20200625165349.GI1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <20200625152331.3784018-6-olteanv@gmail.com>

On Thu, Jun 25, 2020 at 06:23:29PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> In hardware, the AN_RESTART field for these SerDes protocols (SGMII,
> USXGMII) clears the resolved configuration from the PCS's
> auto-negotiation state machine.
> 
> But PHYLINK has a different interpretation of "AN restart". It assumes
> that this Linux system is capable of re-triggering an auto-negotiation
> sequence, something which is only possible with 1000Base-X and
> 2500Base-X, where the auto-negotiation is symmetrical. In SGMII and
> USXGMII, there's an AN master and an AN slave, and it isn't so much of
> "auto-negotiation" as it is "PHY passing the resolved link state on to
> the MAC".

This is not "a different interpretation".

The LX2160A documentation for this PHY says:

  9             Restart Auto Negotiation
 Restart_Auto_N Self-clearing Read / Write command bit, set to '1' to
                restart an auto negotiation sequence. Set to '0'
		(Reset value) in normal operation mode. Note: Controls
		the Clause 37 1000Base-X Auto-negotiation.

It doesn't say anything about clearing anything for SGMII.

Also, the Cisco SGMII specification does not indicate that it is
possible to restart the "autonegotiation" - the PHY is the controlling
end of the SGMII link.  There is no clause in the SGMII specification
that indicates that changing the MAC's tx_config word to the PHY will
have any effect on the PHY once the data path has been established.

Finally, when a restart of negotiation is requested, and we have a PHY
attached in SGMII mode, we will talk to that PHY to cause a restart of
negotiation on the media side, which will implicitly cause the link
to drop and re-establish, causing the SGMII side to indicate link down
and subsequently re-establish according to the media side results.

So, please, lay off your phylink bashing in your commit messages.

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

  reply	other threads:[~2020-06-25 16:54 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-25 15:23 [PATCH net-next 0/7] PHYLINK integration improvements for Felix DSA driver Vladimir Oltean
2020-06-25 15:23 ` [PATCH net-next 1/7] net: dsa: felix: stop writing to read-only fields in MII_BMCR Vladimir Oltean
2020-06-25 16:28   ` Russell King - ARM Linux admin
2020-06-25 15:23 ` [PATCH net-next 2/7] net: dsa: felix: support half-duplex link modes Vladimir Oltean
2020-06-25 16:30   ` Russell King - ARM Linux admin
2020-06-25 15:23 ` [PATCH net-next 3/7] net: dsa: felix: unconditionally configure MAC speed to 1000Mbps Vladimir Oltean
2020-06-25 15:23 ` [PATCH net-next 4/7] net: dsa: felix: set proper pause frame timers based on link speed Vladimir Oltean
2020-06-25 16:37   ` Russell King - ARM Linux admin
2020-06-25 16:48     ` Vladimir Oltean
2020-06-25 16:58       ` Russell King - ARM Linux admin
2020-06-25 17:06         ` Vladimir Oltean
2020-06-25 17:53           ` Russell King - ARM Linux admin
2020-06-25 15:23 ` [PATCH net-next 5/7] net: dsa: felix: delete .phylink_mac_an_restart code Vladimir Oltean
2020-06-25 16:53   ` Russell King - ARM Linux admin [this message]
2020-06-26  8:53     ` Vladimir Oltean
2020-06-26 11:08       ` Russell King - ARM Linux admin
2020-06-26 11:19         ` Vladimir Oltean
2020-06-26 11:32           ` Russell King - ARM Linux admin
2020-06-25 15:23 ` [PATCH net-next 6/7] net: dsa: felix: disable in-band AN in MII_BMCR without reset Vladimir Oltean
2020-06-25 15:23 ` [PATCH net-next 7/7] net: dsa: felix: use resolved link config in mac_link_up() Vladimir Oltean

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200625165349.GI1551@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=alexandru.marginean@nxp.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vivien.didelot@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.