Re: [PATCH] powerpc: 85xx: Add PHY fixup to socrates board code

[Anton Vorontsov]

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?

Has this fixup any effect after phy power down/up sequence?
Otherwise you may encounter same problem after suspend/resume.

 Signed-off-by: Anatolij Gustschin ag...@denx.de
 ---
  arch/powerpc/platforms/85xx/socrates.c |   18 ++
  1 files changed, 18 insertions(+), 0 deletions(-)
 
 diff --git a/arch/powerpc/platforms/85xx/socrates.c 
 b/arch/powerpc/platforms/85xx/socrates.c
 index d0e8443..2275a39 100644
 --- a/arch/powerpc/platforms/85xx/socrates.c
 +++ b/arch/powerpc/platforms/85xx/socrates.c
 @@ -28,6 +28,7 @@
  #include linux/delay.h
  #include linux/seq_file.h
  #include linux/of_platform.h
 +#include linux/phy.h
  
  #include asm/system.h
  #include asm/time.h
 @@ -78,6 +79,21 @@ static void __init socrates_pic_init(void)
   of_node_put(np);
  }
  
 +static int socrates_m88e1121_fixup(struct phy_device *phydev)
 +{
 + int err;
 +
 + err = phy_write(phydev, 0x12, 0);

Do you know the proper names for 0x12 and 0x13 registers
on that chip?

 + if (err  0)
 + return err;
 +
 + err = phy_read(phydev, 0x13);
 + if (err  0)
 + return err;
 +
 + return 0;
 +}
 +
  /*
   * Setup the architecture
   */
 @@ -105,6 +121,8 @@ static struct of_device_id __initdata 
 socrates_of_bus_ids[] = {
  static void __init socrates_init(void)
  {
   of_platform_bus_probe(NULL, socrates_of_bus_ids, NULL);
 + phy_register_fixup_for_uid(0x1410cb0, 0xff0,
 +socrates_m88e1121_fixup);
  }
  
  /*
 -- 
 1.5.6.3

Thanks,

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: 85xx: Add PHY fixup to socrates board code

[Anatolij Gustschin]

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  0x0ff0) == 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.

 Has this fixup any effect after phy power down/up sequence?
 Otherwise you may encounter same problem after suspend/resume.

No, do we need it after phy power down/up sequence?
If each phy interrupt handler remains registered and the phy is
only stopped  (phydev-state == PHY_HALTED) we don't have
this problem, i think. And we do not use PM on this board.
 
 Signed-off-by: Anatolij Gustschin ag...@denx.de
 ---
  arch/powerpc/platforms/85xx/socrates.c |   18 ++
  1 files changed, 18 insertions(+), 0 deletions(-)

 diff --git a/arch/powerpc/platforms/85xx/socrates.c 
 b/arch/powerpc/platforms/85xx/socrates.c
 index d0e8443..2275a39 100644
 --- a/arch/powerpc/platforms/85xx/socrates.c
 +++ b/arch/powerpc/platforms/85xx/socrates.c
 @@ -28,6 +28,7 @@
  #include linux/delay.h
  #include linux/seq_file.h
  #include linux/of_platform.h
 +#include linux/phy.h
  
  #include asm/system.h
  #include asm/time.h
 @@ -78,6 +79,21 @@ static void __init socrates_pic_init(void)
  of_node_put(np);
  }
  
 +static int socrates_m88e1121_fixup(struct phy_device *phydev)
 +{
 +int err;
 +
 +err = phy_write(phydev, 0x12, 0);
 
 Do you know the proper names for 0x12 and 0x13 registers
 on that chip?

Marvell PHY driver defines them as MII_M1011_IMASK and
MII_M1011_IEVENT respectively. I can use these defines
here too. The data sheet is under NDA.

Thanks,
Anatolij
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: 85xx: Add PHY fixup to socrates board code

[Anton Vorontsov]

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  0x0ff0) == 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?

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);
 
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] powerpc: 85xx: Add PHY fixup to socrates board code

[Anton Vorontsov]

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);
  

-- 
Anton Vorontsov
email: cbouatmai...@gmail.com
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev