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 1041BC433F5 for ; Fri, 25 Feb 2022 10:57:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238839AbiBYK6V (ORCPT ); Fri, 25 Feb 2022 05:58:21 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:48976 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S236771AbiBYK6U (ORCPT ); Fri, 25 Feb 2022 05:58:20 -0500 Received: from pandora.armlinux.org.uk (pandora.armlinux.org.uk [IPv6:2001:4d48:ad52:32c8:5054:ff:fe00:142]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C3FCD197B47 for ; Fri, 25 Feb 2022 02:57:48 -0800 (PST) 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=hMGbU3Vkj5WK+JmlrMWQYsksGK4/IrXnet4jcvWIigw=; b=PLtxBaiiLbTzRZ2KGN0n7/bBiB UW5pS3VVOP01KMjlzmItcrUew9yJGjT2/j9bWqrjeHQRx0L9AydORDkyjav4WUfEKHfTnDE/2aK+o J679mFf+KQD1oOpbaJgqkoWVqejRlvAjTBeN5HWzEtKEw68b91P4G9HRDpelWsh+VeUQGqh4431BE 9rauuV+xr5KK+7x+A9lS84k92rl+R1UfyA9Dki3LIU6L02ANIfd3I0aAty49rEnKD2EX1TwSPh2RE 84VoS2E6JvseIGGFQQZ0QeeI4eCf4BebcYER2yH5Jn5LcDvQ1XbOKftP9Q9v8rZK6B2/O06/paSYF pjh139rA==; Received: from shell.armlinux.org.uk ([fd8f:7570:feb6:1:5054:ff:fe00:4ec]:57474) 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 1nNYIB-0005Co-64; Fri, 25 Feb 2022 10:57:43 +0000 Received: from linux by shell.armlinux.org.uk with local (Exim 4.94.2) (envelope-from ) id 1nNYI9-0002zK-0D; Fri, 25 Feb 2022 10:57:41 +0000 Date: Fri, 25 Feb 2022 10:57:40 +0000 From: "Russell King (Oracle)" To: Vladimir Oltean Cc: Marek Beh__n , Andrew Lunn , Vivien Didelot , Florian Fainelli , "David S. Miller" , Jakub Kicinski , netdev@vger.kernel.org Subject: Re: [PATCH RFC net-next 3/6] net: dsa: sja1105: use .mac_select_pcs() interface Message-ID: References: <20220225103913.abn4pc57ow6dy2m6@skbuf> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220225103913.abn4pc57ow6dy2m6@skbuf> Sender: Russell King (Oracle) Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Fri, Feb 25, 2022 at 12:39:13PM +0200, Vladimir Oltean wrote: > On Thu, Feb 24, 2022 at 04:15:26PM +0000, Russell King (Oracle) wrote: > > Convert the PCS selection to use mac_select_pcs, which allows the PCS > > to perform any validation it needs, and removes the need to set the PCS > > in the mac_config() callback, delving into the higher DSA levels to do > > so. > > > > Signed-off-by: Russell King (Oracle) > > --- > > Reviewed-by: Vladimir Oltean Thanks. > > - .phylink_mac_config = sja1105_mac_config, > > Deleting sja1105_mac_config() here is safe not because > phylink_mac_config() stops calling pl->mac_ops->mac_config(), but > because dsa_port_phylink_mac_config() first checks whether > ds->ops->phylink_mac_config is implemented, and that is purely an > artefact of providing a phylib-style ds->ops->adjust_link, right? Yes and no. We already have a several DSA drivers that have NULL phylink_mac_config and that don't provide an adjust_link function. Even if adjust_link was eventually killed off, the test in dsa_port_phylink_mac_config() would still be necessary unless all these DSA drivers are updated with a stub function for it. Consequently, I view phylink_mac_config in DSA as entirely optional and that optionality is already very much a part of the DSA interface, even though that is not the case with the corresponding phylink_mac_ops .mac_config method. Moreover, this optionality is a common theme in DSA switch operations methods. > Maybe it's worth mentioning. Given that .phylink_mac_config is already established as being optional in DSA, does the addition of one more instance need to be explicitly mentioned? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!