netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Oleksij Rempel <o.rempel@pengutronix.de>,
	Andrew Lunn <andrew@lunn.ch>,
	Woojung Huh <woojung.huh@microchip.com>,
	Arun Ramadoss <arun.ramadoss@microchip.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	netdev@vger.kernel.org, UNGLinuxDriver@microchip.com,
	Eric Dumazet <edumazet@google.com>,
	kernel@pengutronix.de, Jakub Kicinski <kuba@kernel.org>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S. Miller" <davem@davemloft.net>
Subject: Re: FWD: Re: [PATCH net-next v1 1/1] net: dsa: microchip: ksz8: Make flow control, speed, and duplex on CPU port configurable
Date: Tue, 11 Apr 2023 12:50:28 +0100	[thread overview]
Message-ID: <ZDVJhN4vyK9ldurD@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230411111609.jhfcvvxbxbkl47ju@skbuf>

On Tue, Apr 11, 2023 at 02:16:09PM +0300, Vladimir Oltean wrote:
> On Tue, Apr 11, 2023 at 10:17:47AM +0100, Russell King (Oracle) wrote:
> > Since we can't manually control the tx and rx pause enables, I think
> > the only sensible way forward with this would be to either globally
> > disable pause on the device, and not report support for any pause
> > modes,
> 
> This implies restarting autoneg on all the other switch ports when one
> port's flow control mode is changed?

From my reading of these global register descriptions, no it doesn't,
and even if we did restart aneg, it would have no overall system effect
because the advertisements for each port haven't been changed. It's
mad hardware.

What I was meaning above is that we configure the entire switch to
either do autonegotiated flow control at setup time, or we configure
the switch to never do flow control.

> > or report support for all pause modes, advertise '11' and
> > let the hardware control it (which means the ethtool configuration
> > for pause would not be functional.)
> > 
> > This needs to be commented in the driver so that in the future we
> > remember why this has been done.
> > 
> > Maybe Andrew and/or Vladimir also have an opinion to share about the
> > best approach here?
> 
> I don't object to documenting that manually forcing flow control off is
> broken and leaving it at that (and the way to force it off would be to
> not advertise any of the 2 bits).
> 
> But why advertise only 11 (Asym_Pause | Pause) when the PHYs integrated
> here have the advertisement configurable (presumably also through the
> micrel.c PHY driver)? They would advertise in accordance with ethtool, no?
> 
> I may have missed something.

I think you have. I'm only talking about the ability to control flow
control manually via ethtool -A. Changing it via the advertisement
(ethtool -s) would still work.

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

  parent reply	other threads:[~2023-04-11 11:50 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <7055f8c2-3dba-49cd-b639-b4b507bc1249@lunn.ch>
2023-04-07 17:44 ` FWD: Re: [PATCH net-next v1 1/1] net: dsa: microchip: ksz8: Make flow control, speed, and duplex on CPU port configurable Russell King (Oracle)
2023-04-11  8:56   ` Oleksij Rempel
2023-04-11  9:17     ` Russell King (Oracle)
2023-04-11  9:55       ` Oleksij Rempel
2023-04-11 11:16       ` Vladimir Oltean
2023-04-11 11:35         ` Vladimir Oltean
2023-04-11 12:00           ` Russell King (Oracle)
2023-04-11 12:16             ` Andrew Lunn
2023-04-11 13:26             ` Vladimir Oltean
2023-04-11 13:33               ` Russell King (Oracle)
2023-04-11 14:28                 ` Vladimir Oltean
2023-04-11 11:50         ` Russell King (Oracle) [this message]
2023-04-11 13:12           ` Vladimir Oltean
2023-04-11 13:26             ` Russell King (Oracle)
2023-04-11 14:33   ` 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=ZDVJhN4vyK9ldurD@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=arun.ramadoss@microchip.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=woojung.huh@microchip.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).