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

2008-01-10 Thread Olaf Hering
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)

2008-01-10 Thread Grant Likely
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)

2008-01-10 Thread Olof Johansson
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

2008-01-09 Thread Grant Likely
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

2008-01-09 Thread 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 [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

2008-01-09 Thread Sven Luther
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

2008-01-09 Thread 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 [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

2008-01-09 Thread Benjamin Herrenschmidt

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

2008-01-09 Thread Grant Likely
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