On Tue, Sep 02, 2008 at 08:43:56AM -0700, David Brownell wrote:
> Snapshot of what's in my tree ... 

Great, a few comments below.

> # should have the base GPIO set up better; minimally
> # in a system header, ideally platform data ...

Could you describe this a bit more ? What do you mean by "base GPIO" ?
If you mean the first gpio number for passing to struct gpio_chip we
could use a platform_data if we make this one also a platform_driver,
right ?

> 
> # the whole twl driver should be "new style".

maybe only twl4030-core.c should hold the whole i2c new style thingy,
all the other drivers (usb, vibrators, keypad, rtc, etc) should/could
be a platform_driver.

>  static struct irq_chip twl4030_gpio_irq_chip = {
> -     .name   = "twl4030-gpio",
> +     .name   = "twl4030",

twl4030-core.c already uses this name. Wouldn't it be odd when cat
/proc/interrupts ? I'd like to see which are gpio interrupts and which
aren't

>       down(&gpio_sem);
>       if (gpio_usage_count & (0x1 << gpio))
>               ret = -EBUSY;

how about also putting the brackets for the if(), maybe in a cleanup
patch before this one ?

>       if ((gpio_usage_count & (0x1 << gpio)) == 0)
>               ret = -EPERM;

missing brackets.

-- 
balbi
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to