[PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Vladimir Oltean]

On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
have interrupt lines connected to the shared IRQ2_B LS1021A pin.

The interrupts are active low, but the GICv2 controller does not support
active-low and falling-edge interrupts, so the only mode it can be
configured in is rising-edge.

The interrupt number was obtained by subtracting 32 from the listed
interrupt ID from LS1021ARM.pdf Table 5-1. Interrupt assignments.

Switching to interrupts offloads the PHY library from the task of
polling the MDIO status and AN registers (1, 4, 5) every second.

Unfortunately, the BCM5464R quad PHY connected to the switch does not
appear to have an interrupt line routed to the SoC.

Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
 arch/arm/boot/dts/ls1021a-tsn.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/ls1021a-tsn.dts b/arch/arm/boot/dts/ls1021a-tsn.dts
index 5b7689094b70..4532b2bd3fd1 100644
--- a/arch/arm/boot/dts/ls1021a-tsn.dts
+++ b/arch/arm/boot/dts/ls1021a-tsn.dts
@@ -203,11 +203,15 @@
 	/* AR8031 */
 	sgmii_phy1: ethernet-phy@1 {
 		reg = <0x1>;
+		/* SGMII1_PHY_INT_B: connected to IRQ2, active low */
+		interrupts = <GIC_SPI 165 IRQ_TYPE_EDGE_RISING>;
 	};
 
 	/* AR8031 */
 	sgmii_phy2: ethernet-phy@2 {
 		reg = <0x2>;
+		/* SGMII2_PHY_INT_B: connected to IRQ2, active low */
+		interrupts = <GIC_SPI 165 IRQ_TYPE_EDGE_RISING>;
 	};
 
 	/* BCM5464 quad PHY */
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Andrew Lunn]

On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
> 
> The interrupts are active low, but the GICv2 controller does not support
> active-low and falling-edge interrupts, so the only mode it can be
> configured in is rising-edge.

Hi Vladimir

So how does this work? The rising edge would occur after the interrupt
handler has completed? What triggers the interrupt handler?

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Vladimir Oltean]

On 09/11/2019, Andrew Lunn <andrew@lunn.ch> wrote:
> On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
>> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
>> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
>>
>> The interrupts are active low, but the GICv2 controller does not support
>> active-low and falling-edge interrupts, so the only mode it can be
>> configured in is rising-edge.
>
> Hi Vladimir
>
> So how does this work? The rising edge would occur after the interrupt
> handler has completed? What triggers the interrupt handler?
>
> 	Andrew
>

Hi Andrew,

I hope I am not terribly confused about this. I thought I am telling
the interrupt controller to raise an IRQ as a result of the
low-to-high transition of the electrical signal. Experimentation sure
seems to agree with me. So the IRQ is generated immediately _after_
the PHY has left the line in open drain and it got pulled up to Vdd.

Thanks,
-Vladimir

[Sorry for the repost, for some reason Gmail decided to send this
email as html earlier]

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Andrew Lunn]

On Sat, Nov 09, 2019 at 05:16:48PM +0200, Vladimir Oltean wrote:
> On Saturday, 9 November 2019, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
> >> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
> >> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
> >>
> >> The interrupts are active low, but the GICv2 controller does not support
> >> active-low and falling-edge interrupts, so the only mode it can be
> >> configured in is rising-edge.
> >
> > Hi Vladimir
> >
> > So how does this work? The rising edge would occur after the interrupt
> > handler has completed? What triggers the interrupt handler?
> >
> >         Andrew
> >
> 
> Hi Andrew,
> 
> I hope I am not terribly confused about this. I thought I am telling the
> interrupt controller to raise an IRQ as a result of the low-to-high transition
> of the electrical signal. Experimentation sure seems to agree with me. So the
> IRQ is generated immediately _after_ the PHY has left the line in open drain
> and it got pulled up to Vdd.

Hi Vladimir

                       t1                    t2

     ------------------\                     /----------------
                        \-------------------/

The interrupt output is active low. So it is high by default. At time
t1 something happens, say the link is established. The interrupt
becomes active, we have a failing edge. We want the interrupt
controller to fire. Lets say it does. The interrupt handler runs, and
clears the interrupt cause. This is at time t2. We then get a rising
edge and the PHY releases the interrupt, and the level returns to
high.

So how does this work if you have the interrupt controller triggering
on a rising edge? The edge won't rise until the interrupt handler
finishes its work.

	 Andrew

   

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Alexander Stein]

 On Saturday, November 9, 2019, 4:21:51 PM CET Vladimir Oltean wrote:
> On 09/11/2019, Andrew Lunn <andrew@lunn.ch> wrote:
> > On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
> >> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
> >> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
> >>
> >> The interrupts are active low, but the GICv2 controller does not support
> >> active-low and falling-edge interrupts, so the only mode it can be
> >> configured in is rising-edge.
> >
> > Hi Vladimir
> >
> > So how does this work? The rising edge would occur after the interrupt
> > handler has completed? What triggers the interrupt handler?
> >
> > 	Andrew
> >
> 
> Hi Andrew,
> 
> I hope I am not terribly confused about this. I thought I am telling
> the interrupt controller to raise an IRQ as a result of the
> low-to-high transition of the electrical signal. Experimentation sure
> seems to agree with me. So the IRQ is generated immediately _after_
> the PHY has left the line in open drain and it got pulled up to Vdd.

It is correct GIC only supports raising edge and active-high. The IRQ[0:5] on ls1021a are a bit special though.
They not directly connected to GIC, but there is an optional inverter, enabled by default. See RM for register SCFG_INTPCR.
If left to default, those pins get actually active-high internally.
There was a patch 2 years ago to add support for this inverter: https://lore.kernel.org/patchwork/patch/860993/

Best regards,
Alexander



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Andrew Lunn]

On Sat, Nov 09, 2019 at 08:52:54PM +0100, Alexander Stein wrote:
>  On Saturday, November 9, 2019, 4:21:51 PM CET Vladimir Oltean wrote:
> > On 09/11/2019, Andrew Lunn <andrew@lunn.ch> wrote:
> > > On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
> > >> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
> > >> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
> > >>
> > >> The interrupts are active low, but the GICv2 controller does not support
> > >> active-low and falling-edge interrupts, so the only mode it can be
> > >> configured in is rising-edge.
> > >
> > > Hi Vladimir
> > >
> > > So how does this work? The rising edge would occur after the interrupt
> > > handler has completed? What triggers the interrupt handler?
> > >
> > > 	Andrew
> > >
> > 
> > Hi Andrew,
> > 
> > I hope I am not terribly confused about this. I thought I am telling
> > the interrupt controller to raise an IRQ as a result of the
> > low-to-high transition of the electrical signal. Experimentation sure
> > seems to agree with me. So the IRQ is generated immediately _after_
> > the PHY has left the line in open drain and it got pulled up to Vdd.
> 

> It is correct GIC only supports raising edge and active-high. The
> IRQ[0:5] on ls1021a are a bit special though.  They not directly
> connected to GIC, but there is an optional inverter, enabled by
> default.

Ah, O.K. So configuring for a rising edge is actually giving a falling
edge. Which is why it works.

Actually supporting this correctly is going a cause some pain. I
wonder how many DT files currently say rising/active high, when in
fact falling/active low is actually being used? And when the IRQ
controller really does support active low and falling, things brake?

Vladimir, since this is a shared interrupt, you really should use
active low here. Maybe the first step is to get control of the
inverter, and define a DT binding which is not going to break
backwards compatibility. And then wire up this interrupt.

	  Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Vladimir Oltean]

On Sat, 9 Nov 2019 at 23:05, Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Sat, Nov 09, 2019 at 08:52:54PM +0100, Alexander Stein wrote:
> >  On Saturday, November 9, 2019, 4:21:51 PM CET Vladimir Oltean wrote:
> > > On 09/11/2019, Andrew Lunn <andrew@lunn.ch> wrote:
> > > > On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
> > > >> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
> > > >> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
> > > >>
> > > >> The interrupts are active low, but the GICv2 controller does not support
> > > >> active-low and falling-edge interrupts, so the only mode it can be
> > > >> configured in is rising-edge.
> > > >
> > > > Hi Vladimir
> > > >
> > > > So how does this work? The rising edge would occur after the interrupt
> > > > handler has completed? What triggers the interrupt handler?
> > > >
> > > >   Andrew
> > > >
> > >
> > > Hi Andrew,
> > >
> > > I hope I am not terribly confused about this. I thought I am telling
> > > the interrupt controller to raise an IRQ as a result of the
> > > low-to-high transition of the electrical signal. Experimentation sure
> > > seems to agree with me. So the IRQ is generated immediately _after_
> > > the PHY has left the line in open drain and it got pulled up to Vdd.
> >
>
> > It is correct GIC only supports raising edge and active-high. The
> > IRQ[0:5] on ls1021a are a bit special though.  They not directly
> > connected to GIC, but there is an optional inverter, enabled by
> > default.
>
> Ah, O.K. So configuring for a rising edge is actually giving a falling
> edge. Which is why it works.
>
> Actually supporting this correctly is going a cause some pain. I
> wonder how many DT files currently say rising/active high, when in
> fact falling/active low is actually being used? And when the IRQ
> controller really does support active low and falling, things brake?
>
> Vladimir, since this is a shared interrupt, you really should use
> active low here. Maybe the first step is to get control of the
> inverter, and define a DT binding which is not going to break
> backwards compatibility. And then wire up this interrupt.
>
>           Andrew

Oh, ok, this is what you mean, thanks Alexander for the clarification.
This sure escalated quickly and is going to keep me busy for a while.

-Vladimir

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs

[Vladimir Oltean]

On Sat, 9 Nov 2019 at 23:37, Vladimir Oltean <olteanv@gmail.com> wrote:
>
> On Sat, 9 Nov 2019 at 23:05, Andrew Lunn <andrew@lunn.ch> wrote:
> >
> > On Sat, Nov 09, 2019 at 08:52:54PM +0100, Alexander Stein wrote:
> > >  On Saturday, November 9, 2019, 4:21:51 PM CET Vladimir Oltean wrote:
> > > > On 09/11/2019, Andrew Lunn <andrew@lunn.ch> wrote:
> > > > > On Sat, Nov 09, 2019 at 12:56:42PM +0200, Vladimir Oltean wrote:
> > > > >> On the LS1021A-TSN board, the 2 Atheros AR8031 PHYs for eth0 and eth1
> > > > >> have interrupt lines connected to the shared IRQ2_B LS1021A pin.
> > > > >>
> > > > >> The interrupts are active low, but the GICv2 controller does not support
> > > > >> active-low and falling-edge interrupts, so the only mode it can be
> > > > >> configured in is rising-edge.
> > > > >
> > > > > Hi Vladimir
> > > > >
> > > > > So how does this work? The rising edge would occur after the interrupt
> > > > > handler has completed? What triggers the interrupt handler?
> > > > >
> > > > >   Andrew
> > > > >
> > > >
> > > > Hi Andrew,
> > > >
> > > > I hope I am not terribly confused about this. I thought I am telling
> > > > the interrupt controller to raise an IRQ as a result of the
> > > > low-to-high transition of the electrical signal. Experimentation sure
> > > > seems to agree with me. So the IRQ is generated immediately _after_
> > > > the PHY has left the line in open drain and it got pulled up to Vdd.
> > >
> >
> > > It is correct GIC only supports raising edge and active-high. The
> > > IRQ[0:5] on ls1021a are a bit special though.  They not directly
> > > connected to GIC, but there is an optional inverter, enabled by
> > > default.
> >
> > Ah, O.K. So configuring for a rising edge is actually giving a falling
> > edge. Which is why it works.
> >
> > Actually supporting this correctly is going a cause some pain. I
> > wonder how many DT files currently say rising/active high, when in
> > fact falling/active low is actually being used? And when the IRQ
> > controller really does support active low and falling, things brake?
> >
> > Vladimir, since this is a shared interrupt, you really should use
> > active low here. Maybe the first step is to get control of the
> > inverter, and define a DT binding which is not going to break
> > backwards compatibility. And then wire up this interrupt.
> >
> >           Andrew
>
> Oh, ok, this is what you mean, thanks Alexander for the clarification.
> This sure escalated quickly and is going to keep me busy for a while.
>
> -Vladimir

Sorry, I'm still a bit in shock, since this hit me in the face from
nowhere, so I hadn't followed the entire history when I sent the above
email.
It looks after all that Kurt and Rasmus have picked this up again and
that the latest patch set is from 2 days ago, I'll take a look at
that...
https://lwn.net/Articles/804103/

Thanks,
-Vladimir

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel