diff mbox

phy: marvell: Fix reg-init bug on 88E1510

Message ID 1455481885-11395-1-git-send-email-clemens.gruber@pqgruber.com
State Superseded, archived
Delegated to: David Miller
Headers show

Commit Message

Clemens Gruber Feb. 14, 2016, 8:31 p.m. UTC
The function marvell_of_reg_init was called too late, in
m88e1510_config_aneg. This function was not reached, due to the phy
state machine being stuck at waiting for interrupts (which were not
enabled and off by default on the 88E1510)
It lead to the ethernet link not coming up at boot.

This commit moves the call of marvell_of_reg_init to config_init.
Now, a marvell,reg-init option to enable PHY interrupts on pin LED[2] is
set early on and the phy state machine does not get stuck anymore.
Tested on i.MX6Q boards with Marvell 88E1510 PHYs.

Signed-off-by: Clemens Gruber <clemens.gruber@pqgruber.com>
---
 drivers/net/phy/marvell.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

Comments

Andrew Lunn Feb. 14, 2016, 8:48 p.m. UTC | #1
On Sun, Feb 14, 2016 at 09:31:25PM +0100, Clemens Gruber wrote:
> The function marvell_of_reg_init was called too late, in
> m88e1510_config_aneg. This function was not reached, due to the phy
> state machine being stuck at waiting for interrupts (which were not
> enabled and off by default on the 88E1510)
> It lead to the ethernet link not coming up at boot.
> 
> This commit moves the call of marvell_of_reg_init to config_init.
> Now, a marvell,reg-init option to enable PHY interrupts on pin LED[2] is
> set early on and the phy state machine does not get stuck anymore.
> Tested on i.MX6Q boards with Marvell 88E1510 PHYs.

This looks like a good fix for you board. But maybe we should look at
the bigger picture. How about calling your new 

> +static int m88e1510_config_init(struct phy_device *phydev)
> +{
> +	int err;
> +
> +	err = marvell_of_reg_init(phydev);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}

marvell_config_init() and adding it to all PHYs which don't have a
specific config_init function.

We then get more consistent behaviour across Marvell PHYs.

   Andrew
Clemens Gruber Feb. 14, 2016, 11:21 p.m. UTC | #2
On Sun, Feb 14, 2016 at 09:48:22PM +0100, Andrew Lunn wrote:
> On Sun, Feb 14, 2016 at 09:31:25PM +0100, Clemens Gruber wrote:
> > The function marvell_of_reg_init was called too late, in
> > m88e1510_config_aneg. This function was not reached, due to the phy
> > state machine being stuck at waiting for interrupts (which were not
> > enabled and off by default on the 88E1510)
> > It lead to the ethernet link not coming up at boot.
> > 
> > This commit moves the call of marvell_of_reg_init to config_init.
> > Now, a marvell,reg-init option to enable PHY interrupts on pin LED[2] is
> > set early on and the phy state machine does not get stuck anymore.
> > Tested on i.MX6Q boards with Marvell 88E1510 PHYs.
> 
> This looks like a good fix for you board. But maybe we should look at
> the bigger picture. How about calling your new 
> 
> > +static int m88e1510_config_init(struct phy_device *phydev)
> > +{
> > +	int err;
> > +
> > +	err = marvell_of_reg_init(phydev);
> > +	if (err < 0)
> > +		return err;
> > +
> > +	return 0;
> > +}
> 
> marvell_config_init() and adding it to all PHYs which don't have a
> specific config_init function.
> 
> We then get more consistent behaviour across Marvell PHYs.

Sounds good. I'll send a new patch tomorrow!

Thanks,
Clemens
diff mbox

Patch

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index e3eb964..c6ef2da 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -446,7 +446,7 @@  static int m88e1510_config_aneg(struct phy_device *phydev)
 	if (err < 0)
 		return err;
 
-	return marvell_of_reg_init(phydev);
+	return 0;
 }
 
 static int m88e1116r_config_init(struct phy_device *phydev)
@@ -788,6 +788,17 @@  static int m88e1145_config_init(struct phy_device *phydev)
 	return 0;
 }
 
+static int m88e1510_config_init(struct phy_device *phydev)
+{
+	int err;
+
+	err = marvell_of_reg_init(phydev);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 /* marvell_read_status
  *
  * Generic status code does not detect Fiber correctly!
@@ -1259,6 +1270,7 @@  static struct phy_driver marvell_drivers[] = {
 		.features = PHY_GBIT_FEATURES,
 		.flags = PHY_HAS_INTERRUPT,
 		.probe = marvell_probe,
+		.config_init = &m88e1510_config_init,
 		.config_aneg = &m88e1510_config_aneg,
 		.read_status = &marvell_read_status,
 		.ack_interrupt = &marvell_ack_interrupt,