On Thu, Dec 23, 2004 at 03:00:13PM -0600, Andy Fleming wrote:

quoted

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