Re: [U-Boot] [PATCH 1/2] net: fix some bugs in LL TEMAC driver

2010/11/19 Stephan Linz linz@li-pro.net

• 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 -1) 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()

I will test your changes on real hw.

Some comments below.

Thanks, Michal

Signed-off-by: Stephan Linz linz@li-pro.net

drivers/net/xilinx_ll_temac.c | 44 ++++++++++++++++++++++++++++++---------- 1 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c index 72a1632..a72e072 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) {
  

•           printf("Looking for phy ... ");
  
  

use puts instead.

           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) {
  

•                   printf("ERROR\n");
  
  

same here

•                   return -1;
  

•           }
  

•           printf("OK\n");
  

•   }
  

•   /* wait for link up */
  

•   printf("Waiting for link ... ");
  
  

and here.

•   retries = 10;
  

•   while (retries-- &&
  

•           ((xps_ll_temac_hostif_get(dev, 0, phy_addr, 1) & 0x24) !=
  

0x24))

•           ;
  

•   if (retries < 0) {
  

•           printf("ERROR\n");
  
  

and here

•           return 0;
   }
  

•   printf("OK\n");
  
  

and here.

   /* get PHY id */
   i = (xps_ll_temac_hostif_get(dev, 0, phy_addr, 2) << 16) | \

@@ -491,7 +508,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 +518,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;
  

}

-- 1.6.0.4