From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:30:22
Hello Dave,
This series adds support to the ftgmac100 driver for the Aspeed ast2400 and
ast2500 SoCs. In particular, they ensure the driver works correctly on the
ast2500 where the MAC block has seen some changes in register layout.
They have been tested on ast2400 and ast2500 systems with the NCSI stack and
with a directly attached PHY.
Cheers,
Joel
Andrew Jeffery (2):
net/ftgmac100: Separate rx page storage from rxdesc
net/ftgmac100: Make EDO{R,T}R bits configurable
Gavin Shan (2):
net/faraday: Avoid PHYSTS_CHG interrupt
net/faraday: Clear stale interrupts
Joel Stanley (3):
net/ftgmac100: Adapt for Aspeed SoCs
net/faraday: Fix phy link irq on Aspeed G5 SoCs
net/faraday: Configure old MDIO interface on Aspeed SoCs
drivers/net/ethernet/faraday/ftgmac100.c | 92 ++++++++++++++++++++++++--------
drivers/net/ethernet/faraday/ftgmac100.h | 8 ++-
2 files changed, 77 insertions(+), 23 deletions(-)
--
2.9.3
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:30:33
From: Andrew Jeffery <redacted>
The ftgmac100 hardware revision in e.g. the Aspeed AST2500 no longer
reserves all bits in RXDES#2 but instead uses the bottom 16 bits to
store MAC frame metadata. Avoid corruption by shifting struct page
pointers out to their own member in struct ftgmac100.
Signed-off-by: Andrew Jeffery <redacted>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:30:39
From: Andrew Jeffery <redacted>
These bits are #defined at a fixed location. In order to support future
hardware that has chosen to move these bits around move the bits into a
member of the struct ftgmac100.
Signed-off-by: Andrew Jeffery <redacted>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 40 +++++++++++++++++++++-----------
drivers/net/ethernet/faraday/ftgmac100.h | 2 --
2 files changed, 26 insertions(+), 16 deletions(-)
@@ -556,10 +561,11 @@ static bool ftgmac100_rx_packet(struct ftgmac100 *priv, int *processed)/*******************************************************************************internalfunctions(transmitdescriptor)*****************************************************************************/-staticvoidftgmac100_txdes_reset(structftgmac100_txdes*txdes)+staticvoidftgmac100_txdes_reset(conststructftgmac100*priv,+structftgmac100_txdes*txdes){/* clear all except end of ring bit */-txdes->txdes0&=cpu_to_le32(FTGMAC100_TXDES0_EDOTR);+txdes->txdes0&=cpu_to_le32(priv->txdes0_edotr_mask);txdes->txdes1=0;txdes->txdes2=0;txdes->txdes3=0;
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:30:47
The RXDES and TXDES registers bits in the ftgmac100 indicates EDO{R,T}R
at bit position 15 for the Faraday Tech IP. However, the version of this
IP present in the Aspeed SoCs has these bits at position 30 in the
registers.
It appers that ast2400 SoCs support both positions, with the 15th bit
marked as reserved but still functional. In the ast2500 this bit is
reused for another function, so we need a work around.
This was confirmed with engineers from Aspeed that using bit 30 is
correct for both the ast2400 and ast2500 SoCs.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:30:55
From: Gavin Shan <redacted>
Bit#11 in MACCR (0x50) designates the signal level for PHY link
status change. It's cleared, meaning high level enabled, by default.
However, we can see continuous interrupt (bit#6) in ISR (0x0) for it
and it's obviously a false alarm. The side effect is CPU cycles wasted
to process the false alarm.
This sets bit#11 in MACCR (0x50) to avoid the bogus interrupt.
Signed-off-by: Gavin Shan <redacted>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 1 +
drivers/net/ethernet/faraday/ftgmac100.h | 1 +
2 files changed, 2 insertions(+)
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:31:04
From: Gavin Shan <redacted>
There is stale interrupt (PHYSTS_CHG in ISR, bit#6 in 0x0) from
the bootloader (uboot) when enabling the MAC. The stale interrupts
aren't part of kernel and should be cleared.
This clears the stale interrupts in ISR (0x0) when enabling the MAC.
Signed-off-by: Gavin Shan <redacted>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 6 ++++++
1 file changed, 6 insertions(+)
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:31:10
On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
continual PHYSTS interrupts:
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
This is because the driver was enabling low-level sensitive interrupt
generation where the systems are wired for high-level. All CPU cycles
are spent servicing this interrupt.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 4 ++++
1 file changed, 4 insertions(+)
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-20 06:31:19
The Aspeed SoCs have a new MDIO interface as an option in the G4 and G5
SoCs. The old one is still available, so select it in order to remain
compatible with the ftgmac100 driver.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 9 +++++++++
drivers/net/ethernet/faraday/ftgmac100.h | 5 +++++
2 files changed, 14 insertions(+)
@@ -1257,12 +1257,21 @@ static int ftgmac100_setup_mdio(struct net_device *netdev)structftgmac100*priv=netdev_priv(netdev);structplatform_device*pdev=to_platform_device(priv->dev);inti,err=0;+u32reg;/* initialize mdio bus */priv->mii_bus=mdiobus_alloc();if(!priv->mii_bus)return-EIO;+if(of_machine_is_compatible("aspeed,ast2400")||+of_machine_is_compatible("aspeed,ast2500")){+/* This driver supports the old MDIO interface */+reg=ioread32(priv->base+FTGMAC100_OFFSET_REVR);+reg&=~FTGMAC100_REVR_NEW_MDIO_INTERFACE;+iowrite32(reg,priv->base+FTGMAC100_OFFSET_REVR);+};+priv->mii_bus->name="ftgmac100_mdio";snprintf(priv->mii_bus->id,MII_BUS_ID_SIZE,"%s-%d",pdev->name,pdev->id);
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2016-09-20 12:13:40
On Tue, 2016-09-20 at 16:00 +0930, Joel Stanley wrote:
On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
continual PHYSTS interrupts:
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
This is because the driver was enabling low-level sensitive interrupt
generation where the systems are wired for high-level. All CPU cycles
are spent servicing this interrupt.
If this is a system wiring issue, should it be represented by a DT
property ?
Cheers,
Ben.
From: Sergei Shtylyov <hidden> Date: 2016-09-20 12:14:08
Hello.
On 9/20/2016 9:30 AM, Joel Stanley wrote:
quoted hunk
On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
continual PHYSTS interrupts:
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
This is because the driver was enabling low-level sensitive interrupt
generation where the systems are wired for high-level. All CPU cycles
are spent servicing this interrupt.
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
drivers/net/ethernet/faraday/ftgmac100.c | 4 ++++
1 file changed, 4 insertions(+)
From: Andrew Lunn <andrew@lunn.ch> Date: 2016-09-20 15:29:52
On Tue, Sep 20, 2016 at 10:13:14PM +1000, Benjamin Herrenschmidt wrote:
On Tue, 2016-09-20 at 16:00 +0930, Joel Stanley wrote:
quoted
On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
continual PHYSTS interrupts:
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
This is because the driver was enabling low-level sensitive interrupt
generation where the systems are wired for high-level. All CPU cycles
are spent servicing this interrupt.
If this is a system wiring issue, should it be represented by a DT
property ?
Is there a device tree binding document somewhere?
Is it possible just to put ACTIVE_HIGH in the right place in the
binding?
Andrew
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-21 02:03:14
On Wed, Sep 21, 2016 at 12:59 AM, Andrew Lunn [off-list ref] wrote:
On Tue, Sep 20, 2016 at 10:13:14PM +1000, Benjamin Herrenschmidt wrote:
quoted
On Tue, 2016-09-20 at 16:00 +0930, Joel Stanley wrote:
quoted
On Aspeed SoC with a direct PHY connection (non-NSCI), we receive
continual PHYSTS interrupts:
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.280000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
[ 20.300000] ftgmac100 1e660000.ethernet eth0: [ISR] = 0x200: PHYSTS_CHG
This is because the driver was enabling low-level sensitive interrupt
generation where the systems are wired for high-level. All CPU cycles
are spent servicing this interrupt.
If this is a system wiring issue, should it be represented by a DT
property ?
Is there a device tree binding document somewhere?
Is it possible just to put ACTIVE_HIGH in the right place in the
binding?
I wrote "wired for high level" wrt the SoC internals. To be honest I
wondered the same thing but it's hard with only one (non-NSCI) system
to test on.
I had a look at the eval board schematic and it appears that the line
has pull down resistors on it, explaining why the IRQ fires when it's
configured to active low. Other machines re-use the pin pin as a GPIO.
So yes, I will change this to a dt property in v2. That will mean
dropping 4/7 "net/faraday: Avoid PHYSTS_CHG interrupt" as well.
Cheers,
Joel
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2016-09-21 09:06:38
On Wed, 2016-09-21 at 11:32 +0930, Joel Stanley wrote:
I had a look at the eval board schematic and it appears that the line
has pull down resistors on it, explaining why the IRQ fires when it's
configured to active low. Other machines re-use the pin pin as a GPIO.
So yes, I will change this to a dt property in v2. That will mean
dropping 4/7 "net/faraday: Avoid PHYSTS_CHG interrupt" as well.
What line is it out of the PHY ? The PHY IRQ ? If yes then it's meant
to be telling you to go look at the PHY registers for a link status
change, but only works if the PHY has also been configured
appropriately...
Mostly we ignore those things in Linux and just poll the PHY.
Cheers,
Ben.
From: Joel Stanley <joel@jms.id.au> Date: 2016-09-21 09:19:06
On Wed, Sep 21, 2016 at 6:33 PM, Benjamin Herrenschmidt
[off-list ref] wrote:
On Wed, 2016-09-21 at 11:32 +0930, Joel Stanley wrote:
quoted
I had a look at the eval board schematic and it appears that the line
has pull down resistors on it, explaining why the IRQ fires when it's
configured to active low. Other machines re-use the pin pin as a GPIO.
So yes, I will change this to a dt property in v2. That will mean
dropping 4/7 "net/faraday: Avoid PHYSTS_CHG interrupt" as well.
What line is it out of the PHY ? The PHY IRQ ? If yes then it's meant
to be telling you to go look at the PHY registers for a link status
change, but only works if the PHY has also been configured
appropriately...
Yep, PHY IRQ.
Mostly we ignore those things in Linux and just poll the PHY.
That's simpler. It's what we're doing on Aspeed systems when using NSCI already.
The driver is already polling the PHY, I propose we mask out this
interrupt for all systems. I gave that a run on my ast2500evb and it
behaved itself.
Cheers,
Joe
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2016-09-21 10:43:27
On Wed, 2016-09-21 at 18:48 +0930, Joel Stanley wrote:
quoted
What line is it out of the PHY ? The PHY IRQ ? If yes then it's meant
to be telling you to go look at the PHY registers for a link status
change, but only works if the PHY has also been configured
appropriately...
Yep, PHY IRQ.
quoted
Mostly we ignore those things in Linux and just poll the PHY.
That's simpler. It's what we're doing on Aspeed systems when using NSCI already.
The driver is already polling the PHY, I propose we mask out this
interrupt for all systems. I gave that a run on my ast2500evb and it
behaved itself.
Right it's the easiest way.
If we want to use interrupts without polling in order to maybe
insignificantly reduce the load in the system, we should then make sure
we configure the IRQ with a matching polarity between the PHY and the
NIC *and* in a way that matches the pull up/down's on the board.
The above is possible but tricky as all the parts have to be right, and
the IRQ configuration of PHYs is basically PHY model specific.
So I'd start with just masking it out.
Cheers,
Ben.