[U-Boot] [U-boot] [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>
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; + } + 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; }

Hi Ben,
Ben Warren wrote:
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.
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.
regards, Ben
Hope this does not confuse more :)
- Richard.

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

Hi Ben,
Ben Warren wrote:
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.
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.
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.
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)
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.
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.
regards,
Ben
participants (2)
-
Ben Warren
-
richardretanubun