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