LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
@ 2019-11-09 10:56 Vladimir Oltean
  2019-11-09 15:09 ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2019-11-09 10:56 UTC (permalink / raw)
  To: shawnguo, mark.rutland, devicetree
  Cc: leoyang.li, robh+dt, linux-arm-kernel, linux-kernel, netdev,
	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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
  2019-11-09 10:56 [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs Vladimir Oltean
@ 2019-11-09 15:09 ` Andrew Lunn
  2019-11-09 15:21   ` Vladimir Oltean
       [not found]   ` <CA+h21hrqczuOhTzWFZKX0XvgjgTzHT=3AdCPvO_eSabOzA3OCQ@mail.gmail.com>
  0 siblings, 2 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-11-09 15:09 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: shawnguo, mark.rutland, devicetree, leoyang.li, robh+dt,
	linux-arm-kernel, linux-kernel, netdev

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
  2019-11-09 15:09 ` Andrew Lunn
@ 2019-11-09 15:21   ` Vladimir Oltean
  2019-11-09 19:52     ` Alexander Stein
       [not found]   ` <CA+h21hrqczuOhTzWFZKX0XvgjgTzHT=3AdCPvO_eSabOzA3OCQ@mail.gmail.com>
  1 sibling, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2019-11-09 15:21 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: shawnguo, mark.rutland, devicetree, leoyang.li, robh+dt,
	linux-arm-kernel, linux-kernel, netdev

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]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
       [not found]   ` <CA+h21hrqczuOhTzWFZKX0XvgjgTzHT=3AdCPvO_eSabOzA3OCQ@mail.gmail.com>
@ 2019-11-09 17:21     ` Andrew Lunn
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Lunn @ 2019-11-09 17:21 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: shawnguo, mark.rutland, devicetree, leoyang.li, robh+dt,
	linux-arm-kernel, linux-kernel, netdev

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

   

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
  2019-11-09 15:21   ` Vladimir Oltean
@ 2019-11-09 19:52     ` Alexander Stein
  2019-11-09 21:05       ` Andrew Lunn
  0 siblings, 1 reply; 8+ messages in thread
From: Alexander Stein @ 2019-11-09 19:52 UTC (permalink / raw)
  To: Vladimir Oltean
  Cc: Andrew Lunn, shawnguo, mark.rutland, devicetree, leoyang.li,
	robh+dt, linux-arm-kernel, linux-kernel, netdev

 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



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
  2019-11-09 19:52     ` Alexander Stein
@ 2019-11-09 21:05       ` Andrew Lunn
  2019-11-09 21:37         ` Vladimir Oltean
  0 siblings, 1 reply; 8+ messages in thread
From: Andrew Lunn @ 2019-11-09 21:05 UTC (permalink / raw)
  To: Alexander Stein
  Cc: Vladimir Oltean, shawnguo, mark.rutland, devicetree, leoyang.li,
	robh+dt, linux-arm-kernel, linux-kernel, netdev

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
  2019-11-09 21:05       ` Andrew Lunn
@ 2019-11-09 21:37         ` Vladimir Oltean
  2019-11-09 21:56           ` Vladimir Oltean
  0 siblings, 1 reply; 8+ messages in thread
From: Vladimir Oltean @ 2019-11-09 21:37 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Alexander Stein, Shawn Guo, Mark Rutland, devicetree, leoyang.li,
	Rob Herring, linux-arm-kernel, lkml, netdev

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs
  2019-11-09 21:37         ` Vladimir Oltean
@ 2019-11-09 21:56           ` Vladimir Oltean
  0 siblings, 0 replies; 8+ messages in thread
From: Vladimir Oltean @ 2019-11-09 21:56 UTC (permalink / raw)
  To: Andrew Lunn, Alexander Stein, Kurt Kanzenbach, Rasmus Villemoes
  Cc: Shawn Guo, Mark Rutland, devicetree, leoyang.li, Rob Herring,
	linux-arm-kernel, lkml, netdev

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, back to index

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-09 10:56 [PATCH] ARM: dts: ls1021a-tsn: Use interrupts for the SGMII PHYs Vladimir Oltean
2019-11-09 15:09 ` Andrew Lunn
2019-11-09 15:21   ` Vladimir Oltean
2019-11-09 19:52     ` Alexander Stein
2019-11-09 21:05       ` Andrew Lunn
2019-11-09 21:37         ` Vladimir Oltean
2019-11-09 21:56           ` Vladimir Oltean
     [not found]   ` <CA+h21hrqczuOhTzWFZKX0XvgjgTzHT=3AdCPvO_eSabOzA3OCQ@mail.gmail.com>
2019-11-09 17:21     ` Andrew Lunn

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git