[PATCH v1] net: phy: do not call adjust_link() form phy_device_attach()

[PATCH v1] net: phy: do not call adjust_link() form phy_device_attach()

[Oleksij Rempel]

phy_device_attach() is usually called from eth_open() to enable
interface, configure MAC, etc. Some times we have situations like this:
1. find and configure PHY
2. define PHY as clock provider for MAC
3. reset and configure MAC
4. detect link and speed and configure MAC accordingly.

Which works as expected unless we use fixed PHY. Since fixed-phy was
handled differently by the PHY code, we did step 4. before step 3. In
this case we will loose MAC speed configuration at least on designware-eqos
Ethernet controller.

With this change, we handle real PHY and virtual fixed-PHY in the same way.
So, adjust_link() (step 4.) will be called after edev->open() (step 3.)
as it should be for both variants.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
 drivers/net/phy/phy.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 85cdd7862f..adff9dadd1 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -54,6 +54,13 @@ int phy_update_status(struct phy_device *phydev)
 			return ret;
 	}
 
+	/*
+	 * If the phy is a fixed-link, set it to active state to trigger
+	 * MAC configuration
+	 */
+	if (!phydev->bus && !phydev->link)
+		phydev->link = 1;
+
 	if (phydev->speed == oldspeed && phydev->duplex == oldduplex &&
 	    phydev->link == oldlink)
 		return 0;
@@ -311,7 +318,7 @@ struct phy_device *of_phy_register_fixed_link(struct device_node *np,
 
 	phydev->dev.parent = &edev->dev;
 	phydev->registered = 1;
-	phydev->link = 1;
+	phydev->link = 0;
 
 	if (of_property_read_u32(np, "speed", &phydev->speed))
 		return NULL;
@@ -401,10 +408,6 @@ static int phy_device_attach(struct phy_device *phy, struct eth_device *edev,
 
 	phy->adjust_link = adjust_link;
 
-	/* If the phy is a fixed-link, then call adjust_link directly */
-	if (!phy->bus && adjust_link)
-		adjust_link(edev);
-
 	return 0;
 }
 
-- 
2.30.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH v1] net: phy: do not call adjust_link() form phy_device_attach()

[Sascha Hauer]

On Thu, Apr 14, 2022 at 09:55:30AM +0200, Oleksij Rempel wrote:
> phy_device_attach() is usually called from eth_open() to enable
> interface, configure MAC, etc. Some times we have situations like this:
> 1. find and configure PHY
> 2. define PHY as clock provider for MAC
> 3. reset and configure MAC
> 4. detect link and speed and configure MAC accordingly.
> 
> Which works as expected unless we use fixed PHY. Since fixed-phy was
> handled differently by the PHY code, we did step 4. before step 3. In
> this case we will loose MAC speed configuration at least on designware-eqos
> Ethernet controller.
> 
> With this change, we handle real PHY and virtual fixed-PHY in the same way.
> So, adjust_link() (step 4.) will be called after edev->open() (step 3.)
> as it should be for both variants.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
>  drivers/net/phy/phy.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 85cdd7862f..adff9dadd1 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -54,6 +54,13 @@ int phy_update_status(struct phy_device *phydev)
>  			return ret;
>  	}
>  
> +	/*
> +	 * If the phy is a fixed-link, set it to active state to trigger
> +	 * MAC configuration
> +	 */
> +	if (!phydev->bus && !phydev->link)
> +		phydev->link = 1;
> +
>  	if (phydev->speed == oldspeed && phydev->duplex == oldduplex &&
>  	    phydev->link == oldlink)
>  		return 0;
> @@ -311,7 +318,7 @@ struct phy_device *of_phy_register_fixed_link(struct device_node *np,
>  
>  	phydev->dev.parent = &edev->dev;
>  	phydev->registered = 1;
> -	phydev->link = 1;
> +	phydev->link = 0;
>  
>  	if (of_property_read_u32(np, "speed", &phydev->speed))
>  		return NULL;
> @@ -401,10 +408,6 @@ static int phy_device_attach(struct phy_device *phy, struct eth_device *edev,
>  
>  	phy->adjust_link = adjust_link;
>  
> -	/* If the phy is a fixed-link, then call adjust_link directly */
> -	if (!phy->bus && adjust_link)
> -		adjust_link(edev);
> -
>  	return 0;
>  }
>  
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox