netdev - RE: [EXT] Re: [PATCH] net: ethernet: fec: Replace interrupt driven MDIO with polled IO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Oct 2020 03:02:29 +0000
From:   Andy Duan <fugang.duan@....com>
To:     Andrew Lunn <andrew@...n.ch>, Greg Ungerer <gerg@...ux-m68k.org>
CC:     Chris Heally <cphealy@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [EXT] Re: [PATCH] net: ethernet: fec: Replace interrupt driven
 MDIO with polled IO

From: Andrew Lunn <andrew@...n.ch> Sent: Tuesday, October 20, 2020 10:40 AM
> 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.
> 
>           Andrew

Andrew, different varants has little different behavior, so the line is required for
Imx6/7/7 platforms but should be removed in imx5 and ColdFire.

As we discuss one solution to resolve the issue, but it bring 30ms latency for kernel boot.

Now, I want to revert the polling mode to original interrupt mode, do you agree ?

Andy