Messages in this thread Patch in this message | | | Date | Wed, 9 Aug 2023 17:01:11 +0100 | From | "Russell King (Oracle)" <> | Subject | Re: [PATCH] net: phy: Don't disable irqs on shutdown if WoL is enabled |
| |
On Wed, Aug 09, 2023 at 06:44:18PM +0300, Ioana Ciornei wrote: > On Wed, Aug 09, 2023 at 03:29:17PM +0100, Russell King (Oracle) wrote: > > On Wed, Aug 09, 2023 at 05:21:55PM +0300, Ioana Ciornei wrote: > > > That's a perfect summary of the problem that I was trying to fix. > > > > > > The board in question is a LS1021ATSN which has two AR8031 PHYs that > > > share an interrupt line. In case only one of the PHYs is probed and > > > there are pending interrupts on the PHY#2 an IRQ storm will happen > > > since there is no entity to clear the interrupt from PHY#2's registers. > > > PHY#1's driver will get stuck in .handle_interrupt() indefinitely. > > > > So I have two further questions: > > 1. Is WoL able to be supported on this hardware? > > I don't know if anyone cares about WoL on the AR8031 PHYs from this > board. > > Both of the PHYs are used in conjuction with 2 eTSEC controllers - which > use the driver in drivers/net/ethernet/freescale/gianfar.c. > > The Ethernet controller does have WoL support, which means that WoL could > still be supported on the board even though we would forbid WoL on the > AR8031 PHYs. > > > 2. AR8031 has a seperate WOL_INT signal that can be used to wake up the > > system. Is this used in the hardware design? > > No, WOL_INT is not connected.
Okay, so we don't need to care about WoL for your hardware setup.
Thinking about this, I wonder whether we could solve your issue by disabling interrupts when the PHY is probed, rather than disabling them on shutdown - something like this? (not build tested)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 3e9909b30938..4d1a37487923 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -3216,6 +3216,8 @@ static int phy_probe(struct device *dev) goto out; } + phy_disable_interrupts(phydev); + /* Start out supporting everything. Eventually, * a controller will attach, and may modify one * or both of these values @@ -3333,16 +3335,6 @@ static int phy_remove(struct device *dev) return 0; } -static void phy_shutdown(struct device *dev) -{ - struct phy_device *phydev = to_phy_device(dev); - - if (phydev->state == PHY_READY || !phydev->attached_dev) - return; - - phy_disable_interrupts(phydev); -} - /** * phy_driver_register - register a phy_driver with the PHY layer * @new_driver: new phy_driver to register @@ -3376,7 +3368,6 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner) new_driver->mdiodrv.driver.bus = &mdio_bus_type; new_driver->mdiodrv.driver.probe = phy_probe; new_driver->mdiodrv.driver.remove = phy_remove; - new_driver->mdiodrv.driver.shutdown = phy_shutdown; new_driver->mdiodrv.driver.owner = owner; new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS; -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
| |