Patchwork powerpc: 85xx: Add PHY fixup to socrates board code

login
register
mail settings
Submitter Anton Vorontsov
Date April 7, 2009, 6:09 p.m.
Message ID <20090407180957.GA2831@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/25690/
State Not Applicable, archived
Delegated to: Kumar Gala
Headers show

Comments

Anton Vorontsov - April 7, 2009, 6:09 p.m.
On Tue, Apr 07, 2009 at 07:24:12PM +0200, Anatolij Gustschin wrote:
> Anton Vorontsov wrote:
> > On Tue, Apr 07, 2009 at 04:19:48PM +0200, Anatolij Gustschin wrote:
> >> If the firmware missed to initialize the PHY correctly,
> >> Linux may hang up on socrates while eth0/eth1 interface
> >> startup (caused by continuous unacknowledged PHY interrupt).
> >>
> >> This patch adds PHY fixup to socrates platform code to
> >> ensure the PHY is pre-initialized correctly. It is needed
> >> to be compatible with older firmware.
> > 
> > Is that really board-specific fixup, or can it be placed
> > somewhere inside drivers/net/phy/marvell.c?
> 
> On this board the multi-PHY is configured to use shared IRQ pin
> for both PHY ports. Placing this fixup in drivers/net/phy/marvell.c
> as e.g. '.config_init' callback could be done, but this will add
> more overhead as the fixup routine have to do more work:
> 
> acquire 'struct mii_bus' pointer and walk through all registered PHYs
> searching for the PHY which use the same interrupt, then getting
> the address of this PHY on the bus and disable and clear PHY irqs
> by writing/reading to/from this PHY, (but only in the case it was
> not already brought up and has interrupts enabled!) e.g.:
> 
> struct mii_bus *bus = phydev->bus;
> int addr;
> 
> for (addr = 0; addr < PHY_MAX_ADDR; addr++) {
> 	struct phy_device *phy = bus->phy_map[addr];
> 
> 	if (addr != phydev->addr && bus->irq[addr] == phydev->irq &&
> 	    (phy->phy_id & 0x0ffffff0) == 0x01410cb0 &&
> 	    !(phy->interrupts & PHY_INTERRUPT_ENABLED)) {
> 
> 		int imask = phy_read(phy, MII_M1011_IMASK);
> 
> 		if (imask) {
> 			phy_write(phy, 0x12, 0); /* disable */
> 			phy_read(phy, 0x13); /* clear */
> 		}
> 	}
> }
> 
> All this to allow support for multiple m88e1121 devices.
> Otherwise, after registering first phy interrupt handler
> and enabling interrupt pending irq on other PHY port or
> other PHY device will lock up the board.
> 
> The fixup in this patch will only be done while mdio bus scan
> before registering a PHY device.

Still, I think you shouldn't do this in a board-specific fixup.
Maybe I'm a bit naive, but shouldn't this work too?
Anton Vorontsov - April 7, 2009, 6:40 p.m.
On Tue, Apr 07, 2009 at 10:09:57PM +0400, Anton Vorontsov wrote:
[...]
> Still, I think you shouldn't do this in a board-specific fixup.
> Maybe I'm a bit naive, but shouldn't this work too?

Yes, I'm naive. The phydev isn't yet bound to any driver, so we
can't call phy_disable_interrupts().

And I don't see any proper solution. :-( After all, it appears
we'll have to live with the board fixup.

> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index b754020..9052937 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -205,6 +205,16 @@ struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
>  
>  	phydev->bus = bus;
>  
> +	/*
> +	 * Some PHYs may have unmasked/pending interrupts, this might
> +	 * cause troubles w/ shared IRQs. So try to put the PHYs into
> +	 * some sane state.
> +	 *
> +	 * NOTE: This won't work if you have two PHYs w/ shared IRQs
> +	 *       on different MDIO buses.
> +	 */
> +	phy_disable_interrupts(phydev);
> +
>  	/* Run all of the fixups for this PHY */
>  	phy_scan_fixups(phydev);
>

Patch

diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index b754020..9052937 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -205,6 +205,16 @@  struct phy_device *mdiobus_scan(struct mii_bus *bus, int addr)
 
 	phydev->bus = bus;
 
+	/*
+	 * Some PHYs may have unmasked/pending interrupts, this might
+	 * cause troubles w/ shared IRQs. So try to put the PHYs into
+	 * some sane state.
+	 *
+	 * NOTE: This won't work if you have two PHYs w/ shared IRQs
+	 *       on different MDIO buses.
+	 */
+	phy_disable_interrupts(phydev);
+
 	/* Run all of the fixups for this PHY */
 	phy_scan_fixups(phydev);