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

Message-ID: <20200427201339.GJ1250287@lunn.ch>
Date:   Mon, 27 Apr 2020 22:13:39 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Leonard Crestez <leonard.crestez@....com>
Cc:     Andy Duan <fugang.duan@....com>,
        David Miller <davem@...emloft.net>,
        netdev <netdev@...r.kernel.org>,
        Chris Healy <Chris.Healy@....aero>,
        dl-linux-imx <linux-imx@....com>, Chris Healy <cphealy@...il.com>
Subject: Re: [PATCH] net: ethernet: fec: Replace interrupt driven MDIO with
 polled IO

Hi Leonard

> Does not help.

Thanks for testing it.

> What does seem to help is inserting prints after the 
> FEC_ENET_MII check but that's probably because it inject a long delay 
> equivalent to the long udelay Andy has mentioned.

Yes, serial ports are slow...

> I found that in my case FEC_ENET_MII is already set on entry to 
> fec_enet_mdio_read, doesn't this make fec_enet_mdio_wait pointless?
> Perhaps the problem is that the MII Interrupt pending bit is not 
> cleared. I can fix the problem like this:
> 
> diff --git drivers/net/ethernet/freescale/fec_main.c 
> drivers/net/ethernet/freescale/fec_main.c
> index 1ae075a246a3..f1330071647c 100644
> --- drivers/net/ethernet/freescale/fec_main.c
> +++ drivers/net/ethernet/freescale/fec_main.c
> @@ -1841,10 +1841,19 @@ static int fec_enet_mdio_read(struct mii_bus 
> *bus, int mii_id, int regnum)
> 
>          ret = pm_runtime_get_sync(dev);
>          if (ret < 0)
>                  return ret;
> 
> +       if (1) {
> +               u32 ievent;
> +               ievent = readl(fep->hwp + FEC_IEVENT);
> +               if (ievent & FEC_ENET_MII) {
> +                       dev_warn(dev, "found FEC_ENET_MII pending\n");
> +                       writel(FEC_ENET_MII, fep->hwp + FEC_IEVENT);
> +               }

How often do you see this warning?

The patch which is causing the regression clears any pending events in
fec_enet_mii_init() and after each time we wait. So the bit should not
be set here. If it is set, the question is why?

The other option is that the hardware is broken. It is setting the
event bit way too soon, before we can actually read the data from the
register.

	Andrew