From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2EBA8C77B75 for ; Wed, 17 May 2023 13:36:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231336AbjEQNg2 (ORCPT ); Wed, 17 May 2023 09:36:28 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46248 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231994AbjEQNgT (ORCPT ); Wed, 17 May 2023 09:36:19 -0400 Received: from metis.ext.pengutronix.de (metis.ext.pengutronix.de [IPv6:2001:67c:670:201:290:27ff:fe1d:cc33]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C655783F0 for ; Wed, 17 May 2023 06:36:05 -0700 (PDT) Received: from ptx.hi.pengutronix.de ([2001:67c:670:100:1d::c0]) by metis.ext.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pzHJm-0007qT-Sa; Wed, 17 May 2023 15:35:50 +0200 Received: from ore by ptx.hi.pengutronix.de with local (Exim 4.92) (envelope-from ) id 1pzHJl-0006VT-4O; Wed, 17 May 2023 15:35:49 +0200 Date: Wed, 17 May 2023 15:35:49 +0200 From: Oleksij Rempel To: "Russell King (Oracle)" Cc: "David S. Miller" , Andrew Lunn , Eric Dumazet , Florian Fainelli , Jakub Kicinski , Paolo Abeni , Vladimir Oltean , Woojung Huh , Arun Ramadoss , 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: <20230517133549.GA8586@pengutronix.de> References: <20230517121034.3801640-1-o.rempel@pengutronix.de> <20230517121034.3801640-2-o.rempel@pengutronix.de> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Sent-From: Pengutronix Hildesheim X-URL: http://www.pengutronix.de/ X-Accept-Language: de,en X-Accept-Content-Type: text/plain User-Agent: Mutt/1.10.1 (2018-07-13) X-SA-Exim-Connect-IP: 2001:67c:670:100:1d::c0 X-SA-Exim-Mail-From: ore@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 |