* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
@ 2012-07-09 18:52 Timur Tabi
2012-07-10 1:54 ` Marek Vasut
2012-07-10 15:54 ` Joe Hershberger
0 siblings, 2 replies; 7+ messages in thread
From: Timur Tabi @ 2012-07-09 18:52 UTC (permalink / raw)
To: u-boot
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 <timur@freescale.com>
Acked-by: Nobuhiro Iwamamatsu <nobuhiro.iwamatsu.yj@renesas.com> (sh_eth part)
Acked-by: Stephan Linz <linz@li-pro.net> (Xilinx part, xilinx_axi_emac and xilinx_ll_temac)
Reviewed-by: Marek Vasut <marex@denx.de> (FEC part)
---
drivers/net/fec_mxc.c | 8 +++++++-
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, 39 insertions(+), 6 deletions(-)
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);
+
+ 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);
+ 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");
+ return ret;
+ }
val = 0;
diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
index 3c1c8f0..f5e314b 100644
--- a/drivers/net/tsec.c
+++ b/drivers/net/tsec.c
@@ -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);
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",
+ phydev->dev->name);
+ return 0;
+ }
switch (phydev->speed) {
case 1000:
diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c
index 27dafc1..b67153b 100644
--- a/drivers/net/xilinx_ll_temac.c
+++ b/drivers/net/xilinx_ll_temac.c
@@ -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);
--
1.7.3.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
2012-07-09 18:52 [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails Timur Tabi
@ 2012-07-10 1:54 ` Marek Vasut
2012-07-10 15:54 ` Joe Hershberger
1 sibling, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2012-07-10 1:54 UTC (permalink / raw)
To: u-boot
Dear Timur Tabi,
> 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 <timur@freescale.com>
> Acked-by: Nobuhiro Iwamamatsu <nobuhiro.iwamatsu.yj@renesas.com> (sh_eth
> part) Acked-by: Stephan Linz <linz@li-pro.net> (Xilinx part,
> xilinx_axi_emac and xilinx_ll_temac) Reviewed-by: Marek Vasut
> <marex@denx.de> (FEC part)
> ---
> drivers/net/fec_mxc.c | 8 +++++++-
> 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, 39 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/fec_mxc.c b/drivers/net/fec_mxc.c
> index eee41d7..5700552 100644
A whining remark ... please keep changelog in here ;-)
[...]
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
2012-07-09 18:52 [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails Timur Tabi
2012-07-10 1:54 ` Marek Vasut
@ 2012-07-10 15:54 ` Joe Hershberger
2012-07-10 16:15 ` Timur Tabi
1 sibling, 1 reply; 7+ messages in thread
From: Joe Hershberger @ 2012-07-10 15:54 UTC (permalink / raw)
To: u-boot
Hi Timur Tabi,
On Mon, Jul 9, 2012 at 1:52 PM, Timur Tabi <timur@freescale.com> wrote:
> 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 <timur@freescale.com>
> Acked-by: Nobuhiro Iwamamatsu <nobuhiro.iwamatsu.yj@renesas.com> (sh_eth part)
> Acked-by: Stephan Linz <linz@li-pro.net> (Xilinx part, xilinx_axi_emac and xilinx_ll_temac)
> Reviewed-by: Marek Vasut <marex@denx.de> (FEC part)
> ---
> drivers/net/fec_mxc.c | 8 +++++++-
> 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, 39 insertions(+), 6 deletions(-)
>
> 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?
> + 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?
> + 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?
> + return ret;
> + }
>
> val = 0;
>
> diff --git a/drivers/net/tsec.c b/drivers/net/tsec.c
> index 3c1c8f0..f5e314b 100644
> --- a/drivers/net/tsec.c
> +++ b/drivers/net/tsec.c
> @@ -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);
>
> 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?
> + phydev->dev->name);
> + return 0;
Why are you returning 0 here and not the return value from phy_startup()?
> + }
>
> switch (phydev->speed) {
> case 1000:
> diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c
> index 27dafc1..b67153b 100644
> --- a/drivers/net/xilinx_ll_temac.c
> +++ b/drivers/net/xilinx_ll_temac.c
> @@ -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);
-Joe
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
2012-07-10 15:54 ` Joe Hershberger
@ 2012-07-10 16:15 ` Timur Tabi
2012-07-10 18:30 ` Joe Hershberger
0 siblings, 1 reply; 7+ messages in thread
From: Timur Tabi @ 2012-07-10 16:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
2012-07-10 16:15 ` Timur Tabi
@ 2012-07-10 18:30 ` Joe Hershberger
2012-07-10 18:35 ` Timur Tabi
0 siblings, 1 reply; 7+ messages in thread
From: Joe Hershberger @ 2012-07-10 18:30 UTC (permalink / raw)
To: u-boot
Hi Timur Tabi,
On Tue, Jul 10, 2012 at 11:15 AM, Timur Tabi <timur@freescale.com> wrote:
> 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.
Should you not at least keep the core message the same? "Could not
initialize PHY"
>>
>>> + 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().
Same here, at least the core message "Could not initialize PHY"
>>> 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. :-)
OK... I'm not sold, but I don't care enough.
>>
>>> + 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.
OK.
Thanks,
-Joe
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
2012-07-10 18:30 ` Joe Hershberger
@ 2012-07-10 18:35 ` Timur Tabi
2012-07-10 18:36 ` Joe Hershberger
0 siblings, 1 reply; 7+ messages in thread
From: Timur Tabi @ 2012-07-10 18:35 UTC (permalink / raw)
To: u-boot
Joe Hershberger wrote:
>>>> @@ -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.
>
> Should you not at least keep the core message the same? "Could not
> initialize PHY"
Well, I suppose I could add the word "PHY" here.
>>>> 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().
>
> Same here, at least the core message "Could not initialize PHY"
I am saying that, just in a different way. "phy startup failure" means
the same as "Could not initialize PHY", but the wording matches the rest
of the function.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails
2012-07-10 18:35 ` Timur Tabi
@ 2012-07-10 18:36 ` Joe Hershberger
0 siblings, 0 replies; 7+ messages in thread
From: Joe Hershberger @ 2012-07-10 18:36 UTC (permalink / raw)
To: u-boot
On Tue, Jul 10, 2012 at 1:35 PM, Timur Tabi <timur@freescale.com> wrote:
> Joe Hershberger wrote:
>
>>>>> 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().
>>
>> Same here, at least the core message "Could not initialize PHY"
>
> I am saying that, just in a different way. "phy startup failure" means
> the same as "Could not initialize PHY", but the wording matches the rest
> of the function.
OK.
-Joe
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-07-10 18:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-09 18:52 [U-Boot] [PATCH 2/2] [v3] net: abort network initialization if the PHY driver fails Timur Tabi
2012-07-10 1:54 ` Marek Vasut
2012-07-10 15:54 ` Joe Hershberger
2012-07-10 16:15 ` Timur Tabi
2012-07-10 18:30 ` Joe Hershberger
2012-07-10 18:35 ` Timur Tabi
2012-07-10 18:36 ` Joe Hershberger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.