Re: call for bge(4) testers

From: Pyun YongHyeon <pyunyh_at_gmail.com>
Date: Thu, 24 Aug 2006 10:11:16 +0900
On Thu, Aug 24, 2006 at 04:42:25AM +0400, Oleg Bulyzhin wrote:
 > On Thu, Aug 24, 2006 at 09:26:32AM +0900, Pyun YongHyeon wrote:
 > > On Wed, Aug 23, 2006 at 04:40:35PM +0400, Oleg Bulyzhin wrote:

[...]

 > > 
 > >  > My idea was: perhaps, under certain condition, concurrent access to PHY could
 > >  > lead to hardware deadlock.
 > >  > 
 > > 
 > > Yes. Because MII bus access needs several steps to access PHY
 > > registers its operation shouldn't be interrupted until all pending
 > > requests are served. 
 > > 
 > > I can't sure you remember my mail for MII lock which modifies
 > > mii_phy_probe API to take an additional mutex. The driver mutex
 > > could be used with MII bus access/callbacks.
 > 
 > Yes, i remember that mail but i didnt had time to dig code deep enough
 > to give an answer. I did it recently, while working on PR.
 > 

Glad to hear that. :-)

 > > If interface is up/running and auto negotiation is in progress MII
 > > layer would inspect BMSR register periodically to know the state
 > > of link. During the time if you run ifconfig(8) to know the state
 > > of the link or to change media type/duplex it will access PHY
 > > registers. Normally it would end up with "link states coalesced"
 > > messages.
 > > 
 > > As you know the two callbacks(vge_ifmedia_upd/vge_ifmedia_sts) will
 > > end up with calling mii_mediachg() or mii_pollstat() which in turn
 > > access PHY registers. So if MII access is properly serialized we
 > > wouldn't get stale data. I guess your fix solves it by protecting
 > > callbacks with driver mutex but it wouldn't fix other cases.
 > > For example see vge_miibus_statchg MII interface.
 > 
 > MII layer does not have it's own callouts, i.e. those autonegotiation 
 > checks of BMSR are triggered by driver's _tick() function, which are locked.
 > So if we have locked _tick() & _ifmedia_(upd|sts) functions, concurrent
 > access to PHY is impossible.
 > (If we are talking about vge_miibus_statchg(), it would be:
 > vge_tick (here we obtain lock) -> mii_tick -> ciphy_service ->
 > mii_phy_update -> vge_miibus_statchg).
 > 

If we set media type manually while autonegotiation is in progress,
wouldn't it access PHY registers without driver lock?

-- 
Regards,
Pyun YongHyeon
Received on Wed Aug 23 2006 - 23:11:12 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:38:59 UTC