MT7621 Flow Control

[Adrian Schmutzler]

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Jaap Buurman
> Sent: Freitag, 7. August 2020 09:55
> To: John Crispin <john at phrozen.org>
> Cc: OpenWrt Development List <openwrt-devel at lists.openwrt.org>; Andre
> Valentin <avalentin at marcant.net>
> Subject: Re: MT7621 Flow Control
> 
> 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=c8f8e5981
> > >> 6eca49d776562d2d302bf990a87faf0
> > >>
> > >> 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-
> r115
> > >> 73/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/009
> > >> 882.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=232bcd8cf4ea5edbd3
> 4d815032ce72b1f1f85661;hp=89be23900738095a8180532d5dd7e585f01bb7c4;
> hb=c8f8e59816eca49d776562d2d302bf990a87faf0;hpb=3e11ddaf2ede4f105b
> c9ac91229623526371a7a2
> 
> 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

In addition to what Stijn just wrote, please be aware that these files are not used at all for kernel 5.4, as we use DSA there.
There appear to be no CONFIG_NET_RALINK options set in mt7621 at all.
So, whatever you do to these files, it won't matter.

I wonder whether it wouldn't be better to remove these parts from "our" driver entirely, so this becomes obvious (as we do not support 4.14 on mt7621 in master anyway).

Best

Adrian

> 
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20200814/6b9b294a/attachment-0001.sig>