Re: [RFC] add phy-handle property for fec_mpc52xx
On Wed, Jan 09, Grant Likely wrote: From: Olaf Hering [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Signed-off-by: Olaf Hering [EMAIL PROTECTED] ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Platform matching style (was:: [RFC] add phy-handle property for fec_mpc52xx)
On 1/9/08, Benjamin Herrenschmidt [EMAIL PROTECTED] wrote: 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. Tangent question: The Efika has device_type = chrp in the root node, but in Linux Efika support does not use CHRP, it uses arch/powerpc/platforms/52xx/efika.c. However, if CHRP support is compiled in then it will see the chrp device_type and bind to it before efika.c has a chance to probe. I see three reasonable solutions to this: 1. Apply a device tree fixup to change device_type from chrp to efika (the current solution) 2. Modify chrp_probe() to check specifically for the Efika when probing 3. Modify the link order so that Efika is probed before CHRP. All three of these solutions will work, but I'd like to get opinions on which is stylistically the best approach (or if there is another approach I'm missing). In general, I'm trying to reduce the Efika fixups down to only what is absolutely necessary and as much as possible work with the provided device tree. Cheers, g. -- Grant Likely, B.Sc., P.Eng. Secret Lab Technologies Ltd. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Platform matching style (was:: [RFC] add phy-handle property for fec_mpc52xx)
On Thu, Jan 10, 2008 at 08:31:18AM -0700, Grant Likely wrote: 1. Apply a device tree fixup to change device_type from chrp to efika (the current solution) 2. Modify chrp_probe() to check specifically for the Efika when probing 3. Modify the link order so that Efika is probed before CHRP. All three of these solutions will work, but I'd like to get opinions on which is stylistically the best approach (or if there is another approach I'm missing). In general, I'm trying to reduce the Efika fixups down to only what is absolutely necessary and as much as possible work with the provided device tree. (3) sounds fragile to me. There's already code in the kernel that does (2): pSeries_probe checks to make sure it's not running on a cell blade. That has the benefit of presenting something closer to the real device tree to the user through /proc/device-tree, not that I'm sure it has all that much value. -Olof ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[RFC] add phy-handle property for fec_mpc52xx
From: Olaf Hering [EMAIL PROTECTED] 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 [EMAIL PROTECTED] --- 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 +ihandlephandle +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
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 [EMAIL PROTECTED] Genesi, Manager, Developer Relations Grant Likely wrote: From: Olaf Hering [EMAIL PROTECTED] 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 [EMAIL PROTECTED] --- 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 + ihandlephandle + 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
On Wed, Jan 09, 2008 at 04:56:17PM +, 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] add phy-handle property for fec_mpc52xx
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 [EMAIL PROTECTED] Genesi, Manager, Developer Relations ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] add phy-handle property for fec_mpc52xx
On Wed, 2008-01-09 at 09:32 -0700, Grant Likely wrote: From: Olaf Hering [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED] 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 + ihandlephandle + 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 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] add phy-handle property for fec_mpc52xx
On 1/9/08, Benjamin Herrenschmidt [EMAIL PROTECTED] wrote: On Wed, 2008-01-09 at 09:32 -0700, Grant Likely wrote: From: Olaf Hering [EMAIL PROTECTED] 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 [EMAIL PROTECTED] Acked-by: Benjamin Herrenschmidt [EMAIL PROTECTED] 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. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev