MT7621 Flow Control

[Jaap Buurman]

On Thu, Aug 6, 2020 at 2:35 PM John Crispin <john at phrozen.org> wrote:
>
>
> On 06.08.20 14:31, Andre Valentin wrote:
> > Hi Jaap,
> >
> >
> > Am 06.08.20 um 13:43 schrieb Jaap Buurman:
> >> Dear all,
> >>
> >> I have noticed the flow control work for mt7621 in the following
> >> Openwrt patch: https://git.openwrt.org/?p=openwrt/openwrt.git;a=commit;h=c8f8e59816eca49d776562d2d302bf990a87faf0
> >>
> >> However, the problem that the patch is supposed to fix is still
> >> occurring, even in combination with other experimental patches
> >> submitted. These experiences can be read about here:
> >> https://forum.openwrt.org/t/mtk-soc-eth-watchdog-timeout-after-r11573/50000/
> >>
> >> However, on this mailing list a user by the name of Kristian claims
> >> that disabling flow control helps fix this problem, as can be read
> >> here: https://lists.openwrt.org/pipermail/openwrt-devel/2017-November/009882.html
> >>
> >>  From what I understood, he was running many mt7621 devices
> >> commercially, with many of them experiencing the issue, which were all
> >> fixed with his own flow control patch. My question is why the decision
> >> was made to only disable flow control on port 5 in the above mentioned
> >> Openwrt patch? AFAIK, Kristian's own patch disables flow control on
> >> all of the ports and he claims the issue is fixed for him. Perhaps the
> >> current patch should be extended to disable flow control on all ports?
> >> What are people's thoughts on this?
> > I'm facing the same issue now after upgrading to 5.4 kernel more often than before.
> > Every second reboot reboot with 5.4 fails with this timeout error.
> >
> >> Yours sincerely,
> >>
> >> Jaap
> > André
> >
> >
> from previous discussions with MTK and looking at the SDK code, the flow
> control should always be disabled.
>
>      John
>

Dear all,

I had a look at the actual patch to see if it would be possible for me
to extend it to disable flow control on all ports. However, I am
unsure if the patch in its current form even does anything, even for
port 5. Here is the diff of the commit in question:
https://git.openwrt.org/?p=openwrt/openwrt.git;a=blobdiff;f=target/linux/ramips/files-4.14/drivers/net/ethernet/mediatek/gsw_mt7621.c;h=232bcd8cf4ea5edbd34d815032ce72b1f1f85661;hp=89be23900738095a8180532d5dd7e585f01bb7c4;hb=c8f8e59816eca49d776562d2d302bf990a87faf0;hpb=3e11ddaf2ede4f105bc9ac91229623526371a7a2

In the old code, the IF statement was used to check silicon revision.
As this code only runs on mt7621 devices, this apparently always
evaluates to TRUE, which means the ELSE statement is never run.
Therefore, the IF condition is removed. However, let's see what was in
the original IF block:

/* (GE1, Force 1000M/FD, FC ON, MAX_RX_LENGTH 1536) */
mtk_switch_w32(gsw, 0x2305e30b, GSW_REG_MAC_P0_MCR);
mt7530_mdio_w32(gsw, 0x3600, 0x5e30b);

As we can see, some values are written to the switch's registers. The
included comment states "FC ON" implying that flow control is enabled,
as expected (since this was the old code).

Now if we look at the new code, we see this:

/* (GE1, Force 1000M/FD, FC OFF, MAX_RX_LENGTH 1536) */
mtk_switch_w32(gsw, 0x2305e30b, GSW_REG_MAC_P0_MCR);
mt7530_mdio_w32(gsw, 0x3600, 0x5e30b);

Here the included comment states "FC OFF" implying that flow control
is disabled. HOWEVER, if we look at the values written and the
location where they are written to, they are identical in the old and
new code. Hence this commit doesn't seem to actually change anything.
Am I reading this correct, or am I missing something?

Jaap