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: Windows password security audit tool. GUI, reports in PDF.

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

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

On Mon, Oct 19, 2020 at 8:02 PM Andy Duan <fugang.duan@....com> wrote:
>
> 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.

Do we know which variants of i.MX6 and i.MX7 do and don't need this?
I'm successfully running with polling mode using the i.MX6q, i.MX6qp,
i.MX7d, and Vybrid, all of which benefit from the considerably higher
throughput achieved with polling.  (In all my use cases I'm working
with an Ethernet Switch attached via MDIO.)

>
> 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