[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Joakim Tjernlund]

Instead of always performing an autoneg, check if the PHY
already has a link and if it matches one of the requested
modes. Initially only 100MbFD is optimized this way.

Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se>
---
 drivers/qe/uec_phy.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c
index 3baffe4..5237960 100644
--- a/drivers/qe/uec_phy.c
+++ b/drivers/qe/uec_phy.c
@@ -351,6 +351,15 @@ static int marvell_config_aneg (struct uec_mii_info *mii_info)
 static int genmii_config_aneg (struct uec_mii_info *mii_info)
 {
 	if (mii_info->autoneg) {
+		/* Speed up the common case, if link is already up, speed and
+		   duplex match, skip auto neg as it already matches */
+		if (!genmii_read_status(mii_info) && mii_info->link)
+			if (mii_info->duplex == DUPLEX_FULL &&
+			    mii_info->speed == SPEED_100)
+				if (mii_info->advertising &
+				    ADVERTISED_100baseT_Full)
+					return 0;
+
 		config_genmii_advert (mii_info);
 		genmii_restart_aneg (mii_info);
 	} else
-- 
1.7.1

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Mike Frysinger]

On Tue, Aug 10, 2010 at 10:36 AM, Joakim Tjernlund wrote:
> Instead of always performing an autoneg, check if the PHY
> already has a link and if it matches one of the requested
> modes. Initially only 100MbFD is optimized this way.

wish the common net/miiphy layer had similar functionality ...
-mike

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Joakim Tjernlund]

vapierfilter at gmail.com wrote on 2010/08/10 22:23:19:
>
> On Tue, Aug 10, 2010 at 10:36 AM, Joakim Tjernlund wrote:
> > Instead of always performing an autoneg, check if the PHY
> > already has a link and if it matches one of the requested
> > modes. Initially only 100MbFD is optimized this way.
>
> wish the common net/miiphy layer had similar functionality ...
> -mike

Thanks, too bad that there is a long way left to common
PHY code ..

 Jocke

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Detlev Zundel]

Hi Jocke,

> Instead of always performing an autoneg, check if the PHY
> already has a link and if it matches one of the requested
> modes. Initially only 100MbFD is optimized this way.

Isn't it about time that we think about _not_ stopping the ethernet
device after every transaction?

Why not initialize it for the first transfer and stop it once we boot a
kernel?  For USB for example we do just that.

Sorry for not providing a concrete patch, but I felt it is worthwhile to
throw that thought into the discussion as it pops up more and more often
nowadays.  For example usb-cdc which currently deregisters the network
device on the host side after _every_ transfer....

Cheers
  Detlev

-- 
You live and learn
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Joakim Tjernlund]

Detlev Zundel <dzu@denx.de> wrote on 2010/08/12 14:58:47:
>
> Hi Jocke,
>
> > Instead of always performing an autoneg, check if the PHY
> > already has a link and if it matches one of the requested
> > modes. Initially only 100MbFD is optimized this way.
>
> Isn't it about time that we think about _not_ stopping the ethernet
> device after every transaction?

Hi Detlev

UEC does this already, my patch was to address the initial delay
you get for the first transaction. Now my PHY based boards gets the link
up just as quick as Fixed PHY for the first transaction.

     Jocke

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Detlev Zundel]

Hi Jocke,

>> > Instead of always performing an autoneg, check if the PHY
>> > already has a link and if it matches one of the requested
>> > modes. Initially only 100MbFD is optimized this way.
>>
>> Isn't it about time that we think about _not_ stopping the ethernet
>> device after every transaction?
>
> Hi Detlev
>
> UEC does this already, my patch was to address the initial delay
> you get for the first transaction. Now my PHY based boards gets the link
> up just as quick as Fixed PHY for the first transaction.

Forgive me to not look into this any deeper, but do I understand you
correctly that you do this by essentially no-oping the eth_halt()
function?  Isn't this then effectively violating what net.c expects the
device to do?

I was thinking that net.c itself should not do this continous start/stop
thing as it has problems on many interfaces.  On one ARM machine I've
again seen problems with the MAC address programming because the
eth_halt() resets the controller and so it forgets its address again.
Also the USB-CDC example where the _whole interface_ on the host side is
being torn down after each tftp transfer prompts me to think along this
line.

So in effect I guess my response was rather a ping to Ben, sorry for
that ;)

Cheers
  Detlev

-- 
Peace of mind isn't at all superficial to technical work.  It's the
whole thing.   That which  produces it is good work  and that which
destroys it is bad work.
                                        -- Robert M. Pirsig
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Joakim Tjernlund]

Detlev Zundel <dzu@denx.de> wrote on 2010/08/13 10:20:46:
>
> Hi Jocke,
>
> >> > Instead of always performing an autoneg, check if the PHY
> >> > already has a link and if it matches one of the requested
> >> > modes. Initially only 100MbFD is optimized this way.
> >>
> >> Isn't it about time that we think about _not_ stopping the ethernet
> >> device after every transaction?
> >
> > Hi Detlev
> >
> > UEC does this already, my patch was to address the initial delay
> > you get for the first transaction. Now my PHY based boards gets the link
> > up just as quick as Fixed PHY for the first transaction.
>
> Forgive me to not look into this any deeper, but do I understand you
> correctly that you do this by essentially no-oping the eth_halt()
> function?  Isn't this then effectively violating what net.c expects the
> device to do?

uec_halt stops the controller as it should. Uec forces an auto_neg
cycle on the PHY the first it is called, after that it just uses
the result from the first auto neg.
My patch simply skip the auto neg. pass iff the PHY already has
a link and duplex&speed already matches what uec wants.

    Jocke

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Ben Warren]

  Hi Detlev,

On 8/13/2010 1:20 AM, Detlev Zundel wrote:
> Hi Jocke,
>
>>>> Instead of always performing an autoneg, check if the PHY
>>>> already has a link and if it matches one of the requested
>>>> modes. Initially only 100MbFD is optimized this way.
>>> Isn't it about time that we think about _not_ stopping the ethernet
>>> device after every transaction?
>> Hi Detlev
>>
>> UEC does this already, my patch was to address the initial delay
>> you get for the first transaction. Now my PHY based boards gets the link
>> up just as quick as Fixed PHY for the first transaction.
> Forgive me to not look into this any deeper, but do I understand you
> correctly that you do this by essentially no-oping the eth_halt()
> function?  Isn't this then effectively violating what net.c expects the
> device to do?
>
> I was thinking that net.c itself should not do this continous start/stop
> thing as it has problems on many interfaces.  On one ARM machine I've
> again seen problems with the MAC address programming because the
> eth_halt() resets the controller and so it forgets its address again.
> Also the USB-CDC example where the _whole interface_ on the host side is
> being torn down after each tftp transfer prompts me to think along this
> line.
>
> So in effect I guess my response was rather a ping to Ben, sorry for
> that ;)
>
Sorry for the delay on this.  I'm all for changing the existing 
behavior.  It seems to me that the only time we would ever want to wind 
an interface down is if we switch the active one (even then, I'm not 
sure).  My world view is limited, but I can't imagine that even changing 
interfaces happens much in real world U-boot usage, that is the non-lab, 
non-interactive use cases.  What would you think about adding something 
like ifup and ifdown commands so that users could explicitly start/stop 
interfaces?
> Cheers
>    Detlev
>
regards,
Ben

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Joakim Tjernlund]

Ben Warren <biggerbadderben@gmail.com> wrote on 2010/08/23 09:08:17:
>
>   Hi Detlev,
>
> On 8/13/2010 1:20 AM, Detlev Zundel wrote:
> > Hi Jocke,
> >
> >>>> Instead of always performing an autoneg, check if the PHY
> >>>> already has a link and if it matches one of the requested
> >>>> modes. Initially only 100MbFD is optimized this way.
> >>> Isn't it about time that we think about _not_ stopping the ethernet
> >>> device after every transaction?
> >> Hi Detlev
> >>
> >> UEC does this already, my patch was to address the initial delay
> >> you get for the first transaction. Now my PHY based boards gets the link
> >> up just as quick as Fixed PHY for the first transaction.
> > Forgive me to not look into this any deeper, but do I understand you
> > correctly that you do this by essentially no-oping the eth_halt()
> > function?  Isn't this then effectively violating what net.c expects the
> > device to do?
> >
> > I was thinking that net.c itself should not do this continous start/stop
> > thing as it has problems on many interfaces.  On one ARM machine I've
> > again seen problems with the MAC address programming because the
> > eth_halt() resets the controller and so it forgets its address again.
> > Also the USB-CDC example where the _whole interface_ on the host side is
> > being torn down after each tftp transfer prompts me to think along this
> > line.
> >
> > So in effect I guess my response was rather a ping to Ben, sorry for
> > that ;)
> >
> Sorry for the delay on this.  I'm all for changing the existing
> behavior.  It seems to me that the only time we would ever want to wind
> an interface down is if we switch the active one (even then, I'm not
> sure).  My world view is limited, but I can't imagine that even changing
> interfaces happens much in real world U-boot usage, that is the non-lab,
> non-interactive use cases.  What would you think about adding something
> like ifup and ifdown commands so that users could explicitly start/stop
> interfaces?

Sure, bringing I/F's up and down needlessly isn't a good thing. However my
patch doesn't change that behaviour. It only optimizes the need for a PHY AN
the first time one performs a eth transaction.

      Jocke

> > Cheers
> >    Detlev
> >
> regards,
> Ben
>
>

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Ben Warren]

Hi Jocke,

On Monday, August 23, 2010, Joakim Tjernlund
<joakim.tjernlund@transmode.se> wrote:
> Ben Warren <biggerbadderben@gmail.com> wrote on 2010/08/23 09:08:17:
>>
>> ? Hi Detlev,
>>
>> On 8/13/2010 1:20 AM, Detlev Zundel wrote:
>> > Hi Jocke,
>> >
>> >>>> Instead of always performing an autoneg, check if the PHY
>> >>>> already has a link and if it matches one of the requested
>> >>>> modes. Initially only 100MbFD is optimized this way.
>> >>> Isn't it about time that we think about _not_ stopping the ethernet
>> >>> device after every transaction?
>> >> Hi Detlev
>> >>
>> >> UEC does this already, my patch was to address the initial delay
>> >> you get for the first transaction. Now my PHY based boards gets the link
>> >> up just as quick as Fixed PHY for the first transaction.
>> > Forgive me to not look into this any deeper, but do I understand you
>> > correctly that you do this by essentially no-oping the eth_halt()
>> > function? ?Isn't this then effectively violating what net.c expects the
>> > device to do?
>> >
>> > I was thinking that net.c itself should not do this continous start/stop
>> > thing as it has problems on many interfaces. ?On one ARM machine I've
>> > again seen problems with the MAC address programming because the
>> > eth_halt() resets the controller and so it forgets its address again.
>> > Also the USB-CDC example where the _whole interface_ on the host side is
>> > being torn down after each tftp transfer prompts me to think along this
>> > line.
>> >
>> > So in effect I guess my response was rather a ping to Ben, sorry for
>> > that ;)
>> >
>> Sorry for the delay on this. ?I'm all for changing the existing
>> behavior. ?It seems to me that the only time we would ever want to wind
>> an interface down is if we switch the active one (even then, I'm not
>> sure). ?My world view is limited, but I can't imagine that even changing
>> interfaces happens much in real world U-boot usage, that is the non-lab,
>> non-interactive use cases. ?What would you think about adding something
>> like ifup and ifdown commands so that users could explicitly start/stop
>> interfaces?
>
> Sure, bringing I/F's up and down needlessly isn't a good thing. However my
> patch doesn't change that behaviour. It only optimizes the need for a PHY AN
> the first time one performs a eth transaction.
>

I know.  I guess my e-mail was more directed towards Detlev's musings.
>  ? ? ?Jocke

Regards,
Ben

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Joakim Tjernlund]

Ben Warren <biggerbadderben@gmail.com> wrote on 2010/08/23 16:12:07:
>
> Hi Jocke,
>
> On Monday, August 23, 2010, Joakim Tjernlund
> <joakim.tjernlund@transmode.se> wrote:
> > Ben Warren <biggerbadderben@gmail.com> wrote on 2010/08/23 09:08:17:
> >>
> >> ? Hi Detlev,
> >>
> >> On 8/13/2010 1:20 AM, Detlev Zundel wrote:
> >> > Hi Jocke,
> >> >
> >> >>>> Instead of always performing an autoneg, check if the PHY
> >> >>>> already has a link and if it matches one of the requested
> >> >>>> modes. Initially only 100MbFD is optimized this way.
> >> >>> Isn't it about time that we think about _not_ stopping the ethernet
> >> >>> device after every transaction?
> >> >> Hi Detlev
> >> >>
> >> >> UEC does this already, my patch was to address the initial delay
> >> >> you get for the first transaction. Now my PHY based boards gets the link
> >> >> up just as quick as Fixed PHY for the first transaction.
> >> > Forgive me to not look into this any deeper, but do I understand you
> >> > correctly that you do this by essentially no-oping the eth_halt()
> >> > function? ?Isn't this then effectively violating what net.c expects the
> >> > device to do?
> >> >
> >> > I was thinking that net.c itself should not do this continous start/stop
> >> > thing as it has problems on many interfaces. ?On one ARM machine I've
> >> > again seen problems with the MAC address programming because the
> >> > eth_halt() resets the controller and so it forgets its address again.
> >> > Also the USB-CDC example where the _whole interface_ on the host side is
> >> > being torn down after each tftp transfer prompts me to think along this
> >> > line.
> >> >
> >> > So in effect I guess my response was rather a ping to Ben, sorry for
> >> > that ;)
> >> >
> >> Sorry for the delay on this. ?I'm all for changing the existing
> >> behavior. ?It seems to me that the only time we would ever want to wind
> >> an interface down is if we switch the active one (even then, I'm not
> >> sure). ?My world view is limited, but I can't imagine that even changing
> >> interfaces happens much in real world U-boot usage, that is the non-lab,
> >> non-interactive use cases. ?What would you think about adding something
> >> like ifup and ifdown commands so that users could explicitly start/stop
> >> interfaces?
> >
> > Sure, bringing I/F's up and down needlessly isn't a good thing. However my
> > patch doesn't change that behaviour. It only optimizes the need for a PHY AN
> > the first time one performs a eth transaction.
> >
>
> I know.  I guess my e-mail was more directed towards Detlev's musings.

Ah, great. You will apply this patch then?

          Jocke

[U-Boot] Start/stop of network devices (was: Re: [PATCH] UEC PHY: Speed up initial PHY neg.)

[Detlev Zundel]

Hi Ben,

[Jocke deleted from CC as this is not about the patch anymore]

> Sorry for the delay on this.  I'm all for changing the existing
> behavior.  It seems to me that the only time we would ever want to
> wind an interface down is if we switch the active one (even then, I'm
> not sure).  My world view is limited, but I can't imagine that even
> changing interfaces happens much in real world U-boot usage, that is
> the non-lab, non-interactive use cases.  What would you think about
> adding something like ifup and ifdown commands so that users could
> explicitly start/stop interfaces?

Actually I was thinking of an automatic initialisiation once we do a
network transfer.  This would be more in line with the current command
line interface.  For this we would need a state variable per device and
start the device if it is down.

The "ifdown" _may_ be implemented as a command, but I think this should
be done automatically once we boot a kernel.  For USB we already have to
do something like this as USB host controllers tend to DMA frame numbers
into memory as long as they are running.  This was an especially
difficult problem to diagnose back then.  From this lesson I think we
should learn and call "stop" methods for every initialized device.

What do you think?

Once more this is going into the "device" model dicussion that we
already thought about a while ago [1].  Maybe someday somebody finds
some time and starts to work on it ;)

Cheers
  Detlev

[1] http://www.denx.de/wiki/view/U-Boot/OLSUbootBOF

-- 
A foolish consistency is the hobgoblin of little minds.
                                   -- Ralph Waldo Emerson             
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] Start/stop of network devices (was: Re: [PATCH] UEC PHY: Speed up initial PHY neg.)

[Joakim Tjernlund]

>
> Hi Ben,
>
> [Jocke deleted from CC as this is not about the patch anymore]

But I still want to know what will happen to it. I didn't
send this patch to start another discussion :)

 Jocke

[U-Boot] [PATCH] UEC PHY: Speed up initial PHY neg.

[Ben Warren]

  Hi Jocke,

On 8/10/2010 7:36 AM, Joakim Tjernlund wrote:
> Instead of always performing an autoneg, check if the PHY
> already has a link and if it matches one of the requested
> modes. Initially only 100MbFD is optimized this way.
>
> Signed-off-by: Joakim Tjernlund<Joakim.Tjernlund@transmode.se>
> ---
>   drivers/qe/uec_phy.c |    9 +++++++++
>   1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/qe/uec_phy.c b/drivers/qe/uec_phy.c
> index 3baffe4..5237960 100644
> --- a/drivers/qe/uec_phy.c
> +++ b/drivers/qe/uec_phy.c
> @@ -351,6 +351,15 @@ static int marvell_config_aneg (struct uec_mii_info *mii_info)
>   static int genmii_config_aneg (struct uec_mii_info *mii_info)
>   {
>   	if (mii_info->autoneg) {
> +		/* Speed up the common case, if link is already up, speed and
> +		   duplex match, skip auto neg as it already matches */
> +		if (!genmii_read_status(mii_info)&&  mii_info->link)
> +			if (mii_info->duplex == DUPLEX_FULL&&
> +			    mii_info->speed == SPEED_100)
> +				if (mii_info->advertising&
> +				    ADVERTISED_100baseT_Full)
> +					return 0;
> +
>   		config_genmii_advert (mii_info);
>   		genmii_restart_aneg (mii_info);
>   	} else
Applied to net/next

regards,
Ben