Joe Hershberger wrote: >> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c >> index eee41d7..5700552 100644 >> --- a/drivers/net/fec_mxc.c >> +++ b/drivers/net/fec_mxc.c >> @@ -510,7 +510,13 @@ 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); >> + > > Why a blank line here when it isn't in the rest of the implementations?
The coding standard requires blank lines after variable declarations. >> + if (ret) { >> + printf("Could not initialize PHY %s\n", >> + fec->phydev->dev->name); >> + return ret; >> + } >> speed = fec->phydev->speed; >> } else { >> speed = _100BASET; >> diff --git a/drivers/net/fm/eth.c b/drivers/net/fm/eth.c >> index f34f4db..2b616ad 100644 >> --- a/drivers/net/fm/eth.c >> +++ b/drivers/net/fm/eth.c >> @@ -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); > > Why is this string different from the others? Consistency? Yes. I tried to keep the messages consistent with the other messages in the function. > >> + return ret; >> + } >> #else >> fm_eth->phydev->speed = SPEED_1000; >> fm_eth->phydev->link = 1; >> diff --git a/drivers/net/sh_eth.c b/drivers/net/sh_eth.c >> index bb57e4d..268d884 100644 >> --- a/drivers/net/sh_eth.c >> +++ b/drivers/net/sh_eth.c >> @@ -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"); > > Why is this string different from the others? Consistency? Yes, it looks like the other messages in sh_eth_config(). >> diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c >> index 7854a04..d777144 100644 >> --- a/drivers/net/xilinx_axi_emac.c >> +++ b/drivers/net/xilinx_axi_emac.c >> @@ -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", > > Lower-case "could" in string? Consistency? Yes, consistency. :-) > >> + phydev->dev->name); >> + return 0; > > Why are you returning 0 here and not the return value from phy_startup()? I answered that in another email. This is what setup_phy() is supposed to return on failure. -- Timur Tabi Linux kernel developer at Freescale _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot