* [U-Boot] [PATCH v5 01/15] Add an I/O tracing feature
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 02/15] arm: Support iotrace feature Simon Glass
` (14 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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
diff --git a/README b/README
index a248ab5..21605b8 100644
--- a/README
+++ b/README
@@ -1000,6 +1000,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
@@ -1171,6 +1172,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
diff --git a/common/Makefile b/common/Makefile
index 391a8d6..7e192c7 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -114,6 +114,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
@@ -261,6 +262,7 @@ obj-$(CONFIG_ANDROID_BOOT_IMAGE) += image-android.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
diff --git a/common/cmd_iotrace.c b/common/cmd_iotrace.c
new file mode 100644
index 0000000..f54276d
--- /dev/null
+++ b/common/cmd_iotrace.c
@@ -0,0 +1,73 @@
+/*
+ * Copyright (c) 2014 Google, Inc
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <command.h>
+#include <iotrace.h>
+
+static void do_print_stats(void)
+{
+ ulong start, size, offset, count;
+
+ printf("iotrace is %sabled\n", iotrace_get_enabled() ? "en" : "dis");
+ iotrace_get_buffer(&start, &size, &offset, &count);
+ printf("Start: %08lx\n", start);
+ printf("Size: %08lx\n", size);
+ printf("Offset: %08lx\n", offset);
+ printf("Output: %08lx\n", start + offset);
+ printf("Count: %08lx\n", count);
+ printf("CRC32: %08lx\n", (ulong)iotrace_get_checksum());
+}
+
+static int do_set_buffer(int argc, char * const argv[])
+{
+ ulong addr = 0, size = 0;
+
+ if (argc == 2) {
+ addr = simple_strtoul(*argv++, NULL, 16);
+ size = simple_strtoul(*argv++, NULL, 16);
+ } else if (argc != 0) {
+ return CMD_RET_USAGE;
+ }
+
+ iotrace_set_buffer(addr, size);
+
+ return 0;
+}
+
+int do_iotrace(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+ const char *cmd = argc < 2 ? NULL : argv[1];
+
+ if (!cmd)
+ return cmd_usage(cmdtp);
+ switch (*cmd) {
+ case 'b':
+ return do_set_buffer(argc - 2, argv + 2);
+ case 'p':
+ iotrace_set_enabled(0);
+ break;
+ case 'r':
+ iotrace_set_enabled(1);
+ break;
+ case 's':
+ do_print_stats();
+ break;
+ default:
+ return CMD_RET_USAGE;
+ }
+
+ return 0;
+}
+
+U_BOOT_CMD(
+ iotrace, 4, 1, do_iotrace,
+ "iotrace utility commands",
+ "stats - display iotrace stats\n"
+ "iotrace buffer <address> <size> - set iotrace buffer\n"
+ "iotrace pause - pause tracing\n"
+ "iotrace resume - resume tracing"
+);
diff --git a/common/iotrace.c b/common/iotrace.c
new file mode 100644
index 0000000..ced426e
--- /dev/null
+++ b/common/iotrace.c
@@ -0,0 +1,169 @@
+/*
+ * Copyright (c) 2014 Google, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#define IOTRACE_IMPL
+
+#include <common.h>
+#include <asm/io.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* Support up to the machine word length for now */
+typedef ulong iovalue_t;
+
+enum iotrace_flags {
+ IOT_8 = 0,
+ IOT_16,
+ IOT_32,
+
+ IOT_READ = 0 << 3,
+ IOT_WRITE = 1 << 3,
+};
+
+/**
+ * struct iotrace_record - Holds a single I/O trace record
+ *
+ * @flags: I/O access type
+ * @addr: Address of access
+ * @value: Value written or read
+ */
+struct iotrace_record {
+ enum iotrace_flags flags;
+ phys_addr_t addr;
+ iovalue_t value;
+};
+
+/**
+ * struct iotrace - current trace status and checksum
+ *
+ * @start: Start address of iotrace buffer
+ * @size: Size of iotrace buffer in bytes
+ * @offset: Current write offset into iotrace buffer
+ * @crc32: Current value of CRC chceksum of trace records
+ * @enabled: true if enabled, false if disabled
+ */
+static struct iotrace {
+ ulong start;
+ ulong size;
+ ulong offset;
+ u32 crc32;
+ bool enabled;
+} iotrace;
+
+static void add_record(int flags, const void *ptr, ulong value)
+{
+ struct iotrace_record srec, *rec = &srec;
+
+ /*
+ * We don't support iotrace before relocation. Since the trace buffer
+ * is set up by a command, it can't be enabled at present. To change
+ * this we would need to set the iotrace buffer@build-time. See
+ * lib/trace.c for how this might be done if you are interested.
+ */
+ if (!(gd->flags & GD_FLG_RELOC) || !iotrace.enabled)
+ return;
+
+ /* Store it if there is room */
+ if (iotrace.offset + sizeof(*rec) < iotrace.size) {
+ rec = (struct iotrace_record *)map_sysmem(
+ iotrace.start + iotrace.offset,
+ sizeof(value));
+ }
+
+ rec->flags = flags;
+ rec->addr = map_to_sysmem(ptr);
+ rec->value = value;
+
+ /* Update our checksum */
+ iotrace.crc32 = crc32(iotrace.crc32, (unsigned char *)rec,
+ sizeof(*rec));
+
+ iotrace.offset += sizeof(struct iotrace_record);
+}
+
+u32 iotrace_readl(const void *ptr)
+{
+ u32 v;
+
+ v = readl(ptr);
+ add_record(IOT_32 | IOT_READ, ptr, v);
+
+ return v;
+}
+
+void iotrace_writel(ulong value, const void *ptr)
+{
+ add_record(IOT_32 | IOT_WRITE, ptr, value);
+ writel(value, ptr);
+}
+
+u16 iotrace_readw(const void *ptr)
+{
+ u32 v;
+
+ v = readw(ptr);
+ add_record(IOT_16 | IOT_READ, ptr, v);
+
+ return v;
+}
+
+void iotrace_writew(ulong value, const void *ptr)
+{
+ add_record(IOT_16 | IOT_WRITE, ptr, value);
+ writew(value, ptr);
+}
+
+u8 iotrace_readb(const void *ptr)
+{
+ u32 v;
+
+ v = readb(ptr);
+ add_record(IOT_8 | IOT_READ, ptr, v);
+
+ return v;
+}
+
+void iotrace_writeb(ulong value, const void *ptr)
+{
+ add_record(IOT_8 | IOT_WRITE, ptr, value);
+ writeb(value, ptr);
+}
+
+void iotrace_reset_checksum(void)
+{
+ iotrace.crc32 = 0;
+}
+
+u32 iotrace_get_checksum(void)
+{
+ return iotrace.crc32;
+}
+
+void iotrace_set_enabled(int enable)
+{
+ iotrace.enabled = enable;
+}
+
+int iotrace_get_enabled(void)
+{
+ return iotrace.enabled;
+}
+
+void iotrace_set_buffer(ulong start, ulong size)
+{
+ iotrace.start = start;
+ iotrace.size = size;
+ iotrace.offset = 0;
+ iotrace.crc32 = 0;
+}
+
+void iotrace_get_buffer(ulong *start, ulong *size, ulong *offset, ulong *count)
+{
+ *start = iotrace.start;
+ *size = iotrace.size;
+ *offset = iotrace.offset;
+ *count = iotrace.offset / sizeof(struct iotrace_record);
+}
diff --git a/include/iotrace.h b/include/iotrace.h
new file mode 100644
index 0000000..9bd1f16
--- /dev/null
+++ b/include/iotrace.h
@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2014 Google, Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#ifndef __IOTRACE_H
+#define __IOTRACE_H
+
+#include <linux/types.h>
+
+/*
+ * This file is designed to be included in arch/<arch>/include/asm/io.h.
+ * It redirects all IO access through a tracing/checksumming feature for
+ * testing purposes.
+ */
+
+#if defined(CONFIG_IO_TRACE) && !defined(IOTRACE_IMPL) && \
+ !defined(CONFIG_SPL_BUILD)
+
+#undef readl
+#define readl(addr) iotrace_readl((const void *)(addr))
+
+#undef writel
+#define writel(val, addr) iotrace_writel(val, (const void *)(addr))
+
+#undef readw
+#define readw(addr) iotrace_readw((const void *)(addr))
+
+#undef writew
+#define writew(val, addr) iotrace_writew(val, (const void *)(addr))
+
+#undef readb
+#define readb(addr) iotrace_readb((const void *)(addr))
+
+#undef writeb
+#define writeb(val, addr) iotrace_writeb(val, (const void *)(addr))
+
+#endif
+
+/* Tracing functions which mirror their io.h counterparts */
+u32 iotrace_readl(const void *ptr);
+void iotrace_writel(ulong value, const void *ptr);
+u16 iotrace_readw(const void *ptr);
+void iotrace_writew(ulong value, const void *ptr);
+u8 iotrace_readb(const void *ptr);
+void iotrace_writeb(ulong value, const void *ptr);
+
+/**
+ * iotrace_reset_checksum() - Reset the iotrace checksum
+ */
+void iotrace_reset_checksum(void);
+
+/**
+ * iotrace_get_checksum() - Get the current checksum value
+ *
+ * @return currect checksum value
+ */
+u32 iotrace_get_checksum(void);
+
+/**
+ * iotrace_set_enabled() - Set whether iotracing is enabled or not
+ *
+ * This controls whether the checksum is updated and a trace record added
+ * for each I/O access.
+ *
+ * @enable: true to enable iotracing, false to disable
+ */
+void iotrace_set_enabled(int enable);
+
+/**
+ * iotrace_get_enabled() - Get whether iotracing is enabled or not
+ *
+ * @return true if enabled, false if disabled
+ */
+int iotrace_get_enabled(void);
+
+/**
+ * iotrace_set_buffer() - Set position and size of iotrace buffer
+ *
+ * Defines where the iotrace buffer goes, and resets the output pointer to
+ * the start of the buffer.
+ *
+ * The buffer can be 0 size in which case the checksum is updated but no
+ * trace records are writen. If the buffer is exhausted, the offset will
+ * continue to increase but not new data will be written.
+ *
+ * @start: Start address of buffer
+ * @size: Size of buffer in bytes
+ */
+void iotrace_set_buffer(ulong start, ulong size);
+
+/**
+ * iotrace_get_buffer() - Get buffer information
+ *
+ * @start: Returns start address of buffer
+ * @size: Returns size of buffer in bytes
+ * @offset: Returns the byte offset where the next output trace record will
+ * @count: Returns the number of trace records recorded
+ * be written (or would be if the buffer was large enough)
+ */
+void iotrace_get_buffer(ulong *start, ulong *size, ulong *offset, ulong *count);
+
+#endif /* __IOTRACE_H */
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 02/15] arm: Support iotrace feature
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 01/15] Add an I/O tracing feature Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 03/15] sandbox: " Simon Glass
` (13 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
Support the iotrace feature for ARM, when enabled.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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(+)
diff --git a/arch/arm/include/asm/io.h b/arch/arm/include/asm/io.h
index 6a1f05a..9f35fd6 100644
--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -437,4 +437,7 @@ out:
#endif /* __mem_isa */
#endif /* __KERNEL__ */
+
+#include <iotrace.h>
+
#endif /* __ASM_ARM_IO_H */
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 03/15] sandbox: Support iotrace feature
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 01/15] Add an I/O tracing feature Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 02/15] arm: Support iotrace feature Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 04/15] Makefile: Support include files for .dts files Simon Glass
` (12 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
Support the iotrace feature for sandbox, and enable it, using some dummy
I/O access methods.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4:
- Correct typo in CONFIG_CMD_IOTRACE
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(+)
diff --git a/arch/sandbox/include/asm/io.h b/arch/sandbox/include/asm/io.h
index 7956041..895fcb8 100644
--- a/arch/sandbox/include/asm/io.h
+++ b/arch/sandbox/include/asm/io.h
@@ -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
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 6bb2546..fdc8b75 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -16,6 +16,9 @@
#endif
+#define CONFIG_IO_TRACE
+#define CONFIG_CMD_IOTRACE
+
#define CONFIG_SYS_TIMER_RATE 1000000
#define CONFIG_BOOTSTAGE
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 04/15] Makefile: Support include files for .dts files
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (2 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 03/15] sandbox: " Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 05/15] dm: Rename struct device_id to udevice_id Simon Glass
` (11 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---
Changes in v5: None
Changes in v4: None
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
diff --git a/arch/arm/dts/include/dt-bindings b/arch/arm/dts/include/dt-bindings
new file mode 120000
index 0000000..0cecb3d
--- /dev/null
+++ b/arch/arm/dts/include/dt-bindings
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file
diff --git a/arch/microblaze/dts/include/dt-bindings b/arch/microblaze/dts/include/dt-bindings
new file mode 120000
index 0000000..0cecb3d
--- /dev/null
+++ b/arch/microblaze/dts/include/dt-bindings
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file
diff --git a/arch/sandbox/dts/include/dt-bindings b/arch/sandbox/dts/include/dt-bindings
new file mode 120000
index 0000000..0cecb3d
--- /dev/null
+++ b/arch/sandbox/dts/include/dt-bindings
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file
diff --git a/arch/x86/dts/include/dt-bindings b/arch/x86/dts/include/dt-bindings
new file mode 120000
index 0000000..0cecb3d
--- /dev/null
+++ b/arch/x86/dts/include/dt-bindings
@@ -0,0 +1 @@
+../../../../include/dt-bindings
\ No newline at end of file
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index c24c5e8..968123c 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -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
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 05/15] dm: Rename struct device_id to udevice_id
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (3 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 04/15] Makefile: Support include files for .dts files Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-11 13:49 ` Jon Loeliger
2014-06-10 23:53 ` [U-Boot] [PATCH v5 06/15] dm: Update README to encourage conversion to driver model Simon Glass
` (10 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
It is best to avoid having any occurence of 'struct device' in driver
model, so rename to achieve this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4:
- Add new patch to rename struct device_id to udevice_id
Changes in v3: None
Changes in v2: None
doc/driver-model/README.txt | 2 +-
drivers/core/lists.c | 2 +-
drivers/demo/demo-shape.c | 2 +-
drivers/demo/demo-simple.c | 2 +-
drivers/gpio/sandbox.c | 2 +-
include/dm/device.h | 6 +++---
test/dm/test-fdt.c | 2 +-
7 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
index dcecb9a..4af89b0 100644
--- a/doc/driver-model/README.txt
+++ b/doc/driver-model/README.txt
@@ -315,7 +315,7 @@ is little or no 'driver model' code to write.
- Moved some data from code into data structure - e.g. store a pointer to
the driver operations structure in the driver, rather than passing it
to the driver bind function.
-- Rename some structures to make them more similar to Linux (struct device
+- Rename some structures to make them more similar to Linux (struct udevice
instead of struct instance, struct platdata, etc.)
- Change the name 'core' to 'uclass', meaning U-Boot class. It seems that
this concept relates to a class of drivers (or a subsystem). We shouldn't
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 205b140..9f2917f 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -94,7 +94,7 @@ int lists_bind_drivers(struct udevice *parent)
* tree error
*/
static int driver_check_compatible(const void *blob, int offset,
- const struct device_id *of_match)
+ const struct udevice_id *of_match)
{
int ret;
diff --git a/drivers/demo/demo-shape.c b/drivers/demo/demo-shape.c
index a68cc10..3fa9c59 100644
--- a/drivers/demo/demo-shape.c
+++ b/drivers/demo/demo-shape.c
@@ -111,7 +111,7 @@ static int shape_ofdata_to_platdata(struct udevice *dev)
return 0;
}
-static const struct device_id demo_shape_id[] = {
+static const struct udevice_id demo_shape_id[] = {
{ "demo-shape", 0 },
{ },
};
diff --git a/drivers/demo/demo-simple.c b/drivers/demo/demo-simple.c
index 11def86..2bcb7df 100644
--- a/drivers/demo/demo-simple.c
+++ b/drivers/demo/demo-simple.c
@@ -32,7 +32,7 @@ static int demo_shape_ofdata_to_platdata(struct udevice *dev)
return demo_parse_dt(dev);
}
-static const struct device_id demo_shape_id[] = {
+static const struct udevice_id demo_shape_id[] = {
{ "demo-simple", 0 },
{ },
};
diff --git a/drivers/gpio/sandbox.c b/drivers/gpio/sandbox.c
index 09cebe2..75ada5d 100644
--- a/drivers/gpio/sandbox.c
+++ b/drivers/gpio/sandbox.c
@@ -239,7 +239,7 @@ static int gpio_sandbox_probe(struct udevice *dev)
return 0;
}
-static const struct device_id sandbox_gpio_ids[] = {
+static const struct udevice_id sandbox_gpio_ids[] = {
{ .compatible = "sandbox,gpio" },
{ }
};
diff --git a/include/dm/device.h b/include/dm/device.h
index ec04982..19f2039 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -75,11 +75,11 @@ struct udevice {
#define device_active(dev) ((dev)->flags & DM_FLAG_ACTIVATED)
/**
- * struct device_id - Lists the compatible strings supported by a driver
+ * struct udevice_id - Lists the compatible strings supported by a driver
* @compatible: Compatible string
* @data: Data for this compatible string
*/
-struct device_id {
+struct udevice_id {
const char *compatible;
ulong data;
};
@@ -121,7 +121,7 @@ struct device_id {
struct driver {
char *name;
enum uclass_id id;
- const struct device_id *of_match;
+ const struct udevice_id *of_match;
int (*bind)(struct udevice *dev);
int (*probe)(struct udevice *dev);
int (*remove)(struct udevice *dev);
diff --git a/test/dm/test-fdt.c b/test/dm/test-fdt.c
index 6eccf11..98e3936 100644
--- a/test/dm/test-fdt.c
+++ b/test/dm/test-fdt.c
@@ -53,7 +53,7 @@ static int testfdt_drv_probe(struct udevice *dev)
return 0;
}
-static const struct device_id testfdt_ids[] = {
+static const struct udevice_id testfdt_ids[] = {
{
.compatible = "denx,u-boot-fdt-test",
.data = DM_TEST_TYPE_FIRST },
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 05/15] dm: Rename struct device_id to udevice_id
2014-06-10 23:53 ` [U-Boot] [PATCH v5 05/15] dm: Rename struct device_id to udevice_id Simon Glass
@ 2014-06-11 13:49 ` Jon Loeliger
2014-06-11 16:12 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Jon Loeliger @ 2014-06-11 13:49 UTC (permalink / raw)
To: u-boot
On Tue, Jun 10, 2014 at 6:53 PM, Simon Glass <sjg@chromium.org> wrote:
> It is best to avoid having any occurence of 'struct device' in driver
> model, so rename to achieve this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
Hmm. It's not just a good idea, right? That change is upstream already
so this is a compilation requirement now, right?
jdl
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v5 05/15] dm: Rename struct device_id to udevice_id
2014-06-11 13:49 ` Jon Loeliger
@ 2014-06-11 16:12 ` Simon Glass
0 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-11 16:12 UTC (permalink / raw)
To: u-boot
Hi Jon,
On 11 June 2014 07:49, Jon Loeliger <loeliger@gmail.com> wrote:
> On Tue, Jun 10, 2014 at 6:53 PM, Simon Glass <sjg@chromium.org> wrote:
>> It is best to avoid having any occurence of 'struct device' in driver
>> model, so rename to achieve this.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>
>
> Hmm. It's not just a good idea, right? That change is upstream already
> so this is a compilation requirement now, right?
No, it's just a rename. But I feel that if we have 'struct udevice' we
should not have 'struct device_id'. It is just confusing.
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v5 06/15] dm: Update README to encourage conversion to driver model
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (4 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 05/15] dm: Rename struct device_id to udevice_id Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 07/15] dm: Use case-insensitive comparison for GPIO banks Simon Glass
` (9 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
Add a note to encourage people to convert drivers to use driver model.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2:
- Update README to encourage conversion to driver model
README | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/README b/README
index 21605b8..99edd5f 100644
--- a/README
+++ b/README
@@ -5334,6 +5334,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:
-----------------------------
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 07/15] dm: Use case-insensitive comparison for GPIO banks
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (5 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 06/15] dm: Update README to encourage conversion to driver model Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 08/15] dm: Add missing header files in lists and root Simon Glass
` (8 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
We want 'N0' and 'n0' to mean the same thing, so ensure that case is not
considered when naming GPIO banks.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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(-)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c
index fa2c2fb..f1bbc58 100644
--- a/drivers/gpio/gpio-uclass.c
+++ b/drivers/gpio/gpio-uclass.c
@@ -58,7 +58,7 @@ int gpio_lookup_name(const char *name, struct udevice **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)
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 08/15] dm: Add missing header files in lists and root
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (6 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 07/15] dm: Use case-insensitive comparison for GPIO banks Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 09/15] dm: Cast away the const-ness of the global_data pointer Simon Glass
` (7 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
These files don't compile in some architectures. Fix it by adding the
missing headers.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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(+)
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 9f2917f..afb59d1 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 4977875..f31be72 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>
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 09/15] dm: Cast away the const-ness of the global_data pointer
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (7 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 08/15] dm: Add missing header files in lists and root Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 10/15] dm: Allow driver model tests only for sandbox Simon Glass
` (6 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
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.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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(-)
diff --git a/drivers/core/root.c b/drivers/core/root.c
index f31be72..70ea97f 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -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;
diff --git a/drivers/core/uclass.c b/drivers/core/uclass.c
index f6867e4..d16d6fc 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/include/dm/device-internal.h b/include/dm/device-internal.h
index ea3df36..67318fa 100644
--- a/include/dm/device-internal.h
+++ b/include/dm/device-internal.h
@@ -84,4 +84,8 @@ int device_remove(struct udevice *dev);
*/
int device_unbind(struct udevice *dev);
+/* Cast away any volatile pointer */
+#define DM_ROOT() (((gd_t *)gd)->dm_root)
+#define DM_UCLASS_ROOT() (((gd_t *)gd)->uclass_root)
+
#endif
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 10/15] dm: Allow driver model tests only for sandbox
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (8 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 09/15] dm: Cast away the const-ness of the global_data pointer Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-11 14:23 ` Jon Loeliger
2014-06-10 23:53 ` [U-Boot] [PATCH v5 11/15] dm: Fix printf() strings in the 'dm' command Simon Glass
` (5 subsequent siblings)
15 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
The GPIO tests require the sandbox GPIO driver, so cannot be run on other
platforms. Similarly for the 'dm test' command.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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(-)
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 4e9afe6..c0f2135 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_SANDBOX),)
obj-$(CONFIG_DM_GPIO) += gpio.o
+endif
diff --git a/test/dm/cmd_dm.c b/test/dm/cmd_dm.c
index 083f15c..180661b 100644
--- a/test/dm/cmd_dm.c
+++ b/test/dm/cmd_dm.c
@@ -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
);
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 11/15] dm: Fix printf() strings in the 'dm' command
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (9 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 10/15] dm: Allow driver model tests only for sandbox Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 12/15] tegra: dts: Bring in GPIO bindings from linux Simon Glass
` (4 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
The values here are int, but the map_to_sysmem() call can return a long.
Add a cast to deal with this.
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v5: None
Changes in v4: None
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(-)
diff --git a/test/dm/cmd_dm.c b/test/dm/cmd_dm.c
index 180661b..8019407 100644
--- a/test/dm/cmd_dm.c
+++ b/test/dm/cmd_dm.c
@@ -23,7 +23,7 @@ static int display_succ(struct udevice *in, char *buf)
char local[16];
struct udevice *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 udevice *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");
}
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 12/15] tegra: dts: Bring in GPIO bindings from linux
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (10 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 11/15] dm: Fix printf() strings in the 'dm' command Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 13/15] tegra: Enable driver model Simon Glass
` (3 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
These files are taken from Linux 3.14.
Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v5: None
Changes in v4:
- Remove 64-bit addresses which are not used in U-Boot
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 | 19 ++++----
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, 155 insertions(+), 28 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
diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
index f52fcf1..59434e0 100644
--- a/arch/arm/dts/tegra114.dtsi
+++ b/arch/arm/dts/tegra114.dtsi
@@ -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>;
diff --git a/arch/arm/dts/tegra124.dtsi b/arch/arm/dts/tegra124.dtsi
index 18a8b24..4561c5f 100644
--- a/arch/arm/dts/tegra124.dtsi
+++ b/arch/arm/dts/tegra124.dtsi
@@ -1,3 +1,6 @@
+#include <dt-bindings/gpio/tegra-gpio.h>
+#include <dt-bindings/interrupt-controller/arm-gic.h>
+
#include "skeleton.dtsi"
/ {
@@ -49,14 +52,14 @@
gpio: gpio at 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>;
+ 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>;
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index 3805750..a524f6e 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -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 {
diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
index fee1c36..7be3791 100644
--- a/arch/arm/dts/tegra30.dtsi
+++ b/arch/arm/dts/tegra30.dtsi
@@ -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>;
diff --git a/include/dt-bindings/gpio/gpio.h b/include/dt-bindings/gpio/gpio.h
new file mode 100644
index 0000000..e6b1e0a
--- /dev/null
+++ b/include/dt-bindings/gpio/gpio.h
@@ -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
diff --git a/include/dt-bindings/gpio/tegra-gpio.h b/include/dt-bindings/gpio/tegra-gpio.h
new file mode 100644
index 0000000..197dc28
--- /dev/null
+++ b/include/dt-bindings/gpio/tegra-gpio.h
@@ -0,0 +1,51 @@
+/*
+ * This header provides constants for binding nvidia,tegra*-gpio.
+ *
+ * The first cell in Tegra's GPIO specifier is the GPIO ID. The macros below
+ * provide names for this.
+ *
+ * The second cell contains standard flag values specified in gpio.h.
+ */
+
+#ifndef _DT_BINDINGS_GPIO_TEGRA_GPIO_H
+#define _DT_BINDINGS_GPIO_TEGRA_GPIO_H
+
+#include <dt-bindings/gpio/gpio.h>
+
+#define TEGRA_GPIO_BANK_ID_A 0
+#define TEGRA_GPIO_BANK_ID_B 1
+#define TEGRA_GPIO_BANK_ID_C 2
+#define TEGRA_GPIO_BANK_ID_D 3
+#define TEGRA_GPIO_BANK_ID_E 4
+#define TEGRA_GPIO_BANK_ID_F 5
+#define TEGRA_GPIO_BANK_ID_G 6
+#define TEGRA_GPIO_BANK_ID_H 7
+#define TEGRA_GPIO_BANK_ID_I 8
+#define TEGRA_GPIO_BANK_ID_J 9
+#define TEGRA_GPIO_BANK_ID_K 10
+#define TEGRA_GPIO_BANK_ID_L 11
+#define TEGRA_GPIO_BANK_ID_M 12
+#define TEGRA_GPIO_BANK_ID_N 13
+#define TEGRA_GPIO_BANK_ID_O 14
+#define TEGRA_GPIO_BANK_ID_P 15
+#define TEGRA_GPIO_BANK_ID_Q 16
+#define TEGRA_GPIO_BANK_ID_R 17
+#define TEGRA_GPIO_BANK_ID_S 18
+#define TEGRA_GPIO_BANK_ID_T 19
+#define TEGRA_GPIO_BANK_ID_U 20
+#define TEGRA_GPIO_BANK_ID_V 21
+#define TEGRA_GPIO_BANK_ID_W 22
+#define TEGRA_GPIO_BANK_ID_X 23
+#define TEGRA_GPIO_BANK_ID_Y 24
+#define TEGRA_GPIO_BANK_ID_Z 25
+#define TEGRA_GPIO_BANK_ID_AA 26
+#define TEGRA_GPIO_BANK_ID_BB 27
+#define TEGRA_GPIO_BANK_ID_CC 28
+#define TEGRA_GPIO_BANK_ID_DD 29
+#define TEGRA_GPIO_BANK_ID_EE 30
+#define TEGRA_GPIO_BANK_ID_FF 31
+
+#define TEGRA_GPIO(bank, offset) \
+ ((TEGRA_GPIO_BANK_ID_##bank * 8) + offset)
+
+#endif
diff --git a/include/dt-bindings/interrupt-controller/arm-gic.h b/include/dt-bindings/interrupt-controller/arm-gic.h
new file mode 100644
index 0000000..1ea1b70
--- /dev/null
+++ b/include/dt-bindings/interrupt-controller/arm-gic.h
@@ -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
diff --git a/include/dt-bindings/interrupt-controller/irq.h b/include/dt-bindings/interrupt-controller/irq.h
new file mode 100644
index 0000000..33a1003
--- /dev/null
+++ b/include/dt-bindings/interrupt-controller/irq.h
@@ -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
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 13/15] tegra: Enable driver model
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (11 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 12/15] tegra: dts: Bring in GPIO bindings from linux Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 14/15] dm: Tidy up four minor code nits Simon Glass
` (2 subsequent siblings)
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
Enable driver model for Tegra boards.
Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Stephen Warren <swarren@nvidia.com>
---
Changes in v5: None
Changes in v4: None
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(+)
diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index 129acf2..3b88a83 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_CMD_DM
+
#define CONFIG_SYS_TIMER_RATE 1000000
#define CONFIG_SYS_TIMER_COUNTER NV_PA_TMRUS_BASE
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 14/15] dm: Tidy up four minor code nits
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (12 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 13/15] tegra: Enable driver model Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-10 23:53 ` [U-Boot] [PATCH v5 15/15] dm: Expand and improve the device lifecycle docs Simon Glass
2014-06-11 14:26 ` [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Jon Loeliger
15 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
There is a spelling mistake and two functions are missing comments
altogether. Also the flags declaration is correct, but doesn't follow
style. Finally, the uclass_get_device() function has some errors in
its documentation.
Fix these problems.
Signed-off-by: Simon Glass <sjg@chromium.org>
Acked-by: Marek Vasut <marex@denx.de>
---
Changes in v5: None
Changes in v4: None
Changes in v3: None
Changes in v2: None
include/dm/device.h | 2 +-
include/dm/lists.h | 20 ++++++++++++++++++++
include/dm/root.h | 2 +-
include/dm/uclass.h | 6 ++++--
4 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/include/dm/device.h b/include/dm/device.h
index 19f2039..ae75a3f 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -21,7 +21,7 @@ struct driver_info;
#define DM_FLAG_ACTIVATED (1 << 0)
/* DM is responsible for allocating and freeing platdata */
-#define DM_FLAG_ALLOC_PDATA (2 << 0)
+#define DM_FLAG_ALLOC_PDATA (1 << 1)
/**
* struct udevice - An instance of a driver
diff --git a/include/dm/lists.h b/include/dm/lists.h
index 7feba4b..49d87e6 100644
--- a/include/dm/lists.h
+++ b/include/dm/lists.h
@@ -32,8 +32,28 @@ struct driver *lists_driver_lookup_name(const char *name);
*/
struct uclass_driver *lists_uclass_lookup(enum uclass_id id);
+/**
+ * lists_bind_drivers() - search for and bind all drivers to parent
+ *
+ * This searches the U_BOOT_DEVICE() structures and creates new devices for
+ * each one. The devices will have @parent as their parent.
+ *
+ * @parent: parent driver (root)
+ * @early_only: If true, bind only drivers with the DM_INIT_F flag. If false
+ * bind all drivers.
+ */
int lists_bind_drivers(struct udevice *parent);
+/**
+ * lists_bind_fdt() - bind a device tree node
+ *
+ * This creates a new device bound to the given device tree node, with
+ * @parent as its parent.
+ *
+ * @parent: parent driver (root)
+ * @blob: device tree blob
+ * @offset: offset of this device tree node
+ */
int lists_bind_fdt(struct udevice *parent, const void *blob, int offset);
#endif
diff --git a/include/dm/root.h b/include/dm/root.h
index 3018bc8..a4826a6 100644
--- a/include/dm/root.h
+++ b/include/dm/root.h
@@ -41,7 +41,7 @@ int dm_scan_platdata(void);
int dm_scan_fdt(const void *blob);
/**
- * dm_init() - Initialize Driver Model structures
+ * dm_init() - Initialise Driver Model structures
*
* This function will initialize roots of driver tree and class tree.
* This needs to be called before anything uses the DM
diff --git a/include/dm/uclass.h b/include/dm/uclass.h
index 931d9c0..0df2e5e 100644
--- a/include/dm/uclass.h
+++ b/include/dm/uclass.h
@@ -96,12 +96,14 @@ int uclass_get(enum uclass_id key, struct uclass **ucp);
/**
* uclass_get_device() - Get a uclass device based on an ID and index
*
+ * The device is probed to activate it ready for use.
+ *
* id: ID to look up
* @index: Device number within that uclass (0=first)
- * @ucp: Returns pointer to uclass (there is only one per for each ID)
+ * devp: Returns pointer to device (there is only one per for each ID)
* @return 0 if OK, -ve on error
*/
-int uclass_get_device(enum uclass_id id, int index, struct udevice **ucp);
+int uclass_get_device(enum uclass_id id, int index, struct udevice **devp);
/**
* uclass_first_device() - Get the first device in a uclass
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 15/15] dm: Expand and improve the device lifecycle docs
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (13 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 14/15] dm: Tidy up four minor code nits Simon Glass
@ 2014-06-10 23:53 ` Simon Glass
2014-06-11 14:19 ` Jon Loeliger
2014-06-11 14:26 ` [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Jon Loeliger
15 siblings, 1 reply; 22+ messages in thread
From: Simon Glass @ 2014-06-10 23:53 UTC (permalink / raw)
To: u-boot
The lifecycle of a device is an important part of driver model. Add to the
existing documentation and clarify it.
Reported-by: Jon Loeliger <jdl@jdl.com>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Thanks for Jon Loeliger <jdl@jdl.com> for helping with the text and
suggesting improvements.
(Jon please comment/adjust to help clarify things further)
Changes in v5:
- Fix a few more typos
Changes in v4:
- Minor spelling fixes
Changes in v3: None
Changes in v2:
- Rename struct device to struct udevice
- Adjust docs as per Jon's review
doc/driver-model/README.txt | 221 ++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 211 insertions(+), 10 deletions(-)
diff --git a/doc/driver-model/README.txt b/doc/driver-model/README.txt
index 4af89b0..14ae3f5 100644
--- a/doc/driver-model/README.txt
+++ b/doc/driver-model/README.txt
@@ -20,7 +20,7 @@ Terminology
-----------
Uclass - a group of devices which operate in the same way. A uclass provides
- a way of accessing invidual devices within the group, but always
+ a way of accessing individual devices within the group, but always
using the same interface. For example a GPIO uclass provides
operations for get/set value. An I2C uclass may have 10 I2C ports,
4 with one driver, and 6 with another.
@@ -222,13 +222,50 @@ device tree) and probe.
Platform Data
-------------
-Where does the platform data come from? See demo-pdata.c which
+Platform data is like Linux platform data, if you are familiar with that.
+It provides the board-specific information to start up a device.
+
+Why is this information not just stored in the device driver itself? The
+idea is that the device driver is generic, and can in principle operate on
+any board that has that type of device. For example, with modern
+highly-complex SoCs it is common for the IP to come from an IP vendor, and
+therefore (for example) the MMC controller may be the same on chips from
+different vendors. It makes no sense to write independent drivers for the
+MMC controller on each vendor's SoC, when they are all almost the same.
+Similarly, we may have 6 UARTs in an SoC, all of which are mostly the same,
+but lie at different addresses in the address space.
+
+Using the UART example, we have a single driver and it is instantiated 6
+times by supplying 6 lots of platform data. Each lot of platform data
+gives the driver name and a pointer to a structure containing information
+about this instance - e.g. the address of the register space. It may be that
+one of the UARTS supports RS-485 operation - this can be added as a flag in
+the platform data, which is set for this one port and clear for the rest.
+
+Think of your driver as a generic piece of code which knows how to talk to
+a device, but needs to know where it is, any variant/option information and
+so on. Platform data provides this link between the generic piece of code
+and the specific way it is bound on a particular board.
+
+Examples of platform data include:
+
+ - The base address of the IP block's register space
+ - Configuration options, like:
+ - the SPI polarity and maximum speed for a SPI controller
+ - the I2C speed to use for an I2C device
+ - the number of GPIOs available in a GPIO device
+
+Where does the platform data come from? It is either held in a structure
+which is compiled into U-Boot, or it can be parsed from the Device Tree
+(see 'Device Tree' below).
+
+For an example of how it can be compiled in, see demo-pdata.c which
sets up a table of driver names and their associated platform data.
The data can be interpreted by the drivers however they like - it is
basically a communication scheme between the board-specific code and
the generic drivers, which are intended to work on any board.
-Drivers can acceess their data via dev->info->platdata. Here is
+Drivers can access their data via dev->info->platdata. Here is
the declaration for the platform data, which would normally appear
in the board file.
@@ -259,21 +296,30 @@ following device tree fragment:
sides = <4>;
};
+This means that instead of having lots of U_BOOT_DEVICE() declarations in
+the board file, we put these in the device tree. This approach allows a lot
+more generality, since the same board file can support many types of boards
+(e,g. with the same SoC) just by using different device trees. An added
+benefit is that the Linux device tree can be used, thus further simplifying
+the task of board-bring up either for U-Boot or Linux devs (whoever gets to
+the board first!).
The easiest way to make this work it to add a few members to the driver:
.platdata_auto_alloc_size = sizeof(struct dm_test_pdata),
.ofdata_to_platdata = testfdt_ofdata_to_platdata,
- .probe = testfdt_drv_probe,
The 'auto_alloc' feature allowed space for the platdata to be allocated
-and zeroed before the driver's ofdata_to_platdata method is called. This
-method reads the information out of the device tree and puts it in
-dev->platdata. Then the probe method is called to set up the device.
+and zeroed before the driver's ofdata_to_platdata() method is called. The
+ofdata_to_platdata() method, which the driver write supplies, should parse
+the device tree node for this device and place it in dev->platdata. Thus
+when the probe method is called later (to set up the device ready for use)
+the platform data will be present.
Note that both methods are optional. If you provide an ofdata_to_platdata
-method then it wlil be called first (after bind). If you provide a probe
-method it will be called next.
+method then it will be called first (during activation). If you provide a
+probe method it will be called next. See Driver Lifecycle below for more
+details.
If you don't want to have the platdata automatically allocated then you
can leave out platdata_auto_alloc_size. In this case you can use malloc
@@ -295,6 +341,161 @@ numbering comes from include/dm/uclass.h. To add a new uclass, add to the
end of the enum there, then declare your uclass as above.
+Driver Lifecycle
+----------------
+
+Here are the stages that a device goes through in driver model. Note that all
+methods mentioned here are optional - e.g. if there is no probe() method for
+a device then it will not be called. A simple device may have very few
+methods actually defined.
+
+1. Bind stage
+
+A device and its driver are bound using one of these two methods:
+
+ - Scan the U_BOOT_DEVICE() definitions. U-Boot It looks up the
+name specified by each, to find the appropriate driver. It then calls
+device_bind() to create a new device and bind' it to its driver. This will
+call the device's bind() method.
+
+ - Scan through the device tree definitions. U-Boot looks at top-level
+nodes in the the device tree. It looks at the compatible string in each node
+and uses the of_match part of the U_BOOT_DRIVER() structure to find the
+right driver for each node. It then calls device_bind() to bind the
+newly-created device to its driver (thereby creating a device structure).
+This will also call the device's bind() method.
+
+At this point all the devices are known, and bound to their drivers. There
+is a 'struct udevice' allocated for all devices. However, nothing has been
+activated (except for the root device). Each bound device that was created
+from a U_BOOT_DEVICE() declaration will hold the platdata pointer specified
+in that declaration. For a bound device created from the device tree,
+platdata will be NULL, but of_offset will be the offset of the device tree
+node that caused the device to be created. The uclass is set correctly for
+the device.
+
+The device's bind() method is permitted to perform simple actions, but
+should not scan the device tree node, not initialise hardware, nor set up
+structures or allocate memory. All of these tasks should be left for
+the probe() method.
+
+Note that compared to Linux, U-Boot's driver model has a separate step of
+probe/remove which is independent of bind/unbind. This is partly because in
+U-Boot it may be expensive to probe devices and we don't want to do it until
+they are needed, or perhaps until after relocation.
+
+2. Activation/probe
+
+When a device needs to be used, U-Boot activates it, by following these
+steps (see device_probe()):
+
+ a. If priv_auto_alloc_size is non-zero, then the device-private space
+ is allocated for the device and zeroed. It will be accessible as
+ dev->priv. The driver can put anything it likes in there, but should use
+ it for run-time information, not platform data (which should be static
+ and known before the device is probed).
+
+ b. If platdata_auto_alloc_size is non-zero, then the platform data space
+ is allocated. This is only useful for device tree operation, since
+ otherwise you would have to specific the platform data in the
+ U_BOOT_DEVICE() declaration. The space is allocated for the device and
+ zeroed. It will be accessible as dev->platdata.
+
+ c. If the device's uclass specifies a non-zero per_device_auto_alloc_size,
+ then this space is allocated and zeroed also. It is allocated for and
+ stored in the device, but it is uclass data. owned by the uclass driver.
+ It is possible for the device to access it.
+
+ d. All parent devices are probed. It is not possible to activate a device
+ unless its predecessors (all the way up to the root device) are activated.
+ This means (for example) that an I2C driver will require that its bus
+ be activated.
+
+ e. If the driver provides a ofdata_to_platdata() method, then this is
+ called to convert the device tree data into platform data. This should
+ do various calls like fdtdec_get_int(gd->fdt_blob, dev->of_offset, ...)
+ to access the node and store the resulting information into dev->platdata.
+ After this point, the device works the same way whether it was bound
+ using a device tree node or U_BOOT_DEVICE() structure. In either case,
+ the platform data is now stored in the platdata structure. Typically you
+ will use the platdata_auto_alloc_size feature to specify the size of the
+ platform data structure, and U-Boot will automatically allocate and zero
+ it for you before entry to ofdata_to_platdata(). But if not, you can
+ allocate it yourself in ofdata_to_platdata(). Note that it is preferable
+ to do all the device tree decoding in ofdata_to_platdata() rather than
+ in probe(). (Apart from the ugliness of mixing configuration and run-time
+ data, one day it is possible that U-Boot will cache platformat data for
+ devices which are regularly de/activated).
+
+ f. The device's probe() method is called. This should do anything that
+ is required by the device to get it going. This could include checking
+ that the hardware is actually present, setting up clocks for the
+ hardware and setting up hardware registers to initial values. The code
+ in probe() can access:
+
+ - platform data in dev->platdata (for configuration)
+ - private data in dev->priv (for run-time state)
+ - uclass data in dev->uclass_priv (for things the uclass stores
+ about this device)
+
+ Note: If you don't use priv_auto_alloc_size then you will need to
+ allocate the priv space here yourself. The same applies also to
+ platdata_auto_alloc_size. Remember to free them in the remove() method.
+
+ g. The device is marked 'activated'
+
+ h. The uclass's post_probe() method is called, if one exists. This may
+ cause the uclass to do some housekeeping to record the device as
+ activated and 'known' by the uclass.
+
+3. Running stage
+
+The device is now activated and can be used. From now until it is removed
+all of the above structures are accessible. The device appears in the
+uclass's list of devices (so if the device is in UCLASS_GPIO it will appear
+as a device in the GPIO uclass). This is the 'running' state of the device.
+
+4. Removal stage
+
+When the device is no-longer required, you can call device_remove() to
+remove it. This performs the probe steps in reverse:
+
+ a. The uclass's pre_remove() method is called, if one exists. This may
+ cause the uclass to do some housekeeping to record the device as
+ deactivated and no-longer 'known' by the uclass.
+
+ b. All the device's children are removed. It is not permitted to have
+ an active child device with a non-active parent. This means that
+ device_remove() is called for all the children recursively at this point.
+
+ c. The device's remove() method is called. At this stage nothing has been
+ deallocated so platform data, private data and the uclass data will all
+ still be present. This is where the hardware can be shut down. It is
+ intended that the device be completely inactive at this point, For U-Boot
+ to be sure that no hardware is running, it should be enough to remove
+ all devices.
+
+ d. The device memory is freed (platform data, private data, uclass data).
+
+ Note: for a U_BOOT_DEVICE() declaration, the platform data is supplied as
+ a static pointer and is not allocated. For device tree, the platform
+ data is allocated during activation and freed during dectivation,
+ typically automatically using platdata_auto_alloc_size. But if that value
+ is 0 then U-Boot will not do the allocation/freeing and you will need to
+ do this yourself in your ofdata_to_platdata() and remove() methods. This
+ difference is tracked by the device's DM_FLAG_ALLOC_PDATA flag.
+
+ e. The device is marked inactive. Note that it is still bound, so the
+ device structure itself is not freed at this point. Should the device be
+ activated again, then the cycle starts again at step 2 above.
+
+5. Unbind stage
+
+The device is unbound. This is the step that actually destroys the device.
+If a parent has children these will be destroyed first. After this point
+the device does not exist and its memory has be deallocated.
+
+
Data Structures
---------------
@@ -310,7 +511,7 @@ Changes since v1
For the record, this implementation uses a very similar approach to the
original patches, but makes at least the following changes:
-- Tried to agressively remove boilerplate, so that for most drivers there
+- Tried to aggressively remove boilerplate, so that for most drivers there
is little or no 'driver model' code to write.
- Moved some data from code into data structure - e.g. store a pointer to
the driver operations structure in the driver, rather than passing it
--
2.0.0.526.g5318336
^ permalink raw reply related [flat|nested] 22+ messages in thread* [U-Boot] [PATCH v5 15/15] dm: Expand and improve the device lifecycle docs
2014-06-10 23:53 ` [U-Boot] [PATCH v5 15/15] dm: Expand and improve the device lifecycle docs Simon Glass
@ 2014-06-11 14:19 ` Jon Loeliger
2014-06-12 2:09 ` Simon Glass
0 siblings, 1 reply; 22+ messages in thread
From: Jon Loeliger @ 2014-06-11 14:19 UTC (permalink / raw)
To: u-boot
On Tue, Jun 10, 2014 at 6:53 PM, Simon Glass <sjg@chromium.org> wrote:
> The lifecycle of a device is an important part of driver model. Add to the
> existing documentation and clarify it.
>
> Reported-by: Jon Loeliger <jdl@jdl.com>
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Thanks for Jon Loeliger <jdl@jdl.com> for helping with the text and
> suggesting improvements.
>
> (Jon please comment/adjust to help clarify things further)
This is way betterer now. Thanks!
Nit typo:
> + e. If the driver provides a ofdata_to_platdata() method, then this is
s/a/an/
> +
> + Note: for a U_BOOT_DEVICE() declaration, the platform data is supplied as
> + a static pointer and is not allocated. For device tree, the platform
> + data is allocated during activation and freed during dectivation,
> + typically automatically using platdata_auto_alloc_size. But if that value
> + is 0 then U-Boot will not do the allocation/freeing and you will need to
> + do this yourself in your ofdata_to_platdata() and remove() methods. This
> + difference is tracked by the device's DM_FLAG_ALLOC_PDATA flag.
The first sentence in that paragraph confused me because I knew where it was
supposed to be headed: namely, the deallocation of the platdata. So when it
used the "not allocated" phrase, I was taken aback.
How about something like this instead?:
Note: Because the platform data for a U_BOOT_DEVICE() is defined with a
static pointer, it is not de-allocated during the remove() method. For a device
instantiated using the device tree data, the platform data will be dynamically
allocated, and thus needs to be deallocated during the remove() method.
If the platdata_auto_alloc_size is non-zero, the deallocation happens
automatically
within the DM core. However, when platdata_auto_alloc_size is 0, both the
allocation (in probe() or preferably ofdata_to_platdata()) and the deallocation
in remove() are the responsibility of the driver author.
If you'd like:
Acked-by: Jon.Loeliger <jdl@jdl.com>
Thanks,
jdl
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v5 15/15] dm: Expand and improve the device lifecycle docs
2014-06-11 14:19 ` Jon Loeliger
@ 2014-06-12 2:09 ` Simon Glass
0 siblings, 0 replies; 22+ messages in thread
From: Simon Glass @ 2014-06-12 2:09 UTC (permalink / raw)
To: u-boot
Hi Jon,
On 11 June 2014 10:19, Jon Loeliger <loeliger@gmail.com> wrote:
> On Tue, Jun 10, 2014 at 6:53 PM, Simon Glass <sjg@chromium.org> wrote:
>> The lifecycle of a device is an important part of driver model. Add to the
>> existing documentation and clarify it.
>>
>> Reported-by: Jon Loeliger <jdl@jdl.com>
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>> Thanks for Jon Loeliger <jdl@jdl.com> for helping with the text and
>> suggesting improvements.
>>
>> (Jon please comment/adjust to help clarify things further)
>
>
> This is way betterer now. Thanks!
>
>
> Nit typo:
>
>> + e. If the driver provides a ofdata_to_platdata() method, then this is
>
> s/a/an/
>
>
>
>> +
>> + Note: for a U_BOOT_DEVICE() declaration, the platform data is supplied as
>> + a static pointer and is not allocated. For device tree, the platform
>> + data is allocated during activation and freed during dectivation,
>> + typically automatically using platdata_auto_alloc_size. But if that value
>> + is 0 then U-Boot will not do the allocation/freeing and you will need to
>> + do this yourself in your ofdata_to_platdata() and remove() methods. This
>> + difference is tracked by the device's DM_FLAG_ALLOC_PDATA flag.
>
> The first sentence in that paragraph confused me because I knew where it was
> supposed to be headed: namely, the deallocation of the platdata. So when it
> used the "not allocated" phrase, I was taken aback.
>
> How about something like this instead?:
>
> Note: Because the platform data for a U_BOOT_DEVICE() is defined with a
> static pointer, it is not de-allocated during the remove() method. For a device
> instantiated using the device tree data, the platform data will be dynamically
> allocated, and thus needs to be deallocated during the remove() method.
> If the platdata_auto_alloc_size is non-zero, the deallocation happens
> automatically
> within the DM core. However, when platdata_auto_alloc_size is 0, both the
> allocation (in probe() or preferably ofdata_to_platdata()) and the deallocation
> in remove() are the responsibility of the driver author.
Thanks, this reads well.
>
> If you'd like:
>
> Acked-by: Jon.Loeliger <jdl@jdl.com>
>
> Thanks,
> jdl
Regards,
Simon
^ permalink raw reply [flat|nested] 22+ messages in thread
* [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs
2014-06-10 23:53 [U-Boot] [PATCH v5 0/15] Collected driver model bug-fixes and docs Simon Glass
` (14 preceding siblings ...)
2014-06-10 23:53 ` [U-Boot] [PATCH v5 15/15] dm: Expand and improve the device lifecycle docs Simon Glass
@ 2014-06-11 14:26 ` Jon Loeliger
15 siblings, 0 replies; 22+ messages in thread
From: Jon Loeliger @ 2014-06-11 14:26 UTC (permalink / raw)
To: u-boot
On Tue, Jun 10, 2014 at 6:53 PM, Simon Glass <sjg@chromium.org> wrote:
> This series collects some of the patches from the Tegra GPIO conversion
> to driver model. That work is still in progress, but the bug fixes and
> iotracing feature should go into this release I think. Also the
> documentation improvements may as well follow since the existings docs
> are proven inadequate.
>
> Changes in v5:
> - Fix a few more typos
Hi Simon,
Thanks for gathering all these patches. I definitely
think they should hit the shiny, new DM repo!
Thanks,
jdl
^ permalink raw reply [flat|nested] 22+ messages in thread