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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
