[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Ben Warren]

Change return values of init() functions in all Ethernet drivers to conform
to the following:

>=0: Success
<0:  Failure

All drivers going forward should return 0 on success.  Current drivers that
return 1 on success were left as-is to minimize changes.

Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
---
 cpu/ixp/npe/npe.c        |    8 ++++----
 cpu/mpc8xx/fec.c         |    6 +++---
 drivers/net/dc2114x.c    |    4 ++--
 drivers/net/eepro100.c   |    4 ++--
 drivers/net/macb.c       |    4 ++--
 drivers/net/pcnet.c      |    4 ++--
 drivers/net/rtl8139.c    |    4 ++--
 drivers/net/rtl8169.c    |    5 +++--
 drivers/net/tsec.c       |    2 +-
 drivers/net/tsi108_eth.c |    4 ++--
 drivers/net/uli526x.c    |    6 +++---
 net/eth.c                |    2 +-
 12 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/cpu/ixp/npe/npe.c b/cpu/ixp/npe/npe.c
index 7e4af44..a33b956 100644
--- a/cpu/ixp/npe/npe.c
+++ b/cpu/ixp/npe/npe.c
@@ -408,25 +408,25 @@ static int npe_init(struct eth_device *dev, bd_t * bis)
 	if (ixEthAccPortRxCallbackRegister(p_npe->eth_id, npe_rx_callback,
 					   (u32)p_npe) != IX_ETH_ACC_SUCCESS) {
 		printf("can't register RX callback!\n");
-		return 0;
+		return -1;
 	}
 
 	if (ixEthAccPortTxDoneCallbackRegister(p_npe->eth_id, npe_tx_callback,
 					       (u32)p_npe) != IX_ETH_ACC_SUCCESS) {
 		printf("can't register TX callback!\n");
-		return 0;
+		return -1;
 	}
 
 	npe_set_mac_address(dev);
 
 	if (ixEthAccPortEnable(p_npe->eth_id) != IX_ETH_ACC_SUCCESS) {
 		printf("can't enable port!\n");
-		return 0;
+		return -1;
 	}
 
 	p_npe->active = 1;
 
-	return 1;
+	return 0;
 }
 
 #if 0 /* test-only: probably have to deal with it when booting linux (for a clean state) */
diff --git a/cpu/mpc8xx/fec.c b/cpu/mpc8xx/fec.c
index 08a3715..1fd4d4e 100644
--- a/cpu/mpc8xx/fec.c
+++ b/cpu/mpc8xx/fec.c
@@ -588,7 +588,7 @@ static int fec_init (struct eth_device *dev, bd_t * bd)
 	}
 	if (i == FEC_RESET_DELAY) {
 		printf ("FEC_RESET_DELAY timeout\n");
-		return 0;
+		return -1;
 	}
 
 	/* We use strictly polling mode only
@@ -708,7 +708,7 @@ static int fec_init (struct eth_device *dev, bd_t * bd)
 
 		if (efis->actual_phy_addr == -1) {
 			printf ("Unable to discover phy!\n");
-			return 0;
+			return -1;
 		}
 #else
 		efis->actual_phy_addr = -1;
@@ -751,7 +751,7 @@ static int fec_init (struct eth_device *dev, bd_t * bd)
 
 	efis->initialized = 1;
 
-	return 1;
+	return 0;
 }
 
 
diff --git a/drivers/net/dc2114x.c b/drivers/net/dc2114x.c
index d5275dc..7238922 100644
--- a/drivers/net/dc2114x.c
+++ b/drivers/net/dc2114x.c
@@ -332,7 +332,7 @@ static int dc21x4x_init(struct eth_device* dev, bd_t* bis)
 
 	if ((INL(dev, DE4X5_STS) & (STS_TS | STS_RS)) != 0) {
 		printf("Error: Cannot reset ethernet controller.\n");
-		return 0;
+		return -1;
 	}
 
 #ifdef CONFIG_TULIP_SELECT_MEDIA
@@ -382,7 +382,7 @@ static int dc21x4x_init(struct eth_device* dev, bd_t* bis)
 
 	send_setup_frame(dev, bis);
 
-	return 1;
+	return 0;
 }
 
 static int dc21x4x_send(struct eth_device* dev, volatile void *packet, int length)
diff --git a/drivers/net/eepro100.c b/drivers/net/eepro100.c
index 738146e..96ed271 100644
--- a/drivers/net/eepro100.c
+++ b/drivers/net/eepro100.c
@@ -485,7 +485,7 @@ int eepro100_initialize (bd_t * bis)
 
 static int eepro100_init (struct eth_device *dev, bd_t * bis)
 {
-	int i, status = 0;
+	int i, status = -1;
 	int tx_cur;
 	struct descriptor *ias_cmd, *cfg_cmd;
 
@@ -598,7 +598,7 @@ static int eepro100_init (struct eth_device *dev, bd_t * bis)
 		goto Done;
 	}
 
-	status = 1;
+	status = 0;
 
   Done:
 	return status;
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 95cdc49..6657d22 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -423,12 +423,12 @@ static int macb_init(struct eth_device *netdev, bd_t *bd)
 #endif
 
 	if (!macb_phy_init(macb))
-		return 0;
+		return -1;
 
 	/* Enable TX and RX */
 	macb_writel(macb, NCR, MACB_BIT(TE) | MACB_BIT(RE));
 
-	return 1;
+	return 0;
 }
 
 static void macb_halt(struct eth_device *netdev)
diff --git a/drivers/net/pcnet.c b/drivers/net/pcnet.c
index 2af0e8f..4e270c9 100644
--- a/drivers/net/pcnet.c
+++ b/drivers/net/pcnet.c
@@ -402,7 +402,7 @@ static int pcnet_init(struct eth_device* dev, bd_t *bis)
     if (i <= 0) {
 	printf("%s: TIMEOUT: controller init failed\n", dev->name);
 	pcnet_reset (dev);
-	return 0;
+	return -1;
     }
 
     /*
@@ -410,7 +410,7 @@ static int pcnet_init(struct eth_device* dev, bd_t *bis)
      */
     pcnet_write_csr (dev, 0, 0x0002);
 
-    return 1;
+    return 0;
 }
 
 static int pcnet_send(struct eth_device* dev, volatile void *packet, int pkt_len)
diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
index 2367180..4c24805 100644
--- a/drivers/net/rtl8139.c
+++ b/drivers/net/rtl8139.c
@@ -273,10 +273,10 @@ static int rtl8139_probe(struct eth_device *dev, bd_t *bis)
 
 	if (inb(ioaddr + MediaStatus) & MSRLinkFail) {
 		printf("Cable not connected or other link failure\n");
-		return(0);
+		return -1 ;
 	}
 
-	return 1;
+	return 0;
 }
 
 /* Serial EEPROM section. */
diff --git a/drivers/net/rtl8169.c b/drivers/net/rtl8169.c
index 63ea2cc..0c9a401 100644
--- a/drivers/net/rtl8169.c
+++ b/drivers/net/rtl8169.c
@@ -620,7 +620,7 @@ static void rtl8169_init_ring(struct eth_device *dev)
 /**************************************************************************
 RESET - Finish setting up the ethernet interface
 ***************************************************************************/
-static void rtl_reset(struct eth_device *dev, bd_t *bis)
+static int rtl_reset(struct eth_device *dev, bd_t *bis)
 {
 	int i;
 	u8 diff;
@@ -649,7 +649,7 @@ static void rtl_reset(struct eth_device *dev, bd_t *bis)
 
 	if (tpc->TxDescArrays == NULL || tpc->RxDescArrays == NULL) {
 		puts("Allocate RxDescArray or TxDescArray failed\n");
-		return;
+		return -1;
 	}
 
 	rtl8169_init_ring(dev);
@@ -669,6 +669,7 @@ static void rtl_reset(struct eth_device *dev, bd_t *bis)
 #ifdef DEBUG_RTL8169
 	printf ("%s elapsed time : %d\n", __FUNCTION__, currticks()-stime);
 #endif
+	return 0;
 }
 
 /**************************************************************************
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index ca6284b..c1a2f07 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -232,7 +232,7 @@ int tsec_init(struct eth_device *dev, bd_t * bd)
 	startup_tsec(dev);
 
 	/* If there's no link, fail */
-	return priv->link;
+	return (priv->link ? 0 : -1);
 
 }
 
diff --git a/drivers/net/tsi108_eth.c b/drivers/net/tsi108_eth.c
index 524e9da..a09115e 100644
--- a/drivers/net/tsi108_eth.c
+++ b/drivers/net/tsi108_eth.c
@@ -792,7 +792,7 @@ static int tsi108_eth_probe (struct eth_device *dev, bd_t * bis)
 	    (dev->enetaddr[0] << 16);
 
 	if (marvell_88e_phy_config(dev, &speed, &duplex) == 0)
-		return 0;
+		return -1;
 
 	value =
 	    MAC_CONFIG_2_PREAMBLE_LENGTH(7) | MAC_CONFIG_2_PAD_CRC |
@@ -864,7 +864,7 @@ static int tsi108_eth_probe (struct eth_device *dev, bd_t * bis)
 	/* enable TX queue */
 	reg_TX_CONTROL(base) = TX_CONTROL_GO | 0x01;
 
-	return 1;
+	return 0;
 }
 
 /*
diff --git a/drivers/net/uli526x.c b/drivers/net/uli526x.c
index 1267c57..8460f69 100644
--- a/drivers/net/uli526x.c
+++ b/drivers/net/uli526x.c
@@ -279,12 +279,12 @@ static int uli526x_init_one(struct eth_device *dev, bd_t *bis)
 	db->desc_pool_ptr = (uchar *)&desc_pool_array[0];
 	db->desc_pool_dma_ptr = (dma_addr_t)&desc_pool_array[0];
 	if (db->desc_pool_ptr == NULL)
-		return 0;
+		return -1;
 
 	db->buf_pool_ptr = &buf_pool[0];
 	db->buf_pool_dma_ptr = (dma_addr_t)&buf_pool[0];
 	if (db->buf_pool_ptr == NULL)
-		return 0;
+		return -1;
 
 	db->first_tx_desc = (struct tx_desc *) db->desc_pool_ptr;
 	db->first_tx_desc_dma = db->desc_pool_dma_ptr;
@@ -331,7 +331,7 @@ static int uli526x_init_one(struct eth_device *dev, bd_t *bis)
 	db->cr6_data |= ULI526X_TXTH_256;
 	db->cr0_data = CR0_DEFAULT;
 	uli526x_init(dev);
-	return 1;
+	return 0;
 }
 
 static void uli526x_disable(struct eth_device *dev)
diff --git a/net/eth.c b/net/eth.c
index 3373a05..92a91a1 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -430,7 +430,7 @@ int eth_init(bd_t *bis)
 	do {
 		debug ("Trying %s\n", eth_current->name);
 
-		if (!eth_current->init(eth_current,bis)) {
+		if (eth_current->init(eth_current,bis) >= 0) {
 			eth_current->state = ETH_STATE_ACTIVE;
 
 			return 0;
-- 
1.5.2.5

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Timur Tabi]

Ben Warren wrote:
> Change return values of init() functions in all Ethernet drivers to conform
> to the following:
> 
>> =0: Success
> <0:  Failure
> 
> All drivers going forward should return 0 on success.  Current drivers that
> return 1 on success were left as-is to minimize changes.
> 
> Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
> ---
>  cpu/ixp/npe/npe.c        |    8 ++++----
>  cpu/mpc8xx/fec.c         |    6 +++---
>  drivers/net/dc2114x.c    |    4 ++--
>  drivers/net/eepro100.c   |    4 ++--
>  drivers/net/macb.c       |    4 ++--
>  drivers/net/pcnet.c      |    4 ++--
>  drivers/net/rtl8139.c    |    4 ++--
>  drivers/net/rtl8169.c    |    5 +++--
>  drivers/net/tsec.c       |    2 +-
>  drivers/net/tsi108_eth.c |    4 ++--
>  drivers/net/uli526x.c    |    6 +++---
>  net/eth.c                |    2 +-
>  12 files changed, 27 insertions(+), 26 deletions(-)

You forgot drivers/qe/uec.c

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Medve Emilian]

> -----Original Message-----
> From: u-boot-users-bounces at lists.sourceforge.net 
> [mailto:u-boot-users-bounces at lists.sourceforge.net] On Behalf 
> Of Tabi Timur
> Sent: Tuesday, January 08, 2008 4:01 PM
> To: Ben Warren
> Cc: u-boot-users at lists.sourceforge.net
> Subject: Re: [U-Boot-Users] [PATCH] Fix Ethernet init() return codes
> 
> Ben Warren wrote:
> > Change return values of init() functions in all Ethernet 
> drivers to conform
> > to the following:
> > 
> >> =0: Success
> > <0:  Failure
> > 
> > All drivers going forward should return 0 on success.  
> Current drivers that
> > return 1 on success were left as-is to minimize changes.
> > 
> > Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
> > ---
> >  cpu/ixp/npe/npe.c        |    8 ++++----
> >  cpu/mpc8xx/fec.c         |    6 +++---
> >  drivers/net/dc2114x.c    |    4 ++--
> >  drivers/net/eepro100.c   |    4 ++--
> >  drivers/net/macb.c       |    4 ++--
> >  drivers/net/pcnet.c      |    4 ++--
> >  drivers/net/rtl8139.c    |    4 ++--
> >  drivers/net/rtl8169.c    |    5 +++--
> >  drivers/net/tsec.c       |    2 +-
> >  drivers/net/tsi108_eth.c |    4 ++--
> >  drivers/net/uli526x.c    |    6 +++---
> >  net/eth.c                |    2 +-
> >  12 files changed, 27 insertions(+), 26 deletions(-)
> 
> You forgot drivers/qe/uec.c

(Maybe because) I already sent a tested patch for this. (?)


Cheers,
Emil.


> -- 
> Timur Tabi
> Linux kernel developer at Freescale

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Ben Warren]

Timur Tabi wrote:
> Ben Warren wrote:
>> Change return values of init() functions in all Ethernet drivers to 
>> conform
>> to the following:
>>
>>> =0: Success
>> <0:  Failure
>>
>> All drivers going forward should return 0 on success.  Current 
>> drivers that
>> return 1 on success were left as-is to minimize changes.
>>
>> Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
>> ---
>>  cpu/ixp/npe/npe.c        |    8 ++++----
>>  cpu/mpc8xx/fec.c         |    6 +++---
>>  drivers/net/dc2114x.c    |    4 ++--
>>  drivers/net/eepro100.c   |    4 ++--
>>  drivers/net/macb.c       |    4 ++--
>>  drivers/net/pcnet.c      |    4 ++--
>>  drivers/net/rtl8139.c    |    4 ++--
>>  drivers/net/rtl8169.c    |    5 +++--
>>  drivers/net/tsec.c       |    2 +-
>>  drivers/net/tsi108_eth.c |    4 ++--
>>  drivers/net/uli526x.c    |    6 +++---
>>  net/eth.c                |    2 +-
>>  12 files changed, 27 insertions(+), 26 deletions(-)
>
> You forgot drivers/qe/uec.c
>
Arrrghhh!  I only looked in cpu and drivers/net
More on the way, I guess...

regards,
Ben

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Ben Warren]

Ben Warren wrote:
> Change return values of init() functions in all Ethernet drivers to 
> conform
> to the following:
>
>> =0: Success
> <0:  Failure
>
> All drivers going forward should return 0 on success.  Current drivers 
> that
> return 1 on success were left as-is to minimize changes.
>
> Signed-off-by: Ben Warren <biggerbadderben@gmail.com>
> -
<snip>

This patch requires a lot of scrutiny, since it's pretty far-reaching 
and I obviously can't test all the drivers that were affected.  If as 
many people as possible can test that would be great.  Alternatively, we 
can pull the patch that caused this mess in the first place.

regards,
Ben

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Timur Tabi]

Ben Warren wrote:
> Alternatively, we 
> can pull the patch that caused this mess in the first place.

Well, this needs to be fixed, and we need to fix all the client drivers.  I just 
wish the initial patch wasn't applied so hastily.  I just can't be believe that 
patch was applied without any testing, because it's was obvious that it was 
going to break lots of client drivers.

-- 
Timur Tabi
Linux kernel developer at Freescale

[U-Boot-Users] [PATCH] Fix Ethernet init() return codes

[Ben Warren]

Timur Tabi wrote:
> Ben Warren wrote:
>> Alternatively, we can pull the patch that caused this mess in the 
>> first place.
>
> Well, this needs to be fixed, and we need to fix all the client 
> drivers.  I just wish the initial patch wasn't applied so hastily.  I 
> just can't be believe that patch was applied without any testing, 
> because it's was obvious that it was going to break lots of client 
> drivers.
>
My fault.  Sorry for causing so much heartache.  Please test the second 
patch so we can move on.

regards,
Ben