Re: [RESEND][PATCH][POWERPC] PIKA Warp: Update platform code to supportRev B boards

["Grant Likely" <grant.likely@secretlab.ca>]

On Sun, Apr 27, 2008 at 6:58 PM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi Sean,
>
>
>  On Sun, 27 Apr 2008 15:25:46 -0400 Sean MacLennan <smaclennan@pikatech.com> wrote:
>  >
>  > +++ b/arch/powerpc/platforms/44x/warp-nand.c
>  > @@ -11,8 +11,10 @@
>  >  #include <linux/mtd/partitions.h>
>  >  #include <linux/mtd/nand.h>
>  >  #include <linux/mtd/ndfc.h>
>  > +#include <linux/of_platform.h>
>
>  You really want linux.of.h (unless there was some preexisting reason to
>  require of_platform.h).
>
>
>  > +static void pika_setup_critical_temp(struct i2c_client *client)
>  > +{
>  > +     struct device_node *np;
>  > +     int irq, rc;
>  > +
>  > +     /* Setup power LEDS for possible critical temp */
>  > +     np = of_find_compatible_node(NULL, NULL, "ibm,gpio-440EP");
>  > +     if (np)
>  > +             np = of_find_compatible_node(np, NULL, "ibm,gpio-440EP");
>
>  Did you really mean to test if (!np) above instead?

Actually, it looks like he's trying to find the second gpio node in the tree.

Sean, if that is true, then this is a very fragile way to do it.
Really, you should have a phandle somewhere that points to the GPIO
node that your LEDs are attached to.  Others have been addressing the
same problem and the consensus seems to be to add a 'leds' node for
each of your leds with a phandle and gpio descriptor to the gpio node.

See the documentation added by this patch (section 't'):
http://patchwork.ozlabs.org/linuxppc/patch?id=18156

Cheers,
g.

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