Hi Andy,

On 21/10/20 12:19 pm, Andy Duan wrote:
From: Greg Ungerer <g...@linux-m68k.org> Sent: Wednesday, October 21, 2020 9:52 
AM
Hi Andrew,

Thanks for the quick response.


On 20/10/20 12:40 pm, Andrew Lunn wrote:
On Tue, Oct 20, 2020 at 12:14:04PM +1000, Greg Ungerer wrote:
Hi Andrew,

Commit f166f890c8f0 ("[PATCH] net: ethernet: fec: Replace interrupt
driven MDIO with polled IO") breaks the FEC driver on at least one of
the ColdFire platforms (the 5208). Maybe others, that is all I have
tested on so far.

Specifically the driver no longer finds any PHY devices when it
probes the MDIO bus at kernel start time.

I have pinned the problem down to this one specific change in this commit:

@@ -2143,8 +2142,21 @@ static int fec_enet_mii_init(struct
platform_device *pdev)
     if (suppress_preamble)
             fep->phy_speed |= BIT(7);
+   /* Clear MMFR to avoid to generate MII event by writing MSCR.
+    * MII event generation condition:
+    * - writing MSCR:
+    *      - mmfr[31:0]_not_zero & mscr[7:0]_is_zero &
+    *        mscr_reg_data_in[7:0] != 0
+    * - writing MMFR:
+    *      - mscr[7:0]_not_zero
+    */
+   writel(0, fep->hwp + FEC_MII_DATA);

At least by removing this I get the old behavior back and everything
works as it did before.

With that write of the FEC_MII_DATA register in place it seems that
subsequent MDIO operations return immediately (that is FEC_IEVENT is
set) - even though it is obvious the MDIO transaction has not completed yet.

Any ideas?

Hi Greg

This has come up before, but the discussion fizzled out without a
final patch fixing the issue. NXP suggested this

writel(0, fep->hwp + FEC_MII_DATA);

Without it, some other FEC variants break because they do generate an
interrupt at the wrong time causing all following MDIO transactions to
fail.

At the moment, we don't seem to have a clear understanding of the
different FEC versions, and how their MDIO implementations vary.

Based on Andy and Chris' comments is something like the attached patch what
we need?

Greg, imx28 platform also requires the flag.

Got it, thanks. I will update the patch. I won't resend a v2 just yet, wait and see if there is any other comments.

Regards
Greg


Reply via email to