[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Stefan Roese]

I've noticed that the first ethernet packet after PHY link establishment
is not tranferred correctly most of the time on my AT91SAM9G25 board.
Here I usually see a timeout of a few seconds, which is quite
annoying.

Adding a small delay (10ms in this case) after the link establishment
helps to solve this problem. With this patch applied, this timeout
on the first packet is not seen any more.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Wenyou Yang <wenyou.yang@atmel.com>
Cc: Eugen Hristev <eugen.hristev@microchip.com>
Cc: Joe Hershberger <joe.hershberger@ni.com>
---
 drivers/net/macb.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index 182331f61d..72614164e9 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -550,8 +550,14 @@ static int macb_phy_init(struct macb_device *macb, const char *name)
 
 		for (i = 0; i < MACB_AUTONEG_TIMEOUT / 100; i++) {
 			status = macb_mdio_read(macb, MII_BMSR);
-			if (status & BMSR_LSTATUS)
+			if (status & BMSR_LSTATUS) {
+				/*
+				 * Delay a bit after the link is established,
+				 * so that the next xfer does not fail
+				 */
+				mdelay(10);
 				break;
+			}
 			udelay(100);
 		}
 	}
-- 
2.21.0

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Eugen.Hristev at microchip.com]

On 27.03.2019 12:20, Stefan Roese wrote:

> I've noticed that the first ethernet packet after PHY link establishment
> is not tranferred correctly most of the time on my AT91SAM9G25 board.
> Here I usually see a timeout of a few seconds, which is quite
> annoying.
> 
> Adding a small delay (10ms in this case) after the link establishment
> helps to solve this problem. With this patch applied, this timeout
> on the first packet is not seen any more.

Hi Stefan,

I find this a bit odd... maybe someone with a different board having 
this Ethernet controller can confirm or infirm this ?
Linux driver for macb has a similar issue ?
Adding a delay just for the sake of it might hide another issue that we 
are missing at this point: why exactly transfer fails right away... it 
is likely that we want to send packets but link in fact is not ready ?
Or any reason why the packet is not correctly transferred? Do you see 
errors on the line or the packet vanishes completely ?
What kind of packet is this first one ? Does it depend on the packet type ?

Sorry for maybe asking too many questions.

I have added Nicolas and Claudiu who have more experience with macb on 
at91 boards

Thanks,
Eugen

> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> Cc: Eugen Hristev <eugen.hristev@microchip.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>
> ---
>   drivers/net/macb.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/macb.c b/drivers/net/macb.c
> index 182331f61d..72614164e9 100644
> --- a/drivers/net/macb.c
> +++ b/drivers/net/macb.c
> @@ -550,8 +550,14 @@ static int macb_phy_init(struct macb_device *macb, const char *name)
>   
>   		for (i = 0; i < MACB_AUTONEG_TIMEOUT / 100; i++) {
>   			status = macb_mdio_read(macb, MII_BMSR);
> -			if (status & BMSR_LSTATUS)
> +			if (status & BMSR_LSTATUS) {
> +				/*
> +				 * Delay a bit after the link is established,
> +				 * so that the next xfer does not fail
> +				 */
> +				mdelay(10);
>   				break;
> +			}
>   			udelay(100);
>   		}
>   	}
> 

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Stefan Roese]

Hi Eugen,

On 29.03.19 15:40, Eugen.Hristev at microchip.com wrote:
>> I've noticed that the first ethernet packet after PHY link establishment
>> is not tranferred correctly most of the time on my AT91SAM9G25 board.
>> Here I usually see a timeout of a few seconds, which is quite
>> annoying.
>>
>> Adding a small delay (10ms in this case) after the link establishment
>> helps to solve this problem. With this patch applied, this timeout
>> on the first packet is not seen any more.
> 
> Hi Stefan,
> 
> I find this a bit odd... maybe someone with a different board having
> this Ethernet controller can confirm or infirm this ?

I've seen such issues of timeouts with the first ethernet packet
after link negotiation on other platforms / controllers a few
times before in U-Boot. Using a short delay after link-up always
did help here.

I'm not saying that this is the perfect solution, but its one that
works and removes these timeouts, which really annoy me.

> Linux driver for macb has a similar issue ?

Not that I am aware of. There might be some delay in the link
establishment hidden as well. I did not check yet.

> Adding a delay just for the sake of it might hide another issue that we
> are missing at this point: why exactly transfer fails right away... it
> is likely that we want to send packets but link in fact is not ready ?

Yes, something like this.

> Or any reason why the packet is not correctly transferred? Do you see
> errors on the line or the packet vanishes completely ?
> What kind of packet is this first one ? Does it depend on the packet type ?

I did not check too closely, as this issue was so similar to the
ones I've seen before in U-Boot (see above). It happens upon
"ping" and "tftp" - that's what I did test with.
  
> Sorry for maybe asking too many questions.

No need to be sorry. If this can be solved in another perhaps
cleaner way, I'll gladly use another solution.
  
> I have added Nicolas and Claudiu who have more experience with macb on
> at91 boards

Sure, let's see what others have to say on this. Did someone else
notice these timeouts resulting in quite big delays?

Thanks,
Stefan

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Eugen.Hristev at microchip.com]

On 29.03.2019 16:52, Stefan Roese wrote:

> Hi Eugen,
> 
> On 29.03.19 15:40, Eugen.Hristev at microchip.com wrote:
>>> I've noticed that the first ethernet packet after PHY link establishment
>>> is not tranferred correctly most of the time on my AT91SAM9G25 board.
>>> Here I usually see a timeout of a few seconds, which is quite
>>> annoying.
>>>
>>> Adding a small delay (10ms in this case) after the link establishment
>>> helps to solve this problem. With this patch applied, this timeout
>>> on the first packet is not seen any more.
>>
>> Hi Stefan,
>>
>> I find this a bit odd... maybe someone with a different board having
>> this Ethernet controller can confirm or infirm this ?
> 
> I've seen such issues of timeouts with the first ethernet packet
> after link negotiation on other platforms / controllers a few
> times before in U-Boot. Using a short delay after link-up always
> did help here.
> 
> I'm not saying that this is the perfect solution, but its one that
> works and removes these timeouts, which really annoy me.
> 
>> Linux driver for macb has a similar issue ?
> 
> Not that I am aware of. There might be some delay in the link
> establishment hidden as well. I did not check yet.
> 
>> Adding a delay just for the sake of it might hide another issue that we
>> are missing at this point: why exactly transfer fails right away... it
>> is likely that we want to send packets but link in fact is not ready ?
> 
> Yes, something like this.

I see there is a small udelay below your code, you add a delay 100 times 
bigger... maybe that small delay is related ?

> 
>> Or any reason why the packet is not correctly transferred? Do you see
>> errors on the line or the packet vanishes completely ?
>> What kind of packet is this first one ? Does it depend on the packet 
>> type ?
> 
> I did not check too closely, as this issue was so similar to the
> ones I've seen before in U-Boot (see above). It happens upon
> "ping" and "tftp" - that's what I did test with.
> 
>> Sorry for maybe asking too many questions.
> 
> No need to be sorry. If this can be solved in another perhaps
> cleaner way, I'll gladly use another solution.
> 
>> I have added Nicolas and Claudiu who have more experience with macb on
>> at91 boards
> 
> Sure, let's see what others have to say on this. Did someone else
> notice these timeouts resulting in quite big delays?
> 
> Thanks,
> Stefan

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Stefan Roese]

On 29.03.19 15:59, Eugen.Hristev at microchip.com wrote:
> 
> 
> On 29.03.2019 16:52, Stefan Roese wrote:
> 
>> Hi Eugen,
>>
>> On 29.03.19 15:40, Eugen.Hristev at microchip.com wrote:
>>>> I've noticed that the first ethernet packet after PHY link establishment
>>>> is not tranferred correctly most of the time on my AT91SAM9G25 board.
>>>> Here I usually see a timeout of a few seconds, which is quite
>>>> annoying.
>>>>
>>>> Adding a small delay (10ms in this case) after the link establishment
>>>> helps to solve this problem. With this patch applied, this timeout
>>>> on the first packet is not seen any more.
>>>
>>> Hi Stefan,
>>>
>>> I find this a bit odd... maybe someone with a different board having
>>> this Ethernet controller can confirm or infirm this ?
>>
>> I've seen such issues of timeouts with the first ethernet packet
>> after link negotiation on other platforms / controllers a few
>> times before in U-Boot. Using a short delay after link-up always
>> did help here.
>>
>> I'm not saying that this is the perfect solution, but its one that
>> works and removes these timeouts, which really annoy me.
>>
>>> Linux driver for macb has a similar issue ?
>>
>> Not that I am aware of. There might be some delay in the link
>> establishment hidden as well. I did not check yet.
>>
>>> Adding a delay just for the sake of it might hide another issue that we
>>> are missing at this point: why exactly transfer fails right away... it
>>> is likely that we want to send packets but link in fact is not ready ?
>>
>> Yes, something like this.
> 
> I see there is a small udelay below your code, you add a delay 100 times
> bigger... maybe that small delay is related ?

The udelay(100) below is for the timeout handling of the link autoneg
loop. This should be probably moved to some loop using get_timer() so
that this udelay can be removed completely.

Coming back to your question, if these delays are related. No, they
are not. The one that I insert is explicitly added *after* the link
was established. I could experiment if a smaller value also works, but
I found that 100ms was working in all my test cases. Possibly 10ms
or even 1ms also works. Not sure.

Thanks,
Stefan

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Eugen.Hristev at microchip.com]

On 29.03.2019 17:24, Stefan Roese wrote:

> 
> On 29.03.19 15:59, Eugen.Hristev at microchip.com wrote:
>>
>>
>> On 29.03.2019 16:52, Stefan Roese wrote:
>>
>>> Hi Eugen,
>>>
>>> On 29.03.19 15:40, Eugen.Hristev at microchip.com wrote:
>>>>> I've noticed that the first ethernet packet after PHY link 
>>>>> establishment
>>>>> is not tranferred correctly most of the time on my AT91SAM9G25 board.
>>>>> Here I usually see a timeout of a few seconds, which is quite
>>>>> annoying.
>>>>>
>>>>> Adding a small delay (10ms in this case) after the link establishment
>>>>> helps to solve this problem. With this patch applied, this timeout
>>>>> on the first packet is not seen any more.
>>>>
>>>> Hi Stefan,
>>>>
>>>> I find this a bit odd... maybe someone with a different board having
>>>> this Ethernet controller can confirm or infirm this ?
>>>
>>> I've seen such issues of timeouts with the first ethernet packet
>>> after link negotiation on other platforms / controllers a few
>>> times before in U-Boot. Using a short delay after link-up always
>>> did help here.
>>>
>>> I'm not saying that this is the perfect solution, but its one that
>>> works and removes these timeouts, which really annoy me.
>>>
>>>> Linux driver for macb has a similar issue ?
>>>
>>> Not that I am aware of. There might be some delay in the link
>>> establishment hidden as well. I did not check yet.
>>>
>>>> Adding a delay just for the sake of it might hide another issue that we
>>>> are missing at this point: why exactly transfer fails right away... it
>>>> is likely that we want to send packets but link in fact is not ready ?
>>>
>>> Yes, something like this.
>>
>> I see there is a small udelay below your code, you add a delay 100 times
>> bigger... maybe that small delay is related ?
> 
> The udelay(100) below is for the timeout handling of the link autoneg
> loop. This should be probably moved to some loop using get_timer() so
> that this udelay can be removed completely.
> 
> Coming back to your question, if these delays are related. No, they
> are not. The one that I insert is explicitly added *after* the link
> was established. I could experiment if a smaller value also works, but
> I found that 100ms was working in all my test cases. Possibly 10ms
> or even 1ms also works. Not sure.

Hi Joe,

If you agree with this change I will take it through my tree. Or any 
feedback appreciated from you or someone familiar with the macb / 
network devices.

Thanks !
Eugen

> 
> Thanks,
> Stefan

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Joe Hershberger]

On Wed, Mar 27, 2019 at 5:20 AM Stefan Roese <sr@denx.de> wrote:
>
> I've noticed that the first ethernet packet after PHY link establishment
> is not tranferred correctly most of the time on my AT91SAM9G25 board.
> Here I usually see a timeout of a few seconds, which is quite
> annoying.
>
> Adding a small delay (10ms in this case) after the link establishment
> helps to solve this problem. With this patch applied, this timeout
> on the first packet is not seen any more.
>
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Wenyou Yang <wenyou.yang@atmel.com>
> Cc: Eugen Hristev <eugen.hristev@microchip.com>
> Cc: Joe Hershberger <joe.hershberger@ni.com>

Acked-by: Joe Hershberger <joe.hershberger@ni.com>

[U-Boot] [PATCH] net: macb: Add small delay after link establishment

[Eugen.Hristev at microchip.com]

On 05.04.2019 04:35, Joe Hershberger wrote:

> 
> On Wed, Mar 27, 2019 at 5:20 AM Stefan Roese <sr@denx.de> wrote:
>>
>> I've noticed that the first ethernet packet after PHY link establishment
>> is not tranferred correctly most of the time on my AT91SAM9G25 board.
>> Here I usually see a timeout of a few seconds, which is quite
>> annoying.
>>
>> Adding a small delay (10ms in this case) after the link establishment
>> helps to solve this problem. With this patch applied, this timeout
>> on the first packet is not seen any more.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Wenyou Yang <wenyou.yang@atmel.com>
>> Cc: Eugen Hristev <eugen.hristev@microchip.com>
>> Cc: Joe Hershberger <joe.hershberger@ni.com>
> 
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
> _______________________________________________

Applied to u-boot-atmel/next

Thanks !