| Message ID | 20201112155513.411604-1-ciorneiioana@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | net: phy: add support for shared interrupts (part 2) | expand |
On Thu, 12 Nov 2020 17:54:55 +0200 Ioana Ciornei wrote: > From: Ioana Ciornei <ioana.ciornei@nxp.com> > > This patch set aims to actually add support for shared interrupts in > phylib and not only for multi-PHY devices. While we are at it, > streamline the interrupt handling in phylib. Ioana, would you mind resending? Looks like the kernel.org patchwork instance is way less reliable at piecing series together :(
On Thu, Nov 12, 2020 at 06:19:10PM -0800, Jakub Kicinski wrote: > On Thu, 12 Nov 2020 17:54:55 +0200 Ioana Ciornei wrote: > > From: Ioana Ciornei <ioana.ciornei@nxp.com> > > > > This patch set aims to actually add support for shared interrupts in > > phylib and not only for multi-PHY devices. While we are at it, > > streamline the interrupt handling in phylib. > > Ioana, would you mind resending? Looks like the kernel.org patchwork > instance is way less reliable at piecing series together :( No problem, I'll resend. So from now on we'll be using the kernel.org patchwork only, right? I am asking this because from what I can tell the ozlabs patchwork was not updated in the last days. Ioana
On Fri, 13 Nov 2020 08:17:00 +0000 Ioana Ciornei wrote: > On Thu, Nov 12, 2020 at 06:19:10PM -0800, Jakub Kicinski wrote: > > On Thu, 12 Nov 2020 17:54:55 +0200 Ioana Ciornei wrote: > > > From: Ioana Ciornei <ioana.ciornei@nxp.com> > > > > > > This patch set aims to actually add support for shared interrupts in > > > phylib and not only for multi-PHY devices. While we are at it, > > > streamline the interrupt handling in phylib. > > > > Ioana, would you mind resending? Looks like the kernel.org patchwork > > instance is way less reliable at piecing series together :( > > No problem, I'll resend. > > So from now on we'll be using the kernel.org patchwork only, right? I > am asking this because from what I can tell the ozlabs patchwork was not > updated in the last days. Yup, that's the plan. We're keeping ozlabs going for a bit just in case kernel.org is acting up (like if the need to repost stuff persists), but the plan is to stop the patchwork entry for netdev on ozlabs once things settle. From where I live ozlabs takes 1s to respond to a request while kernel.org instance takes 0.2s.
From: Ioana Ciornei <ioana.ciornei@nxp.com> This patch set aims to actually add support for shared interrupts in phylib and not only for multi-PHY devices. While we are at it, streamline the interrupt handling in phylib. For a bit of context, at the moment, there are multiple phy_driver ops that deal with this subject: - .config_intr() - Enable/disable the interrupt line. - .ack_interrupt() - Should quiesce any interrupts that may have been fired. It's also used by phylib in conjunction with .config_intr() to clear any pending interrupts after the line was disabled, and before it is going to be enabled. - .did_interrupt() - Intended for multi-PHY devices with a shared IRQ line and used by phylib to discern which PHY from the package was the one that actually fired the interrupt. - .handle_interrupt() - Completely overrides the default interrupt handling logic from phylib. The PHY driver is responsible for checking if any interrupt was fired by the respective PHY and choose accordingly if it's the one that should trigger the link state machine. From my point of view, the interrupt handling in phylib has become somewhat confusing with all these callbacks that actually read the same PHY register - the interrupt status. A more streamlined approach would be to just move the responsibility to write an interrupt handler to the driver (as any other device driver does) and make .handle_interrupt() the only way to deal with interrupts. Another advantage with this approach would be that phylib would gain support for shared IRQs between different PHY (not just multi-PHY devices), something which at the moment would require extending every PHY driver anyway in order to implement their .did_interrupt() callback and duplicate the same logic as in .ack_interrupt(). The disadvantage of making .did_interrupt() mandatory would be that we are slightly changing the semantics of the phylib API and that would increase confusion instead of reducing it. What I am proposing is the following: - As a first step, make the .ack_interrupt() callback optional so that we do not break any PHY driver amid the transition. - Every PHY driver gains a .handle_interrupt() implementation that, for the most part, would look like below: irq_status = phy_read(phydev, INTR_STATUS); if (irq_status < 0) { phy_error(phydev); return IRQ_NONE; } if (!(irq_status & irq_mask)) return IRQ_NONE; phy_trigger_machine(phydev); return IRQ_HANDLED; - Remove each PHY driver's implementation of the .ack_interrupt() by actually taking care of quiescing any pending interrupts before enabling/after disabling the interrupt line. - Finally, after all drivers have been ported, remove the .ack_interrupt() and .did_interrupt() callbacks from phy_driver. This patch set is part 2 of the entire change set and it addresses the changes needed in 9 PHY drivers. The rest can be found on my Github branch here: https://github.com/IoanaCiornei/linux/commits/phylib-shared-irq I do not have access to most of these PHY's, therefore I Cc-ed the latest contributors to the individual PHY drivers in order to have access, hopefully, to more regression testing. Ioana Ciornei (18): net: phy: vitesse: implement generic .handle_interrupt() callback net: phy: vitesse: remove the use of .ack_interrupt() net: phy: microchip: implement generic .handle_interrupt() callback net: phy: microchip: remove the use of .ack_interrupt() net: phy: marvell: implement generic .handle_interrupt() callback net: phy: marvell: remove the use of .ack_interrupt() net: phy: lxt: implement generic .handle_interrupt() callback net: phy: lxt: remove the use of .ack_interrupt() net: phy: nxp-tja11xx: implement generic .handle_interrupt() callback net: phy: nxp-tja11xx: remove the use of .ack_interrupt() net: phy: amd: implement generic .handle_interrupt() callback net: phy: amd: remove the use of .ack_interrupt() net: phy: smsc: implement generic .handle_interrupt() callback net: phy: smsc: remove the use of .ack_interrupt() net: phy: ste10Xp: implement generic .handle_interrupt() callback net: phy: ste10Xp: remove the use of .ack_interrupt() net: phy: adin: implement generic .handle_interrupt() callback net: phy: adin: remove the use of the .ack_interrupt() drivers/net/phy/adin.c | 45 +++++++++++++--- drivers/net/phy/amd.c | 37 +++++++++++-- drivers/net/phy/lxt.c | 94 ++++++++++++++++++++++++++++++---- drivers/net/phy/marvell.c | 88 ++++++++++++++++--------------- drivers/net/phy/microchip.c | 24 +++++++-- drivers/net/phy/microchip_t1.c | 28 +++++++--- drivers/net/phy/nxp-tja11xx.c | 42 +++++++++++++-- drivers/net/phy/smsc.c | 55 ++++++++++++++++---- drivers/net/phy/ste10Xp.c | 53 +++++++++++++------ drivers/net/phy/vitesse.c | 61 ++++++++++++++-------- 10 files changed, 405 insertions(+), 122 deletions(-) Cc: Alexandru Ardelean <alexandru.ardelean@analog.com> Cc: Andre Edich <andre.edich@microchip.com> Cc: Baruch Siach <baruch@tkos.co.il> Cc: Christophe Leroy <christophe.leroy@c-s.fr> Cc: Kavya Sree Kotagiri <kavyasree.kotagiri@microchip.com> Cc: Linus Walleij <linus.walleij@linaro.org> Cc: Marco Felsch <m.felsch@pengutronix.de> Cc: Marek Vasut <marex@denx.de> Cc: Maxim Kochetkov <fido_max@inbox.ru> Cc: Nisar Sayed <Nisar.Sayed@microchip.com> Cc: Oleksij Rempel <o.rempel@pengutronix.de> Cc: Robert Hancock <robert.hancock@calian.com> Cc: Yuiko Oshino <yuiko.oshino@microchip.com>