diff mbox

[U-Boot] armv8: ls2080aqds: fix SGMII repeater settings

Message ID 1476692448-11375-1-git-send-email-shh.xie@gmail.com
State Accepted
Commit c435a7c8c1b872d38fcc671369f6e290d2fff08c
Delegated to: York Sun
Headers show

Commit Message

shaohui xie Oct. 17, 2016, 8:20 a.m. UTC
From: Shaohui Xie <Shaohui.Xie@nxp.com>

The current value to check whether the PHY was configured has dependency
on MC, it expects MC to start PCS AN, this is not true during boot up,
so it should be changed to remove the dependency.

The PHY's register space should be restore to default after accessing
extended space.

Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
---
 board/freescale/ls2080aqds/eth.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

York Sun Nov. 7, 2016, 6:42 p.m. UTC | #1
On 10/17/2016 01:33 AM, shh.xie@gmail.com wrote:
> From: Shaohui Xie <Shaohui.Xie@nxp.com>
>
> The current value to check whether the PHY was configured has dependency
> on MC, it expects MC to start PCS AN, this is not true during boot up,
> so it should be changed to remove the dependency.
>
> The PHY's register space should be restore to default after accessing
> extended space.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
> ---
>  board/freescale/ls2080aqds/eth.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/board/freescale/ls2080aqds/eth.c b/board/freescale/ls2080aqds/eth.c
> index 95ff68b..cf6791e 100644
> --- a/board/freescale/ls2080aqds/eth.c
> +++ b/board/freescale/ls2080aqds/eth.c
> @@ -144,8 +144,10 @@ static void sgmii_configure_repeater(int serdes_port)
>
>  		mdelay(10);
>
> -		if ((value & 0xfff) == 0x40f) {
> +		if ((value & 0xfff) == 0x401) {
>  			printf("DPMAC %d:PHY is ..... Configured\n", dpmac_id);
> +			miiphy_write(dev[mii_bus], riser_phy_addr[dpmac],
> +				     0x1f, 0);
>  			continue;
>  		}
>
> @@ -181,28 +183,29 @@ static void sgmii_configure_repeater(int serdes_port)
>  				if (ret > 0)
>  					goto error;
>
> -				mdelay(1);
> +				mdelay(100);

Why change the delay?

>  				ret = miiphy_read(dev[mii_bus],
>  						  riser_phy_addr[dpmac],
>  						  0x11, &value);
>  				if (ret > 0)
>  					goto error;
> -				mdelay(10);
>
> -				if ((value & 0xfff) == 0x40f) {
> +				if ((value & 0xfff) == 0x401) {
>  					printf("DPMAC %d :PHY is configured ",
>  					       dpmac_id);
>  					printf("after setting repeater 0x%x\n",
>  					       value);
>  					i = 5;
>  					j = 5;
> -				} else
> +				} else {
>  					printf("DPMAC %d :PHY is failed to ",
>  					       dpmac_id);
>  					printf("configure the repeater 0x%x\n",
>  					       value);
>  				}
> +			}
>  		}
> +		miiphy_write(dev[mii_bus], riser_phy_addr[dpmac], 0x1f, 0);
>  	}
>  error:
>  	if (ret)
>

Prabhakar,

Do these changes look correct to you?

York
Prabhakar Kushwaha Nov. 8, 2016, 11:12 a.m. UTC | #2
Hi York,


> -----Original Message-----
> From: york sun
> Sent: Tuesday, November 08, 2016 12:12 AM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> Cc: shh.xie@gmail.com; u-boot@lists.denx.de; S.H. Xie <shaohui.xie@nxp.com>
> Subject: Re: [PATCH] armv8: ls2080aqds: fix SGMII repeater settings
> 
> On 10/17/2016 01:33 AM, shh.xie@gmail.com wrote:
> > From: Shaohui Xie <Shaohui.Xie@nxp.com>
> >
> > The current value to check whether the PHY was configured has dependency
> > on MC, it expects MC to start PCS AN, this is not true during boot up,
> > so it should be changed to remove the dependency.
> >
> > The PHY's register space should be restore to default after accessing
> > extended space.
> >
> > Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
> > ---
> >  board/freescale/ls2080aqds/eth.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/board/freescale/ls2080aqds/eth.c
> b/board/freescale/ls2080aqds/eth.c
> > index 95ff68b..cf6791e 100644
> > --- a/board/freescale/ls2080aqds/eth.c
> > +++ b/board/freescale/ls2080aqds/eth.c
> > @@ -144,8 +144,10 @@ static void sgmii_configure_repeater(int serdes_port)
> >
> >  		mdelay(10);
> >
> > -		if ((value & 0xfff) == 0x40f) {
> > +		if ((value & 0xfff) == 0x401) {
> >  			printf("DPMAC %d:PHY is ..... Configured\n",
> dpmac_id);
> > +			miiphy_write(dev[mii_bus], riser_phy_addr[dpmac],
> > +				     0x1f, 0);
> >  			continue;
> >  		}
> >
> > @@ -181,28 +183,29 @@ static void sgmii_configure_repeater(int
> serdes_port)
> >  				if (ret > 0)
> >  					goto error;
> >
> > -				mdelay(1);
> > +				mdelay(100);
> 
> Why change the delay?
> 
> >  				ret = miiphy_read(dev[mii_bus],
> >  						  riser_phy_addr[dpmac],
> >  						  0x11, &value);
> >  				if (ret > 0)
> >  					goto error;
> > -				mdelay(10);
> >
> > -				if ((value & 0xfff) == 0x40f) {
> > +				if ((value & 0xfff) == 0x401) {
> >  					printf("DPMAC %d :PHY is configured ",
> >  					       dpmac_id);
> >  					printf("after setting repeater 0x%x\n",
> >  					       value);
> >  					i = 5;
> >  					j = 5;
> > -				} else
> > +				} else {
> >  					printf("DPMAC %d :PHY is failed to ",
> >  					       dpmac_id);
> >  					printf("configure the repeater 0x%x\n",
> >  					       value);
> >  				}
> > +			}
> >  		}
> > +		miiphy_write(dev[mii_bus], riser_phy_addr[dpmac], 0x1f, 0);
> >  	}
> >  error:
> >  	if (ret)
> >
> 
> Prabhakar,
> 
> Do these changes look correct to you?
> 

These changes looks fine.
Please note with older code, sometime SGMII riser card was not getting configured. It is now more consistent. 

--prabhakar
shaohui xie Nov. 10, 2016, 5:39 a.m. UTC | #3
Hi York,

Please see inline.

> -----Original Message-----
> From: york sun
> Sent: Tuesday, November 08, 2016 2:42 AM
> To: Prabhakar Kushwaha <prabhakar.kushwaha@nxp.com>
> Cc: shh.xie@gmail.com; u-boot@lists.denx.de; S.H. Xie <shaohui.xie@nxp.com>
> Subject: Re: [PATCH] armv8: ls2080aqds: fix SGMII repeater settings
> 
> On 10/17/2016 01:33 AM, shh.xie@gmail.com wrote:
> > From: Shaohui Xie <Shaohui.Xie@nxp.com>
> >
> > The current value to check whether the PHY was configured has
> > dependency on MC, it expects MC to start PCS AN, this is not true
> > during boot up, so it should be changed to remove the dependency.
> >
> > The PHY's register space should be restore to default after accessing
> > extended space.
> >
> > Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
> > ---
> >  board/freescale/ls2080aqds/eth.c | 13 ++++++++-----
> >  1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/board/freescale/ls2080aqds/eth.c
> > b/board/freescale/ls2080aqds/eth.c
> > index 95ff68b..cf6791e 100644
> > --- a/board/freescale/ls2080aqds/eth.c
> > +++ b/board/freescale/ls2080aqds/eth.c
> > @@ -144,8 +144,10 @@ static void sgmii_configure_repeater(int
> > serdes_port)
> >
> >  		mdelay(10);
> >
> > -		if ((value & 0xfff) == 0x40f) {
> > +		if ((value & 0xfff) == 0x401) {
> >  			printf("DPMAC %d:PHY is ..... Configured\n", dpmac_id);
> > +			miiphy_write(dev[mii_bus], riser_phy_addr[dpmac],
> > +				     0x1f, 0);
> >  			continue;
> >  		}
> >
> > @@ -181,28 +183,29 @@ static void sgmii_configure_repeater(int serdes_port)
> >  				if (ret > 0)
> >  					goto error;
> >
> > -				mdelay(1);
> > +				mdelay(100);
> 
> Why change the delay?
[S.H] During debug I found mdelay(1) was too short for the signal to be stable, 
mdelay(10) worked but occasional still can see failure, so I changed it to 100 to assure it.

Thank you!
Shaohui
York Sun Nov. 23, 2016, 12:54 a.m. UTC | #4
On 10/17/2016 01:33 AM, shh.xie@gmail.com wrote:
> From: Shaohui Xie <Shaohui.Xie@nxp.com>
>
> The current value to check whether the PHY was configured has dependency
> on MC, it expects MC to start PCS AN, this is not true during boot up,
> so it should be changed to remove the dependency.
>
> The PHY's register space should be restore to default after accessing
> extended space.
>
> Signed-off-by: Shaohui Xie <Shaohui.Xie@nxp.com>
> ---


Applied to fsl-qoriq, awaiting upstream. Thanks.

York
diff mbox

Patch

diff --git a/board/freescale/ls2080aqds/eth.c b/board/freescale/ls2080aqds/eth.c
index 95ff68b..cf6791e 100644
--- a/board/freescale/ls2080aqds/eth.c
+++ b/board/freescale/ls2080aqds/eth.c
@@ -144,8 +144,10 @@  static void sgmii_configure_repeater(int serdes_port)
 
 		mdelay(10);
 
-		if ((value & 0xfff) == 0x40f) {
+		if ((value & 0xfff) == 0x401) {
 			printf("DPMAC %d:PHY is ..... Configured\n", dpmac_id);
+			miiphy_write(dev[mii_bus], riser_phy_addr[dpmac],
+				     0x1f, 0);
 			continue;
 		}
 
@@ -181,28 +183,29 @@  static void sgmii_configure_repeater(int serdes_port)
 				if (ret > 0)
 					goto error;
 
-				mdelay(1);
+				mdelay(100);
 				ret = miiphy_read(dev[mii_bus],
 						  riser_phy_addr[dpmac],
 						  0x11, &value);
 				if (ret > 0)
 					goto error;
-				mdelay(10);
 
-				if ((value & 0xfff) == 0x40f) {
+				if ((value & 0xfff) == 0x401) {
 					printf("DPMAC %d :PHY is configured ",
 					       dpmac_id);
 					printf("after setting repeater 0x%x\n",
 					       value);
 					i = 5;
 					j = 5;
-				} else
+				} else {
 					printf("DPMAC %d :PHY is failed to ",
 					       dpmac_id);
 					printf("configure the repeater 0x%x\n",
 					       value);
 				}
+			}
 		}
+		miiphy_write(dev[mii_bus], riser_phy_addr[dpmac], 0x1f, 0);
 	}
 error:
 	if (ret)