From f9d3cab091522c8470e9ebd4a8967d00f49efc4a Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Wed, 23 Sep 2015 12:12:59 -0600 Subject: ARM: tegra: fix GPIO init table programming 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 Reviewed-by: Simon Glass Signed-off-by: Tom Warren --- drivers/gpio/tegra_gpio.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index 4921f0ff42e..c0ae7719e2c 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); -- cgit v1.2.3 From 0c35e3a8b406061005c481fccdb9bf2cfe09fd41 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Wed, 23 Sep 2015 12:13:00 -0600 Subject: ARM: tegra: don't enable GPIOs until direction is set 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 Reviewed-by: Simon Glass Signed-off-by: Tom Warren --- drivers/gpio/tegra_gpio.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) (limited to 'drivers') diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index c0ae7719e2c..2dfd02d6205 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, -- cgit v1.2.3 From 9f75a222c7ff8f475e74252c71c3e83e4aef62c5 Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Fri, 25 Sep 2015 10:44:07 -0600 Subject: gpio: tegra: remove unused type These enum values aren't used anywhere. Remove them. Signed-off-by: Stephen Warren Reviewed-by: Simon Glass Signed-off-by: Tom Warren --- drivers/gpio/tegra_gpio.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers') diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index 2dfd02d6205..f9c06fe88b7 100644 --- a/drivers/gpio/tegra_gpio.c +++ b/drivers/gpio/tegra_gpio.c @@ -25,13 +25,6 @@ DECLARE_GLOBAL_DATA_PTR; -enum { - TEGRA_CMD_INFO, - TEGRA_CMD_PORT, - TEGRA_CMD_OUTPUT, - TEGRA_CMD_INPUT, -}; - struct tegra_gpio_platdata { struct gpio_ctlr_bank *bank; const char *port_name; /* Name of port, e.g. "B" */ -- cgit v1.2.3 From fe82857c4b1667fff8108eab77340ae76016215a Mon Sep 17 00:00:00 2001 From: Stephen Warren Date: Fri, 25 Sep 2015 10:44:08 -0600 Subject: gpio: tegra: use named constants In order to make it clear what the parameters to set_config() and set_direction() mean, and similarly for the return values from the respective get_*(), define named constants for these values. Disassembly shows no diff in the generated code, except that the order of the code in the branches of tegra_gpio_get_function() gets modified without affecting behaviour. Suggested-by: Tom Warren Signed-off-by: Stephen Warren Signed-off-by: Tom Warren --- drivers/gpio/tegra_gpio.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c index f9c06fe88b7..8e880e276f0 100644 --- a/drivers/gpio/tegra_gpio.c +++ b/drivers/gpio/tegra_gpio.c @@ -1,6 +1,6 @@ /* * NVIDIA Tegra20 GPIO handling. - * (C) Copyright 2010-2012 + * (C) Copyright 2010-2012,2015 * NVIDIA Corporation * * SPDX-License-Identifier: GPL-2.0+ @@ -25,6 +25,11 @@ DECLARE_GLOBAL_DATA_PTR; +static const int CONFIG_SFIO = 0; +static const int CONFIG_GPIO = 1; +static const int DIRECTION_INPUT = 0; +static const int DIRECTION_OUTPUT = 1; + struct tegra_gpio_platdata { struct gpio_ctlr_bank *bank; const char *port_name; /* Name of port, e.g. "B" */ @@ -37,7 +42,7 @@ struct tegra_port_info { int base_gpio; /* Port number for this port (0, 1,.., n-1) */ }; -/* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */ +/* Return config of pin 'gpio' as GPIO (1) or SFIO (0) */ static int get_config(unsigned gpio) { struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE; @@ -46,15 +51,15 @@ static int get_config(unsigned gpio) int type; u = readl(&bank->gpio_config[GPIO_PORT(gpio)]); - type = (u >> GPIO_BIT(gpio)) & 1; + type = (u >> GPIO_BIT(gpio)) & 1; debug("get_config: port = %d, bit = %d is %s\n", GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO"); - return type; + return type ? CONFIG_GPIO : CONFIG_SFIO; } -/* Config pin 'gpio' as GPIO or SFPIO, based on 'type' */ +/* Config pin 'gpio' as GPIO or SFIO, based on 'type' */ static void set_config(unsigned gpio, int type) { struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE; @@ -65,7 +70,7 @@ static void set_config(unsigned gpio, int type) GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO"); u = readl(&bank->gpio_config[GPIO_PORT(gpio)]); - if (type) /* GPIO */ + if (type != CONFIG_SFIO) u |= 1 << GPIO_BIT(gpio); else u &= ~(1 << GPIO_BIT(gpio)); @@ -86,7 +91,7 @@ static int get_direction(unsigned gpio) debug("get_direction: port = %d, bit = %d, %s\n", GPIO_FULLPORT(gpio), GPIO_BIT(gpio), dir ? "OUT" : "IN"); - return dir; + return dir ? DIRECTION_OUTPUT : DIRECTION_INPUT; } /* Config GPIO pin 'gpio' as input or output (OE) as per 'output' */ @@ -100,7 +105,7 @@ static void set_direction(unsigned gpio, int output) GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN"); u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]); - if (output) + if (output != DIRECTION_INPUT) u |= 1 << GPIO_BIT(gpio); else u &= ~(1 << GPIO_BIT(gpio)); @@ -135,7 +140,7 @@ static int tegra_gpio_direction_input(struct udevice *dev, unsigned offset) struct tegra_port_info *state = dev_get_priv(dev); /* Configure GPIO direction as input. */ - set_direction(state->base_gpio + offset, 0); + set_direction(state->base_gpio + offset, DIRECTION_INPUT); /* Enable the pin as a GPIO */ set_config(state->base_gpio + offset, 1); @@ -154,7 +159,7 @@ static int tegra_gpio_direction_output(struct udevice *dev, unsigned offset, set_level(gpio, value); /* Configure GPIO direction as output. */ - set_direction(gpio, 1); + set_direction(gpio, DIRECTION_OUTPUT); /* Enable the pin as a GPIO */ set_config(state->base_gpio + offset, 1); @@ -199,18 +204,18 @@ 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: - set_direction(config[i].gpio, 0); + set_direction(config[i].gpio, DIRECTION_INPUT); break; case TEGRA_GPIO_INIT_OUT0: set_level(config[i].gpio, 0); - set_direction(config[i].gpio, 1); + set_direction(config[i].gpio, DIRECTION_OUTPUT); break; case TEGRA_GPIO_INIT_OUT1: set_level(config[i].gpio, 1); - set_direction(config[i].gpio, 1); + set_direction(config[i].gpio, DIRECTION_OUTPUT); break; } - set_config(config[i].gpio, 1); + set_config(config[i].gpio, CONFIG_GPIO); } } -- cgit v1.2.3