From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hans de Goede Date: Sun, 28 Dec 2014 11:35:45 +0100 Subject: [U-Boot] [PATCH 03/17] sunxi: gpio: Add support for gpio pins on the AXP209 pmic In-Reply-To: <1419759268.13595.40.camel@hellion.org.uk> References: <1419447989-21959-1-git-send-email-hdegoede@redhat.com> <1419447989-21959-4-git-send-email-hdegoede@redhat.com> <1419759268.13595.40.camel@hellion.org.uk> Message-ID: <549FDD01.40302@redhat.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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