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

[Andre Przywara <andre.przywara@arm.com>]

On Thu, 20 May 2021 00:10:47 +0200
Andreas Rehn <rehn.andreas86@gmail.com> 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.przywara at arm.com>:  
> 
> > On Wed, 19 May 2021 21:42:08 +0200
> > Andreas Rehn <rehn.andreas86@gmail.com> 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 <rehn.andreas86@gmail.com>
> > > ---
> > >  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;  
> >
> >  
>