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

[Michal Simek]

2010/11/19 Stephan Linz <linz at 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 at 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
>
>