From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
Cc: "Jose.Abreu@synopsys.com" <Jose.Abreu@synopsys.com>,
"andrew@lunn.ch" <andrew@lunn.ch>,
"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
"linux@armlinux.org.uk" <linux@armlinux.org.uk>,
"kuba@kernel.org" <kuba@kernel.org>,
"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
"peppe.cavallaro@st.com" <peppe.cavallaro@st.com>,
"alexandre.torgue@foss.st.com" <alexandre.torgue@foss.st.com>,
"davem@davemloft.net" <davem@davemloft.net>,
"mcoquelin.stm32@gmail.com" <mcoquelin.stm32@gmail.com>,
"weifeng.voon@intel.com" <weifeng.voon@intel.com>,
"boon.leong.ong@intel.com" <boon.leong.ong@intel.com>,
"tee.min.tan@intel.com" <tee.min.tan@intel.com>,
"vee.khee.wong@linux.intel.com" <vee.khee.wong@linux.intel.com>,
"vee.khee.wong@intel.com" <vee.khee.wong@intel.com>,
"linux-stm32@st-md-mailman.stormreply.com"
<linux-stm32@st-md-mailman.stormreply.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [RESEND PATCH net-next v5 1/3] net: stmmac: split xPCS setup from mdio register
Date: Fri, 4 Jun 2021 11:51:54 +0000 [thread overview]
Message-ID: <20210604115153.pux7qbrybs4ckg4j@skbuf> (raw)
In-Reply-To: <20210604105733.31092-2-michael.wei.hong.sit@intel.com>
On Fri, Jun 04, 2021 at 06:57:31PM +0800, Michael Sit Wei Hong wrote:
> From: Voon Weifeng <weifeng.voon@intel.com>
>
> This patch is a preparation patch for the enabling of Intel mGbE 2.5Gbps
> link speed. The Intel mGbR link speed configuration (1G/2.5G) is depends on
> a mdio ADHOC register which can be configured in the bios menu.
> As PHY interface might be different for 1G and 2.5G, the mdio bus need be
> ready to check the link speed and select the PHY interface before probing
> the xPCS.
>
> Signed-off-by: Voon Weifeng <weifeng.voon@intel.com>
> Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac.h | 1 +
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 7 ++
> .../net/ethernet/stmicro/stmmac/stmmac_mdio.c | 64 ++++++++++---------
> 3 files changed, 43 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> index b6cd43eda7ac..fd7212afc543 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h
> @@ -311,6 +311,7 @@ enum stmmac_state {
> int stmmac_mdio_unregister(struct net_device *ndev);
> int stmmac_mdio_register(struct net_device *ndev);
> int stmmac_mdio_reset(struct mii_bus *mii);
> +int stmmac_xpcs_setup(struct mii_bus *mii);
> void stmmac_set_ethtool_ops(struct net_device *netdev);
>
> void stmmac_ptp_register(struct stmmac_priv *priv);
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 6d41dd6f9f7a..c1331c07623d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6991,6 +6991,12 @@ int stmmac_dvr_probe(struct device *device,
> }
> }
>
> + if (priv->plat->mdio_bus_data->has_xpcs) {
stmmac_mdio_register has:
if (!mdio_bus_data)
return 0;
which suggests that some platforms might not populate priv->plat->mdio_bus_data.
Are you sure it is safe to go straight to dereferencing mdio_bus_data->has_xpcs
in the common driver probe function?
> + ret = stmmac_xpcs_setup(priv->mii);
> + if (ret)
> + goto error_xpcs_setup;
> + }
> +
> ret = stmmac_phy_setup(priv);
> if (ret) {
> netdev_err(ndev, "failed to setup phy (%d)\n", ret);
> @@ -7027,6 +7033,7 @@ int stmmac_dvr_probe(struct device *device,
> unregister_netdev(ndev);
> error_netdev_register:
> phylink_destroy(priv->phylink);
> +error_xpcs_setup:
> error_phy_setup:
> if (priv->hw->pcs != STMMAC_PCS_TBI &&
> priv->hw->pcs != STMMAC_PCS_RTBI)
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> index 6312a152c8ad..bc900e240da2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_mdio.c
> @@ -397,6 +397,41 @@ int stmmac_mdio_reset(struct mii_bus *bus)
> return 0;
> }
>
> +int stmmac_xpcs_setup(struct mii_bus *bus)
> +{
> + int mode, addr;
Can you please sort the variables in decreasing order of line length?
Also, "mode" can be of the phy_interface_t type.
> + struct net_device *ndev = bus->priv;
> + struct mdio_xpcs_args *xpcs;
> + struct stmmac_priv *priv;
> + struct mdio_device *mdiodev;
> +
> + priv = netdev_priv(ndev);
> + mode = priv->plat->phy_interface;
> +
> + /* Try to probe the XPCS by scanning all addresses. */
> + for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> + mdiodev = mdio_device_create(bus, addr);
> + if (IS_ERR(mdiodev))
> + continue;
> +
> + xpcs = xpcs_create(mdiodev, mode);
> + if (IS_ERR_OR_NULL(xpcs)) {
> + mdio_device_free(mdiodev);
> + continue;
> + }
> +
> + priv->hw->xpcs = xpcs;
> + break;
> + }
> +
> + if (!priv->hw->xpcs) {
> + dev_warn(priv->device, "No xPCS found\n");
> + return -ENODEV;
> + }
> +
> + return 0;
> +}
> +
> /**
> * stmmac_mdio_register
> * @ndev: net device structure
> @@ -501,40 +536,11 @@ int stmmac_mdio_register(struct net_device *ndev)
> goto no_phy_found;
> }
>
> - /* Try to probe the XPCS by scanning all addresses. */
> - if (mdio_bus_data->has_xpcs) {
> - int mode = priv->plat->phy_interface;
> - struct mdio_device *mdiodev;
> - struct mdio_xpcs_args *xpcs;
> -
> - for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> - mdiodev = mdio_device_create(new_bus, addr);
> - if (IS_ERR(mdiodev))
> - continue;
> -
> - xpcs = xpcs_create(mdiodev, mode);
> - if (IS_ERR_OR_NULL(xpcs)) {
> - mdio_device_free(mdiodev);
> - continue;
> - }
> -
> - priv->hw->xpcs = xpcs;
> - break;
> - }
> -
> - if (!priv->hw->xpcs) {
> - dev_warn(dev, "No XPCS found\n");
> - err = -ENODEV;
> - goto no_xpcs_found;
> - }
> - }
> -
> bus_register_done:
> priv->mii = new_bus;
>
> return 0;
>
> -no_xpcs_found:
> no_phy_found:
> mdiobus_unregister(new_bus);
> bus_register_fail:
> --
> 2.17.1
>
next prev parent reply other threads:[~2021-06-04 11:52 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-04 10:57 [RESEND PATCH net-next v5 0/3] Enable 2.5Gbps speed for stmmac Michael Sit Wei Hong
2021-06-04 10:57 ` [RESEND PATCH net-next v5 1/3] net: stmmac: split xPCS setup from mdio register Michael Sit Wei Hong
2021-06-04 11:51 ` Vladimir Oltean [this message]
2021-06-04 13:08 ` Vladimir Oltean
2021-06-04 10:57 ` [RESEND PATCH net-next v5 2/3] net: pcs: add 2500BASEX support for Intel mGbE controller Michael Sit Wei Hong
2021-06-04 12:00 ` Vladimir Oltean
2021-06-04 10:57 ` [RESEND PATCH net-next v5 3/3] net: stmmac: enable Intel mGbE 2.5Gbps link speed Michael Sit Wei Hong
2021-06-04 12:02 ` Vladimir Oltean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210604115153.pux7qbrybs4ckg4j@skbuf \
--to=vladimir.oltean@nxp.com \
--cc=Jose.Abreu@synopsys.com \
--cc=alexandre.torgue@foss.st.com \
--cc=andrew@lunn.ch \
--cc=boon.leong.ong@intel.com \
--cc=davem@davemloft.net \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-stm32@st-md-mailman.stormreply.com \
--cc=linux@armlinux.org.uk \
--cc=mcoquelin.stm32@gmail.com \
--cc=michael.wei.hong.sit@intel.com \
--cc=netdev@vger.kernel.org \
--cc=peppe.cavallaro@st.com \
--cc=tee.min.tan@intel.com \
--cc=vee.khee.wong@intel.com \
--cc=vee.khee.wong@linux.intel.com \
--cc=weifeng.voon@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).