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 X-Spam-Level: X-Spam-Status: No, score=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83F5CC4338F for ; Tue, 10 Aug 2021 23:53:45 +0000 (UTC) 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 mail.kernel.org (Postfix) with ESMTPS id 40FDD60F41 for ; Tue, 10 Aug 2021 23:53:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 40FDD60F41 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linux.intel.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org 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=67v/9M6LaMRpEVwJS3DTwhBulM4yxZ5CMAiTkA7uIVo=; b=TPpqe7A+6EyS3m Yo9d0DUQUqxIYZ3FhrWlKmVC5sAfYdoadt9JUFu/AJBm8B38S+7fgL3hB/zTmWogez9fpirIjDIC5 5Xu1/jGudge8LMHwCNp/mXeiEj+AZ73FzAhpMx3IPoiCV2crulI7jiWDnE/71Mt007uVHOFkrXWc+ gssJzWRxvyQU/PsxHILIvNFiAxH/qyFUkjR9qKJjlMw+7W5rEUKpxs/awss+mDnFMHfPCnane6txw cfzBCNb7QFstgCTKiTM57aQ+kXfW3WtRhRKXplLVD3n0va3qq6TlKUuKwbB1kGlltS5HJIlrQsJ4x YPqOlzt+pwZRLAbUYkVA==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mDbWJ-0053G1-KK; Tue, 10 Aug 2021 23:50:55 +0000 Received: from mga04.intel.com ([192.55.52.120]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mDbWE-0053FN-V6 for linux-arm-kernel@lists.infradead.org; Tue, 10 Aug 2021 23:50:52 +0000 X-IronPort-AV: E=McAfee;i="6200,9189,10072"; a="213164726" X-IronPort-AV: E=Sophos;i="5.84,311,1620716400"; d="scan'208";a="213164726" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Aug 2021 16:50:49 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.84,311,1620716400"; d="scan'208";a="516087340" Received: from linux.intel.com ([10.54.29.200]) by FMSMGA003.fm.intel.com with ESMTP; 10 Aug 2021 16:50:49 -0700 Received: from linux.intel.com (vwong3-iLBPG3.png.intel.com [10.88.229.80]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by linux.intel.com (Postfix) with ESMTPS id D25E95808D9; Tue, 10 Aug 2021 16:50:44 -0700 (PDT) Date: Wed, 11 Aug 2021 07:50:41 +0800 From: Wong Vee Khee To: Vladimir Oltean Cc: Andrew Lunn , Vivien Didelot , Florian Fainelli , "David S . Miller" , Jakub Kicinski , Giuseppe Cavallaro , Alexandre Torgue , Jose Abreu , Maxime Coquelin , Heiner Kallweit , Russell King , Voon Weifeng , Michael Sit Wei Hong , linux-arm-kernel@lists.infradead.org, linux-stm32@st-md-mailman.stormreply.com, netdev@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH net-next 1/2] net: pcs: xpcs: enable skip xPCS soft reset Message-ID: <20210810235041.GA30818@linux.intel.com> References: <20210809102229.933748-1-vee.khee.wong@linux.intel.com> <20210809102229.933748-2-vee.khee.wong@linux.intel.com> <20210809110626.4kfkegwixiualq2x@skbuf> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210809110626.4kfkegwixiualq2x@skbuf> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210810_165051_061920_00D04020 X-CRM114-Status: GOOD ( 31.45 ) 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 Mon, Aug 09, 2021 at 02:06:26PM +0300, Vladimir Oltean wrote: > Hi VK, > > On Mon, Aug 09, 2021 at 06:22:28PM +0800, Wong Vee Khee wrote: > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c > > index 63fda3fc40aa..c7a3aa862079 100644 > > --- a/drivers/net/pcs/pcs-xpcs.c > > +++ b/drivers/net/pcs/pcs-xpcs.c > > @@ -1081,7 +1081,8 @@ static const struct phylink_pcs_ops xpcs_phylink_ops = { > > }; > > > > struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > > - phy_interface_t interface) > > + phy_interface_t interface, > > + bool skip_reset) > > { > > struct dw_xpcs *xpcs; > > u32 xpcs_id; > > @@ -1113,9 +1114,16 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > > xpcs->pcs.ops = &xpcs_phylink_ops; > > xpcs->pcs.poll = true; > > > > - ret = xpcs_soft_reset(xpcs, compat); > > - if (ret) > > - goto out; > > + if (!skip_reset) { > > + dev_info(&xpcs->mdiodev->dev, "%s: xPCS soft reset\n", > > + __func__); > > + ret = xpcs_soft_reset(xpcs, compat); > > + if (ret) > > + goto out; > > + } else { > > + dev_info(&xpcs->mdiodev->dev, > > + "%s: skip xpcs soft reset\n", __func__); > > + } > > I don't feel like the prints are really necessary. Sounds good to me. I'll remove these. > > > > > return xpcs; > > } > > diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h > > index add077a81b21..0c05a63f3446 100644 > > --- a/include/linux/pcs/pcs-xpcs.h > > +++ b/include/linux/pcs/pcs-xpcs.h > > @@ -36,7 +36,8 @@ void xpcs_validate(struct dw_xpcs *xpcs, unsigned long *supported, > > int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns, > > int enable); > > struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > > - phy_interface_t interface); > > + phy_interface_t interface, > > + bool xpcs_reset); > > void xpcs_destroy(struct dw_xpcs *xpcs); > > > > How about exporting the reset functionality as a separate function, and > the Intel Alder Lake stmmac shim just won't call it? Like this: > > -----------------------------[ cut here ]----------------------------- > diff --git a/drivers/net/dsa/sja1105/sja1105_mdio.c b/drivers/net/dsa/sja1105/sja1105_mdio.c > index 19aea8fb76f6..5acf6742da4d 100644 > --- a/drivers/net/dsa/sja1105/sja1105_mdio.c > +++ b/drivers/net/dsa/sja1105/sja1105_mdio.c > @@ -437,13 +437,17 @@ static int sja1105_mdiobus_pcs_register(struct sja1105_private *priv) > goto out_pcs_free; > } > > - xpcs = xpcs_create(mdiodev, priv->phy_mode[port]); > + xpcs = xpcs_create(mdiodev); > if (IS_ERR(xpcs)) { > rc = PTR_ERR(xpcs); > goto out_pcs_free; > } > > priv->xpcs[port] = xpcs; > + > + rc = xpcs_reset(xpcs, priv->phy_mode[port]); > + if (rc) > + goto out_pcs_free; > } > > priv->mdio_pcs = bus; > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c > index a5d150c5f3d8..81a145009488 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c > @@ -401,12 +401,15 @@ int stmmac_xpcs_setup(struct mii_bus *bus) > { > struct net_device *ndev = bus->priv; > struct mdio_device *mdiodev; > + bool skip_xpcs_soft_reset; > struct stmmac_priv *priv; > struct dw_xpcs *xpcs; > int mode, addr; > + int err; > > priv = netdev_priv(ndev); > mode = priv->plat->phy_interface; > + skip_xpcs_soft_reset = priv->plat->skip_xpcs_soft_reset; > > /* Try to probe the XPCS by scanning all addresses. */ > for (addr = 0; addr < PHY_MAX_ADDR; addr++) { > @@ -414,12 +417,21 @@ int stmmac_xpcs_setup(struct mii_bus *bus) > if (IS_ERR(mdiodev)) > continue; > > - xpcs = xpcs_create(mdiodev, mode); > + xpcs = xpcs_create(mdiodev); > if (IS_ERR_OR_NULL(xpcs)) { > mdio_device_free(mdiodev); > continue; > } > > + if (!skip_xpcs_soft_reset) { > + err = xpcs_reset(xpcs, mode); > + if (err) { > + xpcs_destroy(xpcs); > + mdio_device_free(mdiodev); > + continue; > + } > + } > + > priv->hw->xpcs = xpcs; > break; > } > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c > index 63fda3fc40aa..2e721e57bee4 100644 > --- a/drivers/net/pcs/pcs-xpcs.c > +++ b/drivers/net/pcs/pcs-xpcs.c > @@ -248,6 +248,18 @@ static int xpcs_soft_reset(struct dw_xpcs *xpcs, > return xpcs_poll_reset(xpcs, dev); > } > > +int xpcs_reset(struct dw_xpcs *xpcs, phy_interface_t interface) > +{ > + const struct xpcs_compat *compat; > + > + compat = xpcs_find_compat(xpcs->id, interface); > + if (!compat) > + return -ENODEV; > + > + return xpcs_soft_reset(xpcs, compat); > +} > +EXPORT_SYMBOL_GPL(xpcs_reset); > + > #define xpcs_warn(__xpcs, __state, __args...) \ > ({ \ > if ((__state)->link) \ > @@ -1080,12 +1092,11 @@ static const struct phylink_pcs_ops xpcs_phylink_ops = { > .pcs_link_up = xpcs_link_up, > }; > > -struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > - phy_interface_t interface) > +struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev) > { > struct dw_xpcs *xpcs; > u32 xpcs_id; > - int i, ret; > + int i; > > xpcs = kzalloc(sizeof(*xpcs), GFP_KERNEL); > if (!xpcs) > @@ -1097,35 +1108,18 @@ struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > > for (i = 0; i < ARRAY_SIZE(xpcs_id_list); i++) { > const struct xpcs_id *entry = &xpcs_id_list[i]; > - const struct xpcs_compat *compat; > > if ((xpcs_id & entry->mask) != entry->id) > continue; > > xpcs->id = entry; > - > - compat = xpcs_find_compat(entry, interface); > - if (!compat) { > - ret = -ENODEV; > - goto out; > - } > - > xpcs->pcs.ops = &xpcs_phylink_ops; > xpcs->pcs.poll = true; > > - ret = xpcs_soft_reset(xpcs, compat); > - if (ret) > - goto out; > - > return xpcs; > } > > - ret = -ENODEV; > - > -out: > - kfree(xpcs); > - > - return ERR_PTR(ret); > + return ERR_PTR(-ENODEV); > } > EXPORT_SYMBOL_GPL(xpcs_create); > > diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h > index add077a81b21..d841f55f12cc 100644 > --- a/include/linux/pcs/pcs-xpcs.h > +++ b/include/linux/pcs/pcs-xpcs.h > @@ -35,8 +35,8 @@ void xpcs_validate(struct dw_xpcs *xpcs, unsigned long *supported, > struct phylink_link_state *state); > int xpcs_config_eee(struct dw_xpcs *xpcs, int mult_fact_100ns, > int enable); > -struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev, > - phy_interface_t interface); > +int xpcs_reset(struct dw_xpcs *xpcs, phy_interface_t interface); > +struct dw_xpcs *xpcs_create(struct mdio_device *mdiodev); > void xpcs_destroy(struct dw_xpcs *xpcs); > > #endif /* __LINUX_PCS_XPCS_H */ > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h > index a6f03b36fc4f..0f901773c5e4 100644 > --- a/include/linux/stmmac.h > +++ b/include/linux/stmmac.h > @@ -268,5 +268,6 @@ struct plat_stmmacenet_data { > int msi_rx_base_vec; > int msi_tx_base_vec; > bool use_phy_wol; > + bool skip_xpcs_soft_reset; > }; > #endif > -----------------------------[ cut here ]----------------------------- > > I also gave this patch a run on sja1105 and it still works. Thanks for the suggestion. I tested it out on stmmac and it works. I will use this as it looks neater. :) Regards, VK _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel