Fix Ethernet init() return codes.

PATCH Superseded Ben Warren 8 participants 17 messages Jan 8, 2008 – Jan 10, 2008

Subsystems (2)

Networking Network drivers

lore

Share & tools

lore

Ben Warren

author

2008-01-08 22:23 UTC #0 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

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 <[email protected]>

---

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

cpu/ixp/npe/npe.c +4 -4

index 7e4af44..a33b956 100644
@@ -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) */

cpu/mpc8xx/fec.c +3 -3

index 08a3715..1fd4d4e 100644
@@ -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;
 }

drivers/net/dc2114x.c +2 -2

index d5275dc..7238922 100644
@@ -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)

drivers/net/eepro100.c +2 -2

index 738146e..96ed271 100644
@@ -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;

drivers/net/macb.c +2 -2

index 95cdc49..6657d22 100644
@@ -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)

drivers/net/pcnet.c +2 -2

index 2af0e8f..4e270c9 100644
@@ -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)

drivers/net/rtl8139.c +2 -2

index 2367180..4c24805 100644
@@ -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. */

drivers/net/rtl8169.c +3 -2

index 63ea2cc..0c9a401 100644
@@ -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;
 }
 
 /**************************************************************************

drivers/net/tsec.c +1 -1

index ca6284b..c1a2f07 100644
@@ -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);
 
 }

drivers/net/tsi108_eth.c +2 -2

index 524e9da..a09115e 100644
@@ -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;
 }
 
 /*

drivers/net/uli526x.c +3 -3

index 1267c57..8460f69 100644
@@ -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)

drivers/qe/uec.c +3 -3

index dc2765b..26cfa2b 100644
@@ -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)

net/eth.c +1 -1

index 3373a05..92a91a1 100644
@@ -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;

Signature · 1 line

1.5.2.5

Stefan Roese

2008-01-09 04:58 UTC #1 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

Quoted 11 lines · 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 <[email protected]>

Acked-by Stefan Roese <[email protected]>

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

Jean-Christophe PLAGNIOL-VILLARD

2008-01-09 14:56 UTC #3 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Kim Phillips

Quoted 9 lines · Kim Phillips

Quoted 6 lines · On Wed, 9 Jan 2008 05:58:33 +0100 Stefan Roese <[email protected]> wrote:
Quoted 5 lines · Ben Warren
Change return values of init() functions in all Ethernet drivers to conform
to the following:
	>=0: Success

Quoted 8 lines

Quoted 8 lines
Quoted 6 lines
<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 <[email protected]>
Acked-by Stefan Roese <[email protected]>

for npe drivers

Acked-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Kim Phillips

2008-01-09 15:18 UTC #2 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Stefan Roese

Quoted 6 lines · On Wed, 9 Jan 2008 05:58:33 +0100 Stefan Roese <[email protected]> wrote:

Quoted 5 lines · Ben Warren
Change return values of init() functions in all Ethernet drivers to conform
to the following:
	>=0: Success

Quoted 8 lines

Quoted 6 lines
<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 <[email protected]>
Acked-by Stefan Roese <[email protected]>

for both qe and tsec based devices:

Acked-by Kim Phillips <[email protected]>

this should be applied asap

Kim

Timur Tabi

2008-01-09 16:14 UTC #11 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Ben Warren

Quoted 10 lines · 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 <[email protected]>

Acked-by Timur Tabi <[email protected]>

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

Signature · 2 lines

Timur Tabi
Linux kernel developer at Freescale

Haavard Skinnemoen

2008-01-09 16:26 UTC #12 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Ben Warren

Quoted 9 lines · On Tue, 08 Jan 2008 17:23:21 -0500 Ben Warren <[email protected]> wrote:

drivers/net/macb.c +0 -0

index 95cdc49..6657d22 100644
@@ -423,12 +423,12 @@ static int macb_init(struct eth_device *netdev,

bd_t *bd) #endif

drivers/net/macb.c +0 -1

 	if (!macb_phy_init(macb))
-		return 0;

drivers/net/macb.c static int macb_init(struct eth_device *netdev, bd_t *bd) · line 434

Quoted 10 lines

drivers/net/macb.c +2 -1

+		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 <[email protected]>

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

Haavard

Ben Warren

author

2008-01-09 17:01 UTC #4 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Kim Phillips

Wolfgang,

Quoted 21 lines · Kim Phillips

Quoted 7 lines · On Wed, 9 Jan 2008 05:58:33 +0100 Stefan Roese <[email protected]> wrote:
Quoted 5 lines · Ben Warren
Change return values of init() functions in all Ethernet drivers to conform
to the following:
	>=0: Success
Quoted 9 lines
Quoted 7 lines
<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 <[email protected]>
Acked-by Stefan Roese <[email protected]>

Quoted 9 lines

for both qe and tsec based devices:
Acked-by Kim Phillips <[email protected]>
this should be applied asap
Kim

Please apply directly.

regards,
Ben

Ulf Samuelsson

2008-01-09 18:26 UTC #13 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Haavard Skinnemoen

Quoted 14 lines

Quoted 11 lines · On Tue, 08 Jan 2008 17:23:21 -0500 Ben Warren <[email protected]> wrote:
drivers/net/macb.c +0 -0
index 95cdc49..6657d22 100644
@@ -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;

Quoted 7 lines

Quoted 7 lines
/* 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

Wolfgang Denk

2008-01-09 21:30 UTC #5 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Ben Warren

Dear Ben,

Quoted 25 lines · in message <[email protected]> you wrote:

Quoted 23 lines · Kim Phillips
Quoted 7 lines · On Wed, 9 Jan 2008 05:58:33 +0100 Stefan Roese <[email protected]> wrote:
Quoted 5 lines · Ben Warren
Change return values of init() functions in all Ethernet drivers to conform
to the following:
	>=0: Success
Quoted 9 lines
Quoted 7 lines
<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 <[email protected]>
Acked-by Stefan Roese <[email protected]>
for both qe and tsec based devices:

Quoted 8 lines

Quoted 7 lines
Acked-by Kim Phillips <[email protected]>
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

Signature · 5 lines

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

Ben Warren

author

2008-01-09 21:37 UTC #6 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

Quoted 43 lines · Wolfgang Denk

Dear Ben,
Quoted 30 lines · in message <[email protected]> you wrote:
Quoted 28 lines · Kim Phillips
Quoted 10 lines · On Wed, 9 Jan 2008 05:58:33 +0100 Stefan Roese <[email protected]> wrote:
Quoted 7 lines · Ben Warren
Change return values of init() functions in all Ethernet drivers to conform
to the following:
	>=0: Success

	<0:  Failure
Quoted 9 lines
Quoted 6 lines
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 <[email protected]>
Acked-by Stefan Roese <[email protected]>
for both qe and tsec based devices:
Acked-by Kim Phillips <[email protected]>
Quoted 8 lines
Quoted 6 lines
this should be applied asap
Kim
Please apply directly.

Quoted 8 lines

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

Jean-Christophe PLAGNIOL-VILLARD

2008-01-09 21:50 UTC #7 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

Quoted 11 lines · Ben Warren

Quoted 6 lines · Wolfgang Denk
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.

Wolfgang Denk

2008-01-09 22:01 UTC #14 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Ulf Samuelsson

Quoted 2 lines · In message <[email protected]> you wrote:

Why not use symbolic return values?

Because then nobody knows what the symbols mean.

Quoted 2 lines

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

Quoted 9 lines

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

Signature · 5 lines

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

Ben Warren

author

2008-01-09 22:19 UTC #8 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Jean-Christophe PLAGNIOL-VILLARD

Quoted 13 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 11 lines · Ben Warren
Quoted 7 lines · Wolfgang Denk
Dear Ben,
Best regards,
Wolfgang Denk
Yes, the original V2 patch, not the one before it since it missed QE.

Quoted 9 lines

Quoted 3 lines
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 <[email protected]>

Acked-by Stefan Roese <[email protected]>

Acked-by Jean-Christophe PLAGNIOL-VILLARD <[email protected]>

Acked-by Kim Phillips <[email protected]>

Acked-by Haavard Skinnemoen <[email protected]>

Acked-by Timur Tabi <[email protected]>

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

cpu/ixp/npe/npe.c +4 -4

index 7e4af44..a33b956 100644
@@ -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) */

cpu/mpc8xx/fec.c +3 -3

index 08a3715..1fd4d4e 100644
@@ -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;
 }

drivers/net/dc2114x.c +2 -2

index d5275dc..7238922 100644
@@ -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)

drivers/net/eepro100.c +2 -2

index 738146e..96ed271 100644
@@ -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;

drivers/net/macb.c +2 -2

index 95cdc49..6657d22 100644
@@ -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)

drivers/net/pcnet.c +2 -2

index 2af0e8f..4e270c9 100644
@@ -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)

drivers/net/rtl8139.c +2 -2

index 2367180..4c24805 100644
@@ -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. */

drivers/net/rtl8169.c +3 -2

index 63ea2cc..0c9a401 100644
@@ -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;
 }
 
 /**************************************************************************

drivers/net/tsec.c +1 -1

index ca6284b..c1a2f07 100644
@@ -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);
 
 }

drivers/net/tsi108_eth.c +2 -2

index 524e9da..a09115e 100644
@@ -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;
 }
 
 /*

drivers/net/uli526x.c +3 -3

index 1267c57..8460f69 100644
@@ -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)

drivers/qe/uec.c +3 -3

index dc2765b..26cfa2b 100644
@@ -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)

net/eth.c +1 -1

index 3373a05..92a91a1 100644
@@ -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;

Signature · 1 line

1.5.2.5

Wolfgang Denk

2008-01-09 22:43 UTC #9 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Ben Warren

Dear Ben,

Quoted 3 lines · in message <[email protected]> you wrote:

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

...

Quoted 1 line

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

Signature · 6 lines

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

Ben Warren

author

2008-01-09 22:48 UTC #10 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

Quoted 30 lines · Wolfgang Denk

Dear Ben,
Quoted 3 lines · in message <[email protected]> you wrote:
Quoted 3 lines
Which one? The original "[PATCH V2] Fix Ethernet init() return codes" patch?
...
Quoted 2 lines
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.

Quoted 7 lines

Sorry...
Best regards,
Wolfgang Denk

Will do

Ulf Samuelsson

2008-01-10 00:11 UTC #15 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Wolfgang Denk

Quoted 18 lines

Quoted 2 lines · In message <[email protected]> you wrote:
Why not use symbolic return values?
Because then nobody knows what the symbols mean.
Quoted 2 lines
"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...
Quoted 4 lines
if(myfunction() == SUCCESS) {
}

Quoted 9 lines

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

Haavard Skinnemoen

2008-01-10 08:01 UTC #16 lore raw

PATCH v2 1/1 Fix Ethernet init() return codes.

In reply to Haavard Skinnemoen

Quoted 1 line · On Wed, 9 Jan 2008 17:26:28 +0100 Haavard Skinnemoen <[email protected]>…

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

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

Haavard