[RFC] add phy-handle property for fec_mpc52xx

[RFC] add phy-handle property for fec_mpc52xx

[Grant Likely]

From: Olaf Hering <olaf@aepfle.de>

The new network driver fec_mpc52xx will not work on efika because the
firmware does not provide all required properties.
http://www.powerdeveloper.org/asset/by-id/46 has a Forth script to
create more properties. But only the phy stuff is required to get a
working network.

This should go into the kernel because its appearently
impossible to boot the script via tftp and then load the real boot
binary (yaboot or zimage).

(Olaf's s-o-b line needs to go here)
Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

Here's my respin of Olaf's patch to move it to fixup_device_tree_efika()
and to make it check if the nodes exist before blindly creating them.

Cheers,
g.

 arch/powerpc/kernel/prom_init.c |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 1add6ef..5d89a21 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -2216,6 +2216,45 @@ static void __init fixup_device_tree_efika(void)
 			prom_printf("fixup_device_tree_efika: ",
 				"skipped entry %x - setprop error\n", i);
 	}
+
+	/* Make sure ethernet mdio bus node exists */
+	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio"));
+	if (!PHANDLE_VALID(node)) {
+		prom_printf("Adding Ethernet MDIO node\n");
+		call_prom("interpret", 1, 1,
+			" s\" /builtin\" find-device"
+			" new-device"
+				" 1 encode-int s\" #address-cells\" property"
+				" 0 encode-int s\" #size-cells\" property"
+				" s\" mdio\" 2dup device-name device-type"
+				" s\" mpc5200b-fec-phy\" encode-string"
+				" s\" compatible\" property"
+				" 0xf0003000 0x400 reg"
+				" 0x2 encode-int"
+				" 0x5 encode-int encode+"
+				" 0x3 encode-int encode+"
+				" s\" interrupts\" property"
+			" finish-device");
+	};
+
+	/* Make sure ethernet phy device node exist */
+	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio/ethernet-phy"));
+	if (!PHANDLE_VALID(node)) {
+		prom_printf("Adding Ethernet PHY node\n");
+		call_prom("interpret", 1, 1,
+			" s\" /builtin/mdio\" find-device"
+			" new-device"
+				" s\" ethernet-phy\" device-name"
+				" 0x10 encode-int s\" reg\" property"
+				" my-self"
+				" ihandle>phandle"
+			" finish-device"
+			" s\" /builtin/ethernet\" find-device"
+				" encode-int"
+				" s\" phy-handle\" property"
+			" device-end");
+	}
+
 }
 #else
 #define fixup_device_tree_efika()

Re: [RFC] add phy-handle property for fec_mpc52xx

[Matt Sealey]

Okay I have a better suggestion.

Apply all the prom_init fixes you like. But, instead of swizzling on the
individual nodes, do it blanket for the firmware version.

For instance, wrap the entire efika fixups stuff with a check for the
openprom property "built-on" - this is the date the firmware was built.
efika.forth will change this!

Just make sure it's less than.. let's say, a certain version of efika.forth,
and I will roll a version which has a higher version and some extra features
like CAN/I2C exposure.

If you run efika.forth it will not touch the device tree. If you don't, it
will add the small amount of patches. Add a huge comment that this hunk of
code should be scheduled for deletion at some later date. Put a Kconfig
around it so it can be taken out, even, at distro option.

Does that sound better? Right now I'm a little too busy to write and
compile test code for it so I will not venture to submit a patch..
do we want to work on a comprehensive, required efika.forth release
that fixes these things together, or is it just my job for now?

-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Grant Likely wrote:
> From: Olaf Hering <olaf@aepfle.de>
> 
> The new network driver fec_mpc52xx will not work on efika because the
> firmware does not provide all required properties.
> http://www.powerdeveloper.org/asset/by-id/46 has a Forth script to
> create more properties. But only the phy stuff is required to get a
> working network.
> 
> This should go into the kernel because its appearently
> impossible to boot the script via tftp and then load the real boot
> binary (yaboot or zimage).
> 
> (Olaf's s-o-b line needs to go here)
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
> Here's my respin of Olaf's patch to move it to fixup_device_tree_efika()
> and to make it check if the nodes exist before blindly creating them.
> 
> Cheers,
> g.
> 
>  arch/powerpc/kernel/prom_init.c |   39 +++++++++++++++++++++++++++++++++++++++
>  1 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 1add6ef..5d89a21 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -2216,6 +2216,45 @@ static void __init fixup_device_tree_efika(void)
>  			prom_printf("fixup_device_tree_efika: ",
>  				"skipped entry %x - setprop error\n", i);
>  	}
> +
> +	/* Make sure ethernet mdio bus node exists */
> +	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio"));
> +	if (!PHANDLE_VALID(node)) {
> +		prom_printf("Adding Ethernet MDIO node\n");
> +		call_prom("interpret", 1, 1,
> +			" s\" /builtin\" find-device"
> +			" new-device"
> +				" 1 encode-int s\" #address-cells\" property"
> +				" 0 encode-int s\" #size-cells\" property"
> +				" s\" mdio\" 2dup device-name device-type"
> +				" s\" mpc5200b-fec-phy\" encode-string"
> +				" s\" compatible\" property"
> +				" 0xf0003000 0x400 reg"
> +				" 0x2 encode-int"
> +				" 0x5 encode-int encode+"
> +				" 0x3 encode-int encode+"
> +				" s\" interrupts\" property"
> +			" finish-device");
> +	};
> +
> +	/* Make sure ethernet phy device node exist */
> +	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio/ethernet-phy"));
> +	if (!PHANDLE_VALID(node)) {
> +		prom_printf("Adding Ethernet PHY node\n");
> +		call_prom("interpret", 1, 1,
> +			" s\" /builtin/mdio\" find-device"
> +			" new-device"
> +				" s\" ethernet-phy\" device-name"
> +				" 0x10 encode-int s\" reg\" property"
> +				" my-self"
> +				" ihandle>phandle"
> +			" finish-device"
> +			" s\" /builtin/ethernet\" find-device"
> +				" encode-int"
> +				" s\" phy-handle\" property"
> +			" device-end");
> +	}
> +
>  }
>  #else
>  #define fixup_device_tree_efika()
> 

Re: [RFC] add phy-handle property for fec_mpc52xx

[Sven Luther]

On Wed, Jan 09, 2008 at 04:56:17PM +0000, Matt Sealey wrote:
> Okay I have a better suggestion.
> 
> Apply all the prom_init fixes you like. But, instead of swizzling on the
> individual nodes, do it blanket for the firmware version.
> 
> For instance, wrap the entire efika fixups stuff with a check for the
> openprom property "built-on" - this is the date the firmware was built.
> efika.forth will change this!

This is crazy, and not future proof. The way Grant did it, checking for
the existence of the node before creating is enough for any reasonable
upgrade to the firmware.

If future firmware will break with this, then they will break other
stuff anyway, and a new patch is needed.

> Just make sure it's less than.. let's say, a certain version of efika.forth,
> and I will roll a version which has a higher version and some extra features
> like CAN/I2C exposure.
> 
> If you run efika.forth it will not touch the device tree. If you don't, it
> will add the small amount of patches. Add a huge comment that this hunk of
> code should be scheduled for deletion at some later date. Put a Kconfig
> around it so it can be taken out, even, at distro option.

Have you not read Grant's code ? It create the nodes only if they are
not existent already.

Friendly,

Sven Luther

Re: [RFC] add phy-handle property for fec_mpc52xx

[Matt Sealey]

Sven Luther wrote:

> This is crazy, and not future proof. The way Grant did it, checking for
> the existence of the node before creating is enough for any reasonable
> upgrade to the firmware.
> 
> If future firmware will break with this, then they will break other
> stuff anyway, and a new patch is needed.

In other words, Grant's method is crazy and not future-proof, too.

I don't see the difference, Sven, and this demonstrates entirely

>> Just make sure it's less than.. let's say, a certain version of efika.forth,
>> and I will roll a version which has a higher version and some extra features
>> like CAN/I2C exposure.
>>
>> If you run efika.forth it will not touch the device tree. If you don't, it
>> will add the small amount of patches. Add a huge comment that this hunk of
>> code should be scheduled for deletion at some later date. Put a Kconfig
>> around it so it can be taken out, even, at distro option.
> 
> Have you not read Grant's code ? It create the nodes only if they are
> not existent already.

What if they ARE existant under other names, you will have two ethernet-phy
in the system, maybe with the same name, and this is stupid. What if it adds
the wrong properties if we work out something is wrong somewhere?

Linux prom_init must not be a moving target for firmware development - it is
the hardest thing to replace (it is not like loading and unloading a module)
and it may in fact be polluting valid device trees REGARDLESS of any checks
you might make. You cannot make Linux try and PREDICT how the device tree
looks based on arbitrary name strings, because Linux device tree usage does
not comply with the IEEE 1275-1994 definition of device tree properties.

Grant's patch should look for a compatible property with "ethernet-phy" in
it, check for phy-handle in the ethernet node, make sure that a node very
similar to mdio does not already exist, etc, it is going into a screen of
code to do it comprehensively and future proof.

Just don't do it at all. The burden is on Genesi. So, it's difficult right
now, and hard for users, so what? That's OUR problem, I dare say today it
is MY problem. I do not want this to become a LINUX problem in the future
when a new board comes out that is Efika-compatible, requiring patch
removal, reshuffling, compliance testing etc.

You do realise any new firmware with this kernel needs to work, regardless
of the patch being present, in order to be validated? We can't ship an
Efika board with a firmware which actively breaks because of a Linux
kernel patch for older boards and some ancient bad decision, and saying
"to use a new Efika you must use a brand new Linux kernel" is the kind of
insanity in embedded development which made us choose SmartFirmware over
U-Boot and FDT in the first place..

I am not happy that we are being dragged into this model of playing
"first to mainline" with arbitrary snippets of code which do NOT improve
things in the long term.


-- 
Matt Sealey <matt@genesi-usa.com>
Genesi, Manager, Developer Relations

Re: [RFC] add phy-handle property for fec_mpc52xx

[Benjamin Herrenschmidt]

On Wed, 2008-01-09 at 09:32 -0700, Grant Likely wrote:
> From: Olaf Hering <olaf@aepfle.de>
> 
> The new network driver fec_mpc52xx will not work on efika because the
> firmware does not provide all required properties.
> http://www.powerdeveloper.org/asset/by-id/46 has a Forth script to
> create more properties. But only the phy stuff is required to get a
> working network.
> 
> This should go into the kernel because its appearently
> impossible to boot the script via tftp and then load the real boot
> binary (yaboot or zimage).
> 
> (Olaf's s-o-b line needs to go here)
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

As much as I despise having to work around firmware bugs, it looks
indeed like this one has been broken for way too long to the point where
users are being hurt, distros are being hurt, and telling people to
whack things in nvramrc is just plain gross, so let's merge it.
> ---
> 
> Here's my respin of Olaf's patch to move it to fixup_device_tree_efika()
> and to make it check if the nodes exist before blindly creating them.
> 
> Cheers,
> g.
> 
>  arch/powerpc/kernel/prom_init.c |   39 +++++++++++++++++++++++++++++++++++++++
>  1 files changed, 39 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
> index 1add6ef..5d89a21 100644
> --- a/arch/powerpc/kernel/prom_init.c
> +++ b/arch/powerpc/kernel/prom_init.c
> @@ -2216,6 +2216,45 @@ static void __init fixup_device_tree_efika(void)
>  			prom_printf("fixup_device_tree_efika: ",
>  				"skipped entry %x - setprop error\n", i);
>  	}
> +
> +	/* Make sure ethernet mdio bus node exists */
> +	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio"));
> +	if (!PHANDLE_VALID(node)) {
> +		prom_printf("Adding Ethernet MDIO node\n");
> +		call_prom("interpret", 1, 1,
> +			" s\" /builtin\" find-device"
> +			" new-device"
> +				" 1 encode-int s\" #address-cells\" property"
> +				" 0 encode-int s\" #size-cells\" property"
> +				" s\" mdio\" 2dup device-name device-type"
> +				" s\" mpc5200b-fec-phy\" encode-string"
> +				" s\" compatible\" property"
> +				" 0xf0003000 0x400 reg"
> +				" 0x2 encode-int"
> +				" 0x5 encode-int encode+"
> +				" 0x3 encode-int encode+"
> +				" s\" interrupts\" property"
> +			" finish-device");
> +	};
> +
> +	/* Make sure ethernet phy device node exist */
> +	node = call_prom("finddevice", 1, 1, ADDR("/builtin/mdio/ethernet-phy"));
> +	if (!PHANDLE_VALID(node)) {
> +		prom_printf("Adding Ethernet PHY node\n");
> +		call_prom("interpret", 1, 1,
> +			" s\" /builtin/mdio\" find-device"
> +			" new-device"
> +				" s\" ethernet-phy\" device-name"
> +				" 0x10 encode-int s\" reg\" property"
> +				" my-self"
> +				" ihandle>phandle"
> +			" finish-device"
> +			" s\" /builtin/ethernet\" find-device"
> +				" encode-int"
> +				" s\" phy-handle\" property"
> +			" device-end");
> +	}
> +
>  }
>  #else
>  #define fixup_device_tree_efika()
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [RFC] add phy-handle property for fec_mpc52xx

[Grant Likely]

On 1/9/08, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Wed, 2008-01-09 at 09:32 -0700, Grant Likely wrote:
> > From: Olaf Hering <olaf@aepfle.de>
> >
> > This should go into the kernel because its appearently
> > impossible to boot the script via tftp and then load the real boot
> > binary (yaboot or zimage).
> >
> > (Olaf's s-o-b line needs to go here)
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>
> Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>
> As much as I despise having to work around firmware bugs, it looks
> indeed like this one has been broken for way too long to the point where
> users are being hurt, distros are being hurt, and telling people to
> whack things in nvramrc is just plain gross, so let's merge it.

Thanks Ben.

Olaf, can you please reply with your signed-off-by line?  Then I'll
ask paulus to pick this up for .24

Thanks,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

Re: [RFC] add phy-handle property for fec_mpc52xx

[Olaf Hering]

On Wed, Jan 09, Grant Likely wrote:

> From: Olaf Hering <olaf@aepfle.de>
> 
> The new network driver fec_mpc52xx will not work on efika because the
> firmware does not provide all required properties.
> http://www.powerdeveloper.org/asset/by-id/46 has a Forth script to
> create more properties. But only the phy stuff is required to get a
> working network.
> 
> This should go into the kernel because its appearently
> impossible to boot the script via tftp and then load the real boot
> binary (yaboot or zimage).
> 
> (Olaf's s-o-b line needs to go here)
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>


Signed-off-by: Olaf Hering <olaf@aepfle.de>