All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Lunn <andrew@lunn.ch>
To: John David Anglin <dave.anglin@bell.net>,
	Heiner Kallweit <hkallweit1@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>,
	Vivien Didelot <vivien.didelot@savoirfairelinux.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	netdev@vger.kernel.org
Subject: Re: [PATCH net] dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to exit
Date: Tue, 12 Feb 2019 04:58:06 +0100	[thread overview]
Message-ID: <20190212035806.GE19023@lunn.ch> (raw)
In-Reply-To: <2b6bbb4c-1346-461b-ff7a-cb96b4142f7a@bell.net>

> > Hi David
> >
> > I just tested this on one of my boards. It loops endlessly:
> >
> > [   47.173396] mv88e6xxx_g1_irq_thread_work: c881 a8 80                         
> > [   47.182108] mv88e6xxx_g1_irq_thread_work: c881 a8 80                         
> > [   47.190820] mv88e6xxx_g1_irq_thread_work: c881 a8 80                         
> > [   47.199535] mv88e6xxx_g1_irq_thread_work: c881 a8 80                         
> > [   47.208254] mv88e6xxx_g1_irq_thread_work: c881 a8 80   
> >
> > These are reg, ctl1, reg & ctl1.
> >
> > So there is an unhandled device interrupt.

Hi Heiner

Your patch Fixes: 2b3e88ea6528 ("net: phy: improve phy state
checking") is causing me problems with interrupts for the Marvell
switches.

That change means we don't check the PHY device if it caused an
interrupt when its state is less than UP.

What i'm seeing is that the PHY is interrupting pretty early on after
a reboot when the previous boot had the interface up.

[   10.125702] Marvell 88E6390 mv88e6xxx-0:02: phy_start_interrupts
[   10.162798] Marvell 88E6390 mv88e6xxx-0:02: phy_enable_interrupts
[   10.168931] Marvell 88E6390 mv88e6xxx-0:02: marvell_ack_interrupt
[   10.180164] Marvell 88E6390 mv88e6xxx-0:02: marvell_config_intr 1

a little later it interrupts:

[   12.999717] mv88e6xxx_g1_irq_thread_fn
[   13.007253] mv88e6xxx_g2_irq_thread_fn: 4 811c 4
[   13.012015] libphy: __phy_is_started: phydev->state 1 PHY_UP 3
[   13.017941] Marvell 88E6390 mv88e6xxx-0:02: phy_interrupt: phy_is_started(phydev) 0

The current code just causes it to be ignored. So the interrupts fires
again, and again...

If i change to code to call into the PHY driver and let it handle the
interrupts, things keep running. A little bit later the interface is
configured up:

[   15.921326] mv88e6085 gpio-0:00 red: configuring for phy/gmii link mode
[   15.928693] libphy: __phy_is_started: phydev->state 3 PHY_UP 3
[   15.929442] IPv6: ADDRCONF(NETDEV_UP): red: link is not ready
[   15.935596] Marvell 88E6390 mv88e6xxx-0:02: m88e6390_config_aneg
[   15.935608] Marvell 88E6390 mv88e6xxx-0:02: m88e6390_errata

[   16.071364] Marvell 88E6390 mv88e6xxx-0:02: m88e1510_config_aneg
[   16.112362] Marvell 88E6390 mv88e6xxx-0:02: m88e1318_config_aneg
[   16.151245] Marvell 88E6390 mv88e6xxx-0:02: m88e1121_config_aneg
[   16.368206] Marvell 88E6390 mv88e6xxx-0:02: PHY state change UP -> NOLINK

and after another interrupt the link goes up.

[   19.519840] mv88e6xxx_g1_irq_thread_fn
[   19.528546] mv88e6xxx_g2_irq_thread_fn: 4 811c 4
[   19.534152] libphy: __phy_is_started: phydev->state 5 PHY_UP 3
[   19.540030] Marvell 88E6390 mv88e6xxx-0:02: phy_interrupt: phy_is_started(phydev) 1
[   19.547721] Marvell 88E6390 mv88e6xxx-0:02: m88e1121_did_interrupt
[   19.559829] Marvell 88E6390 mv88e6xxx-0:02: marvell_ack_interrupt
[   19.590753] Marvell 88E6390 mv88e6xxx-0:02: marvell_read_status
[   19.596712] Marvell 88E6390 mv88e6xxx-0:02: marvell_update_link
[   19.628387] Marvell 88E6390 mv88e6xxx-0:02: PHY state change NOLINK -> RUNNING
[   19.628453] mv88e6085 gpio-0:00 red: Link is Up - 1Gbps/Full - flow control off
[   19.635920] IPv6: ADDRCONF(NETDEV_CHANGE): red: link becomes ready

I don't yet know why the first interrupt happens, before we configure
auto-neg, etc. But it is not too unreasonable. We have configured
interrupts, so it could be reporting link down etc.

So i think we might need to revert part of this change, call into the
driver so long as the PHY is not in state PHY_HALTED.

What do you think?

     Andrew

  parent reply	other threads:[~2019-02-12  3:58 UTC|newest]

Thread overview: 52+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-22 19:16 net: phylink: dsa: mv88e6xxx: flaky link detection on switch ports with internal PHYs John David Anglin
2019-01-22 20:28 ` Andrew Lunn
2019-01-22 21:40   ` John David Anglin
2019-01-22 22:36     ` Andrew Lunn
2019-01-22 23:52       ` John David Anglin
2019-01-23  0:11       ` John David Anglin
2019-01-23  0:22         ` Andrew Lunn
2019-01-25 16:30           ` John David Anglin
2019-01-25 16:48             ` Russell King - ARM Linux admin
2019-01-25 18:38               ` John David Anglin
2019-01-30 17:08           ` John David Anglin
2019-01-30 17:28             ` Andrew Lunn
2019-01-30 19:01               ` John David Anglin
2019-01-30 19:09                 ` Andrew Lunn
2019-01-30 22:24               ` John David Anglin
2019-01-30 22:38                 ` Andrew Lunn
2019-01-31  1:27                   ` John David Anglin
2019-01-31 17:27                     ` John David Anglin
2019-02-04 18:37                       ` [PATCH] net: phylink: dsa: mv88e6xxx: Revise irq setup ordering John David Anglin
2019-02-04 19:35                         ` Andrew Lunn
2019-02-04 19:52                           ` John David Anglin
2019-02-04 20:19                             ` Andrew Lunn
2019-02-04 21:38                               ` John David Anglin
2019-02-04 22:47                                 ` Andrew Lunn
2019-02-04 21:59                         ` [PATCH v2] net: " John David Anglin
2019-02-04 23:14                           ` Andrew Lunn
2019-02-05  0:38                             ` John David Anglin
2019-02-05  2:21                               ` Andrew Lunn
2019-02-05 19:20                                 ` John David Anglin
2019-02-05 19:54                                   ` Andrew Lunn
2019-02-05 18:37                           ` David Miller
2019-02-11 18:40                           ` [PATCH net] dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to exit John David Anglin
2019-02-11 23:33                             ` Andrew Lunn
2019-02-12  0:57                               ` John David Anglin
2019-02-12  1:21                                 ` Andrew Lunn
2019-02-12  3:58                                 ` Andrew Lunn [this message]
2019-02-12  6:51                                   ` Heiner Kallweit
2019-02-12 12:56                                     ` Andrew Lunn
2019-02-12 18:42                                       ` Heiner Kallweit
2019-02-12 20:09                                       ` John David Anglin
2019-02-12 16:30                                     ` Russell King - ARM Linux admin
2019-02-12 20:11                                       ` Heiner Kallweit
2019-02-12 20:54                                       ` Heiner Kallweit
2019-02-12 22:55                                         ` Russell King - ARM Linux admin
2019-02-14  2:07                             ` Andrew Lunn
2019-02-14  4:47                               ` David Miller
2019-02-14  4:50                                 ` Andrew Lunn
2019-02-14 15:27                                   ` David Miller
2019-01-22 23:12 ` net: phylink: dsa: mv88e6xxx: flaky link detection on switch ports with internal PHYs Andrew Lunn
2019-01-22 23:48   ` John David Anglin
2019-01-23  0:00   ` John David Anglin
2019-01-23  0:04     ` Florian Fainelli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190212035806.GE19023@lunn.ch \
    --to=andrew@lunn.ch \
    --cc=dave.anglin@bell.net \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=linux@arm.linux.org.uk \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@savoirfairelinux.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.