diff mbox

[U-Boot] WIP: tegra: Use driver model for GPIO driver

Message ID 1399063905-31123-1-git-send-email-sjg@chromium.org
State RFC
Headers show

Commit Message

Simon Glass May 2, 2014, 8:51 p.m. UTC 
This is an implementation of GPIOs for Tegra that uses driver model.
It is written for comment and need work and testing before it is ready
to use.

Specific points for discussion:

1. I can't find much in the way of GPIO device tree bindings, so ended up
just creating the GPIO devices

3. Driver model understand the concept of a bank of GPIOs, but this is
equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think
about this.

3. Since we need a separate device for each named bank, there is a top-level
GPIO device type which creates the GPIO banks. The GPIO bank is the actual
device.

4. There are a few minor changes in the driver model framework to make
things easier. I'll turn these into patches in the end.

Please take a look and let me know what you think.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/arm/dts/tegra20.dtsi              | 142 ++++++++++++
 arch/arm/include/asm/arch-tegra/gpio.h |  14 +-
 board/nvidia/seaboard/seaboard.c       |   2 +-
 drivers/core/lists.c                   |   1 +
 drivers/core/root.c                    |   7 +-
 drivers/core/uclass.c                  |   2 +-
 drivers/gpio/tegra_gpio.c              | 390 ++++++++++++++++++++++++---------
 include/configs/tegra-common.h         |   3 +
 include/dm/device-internal.h           |   4 +
 test/dm/Makefile                       |   2 +
 10 files changed, 453 insertions(+), 114 deletions(-)

Comments

Stephen Warren May 2, 2014, 10:43 p.m. UTC | #1 
On 05/02/2014 02:51 PM, Simon Glass wrote:
> This is an implementation of GPIOs for Tegra that uses driver model.
> It is written for comment and need work and testing before it is ready
> to use.
> 
> Specific points for discussion:
> 
> 1. I can't find much in the way of GPIO device tree bindings, so ended up
> just creating the GPIO devices

The binding is already defined in the Linux kernel at:

Documentation/devicetree/bindings/gpio/gpio.txt
Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt

An example is in:

arch/arm/boot/dts/tegra20.dtsi


> 3. Driver model understand the concept of a bank of GPIOs, but this is
> equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think
> about this.

There's no need at all to expose the banks separately. This is purely an
implementation detail of the internal register layout of the HW, and not
something that anyone outside the GPIO driver need concern itself with.
Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have
textual names derived from the banked register layout, but this has no
practical consequence, and need not be represented anywhere.

I would imagine this is true of any GPIO controller. Why does the driver
model know/care about GPIO banks?

> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> index 3805750..10065da 100644
> --- a/arch/arm/dts/tegra20.dtsi
> +++ b/arch/arm/dts/tegra20.dtsi
> @@ -143,6 +143,148 @@
>  		interrupts = < 64 65 66 67 87 119 121 >;
>  		#gpio-cells = <2>;
>  		gpio-controller;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		port@0 {
> +			reg = <0>;
> +			compatible = "nvidia,tegra20-gpio-bank";
> +			gpio-bank-name = "a";
> +		};

We definitely shouldn't add these port child nodes.
Simon Glass May 3, 2014, 4:25 a.m. UTC | #2 
Hi Stephen,

On 2 May 2014 16:43, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 05/02/2014 02:51 PM, Simon Glass wrote:
>> This is an implementation of GPIOs for Tegra that uses driver model.
>> It is written for comment and need work and testing before it is ready
>> to use.
>>
>> Specific points for discussion:
>>
>> 1. I can't find much in the way of GPIO device tree bindings, so ended up
>> just creating the GPIO devices
>
> The binding is already defined in the Linux kernel at:
>
> Documentation/devicetree/bindings/gpio/gpio.txt
> Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt
>
> An example is in:
>
> arch/arm/boot/dts/tegra20.dtsi

Yes but all the parameters are hard-coded in the driver, not in the
device tree. I ended up doing the same thing, as you probably noticed.

>
>
>> 3. Driver model understand the concept of a bank of GPIOs, but this is
>> equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think
>> about this.
>
> There's no need at all to expose the banks separately. This is purely an
> implementation detail of the internal register layout of the HW, and not
> something that anyone outside the GPIO driver need concern itself with.
> Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have
> textual names derived from the banked register layout, but this has no
> practical consequence, and need not be represented anywhere.
>
> I would imagine this is true of any GPIO controller. Why does the driver
> model know/care about GPIO banks?

For naming - A, B, C, etc. Each of these is considered a 'bank'. At
present each is a separate GPIO device, also. This will allow us to
support I2C extenders and other ways of adding GPIOs.

>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>> index 3805750..10065da 100644
>> --- a/arch/arm/dts/tegra20.dtsi
>> +++ b/arch/arm/dts/tegra20.dtsi
>> @@ -143,6 +143,148 @@
>>               interrupts = < 64 65 66 67 87 119 121 >;
>>               #gpio-cells = <2>;
>>               gpio-controller;
>> +             #address-cells = <1>;
>> +             #size-cells = <0>;
>> +             port@0 {
>> +                     reg = <0>;
>> +                     compatible = "nvidia,tegra20-gpio-bank";
>> +                     gpio-bank-name = "a";
>> +             };
>
> We definitely shouldn't add these port child nodes.

Yes, that was just something I was playing with - it's not used in the
driver as you probably saw.

I see that U-Boot's GPIO stuff is out of date for tegra, so I'll
update that. I'll see if I can remove the need for the second-level
bank devices also. Not quite sure what I'll end up with. Ideas
welcome.

Regards,
Simon
Stephen Warren May 5, 2014, 4:10 p.m. UTC | #3 
On 05/02/2014 10:25 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On 2 May 2014 16:43, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 05/02/2014 02:51 PM, Simon Glass wrote:
>>> This is an implementation of GPIOs for Tegra that uses driver model.
>>> It is written for comment and need work and testing before it is ready
>>> to use.
>>>
>>> Specific points for discussion:
>>>
>>> 1. I can't find much in the way of GPIO device tree bindings, so ended up
>>> just creating the GPIO devices
>>
>> The binding is already defined in the Linux kernel at:
>>
>> Documentation/devicetree/bindings/gpio/gpio.txt
>> Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt
>>
>> An example is in:
>>
>> arch/arm/boot/dts/tegra20.dtsi
> 
> Yes but all the parameters are hard-coded in the driver, not in the
> device tree. I ended up doing the same thing, as you probably noticed.

To be honest, since I saw all the DT binding changes and mentions of
"banks", I didn't look at the driver, since I assumed it'd have to be
re-written to solve those issues first. If the driver doesn't actually
use any of that stuff, then perhaps I should take another look at the patch.

>>> 3. Driver model understand the concept of a bank of GPIOs, but this is
>>> equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think
>>> about this.
>>
>> There's no need at all to expose the banks separately. This is purely an
>> implementation detail of the internal register layout of the HW, and not
>> something that anyone outside the GPIO driver need concern itself with.
>> Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have
>> textual names derived from the banked register layout, but this has no
>> practical consequence, and need not be represented anywhere.
>>
>> I would imagine this is true of any GPIO controller. Why does the driver
>> model know/care about GPIO banks?
> 
> For naming - A, B, C, etc. Each of these is considered a 'bank'. At
> present each is a separate GPIO device, also. This will allow us to
> support I2C extenders and other ways of adding GPIOs.

I don't think individual GPIO controllers have to be broken down into
banks in order for U-Boot to support multiple GPIO controllers. U-Boot
should know that multiple controller instances exist. U-Boot shouldn't
dictate the internal structure of each controller; if it wants to expose
1000 GPIOs in a flat naming space, that should be OK.

Or, when you mentioned "bank", did you mean "HW module" or "GPIO
expander chip" i.e. what the Linux kernel calls a struct gpio_chip,
rather than a "bank" being a sub-division of a whole HW module or GPIO
expander chip?
Simon Glass May 5, 2014, 4:32 p.m. UTC | #4 
Hi Stephen,

On 5 May 2014 10:10, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 05/02/2014 10:25 PM, Simon Glass wrote:
> > Hi Stephen,
> >
> > On 2 May 2014 16:43, Stephen Warren <swarren@wwwdotorg.org> wrote:
> >> On 05/02/2014 02:51 PM, Simon Glass wrote:
> >>> This is an implementation of GPIOs for Tegra that uses driver model.
> >>> It is written for comment and need work and testing before it is ready
> >>> to use.
> >>>
> >>> Specific points for discussion:
> >>>
> >>> 1. I can't find much in the way of GPIO device tree bindings, so ended up
> >>> just creating the GPIO devices
> >>
> >> The binding is already defined in the Linux kernel at:
> >>
> >> Documentation/devicetree/bindings/gpio/gpio.txt
> >> Documentation/devicetree/bindings/gpio/nvidia,tegra20-gpio.txt
> >>
> >> An example is in:
> >>
> >> arch/arm/boot/dts/tegra20.dtsi
> >
> > Yes but all the parameters are hard-coded in the driver, not in the
> > device tree. I ended up doing the same thing, as you probably noticed.
>
> To be honest, since I saw all the DT binding changes and mentions of
> "banks", I didn't look at the driver, since I assumed it'd have to be
> re-written to solve those issues first. If the driver doesn't actually
> use any of that stuff, then perhaps I should take another look at the patch.

Well...you could :-) I sent an updated and cleaned-up version that has
actually been tested.

>
>
> >>> 3. Driver model understand the concept of a bank of GPIOs, but this is
> >>> equivalent to 'port' in Tegra. So it is somewhat confusing. Need to think
> >>> about this.
> >>
> >> There's no need at all to expose the banks separately. This is purely an
> >> implementation detail of the internal register layout of the HW, and not
> >> something that anyone outside the GPIO driver need concern itself with.
> >> Tegra simply has N GPIOs numbered 0..n-1. Admittedly the GPIOs also have
> >> textual names derived from the banked register layout, but this has no
> >> practical consequence, and need not be represented anywhere.
> >>
> >> I would imagine this is true of any GPIO controller. Why does the driver
> >> model know/care about GPIO banks?
> >
> > For naming - A, B, C, etc. Each of these is considered a 'bank'. At
> > present each is a separate GPIO device, also. This will allow us to
> > support I2C extenders and other ways of adding GPIOs.
>
> I don't think individual GPIO controllers have to be broken down into
> banks in order for U-Boot to support multiple GPIO controllers. U-Boot
> should know that multiple controller instances exist. U-Boot shouldn't
> dictate the internal structure of each controller; if it wants to expose
> 1000 GPIOs in a flat naming space, that should be OK.
>
> Or, when you mentioned "bank", did you mean "HW module" or "GPIO
> expander chip" i.e. what the Linux kernel calls a struct gpio_chip,
> rather than a "bank" being a sub-division of a whole HW module or GPIO
> expander chip?

I suppose I mean a conceptual 'bank', such as is used to name GPIOs in
the datasheet. The current GPIO uclass implementation uses one device
per bank, which has the virtue of simplicity, and it's easy to use 'dm
tree' to see the available GPIO banks. But it remains to be seen
whether this fits best with actual SOCs.

Regards,
Simon
diff mbox

Patch

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index 3805750..10065da 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -143,6 +143,148 @@ 
 		interrupts = < 64 65 66 67 87 119 121 >;
 		#gpio-cells = <2>;
 		gpio-controller;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		port@0 {
+			reg = <0>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "a";
+		};
+		port@1 {
+			reg = <1>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "b";
+		};
+		port@2 {
+			reg = <2>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "c";
+		};
+		port@3 {
+			reg = <3>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "d";
+		};
+		port@4 {
+			reg = <4>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "e";
+		};
+		port@5 {
+			reg = <5>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "f";
+		};
+		port@6 {
+			reg = <6>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "g";
+		};
+		port@7 {
+			reg = <7>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "h";
+		};
+		port@8 {
+			reg = <8>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "i";
+		};
+		port@9 {
+			reg = <9>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "j";
+		};
+		port@10 {
+			reg = <10>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "k";
+		};
+		port@11 {
+			reg = <11>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "l";
+		};
+		port@12 {
+			reg = <12>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "m";
+		};
+		port@13 {
+			reg = <13>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "n";
+		};
+		port@14 {
+			reg = <14>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "o";
+		};
+		port@15 {
+			reg = <15>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "p";
+		};
+		port@16 {
+			reg = <16>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "q";
+		};
+		port@17 {
+			reg = <17>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "r";
+		};
+		port@18 {
+			reg = <18>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "s";
+		};
+		port@19 {
+			reg = <19>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "t";
+		};
+		port@20 {
+			reg = <20>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "u";
+		};
+		port@21 {
+			reg = <21>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "v";
+		};
+		port@22 {
+			reg = <22>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "w";
+		};
+		port@23 {
+			reg = <23>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "x";
+		};
+		port@24 {
+			reg = <24>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "y";
+		};
+		port@25 {
+			reg = <25>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "z";
+		};
+		port@26 {
+			reg = <26>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "aa";
+		};
+		port@27 {
+			reg = <27>;
+			compatible = "nvidia,tegra20-gpio-bank";
+			gpio-bank-name = "bb";
+		};
 	};
 
 	pinmux: pinmux@70000000 {
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/core/lists.c b/drivers/core/lists.c
index 4f2c126..6a53362 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -14,6 +14,7 @@ 
 #include <dm/platdata.h>
 #include <dm/uclass.h>
 #include <dm/util.h>
+#include <fdtdec.h>
 #include <linux/compiler.h>
 
 struct driver *lists_driver_lookup_name(const char *name)
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 407bc0d..4427b81 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -10,6 +10,7 @@ 
 #include <common.h>
 #include <errno.h>
 #include <malloc.h>
+#include <libfdt.h>
 #include <dm/device.h>
 #include <dm/device-internal.h>
 #include <dm/lists.h>
@@ -42,9 +43,9 @@  int dm_init(void)
 		dm_warn("Virtual root driver already exists!\n");
 		return -EINVAL;
 	}
-	INIT_LIST_HEAD(&gd->uclass_root);
+	INIT_LIST_HEAD(&DM_UCLASS_ROOT());
 
-	ret = device_bind_by_name(NULL, &root_info, &gd->dm_root);
+	ret = device_bind_by_name(NULL, &root_info, &DM_ROOT());
 	if (ret)
 		return ret;
 
@@ -55,7 +56,7 @@  int dm_scan_platdata(void)
 {
 	int ret;
 
-	ret = lists_bind_drivers(gd->dm_root);
+	ret = lists_bind_drivers(DM_ROOT());
 	if (ret == -ENOENT) {
 		dm_warn("Some drivers were not found\n");
 		ret = 0;
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index 4df5a8b..afb5fdc 100644
--- a/drivers/core/uclass.c
+++ b/drivers/core/uclass.c
@@ -75,7 +75,7 @@  static int uclass_add(enum uclass_id id, struct uclass **ucp)
 	uc->uc_drv = uc_drv;
 	INIT_LIST_HEAD(&uc->sibling_node);
 	INIT_LIST_HEAD(&uc->dev_head);
-	list_add(&uc->sibling_node, &gd->uclass_root);
+	list_add(&uc->sibling_node, &DM_UCLASS_ROOT());
 
 	if (uc_drv->init) {
 		ret = uc_drv->init(uc);
diff --git a/drivers/gpio/tegra_gpio.c b/drivers/gpio/tegra_gpio.c
index 82b30d5..a162bb0 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,25 +31,30 @@  enum {
 	TEGRA_CMD_INPUT,
 };
 
-static struct gpio_names {
-	char name[GPIO_NAME_SIZE];
-} gpio_names[MAX_NUM_GPIOS];
+struct tegra_port_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";
-}
+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) */
+};
+
+#define GPIO_NUM(state, offset) \
+	(((state)->base_port * TEGRA_GPIOS_PER_PORT) + (offset))
 
 /* Return config of pin 'gpio' as GPIO (1) or SFPIO (0) */
-static int get_config(unsigned gpio)
+static int get_config(struct tegra_port_info *state, int offset)
 {
-	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	int gpio = GPIO_NUM(state, offset);
 	u32 u;
 	int type;
 
-	u = readl(&bank->gpio_config[GPIO_PORT(gpio)]);
-	type =  (u >> GPIO_BIT(gpio)) & 1;
+	u = readl(&state->bank->gpio_config[GPIO_PORT(offset)]);
+	type =  (u >> GPIO_BIT(offset)) & 1;
 
 	debug("get_config: port = %d, bit = %d is %s\n",
 		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO");
@@ -51,33 +63,33 @@  static int get_config(unsigned gpio)
 }
 
 /* Config pin 'gpio' as GPIO or SFPIO, based on 'type' */
-static void set_config(unsigned gpio, int type)
+static void set_config(struct tegra_port_info *state, int offset, int type)
 {
-	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	int gpio = GPIO_NUM(state, offset);
 	u32 u;
 
 	debug("set_config: port = %d, bit = %d, %s\n",
-		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), type ? "GPIO" : "SFPIO");
+	      GPIO_FULLPORT(gpio),
+	      GPIO_BIT(gpio),
+	      type ? "GPIO" : "SFPIO");
 
-	u = readl(&bank->gpio_config[GPIO_PORT(gpio)]);
+	u = readl(&state->bank->gpio_config[GPIO_PORT(offset)]);
 	if (type)				/* GPIO */
-		u |= 1 << GPIO_BIT(gpio);
+		u |= 1 << GPIO_BIT(offset);
 	else
-		u &= ~(1 << GPIO_BIT(gpio));
-	writel(u, &bank->gpio_config[GPIO_PORT(gpio)]);
+		u &= ~(1 << GPIO_BIT(offset));
+	writel(u, &state->bank->gpio_config[GPIO_PORT(offset)]);
 }
 
 /* Return GPIO pin 'gpio' direction - 0 = input or 1 = output */
-static int get_direction(unsigned gpio)
+static int get_direction(struct tegra_port_info *state, int offset)
 {
-	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	int gpio = GPIO_NUM(state, offset);
 	u32 u;
 	int dir;
 
-	u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]);
-	dir =  (u >> GPIO_BIT(gpio)) & 1;
+	u = readl(&state->bank->gpio_dir_out[GPIO_PORT(offset)]);
+	dir =  (u >> GPIO_BIT(offset)) & 1;
 
 	debug("get_direction: port = %d, bit = %d, %s\n",
 		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), dir ? "OUT" : "IN");
@@ -86,161 +98,331 @@  static int get_direction(unsigned gpio)
 }
 
 /* Config GPIO pin 'gpio' as input or output (OE) as per 'output' */
-static void set_direction(unsigned gpio, int output)
+static void set_direction(struct tegra_port_info *state, int offset,
+			  int output)
 {
-	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	int gpio = GPIO_NUM(state, offset);
 	u32 u;
 
-	debug("set_direction: port = %d, bit = %d, %s\n",
-		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN");
+	debug("%s: port = %d, bit = %d, %s\n", __func__,
+	      GPIO_FULLPORT(gpio), GPIO_BIT(gpio), output ? "OUT" : "IN");
 
-	u = readl(&bank->gpio_dir_out[GPIO_PORT(gpio)]);
+	u = readl(&state->bank->gpio_dir_out[GPIO_PORT(offset)]);
 	if (output)
-		u |= 1 << GPIO_BIT(gpio);
+		u |= 1 << GPIO_BIT(offset);
 	else
-		u &= ~(1 << GPIO_BIT(gpio));
-	writel(u, &bank->gpio_dir_out[GPIO_PORT(gpio)]);
+		u &= ~(1 << GPIO_BIT(offset));
+	writel(u, &state->bank->gpio_dir_out[GPIO_PORT(offset)]);
 }
 
 /* set GPIO pin 'gpio' output bit as 0 or 1 as per 'high' */
-static void set_level(unsigned gpio, int high)
+static void set_level(struct tegra_port_info *state, int offset, int high)
 {
-	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
-	struct gpio_ctlr_bank *bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	int gpio = GPIO_NUM(state, offset);
 	u32 u;
 
-	debug("set_level: port = %d, bit %d == %d\n",
+	debug("%s: port = %d, bit %d == %d\n", __func__,
 		GPIO_FULLPORT(gpio), GPIO_BIT(gpio), high);
 
-	u = readl(&bank->gpio_out[GPIO_PORT(gpio)]);
+	u = readl(&state->bank->gpio_out[GPIO_PORT(offset)]);
 	if (high)
-		u |= 1 << GPIO_BIT(gpio);
+		u |= 1 << GPIO_BIT(offset);
 	else
-		u &= ~(1 << GPIO_BIT(gpio));
-	writel(u, &bank->gpio_out[GPIO_PORT(gpio)]);
+		u &= ~(1 << GPIO_BIT(offset));
+	writel(u, &state->bank->gpio_out[GPIO_PORT(offset)]);
 }
 
+/* read GPIO OUT value of pin 'gpio' */
+static int get_output_value(struct tegra_port_info *state, int offset)
+{
+	int gpio = GPIO_NUM(state, offset);
+	int val;
+
+	debug("gpio_get_output_value: pin = %d (port %d:bit %d)\n",
+		gpio, GPIO_FULLPORT(gpio), GPIO_BIT(gpio));
+
+	val = readl(&state->bank->gpio_out[GPIO_PORT(offset)]);
+
+	return (val >> GPIO_BIT(offset)) & 1;
+}
+
+#ifdef CONFIG_SPL
+/* set GPIO pin 'gpio' as an output, with polarity 'value' */
+int tegra_spl_gpio_direction_output(int gpio, int value)
+{
+	struct gpio_ctlr *ctlr = (struct gpio_ctlr *)NV_PA_GPIO_BASE;
+	struct tegra_port_info state;
+	int offset = GPIO_BIT(gpio);
+
+	state.bank = &ctlr->gpio_bank[GPIO_BANK(gpio)];
+	state.base_port = GPIO_FULLPORT(gpio);
+
+	/* Configure GPIO output value. */
+	set_level(&state, offset, value);
+
+	/* Configure GPIO direction as output. */
+	set_direction(&state, offset, value);
+
+	return 0;
+}
+#endif /* CONFIG_SPL */
+
 /*
  * Generic_GPIO primitives.
  */
 
-int gpio_request(unsigned gpio, const char *label)
+static int check_reserved(struct tegra_port_info *state, unsigned offset,
+			  const char *func)
 {
-	if (gpio >= MAX_NUM_GPIOS)
-		return -1;
-
-	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]) {
+		printf("tegra_gpio: %s: error: gpio %u not reserved\n",
+		       func, state->base_port + offset);
+		return -EPERM;
 	}
 
-	/* Configure as a GPIO */
-	set_config(gpio, 1);
-
 	return 0;
 }
 
-int gpio_free(unsigned gpio)
+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 (*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(state, offset, 1);
 
-	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_free(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)];
-	int val;
-
-	debug("gpio_get_output_value: 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;
 
-	val = readl(&bank->gpio_out[GPIO_PORT(gpio)]);
+	ret = check_reserved(state, offset, __func__);
+	if (ret)
+		return ret;
+	state->label[offset][0] = '\0';
 
-	return (val >> GPIO_BIT(gpio)) & 1;
+	return 0;
 }
 
 /* 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(state, offset, __func__);
+	if (ret)
+		return ret;
 
 	/* Configure GPIO direction as input. */
-	set_direction(gpio, 0);
+	set_direction(state, 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(state, offset, __func__);
+	if (ret)
+		return ret;
 
 	/* Configure GPIO output value. */
-	set_level(gpio, value);
+	set_level(state, offset, value);
 
 	/* Configure GPIO direction as output. */
-	set_direction(gpio, 1);
+	set_direction(state, 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 = GPIO_NUM(state, offset);
+	int ret;
 	int val;
 
-	debug("gpio_get_value: pin = %d (port %d:bit %d)\n",
+	ret = check_reserved(state, 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(offset)]);
 
-	return (val >> GPIO_BIT(gpio)) & 1;
+	return (val >> GPIO_BIT(offset)) & 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 = GPIO_NUM(state, offset);
+	int ret;
+
+	ret = check_reserved(state, 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);
 
 	/* Configure GPIO output value. */
-	set_level(gpio, value);
+	set_level(state, offset, value);
 
 	return 0;
 }
 
-/*
- * Display Tegra GPIO information
- */
-void gpio_info(void)
+static int tegra_gpio_get_state(struct device *dev, unsigned int offset,
+				char *buf, int bufsize)
 {
-	unsigned c;
-	int type;
+	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+	struct tegra_port_info *state = dev_get_priv(dev);
+	const char *label;
+	int is_output;
+	int is_gpio;
+	int size;
+
+	label = state->label[offset];
+	is_output = get_direction(state, offset);
+	is_gpio = get_config(state, 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) {
+		snprintf(buf, bufsize, "%s: %d [%c]%s%s",
+			is_output ? "out" : " in",
+			is_output ?
+				get_output_value(state, offset) :
+				tegra_gpio_get_value(dev, offset),
+			*label ? 'x' : ' ',
+			*label ? " " : "",
+			label);
+	} else {
+		snprintf(buf, bufsize, "sfpio");
+	}
+
+	return 0;
+}
 
-	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;
+static const struct dm_gpio_ops gpio_port_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,
+};
+
+static int gpio_port_tegra_ofdata_to_platdata(struct device *dev)
+{
+	struct gpio_dev_priv *uc_priv = dev->uclass_priv;
+	struct tegra_port_platdata *plat = dev->platdata;
+
+	uc_priv->gpio_count = TEGRA_GPIOS_PER_PORT;
+	uc_priv->bank_name = plat->port_name;
+
+	return 0;
+}
+
+static int gpio_port_tegra_bind(struct device *dev)
+{
+	struct tegra_port_platdata *plat = dev->platdata;
+	struct tegra_port_info *priv = dev->priv;
+
+	priv->bank = plat->bank;
+	priv->base_port = plat->base_port;
+
+	return 0;
+}
+
+static struct driver gpio_port_tegra = {
+	.name	= "gpio_port_tegra",
+	.id	= UCLASS_GPIO,
+	.priv_auto_alloc_size = sizeof(struct tegra_port_info),
+	.ops	= &gpio_port_tegra_ops,
+	.bind	= gpio_port_tegra_bind,
+	.ofdata_to_platdata = gpio_port_tegra_ofdata_to_platdata,
+};
+
+static char *gpio_port_name(int base_port)
+{
+	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 int gpio_tegra_bind(struct device *parent)
+{
+	struct gpio_ctlr *ctlr;
+	int bank_count;
+	int bank;
+	int ret;
+
+	bank_count = fdtdec_get_int(gd->fdt_blob, parent->of_offset,
+				    "bank-count", 0);
+	ctlr = (struct gpio_ctlr *)fdtdec_get_addr(gd->fdt_blob,
+						   parent->of_offset, "reg");
+	for (bank = 0; bank < bank_count; bank++) {
+		struct tegra_port_platdata plat;
+		struct device *dev;
+		int port;
+
+		plat.bank = &ctlr->gpio_bank[bank];
+		plat.base_port = bank * TEGRA_PORTS_PER_BANK;
+		for (port = 0; port < TEGRA_PORTS_PER_BANK; port++) {
+			/* GPIO name A, B, C, ..., Z, AA, BB, CC, ... */
+			plat.port_name = gpio_port_name(plat.base_port);
+
+			ret = device_bind(parent, &gpio_port_tegra, "bank",
+					  &plat, -1, &dev);
+			if (ret)
+				return ret;
+			plat.base_port++;
+		}
+	}
+
+	return 0;
+}
+
+static const struct device_id tegra_gpio_ids[] = {
+	{ .compatible = "nvidia,tegra20-gpio" },
+	{ .compatible = "nvidia,tegra30-gpio" },
+	{ .compatible = "nvidia,tegra114-gpio" },
+	{ .compatible = "nvidia,tegra124-gpio" },
+	{ }
+};
+
+U_BOOT_DRIVER(gpio_tegra) = {
+	.name	= "gpio_tegra",
+	.id	= UCLASS_GPIO,
+	.of_match = tegra_gpio_ids,
+	.bind	= gpio_tegra_bind,
+};
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index ae786cf..3af7fb8 100644
--- a/include/configs/tegra-common.h
+++ b/include/configs/tegra-common.h
@@ -19,6 +19,9 @@ 
 
 #include <asm/arch/tegra.h>		/* get chip and board defs */
 
+#define CONFIG_DM
+#define CONFIG_DM_GPIO
+
 #define CONFIG_SYS_TIMER_RATE		1000000
 #define CONFIG_SYS_TIMER_COUNTER	NV_PA_TMRUS_BASE
 
diff --git a/include/dm/device-internal.h b/include/dm/device-internal.h
index c026e8e..e95b7a9 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -84,4 +84,8 @@  int device_remove(struct device *dev);
  */
 int device_unbind(struct device *dev);
 
+/* Cast away any volatile pointer */
+#define DM_ROOT()		(((gd_t *)gd)->dm_root)
+#define DM_UCLASS_ROOT()		(((gd_t *)gd)->uclass_root)
+
 #endif
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 4e9afe6..b93f843 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -15,4 +15,6 @@  obj-$(CONFIG_DM_TEST) += ut.o
 # subsystem you must add sandbox tests here.
 obj-$(CONFIG_DM_TEST) += core.o
 obj-$(CONFIG_DM_TEST) += ut.o
+ifneq ($(CONFIG_DM_TEST),)
 obj-$(CONFIG_DM_GPIO) += gpio.o
+endif