[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230517133549.GA8586@pengutronix.de>
Date: Wed, 17 May 2023 15:35:49 +0200
From: Oleksij Rempel <o.rempel@...gutronix.de>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: "David S. Miller" <davem@...emloft.net>,
Andrew Lunn <andrew@...n.ch>,
Eric Dumazet <edumazet@...gle.com>,
Florian Fainelli <f.fainelli@...il.com>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Vladimir Oltean <olteanv@...il.com>,
Woojung Huh <woojung.huh@...rochip.com>,
Arun Ramadoss <arun.ramadoss@...rochip.com>,
kernel@...gutronix.de, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org, UNGLinuxDriver@...rochip.com
Subject: Re: [PATCH net-next v2 1/2] net: dsa: microchip: ksz8: Make flow
control, speed, and duplex on CPU port configurable
On Wed, May 17, 2023 at 01:45:46PM +0100, Russell King (Oracle) wrote:
> On Wed, May 17, 2023 at 02:10:33PM +0200, Oleksij Rempel wrote:
> > +/**
> > + * ksz8_upstram_link_up - Configures the CPU/upstream port of the switch.
> > + * @dev: The KSZ device instance.
> > + * @port: The port number to configure.
> > + * @speed: The desired link speed.
> > + * @duplex: The desired duplex mode.
> > + * @tx_pause: If true, enables transmit pause.
> > + * @rx_pause: If true, enables receive pause.
> > + *
> > + * Description:
> > + * The function configures flow control and speed settings for the CPU/upstream
> > + * port of the switch based on the desired settings, current duplex mode, and
> > + * speed.
> > + */
> > +static void ksz8_upstram_link_up(struct ksz_device *dev, int port, int speed,
> > + int duplex, bool tx_pause, bool rx_pause)
> > +{
> > + u8 ctrl = 0;
> > +
> > + if (duplex) {
> > + if (tx_pause || rx_pause)
> > + ctrl |= SW_FLOW_CTRL;
> > + } else {
> > + ctrl |= SW_HALF_DUPLEX;
> > + if (tx_pause || rx_pause)
> > + ctrl |= SW_HALF_DUPLEX_FLOW_CTRL;
>
> It's come up before whether the pause settings should be used to control
> half-duplex flow control, and I believe the decision was they shouldn't.
Got it, back pressure and pause for flow control are two different
things. I'll remove the half-duplex back pressure control using pause
settings from the patch.
> The other thing I find slightly weird is that this is only being done
> for upstream ports - why would a port that's between switches or the
> switch and the CPU be in half duplex mode?
As for the CPU port half-duplex mode, it's currently configurable via
the device tree. I don't have a specific use case for it, but it's there
if needed. If it's causing confusion though, I'm open to removing it.
What do you think?
> Also, why would such a port want to use some kind of flow control? If
> the CPU starts sending pause frames because its got stuck, doesn't
> that eventually kill the entire network? Also doesn't it limit the
> network bandwidth to the ability of the host CPU *not* to send
> pause frames?
Before this patch, flow control on the CPU port was indeed hard-coded.
This patch lets us disable it if we want to, giving us a bit more
flexibility.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
Powered by blists - more mailing lists