* [PATCH RFC net-next v2 0/2] net: phy: mxl-gpy: broken interrupt fixes @ 2022-12-28 16:40 Michael Walle 2022-12-28 16:40 ` [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling Michael Walle 2022-12-28 16:40 ` [PATCH RFC net-next v2 2/2] net: phy: mxl-gpy: disable interrupts on GPY215 by default Michael Walle 0 siblings, 2 replies; 6+ messages in thread From: Michael Walle @ 2022-12-28 16:40 UTC (permalink / raw) To: Xu Liang, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel, Michael Walle The GPY215 has a broken interrupt pin. This patch series tries to workaround that and because in general that is not possible, disables the interrupts by default and falls back to polling mode. There is an opt-in via the devicetree. The devicetree binding is missing for now because there is still an ongoing discussion. I'm sending this, because I want to get some feedback on the new handling in the phy core. As Andrew pointed out, we cannot change the irq in the PHY's .probe() because a MAC driver might overwrite it afterwards, e.g. the stmmac does so. Instead introduce a new flag which can be set by the PHY driver and which is evaluated just before the PHY is attached and thus the interrupt is requested. Btw. I'm not sure dev_flags is the correct place here. I couldn't see when to use dev_flags and when to use the plain one-bit properties in the struct phy_device. The latter seems to be used internally, but of course there is at least one exception, the .mac_managed_pm is set by the MAC drivers. v2: - new handling of how to disable the interrupts Michael Walle (2): net: phy: allow a phy to opt-out of interrupt handling net: phy: mxl-gpy: disable interrupts on GPY215 by default drivers/net/phy/mxl-gpy.c | 5 +++++ drivers/net/phy/phy_device.c | 7 +++++++ include/linux/phy.h | 2 ++ 3 files changed, 14 insertions(+) -- 2.30.2 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling 2022-12-28 16:40 [PATCH RFC net-next v2 0/2] net: phy: mxl-gpy: broken interrupt fixes Michael Walle @ 2022-12-28 16:40 ` Michael Walle 2022-12-28 16:49 ` Florian Fainelli 2022-12-28 16:40 ` [PATCH RFC net-next v2 2/2] net: phy: mxl-gpy: disable interrupts on GPY215 by default Michael Walle 1 sibling, 1 reply; 6+ messages in thread From: Michael Walle @ 2022-12-28 16:40 UTC (permalink / raw) To: Xu Liang, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel, Michael Walle Until now, it is not possible for a PHY driver to disable interrupts during runtime. If a driver offers the .config_intr() as well as the .handle_interrupt() ops, it is eligible for interrupt handling. Introduce a new flag for the dev_flags property of struct phy_device, which can be set by PHY driver to skip interrupt setup and fall back to polling mode. At the moment, this is used for the MaxLinear PHY which has broken interrupt handling and there is a need to disable interrupts in some cases. Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/net/phy/phy_device.c | 7 +++++++ include/linux/phy.h | 2 ++ 2 files changed, 9 insertions(+) diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 716870a4499c..e4562859ac00 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1487,6 +1487,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, phydev->interrupts = PHY_INTERRUPT_DISABLED; + /* PHYs can request to use poll mode even though they have an + * associated interrupt line. This could be the case if they + * detect a broken interrupt handling. + */ + if (phydev->dev_flags & PHY_F_NO_IRQ) + phydev->irq = PHY_POLL; + /* Port is set to PORT_TP by default and the actual PHY driver will set * it to different value depending on the PHY configuration. If we have * the generic PHY driver we can't figure it out, thus set the old diff --git a/include/linux/phy.h b/include/linux/phy.h index 71eeb4e3b1fd..f1566c7e47a8 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -82,6 +82,8 @@ extern const int phy_10gbit_features_array[1]; #define PHY_POLL_CABLE_TEST 0x00000004 #define MDIO_DEVICE_IS_PHY 0x80000000 +#define PHY_F_NO_IRQ 0x80000000 + /** * enum phy_interface_t - Interface Mode definitions * -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling 2022-12-28 16:40 ` [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling Michael Walle @ 2022-12-28 16:49 ` Florian Fainelli 2022-12-28 16:54 ` Andrew Lunn 2023-01-03 10:27 ` Russell King (Oracle) 0 siblings, 2 replies; 6+ messages in thread From: Florian Fainelli @ 2022-12-28 16:49 UTC (permalink / raw) To: Michael Walle, Xu Liang, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel On 12/28/2022 8:40 AM, Michael Walle wrote: > Until now, it is not possible for a PHY driver to disable interrupts > during runtime. If a driver offers the .config_intr() as well as the > .handle_interrupt() ops, it is eligible for interrupt handling. > Introduce a new flag for the dev_flags property of struct phy_device, which > can be set by PHY driver to skip interrupt setup and fall back to polling > mode. > > At the moment, this is used for the MaxLinear PHY which has broken > interrupt handling and there is a need to disable interrupts in some > cases. > > Signed-off-by: Michael Walle <michael@walle.cc> > --- > drivers/net/phy/phy_device.c | 7 +++++++ > include/linux/phy.h | 2 ++ > 2 files changed, 9 insertions(+) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 716870a4499c..e4562859ac00 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -1487,6 +1487,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, > > phydev->interrupts = PHY_INTERRUPT_DISABLED; > > + /* PHYs can request to use poll mode even though they have an > + * associated interrupt line. This could be the case if they > + * detect a broken interrupt handling. > + */ > + if (phydev->dev_flags & PHY_F_NO_IRQ) > + phydev->irq = PHY_POLL; Cannot you achieve the same thing with the PHY driver mangling phydev->irq to a negative value, or is that too later already by the time your phy driver's probe function is running? > + > /* Port is set to PORT_TP by default and the actual PHY driver will set > * it to different value depending on the PHY configuration. If we have > * the generic PHY driver we can't figure it out, thus set the old > diff --git a/include/linux/phy.h b/include/linux/phy.h > index 71eeb4e3b1fd..f1566c7e47a8 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -82,6 +82,8 @@ extern const int phy_10gbit_features_array[1]; > #define PHY_POLL_CABLE_TEST 0x00000004 > #define MDIO_DEVICE_IS_PHY 0x80000000 > > +#define PHY_F_NO_IRQ 0x80000000 Kudos for using the appropriate namespace for dev_flags :) -- Florian ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling 2022-12-28 16:49 ` Florian Fainelli @ 2022-12-28 16:54 ` Andrew Lunn 2023-01-03 10:27 ` Russell King (Oracle) 1 sibling, 0 replies; 6+ messages in thread From: Andrew Lunn @ 2022-12-28 16:54 UTC (permalink / raw) To: Florian Fainelli Cc: Michael Walle, Xu Liang, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Wed, Dec 28, 2022 at 08:49:35AM -0800, Florian Fainelli wrote: > > > On 12/28/2022 8:40 AM, Michael Walle wrote: > > Until now, it is not possible for a PHY driver to disable interrupts > > during runtime. If a driver offers the .config_intr() as well as the > > .handle_interrupt() ops, it is eligible for interrupt handling. > > Introduce a new flag for the dev_flags property of struct phy_device, which > > can be set by PHY driver to skip interrupt setup and fall back to polling > > mode. > > > > At the moment, this is used for the MaxLinear PHY which has broken > > interrupt handling and there is a need to disable interrupts in some > > cases. > > > > Signed-off-by: Michael Walle <michael@walle.cc> > > --- > > drivers/net/phy/phy_device.c | 7 +++++++ > > include/linux/phy.h | 2 ++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index 716870a4499c..e4562859ac00 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -1487,6 +1487,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, > > phydev->interrupts = PHY_INTERRUPT_DISABLED; > > + /* PHYs can request to use poll mode even though they have an > > + * associated interrupt line. This could be the case if they > > + * detect a broken interrupt handling. > > + */ > > + if (phydev->dev_flags & PHY_F_NO_IRQ) > > + phydev->irq = PHY_POLL; > > Cannot you achieve the same thing with the PHY driver mangling phydev->irq > to a negative value, or is that too later already by the time your phy > driver's probe function is running? It is actually to early. The interrupt is requested when the MAC attaches the PHY. There are is at least one MAC driver which assigns the phydev->irq just before attaching the PHY, a long time after the PHY has probed. Andrew ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling 2022-12-28 16:49 ` Florian Fainelli 2022-12-28 16:54 ` Andrew Lunn @ 2023-01-03 10:27 ` Russell King (Oracle) 1 sibling, 0 replies; 6+ messages in thread From: Russell King (Oracle) @ 2023-01-03 10:27 UTC (permalink / raw) To: Florian Fainelli Cc: Michael Walle, Xu Liang, Andrew Lunn, Heiner Kallweit, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, netdev, linux-kernel On Wed, Dec 28, 2022 at 08:49:35AM -0800, Florian Fainelli wrote: > > > On 12/28/2022 8:40 AM, Michael Walle wrote: > > Until now, it is not possible for a PHY driver to disable interrupts > > during runtime. If a driver offers the .config_intr() as well as the > > .handle_interrupt() ops, it is eligible for interrupt handling. > > Introduce a new flag for the dev_flags property of struct phy_device, which > > can be set by PHY driver to skip interrupt setup and fall back to polling > > mode. > > > > At the moment, this is used for the MaxLinear PHY which has broken > > interrupt handling and there is a need to disable interrupts in some > > cases. > > > > Signed-off-by: Michael Walle <michael@walle.cc> > > --- > > drivers/net/phy/phy_device.c | 7 +++++++ > > include/linux/phy.h | 2 ++ > > 2 files changed, 9 insertions(+) > > > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > > index 716870a4499c..e4562859ac00 100644 > > --- a/drivers/net/phy/phy_device.c > > +++ b/drivers/net/phy/phy_device.c > > @@ -1487,6 +1487,13 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev, > > phydev->interrupts = PHY_INTERRUPT_DISABLED; > > + /* PHYs can request to use poll mode even though they have an > > + * associated interrupt line. This could be the case if they > > + * detect a broken interrupt handling. > > + */ > > + if (phydev->dev_flags & PHY_F_NO_IRQ) > > + phydev->irq = PHY_POLL; > > Cannot you achieve the same thing with the PHY driver mangling phydev->irq > to a negative value, or is that too later already by the time your phy > driver's probe function is running? > > > + > > /* Port is set to PORT_TP by default and the actual PHY driver will set > > * it to different value depending on the PHY configuration. If we have > > * the generic PHY driver we can't figure it out, thus set the old > > diff --git a/include/linux/phy.h b/include/linux/phy.h > > index 71eeb4e3b1fd..f1566c7e47a8 100644 > > --- a/include/linux/phy.h > > +++ b/include/linux/phy.h > > @@ -82,6 +82,8 @@ extern const int phy_10gbit_features_array[1]; > > #define PHY_POLL_CABLE_TEST 0x00000004 > > #define MDIO_DEVICE_IS_PHY 0x80000000 > > +#define PHY_F_NO_IRQ 0x80000000 > > Kudos for using the appropriate namespace for dev_flags :) But eww for placement. PHY_IS_INTERNAL, PHY_RST_AFTER_CLK_EN, PHY_POLL_CABLE_TEST and MDIO_DEVICE_IS_PHY are all used for the MDIO driver's flags member. This new flag is used for the .dev_flags of phy_device - I feel that it should be separated from the above definitions. I also think it could do with a comment, because it's not obvious for future changes that PHY_F_NO_IRQ is used with .dev_flags. -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last! ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH RFC net-next v2 2/2] net: phy: mxl-gpy: disable interrupts on GPY215 by default 2022-12-28 16:40 [PATCH RFC net-next v2 0/2] net: phy: mxl-gpy: broken interrupt fixes Michael Walle 2022-12-28 16:40 ` [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling Michael Walle @ 2022-12-28 16:40 ` Michael Walle 1 sibling, 0 replies; 6+ messages in thread From: Michael Walle @ 2022-12-28 16:40 UTC (permalink / raw) To: Xu Liang, Andrew Lunn, Heiner Kallweit, Russell King, David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni Cc: netdev, linux-kernel, Michael Walle The interrupts on the GPY215B and GPY215C are broken and the only viable fix is to disable them altogether. There is still the possibilty to opt-in via the device tree. Signed-off-by: Michael Walle <michael@walle.cc> --- drivers/net/phy/mxl-gpy.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/net/phy/mxl-gpy.c b/drivers/net/phy/mxl-gpy.c index 147d7a5a9b35..e5972b4ef6e8 100644 --- a/drivers/net/phy/mxl-gpy.c +++ b/drivers/net/phy/mxl-gpy.c @@ -12,6 +12,7 @@ #include <linux/mutex.h> #include <linux/phy.h> #include <linux/polynomial.h> +#include <linux/property.h> #include <linux/netdevice.h> /* PHY ID */ @@ -292,6 +293,10 @@ static int gpy_probe(struct phy_device *phydev) phydev->priv = priv; mutex_init(&priv->mbox_lock); + if (gpy_has_broken_mdint(phydev) && + !device_property_present(dev, "maxlinear,use-broken-interrupts")) + phydev->dev_flags |= PHY_F_NO_IRQ; + fw_version = phy_read(phydev, PHY_FWV); if (fw_version < 0) return fw_version; -- 2.30.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-03 10:28 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-12-28 16:40 [PATCH RFC net-next v2 0/2] net: phy: mxl-gpy: broken interrupt fixes Michael Walle 2022-12-28 16:40 ` [PATCH RFC net-next v2 1/2] net: phy: allow a phy to opt-out of interrupt handling Michael Walle 2022-12-28 16:49 ` Florian Fainelli 2022-12-28 16:54 ` Andrew Lunn 2023-01-03 10:27 ` Russell King (Oracle) 2022-12-28 16:40 ` [PATCH RFC net-next v2 2/2] net: phy: mxl-gpy: disable interrupts on GPY215 by default Michael Walle
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).