[PATCH 01/17] ARM: pxa/raumfeld: add basic structure for devices

[Russell King - ARM Linux]

On Wed, Nov 25, 2009 at 05:51:40PM +0100, Marek Vasut wrote:
> Dne St 25. listopadu 2009 16:26:32 Mike Rapoport napsal(a):
> > Daniel,
> > Below are my comments to the patches. Some of the comments apply to several
> > patches, but I'm too lazy to copy them into relevant threads :)
> > 
> > Daniel Mack wrote:
> > > +static void __init raumfeld_common_init(void)
> > > +{
> > > +	enable_irq_wake(IRQ_WAKEUP0);
> > > +	pxa_set_ffuart_info(NULL);
> > > +
> > > +	gpio_request(mfp_to_gpio(GPIO_W2W_RESET), "Wi2Wi reset");
> > 
> > gpio_request may fail, thought it's unlikely to happen. Anyway, adding
> >  check for it's return value seems to be a good practice.
> 
> Well if the gpio_request fails here, you are doomed anyway. There's nowhere
> to return to. But it's a good idea to fail and don't try to request other
> GPIOs.

Not entirely true.  If a gpio request fails because somehow it was claimed
by something else, that doesn't mean other stuff will also fail.  Also,
don't think that if a gpio_request in such a function fails you have to
exit from it.

A good idea is to do this:

	ret = gpio_request(gpio, name);
	if (ret)
		printk(KERN_ERR "Myplatform: gpio%u(%s) request failed: %d\n",
			gpio, name, ret);

Even better:

static void claim_gpio_output(int gpio, const char *name, int value)
{
	int ret = gpio_request(gpio, name);
	if (ret) {
		printk(KERN_ERR "Myplatform: gpio%u(%s) request failed: %d\n",
			gpio, name, ret);
	} else {
		gpio_direction_output(gpio, value);
	}
}

and then you're not duplicating the multiple gpio calls.