From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ben Warren <biggerbadderben@gmail.com>
Date: Wed, 03 Sep 2008 23:00:18 -0700
Subject: [U-Boot] [U-boot] [PATCH 2/2] NET: QE: UEC: Allow uec
 re-initialization based on netretry environment variable.
In-Reply-To: <48A9D30D.5020008@ruggedcom.com>
References: <48A9D30D.5020008@ruggedcom.com>
Message-ID: <48BF7972.3020905@gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Richard,

richardretanubun wrote:
> Allow uec_init to run more than once, based on the netretry environment 
> variable.
> This allows for manual (back and forth) switching between network 
> interfaces.
>
>   
My general issue with this patch is that if this functionality is really 
useful, it should be in the main device control flow (net/eth.c), not in 
a single driver.
> Signed-off-by: Richard Retanubun <RichardRetanubun_at_ruggedcom.com>
>
> diff --git a/drivers/qe/uec.c b/drivers/qe/uec.c
> index 344c649..88402ca 100644
> --- a/drivers/qe/uec.c
> +++ b/drivers/qe/uec.c
> @@ -1192,9 +1192,17 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
>      uec_private_t        *uec;
>      int            err, i;
>      struct phy_info         *curphy;
> +    char *netretry = NULL;
>  
>      uec = (uec_private_t *)dev->priv;
>  
> +
> +    /* Allow for net re-initialization based on netretry environment 
> setting */
> +    netretry = getenv("netretry");
> +    if (netretry != NULL && (strcmp(netretry, "yes") == 0)) {
> +        uec->the_first_run = 0;
> +    }
> +
>   
I'm sure you know that "netretry" already exists, and has a different 
meaning.  It's not a good idea for an environment variable to do 
different things depending on context.
>      if (uec->the_first_run == 0) {
>          err = init_phy(dev);
>          if (err) {
> @@ -1223,12 +1231,16 @@ static int uec_init(struct eth_device* dev, bd_t 
> *bd)
>          if (err || i <= 0)
>              printf("warning: %s: timeout on PHY link\n", dev->name);
>  
> -        uec->the_first_run = 1;
> +        /* If netretry is not set, or not set to yes, assume no retry */
> +        netretry = getenv("netretry");
> +        if (netretry == NULL || (strcmp(netretry, "yes") != 0)) {
> +            uec->the_first_run = 1;
> +        }
>      }
>  
>      /* Set up the MAC address */
>      if (dev->enetaddr[0] & 0x01) {
> -        printf("%s: MacAddress is multcast address\n",
> +        printf("%s: MacAddress is multicast address\n",
>               __FUNCTION__);
>   
OK, good catch.

Sorry again for taking so long to respond properly.

regards,
Ben