Am 15.11.2017 um 23:04 schrieb Florian Fainelli: > On 11/12/2017 01:08 PM, Heiner Kallweit wrote: >> After commits c974bdbc3e "net: phy: Use threaded IRQ, to allow IRQ from >> sleeping devices" and 664fcf123a30 "net: phy: Threaded interrupts allow >> some simplification" all relevant code pieces run in process context >> anyway and I don't think we need the disabling of interrupts any longer. >> >> Interestingly enough, latter commit already removed the comment >> explaining why interrupts need to be temporarily disabled. >> >> On my system phy interrupt mode works fine with this patch. >> However I may miss something, especially in the context of shared phy >> interrupts, therefore I'd appreciate if more people could test this. > > I am not currently in a position to test this, but this looks very > similar, if not identical to what Ard submitted a few days earlier: >
Thanks for the hint. Indeed it's exactly the same patch, so the one sent by me can be disregarded. > https://patchwork.kernel.org/patch/10048901/ > > Since net-next is closed at the moment, that should allow us to give > this some good amount of testing. > > Thanks > >> >> Signed-off-by: Heiner Kallweit <hkallwe...@gmail.com> >> --- >> drivers/net/phy/phy.c | 26 ++------------------------ >> include/linux/phy.h | 1 - >> 2 files changed, 2 insertions(+), 25 deletions(-) >> >> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c >> index 2b1e67bc1..b3784c9a2 100644 >> --- a/drivers/net/phy/phy.c >> +++ b/drivers/net/phy/phy.c >> @@ -629,9 +629,6 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat) >> if (PHY_HALTED == phydev->state) >> return IRQ_NONE; /* It can't be ours. */ >> >> - disable_irq_nosync(irq); >> - atomic_inc(&phydev->irq_disable); >> - >> phy_change(phydev); >> >> return IRQ_HANDLED; >> @@ -689,7 +686,6 @@ static int phy_disable_interrupts(struct phy_device >> *phydev) >> */ >> int phy_start_interrupts(struct phy_device *phydev) >> { >> - atomic_set(&phydev->irq_disable, 0); >> if (request_threaded_irq(phydev->irq, NULL, phy_interrupt, >> IRQF_ONESHOT | IRQF_SHARED, >> phydev_name(phydev), phydev) < 0) { >> @@ -716,13 +712,6 @@ int phy_stop_interrupts(struct phy_device *phydev) >> >> free_irq(phydev->irq, phydev); >> >> - /* If work indeed has been cancelled, disable_irq() will have >> - * been left unbalanced from phy_interrupt() and enable_irq() >> - * has to be called so that other devices on the line work. >> - */ >> - while (atomic_dec_return(&phydev->irq_disable) >= 0) >> - enable_irq(phydev->irq); >> - >> return err; >> } >> EXPORT_SYMBOL(phy_stop_interrupts); >> @@ -736,7 +725,7 @@ void phy_change(struct phy_device *phydev) >> if (phy_interrupt_is_valid(phydev)) { >> if (phydev->drv->did_interrupt && >> !phydev->drv->did_interrupt(phydev)) >> - goto ignore; >> + return; >> >> if (phy_disable_interrupts(phydev)) >> goto phy_err; >> @@ -748,27 +737,16 @@ void phy_change(struct phy_device *phydev) >> mutex_unlock(&phydev->lock); >> >> if (phy_interrupt_is_valid(phydev)) { >> - atomic_dec(&phydev->irq_disable); >> - enable_irq(phydev->irq); >> - >> /* Reenable interrupts */ >> if (PHY_HALTED != phydev->state && >> phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED)) >> - goto irq_enable_err; >> + goto phy_err; >> } >> >> /* reschedule state queue work to run as soon as possible */ >> phy_trigger_machine(phydev, true); >> return; >> >> -ignore: >> - atomic_dec(&phydev->irq_disable); >> - enable_irq(phydev->irq); >> - return; >> - >> -irq_enable_err: >> - disable_irq(phydev->irq); >> - atomic_inc(&phydev->irq_disable); >> phy_err: >> phy_error(phydev); >> } >> diff --git a/include/linux/phy.h b/include/linux/phy.h >> index dc82a07cb..8a87e441f 100644 >> --- a/include/linux/phy.h >> +++ b/include/linux/phy.h >> @@ -468,7 +468,6 @@ struct phy_device { >> /* Interrupt and Polling infrastructure */ >> struct work_struct phy_queue; >> struct delayed_work state_queue; >> - atomic_t irq_disable; >> >> struct mutex lock; >> >> > >
