[PATCH v2] powerpc/mpc8xxx_gpio.c: extend the driver to support mpc512x gpios

[Grant Likely]

On Sun, Aug 8, 2010 at 1:40 AM, Anton Vorontsov <cbouatmailru at 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.