| Message ID | 20160920063007.24291-7-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 21, 2016 at 12:59 AM, Andrew Lunn <andrew@lunn.ch> wrote: > 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: >> > 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
On Wed, Sep 21, 2016 at 6:33 PM, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote: > 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... 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
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c index 7ba0f2d58a8b..5466df028381 100644 --- a/drivers/net/ethernet/faraday/ftgmac100.c +++ b/drivers/net/ethernet/faraday/ftgmac100.c @@ -223,6 +223,10 @@ static void ftgmac100_start_hw(struct ftgmac100 *priv, int speed) { int maccr = MACCR_ENABLE_ALL; + if (of_machine_is_compatible("aspeed,ast2500")) { + maccr &= ~FTGMAC100_MACCR_PHY_LINK_LEVEL; + } + switch (speed) { default: case 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(+) -- 2.9.3