diff mbox

[U-Boot,RFC,v2,13/13] tegra: Convert tegra GPIO driver to use driver model

Message ID 1399656509-28082-14-git-send-email-sjg@chromium.org
State RFC
Delegated to: Tom Rini
Headers show

Commit Message

Simon Glass May 9, 2014, 5:28 p.m. UTC
This is an implementation of GPIOs for Tegra that uses driver model. It has
been tested on trimslice and also using the new iotrace feature.

The implementation uses a top-level GPIO device (which has no actual GPIOS).
Under this all the banks are created as separate GPIO devices.

The GPIOs are named as per the Tegra datasheet/header files: A0..A7, B0..B7,
..., Z0..Z7, AA0..AA7, etc.

Since driver model is not yet available before relocation, or in SPL, a
special function is provided for seaboard's SPL code.

This series is marked RFC since the Tegra driver may need further discussion.
Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v3:
- Move dm command enable to previous patch
- Use gpio number for the internal helper functions

Changes in v2:
- Split out driver model changes into separate patches
- Correct bugs found during testing

 arch/arm/include/asm/arch-tegra/gpio.h |  14 +-
 board/nvidia/seaboard/seaboard.c       |   2 +-
 drivers/gpio/tegra_gpio.c              | 313 +++++++++++++++++++++++++++------
 include/configs/tegra-common.h         |   1 +
 4 files changed, 266 insertions(+), 64 deletions(-)

Comments

Stephen Warren May 13, 2014, 7:30 p.m. UTC | #1
On 05/09/2014 11:28 AM, Simon Glass wrote:
> This is an implementation of GPIOs for Tegra that uses driver model. It has
> been tested on trimslice and also using the new iotrace feature.
> 
> The implementation uses a top-level GPIO device (which has no actual GPIOS).
> Under this all the banks are created as separate GPIO devices.

I still believe that there is a single Tegra GPIO controller, and this
should be represented by a single device in the U-Boot device model.
Simon Glass May 22, 2014, 1:25 a.m. UTC | #2
Hi Stephen,

On 13 May 2014 09:30, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/09/2014 11:28 AM, Simon Glass wrote:
>> This is an implementation of GPIOs for Tegra that uses driver model. It has
>> been tested on trimslice and also using the new iotrace feature.
>>
>> The implementation uses a top-level GPIO device (which has no actual GPIOS).
>> Under this all the banks are created as separate GPIO devices.
>
> I still believe that there is a single Tegra GPIO controller, and this
> should be represented by a single device in the U-Boot device model.
>

It is possible that this is the best way to go, but it is far from
certain. I have just had a try at implementing this for exynos now
that the GPIO patches have been applied. I find a number of
independent GPIO banks which fits nicely into the driver model idea of
hierarchical devices. I'm on the fence with Tegra, but feel my
approach is reasonable for now.

If we go with the route you suggest, we'll have to deal with a single
device having multiple banks. That may be necessary/expedient at some
point but for now I would prefer to add complexity/features as they
are needed, and not before. Once we have a few more GPIO conversions
done it should be clearer. It is very early days with driver model and
we don't have a lot of information as to what is best. If we want to
change the way GPIO drivers are done in a few months, it is not a big
job and I will happily do it, I just don't want to build more
complexity than we need early on.

There is also no need for a single hardware device to be a single
U-Boot 'struct device'. In particular mfd devices have several
children with different functions. One of the founding principles of
U-Boot's driver model was to support child devices easily and in a
general way. IMO we might as well take advantage of it.

Regards,
Simon
diff mbox

Patch

diff --git a/arch/arm/include/asm/arch-tegra/gpio.h b/arch/arm/include/asm/arch-tegra/gpio.h
index d97190d..80e4a73 100644
--- a/arch/arm/include/asm/arch-tegra/gpio.h
+++ b/arch/arm/include/asm/arch-tegra/gpio.h
@@ -6,6 +6,8 @@ 
 #ifndef _TEGRA_GPIO_H_
 #define _TEGRA_GPIO_H_
 
+#define TEGRA_GPIOS_PER_PORT	8
+#define TEGRA_PORTS_PER_BANK	4
 #define MAX_NUM_GPIOS           (TEGRA_GPIO_PORTS * TEGRA_GPIO_BANKS * 8)
 #define GPIO_NAME_SIZE		20	/* gpio_request max label len */
 
@@ -14,11 +16,13 @@ 
 #define GPIO_FULLPORT(x)	((x) >> 3)
 #define GPIO_BIT(x)		((x) & 0x7)
 
-/*
- * Tegra-specific GPIO API
+/**
+ * tegra_spl_gpio_direction_output() - set the output value of a GPIO
+ *
+ * This function is only used from SPL on seaboard, which needs to enable a
+ * GPIO to get the UART running. It could be done in U-Boot rather than SPL,
+ * but for now, this gets it working
  */
+int tegra_spl_gpio_direction_output(int gpio, int value);
 
-void gpio_info(void);
-
-#define gpio_status()	gpio_info()
 #endif	/* TEGRA_GPIO_H_ */
diff --git a/board/nvidia/seaboard/seaboard.c b/board/nvidia/seaboard/seaboard.c
index ef4e481..e27efcd 100644
--- a/board/nvidia/seaboard/seaboard.c
+++ b/board/nvidia/seaboard/seaboard.c
@@ -22,7 +22,7 @@  void gpio_early_init_uart(void)
 #ifndef CONFIG_SPL_BUILD
 	gpio_request(GPIO_PI3, NULL);
 #endif
-	gpio_direction_output(GPIO_PI3, 0);
+	tegra_spl_gpio_direction_output(GPIO_PI3, 0);
 }
 #endif
 
diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
index 82b30d5..f03d4ed 100644
--- a/drivers/gpio/tegra_gpio.c
+++ b/drivers/gpio/tegra_gpio.c
@@ -12,10 +12,17 @@ 
  */
 
 #include <common.h>
+#include <dm.h>
+#include <malloc.h>
+#include <errno.h>
+#include <fdtdec.h>
 #include <asm/io.h>
 #include <asm/bitops.h>
 #include <asm/arch/tegra.h>
 #include <asm/gpio.h>
+#include <dm/device-internal.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 enum {
 	TEGRA_CMD_INFO,
@@ -24,14 +31,18 @@  enum {
 	TEGRA_CMD_INPUT,
 };
 
-static struct gpio_names {
-	char name[GPIO_NAME_SIZE];
-} gpio_names[MAX_NUM_GPIOS];
+struct tegra_gpio_platdata {
+	struct gpio_ctlr_bank *bank;
+	const char *port_name;	/* Name of port, e.g. "B" */
+	int base_port;		/* Port number for this port (0, 1,.., n-1) */
+};
 
-static char *get_name(int i)
-{
-	return *gpio_names[i].name ? gpio_names[i].name : "UNKNOWN";
-}
+/* Information about each port at run-time */
+struct tegra_port_info {
+	char label[TEGRA_GPIOS_PER_PORT][GPIO_NAME_SIZE];
+	struct gpio_ctlr_bank *bank;
+	int base_port;		/* Port number for this port (0, 1,.., n-1) */
+};
 
 /* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */
 static int get_config(unsigned gpio)
@@ -121,38 +132,71 @@  static void set_level(unsigned gpio, int high)
 	writel(u, &bank->gpio_out[GPIO_PORT(gpio)]);
 }
 
+static int check_reserved(struct device *dev, unsigned offset,
+			  const char *func)
+{
+	struct tegra_port_info *state = dev_get_priv(dev);
+	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+
+	if (!*state->label[offset]) {
+		printf("tegra_gpio: %s: error: gpio %s%d not reserved\n",
+		       func, uc_priv->bank_name, offset);
+		return -EPERM;
+	}
+
+	return 0;
+}
+
+#ifdef CONFIG_SPL
+/* set GPIO pin 'gpio' as an output, with polarity 'value' */
+int tegra_spl_gpio_direction_output(int gpio, int value)
+{
+	/* Configure GPIO output value. */
+	set_level(gpio, value);
+
+	/* Configure GPIO direction as output. */
+	set_direction(gpio, value);
+
+	return 0;
+}
+#endif /* CONFIG_SPL */
+
 /*
  * Generic_GPIO primitives.
  */
 
-int gpio_request(unsigned gpio, const char *label)
+static int tegra_gpio_request(struct device *dev, unsigned offset,
+			      const char *label)
 {
-	if (gpio >= MAX_NUM_GPIOS)
-		return -1;
+	struct tegra_port_info *state = dev_get_priv(dev);
 
-	if (label != NULL) {
-		strncpy(gpio_names[gpio].name, label, GPIO_NAME_SIZE);
-		gpio_names[gpio].name[GPIO_NAME_SIZE - 1] = '\0';
-	}
+	if (*state->label[offset])
+		return -EBUSY;
+
+	strncpy(state->label[offset], label, GPIO_NAME_SIZE);
+	state->label[offset][GPIO_NAME_SIZE - 1] = '\0';
 
 	/* Configure as a GPIO */
-	set_config(gpio, 1);
+	set_config(state->base_port + offset, 1);
 
 	return 0;
 }
 
-int gpio_free(unsigned gpio)
+static int tegra_gpio_free(struct device *dev, unsigned offset)
 {
-	if (gpio >= MAX_NUM_GPIOS)
-		return -1;
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int ret;
+
+	ret = check_reserved(dev, offset, __func__);
+	if (ret)
+		return ret;
+	state->label[offset][0] = '\0';
 
-	gpio_names[gpio].name[0] = '\0';
-	/* Do not configure as input or change pin mux here */
 	return 0;
 }
 
 /* read GPIO OUT value of pin 'gpio' */
-static int gpio_get_output_value(unsigned gpio)
+static int tegra_gpio_get_output_value(unsigned gpio)
 {
 	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
 	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
@@ -166,81 +210,234 @@  static int gpio_get_output_value(unsigned gpio)
 	return (val >> GPIO_BIT(gpio)) & 1;
 }
 
+
 /* set GPIO pin 'gpio' as an input */
-int gpio_direction_input(unsigned gpio)
+static int tegra_gpio_direction_input(struct device *dev, unsigned offset)
 {
-	debug("gpio_direction_input: pin = %d (port %d:bit %d)\n",
-		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int ret;
+
+	ret = check_reserved(dev, offset, __func__);
+	if (ret)
+		return ret;
 
 	/* Configure GPIO direction as input. */
-	set_direction(gpio, 0);
+	set_direction(state->base_port + offset, 0);
 
 	return 0;
 }
 
 /* set GPIO pin 'gpio' as an output, with polarity 'value' */
-int gpio_direction_output(unsigned gpio, int value)
+static int tegra_gpio_direction_output(struct device *dev, unsigned offset,
+				       int value)
 {
-	debug("gpio_direction_output: pin = %d (port %d:bit %d) = %s\n",
-		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio),
-		value ? "HIGH" : "LOW");
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int ret;
+
+	ret = check_reserved(dev, offset, __func__);
+	if (ret)
+		return ret;
 
 	/* Configure GPIO output value. */
-	set_level(gpio, value);
+	set_level(state->base_port + offset, value);
 
 	/* Configure GPIO direction as output. */
-	set_direction(gpio, 1);
+	set_direction(state->base_port + offset, 1);
 
 	return 0;
 }
 
 /* read GPIO IN value of pin 'gpio' */
-int gpio_get_value(unsigned gpio)
+static int tegra_gpio_get_value(struct device *dev, unsigned offset)
 {
-	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int gpio = state->base_port + offset;
+	int ret;
 	int val;
 
-	debug("gpio_get_value: pin = %d (port %d:bit %d)\n",
-		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
+	ret = check_reserved(dev, offset, __func__);
+	if (ret)
+		return ret;
+
+	debug("%s: pin = %d (port %d:bit %d)\n", __func__,
+	      gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
 
-	val = readl(&bank->gpio_in[GPIO_PORT(gpio)]);
+	val = readl(&state->bank->gpio_in[GPIO_PORT(gpio)]);
 
 	return (val >> GPIO_BIT(gpio)) & 1;
 }
 
 /* write GPIO OUT value to pin 'gpio' */
-int gpio_set_value(unsigned gpio, int value)
+static int tegra_gpio_set_value(struct device *dev, unsigned offset, int value)
 {
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int gpio = state->base_port + offset;
+	int ret;
+
+	ret = check_reserved(dev, offset, __func__);
+	if (ret)
+		return ret;
+
 	debug("gpio_set_value: pin = %d (port %d:bit %d), value = %d\n",
-		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio), value);
+	      gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio), value);
 
 	/* Configure GPIO output value. */
-	set_level(gpio, value);
+	set_level(state->base_port + offset, value);
 
 	return 0;
 }
 
-/*
- * Display Tegra GPIO information
+static int tegra_gpio_get_state(struct device *dev, unsigned int offset,
+				char *buf, int bufsize)
+{
+	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+	struct tegra_port_info *state = dev_get_priv(dev);
+	int gpio = state->base_port + offset;
+	const char *label;
+	int is_output;
+	int is_gpio;
+	int size;
+
+	label = state->label[offset];
+	is_gpio = get_config(state->base_port + offset); /* GPIO, not SFPIO */
+	size = snprintf(buf, bufsize, "%s%d: ",
+			uc_priv->bank_name ? uc_priv->bank_name : "", offset);
+	buf += size;
+	bufsize -= size;
+	if (is_gpio) {
+		is_output = get_direction(gpio);
+
+		snprintf(buf, bufsize, "%s: %d [%c]%s%s",
+			is_output ? "out" : " in",
+			is_output ?
+				tegra_gpio_get_output_value(gpio) :
+				tegra_gpio_get_value(dev, offset),
+			*label ? 'x' : ' ',
+			*label ? " " : "",
+			label);
+	} else {
+		snprintf(buf, bufsize, "sfpio");
+	}
+
+	return 0;
+}
+
+static const struct dm_gpio_ops gpio_tegra_ops = {
+	.request		= tegra_gpio_request,
+	.free			= tegra_gpio_free,
+	.direction_input	= tegra_gpio_direction_input,
+	.direction_output	= tegra_gpio_direction_output,
+	.get_value		= tegra_gpio_get_value,
+	.set_value		= tegra_gpio_set_value,
+	.get_state		= tegra_gpio_get_state,
+};
+
+/**
+ * Returns the name of a GPIO port
+ *
+ * GPIOs are named A, B, C, ..., Z, AA, BB, CC, ...
+ *
+ * @base_port: Base port number (0, 1..n-1)
+ * @return allocated string containing the name
  */
-void gpio_info(void)
+static char *gpio_port_name(int base_port)
 {
-	unsigned c;
-	int type;
+	char *name, *s;
+
+	name = malloc(3);
+	if (name) {
+		s = name;
+		*s++ = 'A' + (base_port % 26);
+		if (base_port >= 26)
+			*s++ = *name;
+		*s = '\0';
+	}
+
+	return name;
+}
+
+static const struct device_id tegra_gpio_ids[] = {
+	{ .compatible = "nvidia,tegra30-gpio" },
+	{ .compatible = "nvidia,tegra20-gpio" },
+	{ }
+};
+
+static int gpio_tegra_probe(struct device *dev)
+{
+	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+	struct tegra_port_info *priv = dev->priv;
+	struct tegra_gpio_platdata *plat = dev->platdata;
 
-	for (c = 0; c < MAX_NUM_GPIOS; c++) {
-		type = get_config(c);		/* GPIO, not SFPIO */
-		if (type) {
-			printf("GPIO_%d:\t%s is an %s, ", c,
-				get_name(c),
-				get_direction(c) ? "OUTPUT" : "INPUT");
-			if (get_direction(c))
-				printf("value = %d", gpio_get_output_value(c));
-			else
-				printf("value = %d", gpio_get_value(c));
-			printf("\n");
-		} else
-			continue;
+	/* Only child devices have ports */
+	if (!plat)
+		return 0;
+
+	priv->bank = plat->bank;
+	priv->base_port = plat->base_port;
+
+	uc_priv->gpio_count = TEGRA_GPIOS_PER_PORT;
+	uc_priv->bank_name = plat->port_name;
+
+	return 0;
+}
+
+/**
+ * We have a top-level GPIO device with no actual GPIOs. It has a child
+ * device for each Tegra port.
+ */
+static int gpio_tegra_bind(struct device *parent)
+{
+	struct tegra_gpio_platdata *plat = parent->platdata;
+	struct gpio_ctlr *ctlr;
+	int bank_count;
+	int bank;
+	int ret;
+	int len;
+
+	/* If this is a child device, there is nothing to do here */
+	if (plat)
+		return 0;
+
+	/*
+	 * This driver does not make use of interrupts, other than to figure
+	 * out the number of GPIO banks
+	 */
+	if (!fdt_getprop(gd->fdt_blob, parent->of_offset, "interrupts", &len))
+		return -EINVAL;
+	bank_count = len / 3 / sizeof(u32);
+	ctlr = (struct gpio_ctlr *)fdtdec_get_addr(gd->fdt_blob,
+						   parent->of_offset, "reg");
+	for (bank = 0; bank < bank_count; bank++) {
+		int port;
+
+		for (port = 0; port < TEGRA_PORTS_PER_BANK; port++) {
+			struct tegra_gpio_platdata *plat;
+			struct device *dev;
+
+			plat = calloc(1, sizeof(*plat));
+			if (!plat)
+				return -ENOMEM;
+			plat->bank = &ctlr->gpio_bank[bank];
+			plat->base_port = bank * TEGRA_PORTS_PER_BANK + port;
+			plat->port_name = gpio_port_name(plat->base_port);
+
+			ret = device_bind(parent, parent->driver,
+					  plat->port_name, plat, -1, &dev);
+			if (ret)
+				return ret;
+			dev->of_offset = parent->of_offset;
+		}
 	}
+
+	return 0;
 }
+
+U_BOOT_DRIVER(gpio_tegra) = {
+	.name	= "gpio_tegra",
+	.id	= UCLASS_GPIO,
+	.of_match = tegra_gpio_ids,
+	.bind	= gpio_tegra_bind,
+	.probe = gpio_tegra_probe,
+	.priv_auto_alloc_size = sizeof(struct tegra_port_info),
+	.ops	= &gpio_tegra_ops,
+};
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index 24b909b..1069218 100644
--- a/include/configs/tegra-common.h
+++ b/include/configs/tegra-common.h
@@ -21,6 +21,7 @@ 
 
 #define CONFIG_DM
 #define CONFIG_CMD_DM
+#define CONFIG_DM_GPIO
 
 #define CONFIG_SYS_TIMER_RATE		1000000
 #define CONFIG_SYS_TIMER_COUNTER	NV_PA_TMRUS_BASE