NET: QE: UEC: Allow uec re-initialization based on netretry environment variable.

PATCH New richardretanubun 2 participants 5 messages Aug 18, 2008 – Sep 16, 2008

Share & tools

richardretanubun

author

2008-08-18 19:52 UTC #0 lore raw

PATCH 2/2 NET: QE: UEC: Allow uec re-initialization based on netretry environment variable.

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.

Signed-off-by Richard Retanubun <RichardRetanubun_at_ruggedcom.com>

drivers/qe/uec.c +3 -0

index 344c649..88402ca 100644
@@ -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;
+ }
+

     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__);
         return -1;
     }

Ben Warren

2008-08-18 21:17 UTC #1 lore raw

PATCH 2/2 NET: QE: UEC: Allow uec re-initialization based on netretry environment variable.

Quoted 6 lines · richardretanubun

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.

Can't you do this by changing the 'ethactive' environment variable?

regards,
Ben

richardretanubun

author

2008-08-18 23:03 UTC #2 lore raw

PATCH 2/2 NET: QE: UEC: Allow uec re-initialization based on netretry environment variable.

Hi Ben,

Quoted 8 lines · Ben Warren

Quoted 6 lines · richardretanubun
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.
Can't you do this by changing the 'ethactive' environment variable?

You are correct, changing 'ethact' environment variable works if you are actually changing the network interface (say from "FSL UEC0" to "FSL UEC1")

I'm sorry for not adding this earlier.

The scenario I am trying to handle is if switching network interface fails. I am trying to switch and activate specific network interfaces for testing and
often times the interface will fail to initialize and will have to be reinitialized

I am using these function call sequence to do activate the specific eth interface repeatedly:

void net_set_eth(char *newEthDev)

{
    ....
    setenv("ethact", newEthDev);
    eth_set_current();
    eth_init(gd->bd);
    ....
}

By adding the patch, I can just keep trying the same eth interface. The 'ethrotate' environment variable will also work,
but only for rotating once, once a failure occur, there is no way to retry the same ethernet connection.

Quoted 3 lines

regards,
Ben

Hope this does not confuse more :)

- Richard.

Ben Warren

2008-09-04 06:00 UTC #3 lore raw

PATCH 2/2 NET: QE: UEC: Allow uec re-initialization based on netretry environment variable.

In reply to richardretanubun

Hi Richard,

Quoted 6 lines · richardretanubun

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.

Quoted 18 lines

Signed-off-by Richard Retanubun <RichardRetanubun_at_ruggedcom.com>

drivers/qe/uec.c +3 -0

index 344c649..88402ca 100644
@@ -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 */

drivers/qe/uec.c

Quoted 5 lines

drivers/qe/uec.c +5 -0

+    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.

Quoted 12 lines

     if (uec->the_first_run == 0) {
         err = init_phy(dev);
         if (err) {

drivers/qe/uec.c +3 -1

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

drivers/qe/uec.c static int uec_init(struct eth_device* dev, bd_t · line 1246

Quoted 10 lines

drivers/qe/uec.c +3 -1

+            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

richardretanubun

author

2008-09-16 13:10 UTC #4 lore raw

PATCH 2/2 NET: QE: UEC: Allow uec re-initialization based on netretry environment variable.

Hi Ben,

Quoted 8 lines · Ben Warren

Hi Richard,
Quoted 3 lines · richardretanubun
Allow uec_init to run more than once, based on the netretry environment variable.

Quoted 9 lines

Quoted 5 lines
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.

Understood, I am still new with u-boot and not that familiar with network device drivers. I will look into this file

and resubmit something when I think I have something that can apply to all devices.

Quoted 38 lines

Signed-off-by Richard Retanubun <RichardRetanubun_at_ruggedcom.com>

drivers/qe/uec.c +0 -0

index 344c649..88402ca 100644

--- a/drivers/qe/uec.c

+++ b/drivers/qe/uec.c

drivers/qe/uec.c +3 -0

@@ -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 */

drivers/qe/uec.c +3 -0

+    netretry = getenv("netretry");
 
+    if (netretry != NULL && (strcmp(netretry, "yes") == 0)) {
 
+        uec->the_first_run = 0;

Quoted 9 lines

drivers/qe/uec.c static int uec_init(struct eth_device* dev, bd_t *bd) · line 1199
Quoted 5 lines
drivers/qe/uec.c +2 -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.

I do, I thought the purpose was to retry a network operation that failed. Since a network operation can fail because of many causes, I choose to 'broaden' the scope to include bad PHY configurations.

If I have to choose a different environment variable name to control this what would be a good one? (miiretry)

Quoted 39 lines

     if (uec->the_first_run == 0) {

         err = init_phy(dev);

         if (err) {

drivers/qe/uec.c +6 -2

@@ -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",

Quoted 8 lines

Quoted 3 lines
              __FUNCTION__);
OK, good catch.
Sorry again for taking so long to respond properly.

Don't worry, as you can see I am just as capable of responding (even) later :)

Thank you for the advice.

I will study the way making a more "generic" form of this patch.

I'm afraid It won't happen soon though, the hardware I'm supposed to be booting is arriving soon.

I will submit an updated patch when I got something working on this.

Quoted 5 lines

regards,
Ben