From: Andrew Lunn <andrew@lunn.ch>
To: Ivan Mikhaylov <i.mikhaylov@yadro.com>
Cc: "David S . Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Po-Yu Chuang <ratbert@faraday-tech.com>,
Joel Stanley <joel@jms.id.au>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
openbmc@lists.ozlabs.org
Subject: Re: [PATCH v1 1/2] net: ftgmac100: move phy connect out from ftgmac100_setup_mdio
Date: Sat, 17 Oct 2020 22:32:26 +0200 [thread overview]
Message-ID: <20201017203226.GQ456889@lunn.ch> (raw)
In-Reply-To: <20201015124917.8168-2-i.mikhaylov@yadro.com>
On Thu, Oct 15, 2020 at 03:49:16PM +0300, Ivan Mikhaylov wrote:
> Split MDIO registration and PHY connect into ftgmac100_setup_mdio and
> ftgmac100_mii_probe.
>
> Signed-off-by: Ivan Mikhaylov <i.mikhaylov@yadro.com>
> ---
> drivers/net/ethernet/faraday/ftgmac100.c | 92 ++++++++++++------------
> 1 file changed, 47 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
> index 87236206366f..6997e121824b 100644
> --- a/drivers/net/ethernet/faraday/ftgmac100.c
> +++ b/drivers/net/ethernet/faraday/ftgmac100.c
> @@ -1044,11 +1044,47 @@ static void ftgmac100_adjust_link(struct net_device *netdev)
> schedule_work(&priv->reset_task);
> }
>
> -static int ftgmac100_mii_probe(struct ftgmac100 *priv, phy_interface_t intf)
> +static int ftgmac100_mii_probe(struct net_device *netdev)
> {
> - struct net_device *netdev = priv->netdev;
> + struct ftgmac100 *priv = netdev_priv(netdev);
> + struct platform_device *pdev = to_platform_device(priv->dev);
> + struct device_node *np = pdev->dev.of_node;
> + phy_interface_t phy_intf = PHY_INTERFACE_MODE_RGMII;
> struct phy_device *phydev;
Reverse Christmas tree.
>
> + /* Get PHY mode from device-tree */
> + if (np) {
> + /* Default to RGMII. It's a gigabit part after all */
> + phy_intf = of_get_phy_mode(np, &phy_intf);
> + if (phy_intf < 0)
> + phy_intf = PHY_INTERFACE_MODE_RGMII;
I know you are just moving code around, but it is better to do:
> + err = of_get_phy_mode(np, &phy_intf);
> + if (err)
> + phy_intf = PHY_INTERFACE_MODE_RGMII;
With the code you have, you are probably going to get an email about
assigning an int to an unsigned int type from Colin..
> @@ -1860,6 +1854,14 @@ static int ftgmac100_probe(struct platform_device *pdev)
> err = ftgmac100_setup_mdio(netdev);
> if (err)
> goto err_setup_mdio;
> +
> + err = ftgmac100_mii_probe(netdev);
> + if (err) {
> + dev_err(priv->dev, "MII probe failed!\n");
> + mdiobus_unregister(priv->mii_bus);
> + goto err_setup_mdio;
> + }
It is more uniform to add a new label and add the
mdiobus_unregister(priv->mii_bus) there. All the other error handling
works like that.
Andrew
next prev parent reply other threads:[~2020-10-17 20:32 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-15 12:49 [PATCH v1 0/2] add ast2400/2500 phy-handle support Ivan Mikhaylov
2020-10-15 12:49 ` [PATCH v1 1/2] net: ftgmac100: move phy connect out from ftgmac100_setup_mdio Ivan Mikhaylov
2020-10-17 20:32 ` Andrew Lunn [this message]
2020-10-19 23:30 ` Benjamin Herrenschmidt
2020-10-15 12:49 ` [PATCH v1 2/2] net: ftgmac100: add handling of mdio/phy nodes for ast2400/2500 Ivan Mikhaylov
2020-10-17 20:39 ` Andrew Lunn
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=20201017203226.GQ456889@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=i.mikhaylov@yadro.com \
--cc=joel@jms.id.au \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=openbmc@lists.ozlabs.org \
--cc=ratbert@faraday-tech.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).