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 04983C27C77 for ; Fri, 14 Jun 2024 17:22:30 +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=pGSKTYEY3XLoZvCSCn2sR/kLClqwmlZjZwZrOeUP/pI=; b=oQm+HzIiZXD3mJAdgeRanbeKDV uqME/oyr+vnExnwz+boOlnttrFnW0I3nwcPOVrN/70gA3XO5P5QB+5AKEtYjF3+OSCwo5d63/gk+S xzjgznn7pl8lXDKC90RDeT850DvMrQr72mAshCImoslXWPG7fC6QWUsWfNeezm1a5vTcSBF25BYEi LpwN4SKYXLJLiXHDMCWELc+pPTRrWuy245ir3n9Je35qn6Ibbz+ftqUZkeCrlbrUduIYdcNvXh5AL WFDHjPzFxcaFWkWUJnvvhpGPSOS796evGJVX4gg74EB2eyi4+rzCLLIwbeThKFaSVK/S1kDq4Jhi3 /T3ozu/g==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sIAcu-00000003cGs-11bv; Fri, 14 Jun 2024 17:22:12 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:32c8:5054:ff:fe00:142]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sIAcq-00000003cFr-37B2 for linux-arm-kernel@lists.infradead.org; Fri, 14 Jun 2024 17:22:11 +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=pGSKTYEY3XLoZvCSCn2sR/kLClqwmlZjZwZrOeUP/pI=; b=Y4ROH0HXzL2kfp04FwoQZ+KxX1 orX20RnLFtLM8Is4YgL8rtOC+L2u/EHCsJF26vNb5zHBx8SYBsNl/Lc9lWxXP46V9wyeadOKZheLJ rUhSy3+ecQSM1HOgbO/RX5hsgC04OvtitpYnpY9CUcuI+qByWcXU0uQCofC9avCiPp3JUmFHJrHD8 GV9CfkggdTJl+3sOLK8meqVcN2PUbtLcEEpqlFXxuL7bY7OeHhSfr5KKRiQJnd//ME/SSWXmcCryY M+VVylxr9dmV4GJQAuRvsic8nYrekNFNbyX1Um5IIn7NHNpf/HemgB1a+6GvsIPF4PppzaR4tig+L 5NrGt8GQ==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:55500) by pandora.armlinux.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.96) (envelope-from ) id 1sIAcV-00025Z-1h; Fri, 14 Jun 2024 18:21:47 +0100 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1sIAcT-0002IT-B0; Fri, 14 Jun 2024 18:21:45 +0100 Date: Fri, 14 Jun 2024 18:21:45 +0100 From: "Russell King (Oracle)" To: Serge Semin Cc: Alexandre Torgue , Andrew Halaney , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Jose Abreu , linux-arm-kernel@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, Maxime Coquelin , netdev@vger.kernel.org, Paolo Abeni , Romain Gantois Subject: Re: [PATCH net-next v2 1/5] net: stmmac: add select_pcs() platform method Message-ID: References: <2xl2icmnhym4pzikivo6wqeyqny6ewrbqlfvsxrisykztdcaip@mp54uqtmrgyf> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <2xl2icmnhym4pzikivo6wqeyqny6ewrbqlfvsxrisykztdcaip@mp54uqtmrgyf> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240614_102209_252007_8D5CAC3A X-CRM114-Status: GOOD ( 27.10 ) 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 Fri, Jun 14, 2024 at 07:38:40PM +0300, Serge Semin wrote: > Hi Russell > > On Thu, Jun 13, 2024 at 11:36:06AM +0100, Russell King (Oracle) wrote: > > Allow platform drivers to provide their logic to select an appropriate > > PCS. > > > > Tested-by: Romain Gantois > > Reviewed-by: Romain Gantois > > Signed-off-by: Russell King (Oracle) > > --- > > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++++ > > include/linux/stmmac.h | 4 +++- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index bbedf2a8c60f..302aa4080de3 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > @@ -949,6 +949,13 @@ static struct phylink_pcs *stmmac_mac_select_pcs(struct phylink_config *config, > > phy_interface_t interface) > > { > > struct stmmac_priv *priv = netdev_priv(to_net_dev(config->dev)); > > + struct phylink_pcs *pcs; > > + > > + if (priv->plat->select_pcs) { > > + pcs = priv->plat->select_pcs(priv, interface); > > + if (!IS_ERR(pcs)) > > + return pcs; > > + } > > > > if (priv->hw->xpcs) > > return &priv->hw->xpcs->pcs; > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > > index 8f0f156d50d3..9c54f82901a1 100644 > > --- a/include/linux/stmmac.h > > +++ b/include/linux/stmmac.h > > @@ -13,7 +13,7 @@ > > #define __STMMAC_PLATFORM_DATA > > > > #include > > -#include > > +#include > > > > #define MTL_MAX_RX_QUEUES 8 > > #define MTL_MAX_TX_QUEUES 8 > > @@ -271,6 +271,8 @@ struct plat_stmmacenet_data { > > void (*dump_debug_regs)(void *priv); > > > int (*pcs_init)(struct stmmac_priv *priv); > > void (*pcs_exit)(struct stmmac_priv *priv); > > + struct phylink_pcs *(*select_pcs)(struct stmmac_priv *priv, > > + phy_interface_t interface); > > Just a small note/nitpick. We've got pcs_init() and pcs_exit() > callbacks. Both of them have the pcs_ prefix followed by the action > verb. What about using the same notation for the PCS-select method, > using the plat_stmmacenet_data::pcs_select() callback-name instead? >From phylink's perspective, it's not part of the PCS, it's something that the MAC does. The interface passed in to mac_select_pcs() so so the MAC code can decide which PCS (if it has many to choose from) will be used for the specified interface mode to either a PHY or other device connected to the netdev. It isn't a PCS operation, it's an operation that returns an appropriate PCS. So, I want to keep "select_pcs" as at least a suffix, because it is selecting a PCS. Eventually, I would like to see the stmmac implementations check the "interface" passed to it before deciding whether to return a PCS or not - thus how it's intended to be implemented. "pcs_select" seems to make it sound like it's part of a PCS implementation, which as I've explained above, it isn't. It also doesn't convey that it's selecting a PCS based on its arguments. I'd also like to keep the ability to grep for "select_pcs" to find implementations and not have complex grep expressions to find whatever the driver has called it! To that end, I much prefer that drivers that name sub-implementations the same way that phylink names them to make grepping easier. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!