[PATCH 1/1] phy: phy_update_status wait auto neg done when enabled

[PATCH 1/1] phy: phy_update_status wait auto neg done when enabled

[Jean-Christophe PLAGNIOL-VILLARD]

if the autoneg was start by the hw wait it to be finished

Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
---
 drivers/net/phy/phy.c |   26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 0c2e602..43738c2 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -28,6 +28,25 @@
 #define PHY_AN_TIMEOUT	10
 
 static int genphy_config_init(struct phy_device *phydev);
+static int phy_aneg_done(struct phy_device *phydev);
+
+static int phy_wait_aneg_read_status(struct phy_device *dev, struct phy_driver *drv)
+{
+	int ret;
+
+	if (AUTONEG_ENABLE != dev->autoneg)
+		return 0;
+
+	ret = phy_aneg_done(dev);
+	if (ret)
+		return ret;
+
+	/*
+	 * re-read the status as the aneg may not be finished
+	 * when we read it the first time
+	 */
+	return drv->read_status(dev);
+}
 
 int phy_update_status(struct phy_device *dev)
 {
@@ -35,11 +54,18 @@ int phy_update_status(struct phy_device *dev)
 	struct eth_device *edev = dev->attached_dev;
 	int ret;
 	int oldspeed = dev->speed, oldduplex = dev->duplex;
+	int old_link = dev->link;
 
 	ret = drv->read_status(dev);
 	if (ret)
 		return ret;
 
+	if (dev->link && !old_link) {
+		ret = phy_wait_aneg_read_status(dev, drv);
+		if (ret)
+			return ret;
+	}
+
 	if (dev->speed == oldspeed && dev->duplex == oldduplex)
 		return 0;
 
-- 
1.7.10.4


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

Re: [PATCH 1/1] phy: phy_update_status wait auto neg done when enabled

[Sascha Hauer]

On Tue, Nov 13, 2012 at 10:14:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> if the autoneg was start by the hw wait it to be finished
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> ---
>  drivers/net/phy/phy.c |   26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 
> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> index 0c2e602..43738c2 100644
> --- a/drivers/net/phy/phy.c
> +++ b/drivers/net/phy/phy.c
> @@ -28,6 +28,25 @@
>  #define PHY_AN_TIMEOUT	10
>  
>  static int genphy_config_init(struct phy_device *phydev);
> +static int phy_aneg_done(struct phy_device *phydev);
> +
> +static int phy_wait_aneg_read_status(struct phy_device *dev, struct phy_driver *drv)
> +{
> +	int ret;
> +
> +	if (AUTONEG_ENABLE != dev->autoneg)
> +		return 0;
> +
> +	ret = phy_aneg_done(dev);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * re-read the status as the aneg may not be finished
> +	 * when we read it the first time
> +	 */
> +	return drv->read_status(dev);
> +}
>  
>  int phy_update_status(struct phy_device *dev)
>  {
> @@ -35,11 +54,18 @@ int phy_update_status(struct phy_device *dev)
>  	struct eth_device *edev = dev->attached_dev;
>  	int ret;
>  	int oldspeed = dev->speed, oldduplex = dev->duplex;
> +	int old_link = dev->link;
>  
>  	ret = drv->read_status(dev);
>  	if (ret)
>  		return ret;
>  
> +	if (dev->link && !old_link) {
> +		ret = phy_wait_aneg_read_status(dev, drv);
> +		if (ret)
> +			return ret;
> +	}

This does not help. dev->link is false after reset, so we never get into
phy_wait_aneg_read_status() which waits for a link.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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

Re: [PATCH 1/1] phy: phy_update_status wait auto neg done when enabled

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:05 Wed 14 Nov     , Sascha Hauer wrote:
> On Tue, Nov 13, 2012 at 10:14:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > if the autoneg was start by the hw wait it to be finished
> > 
> > Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
> > ---
> >  drivers/net/phy/phy.c |   26 ++++++++++++++++++++++++++
> >  1 file changed, 26 insertions(+)
> > 
> > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
> > index 0c2e602..43738c2 100644
> > --- a/drivers/net/phy/phy.c
> > +++ b/drivers/net/phy/phy.c
> > @@ -28,6 +28,25 @@
> >  #define PHY_AN_TIMEOUT	10
> >  
> >  static int genphy_config_init(struct phy_device *phydev);
> > +static int phy_aneg_done(struct phy_device *phydev);
> > +
> > +static int phy_wait_aneg_read_status(struct phy_device *dev, struct phy_driver *drv)
> > +{
> > +	int ret;
> > +
> > +	if (AUTONEG_ENABLE != dev->autoneg)
> > +		return 0;
> > +
> > +	ret = phy_aneg_done(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	/*
> > +	 * re-read the status as the aneg may not be finished
> > +	 * when we read it the first time
> > +	 */
> > +	return drv->read_status(dev);
> > +}
> >  
> >  int phy_update_status(struct phy_device *dev)
> >  {
> > @@ -35,11 +54,18 @@ int phy_update_status(struct phy_device *dev)
> >  	struct eth_device *edev = dev->attached_dev;
> >  	int ret;
> >  	int oldspeed = dev->speed, oldduplex = dev->duplex;
> > +	int old_link = dev->link;
> >  
> >  	ret = drv->read_status(dev);
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (dev->link && !old_link) {
> > +		ret = phy_wait_aneg_read_status(dev, drv);
> > +		if (ret)
> > +			return ret;
> > +	}
> 
> This does not help. dev->link is false after reset, so we never get into
> phy_wait_aneg_read_status() which waits for a link.
read_status update the dev->link as we call update_link

if !dev->link means no cable detected

if the phy is reset the link is ready immediately or that means the reset is
not finished and the phy is not stable

this may means the phy need specific init

Best Regards,
J.

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

Re: [PATCH 1/1] phy: phy_update_status wait auto neg done when enabled

[Sascha Hauer]

On Wed, Nov 14, 2012 at 10:34:58AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 09:05 Wed 14 Nov     , Sascha Hauer wrote:
> > On Tue, Nov 13, 2012 at 10:14:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > >  
> > >  int phy_update_status(struct phy_device *dev)
> > >  {
> > > @@ -35,11 +54,18 @@ int phy_update_status(struct phy_device *dev)
> > >  	struct eth_device *edev = dev->attached_dev;
> > >  	int ret;
> > >  	int oldspeed = dev->speed, oldduplex = dev->duplex;
> > > +	int old_link = dev->link;
> > >  
> > >  	ret = drv->read_status(dev);
> > >  	if (ret)
> > >  		return ret;
> > >  
> > > +	if (dev->link && !old_link) {
> > > +		ret = phy_wait_aneg_read_status(dev, drv);
> > > +		if (ret)
> > > +			return ret;
> > > +	}
> > 
> > This does not help. dev->link is false after reset, so we never get into
> > phy_wait_aneg_read_status() which waits for a link.
> read_status update the dev->link as we call update_link
> 
> if !dev->link means no cable detected

How should the phy detect the cable if we do not wait for it being
detected?

Here's what I get from my board:

> barebox@Freescale i.MX53 LOCO:/ dhcp 
> r 0x0000 0x0001 0x7809

BMSR_ANEGCOMPLETE (1<<5) not set, this means autonegotiation is not complete

> r 0x0000 0x0004 0x01e1
> r 0x0000 0x0000 0x3000
> r 0x0000 0x0001 0x7809

Still not complete,

> r 0x0000 0x0001 0x7809
> r 0x0000 0x0005 0x0001
> r 0x0000 0x0004 0x01e1
> dhcp failed: Network is down

but we do not wait for it, so network down

> dhcp: Network is down
> barebox@Freescale i.MX53 LOCO:/ dhcp 
> r 0x0000 0x0001 0x782d

Next try, a few seconds later. BMSR_ANEGCOMPLETE is set, autonegotiation is complete,
BMSR_LSTATUS showd that link status is up.

> r 0x0000 0x0001 0x782d
> r 0x0000 0x0005 0x45e1
> r 0x0000 0x0004 0x01e1
> 100Mbps full duplex link detected

And it works.

I do not believe that this behaviour is specific to the special phy on
this board.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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

Re: [PATCH 1/1] phy: phy_update_status wait auto neg done when enabled

[Jean-Christophe PLAGNIOL-VILLARD]

On 13:04 Wed 14 Nov     , Sascha Hauer wrote:
> On Wed, Nov 14, 2012 at 10:34:58AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > On 09:05 Wed 14 Nov     , Sascha Hauer wrote:
> > > On Tue, Nov 13, 2012 at 10:14:05PM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> > > >  
> > > >  int phy_update_status(struct phy_device *dev)
> > > >  {
> > > > @@ -35,11 +54,18 @@ int phy_update_status(struct phy_device *dev)
> > > >  	struct eth_device *edev = dev->attached_dev;
> > > >  	int ret;
> > > >  	int oldspeed = dev->speed, oldduplex = dev->duplex;
> > > > +	int old_link = dev->link;
> > > >  
> > > >  	ret = drv->read_status(dev);
> > > >  	if (ret)
> > > >  		return ret;
> > > >  
> > > > +	if (dev->link && !old_link) {
> > > > +		ret = phy_wait_aneg_read_status(dev, drv);
> > > > +		if (ret)
> > > > +			return ret;
> > > > +	}
> > > 
> > > This does not help. dev->link is false after reset, so we never get into
> > > phy_wait_aneg_read_status() which waits for a link.
> > read_status update the dev->link as we call update_link
> > 
> > if !dev->link means no cable detected
> 
> How should the phy detect the cable if we do not wait for it being
> detected?
we do not have too

I'm going to try on the loco

Best Regards,
J.

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