Added .config_intr and .handle_interrupt callbacks.
Link event interrupt will trigger an interrupt every time when the link
goes up or down.
Signed-off-by: Radu Pirea (NXP OSS) <[email protected]>
---
drivers/net/phy/nxp-c45-tja11xx.c | 33 +++++++++++++++++++++++++++++++
1 file changed, 33 insertions(+)
diff --git a/drivers/net/phy/nxp-c45-tja11xx.c b/drivers/net/phy/nxp-c45-tja11xx.c
index 95307097ebff..54eb74e46cbe 100644
--- a/drivers/net/phy/nxp-c45-tja11xx.c
+++ b/drivers/net/phy/nxp-c45-tja11xx.c
@@ -28,6 +28,11 @@
#define DEVICE_CONTROL_CONFIG_GLOBAL_EN BIT(14)
#define DEVICE_CONTROL_CONFIG_ALL_EN BIT(13)
+#define VEND1_PHY_IRQ_ACK 0x80A0
+#define VEND1_PHY_IRQ_EN 0x80A1
+#define VEND1_PHY_IRQ_STATUS 0x80A2
+#define PHY_IRQ_LINK_EVENT BIT(1)
+
#define VEND1_PHY_CONTROL 0x8100
#define PHY_CONFIG_EN BIT(14)
#define PHY_START_OP BIT(0)
@@ -188,6 +193,32 @@ static int nxp_c45_start_op(struct phy_device *phydev)
PHY_START_OP);
}
+static int nxp_c45_config_intr(struct phy_device *phydev)
+{
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ return phy_set_bits_mmd(phydev, MDIO_MMD_VEND1,
+ VEND1_PHY_IRQ_EN, PHY_IRQ_LINK_EVENT);
+ else
+ return phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1,
+ VEND1_PHY_IRQ_EN, PHY_IRQ_LINK_EVENT);
+}
+
+static irqreturn_t nxp_c45_handle_interrupt(struct phy_device *phydev)
+{
+ irqreturn_t ret = IRQ_NONE;
+ int irq;
+
+ irq = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_STATUS);
+ if (irq & PHY_IRQ_LINK_EVENT) {
+ phy_trigger_machine(phydev);
+ phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_ACK,
+ PHY_IRQ_LINK_EVENT);
+ ret = IRQ_HANDLED;
+ }
+
+ return ret;
+}
+
static int nxp_c45_soft_reset(struct phy_device *phydev)
{
int ret;
@@ -560,6 +591,8 @@ static struct phy_driver nxp_c45_driver[] = {
.soft_reset = nxp_c45_soft_reset,
.config_aneg = nxp_c45_config_aneg,
.config_init = nxp_c45_config_init,
+ .config_intr = nxp_c45_config_intr,
+ .handle_interrupt = nxp_c45_handle_interrupt,
.read_status = nxp_c45_read_status,
.suspend = genphy_c45_pma_suspend,
.resume = genphy_c45_pma_resume,
base-commit: cad4162a90aeff737a16c0286987f51e927f003a
--
2.31.1
> +static irqreturn_t nxp_c45_handle_interrupt(struct phy_device *phydev)
> +{
> + irqreturn_t ret = IRQ_NONE;
> + int irq;
> +
> + irq = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_STATUS);
> + if (irq & PHY_IRQ_LINK_EVENT) {
> + phy_trigger_machine(phydev);
> + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_ACK,
> + PHY_IRQ_LINK_EVENT);
The ordering here is interesting. Could phy_trigger_machine() cause a
second interrupt? Which you then clear without acting upon before
exiting the interrupt handler? I think you should ACK the interrupt
before calling phy_trigger_machine().
Andrew
On Fri, Apr 23, 2021 at 03:07:51PM +0200, Andrew Lunn wrote:
> > +static irqreturn_t nxp_c45_handle_interrupt(struct phy_device *phydev)
> > +{
> > + irqreturn_t ret = IRQ_NONE;
> > + int irq;
> > +
> > + irq = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_STATUS);
> > + if (irq & PHY_IRQ_LINK_EVENT) {
> > + phy_trigger_machine(phydev);
> > + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_ACK,
> > + PHY_IRQ_LINK_EVENT);
>
> The ordering here is interesting. Could phy_trigger_machine() cause a
> second interrupt? Which you then clear without acting upon before
> exiting the interrupt handler? I think you should ACK the interrupt
> before calling phy_trigger_machine().
I thought that the irqchip driver keeps the interrupt line disabled
until the handler finishes, and that recursive interrupts aren't a thing
in Linux? Even with threaded interrupts, I thought this is what
IRQF_ONESHOT deals with.
On Fri, Apr 23, 2021 at 04:42:29PM +0300, Vladimir Oltean wrote:
> On Fri, Apr 23, 2021 at 03:07:51PM +0200, Andrew Lunn wrote:
> > > +static irqreturn_t nxp_c45_handle_interrupt(struct phy_device *phydev)
> > > +{
> > > + irqreturn_t ret = IRQ_NONE;
> > > + int irq;
> > > +
> > > + irq = phy_read_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_STATUS);
> > > + if (irq & PHY_IRQ_LINK_EVENT) {
> > > + phy_trigger_machine(phydev);
> > > + phy_write_mmd(phydev, MDIO_MMD_VEND1, VEND1_PHY_IRQ_ACK,
> > > + PHY_IRQ_LINK_EVENT);
> >
> > The ordering here is interesting. Could phy_trigger_machine() cause a
> > second interrupt? Which you then clear without acting upon before
> > exiting the interrupt handler? I think you should ACK the interrupt
> > before calling phy_trigger_machine().
>
> I thought that the irqchip driver keeps the interrupt line disabled
> until the handler finishes, and that recursive interrupts aren't a thing
> in Linux? Even with threaded interrupts, I thought this is what
> IRQF_ONESHOT deals with.
Ah, you mean the driver could be ACKing an event which was not the event
that triggered this IRQ. In that case, the ordering should be reversed,
sorry for the noise.
> Ah, you mean the driver could be ACKing an event which was not the event
> that triggered this IRQ. In that case, the ordering should be reversed,
> sorry for the noise.
Yes, that is what i was meaning.
Andrew