Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-06-03 Thread Andreas Rehn
Am Do., 3. Juni 2021 um 16:43 Uhr schrieb Heinrich Schuchardt <
xypron.g...@gmx.de>:

> On 6/3/21 3:56 PM, Andre Przywara wrote:
> > On Fri, 21 May 2021 22:14:00 +0200
> > Andreas Rehn  wrote:
> >
> > Hi,
> >
> >> sorry for the late response.
> >
> > same ;-)
> >
> >> I run some test runs and maybe there is something with the phy itself
> >> or something is missing on sun8i_emac_eth_stop/start?
> >>
> >> if you have any patches/ideas to test - let me know!
> >> maybe someone has an idea how I can try to force the Linux mainline
> driver
> >> in the same situation?
> >> just want to know if there is the same behavior.
> >
> > So... I think there are at least three different problems at play here:
> > 1) EMAC soft reset timeout:
> > as mentioned, I believe the timeout value itself is a red herring,
> > as it is an automatic operation (the bit flips back to 0 once the
> > reset is done). Waiting much longer sounds weird, the MAC should
> > reset immediately, since at this point it doesn't talk to anyone: it
> > just pushes the "reset switch" on its internal state. However there
> > might be more to it, see below.
>

I totally agree. Disabling the soft-reset results not in a timeout at 100
MB/s full-duplex
and the download starts immediately. For me, this looks like a wrong usage
of the softreset too.
Maybe this occurs only if the die supports an internal phy?


> > 2) TFTP timeout and resulting slow transfer speed:
> > This is a totally unrelated and somewhat normal behaviour: TFTP uses
> > UDP, so it's not connection oriented. UDP packets might get lost,
> > for instance due to collisions on the wire. TCP handles those loses
> > transparently and swiftly, that's why you don't notice them there.
> > What makes this so annoying is the long timeout value of 5 seconds,
> > which drastically reduces the overall transfer rate. You can tweak
> > this value by changing TIMEOUT at the beginning of net/tftp.c. If
> > you put 100 there, you will probably barely notice them anymore. The
> > 5 seconds seem to come from the TFTP RFC, so it's hard to argue
> > against it.
>

Agree, I just wanted to give you the exact behaviors/measurements.


> > 3) PHY autonegotiation timeout:
> > This is again independent from the others, especially the MAC soft
> > reset timeout. U-Boot's network stack tries to speak to the PHY via
> > the MDIO bus: PHY_ANEG_TIMEOUT is the macro putting a limit here.
> > There is currently the default 4 seconds fallback value in effect
> > for sunxi here: this might be too short for some situations. Grep
> > for that value to find much longer timeouts for some other
> > platforms. You can try to define this for the V3s in
> > include/configs/sunxi-common.h, and see if that improves things.
> > Happy to take a patch to that effect.


I didn't run into this after i disabled the soft-reset for 100 MB/s
full-duplex!
And yes, I expect a timeout if the cable is not connected but when you
connect the cable
until the timeout is reached, the link will be established.


> >

>
> > Regarding 1): Heinrich reported the same problem on a H3 board, and
> > bisected it down to some other patch. This again does not seem to be
> > related, and I start to wonder if we indeed handle the soft reset
> > wrongly, as mentioned in you v2 patch.
> > I will have a closer look later at when exactly we issue the soft
> > reset, maybe we do it too often? We probably should do it only once,
> > and not on every new network request. Or maybe we need some delay
> > after the soft reset returns, because it's doing so prematurely? But
> > just dropping it does not sound right, although it's interesting that
> > Linux doesn't need it.
>

I was also wondering about the linux driver but i didn't see any wrong
behaviors if i disable it on u-boot too.


>
> Applying
>
> net: sun8i-emac: v3s: fix soft reset timeout
>
> https://patchwork.ozlabs.org/project/uboot/patch/20210522232340.201471-1-rehn.andrea...@gmail.com/
>
> and
>
>  /* Soft reset MAC */
> -   if (!IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {
> +   if (1 || !IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {
>
> does not solve the problem I see on the OrangePi PC:
>
> => dhcp
> sun8i_emac_eth_start: Timeout
>
> So it seems we are talking about different issues.


> Applying "net: sun8i-emac: v3s: fix soft reset timeout" on top of
>
> "net: sun8i-emac: fix MDIO frequency"
>
> https://patchwork.ozlabs.org/project/uboot/patch/20210603075242.96527-1-xypron.g...@gmx.de/
>
> does not do any harm nor does it show any benefit for tFTP transfer on
> the OrangePi PC.
>
> Best regards
>
> Heinrich
>
> >
> >>
> >> test-scenario:
> >>  download 10 times zImage and dtb over tftp,
> >>  static ip, no reset, timeout = 1000
> >> 10 duplex half:
> >>  soft reset time 0us with 3 tftp timeouts and recover
> >>  lowest speed:   369.1 KiB/s
> >>  max speed:  779.3 KiB/s
> >> 10 duplex 

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-06-03 Thread Heinrich Schuchardt

On 6/3/21 3:56 PM, Andre Przywara wrote:

On Fri, 21 May 2021 22:14:00 +0200
Andreas Rehn  wrote:

Hi,


sorry for the late response.


same ;-)


I run some test runs and maybe there is something with the phy itself
or something is missing on sun8i_emac_eth_stop/start?

if you have any patches/ideas to test - let me know!
maybe someone has an idea how I can try to force the Linux mainline driver
in the same situation?
just want to know if there is the same behavior.


So... I think there are at least three different problems at play here:
1) EMAC soft reset timeout:
as mentioned, I believe the timeout value itself is a red herring,
as it is an automatic operation (the bit flips back to 0 once the
reset is done). Waiting much longer sounds weird, the MAC should
reset immediately, since at this point it doesn't talk to anyone: it
just pushes the "reset switch" on its internal state. However there
might be more to it, see below.
2) TFTP timeout and resulting slow transfer speed:
This is a totally unrelated and somewhat normal behaviour: TFTP uses
UDP, so it's not connection oriented. UDP packets might get lost,
for instance due to collisions on the wire. TCP handles those loses
transparently and swiftly, that's why you don't notice them there.
What makes this so annoying is the long timeout value of 5 seconds,
which drastically reduces the overall transfer rate. You can tweak
this value by changing TIMEOUT at the beginning of net/tftp.c. If
you put 100 there, you will probably barely notice them anymore. The
5 seconds seem to come from the TFTP RFC, so it's hard to argue
against it.
3) PHY autonegotiation timeout:
This is again independent from the others, especially the MAC soft
reset timeout. U-Boot's network stack tries to speak to the PHY via
the MDIO bus: PHY_ANEG_TIMEOUT is the macro putting a limit here.
There is currently the default 4 seconds fallback value in effect
for sunxi here: this might be too short for some situations. Grep
for that value to find much longer timeouts for some other
platforms. You can try to define this for the V3s in
include/configs/sunxi-common.h, and see if that improves things.
Happy to take a patch to that effect.


Regarding 1): Heinrich reported the same problem on a H3 board, and
bisected it down to some other patch. This again does not seem to be
related, and I start to wonder if we indeed handle the soft reset
wrongly, as mentioned in you v2 patch.
I will have a closer look later at when exactly we issue the soft
reset, maybe we do it too often? We probably should do it only once,
and not on every new network request. Or maybe we need some delay
after the soft reset returns, because it's doing so prematurely? But
just dropping it does not sound right, although it's interesting that
Linux doesn't need it.


Applying

net: sun8i-emac: v3s: fix soft reset timeout
https://patchwork.ozlabs.org/project/uboot/patch/20210522232340.201471-1-rehn.andrea...@gmail.com/

and

/* Soft reset MAC */
-   if (!IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {
+   if (1 || !IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {

does not solve the problem I see on the OrangePi PC:

=> dhcp
sun8i_emac_eth_start: Timeout

So it seems we are talking about different issues.

Applying "net: sun8i-emac: v3s: fix soft reset timeout" on top of

"net: sun8i-emac: fix MDIO frequency"
https://patchwork.ozlabs.org/project/uboot/patch/20210603075242.96527-1-xypron.g...@gmx.de/

does not do any harm nor does it show any benefit for tFTP transfer on
the OrangePi PC.

Best regards

Heinrich





test-scenario:
 download 10 times zImage and dtb over tftp,
 static ip, no reset, timeout = 1000
10 duplex half:
 soft reset time 0us with 3 tftp timeouts and recover
 lowest speed:   369.1 KiB/s
 max speed:  779.3 KiB/s
10 duplex full:
 soft reset time 0us with 0 tftp timeouts and recover
 lowest speed:   656.3 KiB/s
 max speed:  752.9 KiB/s
100 duplex half:
 soft reset time 0us with 1 tftp timeout and recover
 lowest speed:   1.6 MiB/s
 max speed:  2.7 MiB/s

100 duplex full:


what are those values before and after the comma below?

Cheers,
Andre



 try1:   0us, 63 us with 0 tftp timeout and recover
 try2:   1001000 us sun8i_emac_eth_start: Timeout
 -> 5 times
 -> reconnect cable
 try3:   382000us, 502000us with 0 tftp timeout and recover
 try4:   33 us, 1001000 us sun8i_emac_eth_start: Timeout
 -> 2 times
 -> 192000 us
 try5:   power up with cable pluged in:
 58000 us, 373000 us with 0 tftp timeout and recover
 try6:   354000 us, 494000 us with 0 tftp timeout and recover
 try7:   1001000 us sun8i_emac_eth_start: Timeout
 -> 3 times
 -> 

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-06-03 Thread Andre Przywara
On Fri, 21 May 2021 22:14:00 +0200
Andreas Rehn  wrote:

Hi,

> sorry for the late response.

same ;-)

> I run some test runs and maybe there is something with the phy itself
> or something is missing on sun8i_emac_eth_stop/start?
> 
> if you have any patches/ideas to test - let me know!
> maybe someone has an idea how I can try to force the Linux mainline driver
> in the same situation?
> just want to know if there is the same behavior.

So... I think there are at least three different problems at play here:
1) EMAC soft reset timeout:
   as mentioned, I believe the timeout value itself is a red herring,
   as it is an automatic operation (the bit flips back to 0 once the
   reset is done). Waiting much longer sounds weird, the MAC should
   reset immediately, since at this point it doesn't talk to anyone: it
   just pushes the "reset switch" on its internal state. However there
   might be more to it, see below.
2) TFTP timeout and resulting slow transfer speed:
   This is a totally unrelated and somewhat normal behaviour: TFTP uses
   UDP, so it's not connection oriented. UDP packets might get lost,
   for instance due to collisions on the wire. TCP handles those loses
   transparently and swiftly, that's why you don't notice them there.
   What makes this so annoying is the long timeout value of 5 seconds,
   which drastically reduces the overall transfer rate. You can tweak
   this value by changing TIMEOUT at the beginning of net/tftp.c. If
   you put 100 there, you will probably barely notice them anymore. The
   5 seconds seem to come from the TFTP RFC, so it's hard to argue
   against it.
3) PHY autonegotiation timeout:
   This is again independent from the others, especially the MAC soft
   reset timeout. U-Boot's network stack tries to speak to the PHY via
   the MDIO bus: PHY_ANEG_TIMEOUT is the macro putting a limit here.
   There is currently the default 4 seconds fallback value in effect
   for sunxi here: this might be too short for some situations. Grep
   for that value to find much longer timeouts for some other
   platforms. You can try to define this for the V3s in
   include/configs/sunxi-common.h, and see if that improves things.
   Happy to take a patch to that effect.


Regarding 1): Heinrich reported the same problem on a H3 board, and
bisected it down to some other patch. This again does not seem to be
related, and I start to wonder if we indeed handle the soft reset
wrongly, as mentioned in you v2 patch.
I will have a closer look later at when exactly we issue the soft
reset, maybe we do it too often? We probably should do it only once,
and not on every new network request. Or maybe we need some delay
after the soft reset returns, because it's doing so prematurely? But
just dropping it does not sound right, although it's interesting that
Linux doesn't need it.

> 
> test-scenario:
> download 10 times zImage and dtb over tftp,
> static ip, no reset, timeout = 1000
> 10 duplex half:
> soft reset time 0us with 3 tftp timeouts and recover
> lowest speed:   369.1 KiB/s
> max speed:  779.3 KiB/s
> 10 duplex full:
> soft reset time 0us with 0 tftp timeouts and recover
> lowest speed:   656.3 KiB/s
> max speed:  752.9 KiB/s
> 100 duplex half:
> soft reset time 0us with 1 tftp timeout and recover
> lowest speed:   1.6 MiB/s
> max speed:  2.7 MiB/s
> 
> 100 duplex full:

what are those values before and after the comma below?

Cheers,
Andre


> try1:   0us, 63 us with 0 tftp timeout and recover
> try2:   1001000 us sun8i_emac_eth_start: Timeout
> -> 5 times
> -> reconnect cable  
> try3:   382000us, 502000us with 0 tftp timeout and recover
> try4:   33 us, 1001000 us sun8i_emac_eth_start: Timeout
> -> 2 times
> -> 192000 us  
> try5:   power up with cable pluged in:
> 58000 us, 373000 us with 0 tftp timeout and recover
> try6:   354000 us, 494000 us with 0 tftp timeout and recover
> try7:   1001000 us sun8i_emac_eth_start: Timeout
> -> 3 times
> -> 1001000 us sun8i_emac_eth_start: Timeout, 626000 us  
> next tries with fresh startup
> try8:   845000 us, 594000 us
> try9:   903000 us, 479000 us
> try10:  638000 us, 50 us
> try11:  1001000 us sun8i_emac_eth_start: Timeout, 333000 us
> try12:  63000 us, 489000 us
> lowest speed:   1.6 MiB/s
> max speed:  2.7 MiB/s
> 
> when switching from 100 duplex half to full and try to run tftp download
> for zImage and dtb
>  try1:
> reset MAC done after: 0 us
> ethernet@1c3 Waiting for PHY auto negotiation to complete.
> TIMEOUT !
> reset MAC done after: 0 us
> ethernet@1c3 Waiting for PHY auto negotiation to complete.
> TIMEOUT !
>  try2:
> 

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-05-21 Thread Andreas Rehn
sorry for the late response.

I run some test runs and maybe there is something with the phy itself
or something is missing on sun8i_emac_eth_stop/start?

if you have any patches/ideas to test - let me know!
maybe someone has an idea how I can try to force the Linux mainline driver
in the same situation?
just want to know if there is the same behavior.

test-scenario:
download 10 times zImage and dtb over tftp,
static ip, no reset, timeout = 1000
10 duplex half:
soft reset time 0us with 3 tftp timeouts and recover
lowest speed:   369.1 KiB/s
max speed:  779.3 KiB/s
10 duplex full:
soft reset time 0us with 0 tftp timeouts and recover
lowest speed:   656.3 KiB/s
max speed:  752.9 KiB/s
100 duplex half:
soft reset time 0us with 1 tftp timeout and recover
lowest speed:   1.6 MiB/s
max speed:  2.7 MiB/s

100 duplex full:
try1:   0us, 63 us with 0 tftp timeout and recover
try2:   1001000 us sun8i_emac_eth_start: Timeout
-> 5 times
-> reconnect cable
try3:   382000us, 502000us with 0 tftp timeout and recover
try4:   33 us, 1001000 us sun8i_emac_eth_start: Timeout
-> 2 times
-> 192000 us
try5:   power up with cable pluged in:
58000 us, 373000 us with 0 tftp timeout and recover
try6:   354000 us, 494000 us with 0 tftp timeout and recover
try7:   1001000 us sun8i_emac_eth_start: Timeout
-> 3 times
-> 1001000 us sun8i_emac_eth_start: Timeout, 626000 us
next tries with fresh startup
try8:   845000 us, 594000 us
try9:   903000 us, 479000 us
try10:  638000 us, 50 us
try11:  1001000 us sun8i_emac_eth_start: Timeout, 333000 us
try12:  63000 us, 489000 us
lowest speed:   1.6 MiB/s
max speed:  2.7 MiB/s

when switching from 100 duplex half to full and try to run tftp download
for zImage and dtb
 try1:
reset MAC done after: 0 us
ethernet@1c3 Waiting for PHY auto negotiation to complete.
TIMEOUT !
reset MAC done after: 0 us
ethernet@1c3 Waiting for PHY auto negotiation to complete.
TIMEOUT !
 try2:
reset MAC done after: 0 us
Using ethernet@1c3 device
TFTP from server 192.168.5.80; our IP address is 192.168.5.78
Filename 'zImage'.
Load address: 0x4200
Loading:
#
#
#

2.4 MiB/s
done
Bytes transferred = 3790520 (39d6b8 hex)
reset MAC done after: 1001000 us
sun8i_emac_eth_start: Timeout


Am Do., 20. Mai 2021 um 02:18 Uhr schrieb Andre Przywara <
andre.przyw...@arm.com>:

> On Thu, 20 May 2021 00:10:47 +0200
> Andreas Rehn  wrote:
>
> > hey,
> >
> > sure. I give it a try tomorrow.
> > with 250 ms, for example, I ran into timeouts after the first tftp
> download.
> > after a manual retry, it works fine but retry is not a valid production
> > behavior.
>
> Just read the arch timer after the SOFT_RST write and after the first
> read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at
> 24MHz this is less than a *micro*second for the MAC to reset. So the 10
> ms are already plenty.
> Are you sure that it's this timeout value that is improving things for
> you?
>
> Cheers,
> Andre
>
> > greetings
> > Andreas
> >
> > Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara <
> > andre.przyw...@arm.com>:
> >
> > > On Wed, 19 May 2021 21:42:08 +0200
> > > Andreas Rehn  wrote:
> > >
> > > Hi,
> > >
> > > > v3s emac soft reset tooks quit longer if autonegation is active
> > > > on 100 Mbit full duplex pairs what can result in
> > > > `sun8i_emac_eth_start: Timeout` error
> > >
> > > Mmmh, why the 500ms? Can you figure out how long it typically
> > > takes for you? By open-coding wait_for_bit_le32() and printing the time
> > > it took to flip the bit back?
> > >
> > > Happy to change this then when we have some data.
> > >
> > > Cheers,
> > > Andre
> > >
> > > > wait_for_bit_le32 polls register value each ms.
> > > > Increasing the timeout for setup do not effect current behavior
> > > > but reduces unexpected behaviors (e.g. timeouts on tftp download).
> > > >
> > > > Signed-off-by: Andreas Rehn 
> > > > ---
> > > >  drivers/net/sun8i_emac.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > > index 0e7ad3b0d4..23fd35f9e1 100644
> > > > --- a/drivers/net/sun8i_emac.c
> > > > +++ b/drivers/net/sun8i_emac.c
> > > > @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice
> *dev)
> > > >   /* Soft reset 

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-05-19 Thread Andre Przywara
On Thu, 20 May 2021 00:10:47 +0200
Andreas Rehn  wrote:

> hey,
> 
> sure. I give it a try tomorrow.
> with 250 ms, for example, I ran into timeouts after the first tftp download.
> after a manual retry, it works fine but retry is not a valid production
> behavior.

Just read the arch timer after the SOFT_RST write and after the first
read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at
24MHz this is less than a *micro*second for the MAC to reset. So the 10
ms are already plenty.
Are you sure that it's this timeout value that is improving things for
you?

Cheers,
Andre

> greetings
> Andreas
> 
> Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara <
> andre.przyw...@arm.com>:  
> 
> > On Wed, 19 May 2021 21:42:08 +0200
> > Andreas Rehn  wrote:
> >
> > Hi,
> >  
> > > v3s emac soft reset tooks quit longer if autonegation is active
> > > on 100 Mbit full duplex pairs what can result in
> > > `sun8i_emac_eth_start: Timeout` error  
> >
> > Mmmh, why the 500ms? Can you figure out how long it typically
> > takes for you? By open-coding wait_for_bit_le32() and printing the time
> > it took to flip the bit back?
> >
> > Happy to change this then when we have some data.
> >
> > Cheers,
> > Andre
> >  
> > > wait_for_bit_le32 polls register value each ms.
> > > Increasing the timeout for setup do not effect current behavior
> > > but reduces unexpected behaviors (e.g. timeouts on tftp download).
> > >
> > > Signed-off-by: Andreas Rehn 
> > > ---
> > >  drivers/net/sun8i_emac.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > index 0e7ad3b0d4..23fd35f9e1 100644
> > > --- a/drivers/net/sun8i_emac.c
> > > +++ b/drivers/net/sun8i_emac.c
> > > @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev)
> > >   /* Soft reset MAC */
> > >   writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1);
> > >   ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
> > > - EMAC_CTL1_SOFT_RST, false, 10, true);
> > > + EMAC_CTL1_SOFT_RST, false, 500, true);
> > >   if (ret) {
> > >   printf("%s: Timeout\n", __func__);
> > >   return ret;  
> >
> >  
> 



Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-05-19 Thread Andre Przywara
On Thu, 20 May 2021 00:10:47 +0200
Andreas Rehn  wrote:

Hi,

> sure. I give it a try tomorrow.
> with 250 ms, for example, I ran into timeouts after the first tftp download.
> after a manual retry, it works fine but retry is not a valid production
> behavior.

Well, I see occasional TFTP hiccups as well, across different boards.
They are never fatal, the TFTP protocol just triggers a
re-transmission. It's mostly an annoyance.

Some time ago I tried to debug this further, but couldn't find the real
reason for this. I was always tempted to shorten the TFTP timeout, as
packet loss can happen anyway (this is UDP!), and the default timeout of
5 secs sounds ridiculously long (but is mentioned in the original RFC,
IIRC).

Anyway I doubt that this timeout value has any real impact: the soft
reset bit automatically clears when it's reset, so this wait is just a
safety measure to avoid waiting forever in case something goes wrong.
So when we don't reset within 10ms, the whole MAC won't start. And keep
in mind, this is just the MAC soft reset, it's not negotiating anything
on the PHY side or over the wire.

So do you have any deeper insight here? If the 10ms are too short, and
you can show me numbers that it needs longer, I am happy to change that.

Cheers,
Andre

> Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara <
> andre.przyw...@arm.com>:  
> 
> > On Wed, 19 May 2021 21:42:08 +0200
> > Andreas Rehn  wrote:
> >
> > Hi,
> >  
> > > v3s emac soft reset tooks quit longer if autonegation is active
> > > on 100 Mbit full duplex pairs what can result in
> > > `sun8i_emac_eth_start: Timeout` error  
> >
> > Mmmh, why the 500ms? Can you figure out how long it typically
> > takes for you? By open-coding wait_for_bit_le32() and printing the time
> > it took to flip the bit back?
> >
> > Happy to change this then when we have some data.
> >
> > Cheers,
> > Andre
> >  
> > > wait_for_bit_le32 polls register value each ms.
> > > Increasing the timeout for setup do not effect current behavior
> > > but reduces unexpected behaviors (e.g. timeouts on tftp download).
> > >
> > > Signed-off-by: Andreas Rehn 
> > > ---
> > >  drivers/net/sun8i_emac.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > > index 0e7ad3b0d4..23fd35f9e1 100644
> > > --- a/drivers/net/sun8i_emac.c
> > > +++ b/drivers/net/sun8i_emac.c
> > > @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev)
> > >   /* Soft reset MAC */
> > >   writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1);
> > >   ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
> > > - EMAC_CTL1_SOFT_RST, false, 10, true);
> > > + EMAC_CTL1_SOFT_RST, false, 500, true);
> > >   if (ret) {
> > >   printf("%s: Timeout\n", __func__);
> > >   return ret;  
> >
> >  
> 



Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-05-19 Thread Andreas Rehn
hey,

sure. I give it a try tomorrow.
with 250 ms, for example, I ran into timeouts after the first tftp download.
after a manual retry, it works fine but retry is not a valid production
behavior.

greetings
Andreas

Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara <
andre.przyw...@arm.com>:

> On Wed, 19 May 2021 21:42:08 +0200
> Andreas Rehn  wrote:
>
> Hi,
>
> > v3s emac soft reset tooks quit longer if autonegation is active
> > on 100 Mbit full duplex pairs what can result in
> > `sun8i_emac_eth_start: Timeout` error
>
> Mmmh, why the 500ms? Can you figure out how long it typically
> takes for you? By open-coding wait_for_bit_le32() and printing the time
> it took to flip the bit back?
>
> Happy to change this then when we have some data.
>
> Cheers,
> Andre
>
> > wait_for_bit_le32 polls register value each ms.
> > Increasing the timeout for setup do not effect current behavior
> > but reduces unexpected behaviors (e.g. timeouts on tftp download).
> >
> > Signed-off-by: Andreas Rehn 
> > ---
> >  drivers/net/sun8i_emac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> > index 0e7ad3b0d4..23fd35f9e1 100644
> > --- a/drivers/net/sun8i_emac.c
> > +++ b/drivers/net/sun8i_emac.c
> > @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev)
> >   /* Soft reset MAC */
> >   writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1);
> >   ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
> > - EMAC_CTL1_SOFT_RST, false, 10, true);
> > + EMAC_CTL1_SOFT_RST, false, 500, true);
> >   if (ret) {
> >   printf("%s: Timeout\n", __func__);
> >   return ret;
>
>

-- 
Mit freundlichen Grüßen / kind regards
Andreas Rehn | Master of Engineering (M.Eng)


Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-05-19 Thread Andre Przywara
On Wed, 19 May 2021 21:42:08 +0200
Andreas Rehn  wrote:

Hi,

> v3s emac soft reset tooks quit longer if autonegation is active
> on 100 Mbit full duplex pairs what can result in
> `sun8i_emac_eth_start: Timeout` error

Mmmh, why the 500ms? Can you figure out how long it typically
takes for you? By open-coding wait_for_bit_le32() and printing the time
it took to flip the bit back?

Happy to change this then when we have some data.

Cheers,
Andre

> wait_for_bit_le32 polls register value each ms.
> Increasing the timeout for setup do not effect current behavior
> but reduces unexpected behaviors (e.g. timeouts on tftp download).
> 
> Signed-off-by: Andreas Rehn 
> ---
>  drivers/net/sun8i_emac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
> index 0e7ad3b0d4..23fd35f9e1 100644
> --- a/drivers/net/sun8i_emac.c
> +++ b/drivers/net/sun8i_emac.c
> @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev)
>   /* Soft reset MAC */
>   writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1);
>   ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
> - EMAC_CTL1_SOFT_RST, false, 10, true);
> + EMAC_CTL1_SOFT_RST, false, 500, true);
>   if (ret) {
>   printf("%s: Timeout\n", __func__);
>   return ret;



[PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

2021-05-19 Thread Andreas Rehn
v3s emac soft reset tooks quit longer if autonegation is active
on 100 Mbit full duplex pairs what can result in
`sun8i_emac_eth_start: Timeout` error

wait_for_bit_le32 polls register value each ms.
Increasing the timeout for setup do not effect current behavior
but reduces unexpected behaviors (e.g. timeouts on tftp download).

Signed-off-by: Andreas Rehn 
---
 drivers/net/sun8i_emac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 0e7ad3b0d4..23fd35f9e1 100644
--- a/drivers/net/sun8i_emac.c
+++ b/drivers/net/sun8i_emac.c
@@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev)
/* Soft reset MAC */
writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1);
ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
-   EMAC_CTL1_SOFT_RST, false, 10, true);
+   EMAC_CTL1_SOFT_RST, false, 500, true);
if (ret) {
printf("%s: Timeout\n", __func__);
return ret;
-- 
2.25.1