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

[U-Boot-Users] [PATCH V2] 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>
---

This one includes QE UEC.  Please let me know if any other drivers are missing!

 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 +++---
 drivers/qe/uec.c         |    6 +++---
 net/eth.c                |    2 +-
 13 files changed, 30 insertions(+), 29 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/drivers/qe/uec.c b/drivers/qe/uec.c
index dc2765b..26cfa2b 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -1110,7 +1110,7 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
 		if (dev->enetaddr[0] & 0x01) {
 			printf("%s: MacAddress is multcast address\n",
 				 __FUNCTION__);
-			return 0;
+			return -1;
 		}
 		uec_set_mac_address(uec, dev->enetaddr);
 		uec->the_first_run = 1;
@@ -1119,10 +1119,10 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
 	err = uec_open(uec, COMM_DIR_RX_AND_TX);
 	if (err) {
 		printf("%s: cannot enable UEC device\n", dev->name);
-		return 0;
+		return -1;
 	}
 
-	return uec->mii_info->link;
+	return (uec->mii_info->link ? 0 : -1);
 }
 
 static void uec_halt(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 V2] Fix Ethernet init() return codes.

[Stefan Roese]

On Tuesday 08 January 2008, 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>

Acked-by: Stefan Roese <sr@denx.de>

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 09:18 Wed 09 Jan     , Kim Phillips wrote:
> On Wed, 9 Jan 2008 05:58:33 +0100
> Stefan Roese <sr@denx.de> wrote:
> 
> > On Tuesday 08 January 2008, 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>
> > 
> > Acked-by: Stefan Roese <sr@denx.de>

for npe drivers

Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>

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

[Kim Phillips]

On Wed, 9 Jan 2008 05:58:33 +0100
Stefan Roese <sr@denx.de> wrote:

> On Tuesday 08 January 2008, 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>
> 
> Acked-by: Stefan Roese <sr@denx.de>

for both qe and tsec based devices:

Acked-by: Kim Phillips <kim.phillips@freescale.com>

this should be applied asap

Kim

[U-Boot-Users] [PATCH V2] 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>

Acked-By: Timur Tabi <timur@freescale.com>

Tested on an 8360 (QE), although I see Kim beat me to it.

-- 
Timur Tabi
Linux kernel developer at Freescale

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

[Haavard Skinnemoen]

On Tue, 08 Jan 2008 17:23:21 -0500
Ben Warren <biggerbadderben@gmail.com> wrote:

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

Acked-by: Haavard Skinnemoen <hskinnemoen@atmel.com>

Will test at the first opportunity...hopefully tonight.

Haavard

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

[Ben Warren]

Wolfgang,

Kim Phillips wrote:
> On Wed, 9 Jan 2008 05:58:33 +0100
> Stefan Roese <sr@denx.de> wrote:
>
>   
>> On Tuesday 08 January 2008, 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>
>>>       
>> Acked-by: Stefan Roese <sr@denx.de>
>>     
>
> for both qe and tsec based devices:
>
> Acked-by: Kim Phillips <kim.phillips@freescale.com>
>
> this should be applied asap
>
> Kim
>
>   
Please apply directly.

regards,
Ben

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

[Ulf Samuelsson]

> On Tue, 08 Jan 2008 17:23:21 -0500
> Ben Warren <biggerbadderben@gmail.com> wrote:
> 
>> 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;
>>  }
>>  


Why not use symbolic return values?
"return SUCCESS;" seems much more appropriate than 
"return 0;"  or "return -1;"

if(myfunction() == SUCCESS) {

}

is much more clear than 

if(myfunction()) {

}


Best Regards
Ulf Samuelsson

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

[Wolfgang Denk]

Dear Ben,

in message <4784FDFE.8030801@gmail.com> you wrote:
> 
> Kim Phillips wrote:
> > On Wed, 9 Jan 2008 05:58:33 +0100
> > Stefan Roese <sr@denx.de> wrote:
> >
> >   
> >> On Tuesday 08 January 2008, 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>
> >>>       
> >> Acked-by: Stefan Roese <sr@denx.de>
> >>     
> >
> > for both qe and tsec based devices:
> >
> > Acked-by: Kim Phillips <kim.phillips@freescale.com>
> >
> > this should be applied asap
> >
> > Kim
> >
> >   
> Please apply directly.

Which one? The original "[PATCH V2] Fix Ethernet init() return codes"
patch?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The use of COBOL cripples the mind; its teaching  should,  therefore,
be regarded as a criminal offense.                   - E. W. Dijkstra

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

[Ben Warren]

Wolfgang Denk wrote:
> Dear Ben,
>
> in message <4784FDFE.8030801@gmail.com> you wrote:
>   
>> Kim Phillips wrote:
>>     
>>> On Wed, 9 Jan 2008 05:58:33 +0100
>>> Stefan Roese <sr@denx.de> wrote:
>>>
>>>   
>>>       
>>>> On Tuesday 08 January 2008, 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>
>>>>>       
>>>>>           
>>>> Acked-by: Stefan Roese <sr@denx.de>
>>>>     
>>>>         
>>> for both qe and tsec based devices:
>>>
>>> Acked-by: Kim Phillips <kim.phillips@freescale.com>
>>>
>>> this should be applied asap
>>>
>>> Kim
>>>
>>>   
>>>       
>> Please apply directly.
>>     
>
> Which one? The original "[PATCH V2] Fix Ethernet init() return codes"
> patch?
>
> Best regards,
>
> Wolfgang Denk
>
>   
Yes, the original V2 patch, not the one before it since it missed QE.

regards,
Ben

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

[Jean-Christophe PLAGNIOL-VILLARD]

On 16:37 Wed 09 Jan     , Ben Warren wrote:
> Wolfgang Denk wrote:
> > Dear Ben,
> > Best regards,
> >
> > Wolfgang Denk
> >
> >   
> Yes, the original V2 patch, not the one before it since it missed QE.
> 
> regards,
> Ben
Hi Ben,

If I'm not wrong,
It will be good to add all Acked-By: ....
to the patch

Best regards,
J.

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

[Wolfgang Denk]

In message <014701c852ed$4b0c4040$6103170a@atmel.com> you wrote:
>
> Why not use symbolic return values?

Because then nobody knows what the symbols mean.

> "return SUCCESS;" seems much more appropriate than 
> "return 0;"  or "return -1;"

These constants are unmistakeable for anyone who ever wrote a program
under any Unix system...

> if(myfunction() == SUCCESS) {
> 
> }
> 
> is much more clear than 
> 
> if(myfunction()) {
> 
> }

Not to me. The former needs a lookup of the define, the latter is
obvious.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Es gibt immer genug fuer die Beduerfnisse aller, aber  niemals  genug
fuer die Gier einzelner.                                    -- Ghandi

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

[Ben Warren]

Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 16:37 Wed 09 Jan     , Ben Warren wrote:
>   
>> Wolfgang Denk wrote:
>>     
>>> Dear Ben,
>>> Best regards,
>>>
>>> Wolfgang Denk
>>>
>>>   
>>>       
>> Yes, the original V2 patch, not the one before it since it missed QE.
>>
>> regards,
>> Ben
>>     
> Hi Ben,
>
> If I'm not wrong,
> It will be good to add all Acked-By: ....
> to the patch
>   
OK, hope I didn't leave anybody out:


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>
Acked-by: Stefan Roese <sr@denx.de>
Acked-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com>
Acked-by: Kim Phillips <kim.phillips@freescale.com>
Acked-by: Haavard Skinnemoen <hskinnemoen@atmel.com>
Acked-By: Timur Tabi <timur@freescale.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 +++---
 drivers/qe/uec.c         |    6 +++---
 net/eth.c                |    2 +-
 13 files changed, 30 insertions(+), 29 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/drivers/qe/uec.c b/drivers/qe/uec.c
index dc2765b..26cfa2b 100644
--- a/drivers/qe/uec.c
+++ b/drivers/qe/uec.c
@@ -1110,7 +1110,7 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
 		if (dev->enetaddr[0] & 0x01) {
 			printf("%s: MacAddress is multcast address\n",
 				 __FUNCTION__);
-			return 0;
+			return -1;
 		}
 		uec_set_mac_address(uec, dev->enetaddr);
 		uec->the_first_run = 1;
@@ -1119,10 +1119,10 @@ static int uec_init(struct eth_device* dev, bd_t *bd)
 	err = uec_open(uec, COMM_DIR_RX_AND_TX);
 	if (err) {
 		printf("%s: cannot enable UEC device\n", dev->name);
-		return 0;
+		return -1;
 	}
 
-	return uec->mii_info->link;
+	return (uec->mii_info->link ? 0 : -1);
 }
 
 static void uec_halt(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 V2] Fix Ethernet init() return codes.

[Wolfgang Denk]

Dear Ben,

in message <47853EA0.80008@gmail.com> you wrote:
>
> > Which one? The original "[PATCH V2] Fix Ethernet init() return codes"
> > patch?
...
> Yes, the original V2 patch, not the one before it since it missed QE.

Can you please rebase it? It fails now:

error: patch failed: cpu/mpc8xx/fec.c:588
error: cpu/mpc8xx/fec.c: patch does not apply
error: patch failed: drivers/net/rtl8169.c:620
error: drivers/net/rtl8169.c: patch does not apply
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merged cpu/mpc8xx/fec.c
CONFLICT (content): Merge conflict in cpu/mpc8xx/fec.c
Auto-merged drivers/net/rtl8169.c
CONFLICT (content): Merge conflict in drivers/net/rtl8169.c
Auto-merged drivers/net/tsec.c
Auto-merged drivers/qe/uec.c
Auto-merged net/eth.c
Failed to merge in the changes.
Patch failed at 0001.

Sorry...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"...one of the main causes of the fall of the Roman Empire was  that,
lacking  zero,  they had no way to indicate successful termination of
their C programs."                                     - Robert Firth

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

[Ben Warren]

Wolfgang Denk wrote:
> Dear Ben,
>
> in message <47853EA0.80008@gmail.com> you wrote:
>   
>>> Which one? The original "[PATCH V2] Fix Ethernet init() return codes"
>>> patch?
>>>       
> ...
>   
>> Yes, the original V2 patch, not the one before it since it missed QE.
>>     
>
> Can you please rebase it? It fails now:
>
> error: patch failed: cpu/mpc8xx/fec.c:588
> error: cpu/mpc8xx/fec.c: patch does not apply
> error: patch failed: drivers/net/rtl8169.c:620
> error: drivers/net/rtl8169.c: patch does not apply
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> Auto-merged cpu/mpc8xx/fec.c
> CONFLICT (content): Merge conflict in cpu/mpc8xx/fec.c
> Auto-merged drivers/net/rtl8169.c
> CONFLICT (content): Merge conflict in drivers/net/rtl8169.c
> Auto-merged drivers/net/tsec.c
> Auto-merged drivers/qe/uec.c
> Auto-merged net/eth.c
> Failed to merge in the changes.
> Patch failed at 0001.
>
> Sorry...
>
> Best regards,
>
> Wolfgang Denk
>
>   
Will do

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

[Ulf Samuelsson]

> In message <014701c852ed$4b0c4040$6103170a@atmel.com> you wrote:
>>
>> Why not use symbolic return values?
> 
> Because then nobody knows what the symbols mean.
> 
>> "return SUCCESS;" seems much more appropriate than 
>> "return 0;"  or "return -1;"
> 
> These constants are unmistakeable for anyone who ever wrote a program
> under any Unix system...
> 
>> if(myfunction() == SUCCESS) {
>> 
>> }
>> 
>> is much more clear than 
>> 
>> if(myfunction()) {
>> 
>> }
> 
> Not to me. The former needs a lookup of the define, the latter is
> obvious.
>

In properly structured programs, the representation of an enumeration variable is irrelevant.
I.E: You do not need to lookup the value of SUCCESS, (but you do need to know that
the return value "SUCCESS" means that the call did not fail).

You have the power to ack/nack patches, I am sure you are capable of nack'ing
patches which return SUCCESS when the call fails.

Obviously code like

"if (myfunction() > SUCCESS)"
is not going to be acceptable, but that should probably be
replaced by 

"if (myfunction() != SUCCESS)"

 
Best regards,
Ulf Samuelsson

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

[Haavard Skinnemoen]

On Wed, 9 Jan 2008 17:26:28 +0100
Haavard Skinnemoen <hskinnemoen@atmel.com> wrote:

> Will test at the first opportunity...hopefully tonight.

Ok, as expected, tftpboot fails without this patch and succeeds with
it. Thanks!

Haavard