[PATCH] powerpc/8313erdb: fix kernel crash

[PATCH] powerpc/8313erdb: fix kernel crash

[Li Yang]

Probe the new mdio node added by b31a1d8b.  Fix kernel panic problem when
gianfar driver wants to get the of_platform_device of that mdio.

Signed-off-by: Li Yang <leoli@freescale.com>
---
 arch/powerpc/boot/dts/mpc8313erdb.dts |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/boot/dts/mpc8313erdb.dts
index d4df8b6..917d466 100644
--- a/arch/powerpc/boot/dts/mpc8313erdb.dts
+++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
@@ -222,7 +222,7 @@
 			cell-index = <1>;
 			device_type = "network";
 			model = "eTSEC";
-			compatible = "gianfar";
+			compatible = "gianfar", "simple-bus";
 			reg = <0x25000 0x1000>;
 			local-mac-address = [ 00 00 00 00 00 00 ];
 			interrupts = <34 0x8 33 0x8 32 0x8>;
-- 
1.5.4

Re: [PATCH] powerpc/8313erdb: fix kernel crash

[Kumar Gala]

On Jan 5, 2009, at 4:47 AM, Li Yang wrote:

> Probe the new mdio node added by b31a1d8b.  Fix kernel panic problem  
> when
> gianfar driver wants to get the of_platform_device of that mdio.
>
> Signed-off-by: Li Yang <leoli@freescale.com>
> ---
> arch/powerpc/boot/dts/mpc8313erdb.dts |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/ 
> boot/dts/mpc8313erdb.dts
> index d4df8b6..917d466 100644
> --- a/arch/powerpc/boot/dts/mpc8313erdb.dts
> +++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
> @@ -222,7 +222,7 @@
> 			cell-index = <1>;
> 			device_type = "network";
> 			model = "eTSEC";
> -			compatible = "gianfar";
> +			compatible = "gianfar", "simple-bus";
> 			reg = <0x25000 0x1000>;
> 			local-mac-address = [ 00 00 00 00 00 00 ];
> 			interrupts = <34 0x8 33 0x8 32 0x8>;

This isn't correct at all.  Not sure what the panic is but adding  
"simple-bus" isn't correct.

- k

Re: [PATCH] powerpc/8313erdb: fix kernel crash

[Scott Wood]

On Mon, Jan 05, 2009 at 10:25:08AM -0600, Kumar Gala wrote:
> 
> On Jan 5, 2009, at 4:47 AM, Li Yang wrote:
> 
> >Probe the new mdio node added by b31a1d8b.  Fix kernel panic problem  
> >when
> >gianfar driver wants to get the of_platform_device of that mdio.
> >
> >Signed-off-by: Li Yang <leoli@freescale.com>
> >---
> >arch/powerpc/boot/dts/mpc8313erdb.dts |    2 +-
> >1 files changed, 1 insertions(+), 1 deletions(-)
> >
> >diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/ 
> >boot/dts/mpc8313erdb.dts
> >index d4df8b6..917d466 100644
> >--- a/arch/powerpc/boot/dts/mpc8313erdb.dts
> >+++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
> >@@ -222,7 +222,7 @@
> >			cell-index = <1>;
> >			device_type = "network";
> >			model = "eTSEC";
> >-			compatible = "gianfar";
> >+			compatible = "gianfar", "simple-bus";
> >			reg = <0x25000 0x1000>;
> >			local-mac-address = [ 00 00 00 00 00 00 ];
> >			interrupts = <34 0x8 33 0x8 32 0x8>;
> 
> This isn't correct at all.  Not sure what the panic is but adding  
> "simple-bus" isn't correct.

It looks like the mdio node got added in the wrong place on 8313erdb,
relative to where it went on all other boards -- though if we were to
ignore existing practice, it probably does belong under the gianfar node
(and not as a freestanding device).

-Scott

Re: [PATCH] powerpc/8313erdb: fix kernel crash

[Kumar Gala]

On Jan 5, 2009, at 1:27 PM, Scott Wood wrote:

> On Mon, Jan 05, 2009 at 10:25:08AM -0600, Kumar Gala wrote:
>>
>> On Jan 5, 2009, at 4:47 AM, Li Yang wrote:
>>
>>> Probe the new mdio node added by b31a1d8b.  Fix kernel panic problem
>>> when
>>> gianfar driver wants to get the of_platform_device of that mdio.
>>>
>>> Signed-off-by: Li Yang <leoli@freescale.com>
>>> ---
>>> arch/powerpc/boot/dts/mpc8313erdb.dts |    2 +-
>>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/
>>> boot/dts/mpc8313erdb.dts
>>> index d4df8b6..917d466 100644
>>> --- a/arch/powerpc/boot/dts/mpc8313erdb.dts
>>> +++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
>>> @@ -222,7 +222,7 @@
>>> 			cell-index = <1>;
>>> 			device_type = "network";
>>> 			model = "eTSEC";
>>> -			compatible = "gianfar";
>>> +			compatible = "gianfar", "simple-bus";
>>> 			reg = <0x25000 0x1000>;
>>> 			local-mac-address = [ 00 00 00 00 00 00 ];
>>> 			interrupts = <34 0x8 33 0x8 32 0x8>;
>>
>> This isn't correct at all.  Not sure what the panic is but adding
>> "simple-bus" isn't correct.
>
> It looks like the mdio node got added in the wrong place on 8313erdb,
> relative to where it went on all other boards -- though if we were to
> ignore existing practice, it probably does belong under the gianfar  
> node
> (and not as a freestanding device).

Is there any reason that we can't change the 8313 dts to make the mdio  
standalone like all the other .dts?  (not sure if there is some pwr  
mgmt impact)

- k

RE: [PATCH] powerpc/8313erdb: fix kernel crash

[Li Yang]

> -----Original Message-----
> From: Kumar Gala [mailto:galak@kernel.crashing.org]=20
> Sent: Tuesday, January 06, 2009 12:25 AM
> To: Li Yang-R58472
> Cc: linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] powerpc/8313erdb: fix kernel crash
>=20
>=20
> On Jan 5, 2009, at 4:47 AM, Li Yang wrote:
>=20
> > Probe the new mdio node added by b31a1d8b.  Fix kernel=20
> panic problem=20
> > when gianfar driver wants to get the of_platform_device of=20
> that mdio.
> >
> > Signed-off-by: Li Yang <leoli@freescale.com>
> > ---
> > arch/powerpc/boot/dts/mpc8313erdb.dts |    2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/arch/powerpc/boot/dts/mpc8313erdb.dts b/arch/powerpc/=20
> > boot/dts/mpc8313erdb.dts index d4df8b6..917d466 100644
> > --- a/arch/powerpc/boot/dts/mpc8313erdb.dts
> > +++ b/arch/powerpc/boot/dts/mpc8313erdb.dts
> > @@ -222,7 +222,7 @@
> > 			cell-index =3D <1>;
> > 			device_type =3D "network";
> > 			model =3D "eTSEC";
> > -			compatible =3D "gianfar";
> > +			compatible =3D "gianfar", "simple-bus";
> > 			reg =3D <0x25000 0x1000>;
> > 			local-mac-address =3D [ 00 00 00 00 00 00 ];
> > 			interrupts =3D <34 0x8 33 0x8 32 0x8>;
>=20
> This isn't correct at all.  Not sure what the panic is but=20
> adding "simple-bus" isn't correct.

The panic is caused because mdio node associated with this TSEC is not
probed.  It may be better to have more exception check in the gianfar
driver rather than panic directly.  Anyway, mdio need to be probed as
of_platform_device for it to work correctly with phy driver.

I'm not sure if adding "simple-bus" is the preferred solution.  It is
being discussed in a separate mail thread.

- Leo