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

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

[Anatolij Gustschin]

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.

Signed-off-by: Anatolij Gustschin <agust@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);
+	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, 0xffffff0,
+				   socrates_m88e1121_fixup);
 }
 
 /*
-- 
1.5.6.3

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 <agust@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, 0xffffff0,
> +				   socrates_m88e1121_fixup);
>  }
>  
>  /*
> -- 
> 1.5.6.3

Thanks,

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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 & 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.

> 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 <agust@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

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 & 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?

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

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: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

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

[Anatolij Gustschin]

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.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
Changes since first version:
	use macros instead of register numbers as
	suggested by Anton

Kumar, could you please consider this patch for
inclusion into 2.6.30? Thanks!

 arch/powerpc/platforms/85xx/socrates.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/85xx/socrates.c b/arch/powerpc/platforms/85xx/socrates.c
index d0e8443..bde0a97 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,23 @@ static void __init socrates_pic_init(void)
 	of_node_put(np);
 }
 
+#define MII_M1011_IMASK		0x12
+#define MII_M1011_IEVENT	0x13
+static int socrates_m88e1121_fixup(struct phy_device *phydev)
+{
+	int err;
+
+	err = phy_write(phydev, MII_M1011_IMASK, 0);
+	if (err < 0)
+		return err;
+
+	err = phy_read(phydev, MII_M1011_IEVENT);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 /*
  * Setup the architecture
  */
@@ -105,6 +123,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, 0xffffff0,
+				   socrates_m88e1121_fixup);
 }
 
 /*
-- 
1.5.6.3

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

[Kumar Gala]

On Apr 21, 2009, at 12:19 PM, 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.
>
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> Changes since first version:
> 	use macros instead of register numbers as
> 	suggested by Anton
>
> Kumar, could you please consider this patch for
> inclusion into 2.6.30? Thanks!

Sorry.  I dont think this is board specific and should at a minimum be  
done in m88e1011_config_init in drivers/net/phy/marvell.c.  Not sure  
how 88E1011 differs from 88E1111, but I'm wondering if you really want  
to set config_init for m88e1011 to m88e1111_config_init

- k

> arch/powerpc/platforms/85xx/socrates.c |   20 ++++++++++++++++++++
> 1 files changed, 20 insertions(+), 0 deletions(-)
>
> diff --git a/arch/powerpc/platforms/85xx/socrates.c b/arch/powerpc/ 
> platforms/85xx/socrates.c
> index d0e8443..bde0a97 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,23 @@ static void __init socrates_pic_init(void)
> 	of_node_put(np);
> }
>
> +#define MII_M1011_IMASK		0x12
> +#define MII_M1011_IEVENT	0x13
> +static int socrates_m88e1121_fixup(struct phy_device *phydev)
> +{
> +	int err;
> +
> +	err = phy_write(phydev, MII_M1011_IMASK, 0);
> +	if (err < 0)
> +		return err;
> +
> +	err = phy_read(phydev, MII_M1011_IEVENT);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +
> /*
>  * Setup the architecture
>  */
> @@ -105,6 +123,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, 0xffffff0,
> +				   socrates_m88e1121_fixup);
> }
>
> /*
> -- 
> 1.5.6.3

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

[Kumar Gala]

On Apr 21, 2009, at 4:54 PM, Kumar Gala wrote:

>
> On Apr 21, 2009, at 12:19 PM, 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.
>>
>> Signed-off-by: Anatolij Gustschin <agust@denx.de>
>> ---
>> Changes since first version:
>> 	use macros instead of register numbers as
>> 	suggested by Anton
>>
>> Kumar, could you please consider this patch for
>> inclusion into 2.6.30? Thanks!
>
> Sorry.  I dont think this is board specific and should at a minimum  
> be done in m88e1011_config_init in drivers/net/phy/marvell.c.  Not  
> sure how 88E1011 differs from 88E1111, but I'm wondering if you  
> really want to set config_init for m88e1011 to m88e1111_config_init
>
> - k

I got confused by the #'s.. I think we should have a struct in  
marvell.c for m88e1121 which I'm guessing is the PHY you are using.

- k

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

[Anatolij Gustschin]

Kumar Gala wrote:
> 
> On Apr 21, 2009, at 4:54 PM, Kumar Gala wrote:
> 
>>
>> On Apr 21, 2009, at 12:19 PM, 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.
>>>
>>> Signed-off-by: Anatolij Gustschin <agust@denx.de>
>>> ---
>>> Changes since first version:
>>>     use macros instead of register numbers as
>>>     suggested by Anton
>>>
>>> Kumar, could you please consider this patch for
>>> inclusion into 2.6.30? Thanks!
>>
>> Sorry.  I dont think this is board specific and should at a minimum be
>> done in m88e1011_config_init in drivers/net/phy/marvell.c.  Not sure
>> how 88E1011 differs from 88E1111, but I'm wondering if you really want
>> to set config_init for m88e1011 to m88e1111_config_init
>>
>> - k
> 
> I got confused by the #'s.. I think we should have a struct in marvell.c
> for m88e1121 which I'm guessing is the PHY you are using.

yes, m88e1121 is correct. In 2.6.30-rc2 there is already a m88e1121
struct in marvell_drivers[] in drivers/net/phy/marvell.c.
The reason I'm not doing the m88e1121 pre-init stuff in config_init
is as follows:

m88e1121 is a multi-PHY device with two PHY ports and each port
could signal an interrupt. This PHY device can be pin-strapped to use
shared interrupt pin for both PHY ports (as used on socrates board).
PHY specific config_init will be called e.g. while eth0 startup in
phy_attach() which is called from phy_connect() in drivers/net/phy/phy_device.c.
phy_connect() then calls phy_start_interrupts() which registers the
interrupt handler and enables the interrupts for the PHY-port config_init
is called for. Now interrupts can be cleared/acknowledged for this
PHY-port (eth0 interface), but interrupts from the another PHY-port
can not be acknowledged as eth1 was not brought up yet.

Placing this fixup in drivers/net/phy/marvell.c as in 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.

Anatolij

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

[Kumar Gala]

On Apr 21, 2009, at 6:24 PM, Anatolij Gustschin wrote:

> Kumar Gala wrote:
>>
>> On Apr 21, 2009, at 4:54 PM, Kumar Gala wrote:
>>
>>>
>>> On Apr 21, 2009, at 12:19 PM, 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.
>>>>
>>>> Signed-off-by: Anatolij Gustschin <agust@denx.de>
>>>> ---
>>>> Changes since first version:
>>>>    use macros instead of register numbers as
>>>>    suggested by Anton
>>>>
>>>> Kumar, could you please consider this patch for
>>>> inclusion into 2.6.30? Thanks!
>>>
>>> Sorry.  I dont think this is board specific and should at a  
>>> minimum be
>>> done in m88e1011_config_init in drivers/net/phy/marvell.c.  Not sure
>>> how 88E1011 differs from 88E1111, but I'm wondering if you really  
>>> want
>>> to set config_init for m88e1011 to m88e1111_config_init
>>>
>>> - k
>>
>> I got confused by the #'s.. I think we should have a struct in  
>> marvell.c
>> for m88e1121 which I'm guessing is the PHY you are using.
>
> yes, m88e1121 is correct. In 2.6.30-rc2 there is already a m88e1121
> struct in marvell_drivers[] in drivers/net/phy/marvell.c.
> The reason I'm not doing the m88e1121 pre-init stuff in config_init
> is as follows:
>
> m88e1121 is a multi-PHY device with two PHY ports and each port
> could signal an interrupt. This PHY device can be pin-strapped to use
> shared interrupt pin for both PHY ports (as used on socrates board).
> PHY specific config_init will be called e.g. while eth0 startup in
> phy_attach() which is called from phy_connect() in drivers/net/phy/ 
> phy_device.c.
> phy_connect() then calls phy_start_interrupts() which registers the
> interrupt handler and enables the interrupts for the PHY-port  
> config_init
> is called for. Now interrupts can be cleared/acknowledged for this
> PHY-port (eth0 interface), but interrupts from the another PHY-port
> can not be acknowledged as eth1 was not brought up yet.
>
> Placing this fixup in drivers/net/phy/marvell.c as in 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.

Did we ever resolve this?

- k

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

[Anatolij Gustschin]

Kumar Gala wrote:
> 
> On Apr 21, 2009, at 6:24 PM, Anatolij Gustschin wrote:
> 
>> Kumar Gala wrote:
>>>
>>> On Apr 21, 2009, at 4:54 PM, Kumar Gala wrote:
>>>
>>>>
>>>> On Apr 21, 2009, at 12:19 PM, 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.
>>>>>
>>>>> Signed-off-by: Anatolij Gustschin <agust@denx.de>
>>>>> ---
>>>>> Changes since first version:
>>>>>    use macros instead of register numbers as
>>>>>    suggested by Anton
>>>>>
>>>>> Kumar, could you please consider this patch for
>>>>> inclusion into 2.6.30? Thanks!
>>>>
>>>> Sorry.  I dont think this is board specific and should at a minimum be
>>>> done in m88e1011_config_init in drivers/net/phy/marvell.c.  Not sure
>>>> how 88E1011 differs from 88E1111, but I'm wondering if you really want
>>>> to set config_init for m88e1011 to m88e1111_config_init
>>>>
>>>> - k
>>>
>>> I got confused by the #'s.. I think we should have a struct in marvell.c
>>> for m88e1121 which I'm guessing is the PHY you are using.
>>
>> yes, m88e1121 is correct. In 2.6.30-rc2 there is already a m88e1121
>> struct in marvell_drivers[] in drivers/net/phy/marvell.c.
>> The reason I'm not doing the m88e1121 pre-init stuff in config_init
>> is as follows:
>>
>> m88e1121 is a multi-PHY device with two PHY ports and each port
>> could signal an interrupt. This PHY device can be pin-strapped to use
>> shared interrupt pin for both PHY ports (as used on socrates board).
>> PHY specific config_init will be called e.g. while eth0 startup in
>> phy_attach() which is called from phy_connect() in
>> drivers/net/phy/phy_device.c.
>> phy_connect() then calls phy_start_interrupts() which registers the
>> interrupt handler and enables the interrupts for the PHY-port config_init
>> is called for. Now interrupts can be cleared/acknowledged for this
>> PHY-port (eth0 interface), but interrupts from the another PHY-port
>> can not be acknowledged as eth1 was not brought up yet.
>>
>> Placing this fixup in drivers/net/phy/marvell.c as in 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.
> 
> Did we ever resolve this?

No. I think newer U-Boot should be used instead of fixing this
in Linux PHY driver or board specific code. The problem is fixed
in U-Boot v2009.01.

Anatolij