net: phy: Tell caller result of phy_change()

In 664fcf123a (net: phy: Threaded interrupts allow some simplification)
the phy_interrupt system was changed to use a traditional threaded
interrupt scheme instead of a workqueue approach.

With this change, the phy status check moved into phy_change, which
did not report back to the caller whether or not the interrupt was
handled. This means that, in the case of a shared phy interrupt,
only the first phydev's interrupt registers are checked (since
phy_interrupt() would always return IRQ_HANDLED). This leads to
interrupt storms when it is a secondary device that's actually the
interrupt source.

Signed-off-by: Brad Mouring <brad.mouring@ni.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
This commit is contained in:
Brad Mouring 2018-03-08 16:23:03 -06:00 committed by David S. Miller
parent ddc502dfed
commit a2c054a896
2 changed files with 77 additions and 79 deletions

View File

@ -617,40 +617,6 @@ static void phy_error(struct phy_device *phydev)
phy_trigger_machine(phydev, false);
}
/**
* phy_interrupt - PHY interrupt handler
* @irq: interrupt line
* @phy_dat: phy_device pointer
*
* Description: When a PHY interrupt occurs, the handler disables
* interrupts, and uses phy_change to handle the interrupt.
*/
static irqreturn_t phy_interrupt(int irq, void *phy_dat)
{
struct phy_device *phydev = phy_dat;
if (PHY_HALTED == phydev->state)
return IRQ_NONE; /* It can't be ours. */
phy_change(phydev);
return IRQ_HANDLED;
}
/**
* phy_enable_interrupts - Enable the interrupts from the PHY side
* @phydev: target phy_device struct
*/
static int phy_enable_interrupts(struct phy_device *phydev)
{
int err = phy_clear_interrupt(phydev);
if (err < 0)
return err;
return phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
}
/**
* phy_disable_interrupts - Disable the PHY interrupts from the PHY side
* @phydev: target phy_device struct
@ -677,6 +643,83 @@ phy_err:
return err;
}
/**
* phy_change - Called by the phy_interrupt to handle PHY changes
* @phydev: phy_device struct that interrupted
*/
static irqreturn_t phy_change(struct phy_device *phydev)
{
if (phy_interrupt_is_valid(phydev)) {
if (phydev->drv->did_interrupt &&
!phydev->drv->did_interrupt(phydev))
return IRQ_NONE;
if (phydev->state == PHY_HALTED)
if (phy_disable_interrupts(phydev))
goto phy_err;
}
mutex_lock(&phydev->lock);
if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
phydev->state = PHY_CHANGELINK;
mutex_unlock(&phydev->lock);
/* reschedule state queue work to run as soon as possible */
phy_trigger_machine(phydev, true);
if (phy_interrupt_is_valid(phydev) && phy_clear_interrupt(phydev))
goto phy_err;
return IRQ_HANDLED;
phy_err:
phy_error(phydev);
return IRQ_NONE;
}
/**
* phy_change_work - Scheduled by the phy_mac_interrupt to handle PHY changes
* @work: work_struct that describes the work to be done
*/
void phy_change_work(struct work_struct *work)
{
struct phy_device *phydev =
container_of(work, struct phy_device, phy_queue);
phy_change(phydev);
}
/**
* phy_interrupt - PHY interrupt handler
* @irq: interrupt line
* @phy_dat: phy_device pointer
*
* Description: When a PHY interrupt occurs, the handler disables
* interrupts, and uses phy_change to handle the interrupt.
*/
static irqreturn_t phy_interrupt(int irq, void *phy_dat)
{
struct phy_device *phydev = phy_dat;
if (PHY_HALTED == phydev->state)
return IRQ_NONE; /* It can't be ours. */
return phy_change(phydev);
}
/**
* phy_enable_interrupts - Enable the interrupts from the PHY side
* @phydev: target phy_device struct
*/
static int phy_enable_interrupts(struct phy_device *phydev)
{
int err = phy_clear_interrupt(phydev);
if (err < 0)
return err;
return phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED);
}
/**
* phy_start_interrupts - request and enable interrupts for a PHY device
* @phydev: target phy_device struct
@ -719,50 +762,6 @@ int phy_stop_interrupts(struct phy_device *phydev)
}
EXPORT_SYMBOL(phy_stop_interrupts);
/**
* phy_change - Called by the phy_interrupt to handle PHY changes
* @phydev: phy_device struct that interrupted
*/
void phy_change(struct phy_device *phydev)
{
if (phy_interrupt_is_valid(phydev)) {
if (phydev->drv->did_interrupt &&
!phydev->drv->did_interrupt(phydev))
return;
if (phydev->state == PHY_HALTED)
if (phy_disable_interrupts(phydev))
goto phy_err;
}
mutex_lock(&phydev->lock);
if ((PHY_RUNNING == phydev->state) || (PHY_NOLINK == phydev->state))
phydev->state = PHY_CHANGELINK;
mutex_unlock(&phydev->lock);
/* reschedule state queue work to run as soon as possible */
phy_trigger_machine(phydev, true);
if (phy_interrupt_is_valid(phydev) && phy_clear_interrupt(phydev))
goto phy_err;
return;
phy_err:
phy_error(phydev);
}
/**
* phy_change_work - Scheduled by the phy_mac_interrupt to handle PHY changes
* @work: work_struct that describes the work to be done
*/
void phy_change_work(struct work_struct *work)
{
struct phy_device *phydev =
container_of(work, struct phy_device, phy_queue);
phy_change(phydev);
}
/**
* phy_stop - Bring down the PHY link, and stop checking the status
* @phydev: target phy_device struct

View File

@ -1012,7 +1012,6 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
int phy_drivers_register(struct phy_driver *new_driver, int n,
struct module *owner);
void phy_state_machine(struct work_struct *work);
void phy_change(struct phy_device *phydev);
void phy_change_work(struct work_struct *work);
void phy_mac_interrupt(struct phy_device *phydev);
void phy_start_machine(struct phy_device *phydev);