Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 17 May 2023 13:45:46 +0100
From:   "Russell King (Oracle)" <linux@armlinux.org.uk>
To:     Oleksij Rempel <o.rempel@pengutronix.de>
Cc:     "David S. Miller" <davem@davemloft.net>,
        Andrew Lunn <andrew@lunn.ch>,
        Eric Dumazet <edumazet@google.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        Woojung Huh <woojung.huh@microchip.com>,
        Arun Ramadoss <arun.ramadoss@microchip.com>,
        kernel@pengutronix.de, linux-kernel@vger.kernel.org,
        netdev@vger.kernel.org, UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next v2 1/2] net: dsa: microchip: ksz8: Make flow
 control, speed, and duplex on CPU port configurable
Message-ID: <ZGTMepjv33bJbck/@shell.armlinux.org.uk>
References: <20230517121034.3801640-1-o.rempel@pengutronix.de>
 <20230517121034.3801640-2-o.rempel@pengutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230517121034.3801640-2-o.rempel@pengutronix.de>
Sender: Russell King (Oracle) <linux@armlinux.org.uk>
Precedence: bulk

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.

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?

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?

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