Re: Aw: Re: Re: [PATCH v3 0/5] arm: dts: mt7623: relocate gmacs, mt7530 switch, and add port@5

[Arınç ÜNAL]

On 18.02.2023 22:11, Frank Wunderlich wrote:
> Am 18. Februar 2023 18:02:11 MEZ schrieb "Arınç ÜNAL" <arinc.unal@xxxxxxxxxx>:
> > On 18.02.2023 15:18, Frank Wunderlich wrote:
> > > Hi,
> > >
> > > > Gesendet: Samstag, 18. Februar 2023 um 11:49 Uhr
> > > > Von: "Arınç ÜNAL" <arinc.unal@xxxxxxxxxx>
> > > > An: "Frank Wunderlich" <frank-w@xxxxxxxxxxxxxxx>
> > >
> > > > On 18.02.2023 13:24, Frank Wunderlich wrote:
>
> > Ok, so according to your tests, traffic through gmac1 is not very good. gmac0 should be the default DSA master.
>
> > By the way, did you make a bug report of this, by sending a mail to netdev mailing list or some other way?
>
> No,not yet, because gmac1 is not yet used and i had very limited time. I'm not sure it is an issue in mt7530 driver or mtk_soc_eth, not idea how to figure this out. Maybe it can be fixed with a delayed rgmii phy-mode (rgmii-rx-id i guess as traffic affected is rx on switch,tx on gmac).

My experience with rgmii delays is that if the correct delay is not set 
for tx or rx, that tx or rx will just not work at all. But testing it 
anyway won't hurt.

Can you also remove this from mt7530.c and do the test again? See if the 
performance changes at all.

        /* Set core clock into 500Mhz */
        core_write(priv, CORE_GSWPLL_GRP2,
                   RG_GSWPLL_POSDIV_500M(1) |
                   RG_GSWPLL_FBKDIV_500M(25));

> > > isn't there a way to leave ports by default on the the better gmac (gmac0=trgmii)?
> > > maybe moving port5 below port6...not nice, but then port6 is the first cpu found.
> >
> > This could be done but it comes off as an improper way to me.
>
> Just thought about a way without the need of changing driver or adding a new dt property which can be wrongly used (who prevents anybody set the "default-cpu" property on more than 1 cpu-port?), Or define it at switch node level not port level (e.g. via phandle).

This is why we write json-schema documentation.

> If the 5-below-6 way is not the right one i would prefer a driver solution, e.g. let driver choose the best cpu based on fixed value (constant via define) or on highest throughput (trgmii > rgmii),but last may fail if both cpu-ports have same speed like mt7531.
>
> These are the possible solutions i see atm. Maybe dsa people can share their opinion.

You should share your opinion on my mail to netdev mailing list I 
recently CC'd you.

> > > set master in userspace-config? i remember you've sent a patch adding callback for it.
> >
> > You can change the DSA master using iproute2-6.1.0 and above with this patch which should be in your inbox as well.
> >
> > https://lore.kernel.org/netdev/20230211184101.651462-1-richard@xxxxxxxxxxxxxxx/
>
> I try to test it.
>
> > The only issue I see here is that, with this patch series, port5 becomes the default CPU port which is not preferred for the reasons you explained.
> >
> > So once we can have port6 to be the default CPU port of the DSA slaves, there's no issues left.
>
> For me this would be ok if the current configuration does not change, right :)

Userspace configuration won't change, not with the current patch, not ever.

Arınç