On Wed, 2014-12-24 at 20:06 +0100, Hans de Goede wrote:
> @@ -27,6 +32,17 @@ enum axp209_reg {
>  
>  #define AXP209_POWEROFF                      (1 << 7)
>  
> +#define AXP209_GPIO_OUTPUT_LOW               0x00
> +#define AXP209_GPIO_OUTPUT_HIGH              0x01
> +#define AXP209_GPIO_INPUT            0x02
> +
> +#define AXP209_GPIO_OUTPUT_LOW               0x00
> +#define AXP209_GPIO_OUTPUT_HIGH              0x01

Aren't these LOW+HIGH ones duplicated?

Also, they lack the helpful comments which you added to the GPIO3 ones.

I'd be included to add a /* GPIO3 is different from the others */ just
here too (also: "sigh" re h/w inconsistency...).

> +#define AXP209_GPIO3_OUTPUT_LOW              0x00 /* Drive pin low, Output 
> mode */
> +#define AXP209_GPIO3_OUTPUT_HIGH     0x02 /* Float pin, Output mode */

Is a floating output really a thing or is this a cut-and-paste-o?

> +#define AXP209_GPIO3_INPUT           0x06 /* Float pin, Input mode */
> +
> +int axp_gpio_direction_output(unsigned int pin, unsigned int val)
> +{
> +     u8 reg = axp209_get_gpio_ctrl_reg(pin);
> +
> +     if (val) {
> +             val = (pin == 3) ? AXP209_GPIO3_OUTPUT_HIGH : 
> +                                AXP209_GPIO_OUTPUT_HIGH;
> +     } else {
> +             val = (pin == 3) ? AXP209_GPIO3_OUTPUT_LOW : 
> +                                AXP209_GPIO_OUTPUT_LOW;

Both OUTPUT_LOW values happen to be the same, although I could see why
you might want to do it this way for consistency.

Ian.

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to