Re: RFC: PHY Abstraction Layer II

[James Chapman <jchapman@katalix.com>]

Andy Fleming wrote:
> Ok, here's the new patch with changes suggested by James Chapman:

I guess I still have questions about the way interrupts are used.

Using an interrupt to schedule a work queue which then sets a variable
that is used by a timer seems odd. Why not do all the work in the work
queue and schedule it from the interrupt handler or timer?

Many network devices have status bits in their interrupt status
registers to indicate PHY events. These bits are handled in the
network device driver; there is no separate irq. It would obviously be
good to change these drivers to hook up your abstraction layer to
handle PHY functions. You suggested previously that the network driver
and PHY driver could share the irq when the PHY's interrupt is
indicated through the network device's status registers. I still don't
see how SA_SHIRQ could work given that the PHY code uses
disable_irq_nosync() and enable_irq() when handling interrupts. If the
irq is shared with the net device interrupt, this will cause packet
interrupts to stop until the phy's work queue callback is called. If
the irq is shared with other possibly unrelated devices, other bad
effects could occur.

Perhaps the following code (taken from phy_change()) could be called
by the net driver when link state changes are detected by its
interrupt or NAPI poll handler?

if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state)) {
	phydev->state = PHY_CHANGELINK;
	schedule_work(&phydev->phy_queue);
}

In the above case, the net driver would use phy_connect() to connect
its PHY and phydev->irq would be -1 to disable use of PHY interrupts
by the PHY code. Would this work? If so, a new API function for net
drivers to call would be useful.

Also, did you mean to leave the #if 0 code in davicom.c?

/james