Date:   Tue, 6 Apr 2021 19:04:58 +0900
From:   Daniel Palmer <daniel@...f.com>
To:     Willy Tarreau <w@....eu>
Cc:     Jakub Kicinski <kuba@...nel.org>, netdev@...r.kernel.org,
        David Miller <davem@...emloft.net>,
        Nicolas Ferre <nicolas.ferre@...rochip.com>,
        Claudiu Beznea <claudiu.beznea@...rochip.com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>
Subject: Re: [PATCH] Revert "macb: support the two tx descriptors on at91rm9200"

Hi Willy,

I've been messing with the SSD202D (sibling of the MSC313E) recently
and the ethernet performance was awful.
I remembered this revert and reverted it and it makes the ethernet
work pretty well.

So I would like to find some way of making this patch work and I did
some digging..

On Thu, 10 Dec 2020 at 03:47, Willy Tarreau <w@....eu> wrote:
>
> This reverts commit 0a4e9ce17ba77847e5a9f87eed3c0ba46e3f82eb.
>
> The code was developed and tested on an MSC313E SoC, which seems to be
> half-way between the AT91RM9200 and the AT91SAM9260 in that it supports
> both the 2-descriptors mode and a Tx ring.

The MSC313E and later seem to have a hidden "TX ring" with 4 descriptors.
It looks like instead of implementing a ring in memory like the RX
side has and all other macb variations they decided to put a 4 entry
FIFO behind the TAR/TCR registers.
You can load TAR/TCR normally so it is backwards compatible to the
at91rm9200 but you can fill it with multiple frames to transmit
without waiting for TX to end.
There are some extra bits in TSR that seem to indicate how many frames
are still waiting to be transmitted and the way BNQ works is a little
different.

> It turns out that after the code was merged I could notice that the
> controller would sometimes lock up, and only when dealing with sustained
> bidirectional transfers, in which case it would report a Tx overrun
> condition right after having reported being ready, and will stop sending
> even after the status is cleared (a down/up cycle fixes it though).

I can reproduce this with iperf3's bidirectional mode without fail.

I hacked up the driver a bit so that the TX path sends each frame 6
times recording the TSR register before and after to see what is
happening:

# udhcpc
udhcpc: started, v1.31.1
[   12.944302] Doing phy power up
[   13.147460] macb 1f2a2000.emac eth0: PHY
[1f2a2000.emac-ffffffff:00] driver [msc313e phy] (irq=POLL)
[   13.156655] macb 1f2a2000.emac eth0: configuring for phy/mii link mode
udhcpc: sending discover
[   15.205691] macb 1f2a2000.emac eth0: Link is Up - 100Mbps/Full -
flow control off
[   15.213358] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
[   15.245235] pre0 41001fb8, post0 30001fb0 -> IDLETSR clear
[   15.249291] pre1 30001f90, post1 20001d90 -> FIFOIDLE1 clear
[   15.253331] pre2 20001d90, post2 10001990 -> FIFOIDLE2 clear
[   15.257385] pre3 10001990, post3 00001190 -> FIFOIDLE3 clear
[   15.261435] pre4 00001190, post4 f0000191 -> FIFOIDLE4 clear, OVR set
[   15.265485] pre5 f0000190, post5 f0000191 -> OVR set
[   15.269535] pre6 f0000190, post6 e0000181 -> OVR set, BNQ clear

There seems to be a FIFO empty counter in the top of the register but
this is totally undocumented.
There are two new status bits TBNQ and FBNQ at bits 7 and 8. I have no
idea what they mean.
Bits 9 through 12 are some new status bits that seem to show if a FIFO
slot is inuse.
I can't find any mention of these bits anywhere except the header of
the vendor driver so I think these are specific to MStar's macb.

The interesting part though is that BNQ does not get cleared until
multiple frames have been pushed in and after OVR is set.
I think this is what breaks your code when it runs on the MSC313E
version of MACB.

Anyhow. I'm working on a version of your patch that should work with
both the at91rm9200 and the MSC313E.

Thanks,

Daniel

Powered by blists - more mailing lists