Re: [PATCH RFC 24/30] net: fec: better implementation of iMX6 ERR006358 quirk

On Sun, Jun 22, 2014 at 08:38:49AM +0000, fugang.duan@freescale.com wrote:

From: Russell King - ARM Linux <redacted> Data: Sunday, June 22, 2014 4:13 PM

quoted

To: Duan Fugang-B38611
Cc: linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org
Subject: Re: [PATCH RFC 24/30] net: fec: better implementation of iMX6
ERR006358 quirk

On Sun, Jun 22, 2014 at 07:49:11AM +0000, fugang.duan@freescale.com wrote:

quoted

From: Russell King <redacted> Data: Friday, June 20, 2014
8:14 PM

quoted

To: linux-arm-kernel@lists.infradead.org
Cc: Duan Fugang-B38611; netdev@vger.kernel.org
Subject: [PATCH RFC 24/30] net: fec: better implementation of iMX6
ERR006358 quirk

+
+	/* ERR006538: Keep the transmitter going */
+	if (fep->dirty_tx != fep->cur_tx &&
+	    readl(fep->hwp + FEC_X_DES_ACTIVE) == 0)
+		writel(0, fep->hwp + FEC_X_DES_ACTIVE);
}

...

quoted

While I agree that we can read back to check whether the device indicates
that transmit is active, there's no point to the other tests.  If there
are entries in the transmit ring but the transmitter indicates that it is
not active, then it is obvious that the bug has been hit.  This is exactly
what my implementation above does.

The condition "fep->dirty_tx != fep->cur_tx" is not only limited for the errata. 
I mean only add extra trigger TDAR for the issue.

Yes, I agree that test is wrong (that's what comes from shuffling the
patches... subsequent patches modify the indexing mechanism).  It should
be:

        if (bdp != fep->cur_tx &&
            readl(fep->hwp + FEC_X_DES_ACTIVE) == 0)
                writel(0, fep->hwp + FEC_X_DES_ACTIVE);

Since "bdp" is the post-incremented dirty_tx pointer, which, when it is
equal to fep->cur_tx means that the ring is empty.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.