Patchwork [U-Boot,V2,1/1] net: fix some bugs in LL TEMAC driver

login
register
mail settings
Submitter Stephan Linz
Date Nov. 22, 2010, 9:46 a.m.
Message ID <1290419192-13422-2-git-send-email-linz@li-pro.net>
Download mbox | patch
Permalink /patch/72513/
State Changes Requested
Headers show

Comments

Stephan Linz - Nov. 22, 2010, 9:46 a.m.
* avoid using link variable uninitialized
  * avoid using phy_addr variable with invalid value
  * reorganize phy control: first looking for phy than link
  * return with error (result value 0) if no phy/link was found
  * fix boolean mistake in wait for link: wait as long as we got
    phy register 1 has no link indication (BMSR != 0x24)
  * expand the 'first run' flag handling in ll_temac_init() in
    respect to possible error detection in xps_ll_temac_phy_ctrl()

Signed-off-by: Stephan Linz <linz@li-pro.net>
---
 drivers/net/xilinx_ll_temac.c |   52 ++++++++++++++++++++++++++++++-----------
 1 files changed, 38 insertions(+), 14 deletions(-)
Wolfgang Denk - Nov. 28, 2010, 8:35 p.m.
Dear Stephan Linz,

In message <1290419192-13422-2-git-send-email-linz@li-pro.net> you wrote:
>   * avoid using link variable uninitialized
>   * avoid using phy_addr variable with invalid value
>   * reorganize phy control: first looking for phy than link
>   * return with error (result value 0) if no phy/link was found
>   * fix boolean mistake in wait for link: wait as long as we got
>     phy register 1 has no link indication (BMSR != 0x24)
>   * expand the 'first run' flag handling in ll_temac_init() in
>     respect to possible error detection in xps_ll_temac_phy_ctrl()
> 
> Signed-off-by: Stephan Linz <linz@li-pro.net>
> ---
>  drivers/net/xilinx_ll_temac.c |   52 ++++++++++++++++++++++++++++++-----------
>  1 files changed, 38 insertions(+), 14 deletions(-)

Please merges these fixzes into the next submission of the LL TEMAC
driver.

Thanks.

Best regards,

Wolfgang Denk
Stephan Linz - Nov. 29, 2010, 11:06 a.m.
Am Sonntag, 28. November 2010, um 21:35:58 schrieb Wolfgang Denk:
> Dear Stephan Linz,

Hello Wolfgang Denk,

>
> In message <1290419192-13422-2-git-send-email-linz@li-pro.net> you wrote:
> >   * avoid using link variable uninitialized
> >   * avoid using phy_addr variable with invalid value
> >   * reorganize phy control: first looking for phy than link
> >   * return with error (result value 0) if no phy/link was found
> >   * fix boolean mistake in wait for link: wait as long as we got
> >     phy register 1 has no link indication (BMSR != 0x24)
> >   * expand the 'first run' flag handling in ll_temac_init() in
> >     respect to possible error detection in xps_ll_temac_phy_ctrl()
> >
> > Signed-off-by: Stephan Linz <linz@li-pro.net>
> > ---
> >  drivers/net/xilinx_ll_temac.c |   52
> > ++++++++++++++++++++++++++++++----------- 1 files changed, 38
> > insertions(+), 14 deletions(-)
>
> Please merges these fixzes into the next submission of the LL TEMAC
> driver.

Sure, I will maintain this patchset until I get Michal's ACK and/or he will 
merge the corrections into the next LL TEMAC driver submission. I'm in contact 
with him.


Best regards,
Stephan Linz
Michal Simek - Dec. 3, 2010, 8:48 a.m.
Stephan Linz wrote:
>   * avoid using link variable uninitialized
>   * avoid using phy_addr variable with invalid value
>   * reorganize phy control: first looking for phy than link
>   * return with error (result value 0) if no phy/link was found
>   * fix boolean mistake in wait for link: wait as long as we got
>     phy register 1 has no link indication (BMSR != 0x24)
>   * expand the 'first run' flag handling in ll_temac_init() in
>     respect to possible error detection in xps_ll_temac_phy_ctrl()
> 
> Signed-off-by: Stephan Linz <linz@li-pro.net>
> ---
>  drivers/net/xilinx_ll_temac.c |   52 ++++++++++++++++++++++++++++++-----------
>  1 files changed, 38 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c
> index 72a1632..ff93c1f 100644
> --- a/drivers/net/xilinx_ll_temac.c
> +++ b/drivers/net/xilinx_ll_temac.c
> @@ -225,24 +225,21 @@ static void read_phy_reg (struct eth_device *dev, int phy_addr)
>  #endif
>  
>  static int phy_addr = -1;
> -static int link;
> +static int link = 0;


Static variables are always initialized to 0.  I fixed this in your patch.

Added to microblaze custodian repository.

Thanks,
Michal

Patch

diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c
index 72a1632..ff93c1f 100644
--- a/drivers/net/xilinx_ll_temac.c
+++ b/drivers/net/xilinx_ll_temac.c
@@ -225,24 +225,21 @@  static void read_phy_reg (struct eth_device *dev, int phy_addr)
 #endif
 
 static int phy_addr = -1;
-static int link;
+static int link = 0;
 
 /* setting ll_temac and phy to proper setting */
 static int xps_ll_temac_phy_ctrl(struct eth_device *dev)
 {
-	int i;
+	int i, retries;
 	unsigned int result;
-	unsigned retries = 10;
 
+	 /* link is setup */
 	if (link == 1)
-		return 1; /* link is setup */
-
-	/* wait for link up */
-	while (retries-- &&
-		((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) == 0x24))
-		;
+		return 1;
 
+	/* try out if have ever found the right phy? */
 	if (phy_addr == -1) {
+		puts("Looking for phy ... ");
 		for (i = 31; i >= 0; i--) {
 			result = xps_ll_temac_hostif_get(dev, 0, i, 1);
 			if ((result & 0x0ffff) != 0x0ffff) {
@@ -251,7 +248,27 @@  static int xps_ll_temac_phy_ctrl(struct eth_device *dev)
 				break;
 			}
 		}
+
+		/* no success? -- wery bad */
+		if (phy_addr == -1) {
+			puts("ERROR\n");
+			return 0;
+		}
+		puts("OK\n");
+	}
+
+	/* wait for link up */
+	puts("Waiting for link ... ");
+	retries = 10;
+	while (retries-- &&
+		((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) != 0x24))
+		;
+
+	if (retries < 0) {
+		puts("ERROR\n");
+		return 0;
 	}
+	puts("OK\n");
 
 	/* get PHY id */
 	i = (xps_ll_temac_hostif_get(dev, 0, phy_addr, 2) << 16) | \
@@ -275,19 +292,21 @@  static int xps_ll_temac_phy_ctrl(struct eth_device *dev)
 		result = xps_ll_temac_hostif_get(dev, 0, phy_addr, 5);
 		if ((result & 0x8000) == 0x8000) {
 			xps_ll_temac_indirect_set(dev, 0, EMMC, 0x80000000);
-			printf("1000BASE-T/FD\n");
+			puts("1000BASE-T/FD\n");
 			link = 1;
 		} else if ((result & 0x4000) == 0x4000) {
 			xps_ll_temac_indirect_set(dev, 0, EMMC, 0x40000000);
-			printf("100BASE-T/FD\n");
+			puts("100BASE-T/FD\n");
 			link = 1;
 		} else {
-			printf("Unsupported mode\n");
+			puts("Unsupported mode\n");
 			link = 0;
 			return 0;
 		}
 		return 1;
 	}
+
+	puts("Unsupported PHY\n");
 	return 0;
 }
 
@@ -491,7 +510,6 @@  static int ll_temac_init(struct eth_device *dev, bd_t *bis)
 #endif
 	if (!first)
 		return 0;
-	first = 0;
 
 	xps_ll_temac_init(dev, bis);
 
@@ -502,7 +520,13 @@  static int ll_temac_init(struct eth_device *dev, bd_t *bis)
 	for (i = 0; i < 32; i++)
 		read_phy_reg(dev, i);
 #endif
-	xps_ll_temac_phy_ctrl(dev);
+
+	if (xps_ll_temac_phy_ctrl(dev) == 0) {
+		xps_ll_temac_halt(dev);
+		return -1;
+	}
+
+	first = 0;
 	return 1;
 }