
Hi,
On 28-12-14 10:34, Ian Campbell wrote:
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?
Oops, copy + paste fail.
Also, they lack the helpful comments which you added to the GPIO3 ones.
That is because they are self explanatory, unlike gpio3 which is interesting with decoupled control bits, gpio0-2 simply have 3 bits which code 0 - 7, with 3+ being not interesting to us, so we only use 0-2 which are low / high / input. Still I'll add some comments for you :)
I'd be included to add a /* GPIO3 is different from the others */ just here too (also: "sigh" re h/w inconsistency...).
Will do.
+#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?
Rather then a push-pull driver, gpio3 seems to have an open collector / open drain driver, in combination with the output enable transistor which one would expect on a tristate driver. So it seems there are 2 ways to float the pin, one by not driving the output-enable transistor, but also by driving the output-enable transister, but not pulling the output low, at least the datasheet describes 2 separate bits one to select output/input the other to select drive-low/float.
I've chosen to map drvie output, but do not pull output low as AXP209_GPIO3_OUTPUT_HIGH, and that is what the comment tries to convey. Note this is all my interpretation of the not so helpful datasheet.
+#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.
I also notices the happen to be the same, but I indeed did things this way for consistency.
Regards,
Hans