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 bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id C23C4C433EF for ; Fri, 22 Jul 2022 13:25:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NMeYHNL5d5CcjZTBn7J7fImMgDYIdA6X5wrXWMd9d4Y=; b=YhPaKt/OfNSwX5 fnyghOLh8Fza7e4EtKP+vxt87/JCYKgldvNq+yQ53Wp0UOct4id1Non8INml9um9aTbYFZhO0WTyT kJ7BK1CfnDWAv8cx3AaPOp5A5SCJvypzVaXHALFO0Nsd3FQRUGC2X+MNNequVy5DI/dxL3uyQhI/X d4pfggjSVu+Cmf/f69HKdavO0rZXPF9bipR5tGCJdwNpJlQWV/ktAZ52I5jR3ChUZmoQYWdyS8w4R 8wt4bqFpahrkZZFmq9RPHS6kW0pjtvViCEnPQpAKtnfRABx6RKX6ssc/CvlHD4kZ9p5TrZecdLKKD mhd430XxmiZpYbFy2yng==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEsdW-004yVu-4Q; Fri, 22 Jul 2022 13:24:10 +0000 Received: from desiato.infradead.org ([2001:8b0:10b:1:d65d:64ff:fe57:4e05]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEsdR-004ySI-Pu; Fri, 22 Jul 2022 13:24:05 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=VrOXmyUnuTc5xwZTdtW/lrqcLh//iHh97nkUOkQ0u/Q=; b=JEgEMO9u+nhruMUWcoLBmlDL7s +HqZKSh1zKZ5bI0mIqRAhKvnSMMiKq0XPsOj5tYRpL9+8c2DiPBUsL7ogZi0f4XlMSnJ1jxauZIlg qLj8eNN+DH2E4xZ5PeX6re1BkazTd7uRrpt+2BWwde1Nq4clKfYLRNaeLNg9hDM6psnLtuCaXrw5G 9RgBAw7rAHjZbVPCMz7uRjms6rpbAwqkHWBRpcBGQun4zCuMIYSfo6kUuflMXinAUBAiv5zI+F+nk KpfXjeWU3yeU8sOYilOchxbLsu+fbHL4R3wmw47ofIGAOCrFWwSdTKpwNWLRY6wLa80+sdTIfDwfy km6mEHHQ==; Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by desiato.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1oEsdM-005rj3-3N; Fri, 22 Jul 2022 13:24:02 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2019; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=VrOXmyUnuTc5xwZTdtW/lrqcLh//iHh97nkUOkQ0u/Q=; b=1eQynhfG1ijDSwjXUn5TcS3pPq KFTspin9KhbMAylxXiqNeqKbXSCm/MY0XJmCwtzvpKdIndKd+DLgO2nALy80O+VXpHyo/VmsdOp3Q qk40kViUq4NVWvlovRbqAqYneLCHKBzbdnHQBvDVvGo+X+g5Cbkk6fD5QjJG6Z6B8I+c1mXdIMl1t TQhTkUbU7798s95iuFibC18QFuAb0b4cF0jw8YIib+Es69BMVnvKCbRDuYK//uidWnKqglsx/KaLR u/lwJRulW7bOOe2qj4vG5nDspR/odDJCPd4hL5H3+7Vm8DkyUUyEvluVASob6bOhdszRP5f+KMnnx WZ1N+LFA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:33512) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1oEsW2-0006y5-Au; Fri, 22 Jul 2022 14:16:26 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1oEsVg-0005qk-Kv; Fri, 22 Jul 2022 14:16:04 +0100 Date: Fri, 22 Jul 2022 14:16:04 +0100 From: "Russell King (Oracle)" To: Vladimir Oltean Cc: Marek =?iso-8859-1?Q?Beh=FAn?= , Andrew Lunn , Heiner Kallweit , Alexandre Belloni , Alvin __ipraga , Andy Shevchenko , Claudiu Manoil , Daniel Scally , "David S. Miller" , DENG Qingfang , Eric Dumazet , Florian Fainelli , George McCollister , Greg Kroah-Hartman , Hauke Mehrtens , Heikki Krogerus , Jakub Kicinski , Kurt Kanzenbach , Landen Chao , Linus Walleij , linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org, Matthias Brugger , netdev@vger.kernel.org, Paolo Abeni , "Rafael J. Wysocki" , Sakari Ailus , Sean Wang , UNGLinuxDriver@microchip.com, Vivien Didelot , Woojung Huh Subject: Re: [PATCH net-next 3/6] net: dsa: add support for retrieving the interface mode Message-ID: References: <20220721151533.3zomvnfogshk5ze3@skbuf> <20220721192145.1f327b2a@dellmb> <20220721192145.1f327b2a@dellmb> <20220721182216.z4vdaj4zfb6w3emo@skbuf> <20220721213645.57ne2jf7f6try4ec@skbuf> <20220722105238.qhfq5myqa4ixkvy4@skbuf> <20220722124629.7y3p7nt6jmm5hecq@skbuf> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220722124629.7y3p7nt6jmm5hecq@skbuf> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220722_142400_647804_679F340D X-CRM114-Status: GOOD ( 58.21 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jul 22, 2022 at 03:46:29PM +0300, Vladimir Oltean wrote: > On Fri, Jul 22, 2022 at 12:44:17PM +0100, Russell King (Oracle) wrote: > > Today, there is no guarantee - because it depends on how people have > > chosen to implement 2500base-X, and whether the hardware requires the > > use of in-band AN or prohibits it. This is what happens when stuff > > isn't standardised - one ends up with differing implementations doing > > different things, and this has happened not _only_ at hardware level > > but also software level as well. > > > > You have to also throw into this that various implementations also have > > an "AN bypass" flag, which means if they see what looks like a valid > > SERDES data stream, but do not see the AN data, after a certain timeout > > they allow the link to come up - and again, whether that is enabled or > > not is pot luck today. > > Interesting. After the timeout expires, does the lane ever transition > back into the encoding required for AN mode, in case there appears at a > later time someone willing to negotiate? They don't document that it does. > > > similarly, there is a good chance that the DT description below might > > > result in a functional link: > > > > > > ð0 { > > > phy-mode = "2500base-x"; > > > managed = "in-band-status"; > > > }; > > > > > > &switch_cpu_port { > > > ethernet = <ð0>; > > > phy-mode = "25000base-x"; > > > > > > fixed-link { > > > speed = <2500>; > > > full-duplex; > > > }; > > > }; > > > > > > There is no expectation from either DT description to use in-band > > > autoneg or not. > > > > > > The fact that of_phy_is_fixed_link() was made by Stas Sergeev to say > > > that a 'managed' link with the value != 'auto' is fixed prompted me to > > > study exactly what those changes were about. > > > > From what I can see, there is no formal definition of "in-band-status" > > beyond what it says on the tin. The description in the DT binding > > specification, which is really where this should be formally documented, > > is totally lacking. > > > > > This patch introduces the new string property 'managed' that allows > > > the user to set the management type explicitly. > > > The supported values are: > > > "auto" - default. Uses either MDIO or nothing, depending on the presence > > > of the fixed-link node > > > "in-band-status" - use in-band status > > > > This, and how this is implemented by mvneta, is the best we have to go > > on for the meaning of this. > > > > > This is why I am asking whether there is any formal definition of what > > > managed = "in-band-status" means. You've said it means about retrieving > > > link status from the PCS. What are you basing upon when you are saying that? > > > > Given that this managed property was introduced for mvneta, mvneta's > > implementation of it is the best reference we have to work out what > > the intentions of it were beyond the commit text. > > > > With in-band mode enabled, mvneta makes use of a fixed-link PHY, and > > updates the fixed-link PHY with the status from its GMAC block (which > > is the combined PCS+MAC). > > > > So, when in-band mode is specified, the results from SGMII or 1000base-X > > negotiation are read from the MAC side of the link, pushed into the > > fixed-PHY, which then are reflected back into the driver via the usual > > phylib adjust_link(). > > > > Have a read through mvneta's code at this commit: > > > > git show 2eecb2e04abb62ef8ea7b43e1a46bdb5b99d1bf8:drivers/net/ethernet/marvell/mvneta.c > > > > specifically, mvneta_fixed_link_update() and mvneta_adjust_link(). > > Note that when operating in in-band mode, there is actually no need > > for the configuration of MVNETA_GMAC_AUTONEG_CONFIG to be touched > > in any way since the values read from the MVNETA_GMAC_STATUS register > > indicate what parameters the MAC is actually using. (The speed, > > duplex, and pause bits in AUTONEG_CONFIG are ignored anyway if AN > > is enabled.) > > I view this as just an implementation detail and not as something that > influences what managed = "in-band-status" is supposed to mean. > > > I know this is rather wooly, but not everything is defined in black and > > white, and we need to do the best we can with the information that is > > available. > > So mvneta at the stage of the commit you've mentioned calls > mvneta_set_autoneg() with the value of pp->use_inband_status. There is > then the exception to be made for the PCS being what's exposed to the > medium, and in that case, ethtool may also override the pp->use_inband_status > variable (which in turn affects the autoneg). > > So if we take mvneta at this commit as the reference, what we learn is > that using in-band status essentially depends on using in-band autoneg > in the first place. > > What is hard for me to comprehend is how we ever came to conclude that > for SERDES protocols where clause 37 is possible (2500base-x should be > part of this group), managed = "in-band-status" does not imply in-band > autoneg, considering the mvneta precedent. That is a recent addition, since the argument was made that when using a 1000base-X fibre transceiver, using ethtool to disable autoneg is a reasonable thing to do - and something that was supported with mvneta_ethtool_set_link_ksettings() as it stands at the point in the commit above. > And why would we essentially redefine its meaning by stating that no, > it is only about the status, not about the autoneg, even though the > status comes from the autoneg for these protocols. I'm not sure I understand what you're getting at there. Going back to the mvneta combined PCS+MAC implementation, we read the link parameters from the PCS when operating in in-band mode and throw them at the fixed PHY so that ethtool works, along with all the usual link up/down state reporting, carrier etc. If autoneg is disabled, then we effectively operate in fixed-link mode (use_inband_status becomes false, and we start forcing the link up/down and also force the speed and duplex parameters by disabling autoneg.) Note that this version of mvneta does not support 1000base-X mode, only SGMII is actually supported. There's a few things that are rather confusing in the driver: MVNETA_GMAC_INBAND_AN_ENABLE - this controls whether in-band negotiation is performed or not. MVNETA_GMAC_AN_SPEED_EN - this controls whether the result of in-band negotiation for speed is used, or the manually programmed speed in this register. MVNETA_GMAC_AN_DUPLEX_EN - same for duplex. MVNETA_GMAC_AN_FLOW_CTRL_EN - same for pause (only symmetric pause is supported) MVNETA_GMAC2_INBAND_AN_ENABLE - misnamed, it selects whether SGMII (set) or 1000base-X (unset) format for the 16-bit control word is used. There is another bit in MVNETA_GMAC_CTRL_0 that selects between 1000base-X and SGMII operation mode, and when this bit is set for 1000base-X. This version of the driver doesn't support 1000base-X, so this bit is never set. -- 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