On Friday 10 June 2011, Ying-Chun Liu (PaulLiu) wrote: > From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org> > > Add DA9052 gpio driver from Dialog. > Modify Kconfig/Makefile for DA9052 gpio driver. > > Signed-off-by: Zhou Jingyu <jingyu.z...@freescale.com> > Acked-by: Lily Zhang <r58...@freescale.com> > Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org> > --- > drivers/gpio/Kconfig | 7 + > drivers/gpio/Makefile | 1 + > drivers/gpio/da9052-gpio.c | 731 > ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 739 insertions(+), 0 deletions(-) > create mode 100644 drivers/gpio/da9052-gpio.c
The name should now be 'gpio-da9052.c', not da9052-gpio.c > +#if (DA9052_GPIO_PIN_0 == DA9052_GPIO_CONFIG) > + da9052_lock(da9052); > + msg.addr = DA9052_GPIO0001_REG; > + msg.data = 0; > + > + if (da9052->read(da9052, &msg)) { > + da9052_unlock(da9052); > + return -EIO; > + } > + > + created_val = create_gpio_config_value(DEFAULT_GPIO0_FUNCTION, > + DEFAULT_GPIO0_TYPE, DEFAULT_GPIO0_MODE); > + msg.data &= DA9052_GPIO_MASK_UPPER_NIBBLE; > + msg.data |= created_val; > + > + if (da9052->write(da9052, &msg)) { > + da9052_unlock(da9052); > + return -EIO; > + } > + da9052_unlock(da9052); > +#endif Why the ifdef? All configuration needs to happen at runtime, not at compile-time if you want to run on multiple different configurations. You are duplicating this for a number of pins. Please make this an separate function that takes arguments depending on the configuration to avoid having the same code multiple times. > +s32 da9052_gpio_multiple_read(struct da9052_gpio_multiple_read > *multiple_port, > + struct da9052 *da9052) > +{ > +} > +EXPORT_SYMBOL(da9052_gpio_multiple_read); Why does the gpio driver export functions? It should be self-contained AFAICT. Arnd _______________________________________________ linaro-dev mailing list linaro-dev@lists.linaro.org http://lists.linaro.org/mailman/listinfo/linaro-dev