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
|