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 CB4F7CDE009 for ; Thu, 26 Sep 2024 13:53:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id: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-Owner; bh=7+zzLjRllKvgaQXPsb5e4jFYVKK2ubkPN9j82GqItdI=; b=t1gqZYMsbn6tKRtqP86Qj3m4yz NHfV0yyN3SjxiaY68173xaPno1Jw3y81nzP2+CZi+NJgoxesABXU+aqk+T+OrdBAccJkz8d39tWKH gCH8+xYpD+7taNFsJoMX0SP9a2WmGp5BI9kRMG6lBsifIxXk2qhd4RHtJQmNT0K3qS0J7CqAuXfxF rvIPeyIqZf8XWtE7wuxEKB4SVytqDtK/2NIE+aPoNKosgNWO6hTHhnKXuFHJpeclTE1WltPCpl1uW nWQpL3bLZzfRmp7D+t7vdbLNPRwi3dJjgoKx14+e4MQYEZIUHBqXEGlNluE54rPo6Mdvgc15we/G/ JJTL2z/Q==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98 #2 (Red Hat Linux)) id 1stow4-00000008VZe-2FxF; Thu, 26 Sep 2024 13:53:36 +0000 Received: from mail-ej1-x632.google.com ([2a00:1450:4864:20::632]) by bombadil.infradead.org with esmtps (Exim 4.98 #2 (Red Hat Linux)) id 1stosZ-00000008Uzg-1CgE for linux-arm-kernel@lists.infradead.org; Thu, 26 Sep 2024 13:50:00 +0000 Received: by mail-ej1-x632.google.com with SMTP id a640c23a62f3a-a8a765f980dso12349366b.1 for ; Thu, 26 Sep 2024 06:49:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1727358597; x=1727963397; darn=lists.infradead.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=7+zzLjRllKvgaQXPsb5e4jFYVKK2ubkPN9j82GqItdI=; b=UZCAmqO6ClQCwCGTh0OpFFW5F1FwwpuhZOnsEJuumRkXxMJtK85JKrB1Nv+oN82HDB YRpo7RJY28NgfLa78YFSwaR2oXouBN5Zc/oVTRMRMOFxklzPP15Xxit4Ol6RFXzr6tth sOivC60aiNhZlfVzExWLVMboGEZQN4KdCe6XXU1mZquzXrMaMQJNzM2gFcd7Ps9Df3dL 7bBt3mMQmjDz01eVttpPOTmureq7t7NHAQbdg1CV1pm8KIsbplhWh2oydBp1oNhZpUnm tRoLZ99MA1mZqf0VhN/f2/gdh/Tf47QkG6aFD4YbmVzmkmIszB4LBrhY4T5ExqbhDyS6 pdZg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727358597; x=1727963397; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=7+zzLjRllKvgaQXPsb5e4jFYVKK2ubkPN9j82GqItdI=; b=RqAYae93S/hIoXPnsJTrTO7c4P5V2ODimWEiK7pH99boLmwutu7pEWlg3aPLazuLSO K25JU/uyQVEh4TRKR6l/QsdII20UV8XF6yIIThhVBEIYpzVS0Xchlev2xH9i7IHJsOvg Ae2oduASu1w0qdtQ0AvtHArZrJFAeF6hEensE2bbqKGcJd3WHdEvSe0aX8iIvyiZPCAl hVEEdaS+qR6DyOAx1hEAw27fVBGtQpGAtJni2t2FCIX5x8cy6uTgPVESkNX/sD9u9WY7 sQx/nRssl4EZgZhNEdppJcJlYO1S+QheGmlg8I7F0mOjcNLwUeFMUx12ZwUdxdyAi9Ck Z77A== X-Forwarded-Encrypted: i=1; AJvYcCVIR9MtIkMTPONefGO4XYoYQsN3Zmk79zNug0p2GJ0AAQyByE721xOuXUi8pWzCRCxhEniC711EGCZ/6v/lhW5G@lists.infradead.org X-Gm-Message-State: AOJu0YxVXRYhvin6J/WDzI4jzKl0f27OPa3Wpur59MuvJt2lKSEC+YF7 OqMTo0RMCvtug8wAr1lJHDI2rs+QffrxiscV66lFuLzbEf5Ky7Vs X-Google-Smtp-Source: AGHT+IFpnlRuaodvuzoE0X+W5IOt9tpTDOpsyXF7kzVQoQXrBi+0dHlWnNst0pRaJ2gCgm9xWycbmA== X-Received: by 2002:a17:906:d550:b0:a8a:9054:8396 with SMTP id a640c23a62f3a-a93b267b48fmr114945166b.7.1727358596598; Thu, 26 Sep 2024 06:49:56 -0700 (PDT) Received: from skbuf ([188.25.134.29]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a93930f79c3sm359701666b.176.2024.09.26.06.49.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Sep 2024 06:49:55 -0700 (PDT) Date: Thu, 26 Sep 2024 16:49:52 +0300 From: Vladimir Oltean To: "Russell King (Oracle)" Cc: Andrew Lunn , Heiner Kallweit , Alexandre Torgue , "David S. Miller" , Eric Dumazet , Florian Fainelli , Jakub Kicinski , Jiawen Wu , Jose Abreu , Jose Abreu , linux-arm-kernel@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, Maxime Coquelin , Mengyuan Lou , netdev@vger.kernel.org, Paolo Abeni Subject: Re: [PATCH RFC 00/10] net: pcs: xpcs: cleanups batch 1 Message-ID: <20240926134952.atabwkzfal44r3lk@skbuf> References: <20240925134337.y7s72tdomvpcehsu@skbuf> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240926_064959_354286_0FCC04CC X-CRM114-Status: GOOD ( 45.36 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Sep 26, 2024 at 12:41:27PM +0100, Russell King (Oracle) wrote: > On Wed, Sep 25, 2024 at 04:43:37PM +0300, Vladimir Oltean wrote: > > Hi Russell, > > > > On Mon, Sep 23, 2024 at 03:00:26PM +0100, Russell King (Oracle) wrote: > > > First, sorry for the bland series subject - this is the first in a > > > number of cleanup series to the XPCS driver. > > > > I presume you intend to remove the rest of the exported xpcs functions > > as well, in further "batches". Could you share in advance some details > > about what you plan to do with xpcs_get_an_mode() as used in stmmac? > > I've been concentrating more on the sja1105 and wangxun users with this > cleanup, as changing stmmac is going to be quite painful - so I've left > this as something for the future. stmmac already stores a phylink_pcs > pointer, but we can't re-use that for XPCS because stmmac needs to know > that it's an XPCS vs some other PCS due to the direct calls such as > xpcs_get_an_mode() and xpcs_config_eee(). > > When I was working on EEE support at phylink level, I did try to figure > out what xpcs_config_eee() is all about, what it's trying to do, why, > and how it would fit into any phylink-based EEE scheme, but I never got > very far with that due to lack of documentation. > > So, at the moment I have no plans to touch the prototypes of > xpcs_get_an_mode(), xpcs_config_eee() nor xpcs_get_interfaces(). With > the entire patch series being so large already, I'm in no hurry to add > patches for this - which would need yet more work on stmmac that I'm > no longer willing to do. Ok, but I guess that the (very) long-term plan still is that direct calls from the MAC driver into symbols exported by the PCS are no longer going to be a thing, right? > > if (xpcs_get_an_mode(priv->hw->xpcs, mode) != DW_AN_C73)) > > > > I'm interested because I actually have some downstream NXP patches which > > introduce an entirely new MLO_AN_C73 negotiating mode in phylink (though > > they don't convert XPCS to it, sadly). Just wondering where this is going > > in your view. > > To give a flavour of what remains: > > net: pcs: xpcs: move Wangxun VR_XS_PCS_DIG_CTRL1 configuration > net: pcs: xpcs: correctly place DW_VR_MII_DIG_CTRL1_2G5_EN > net: pcs: xpcs: use dev_*() to print messages > net: pcs: xpcs: convert to use read_poll_timeout() > net: pcs: xpcs: add _modify() accessors > net: pcs: xpcs: use FIELD_PREP() and FIELD_GET() > net: pcs: xpcs: convert to use linkmode_adv_to_c73() > net: pcs: xpcs: add xpcs_linkmode_supported() > net: mdio: add linkmode_adv_to_c73() > net: pcs: xpcs: move searching ID list out of line > net: pcs: xpcs: rename xpcs_get_id() > net: pcs: xpcs: move definition of struct dw_xpcs to private header > net: pcs: xpcs: provide a helper to get the phylink pcs given xpcs > net: pcs: xpcs: pass xpcs instead of xpcs->id to xpcs_find_compat() > net: pcs: xpcs: don't use array for interface > net: pcs: xpcs: remove dw_xpcs_compat enum > > which looks like this on the diffstat: > > drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c | 2 +- > drivers/net/pcs/pcs-xpcs-nxp.c | 24 +- > drivers/net/pcs/pcs-xpcs-wx.c | 51 +-- > drivers/net/pcs/pcs-xpcs.c | 521 +++++++++------------- > drivers/net/pcs/pcs-xpcs.h | 42 +- > include/linux/mdio.h | 40 ++ > include/linux/pcs/pcs-xpcs.h | 19 +- > 7 files changed, 303 insertions(+), 396 deletions(-) Ok, I don't see anything major on the clause 73 autoneg front. Which I guess is good? (because at least there aren't competing ideas in flight about phylink's role for this operating mode) The bad part is that some user-visible functional changes in xpcs will most likely be in order. So will probably not be able to be converted without someone with both access to the 10G hardware and the motivation to do so (and this might make the conversion unreachable for me too). For example, without having seen the content of your patch list, I can only assume linkmode_adv_to_c73() is based on the ethtool link modes that _xpcs_config_aneg_c73(), mii_c73_mod_linkmode() and phylink_c73_priority_resolution[] treat. But I'm already objecting that 2500baseX shouldn't be in those tables. There should have been a new 2500base-KX ethtool mode, which is one of the amendments to 802.3-2018 called 802.3cb-2018. I also have other objections to XPCS's implementation of C73, but I don't think this is the right context to point them all out. The gist is that at least for this area, I don't think it would be a good idea at all to base core phylink support based on what XPCS has/does. I guess what I'm saying is that taking a break from stmmac until the groundwork in the core has been laid out through some other vector also seems like the best idea to me. Would you be interested in seeing an alternative implementation of clause 73 support (for the Lynx PCS), this time centered around phylink_pcs, even if it doesn't touch stmmac/xpcs? As a side effect of that work, it would maybe provide a long-term avenue of avoiding the xpcs_get_interfaces() and xpcs_get_an_mode() direct calls, as well as a more consolidated framework for C73 in XPCS to be reimplemented by somebody. (warning, this implementation will be quite large, so the question is also about your time/energy availability in the near future).