linux-kernel - RE: [PATCH v1 net-next] net: stmmac: Add support for MDIO interrupts

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:   Wed, 28 Aug 2019 17:07:16 +0000
From:   "Voon, Weifeng" <weifeng.voon@...el.com>
To:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>
CC:     "David S. Miller" <davem@...emloft.net>,
        Maxime Coquelin <mcoquelin.stm32@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Jose Abreu <joabreu@...opsys.com>,
        "Giuseppe Cavallaro" <peppe.cavallaro@...com>,
        Alexandre Torgue <alexandre.torgue@...com>,
        "Ong, Boon Leong" <boon.leong.ong@...el.com>
Subject: RE: [PATCH v1 net-next] net: stmmac: Add support for MDIO interrupts

> >> DW EQoS v5.xx controllers added capability for interrupt generation
> >> when MDIO interface is done (GMII Busy bit is cleared).
> >> This patch adds support for this interrupt on supported HW to avoid
> >> polling on GMII Busy bit.
> >>
> >> stmmac_mdio_read() & stmmac_mdio_write() will sleep until wake_up()
> >> is called by the interrupt handler.
> >
> > Hi Voon
> >
> > I _think_ there are some order of operation issues here. The mdiobus
> > is registered in the probe function. As soon as of_mdiobus_register()
> > is called, the MDIO bus must work. At that point MDIO read/writes can
> > start to happen.
> >
> > As far as i can see, the interrupt handler is only requested in
> > stmmac_open(). So it seems like any MDIO operations after probe, but
> > before open are going to fail?
> 
> AFAIR, wait_event_timeout() will continue to busy loop and wait until
> the timeout, but not return an error because the polled condition was
> true, at least that is my recollection from having the same issue with
> the bcmgenet driver before it was moved to connecting to the PHY in the
> ndo_open() function.
> --
> Florian

Florian is right as the poll condition is still true after the timeout. 
Hence, any mdio operation after probe and before ndo_open will still work.
The only cons here is that attaching the PHY will takes a full length of 
timeout time for each mdio_read and mdio_write. 
So we should attach the phy only after the interrupt handler is requested?