Messages in this thread | | | Subject | Re: [RFC PATCH 1/2] net: phy: let the driver register its own IRQ handler | From | Heiner Kallweit <> | Date | Wed, 26 Feb 2020 08:27:58 +0100 |
| |
On 26.02.2020 00:08, Michael Walle wrote: > There are more and more PHY drivers which has more than just the PHY > link change interrupts. For example, temperature thresholds or PTP > interrupts. > > At the moment it is not possible to correctly handle interrupts for PHYs > which has a clear-on-read interrupt status register. It is also likely > that the current approach of the phylib isn't working for all PHYs out > there. > > Therefore, this patch let the PHY driver register its own interrupt > handler. To notify the phylib about a link change, the interrupt handler > has to call the new function phy_drv_interrupt(). >
We have phy_driver callback handle_interrupt for custom interrupt handlers. Any specific reason why you can't use it for your purposes?
> Signed-off-by: Michael Walle <michael@walle.cc> > --- > drivers/net/phy/phy.c | 15 +++++++++++++++ > drivers/net/phy/phy_device.c | 6 ++++-- > include/linux/phy.h | 2 ++ > 3 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c > index d76e038cf2cb..f25aacbcf1d9 100644 > --- a/drivers/net/phy/phy.c > +++ b/drivers/net/phy/phy.c > @@ -942,6 +942,21 @@ void phy_mac_interrupt(struct phy_device *phydev) > } > EXPORT_SYMBOL(phy_mac_interrupt); > > +/** > + * phy_drv_interrupt - PHY drivers says the link has changed > + * @phydev: phy_device struct with changed link > + * > + * The PHY driver may implement his own interrupt handler. It will call this > + * function to notify us about a link change. Trigger the state machine and > + * work a work queue. > + */
This function would duplicate phy_mac_interrupt().
> +void phy_drv_interrupt(struct phy_device *phydev) > +{ > + /* Trigger a state machine change */ > + phy_trigger_machine(phydev); > +} > +EXPORT_SYMBOL(phy_drv_interrupt); > + > static void mmd_eee_adv_to_linkmode(unsigned long *advertising, u16 eee_adv) > { > linkmode_zero(advertising); > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 6a5056e0ae77..6d8c94e61251 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -965,7 +965,8 @@ int phy_connect_direct(struct net_device *dev, struct phy_device *phydev, > return rc; > > phy_prepare_link(phydev, handler); > - if (phy_interrupt_is_valid(phydev)) > + if (phy_interrupt_is_valid(phydev) && > + phydev->drv->flags & PHY_HAS_OWN_IRQ_HANDLER) > phy_request_interrupt(phydev);
Here most likely a ! is missing. because as-is you would break current phylib interrupt mode. Where in the PHY driver (in which callback) do you want to register your own interrupt handler?
> > return 0; > @@ -2411,7 +2412,8 @@ EXPORT_SYMBOL(phy_validate_pause); > > static bool phy_drv_supports_irq(struct phy_driver *phydrv) > { > - return phydrv->config_intr && phydrv->ack_interrupt; > + return ((phydrv->config_intr && phydrv->ack_interrupt) || > + phydrv->flags & PHY_HAS_OWN_IRQ_HANDLER); > } > > /** > diff --git a/include/linux/phy.h b/include/linux/phy.h > index c570e162e05e..46f73b94fd60 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -75,6 +75,7 @@ extern const int phy_10gbit_features_array[1]; > > #define PHY_IS_INTERNAL 0x00000001 > #define PHY_RST_AFTER_CLK_EN 0x00000002 > +#define PHY_HAS_OWN_IRQ_HANDLER 0x00000004 > #define MDIO_DEVICE_IS_PHY 0x80000000 > > /* Interface Mode definitions */ > @@ -1235,6 +1236,7 @@ int phy_drivers_register(struct phy_driver *new_driver, int n, > void phy_state_machine(struct work_struct *work); > void phy_queue_state_machine(struct phy_device *phydev, unsigned long jiffies); > void phy_mac_interrupt(struct phy_device *phydev); > +void phy_drv_interrupt(struct phy_device *phydev); > void phy_start_machine(struct phy_device *phydev); > void phy_stop_machine(struct phy_device *phydev); > void phy_ethtool_ksettings_get(struct phy_device *phydev, >
| |