Re: gianfar: Implement MAC reset and reconfig procedure

Re: gianfar: Implement MAC reset and reconfig procedure

[Daniel Walker]

Hi,

I would like to report an issue in the gianfar driver. The issue is as follows. 

We have a P2020 board that uses the gianfar driver, and we have a m88e1101
PHY connect. When the interface is initially brought up traffic flows as
normal. If you take the interface down then bring it back up traffic stops
flowing. If you do this sequence over and over up/down/up we find that the
interface will allow traffic to flow at a low percentage.

In v4.9 interface allows traffic about %10 of the time.

In v4.19-rc8 the allows traffic %30 of the time.

After bisecting I found that in v3.14 the interface was rock solid and never did
we see this issue. However, v3.15 we started to see this issue. After bisecting I
found the following change is the first one which causes the issue,

a328ac9 gianfar: Implement MAC reset and reconfig procedure

I was able to revert this in v3.15 , however with later development a revert
doesn't appear to be possible. We have no fix for this currently.

I can do testing if you have an idea what might cause the issue.

Daniel

Re: gianfar: Implement MAC reset and reconfig procedure

[Florian Fainelli]

On 10/16/2018 02:36 PM, Daniel Walker wrote:
> Hi,
> 
> I would like to report an issue in the gianfar driver. The issue is as follows. 
> 
> We have a P2020 board that uses the gianfar driver, and we have a m88e1101
> PHY connect. When the interface is initially brought up traffic flows as
> normal. If you take the interface down then bring it back up traffic stops
> flowing. If you do this sequence over and over up/down/up we find that the
> interface will allow traffic to flow at a low percentage.
> 
> In v4.9 interface allows traffic about %10 of the time.
> 
> In v4.19-rc8 the allows traffic %30 of the time.
> 
> After bisecting I found that in v3.14 the interface was rock solid and never did
> we see this issue. However, v3.15 we started to see this issue. After bisecting I
> found the following change is the first one which causes the issue,
> 
> a328ac9 gianfar: Implement MAC reset and reconfig procedure
> 
> I was able to revert this in v3.15 , however with later development a revert
> doesn't appear to be possible. We have no fix for this currently.
> 
> I can do testing if you have an idea what might cause the issue.

What we have seen being typically the problem is that when you have a
PHY connection whereby the PHY provides the RX clock to the MAC (e.g:
RGMII), it is very easy to get in a situation where the PHY clock is
stopped, and the MAC is asked to be reset, but the HW design does not
like that at all since it e.g: stops on packet boundaries and need some
clock cycles to do that, and that results in all sorts of issues (in our
case it was some FIFO corruption). We solved that in bcmgenet.c with
looping internally the TX clock to the RX clock to make sure the
Ethernet MAC (UniMAC in our designs) was successfully reset:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28c2d1a7a0bfdf3617800d2beae1c67983c03d15

Could that somehow be the problem here?
-- 
Florian

Re: gianfar: Implement MAC reset and reconfig procedure

[Daniel Walker]

On Tue, Oct 16, 2018 at 03:03:07PM -0700, Florian Fainelli wrote:
> On 10/16/2018 02:36 PM, Daniel Walker wrote:
> > Hi,
> > 
> > I would like to report an issue in the gianfar driver. The issue is as follows. 
> > 
> > We have a P2020 board that uses the gianfar driver, and we have a m88e1101
> > PHY connect. When the interface is initially brought up traffic flows as
> > normal. If you take the interface down then bring it back up traffic stops
> > flowing. If you do this sequence over and over up/down/up we find that the
> > interface will allow traffic to flow at a low percentage.
> > 
> > In v4.9 interface allows traffic about %10 of the time.
> > 
> > In v4.19-rc8 the allows traffic %30 of the time.
> > 
> > After bisecting I found that in v3.14 the interface was rock solid and never did
> > we see this issue. However, v3.15 we started to see this issue. After bisecting I
> > found the following change is the first one which causes the issue,
> > 
> > a328ac9 gianfar: Implement MAC reset and reconfig procedure
> > 
> > I was able to revert this in v3.15 , however with later development a revert
> > doesn't appear to be possible. We have no fix for this currently.
> > 
> > I can do testing if you have an idea what might cause the issue.
> 
> What we have seen being typically the problem is that when you have a
> PHY connection whereby the PHY provides the RX clock to the MAC (e.g:
> RGMII), it is very easy to get in a situation where the PHY clock is
> stopped, and the MAC is asked to be reset, but the HW design does not
> like that at all since it e.g: stops on packet boundaries and need some
> clock cycles to do that, and that results in all sorts of issues (in our
> case it was some FIFO corruption). We solved that in bcmgenet.c with
> looping internally the TX clock to the RX clock to make sure the
> Ethernet MAC (UniMAC in our designs) was successfully reset:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=28c2d1a7a0bfdf3617800d2beae1c67983c03d15
> 
> Could that somehow be the problem here?

A little more context on this issue after some debugging.


The patch which I quote above adds a line into int startup_gfar() which does,

gfar_mac_reset(priv);

If this line is removed then everything starts working again (this is debugging
at the v3.15 source level).

On further inspection the block of code inside gfar_mac_reset() is causes a
problem is this one,

        /* Initialize MACCFG2. */
        tempval = MACCFG2_INIT_SETTINGS;
        if (gfar_has_errata(priv, GFAR_ERRATA_74))
                tempval |= MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK;
        gfar_write(&regs->maccfg2, tempval);

and if you change this block to this,

        tempval = gfar_read(&regs->maccfg2);
        if (gfar_has_errata(priv, GFAR_ERRATA_74))
                tempval |= MACCFG2_HUGEFRAME | MACCFG2_LENGTHCHECK;
        gfar_write(&regs->maccfg2, tempval);

Then everything starts working.

At least on my hardware if you gfar_read() when the hardware first comes up it doesn't cause any issues
however, I don't know about other hardware. It would seems that MACCFG2_INIT_SETTINGS is not set up
correctly or shouldn't be used in this context.

Daniel