All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Sean Anderson <sean.anderson@seco.com>
Cc: Jose Abreu <Jose.Abreu@synopsys.com>,
	Andrew Lunn <andrew@lunn.ch>,
	linux-kernel@vger.kernel.org,
	"David S . Miller" <davem@davemloft.net>,
	UNGLinuxDriver@microchip.com, netdev@vger.kernel.org,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Jakub Kicinski <kuba@kernel.org>, Marcin Wojtas <mw@semihalf.com>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	linux-arm-kernel@lists.infradead.org,
	Heiner Kallweit <hkallweit1@gmail.com>
Subject: Re: [PATCH] net: phylink: Pass state to pcs_config
Date: Wed, 15 Dec 2021 00:49:14 +0000	[thread overview]
Message-ID: <Ybk7iuxdin69MjTo@shell.armlinux.org.uk> (raw)
In-Reply-To: <de1f7214-58c8-cdc6-1d29-08c979ce68f1@seco.com>

On Tue, Dec 14, 2021 at 07:16:53PM -0500, Sean Anderson wrote:
> Ok, so let me clarify my understanding. Perhaps this can be eliminated
> through a different approach.
> 
> When I read the datasheet for mvneta (which hopefully has the same
> logic here, since I could not find a datasheet for an mvpp2 device), I
> noticed that the Pause_Adv bit said
> 
> > It is valid only if flow control mode is defined by Auto-Negotiation
> > (as defined by the <AnFcEn> bit).
> 
> Which I interpreted to mean that if AnFcEn was clear, then no flow
> control was advertised. But perhaps it instead means that the logic is
> something like
> 
> if (AnFcEn)
> 	Config_Reg.PAUSE = Pause_Adv;
> else
> 	Config_Reg.PAUSE = SetFcEn;
> 
> which would mean that we can just clear AnFcEn in link_up if the
> autonegotiated pause settings are different from the configured pause
> settings.

Having actually played with this hardware quite a bit and observed what
it sends, what it implements for advertising is:

	Config_Reg.PAUSE = Pause_Adv;

Config_Reg gets sent over the 1000BASE-X link to the link partner, and
we receive Remote_Reg from the link partner.

Then, the hardware implements:

	if (AnFcEn)
		MAC_PAUSE = Config_Reg.PAUSE & Remote_Reg.PAUSE;
	else
		MAC_PAUSE = SetFcEn;

In otherwords, AnFcEn controls whether the result of autonegotiation
or the value of SetFcEn controls whether the MAC enables symmetric
pause mode.

Pause_Adv comes from the advertisement, and this is controlled from
ethtool -s and somewhat by ethtool -A.

AnFcEn is controlled purely and only by ethtool -A ... autoneg on|off.
You can't derive this from "state".

SetFcEn comes from ethtool -A ... tx and rx parameters (which must
both be on or both be off.)

Since we have no knowledge what Remote_Reg contains (it's not made
accessible by the hardware), it's impossible to derive whether the
autonegotiated pause settings are different from the configured pause
settings.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

WARNING: multiple messages have this Message-ID (diff)
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Sean Anderson <sean.anderson@seco.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Marcin Wojtas <mw@semihalf.com>,
	UNGLinuxDriver@microchip.com,
	"David S . Miller" <davem@davemloft.net>,
	Claudiu Beznea <claudiu.beznea@microchip.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Jose Abreu <Jose.Abreu@synopsys.com>,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Subject: Re: [PATCH] net: phylink: Pass state to pcs_config
Date: Wed, 15 Dec 2021 00:49:14 +0000	[thread overview]
Message-ID: <Ybk7iuxdin69MjTo@shell.armlinux.org.uk> (raw)
In-Reply-To: <de1f7214-58c8-cdc6-1d29-08c979ce68f1@seco.com>

On Tue, Dec 14, 2021 at 07:16:53PM -0500, Sean Anderson wrote:
> Ok, so let me clarify my understanding. Perhaps this can be eliminated
> through a different approach.
> 
> When I read the datasheet for mvneta (which hopefully has the same
> logic here, since I could not find a datasheet for an mvpp2 device), I
> noticed that the Pause_Adv bit said
> 
> > It is valid only if flow control mode is defined by Auto-Negotiation
> > (as defined by the <AnFcEn> bit).
> 
> Which I interpreted to mean that if AnFcEn was clear, then no flow
> control was advertised. But perhaps it instead means that the logic is
> something like
> 
> if (AnFcEn)
> 	Config_Reg.PAUSE = Pause_Adv;
> else
> 	Config_Reg.PAUSE = SetFcEn;
> 
> which would mean that we can just clear AnFcEn in link_up if the
> autonegotiated pause settings are different from the configured pause
> settings.

Having actually played with this hardware quite a bit and observed what
it sends, what it implements for advertising is:

	Config_Reg.PAUSE = Pause_Adv;

Config_Reg gets sent over the 1000BASE-X link to the link partner, and
we receive Remote_Reg from the link partner.

Then, the hardware implements:

	if (AnFcEn)
		MAC_PAUSE = Config_Reg.PAUSE & Remote_Reg.PAUSE;
	else
		MAC_PAUSE = SetFcEn;

In otherwords, AnFcEn controls whether the result of autonegotiation
or the value of SetFcEn controls whether the MAC enables symmetric
pause mode.

Pause_Adv comes from the advertisement, and this is controlled from
ethtool -s and somewhat by ethtool -A.

AnFcEn is controlled purely and only by ethtool -A ... autoneg on|off.
You can't derive this from "state".

SetFcEn comes from ethtool -A ... tx and rx parameters (which must
both be on or both be off.)

Since we have no knowledge what Remote_Reg contains (it's not made
accessible by the hardware), it's impossible to derive whether the
autonegotiated pause settings are different from the configured pause
settings.

-- 
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:[~2021-12-15  0:51 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-14 23:34 [PATCH] net: phylink: Pass state to pcs_config Sean Anderson
2021-12-14 23:34 ` Sean Anderson
2021-12-14 23:45 ` Russell King (Oracle)
2021-12-14 23:45   ` Russell King (Oracle)
2021-12-15  0:16   ` Sean Anderson
2021-12-15  0:16     ` Sean Anderson
2021-12-15  0:49     ` Russell King (Oracle) [this message]
2021-12-15  0:49       ` Russell King (Oracle)
2021-12-15  1:12       ` Russell King (Oracle)
2021-12-15  1:12         ` Russell King (Oracle)
2021-12-16 17:02         ` Sean Anderson
2021-12-16 17:02           ` Sean Anderson
2021-12-16 17:26           ` Russell King (Oracle)
2021-12-16 17:26             ` Russell King (Oracle)
2021-12-16 17:51             ` Sean Anderson
2021-12-16 17:51               ` Sean Anderson
2021-12-16 18:05               ` Russell King (Oracle)
2021-12-16 18:05                 ` Russell King (Oracle)
2021-12-16 18:29                 ` Sean Anderson
2021-12-16 18:29                   ` Sean Anderson
2021-12-16 18:58                   ` Russell King (Oracle)
2021-12-16 18:58                     ` Russell King (Oracle)
2021-12-16 19:00                   ` Sean Anderson
2021-12-16 19:00                     ` Sean Anderson

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=Ybk7iuxdin69MjTo@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=Jose.Abreu@synopsys.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.beznea@microchip.com \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=horatiu.vultur@microchip.com \
    --cc=ioana.ciornei@nxp.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mw@semihalf.com \
    --cc=netdev@vger.kernel.org \
    --cc=sean.anderson@seco.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.