phylib: phy_startup() should return an error code on failure
Subsystems (3)
PATCH 1/2 phylib: phy_startup() should return an error code on failure
phy_startup() calls the PHY driver's startup function, but it ignores the return code from that function, and so it never returns any failures.
Signed-off-by Timur Tabi <[email protected]>
--- drivers/net/phy/phy.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)
drivers/net/phy/phy.c +4 -1
@@ -723,10 +723,13 @@ struct phy_device *phy_connect(struct mii_dev *bus, int addr,return phydev; } +/* +* Start the PHY. Returns 0 on success, or a negative error code. +*/ int phy_startup (struct phy_device *phydev) {if (phydev->drv->startup) - phydev->drv->startup (phydev); +return phydev->drv->startup (phydev);return 0 ; }
Signature · 1 line
1.7.3.4
PATCH 2/2 net: abort network initialization if the PHY driver fails
Now that phy_startup() can return an actual error code, check for that error code and abort network initialization if the PHY fails.
Signed-off-by Timur Tabi <[email protected]>
--- drivers/net/fec_mxc.c | 7 ++++++- drivers/net/fm/eth.c | 9 ++++++++- drivers/net/sh_eth.c | 6 +++++- drivers/net/tsec.c | 8 +++++++- drivers/net/xilinx_axi_emac.c | 6 +++++- drivers/net/xilinx_ll_temac.c | 8 +++++++- 6 files changed, 38 insertions(+), 6 deletions(-)
drivers/net/fec_mxc.c +6 -1
@@ -510,7 +510,12 @@ static int fec_open(struct eth_device *edev)fec_eth_phy_config (edev);if (fec->phydev) {/* Start up the PHY */ -phy_startup (fec->phydev); +int ret =phy_startup (fec->phydev); +if (ret) { +printf ("Could not initialize PHY %s\n" , + fm_eth->phydev->dev->name); +return ret; + } speed = fec->phydev->speed; }else { speed = _100BASET;
drivers/net/fm/eth.c +8 -1
@@ -363,6 +363,9 @@ static int fm_eth_open(struct eth_device *dev, bd_t *bd) {struct fm_eth *fm_eth;struct fsl_enet_mac *mac; +#ifdef CONFIG_PHYLIB +int ret; +#endif fm_eth = (struct fm_eth *)dev->priv; mac = fm_eth->mac;
@@ -384,7 +387,11 @@ static int fm_eth_open(struct eth_device *dev, bd_t *bd)fmc_tx_port_graceful_stop_disable (fm_eth);#ifdef CONFIG_PHYLIB -phy_startup (fm_eth->phydev); + ret =phy_startup (fm_eth->phydev); +if (ret) { +printf ("%s: Could not initialize\n" , fm_eth->phydev->dev->name); +return ret; + }#else fm_eth->phydev->speed =SPEED_1000 ; fm_eth->phydev->link =1 ;
drivers/net/sh_eth.c +5 -1
@@ -415,7 +415,11 @@ static int sh_eth_config(struct sh_eth_dev *eth, bd_t *bd) goto err_phy_cfg; } phy = port_info->phydev; -phy_startup (phy); + ret =phy_startup (phy); +if (ret) { +printf (SHETHER_NAME ": phy startup failure\n" ); +return ret; + } val =0 ;
drivers/net/tsec.c +7 -1
@@ -480,6 +480,7 @@ static int tsec_init(struct eth_device *dev, bd_t * bd)int i;struct tsec_private *priv = (struct tsec_private *)dev->priv;tsec_t *regs = priv->regs; +int ret;/* Make sure the controller is stopped */ tsec_halt (dev);
@@ -511,7 +512,12 @@ static int tsec_init(struct eth_device *dev, bd_t * bd)startup_tsec (dev);/* Start up the PHY */ -phy_startup (priv->phydev); + ret =phy_startup (priv->phydev); +if (ret) { +printf ("Could not initialize PHY %s\n" , + priv->phydev->dev->name); +return ret; + }adjust_link (priv, priv->phydev);
drivers/net/xilinx_axi_emac.c +5 -1
@@ -272,7 +272,11 @@ static int setup_phy(struct eth_device *dev) phydev->advertising = phydev->supported; priv->phydev = phydev;phy_config (phydev); -phy_startup (phydev); +if (phy_startup (phydev)) { +printf ("axiemac: could not initialize PHY %s\n" , + phydev->dev->name); +return 0 ; + }switch (phydev->speed) {case 1000 :
drivers/net/xilinx_ll_temac.c +7 -1
@@ -232,6 +232,7 @@ static void ll_temac_halt(struct eth_device *dev)static int ll_temac_init (struct eth_device *dev,bd_t *bis) {struct ll_temac *ll_temac = dev->priv; +int ret;printf ("%s: Xilinx XPS LocalLink Tri-Mode Ether MAC #%d at 0x%08X.\n" , dev->name, dev->index, dev->iobase);
@@ -240,7 +241,12 @@ static int ll_temac_init(struct eth_device *dev, bd_t *bis)return -1 ;/* Start up the PHY */ -phy_startup (ll_temac->phydev); + ret =phy_startup (ll_temac->phydev); +if (ret) { +printf ("%s: Could not initialize PHY %s\n" , + dev->name, ll_temac->phydev->dev->name); +return ret; + }if (!ll_temac_adjust_link (dev)) {ll_temac_halt (dev);
Signature · 1 line
1.7.3.4
PATCH 2/2 net: abort network initialization if the PHY driver fails
Hi,
2012/7/6 Timur Tabi <[email protected]>:
Quoted 18 lines
Now that
phy_startup()can return an actual error code, check for that error code and abort network initialization if the PHY fails.Signed-off-by Timur Tabi <[email protected]>--- drivers/net/fec_mxc.c | 7 ++++++- drivers/net/fm/eth.c | 9 ++++++++- drivers/net/sh_eth.c | 6 +++++- drivers/net/tsec.c | 8 +++++++- drivers/net/xilinx_axi_emac.c | 6 +++++- drivers/net/xilinx_ll_temac.c | 8 +++++++- 6 files changed, 38 insertions(+), 6 deletions(-)drivers/net/fec_mxc.c +0
-0
drivers/net/fec_mxc.c static int fec_open(struct eth_device *edev) · line 516
Quoted 8 lines
drivers/net/fec_mxc.c +4
-1@@ -510,7 +510,12 @@ static int fec_open(struct eth_device *edev)fec_eth_phy_config (edev);if (fec->phydev) {/* Start up the PHY */ -phy_startup (fec->phydev); +int ret =phy_startup (fec->phydev); +if (ret) { +printf ("Could not initialize PHY %s\n" , + fm_eth->phydev->dev->name);
Probably this is fec, not fm_eth.
Quoted 82 lines
drivers/net/fec_mxc.c +2
-0+return ret; + } speed = fec->phydev->speed; }else { speed = _100BASET;drivers/net/fm/eth.c +8
-1@@ -363,6 +363,9 @@ static int fm_eth_open(struct eth_device *dev, bd_t *bd) {struct fm_eth *fm_eth;struct fsl_enet_mac *mac; +#ifdef CONFIG_PHYLIB +int ret; +#endif fm_eth = (struct fm_eth *)dev->priv; mac = fm_eth->mac;@@ -384,7 +387,11 @@ static int fm_eth_open(struct eth_device *dev, bd_t *bd)fmc_tx_port_graceful_stop_disable (fm_eth);#ifdef CONFIG_PHYLIB -phy_startup (fm_eth->phydev); + ret =phy_startup (fm_eth->phydev); +if (ret) { +printf ("%s: Could not initialize\n" , fm_eth->phydev->dev->name); +return ret; + }#else fm_eth->phydev->speed =SPEED_1000 ; fm_eth->phydev->link =1 ;drivers/net/sh_eth.c +5
-1@@ -415,7 +415,11 @@ static int sh_eth_config(struct sh_eth_dev *eth, bd_t *bd) goto err_phy_cfg; } phy = port_info->phydev; -phy_startup (phy); + ret =phy_startup (phy); +if (ret) { +printf (SHETHER_NAME ": phy startup failure\n" ); +return ret; + } val =0 ;drivers/net/tsec.c +7
-1@@ -480,6 +480,7 @@ static int tsec_init(struct eth_device *dev, bd_t * bd)int i;struct tsec_private *priv = (struct tsec_private *)dev->priv;tsec_t *regs = priv->regs; +int ret;/* Make sure the controller is stopped */ tsec_halt (dev);@@ -511,7 +512,12 @@ static int tsec_init(struct eth_device *dev, bd_t * bd)startup_tsec (dev);/* Start up the PHY */ -phy_startup (priv->phydev); + ret =phy_startup (priv->phydev); +if (ret) { +printf ("Could not initialize PHY %s\n" , + priv->phydev->dev->name); +return ret; + }adjust_link (priv, priv->phydev);drivers/net/xilinx_axi_emac.c +0
-0@@ -272,7 +272,11 @@ static int setup_phy(struct eth_device *dev) phydev->advertising = phydev->supported; priv->phydev = phydev;
drivers/net/xilinx_axi_emac.c static int setup_phy(struct eth_device *dev) · line 282
Quoted 10 lines
drivers/net/xilinx_axi_emac.c +5
-1phy_config (phydev); -phy_startup (phydev); +if (phy_startup (phydev)) { +printf ("axiemac: could not initialize PHY %s\n" , + phydev->dev->name); +return 0 ; + }switch (phydev->speed) {case 1000 :
Why doesn't this driver use a ret variable?
Other driver's patch used this.
Quoted 27 lines
drivers/net/xilinx_ll_temac.c +7
-2@@ -232,6 +232,7 @@ static void ll_temac_halt(struct eth_device *dev)static int ll_temac_init (struct eth_device *dev,bd_t *bis) {struct ll_temac *ll_temac = dev->priv; +int ret;printf ("%s: Xilinx XPS LocalLink Tri-Mode Ether MAC #%d at 0x%08X.\n" , dev->name, dev->index, dev->iobase);@@ -240,7 +241,12 @@ static int ll_temac_init(struct eth_device *dev, bd_t *bis)return -1 ;/* Start up the PHY */ -phy_startup (ll_temac->phydev); + ret =phy_startup (ll_temac->phydev); +if (ret) { +printf ("%s: Could not initialize PHY %s\n" , + dev->name, ll_temac->phydev->dev->name); +return ret; + }if (!ll_temac_adjust_link (dev)) {ll_temac_halt (dev); --
Quoted 2 lines
1.7.3.4
I acked sh_eth's part. Thank you.
Best regards,
Nobuhiro
Signature · 1 line
Nobuhiro Iwamatsu
PATCH 2/2 net: abort network initialization if the PHY driver fails
Quoted 7 lines · Nobuhiro Iwamatsu
Quoted 7 lines
drivers/net/fec_mxc.c +0
-0@@ -510,7 +510,12 @@ static int fec_open(struct eth_device *edev)fec_eth_phy_config (edev);if (fec->phydev) {
Quoted 8 lines
drivers/net/fec_mxc.c static int fec_open(struct eth_device *edev) · line 516
Quoted 6 lines
drivers/net/fec_mxc.c +4
-1/* Start up the PHY */ -phy_startup (fec->phydev); +int ret =phy_startup (fec->phydev); +if (ret) { +printf ("Could not initialize PHY %s\n" , + fm_eth->phydev->dev->name);Probably this is fec, not
fm_eth.
Thanks. I thought I was being diligent, and I ran MAKEALL, but I guess it wasn't enough.
Quoted 15 lines
Quoted 15 lines
drivers/net/xilinx_axi_emac.c +5
-1@@ -272,7 +272,11 @@ static int setup_phy(struct eth_device *dev) phydev->advertising = phydev->supported; priv->phydev = phydev;phy_config (phydev); -phy_startup (phydev); +if (phy_startup (phydev)) { +printf ("axiemac: could not initialize PHY %s\n" , + phydev->dev->name); +return 0 ; + }
Quoted 5 lines
drivers/net/xilinx_axi_emac.c static int setup_phy(struct eth_device *dev) · line 282
Quoted 2 lines
drivers/net/xilinx_axi_emac.c +0
-0switch (phydev->speed) {case 1000 :Why doesn't this driver use a ret variable?
Other driver's patch used this.
Because setup_phy() returns either 0 (failure) or 1 (success).
Signature · 2 lines
Timur Tabi Linux kernel developer at Freescale
Joe Hershberger
AppliedPATCH 1/2 phylib: phy_startup() should return an error code on failure
In reply to Timur TabiHi Timur Tabi,
Quoted 5 lines · Timur Tabi
phy_startup()calls the PHY driver's startup function, but it ignores the return code from that function, and so it never returns any failures.Signed-off-by Timur Tabi <[email protected]>---
Applied to next, thanks.
-Joe