[U-Boot-Users] [PATCH V2] 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 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;
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@denx.de =====================================================================
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
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
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
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
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
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.
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;
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
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
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.
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
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
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
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
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
participants (8)
-
Ben Warren -
Haavard Skinnemoen -
Jean-Christophe PLAGNIOL-VILLARD -
Kim Phillips -
Stefan Roese -
Timur Tabi -
Ulf Samuelsson -
Wolfgang Denk