From: Florian Fainelli <f.fainelli@gmail•com>
To: arturo.buzarra@digi•com, netdev@vger•kernel.org,
Andrew Lunn <andrew@lunn•ch>,
Heiner Kallweit <hkallweit1@gmail•com>,
Vladimir Oltean <olteanv@gmail•com>,
Russell King <rmk+kernel@armlinux•org.uk>
Subject: Re: [PATCH] net: phy: return EPROBE_DEFER if PHY is not accessible
Date: Fri, 17 Mar 2023 10:05:14 -0700 [thread overview]
Message-ID: <a1dc47d6-fe07-2b13-ae53-ec6ea949333a@gmail.com> (raw)
In-Reply-To: <20230317121646.19616-1-arturo.buzarra@digi.com>
+Andrew, Heiner, Vladimir and Russell,
(please always copy the maintainers of the piece of code you are modifying).
On 3/17/23 05:16, arturo.buzarra@digi•com wrote:
> From: Arturo Buzarra <arturo.buzarra@digi•com>
>
> A PHY driver can dynamically determine the devices features, but in some
> circunstances, the PHY is not yet ready and the read capabilities does not fail
> but returns an undefined value, so incorrect capabilities are assumed and the
> initialization process fails. This commit postpones the PHY probe to ensure the
> PHY is accessible.
We need more specifics here, what type of PHY device are you seeing this
with? Keying off all 0s or all 1s is problematic because while it could
signal that the PHY is not ready in your particular case, it could also
mean that you have an electrical issue whereby the MDIO data line is
pulled down, respectively high too hard. In that case, we would rather
error out earlier than later, because no amount of probe deferral will
solve that.
If your PHY requires "some time" before it can be ready you have a
number of ways to achieve that:
- implement phy_driver::probe which may load firmware, initialize
internal state, etc.
- implement a phy_driver::get_features
Using deferred reads until MII_BMSR contains what you want is unlikely
to be the recommended way by your vendor to ensure the PHY is ready.
There has got to be some sort of vendor specific register that can be
polled to indicate readiness.
>
> Signed-off-by: Arturo Buzarra <arturo.buzarra@digi•com>
> ---
> drivers/net/phy/phy_device.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 1785f1cead97..f8c31e741936 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -2628,10 +2628,14 @@ int genphy_read_abilities(struct phy_device *phydev)
> phydev->supported);
>
> val = phy_read(phydev, MII_BMSR);
> if (val < 0)
> return val;
> + if (val == 0x0000 || val == 0xffff) {
> + phydev_err(phydev, "PHY is not accessible\n");
> + return -EPROBE_DEFER;
> + }
>
> linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported,
> val & BMSR_ANEGCAPABLE);
>
> linkmode_mod_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT, phydev->supported,
--
Florian
next prev parent reply other threads:[~2023-03-17 17:06 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-17 12:16 [PATCH] net: phy: return EPROBE_DEFER if PHY is not accessible arturo.buzarra
2023-03-17 17:05 ` Florian Fainelli [this message]
2023-03-17 17:24 ` Andrew Lunn
2023-03-17 18:21 ` Heiner Kallweit
2023-03-20 9:45 ` Buzarra, Arturo
2023-03-20 10:12 ` Russell King (Oracle)
2023-03-20 12:00 ` Andrew Lunn
2023-03-23 8:02 ` Buzarra, Arturo
2023-03-23 14:19 ` Andrew Lunn
2023-03-23 14:35 ` Russell King (Oracle)
2023-03-30 7:46 ` Buzarra, Arturo
2023-03-30 12:01 ` 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=a1dc47d6-fe07-2b13-ae53-ec6ea949333a@gmail.com \
--to=f.fainelli@gmail$(echo .)com \
--cc=andrew@lunn$(echo .)ch \
--cc=arturo.buzarra@digi$(echo .)com \
--cc=hkallweit1@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=olteanv@gmail$(echo .)com \
--cc=rmk+kernel@armlinux$(echo .)org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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