Message ID | 20230404091442.3540092-1-michael.wei.hong.sit@intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b0ea:0:b0:3b6:4342:cba0 with SMTP id b10csp2896607vqo; Tue, 4 Apr 2023 02:43:43 -0700 (PDT) X-Google-Smtp-Source: AKy350aQV37XiFpwID5vYX0cg0bTtCDbn4qZ6TErbUhGQZ59sHtKwBbZUhiZCLhPCj4yGM/RDsah X-Received: by 2002:aa7:d6cc:0:b0:4af:81fb:4c72 with SMTP id x12-20020aa7d6cc000000b004af81fb4c72mr1962708edr.34.1680601423308; Tue, 04 Apr 2023 02:43:43 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1680601423; cv=none; d=google.com; s=arc-20160816; b=TorvKmlUqqkpSYQ6YMQyLIHE4i/+9mobYj+/AWY8C/anNUePZFJJVkYUXjk7ku4Ti0 WLfM6dYiVfDU+OCVJdJ+Hn+s/+Dyj1NMpnhJ1quExjNPyPCoTxJU7N/EdTxc/WlkWWs5 Lgwufi8hG3niZGUrR4k3eu3akmVb9JB+7tbMPVT8BUiSwQ+3ICsfRGdhOHIDrv+uk8O4 MMuAwT+ec2Iqg27mov8fjRusjzO/QYjMt2Pp52WxVlH81iyiAP9nwQO7IMGVT92fLs0G yLZuvjNjPQqr4Ne0QYGGzwoGVHymXPh9mDTf2HoUnhcJGVkxY4Jsfj4I3wcNHEAYoviR zcYg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=l/Bz5wQ5kpabrYOyx7ZPRtyWcWgT2YAvaEgAEyYv3Hw=; b=XS2+qxv5bDWOigCtz21yR/pmvAxK+/Z80H8JIsSSB4S2/9K0ifHMKf7mBv4D/WbTqP QOqtLp1ofp+2XXcwjmz8fcaEYsJ+TWVZyKz00n4hV51Q3/gr+pkNg5uSl1U6T3JQRwDX rt6VowxDd4lQHxqqvxuvdfGTZ6yQkx3ekgqFMpLVvij1DC1GXtnVOUJCZXtOcaC0N+Mp YgJ3BQ8bqC09jtDAB+NNfgd4/MY0WF6l7AYsxia8tpkJCwueXH/nFcSpB49exIRSLgaI yG0KCYJL9hHJayW0oUeJYczyq3USuch3o10qSYhxyGrq9YE+us2+xWr4aARme+cx8WS/ pZVw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=b7yuJDj6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id dy15-20020a05640231ef00b004ad03d31db6si2675662edb.278.2023.04.04.02.43.19; Tue, 04 Apr 2023 02:43:43 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=b7yuJDj6; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234059AbjDDJP5 (ORCPT <rfc822;zwp10758@gmail.com> + 99 others); Tue, 4 Apr 2023 05:15:57 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50716 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233955AbjDDJP4 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 4 Apr 2023 05:15:56 -0400 Received: from mga06.intel.com (mga06b.intel.com [134.134.136.31]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 1C1D41A7; Tue, 4 Apr 2023 02:15:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1680599755; x=1712135755; h=from:to:cc:subject:date:message-id:mime-version: content-transfer-encoding; bh=c3TWM/U7ae/+krBKWKZnTGu8xrMC3rbZUK6++vwlxtY=; b=b7yuJDj6TtQ6TcoPe/S0KVJYushwCH4iX8SyI7bepbqf4yQMXCljYqVi qn9Byf2j5hcTEPZG6jCKsJ8YTW3H1NWuPyHoOd0mPdIOMi65jnnbqCApg CkNdt9LQZOhRr8nzqhFYyDf+o+NAH9AOjlPoopH8OkZfwon8tRJVCJfS2 +1631Pydg8DGa9btbzHCTu4A8e47YOYbgH/OCVns2TNtHx2EJxZ9Py8Kk l82qLfoqKLTDRJz7s5GR0NeIS3sysBWoWmyxKF1XVZU5hbVbT5IBaHYWB 1RPnzqr8REwgfnKapyxTBsD4D4exejeYZ84A49tq+Se5FpHyt4sywEp6I Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10669"; a="404893346" X-IronPort-AV: E=Sophos;i="5.98,317,1673942400"; d="scan'208";a="404893346" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Apr 2023 02:15:54 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10669"; a="663513778" X-IronPort-AV: E=Sophos;i="5.98,317,1673942400"; d="scan'208";a="663513778" Received: from mike-ilbpg1.png.intel.com ([10.88.227.76]) by orsmga006.jf.intel.com with ESMTP; 04 Apr 2023 02:15:46 -0700 From: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com> To: Giuseppe Cavallaro <peppe.cavallaro@st.com>, Alexandre Torgue <alexandre.torgue@foss.st.com>, Jose Abreu <joabreu@synopsys.com>, "David S . Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Maxime Coquelin <mcoquelin.stm32@gmail.com>, Ong Boon Leong <boon.leong.ong@intel.com>, netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linux@armlinux.org.uk, hkallweit1@gmail.com, andrew@lunn.ch, Martin Blumenstingl <martin.blumenstingl@googlemail.com> Cc: Looi Hong Aun <hong.aun.looi@intel.com>, Voon Weifeng <weifeng.voon@intel.com>, Lai Peter Jun Ann <peter.jun.ann.lai@intel.com>, Zulkifli Muhammad Husaini <muhammad.husaini.zulkifli@intel.com>, Tan Tee Min <tee.min.tan@intel.com>, hock.leong.kweh@intel.com Subject: [RFC net 1/1] net: stmmac: skip PHY scanning when PHY already attached in DT mode Date: Tue, 4 Apr 2023 17:14:42 +0800 Message-Id: <20230404091442.3540092-1-michael.wei.hong.sit@intel.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=0.5 required=5.0 tests=AC_FROM_MANY_DOTS, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF, RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1762238318004369062?= X-GMAIL-MSGID: =?utf-8?q?1762238318004369062?= |
Series |
[RFC,net,1/1] net: stmmac: skip PHY scanning when PHY already attached in DT mode
|
|
Commit Message
Sit, Michael Wei Hong
April 4, 2023, 9:14 a.m. UTC
If PHY is successfully attached during phylink_fwnode_phy_connect()
in DT mode. MAC should not need to scan for PHY again.
Adding a logic to check if ovr_an_inband is set before scanning for
a PHY, since phylink_fwnode_phy_connect() returns 0 when
phy_fwnode = fwnode_get_phy_node(fwnode);
if (IS_ERR(phy_fwnode)) {
if (pl->cfg_link_an_mode == MLO_AN_PHY)
return -ENODEV;
return 0;
}
Fixes: fe2cfbc96803 ("net: stmmac: check if MAC needs to attach to a PHY")
Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Comments
Hi, On 04.04.2023 11:14, Michael Sit Wei Hong wrote: > If PHY is successfully attached during phylink_fwnode_phy_connect() > in DT mode. MAC should not need to scan for PHY again. > > Adding a logic to check if ovr_an_inband is set before scanning for > a PHY, since phylink_fwnode_phy_connect() returns 0 when > > phy_fwnode = fwnode_get_phy_node(fwnode); > if (IS_ERR(phy_fwnode)) { > if (pl->cfg_link_an_mode == MLO_AN_PHY) > return -ENODEV; > return 0; > } > > Fixes: fe2cfbc96803 ("net: stmmac: check if MAC needs to attach to a PHY") > Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com> This fixes broken ethernet observed recently on various Amlogic Meson based boards (like Khadas VIM3 or Odroid-C4). Thanks! Tested-by: Marek Szyprowski <m.szyprowski@samsung.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index d41a5f92aee7..4b8d3d975678 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1149,7 +1149,7 @@ static int stmmac_init_phy(struct net_device *dev) > /* Some DT bindings do not set-up the PHY handle. Let's try to > * manually parse it > */ > - if (!fwnode || phy_needed || ret) { > + if (!fwnode || (phy_needed && priv->phylink_config.ovr_an_inband) || ret) { > int addr = priv->plat->phy_addr; > struct phy_device *phydev; > Best regards
On Tue, Apr 04, 2023 at 05:14:42PM +0800, Michael Sit Wei Hong wrote: > If PHY is successfully attached during phylink_fwnode_phy_connect() > in DT mode. MAC should not need to scan for PHY again. > > Adding a logic to check if ovr_an_inband is set before scanning for > a PHY, since phylink_fwnode_phy_connect() returns 0 when > > phy_fwnode = fwnode_get_phy_node(fwnode); > if (IS_ERR(phy_fwnode)) { > if (pl->cfg_link_an_mode == MLO_AN_PHY) > return -ENODEV; > return 0; > } > > Fixes: fe2cfbc96803 ("net: stmmac: check if MAC needs to attach to a PHY") > Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com> > --- > drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index d41a5f92aee7..4b8d3d975678 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -1149,7 +1149,7 @@ static int stmmac_init_phy(struct net_device *dev) > /* Some DT bindings do not set-up the PHY handle. Let's try to > * manually parse it > */ > - if (!fwnode || phy_needed || ret) { > + if (!fwnode || (phy_needed && priv->phylink_config.ovr_an_inband) || ret) { > int addr = priv->plat->phy_addr; > struct phy_device *phydev; > Sorry, but this just doesn't look right to me. And Gnrrrrr, I wish I'd spotted this stupidity during the review of phylink_expects_phy(). phy_needed will be true if phylink thinks there should be a PHY on the link, that being: MLO_AN_PHY mode MLO_AN_INBAND mode and non-802.3z interface mode If !phy_needed, then the code should not be attempting to attach a PHY, but calling phylink_fwnode_phy_connect() is fine as it will just return zero. If phy_needed is true, then phylink_fwnode_phy_connect() will check to see whether a PHY is in the fwnode. If we fail to find a PHY, then if we're in MLO_AN_PHY mode, that's an error, and we return -ENODEV. If there is no PHY device associated with the handle, we also return -ENODEV. If phy_needed is true, and phylink_fwnode_phy_connect() doesn't find a PHY in the fwnode, and we're in MLO_AN_INBAND mode (e.g. for SGMII) then we'll return zero, because we can cope without a PHY in this instance - it's a success. If we do find a PHY, then we will make use of it, and also return zero. The problem is this hacky code wants to know the difference between those two situations, but phylink doesn't allow you to, and I don't think now that phylink_expects_phy() solves that problem. I think you're better off doing this: struct fwnode_handle *phy_fwnode; if (!phylink_expects_phy(priv->phylink)) return 0; fwnode = of_fwnode_handle(priv->plat->phylink_node); if (!fwnode) fwnode = dev_fwnode(priv->device); if (fwnode) phy_fwnode = fwnode_get_phy_node(fwnode); else phy_fwnode = NULL; if (!phy_fwnode) { ... do non-DT PHY stuff ... ret = phylink_connect_phy(priv->phylink, phydev); } else { fwnode_handle_put(phy_fwnode); ret = phylink_fwnode_phy_connect(priv->phylink, fwnode, 0); } ... ethtool wol stuff ... Doesn't that more closely reflect what you actually want this code to be doing, rather than messing about trying to guess it from phylink's return code etc?
On Tue, Apr 4, 2023 at 11:15 AM Michael Sit Wei Hong <michael.wei.hong.sit@intel.com> wrote: > > If PHY is successfully attached during phylink_fwnode_phy_connect() > in DT mode. MAC should not need to scan for PHY again. > > Adding a logic to check if ovr_an_inband is set before scanning for > a PHY, since phylink_fwnode_phy_connect() returns 0 when > > phy_fwnode = fwnode_get_phy_node(fwnode); > if (IS_ERR(phy_fwnode)) { > if (pl->cfg_link_an_mode == MLO_AN_PHY) > return -ENODEV; > return 0; > } > > Fixes: fe2cfbc96803 ("net: stmmac: check if MAC needs to attach to a PHY") > Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com> Thank you for working on this! Your patch fixes the issue I reported, Ethernet is back on my X96 Air so this patch is: Tested-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> My understanding is that Russell King is asking for a second iteration of this patch with his feedback incorporated. I'm happy to test this second version as well if you keep me Cc'ed. For this second version you can also (unconditionally) add: Reported-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Best regards, Martin
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index d41a5f92aee7..4b8d3d975678 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1149,7 +1149,7 @@ static int stmmac_init_phy(struct net_device *dev) /* Some DT bindings do not set-up the PHY handle. Let's try to * manually parse it */ - if (!fwnode || phy_needed || ret) { + if (!fwnode || (phy_needed && priv->phylink_config.ovr_an_inband) || ret) { int addr = priv->plat->phy_addr; struct phy_device *phydev;