On Sun, Aug 8, 2010 at 1:40 AM, Anton Vorontsov <cbouatmai...@gmail.com> wrote:
> On Sat, Aug 07, 2010 at 06:40:22PM -0600, Grant Likely wrote:
> [...]
>> > static int __init mpc8xxx_add_gpiochips(void)
>> > {
>> >+ const struct of_device_id *id;
>> > struct device_node *np;
>> >
>> >- for_each_compatible_node(np, NULL, "fsl,mpc8349-gpio")
>> >- mpc8xxx_add_controller(np);
>> >-
>> >- for_each_compatible_node(np, NULL, "fsl,mpc8572-gpio")
>> >- mpc8xxx_add_controller(np);
>> >-
>> >- for_each_compatible_node(np, NULL, "fsl,mpc8610-gpio")
>> >+ for_each_matching_node(np, mpc8xxx_gpio_ids) {
>> >+ id = of_match_node(mpc8xxx_gpio_ids, np);
>> >+ if (id)
>> >+ np->data = id->data;
>> > mpc8xxx_add_controller(np);
>> >+ }
> [...]
>> Actually, there is absolutely no reason to keep mpc8xxx_add_gpiochip()
>> as a separate function with the simplification of
>> mpc8xxx_add_gpiochips(). I'd simplify the whole thing by merging the
>> two functions together.
>
> You mean mpc8xxx_add_controller()? Putting 65-line function
> on a second indentation level, inside the for loop... sounds
> like a bad idea.
That's a good point. I was thinking about it from the wrong way
around (that the function could bail at the top on a non-match; which
obviously isn't the case). Anatolij, I was wrong on this point.
Sorry I didn't reply sooner before you respun the patch. :-(
g.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
