On Thu, Dec 23, 2004 at 03:00:13PM -0600, Andy Fleming wrote: > > >Adds a Phy Abstraction Layer which allows ethernet controllers to > >manage their PHYs without knowing the details of how the particular > >PHY device operates. This code steals heavily from BenH's sungem > >driver, and got some stuff out of Jason McMullan's patch.
Some random notes from quick look at the code: 1) IMO if we can extract some info from the PHY using _standard_ registers we should use them, even if the PHY provides some custom ones. I suspect that _all_ XXX_read_status functions for different PHYs in your patch can be easily handled by generic IEEE 802.3 compliant code (you need to update genphy_read_status to properly handle GigE of course). 2) genphy can be changed to handle GigE speeds as well. 3) I think it's better for the genphy case to _detect_ PHY features instead of hard coding PHY_BASIC_FEATURES. In this case you can easily handle 10/100 and 10/100/1000 PHYs by genphy code. 4) Pause negotiation/advertising is completely missing. 5) PHY interrupt sharing looks broken. Although you request_irq using SA_SHIRQ flag, in the handler itself you don't detect whether this interrupt is for this PHY or not, and return IRQ_HANDLED. This will result in first registered PHY hijacking IRQs from other PHYs (or devices) sharing the same IRQ line. -- Eugene
