[U-Boot] [PATCH 1/2] ARM: tegra: fix GPIO init table programming

From: Stephen Warren swarren@nvidia.com
Tegra's gpio_config_table() currently uses common GPIO APIs. These used to work without requesting the GPIO, but since commit 2fccd2d96bad "tegra: Convert tegra GPIO driver to use driver model" no longer do so. This prevents any of the GPIO initialization table from being applied to HW. Fix gpio_config_table() to directly program the HW to solve this.
Fixes: 2fccd2d96bad ("tegra: Convert tegra GPIO driver to use driver model") Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/gpio/tegra_gpio.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index 4921f0ff42e9..c0ae7719e2c9 100644 --- a/drivers/gpio/tegra_gpio.c +++ b/drivers/gpio/tegra_gpio.c @@ -211,13 +211,15 @@ void gpio_config_table(const struct tegra_gpio_config *config, int len) for (i = 0; i < len; i++) { switch (config[i].init) { case TEGRA_GPIO_INIT_IN: - gpio_direction_input(config[i].gpio); + set_direction(config[i].gpio, 0); break; case TEGRA_GPIO_INIT_OUT0: - gpio_direction_output(config[i].gpio, 0); + set_level(config[i].gpio, 0); + set_direction(config[i].gpio, 1); break; case TEGRA_GPIO_INIT_OUT1: - gpio_direction_output(config[i].gpio, 1); + set_level(config[i].gpio, 1); + set_direction(config[i].gpio, 1); break; } set_config(config[i].gpio, 1);

From: Stephen Warren swarren@nvidia.com
Tegra's GPIO driver currently enables pins as GPIO as soon as they're requested. This is not safe, since the desired direction and output value are not yet known. This could cause a glitch on the output pins between gpio_request() and gpio_direction_*(), depending on what values happen to be in the GPIO controller's in/out and out-value registers vs. the final desired configuration.
To solve this, defer enabling pins as GPIOs until some gpio_direction_*() is invoked, and the desired configuration is explicitly programmed.
In theory this change could cause regressions, if code exists that claims a GPIO, never explicitly sets a direction, and then gets/sets the GPIO value based on that assumption. However, I've read through all the Tegra- related board files and device drivers that touch GPIOs and I do not see such buggy code anywhere.
Signed-off-by: Stephen Warren swarren@nvidia.com --- drivers/gpio/tegra_gpio.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index c0ae7719e2c9..2dfd02d62053 100644 --- a/drivers/gpio/tegra_gpio.c +++ b/drivers/gpio/tegra_gpio.c @@ -136,17 +136,6 @@ static void set_level(unsigned gpio, int high) * Generic_GPIO primitives. */
-static int tegra_gpio_request(struct udevice *dev, unsigned offset, - const char *label) -{ - struct tegra_port_info *state = dev_get_priv(dev); - - /* Configure as a GPIO */ - set_config(state->base_gpio + offset, 1); - - return 0; -} - /* set GPIO pin 'gpio' as an input */ static int tegra_gpio_direction_input(struct udevice *dev, unsigned offset) { @@ -155,6 +144,9 @@ static int tegra_gpio_direction_input(struct udevice *dev, unsigned offset) /* Configure GPIO direction as input. */ set_direction(state->base_gpio + offset, 0);
+ /* Enable the pin as a GPIO */ + set_config(state->base_gpio + offset, 1); + return 0; }
@@ -171,6 +163,9 @@ static int tegra_gpio_direction_output(struct udevice *dev, unsigned offset, /* Configure GPIO direction as output. */ set_direction(gpio, 1);
+ /* Enable the pin as a GPIO */ + set_config(state->base_gpio + offset, 1); + return 0; }
@@ -256,7 +251,6 @@ static int tegra_gpio_xlate(struct udevice *dev, struct gpio_desc *desc, }
static const struct dm_gpio_ops gpio_tegra_ops = { - .request = tegra_gpio_request, .direction_input = tegra_gpio_direction_input, .direction_output = tegra_gpio_direction_output, .get_value = tegra_gpio_get_value,

On Wednesday, 23 September 2015, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Tegra's GPIO driver currently enables pins as GPIO as soon as they're requested. This is not safe, since the desired direction and output value are not yet known. This could cause a glitch on the output pins between gpio_request() and gpio_direction_*(), depending on what values happen to be in the GPIO controller's in/out and out-value registers vs. the final desired configuration.
To solve this, defer enabling pins as GPIOs until some gpio_direction_*() is invoked, and the desired configuration is explicitly programmed.
In theory this change could cause regressions, if code exists that claims a GPIO, never explicitly sets a direction, and then gets/sets the GPIO value based on that assumption. However, I've read through all the Tegra- related board files and device drivers that touch GPIOs and I do not see such buggy code anywhere.
Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/gpio/tegra_gpio.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Wednesday, 23 September 2015, Stephen Warren swarren@wwwdotorg.org wrote:
From: Stephen Warren swarren@nvidia.com
Tegra's gpio_config_table() currently uses common GPIO APIs. These used to work without requesting the GPIO, but since commit 2fccd2d96bad "tegra: Convert tegra GPIO driver to use driver model" no longer do so. This prevents any of the GPIO initialization table from being applied to HW. Fix gpio_config_table() to directly program the HW to solve this.
Fixes: 2fccd2d96bad ("tegra: Convert tegra GPIO driver to use driver model") Signed-off-by: Stephen Warren swarren@nvidia.com
drivers/gpio/tegra_gpio.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (2)
-
Simon Glass
-
Stephen Warren