[RFC] net/fs_enet: send a reset request to the PHY on init

Usually u-boot sends a phy request in its network init routine. An uboot
without network support doesn't do it and I endup without working
network. I still can switch between 10/100Mbit (according to the LED on
the hub and phy registers) but I can't send or receive any data.

At this point I'm not sure if the PowerON Reset takes the PHY a few
nsecs too early out of reset or if this reset is required and everyone
relies on U-boot performing this reset.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
This is done on a custom mpc512x board. Unfortunately I don't have other
boards to check. The PHY is a AMD Am79C874, phylib uses the generic one.

 drivers/net/fs_enet/fs_enet-main.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

On Wed, Sep 2, 2009 at 5:04 AM, Sebastian Andrzej
Siewior<bigeasy@linutronix.de> wrote:
> Usually u-boot sends a phy request in its network init routine. An uboot
> without network support doesn't do it and I endup without working
> network. I still can switch between 10/100Mbit (according to the LED on
> the hub and phy registers) but I can't send or receive any data.
>
> At this point I'm not sure if the PowerON Reset takes the PHY a few
> nsecs too early out of reset or if this reset is required and everyone
> relies on U-boot performing this reset.
>
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> This is done on a custom mpc512x board. Unfortunately I don't have other
> boards to check. The PHY is a AMD Am79C874, phylib uses the generic one.
>
>  drivers/net/fs_enet/fs_enet-main.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c
> index ee15402..a3c962b 100644
> --- a/drivers/net/fs_enet/fs_enet-main.c
> +++ b/drivers/net/fs_enet/fs_enet-main.c
> @@ -823,7 +823,8 @@ static int fs_init_phy(struct net_device *dev)
>        }
>
>        fep->phydev = phydev;
> -
> +       phy_write(phydev, MII_BMCR, BMCR_RESET);
> +       udelay(1);

What version of the kernel are you using?  The line numbers don't
match up with kernel mainline, so I wonder if this is before or after
the OF MDIO rework changes.

Regardless, this doesn't look right.  It certainly isn't right for the
driver to do an unconditional PHY reset when it doesn't actually know
what phy is attached.  For most boards I'm sure this is not desirable
because it will cause a delay while the PHY auto negotiates.
Depending on when the first network traffic begins, can cause several
seconds of boot delay.

Best would be to do this in U-Boot.  Otherwise, I think I would rather
see it at phy_device probe time.  At least then it would be on a
per-phy basis, or could be controlled by a property in the device tree
so that all boards don't get the same impact.

g.

Grant Likely wrote:
  > What version of the kernel are you using?  The line numbers don't
> match up with kernel mainline, so I wonder if this is before or after
> the OF MDIO rework changes.
It is the kernel which was shipped in ads5121's bsp which is 2.6.24.

> Regardless, this doesn't look right.  It certainly isn't right for the
> driver to do an unconditional PHY reset when it doesn't actually know
> what phy is attached.  For most boards I'm sure this is not desirable
> because it will cause a delay while the PHY auto negotiates.
> Depending on when the first network traffic begins, can cause several
> seconds of boot delay.
> 
> Best would be to do this in U-Boot.  Otherwise, I think I would rather
> see it at phy_device probe time.  At least then it would be on a
> per-phy basis, or could be controlled by a property in the device tree
> so that all boards don't get the same impact.
I have no network support in boot loader so I can't do it there. Doing it 
at phy-probe time sounds reasonable.
So all other boards are doing this kind of reset in u-boot?

> g.
> 


Sebastian

On Fri, Sep 4, 2009 at 9:38 AM, Sebastian Andrzej
Siewior<bigeasy@linutronix.de> wrote:
> Grant Likely wrote:
>  > What version of the kernel are you using?  The line numbers don't
>>
>> match up with kernel mainline, so I wonder if this is before or after
>> the OF MDIO rework changes.
>
> It is the kernel which was shipped in ads5121's bsp which is 2.6.24.

Okay, I can safely ignore this then.  Wolfgang may be interested
though.  He's been doing some work to get 5121 support mainlined.

> I have no network support in boot loader so I can't do it there. Doing it at
> phy-probe time sounds reasonable.
> So all other boards are doing this kind of reset in u-boot?

In general I take the approach that as much as possible firmware
should have devices in a sane state before booting the kernel just to
avoid doing board specific fixup stuff in the kernel tree.  But this
isn't law, just more of a rule of thumb that I go by.  2nd resort is
to create a board specific platform code file and put it there
(arch/powerpc/platforms/*).

g.