On Wed, Aug 27, 2014 at 03:28:06PM +0200, Thierry Reding wrote:
On Tue, Aug 26, 2014 at 03:54:50PM +0300, Tuomas Tynkkynen wrote:
On 18/08/14 10:16, Thierry Reding wrote: [...]
+static int as3722_gpio_direction_output(u8 gpio, u8 level) +{
- u8 value;
- int err;
- if (gpio > 7)
return -EINVAL;- if (level == 0)
value = AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDL;- else
value = AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDH;- err = as3722_write(AS3722_GPIO_CONTROL(gpio), value);
- if (err) {
error("as3722: failed to configure GPIO#%u as output: %d\n",gpio, err);return err;- }
- err = as3722_gpio_set(gpio, level);
- if (err < 0) {
error("as3722: failed to set GPIO#%u high: %d\n", gpio, err);return err;- }
- return 0;
+}
This function doesn't work correctly if the GPIO was originally configured as inverted and low, which GPIO#2 seems to be. (as3722_read(AS3722_GPIO_CONTROL(2), &value) returns value == 0x87)...
That should be equivalent to what we're setting but is a somewhat weird default. I guess the fact that we're inverting it and then changing the value to high in separate transactions makes the output flip twice.
+int tegra_pcie_board_init(void) +{
[...]
- err = as3722_gpio_direction_output(2, 1);
- if (err < 0) {
error("as3722: failed to set GPIO#2 high: %d\n", err);return err;- }
[...]
On my board, this call results in UART corruption, like this:
tegra-pcie: non-prefetchable memory: 0x13000000-0x20000000 tegra-pcie: prefetchable memory: 0x20000000-0x40000000 ¥É½¥¹½bªÍ¥¹b2x1, 1x1 configuration ¹Í5Rþtegra-pcie: probing port 1, using 1 lanes
Likely because GPIO#2 controls the +3.3V_LP0 rail, which powers the UART level shifters. Commenting the function call out fixes the corruption and PCI-E still works fine.
If I add a udelay(500) after the above I'm not able to reproduce the UART breakage anymore. But I guess making the AS3722 GPIO code smarter would be helpful. In the kernel this is done by checking the invert bit and then setting the value accordingly. I suppose the same could be done for the mode bits. I'll see if I can work up a patch.
How about this:
diff --git a/drivers/power/as3722.c b/drivers/power/as3722.c index 59d1bf1b50b0..393dc8608d07 100644 --- a/drivers/power/as3722.c +++ b/drivers/power/as3722.c @@ -17,6 +17,7 @@ #define AS3722_GPIO_CONTROL(n) (0x08 + (n)) #define AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDH (1 << 0) #define AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDL (7 << 0) +#define AS3722_GPIO_CONTROL_MODE_MASK (7 << 0) #define AS3722_GPIO_CONTROL_INVERT (1 << 7) #define AS3722_LDO_VOLTAGE(n) (0x10 + (n)) #define AS3722_GPIO_SIGNAL_OUT 0x20 @@ -220,10 +221,21 @@ int as3722_gpio_direction_output(struct as3722 *pmic, unsigned int gpio, if (gpio > 7) return -EINVAL;
+ err = as3722_read(pmic, AS3722_GPIO_CONTROL(gpio), &value); + if (err < 0) { + error("failed to read GPIO#%u control register: %d", gpio, err); + return err; + } + + if (value & AS3722_GPIO_CONTROL_INVERT) + level = !level; + + value &= ~AS3722_GPIO_CONTROL_MODE_MASK; + if (level == 0) - value = AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDL; + value |= AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDL; else - value = AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDH; + value |= AS3722_GPIO_CONTROL_MODE_OUTPUT_VDDH;
err = as3722_write(pmic, AS3722_GPIO_CONTROL(gpio), value); if (err) {