Messages in this thread |  | | Date | Thu, 18 Jan 2024 18:36:16 +0100 | | Subject | Re: [PATCH] net: phy: adin1100: Fix nullptr exception for phy interrupts | | From | Heiner Kallweit <> |
| |
On 18.01.2024 17:53, Andrew Lunn wrote: > On Thu, Jan 18, 2024 at 11:43:41AM +0100, Andre Werner wrote: >> If using ADIN1100 as an external phy, e.g. in combination with >> "smsc95xx", we ran into nullptr exception by creating a link. >> >> In our case the "smsc95xx" does not check for an available interrupt handler >> on external phy driver to use poll instead of interrupts if no handler is >> available. So we decide to implement a small handler in the phy driver >> to support other MACs as well. >> >> I update the driver to add an interrupt handler because libphy >> does not check if their is a interrupt handler available either. >> >> There are several interrupts maskable at the phy, but only link change interrupts >> are handled by the driver yet. >> >> We tested the combination "smsc95xx" and "adin1100" with Linux Kernel 6.6.9 >> and Linux Kernel 6.1.0, respectively. > > Hi Andre > > A few different things.... > > Please could you give more details of the null pointer > exception. phylib should test if the needed methods have been > implemented in the PHY driver, and not tried to use interrupts when > they are missing. It should of polled the PHY. So i would like to > understand what went wrong. Maybe we have a phylib core bug we should > be fixing. Or a bug in the smsc95xx driver. > Seems to be a bug in smsc95xx. The following is the relevant code piece.
ret = mdiobus_register(pdata->mdiobus); if (ret) { netdev_err(dev->net, "Could not register MDIO bus\n"); goto free_mdio; }
pdata->phydev = phy_find_first(pdata->mdiobus); if (!pdata->phydev) { netdev_err(dev->net, "no PHY found\n"); ret = -ENODEV; goto unregister_mdio; }
pdata->phydev->irq = phy_irq;
The interrupt is set too late, after phy_probe(), where we have this:
if (!phy_drv_supports_irq(phydrv) && phy_interrupt_is_valid(phydev)) phydev->irq = PHY_POLL;
So we would have two steps: 1. Fix the smsc95xx bug (as the same issue could occur with another PHY type) 2. Add interrupt support to adin1100 as an improvement
> Please take a read of > https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html > > Patches like this should be against net-next, not 6.6.9 etc. Also, > net-next is currently closed due to the merge window being open. Its > fine to post patches, but please mark them RFC until the merge window > is over. > > The patch itself looks O.K, but i would make the commit message more > formal. You can add additional comments under the --- which will not > become part of the git history. > > Andrew
|  |