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

Reply via email to