Enable driver model for GPIOs on Tegra

Now that driver model is part of U-Boot, the task of converting drivers over to it begins. GPIO is one of the easiest to convert, since it already has a sandbox driver and a uclass driver.

The Tegra GPIO driver is relatively simple since it has a linear numbering and already uses the generic GPIO framework.

Along the way some minor deficiencies were found with driver model - these are corrected in this series.

Also it was difficult to exhaustively test the new driver against the old, so a new 'iotrace' framework was created to make this easier for future driver authors.

This series has been tested on Trimslice (Tegra 20). I will try it on a beaver also.

Changes in v3:
- Remove use of bool in header file to avoid exynos5 build failure
- Create a symlink for each arch to dt-bindings
- Bring in GPIO bindings for tegra{30,114,124} also
- Fix typo in commit subject
- Enable dm command in this patch instead of the next
- Move dm command enable to previous patch
- Use gpio number for the internal helper functions

Changes in v2:
- Add a new patch for an I/O tracing feature
- Add a new patch to enable iotrace for arm
- Add a new patch to enable iotrace for sandbox
- Add new patch to support include files for .dts files
- Add new patch to bring in Tegra device tree files from linux
- Update README to encourage conversion to driver model
- Add new patch to use case-insensitive comparison for GPIO banks
- Add new patch to add missing header files in lists and root
- Add new patch to deal with const-ness of the global_data pointer
- Add new patch to allow driver model tests only for sandbox
- Add new patch to fix printf() strings in the 'dm' command
- Split out a separate patch to enable driver model for tegra
- Split out driver model changes into separate patches
- Correct bugs found during testing

Simon Glass (13):
Add an I/O tracing feature
arm: Support iotrace feature
sandbox: Support iotrace feature
Makefile: Support include files for .dts files
tegra: dts: Bring in GPIO bindings from linux
dm: Update README to encourage conversion to driver model
dm: Use case-insensitive comparison for GPIO banks
dm: Add missing header files in lists and root
dm: Cast away the const-ness of the global_data pointer
dm: Allow driver model tests only for sandbox
dm: Fix printf() strings in the 'dm' command
tegra: Enable driver model
tegra: Convert tegra GPIO driver to use driver model

 README                                             |  28 ++
 arch/arm/dts/include/dt-bindings                   |   1 +
 arch/arm/dts/tegra114.dtsi                         |  21 +-
 arch/arm/dts/tegra124.dtsi                         |  23 +-
 arch/arm/dts/tegra20.dtsi                          |  15 +-
 arch/arm/dts/tegra30.dtsi                          |  21 +-
 arch/arm/include/asm/arch-tegra/gpio.h             |  14 +-
 arch/arm/include/asm/io.h                          |   3 +
 arch/microblaze/dts/include/dt-bindings            |   1 +
 arch/sandbox/dts/include/dt-bindings               |   1 +
 arch/sandbox/include/asm/io.h                      |  10 +
 arch/x86/dts/include/dt-bindings                   |   1 +
 board/nvidia/seaboard/seaboard.c                   |   2 +-
 common/Makefile                                    |   2 +
 common/cmd_iotrace.c                               |  73 +++++
 common/iotrace.c                                   | 169 +++++++++++
 drivers/core/lists.c                               |   1 +
 drivers/core/root.c                                |   7 +-
 drivers/core/uclass.c                              |   2 +-
 drivers/gpio/gpio-uclass.c                         |   2 +-
 drivers/gpio/tegra_gpio.c                          | 313 +++++++++++++++++----
 include/configs/sandbox.h                          |   3 +
 include/configs/tegra-common.h                     |   4 +
 include/dm/device-internal.h                       |   4 +
 include/dt-bindings/gpio/gpio.h                    |  15 +
 include/dt-bindings/gpio/tegra-gpio.h              |  51 ++++
 include/dt-bindings/interrupt-controller/arm-gic.h |  22 ++
 include/dt-bindings/interrupt-controller/irq.h     |  19 ++
 include/iotrace.h                                  | 104 +++++++
 scripts/Makefile.lib                               |   1 +
 test/dm/Makefile                                   |   2 +
 test/dm/cmd_dm.c                                   |  19 +-
 32 files changed, 849 insertions(+), 105 deletions(-)
 create mode 120000 arch/arm/dts/include/dt-bindings
 create mode 120000 arch/microblaze/dts/include/dt-bindings
 create mode 120000 arch/sandbox/dts/include/dt-bindings
 create mode 120000 arch/x86/dts/include/dt-bindings
 create mode 100644 common/cmd_iotrace.c
 create mode 100644 common/iotrace.c
 create mode 100644 include/dt-bindings/gpio/gpio.h
 create mode 100644 include/dt-bindings/gpio/tegra-gpio.h
 create mode 100644 include/dt-bindings/interrupt-controller/arm-gic.h
 create mode 100644 include/dt-bindings/interrupt-controller/irq.h
 create mode 100644 include/iotrace.h

1.9.1.423.g4596e3a

When debugging drivers it is useful to see what I/O accesses were done and in what order.

Even if the individual accesses are of little interest it can be useful to verify that the access pattern is consistent each time an operation is performed. In this case a checksum can be used to characterise the operation of a driver. The checksum can be compared across different runs of the operation to verify that the driver is working properly.

In particular, when performing major refactoring of the driver, where the access pattern should not change, the checksum provides assurance that the refactoring work has not broken the driver.

Add an I/O tracing feature and associated commands to provide this facility. It works by sneaking into the io.h heder for an architecture and redirecting I/O accesses through its tracing mechanism.

For now no commands are provided to examine the trace buffer. The format is fairly simple, so 'md' is a reasonable substitute.

Note: The checksum feature is only useful for I/O regions where the contents do not change outside of software control. Where this is not suitable you can fall back to manually comparing the addresses. It might be useful to enhance tracing to only checksum the accesses and not the data read/written.

---

Changes in v3:
- Remove use of bool in header file to avoid exynos5 build failure

Changes in v2:
- Add a new patch for an I/O tracing feature

 README               |  23 +++++++
 common/Makefile      |   2 +
 common/cmd_iotrace.c |  73 ++++++++++++++++++++++
 common/iotrace.c     | 169 +++++++++++++++++++++++++++++++++++++++++++++++++++
 include/iotrace.h    | 104 +++++++++++++++++++++++++++++++
 5 files changed, 371 insertions(+)
 create mode 100644 common/cmd_iotrace.c
 create mode 100644 common/iotrace.c
 create mode 100644 include/iotrace.h

index b973344..b226458 100644
@@ -987,6 +987,7 @@ The following options need to be configured:
 		CONFIG_CMD_IMLS		  List all images found in NOR flash
 		CONFIG_CMD_IMLS_NAND	* List all images found in NAND flash
 		CONFIG_CMD_IMMAP	* IMMR dump support
+		CONFIG_CMD_IOTRACE	* I/O tracing for debugging
 		CONFIG_CMD_IMPORTENV	* import an environment
 		CONFIG_CMD_INI		* import data from an ini file into the env
 		CONFIG_CMD_IRQ		* irqinfo

@@ -1158,6 +1159,28 @@ The following options need to be configured:
 		Note that if the GPIO device uses I2C, then the I2C interface
 		must also be configured. See I2C Support, below.
 
+- I/O tracing:
+		When CONFIG_IO_TRACE is selected, U-Boot intercepts all I/O
+		accesses and can checksum them or write a list of them out
+		to memory. See the 'iotrace' command for details. This is
+		useful for testing device drivers since it can confirm that
+		the driver behaves the same way before and after a code
+		change. Currently this is supported on sandbox and arm. To
+		add support for your architecture, add '#include <iotrace.h>'
+		to the bottom of arch/<arch>/include/asm/io.h and test.
+
+		Example output from the 'iotrace stats' command is below.
+		Note that if the trace buffer is exhausted, the checksum will
+		still continue to operate.
+
+			iotrace is enabled
+			Start:  10000000	(buffer start address)
+			Size:   00010000	(buffer size)
+			Offset: 00000120	(current buffer offset)
+			Output: 10000120	(start + offset)
+			Count:  00000018	(number of trace records)
+			CRC32:  9526fb66	(CRC32 of all trace records)
+
 - Timestamp Support:
 
 		When CONFIG_TIMESTAMP is selected, the timestamp

index 7c853ae..0dccb90 100644
@@ -96,6 +96,7 @@ obj-$(CONFIG_CMD_FUSE) += cmd_fuse.o
 obj-$(CONFIG_CMD_GETTIME) += cmd_gettime.o
 obj-$(CONFIG_CMD_GPIO) += cmd_gpio.o
 obj-$(CONFIG_CMD_I2C) += cmd_i2c.o
+obj-$(CONFIG_CMD_IOTRACE) += cmd_iotrace.o
 obj-$(CONFIG_CMD_HASH) += cmd_hash.o
 obj-$(CONFIG_CMD_IDE) += cmd_ide.o
 obj-$(CONFIG_CMD_IMMAP) += cmd_immap.o

@@ -240,6 +241,7 @@ obj-y += image.o
 obj-$(CONFIG_OF_LIBFDT) += image-fdt.o
 obj-$(CONFIG_FIT) += image-fit.o
 obj-$(CONFIG_FIT_SIGNATURE) += image-sig.o
+obj-$(CONFIG_IO_TRACE) += iotrace.o
 obj-y += memsize.o
 obj-y += stdio.o
 

1.9.1.423.g4596e3a

Support the iotrace feature for ARM, when enabled.

---

Changes in v3: None
Changes in v2:
- Add a new patch to enable iotrace for arm

 arch/arm/include/asm/io.h | 3 +++
 1 file changed, 3 insertions(+)

index 6a1f05a..9f35fd6 100644
@@ -437,4 +437,7 @@ out:
 
 #endif	/* __mem_isa */
 #endif	/* __KERNEL__ */
+
+#include <iotrace.h>
+
 #endif	/* __ASM_ARM_IO_H */

1.9.1.423.g4596e3a

Support the iotrace feature for sandbox, and enable it, using some dummy I/O access methods.

---

Changes in v3: None
Changes in v2:
- Add a new patch to enable iotrace for sandbox

 arch/sandbox/include/asm/io.h | 10 ++++++++++
 include/configs/sandbox.h     |  3 +++
 2 files changed, 13 insertions(+)

index 7956041..895fcb8 100644
@@ -40,4 +40,14 @@ static inline void unmap_sysmem(const void *vaddr)
 /* Map from a pointer to our RAM buffer */
 phys_addr_t map_to_sysmem(const void *ptr);
 
+/* Define nops for sandbox I/O access */
+#define readb(addr) 0
+#define readw(addr) 0
+#define readl(addr) 0
+#define writeb(v, addr)
+#define writew(v, addr)
+#define writel(v, addr)
+
+#include <iotrace.h>
+
 #endif

index fa62cb6..8ca2a55 100644
@@ -16,6 +16,9 @@
 
 #endif
 
+#define CONFIG_IO_TRACE
+#define CONFIG_CMD_IO_TRACE
+
 #define CONFIG_SYS_TIMER_RATE		1000000
 
 #define CONFIG_BOOTSTAGE

1.9.1.423.g4596e3a

Dear Simon Glass,

<snipped>

Hi Rommel,

Thanks - I didn't actually see your email - it's a good idea to keep the author in cc.

Regards,
Simon

Linux supports this, and if we are to have compatible device tree files, U-Boot should also.

Avoid giving the device tree files access to U-Boot's include/ directory. Only include/dt-bindings is accessible.

---

Changes in v3:
- Create a symlink for each arch to dt-bindings

Changes in v2:
- Add new patch to support include files for .dts files

 arch/arm/dts/include/dt-bindings        | 1 +
 arch/microblaze/dts/include/dt-bindings | 1 +
 arch/sandbox/dts/include/dt-bindings    | 1 +
 arch/x86/dts/include/dt-bindings        | 1 +
 scripts/Makefile.lib                    | 1 +
 5 files changed, 5 insertions(+)
 create mode 120000 arch/arm/dts/include/dt-bindings
 create mode 120000 arch/microblaze/dts/include/dt-bindings
 create mode 120000 arch/sandbox/dts/include/dt-bindings
 create mode 120000 arch/x86/dts/include/dt-bindings

new file mode 120000
index 0000000..0cecb3d
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file

new file mode 120000
index 0000000..0cecb3d
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file

new file mode 120000
index 0000000..0cecb3d
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file

new file mode 120000
index 0000000..0cecb3d
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file

index a04439d..722afed 100644
@@ -153,6 +153,7 @@ ld_flags       = $(LDFLAGS) $(ldflags-y)
 # Modified for U-Boot
 dtc_cpp_flags  = -Wp,-MD,$(depfile).pre.tmp -nostdinc                    \
 		 -I$(srctree)/arch/$(ARCH)/dts                           \
+		 -I$(srctree)/arch/$(ARCH)/dts/include                   \
 		 -undef -D__DTS__
 
 # Finds the multi-part object the current object will be linked into

1.9.1.423.g4596e3a

Hi Simon,

Thanks!

Best Regards
Masahiro Yamada

These files are taken from Linux 3.14.

---

Changes in v3:
- Bring in GPIO bindings for tegra{30,114,124} also

Changes in v2:
- Add new patch to bring in Tegra device tree files from linux

 arch/arm/dts/tegra114.dtsi                         | 21 +++++----
 arch/arm/dts/tegra124.dtsi                         | 23 +++++-----
 arch/arm/dts/tegra20.dtsi                          | 15 ++++++-
 arch/arm/dts/tegra30.dtsi                          | 21 +++++----
 include/dt-bindings/gpio/gpio.h                    | 15 +++++++
 include/dt-bindings/gpio/tegra-gpio.h              | 51 ++++++++++++++++++++++
 include/dt-bindings/interrupt-controller/arm-gic.h | 22 ++++++++++
 include/dt-bindings/interrupt-controller/irq.h     | 19 ++++++++
 8 files changed, 157 insertions(+), 30 deletions(-)
 create mode 100644 include/dt-bindings/gpio/gpio.h
 create mode 100644 include/dt-bindings/gpio/tegra-gpio.h
 create mode 100644 include/dt-bindings/interrupt-controller/arm-gic.h
 create mode 100644 include/dt-bindings/interrupt-controller/irq.h

index f52fcf1..59434e0 100644
@@ -1,3 +1,6 @@
+#include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include "skeleton.dtsi"
 
 / {

@@ -46,17 +49,17 @@
 			      0 143 0x04>;
 	};
 
-	gpio: gpio {
+	gpio: gpio at 6000d000 {
 		compatible = "nvidia,tegra114-gpio", "nvidia,tegra30-gpio";
 		reg = <0x6000d000 0x1000>;
-		interrupts = <0 32 0x04
-			      0 33 0x04
-			      0 34 0x04
-			      0 35 0x04
-			      0 55 0x04
-			      0 87 0x04
-			      0 89 0x04
-			      0 125 0x04>;
+		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
 		#interrupt-cells = <2>;

index 18a8b24..a13cfe4 100644
@@ -1,3 +1,6 @@
+#include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include "skeleton.dtsi"
 
 / {

@@ -46,17 +49,17 @@
 			      0 143 0x04>;
 	};
 
-	gpio: gpio at 6000d000 {
+	gpio: gpio at 0,6000d000 {
 		compatible = "nvidia,tegra124-gpio", "nvidia,tegra30-gpio";
-		reg = <0x6000d000 0x1000>;
-		interrupts = <0 32 0x04
-			      0 33 0x04
-			      0 34 0x04
-			      0 35 0x04
-			      0 55 0x04
-			      0 87 0x04
-			      0 89 0x04
-			      0 125 0x04>;
+		reg = <0x0 0x6000d000 0x0 0x1000>;
+		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
 		#interrupt-cells = <2>;

index 3805750..a524f6e 100644
@@ -1,3 +1,6 @@
+#include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include "skeleton.dtsi"
 
 / {

@@ -139,10 +142,18 @@
 
 	gpio: gpio at 6000d000 {
 		compatible = "nvidia,tegra20-gpio";
-		reg = < 0x6000d000 0x1000 >;
-		interrupts = < 64 65 66 67 87 119 121 >;
+		reg = <0x6000d000 0x1000>;
+		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
+		#interrupt-cells = <2>;
+		interrupt-controller;
 	};
 
 	pinmux: pinmux at 70000000 {

index fee1c36..7be3791 100644
@@ -1,3 +1,6 @@
+#include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
 #include "skeleton.dtsi"
 
 / {

@@ -47,17 +50,17 @@
 		clocks = <&tegra_car 34>;
 	};
 
-	gpio: gpio {
+	gpio: gpio at 6000d000 {
 		compatible = "nvidia,tegra30-gpio";
 		reg = <0x6000d000 0x1000>;
-		interrupts = <0 32 0x04
-			      0 33 0x04
-			      0 34 0x04
-			      0 35 0x04
-			      0 55 0x04
-			      0 87 0x04
-			      0 89 0x04
-			      0 125 0x04>;
+		interrupts = <GIC_SPI 32 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 33 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 34 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 35 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 55 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 87 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 89 IRQ_TYPE_LEVEL_HIGH>,
+			     <GIC_SPI 125 IRQ_TYPE_LEVEL_HIGH>;
 		#gpio-cells = <2>;
 		gpio-controller;
 		#interrupt-cells = <2>;

new file mode 100644
index 0000000..e6b1e0a
@@ -0,0 +1,15 @@
+/*
+ * This header provides constants for most GPIO bindings.
+ *
+ * Most GPIO bindings include a flags cell as part of the GPIO specifier.
+ * In most cases, the format of the flags cell uses the standard values
+ * defined in this header.
+ */
+
+#ifndef _DT_BINDINGS_GPIO_GPIO_H
+#define _DT_BINDINGS_GPIO_GPIO_H
+
+#define GPIO_ACTIVE_HIGH 0
+#define GPIO_ACTIVE_LOW 1
+
+#endif

new file mode 100644
index 0000000..1ea1b70
@@ -0,0 +1,22 @@
+/*
+ * This header provides constants for the ARM GIC.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_ARM_GIC_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_ARM_GIC_H
+
+#include <dt-bindings/interrupt-controller/irq.h>
+
+/* interrupt specific cell 0 */
+
+#define GIC_SPI 0
+#define GIC_PPI 1
+
+/*
+ * Interrupt specifier cell 2.
+ * The flaggs in irq.h are valid, plus those below.
+ */
+#define GIC_CPU_MASK_RAW(x) ((x) << 8)
+#define GIC_CPU_MASK_SIMPLE(num) GIC_CPU_MASK_RAW((1 << (num)) - 1)
+
+#endif

new file mode 100644
index 0000000..33a1003
@@ -0,0 +1,19 @@
+/*
+ * This header provides constants for most IRQ bindings.
+ *
+ * Most IRQ bindings include a flags cell as part of the IRQ specifier.
+ * In most cases, the format of the flags cell uses the standard values
+ * defined in this header.
+ */
+
+#ifndef _DT_BINDINGS_INTERRUPT_CONTROLLER_IRQ_H
+#define _DT_BINDINGS_INTERRUPT_CONTROLLER_IRQ_H
+
+#define IRQ_TYPE_NONE		0
+#define IRQ_TYPE_EDGE_RISING	1
+#define IRQ_TYPE_EDGE_FALLING	2
+#define IRQ_TYPE_EDGE_BOTH	(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING)
+#define IRQ_TYPE_LEVEL_HIGH	4
+#define IRQ_TYPE_LEVEL_LOW	8
+
+#endif

1.9.1.423.g4596e3a

...

Add a note to encourage people to convert drivers to use driver model.

---

Changes in v3: None
Changes in v2:
- Update README to encourage conversion to driver model

 README | 5 +++++
 1 file changed, 5 insertions(+)

index b226458..16485c3 100644
@@ -5239,6 +5239,11 @@ Information structure as we define in include/asm-<arch>/u-boot.h,
 and make sure that your definition of IMAP_ADDR uses the same value
 as your U-Boot configuration in CONFIG_SYS_IMMR.
 
+Note that U-Boot now has a driver model, a unified model for drivers.
+If you are adding a new driver, plumb it into driver model. If there
+is no uclass available, you are encouraged to create one. See
+doc/driver-model.
+
 
 Configuring the Linux kernel:
 -----------------------------

1.9.1.423.g4596e3a

We want 'N0' and 'n0' to mean the same thing, so ensure that case is not considered when naming GPIO banks.

---

Changes in v3: None
Changes in v2:
- Add new patch to use case-insensitive comparison for GPIO banks

 drivers/gpio/gpio-uclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 56bfd11..ec605dc 100644
@@ -58,7 +58,7 @@ int gpio_lookup_name(const char *name, struct device **devp,
 		uc_priv = dev->uclass_priv;
 		len = uc_priv->bank_name ? strlen(uc_priv->bank_name) : 0;
 
-		if (!strncmp(name, uc_priv->bank_name, len)) {
+		if (!strncasecmp(name, uc_priv->bank_name, len)) {
 			if (strict_strtoul(name + len, 10, &offset))
 				continue;
 			if (devp)

1.9.1.423.g4596e3a

These files don't compile in some architectures. Fix it by adding the missing headers.

---

Changes in v3: None
Changes in v2:
- Add new patch to add missing header files in lists and root

 drivers/core/lists.c | 1 +
 drivers/core/root.c  | 1 +
 2 files changed, 2 insertions(+)

index 4f2c126..6a53362 100644
@@ -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)

index 407bc0d..88d2f45 100644
@@ -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>

1.9.1.423.g4596e3a

In a very few cases we need to adjust the driver model root device, such as when setting it up at initialisation. Add a macro to make this easier.

---

Changes in v3:
- Fix typo in commit subject

Changes in v2:
- Add new patch to deal with const-ness of the global_data pointer

 drivers/core/root.c          | 6 +++---
 drivers/core/uclass.c        | 2 +-
 include/dm/device-internal.h | 4 ++++
 3 files changed, 8 insertions(+), 4 deletions(-)

index 88d2f45..4427b81 100644
@@ -43,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;
 

@@ -56,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;

index 4df5a8b..afb5fdc 100644
@@ -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);

index c026e8e..e95b7a9 100644
@@ -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

1.9.1.423.g4596e3a

The GPIO tests require the sandbox GPIO driver, so cannot be run on other platforms. Similarly for the 'dm test' command.

---

Changes in v3: None
Changes in v2:
- Add new patch to allow driver model tests only for sandbox

 test/dm/Makefile |  2 ++
 test/dm/cmd_dm.c | 11 +++++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

index 4e9afe6..c0f2135 100644
@@ -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_SANDBOX),)
 obj-$(CONFIG_DM_GPIO) += gpio.o
+endif

index a03fe20..62eccd6 100644
@@ -93,16 +93,23 @@ static int do_dm_dump_uclass(cmd_tbl_t *cmdtp, int flag, int argc,
 	return 0;
 }
 
+#ifdef CONFIG_DM_TEST
 static int do_dm_test(cmd_tbl_t *cmdtp, int flag, int argc,
 			  char * const argv[])
 {
 	return dm_test_main();
 }
+#define TEST_HELP "\ndm test         Run tests"
+#else
+#define TEST_HELP
+#endif
 
 static cmd_tbl_t test_commands[] = {
 	U_BOOT_CMD_MKENT(tree, 0, 1, do_dm_dump_all, "", ""),
 	U_BOOT_CMD_MKENT(uclass, 1, 1, do_dm_dump_uclass, "", ""),
+#ifdef CONFIG_DM_TEST
 	U_BOOT_CMD_MKENT(test, 1, 1, do_dm_test, "", ""),
+#endif
 };
 
 static int do_dm(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])

@@ -128,6 +135,6 @@ U_BOOT_CMD(
 	dm,	2,	1,	do_dm,
 	"Driver model low level access",
 	"tree         Dump driver model tree\n"
-	"dm uclass        Dump list of instances for each uclass\n"
-	"dm test         Run tests"
+	"dm uclass        Dump list of instances for each uclass"
+	TEST_HELP
 );

1.9.1.423.g4596e3a

The values here are int, but the map_to_sysmem() call can return a long. Add a cast to deal with this.

---

Changes in v3: None
Changes in v2:
- Add new patch to fix printf() strings in the 'dm' command

 test/dm/cmd_dm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

index 62eccd6..c6b2eb8 100644
@@ -23,7 +23,7 @@ static int display_succ(struct device *in, char *buf)
 	char local[16];
 	struct device *pos, *n, *prev = NULL;
 
-	printf("%s- %s @ %08x", buf, in->name, map_to_sysmem(in));
+	printf("%s- %s @ %08x", buf, in->name, (uint)map_to_sysmem(in));
 	if (in->flags & DM_FLAG_ACTIVATED)
 		puts(" - activated");
 	puts("\n");

@@ -62,7 +62,7 @@ static int do_dm_dump_all(cmd_tbl_t *cmdtp, int flag, int argc,
 	struct device *root;
 
 	root = dm_root();
-	printf("ROOT %08x\n", map_to_sysmem(root));
+	printf("ROOT %08x\n", (uint)map_to_sysmem(root));
 	return dm_dump(root);
 }
 

@@ -84,8 +84,8 @@ static int do_dm_dump_uclass(cmd_tbl_t *cmdtp, int flag, int argc,
 		for (ret = uclass_first_device(id, &dev);
 		     dev;
 		     ret = uclass_next_device(&dev)) {
-			printf("  %s @  %08x:\n", dev->name,
-			       map_to_sysmem(dev));
+			printf("  %s @ %08x:\n", dev->name,
+			       (uint)map_to_sysmem(dev));
 		}
 		puts("\n");
 	}

1.9.1.423.g4596e3a

Enable driver model for Tegra boards.

---

Changes in v3:
- Enable dm command in this patch instead of the next

Changes in v2:
- Split out a separate patch to enable driver model for tegra

 include/configs/tegra-common.h | 3 +++
 1 file changed, 3 insertions(+)

index ae786cf..24b909b 100644
@@ -19,6 +19,9 @@
 
 #include <asm/arch/tegra.h>		/* get chip and board defs */
 
+#define CONFIG_DM
+#define CONFIG_CMD_DM
+
 #define CONFIG_SYS_TIMER_RATE		1000000
 #define CONFIG_SYS_TIMER_COUNTER	NV_PA_TMRUS_BASE
 

1.9.1.423.g4596e3a

Seems fine to me,

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.

---

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(-)

index d97190d..80e4a73 100644
@@ -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_ */

index ef4e481..e27efcd 100644
@@ -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
 

index 24b909b..1069218 100644
@@ -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

1.9.1.423.g4596e3a

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.

Hi Stephen,

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

Hi,

One little update - region size increases:

$ ./tools/buildman/buildman -b us-gpio6  -sS seaboard
...
13: tegra: Enable driver model
       arm: (for 1/1 boards)  all +5088.0  bss -4.0  data +182.0
rodata +766.0  spl/u-boot-spl:all +16.0  spl/u-boot-spl:data +16.0
text +4144.0
14: tegra: Convert tegra GPIO driver to use driver model
       arm: (for 1/1 boards)  all -1422.0  bss -4480.0  data +88.0
rodata +330.0  spl/u-boot-spl:all +32.0  spl/u-boot-spl:text +32.0
text +2640.0

Regards,
Simon