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

From: Anton Vorontsov <[email-masked]>
Date: Sun, 8 Aug 2010 11:40:35 +0400

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.

-- 
Anton Vorontsov
email: [email-masked]
irc://irc.freenode.net/bd2