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

From: Russell King - ARM Linux <redacted> Data: Tuesday, June 24, 2014 6:29 PM

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

On Sun, Jun 22, 2014 at 09:49:11AM +0100, Russell King - ARM Linux wrote:

quoted

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

wrote:

quoted

quoted

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

quoted

To: Duan Fugang-B38611
Cc: linux-arm-kernel at lists.infradead.org; netdev at 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 at freescale.com

wrote:

quoted

quoted

quoted

quoted

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

quoted

To: linux-arm-kernel at lists.infradead.org
Cc: Duan Fugang-B38611; netdev at 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

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.

quoted

quoted

quoted

The condition "fep->dirty_tx != fep->cur_tx" is not only limited for

the errata.

quoted

quoted

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.

Any further comments, or do I take the silence to mean that you agree with
the above statement?  I would like to get this settled to I can spin v2 of
this set.

Thanks.

I agree your statement.

Thanks,
Andy