diff mbox series

[2/2] net: phy: atheros: avoid error in ar803x_of_init() when PHY has no OF node

Message ID 20220223132056.3364440-3-vladimir.oltean@nxp.com
State Accepted
Commit 5faf161d07099d25fcb67f203c5576f258a11fbf
Delegated to: Ramon Fried
Headers show
Series PHY of_init breakage with non-OF | expand

Commit Message

Vladimir Oltean Feb. 23, 2022, 1:20 p.m. UTC 
A DM_ETH driver may use phy_connect() towards a PHY address on an MDIO
bus which is not specified in the device tree, as evidenced by:

pfe_eth_probe
-> pfe_phy_configure
   -> phy_connect

When this happens, the PHY will have an invalid OF node.

When ar803x_config() runs, it silently fails at ar803x_of_init(), and
therefore, fails to run the rest of the initialization.

This makes MII_BMCR contain what it had after BMCR_RESET (0x8000) has
been written into it by phy_reset(). Since BMCR_RESET is volatile and
self-clearing, the MII_BMCR ends up having a value of 0x0. The further
configuration of this register, which is supposed to be handled by
genphy_config_aneg() lower in ar803x_config(), never gets a chance to
run due to this early error from ar803x_of_init().

As a result of having MII_BMCR as 0, the following symptom appears:

=> setenv ethact pfe_eth0
=> setenv ipaddr 10.0.0.1
=> ping 10.0.0.2
pfe_eth0 Waiting for PHY auto negotiation to complete......... TIMEOUT !
Could not initialize PHY pfe_eth0

Manually writing 0x1140 into register 0 of the PHY makes the connection
work, but it is rather desirable that the port works without any manual
intervention.

Fixes: fe6293a80959 ("phy: atheros: add device tree bindings and config")
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/net/phy/atheros.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ramon Fried March 3, 2022, 7:53 a.m. UTC | #1 
On Wed, Feb 23, 2022 at 3:21 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
>
> A DM_ETH driver may use phy_connect() towards a PHY address on an MDIO
> bus which is not specified in the device tree, as evidenced by:
>
> pfe_eth_probe
> -> pfe_phy_configure
>    -> phy_connect
>
> When this happens, the PHY will have an invalid OF node.
>
> When ar803x_config() runs, it silently fails at ar803x_of_init(), and
> therefore, fails to run the rest of the initialization.
>
> This makes MII_BMCR contain what it had after BMCR_RESET (0x8000) has
> been written into it by phy_reset(). Since BMCR_RESET is volatile and
> self-clearing, the MII_BMCR ends up having a value of 0x0. The further
> configuration of this register, which is supposed to be handled by
> genphy_config_aneg() lower in ar803x_config(), never gets a chance to
> run due to this early error from ar803x_of_init().
>
> As a result of having MII_BMCR as 0, the following symptom appears:
>
> => setenv ethact pfe_eth0
> => setenv ipaddr 10.0.0.1
> => ping 10.0.0.2
> pfe_eth0 Waiting for PHY auto negotiation to complete......... TIMEOUT !
> Could not initialize PHY pfe_eth0
>
> Manually writing 0x1140 into register 0 of the PHY makes the connection
> work, but it is rather desirable that the port works without any manual
> intervention.
>
> Fixes: fe6293a80959 ("phy: atheros: add device tree bindings and config")
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> ---
>  drivers/net/phy/atheros.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> index f922fecd6b5d..fa1fe08518f4 100644
> --- a/drivers/net/phy/atheros.c
> +++ b/drivers/net/phy/atheros.c
> @@ -199,7 +199,7 @@ static int ar803x_of_init(struct phy_device *phydev)
>
>         node = phy_get_ofnode(phydev);
>         if (!ofnode_valid(node))
> -               return -EINVAL;
> +               return 0;
>
>         priv = malloc(sizeof(*priv));
>         if (!priv)
> --
> 2.25.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Ramon Fried April 1, 2022, 7:11 p.m. UTC | #2 
On Thu, Mar 3, 2022 at 9:53 AM Ramon Fried <rfried.dev@gmail.com> wrote:
>
> On Wed, Feb 23, 2022 at 3:21 PM Vladimir Oltean <vladimir.oltean@nxp.com> wrote:
> >
> > A DM_ETH driver may use phy_connect() towards a PHY address on an MDIO
> > bus which is not specified in the device tree, as evidenced by:
> >
> > pfe_eth_probe
> > -> pfe_phy_configure
> >    -> phy_connect
> >
> > When this happens, the PHY will have an invalid OF node.
> >
> > When ar803x_config() runs, it silently fails at ar803x_of_init(), and
> > therefore, fails to run the rest of the initialization.
> >
> > This makes MII_BMCR contain what it had after BMCR_RESET (0x8000) has
> > been written into it by phy_reset(). Since BMCR_RESET is volatile and
> > self-clearing, the MII_BMCR ends up having a value of 0x0. The further
> > configuration of this register, which is supposed to be handled by
> > genphy_config_aneg() lower in ar803x_config(), never gets a chance to
> > run due to this early error from ar803x_of_init().
> >
> > As a result of having MII_BMCR as 0, the following symptom appears:
> >
> > => setenv ethact pfe_eth0
> > => setenv ipaddr 10.0.0.1
> > => ping 10.0.0.2
> > pfe_eth0 Waiting for PHY auto negotiation to complete......... TIMEOUT !
> > Could not initialize PHY pfe_eth0
> >
> > Manually writing 0x1140 into register 0 of the PHY makes the connection
> > work, but it is rather desirable that the port works without any manual
> > intervention.
> >
> > Fixes: fe6293a80959 ("phy: atheros: add device tree bindings and config")
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> > ---
> >  drivers/net/phy/atheros.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
> > index f922fecd6b5d..fa1fe08518f4 100644
> > --- a/drivers/net/phy/atheros.c
> > +++ b/drivers/net/phy/atheros.c
> > @@ -199,7 +199,7 @@ static int ar803x_of_init(struct phy_device *phydev)
> >
> >         node = phy_get_ofnode(phydev);
> >         if (!ofnode_valid(node))
> > -               return -EINVAL;
> > +               return 0;
> >
> >         priv = malloc(sizeof(*priv));
> >         if (!priv)
> > --
> > 2.25.1
> >
> Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Applied to u-boot-net/next
Thanks,
Ramon
Vladimir Oltean April 8, 2022, 12:35 p.m. UTC | #3 
Hi Ramon,

On Fri, Apr 01, 2022 at 10:11:13PM +0300, Ramon Fried wrote:
> On Thu, Mar 3, 2022 at 9:53 AM Ramon Fried <rfried.dev@gmail.com> wrote:
> > Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
> Applied to u-boot-net/next
> Thanks,
> Ramon

Did you push this commit anywhere? I don't see it here
https://source.denx.de/u-boot/custodians/u-boot-net/-/commits/next/
diff mbox series

Patch

diff --git a/drivers/net/phy/atheros.c b/drivers/net/phy/atheros.c
index f922fecd6b5d..fa1fe08518f4 100644
--- a/drivers/net/phy/atheros.c
+++ b/drivers/net/phy/atheros.c
@@ -199,7 +199,7 @@  static int ar803x_of_init(struct phy_device *phydev)
 
 	node = phy_get_ofnode(phydev);
 	if (!ofnode_valid(node))
-		return -EINVAL;
+		return 0;
 
 	priv = malloc(sizeof(*priv));
 	if (!priv)