[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

At present we have a lot of ad-hoc init functions related to boards, for
example board_early_init_f(), board_misc_init_f() and dram_init().

There are used in different ways by different boards as useful hooks to
do the required init and sequence it correctly. Some functions are always
enabled but have a __weak default. Some are controlled by the existence
of a CONFIG.

There are two main init sequences: board_init_f() (f for running from
read-only flash) which runs before relocation and board_init_r() (r for
relocated) which runs afterwards.

One problem with the current sequence is that it has a lot of
arch-specific #ifdefs around various functions. There are also #ifdefs
for various features. There has been quite a bit of discussion about how
to tidy this up and at least one RFC series[1].

Now that we have driver model we can use this to deal with the init
sequences. This approach has several advantages:

- We have a path to remove the #ifdefs
- It is easy for multiple parts of the code to implement the same hook
- We can track what is called and what is not
- We don't need weak functions
- We can eventually adjust the sequence to improve naming or to add new
init phases
- It provides a model for how we might deal with ft_board_setup() and
friends

This series starts the process of replacing the pre-relocation init
sequence with a driver-model solution. It defines a uclass, adds tests
and converts sandbox and a few x86 boards over to use this new setup.

This series is not ready for use yet as the rest of the init sequence
hooks need to be converted. But there is enough here to show the idea.

Comments welcome.

[1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html


Simon Glass (16):
  x86: Drop leading spaces in cpu_x86_get_desc()
  x86: Display the SPL banner only once
  x86: config: Enable dhrystone command for link
  dm: board: Add a uclass for init functions
  dm: board: Add a command to view phase info
  dm: board: Adjust pre-relocation init hooks
  dm: board: Support printing CPU info using the uclass
  dm: sandbox: Convert to using driver-model baord init
  dm: board: Add tests for the board uclass
  dm: board: Add documentation
  dm: x86: board: Enable CONFIG_BOARD
  dm: x86: board: Add a BOARD_F_RESERVE_ARCH driver
  dm: x86: board: ivybridge: Add support for CONFIG_BOARD
  dm: x86: board: ivybridge: Switch to use CONFIG_BOARD
  dm: x86: board: ivybridge: Remove old board init code
  dm: board: Add information about the new board init to the README

 README                                            |   3 +
 arch/Kconfig                                      |   5 +
 arch/sandbox/dts/test.dts                         |  12 ++
 arch/sandbox/include/asm/state.h                  |   3 +
 arch/sandbox/include/asm/test.h                   |   9 ++
 arch/x86/cpu/coreboot/coreboot.c                  |   2 +
 arch/x86/cpu/cpu.c                                |  41 ++++-
 arch/x86/cpu/cpu_x86.c                            |   6 +-
 arch/x86/cpu/ivybridge/cpu.c                      |  57 +++++--
 arch/x86/cpu/ivybridge/sdram.c                    |   6 +-
 arch/x86/cpu/ivybridge/sdram_nop.c                |  36 ++++-
 arch/x86/cpu/x86_64/cpu.c                         |   2 +
 arch/x86/include/asm/arch-ivybridge/sandybridge.h |   7 +
 arch/x86/lib/spl.c                                |  11 +-
 board/google/chromebook_link/link.c               |   5 -
 board/google/chromebox_panther/panther.c          |   5 -
 board/sandbox/sandbox.c                           |  44 +++++-
 cmd/Kconfig                                       |   8 +
 cmd/Makefile                                      |   1 +
 cmd/board.c                                       |  58 +++++++
 common/Kconfig                                    |  31 ++++
 common/board_f.c                                  |  58 ++++++-
 common/init/Makefile                              |   1 +
 common/init/board-uclass.c                        | 108 +++++++++++++
 configs/chromebook_link64_defconfig               |   2 +
 configs/chromebook_link_defconfig                 |   2 +
 configs/chromebox_panther_defconfig               |   1 +
 doc/driver-model/board-info.txt                   | 181 ++++++++++++++++++++++
 drivers/cpu/cpu-uclass.c                          |  19 +++
 drivers/misc/Makefile                             |   1 +
 drivers/misc/board_sandbox.c                      |  44 ++++++
 include/asm-generic/global_data.h                 |   5 +
 include/board.h                                   | 170 ++++++++++++++++++++
 include/cpu.h                                     |   5 +
 include/dm/uclass-id.h                            |   1 +
 test/dm/Makefile                                  |   1 +
 test/dm/board.c                                   |  74 +++++++++
 37 files changed, 980 insertions(+), 45 deletions(-)
 create mode 100644 cmd/board.c
 create mode 100644 common/init/board-uclass.c
 create mode 100644 doc/driver-model/board-info.txt
 create mode 100644 drivers/misc/board_sandbox.c
 create mode 100644 include/board.h
 create mode 100644 test/dm/board.c

-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 01/16] x86: Drop leading spaces in cpu_x86_get_desc()

[Simon Glass]

The Intel CPU name can have leading spaces. Remove them since they are not
useful.

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

 arch/x86/cpu/cpu_x86.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/cpu/cpu_x86.c b/arch/x86/cpu/cpu_x86.c
index 8be14b5929..b465b14a94 100644
--- a/arch/x86/cpu/cpu_x86.c
+++ b/arch/x86/cpu/cpu_x86.c
@@ -41,10 +41,14 @@ int cpu_x86_get_vendor(struct udevice *dev, char *buf, int size)
 
 int cpu_x86_get_desc(struct udevice *dev, char *buf, int size)
 {
+	char *ptr;
+
 	if (size < CPU_MAX_NAME_LEN)
 		return -ENOSPC;
 
-	cpu_get_name(buf);
+	ptr = cpu_get_name(buf);
+	if (ptr != buf)
+		strcpy(buf, ptr);
 
 	return 0;
 }
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 02/16] x86: Display the SPL banner only once

[Simon Glass]

At present on a cold reboot we must reset the CPU to get it to full speed.
With 64-bit U-Boot this happens in SPL. At present we print the banner
before doing this, the end result being that we print the banner twice.
Print the banner a little later (after the CPU is ready) to avoid this.

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

 arch/x86/lib/spl.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index ed2d40b552..fa93d64a7a 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -37,8 +37,6 @@ static int x86_spl_init(void)
 		debug("%s: spl_init() failed\n", __func__);
 		return ret;
 	}
-	preloader_console_init();
-
 	ret = arch_cpu_init();
 	if (ret) {
 		debug("%s: arch_cpu_init() failed\n", __func__);
@@ -49,6 +47,7 @@ static int x86_spl_init(void)
 		debug("%s: arch_cpu_init_dm() failed\n", __func__);
 		return ret;
 	}
+	preloader_console_init();
 	ret = print_cpuinfo();
 	if (ret) {
 		debug("%s: print_cpuinfo() failed\n", __func__);
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 03/16] x86: config: Enable dhrystone command for link

[Simon Glass]

Enable this command so we can get an approximate performance measurement.

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

 configs/chromebook_link64_defconfig | 1 +
 configs/chromebook_link_defconfig   | 1 +
 2 files changed, 2 insertions(+)

diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig
index 3ab34cd72b..749cfd43b0 100644
--- a/configs/chromebook_link64_defconfig
+++ b/configs/chromebook_link64_defconfig
@@ -86,4 +86,5 @@ CONFIG_FRAMEBUFFER_VESA_MODE_11A=y
 CONFIG_VIDEO_IVYBRIDGE_IGD=y
 CONFIG_CONSOLE_SCROLL_LINES=5
 CONFIG_USE_PRIVATE_LIBGCC=y
+CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig
index 85b7d5fcd9..5ebb556f90 100644
--- a/configs/chromebook_link_defconfig
+++ b/configs/chromebook_link_defconfig
@@ -69,4 +69,5 @@ CONFIG_FRAMEBUFFER_VESA_MODE_11A=y
 CONFIG_VIDEO_IVYBRIDGE_IGD=y
 CONFIG_CONSOLE_SCROLL_LINES=5
 CONFIG_USE_PRIVATE_LIBGCC=y
+CONFIG_CMD_DHRYSTONE=y
 CONFIG_TPM=y
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 04/16] dm: board: Add a uclass for init functions

[Simon Glass]

Add a uclass to handle board init. This allows drivers to be provided to
perform the various phases of init. Functions are provided to call all
devices that can handle a particular phase.

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

 common/Kconfig                    |  31 +++++++
 common/init/Makefile              |   1 +
 common/init/board-uclass.c        | 108 ++++++++++++++++++++++++
 include/asm-generic/global_data.h |   5 ++
 include/board.h                   | 170 ++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h            |   1 +
 6 files changed, 316 insertions(+)
 create mode 100644 common/init/board-uclass.c
 create mode 100644 include/board.h

diff --git a/common/Kconfig b/common/Kconfig
index 8f73c8f757..7d2bd15371 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -395,6 +395,36 @@ config DISPLAY_BOARDINFO
 
 menu "Start-up hooks"
 
+config BOARD
+	bool "Support using driver model for board start-up hooks, etc."
+	help
+	  This adds support for the board uclass and associated functions. With
+	  this you can create and use BOARD drivers. However unless
+	  BOARD_ENABLE is also set, the existing init sequence will continue
+	  to be used. This is designed to ease migration of boards, since
+	  support for both methods can be provided during the transition
+	  period.
+
+config SPL_BOARD
+	bool "Support using driver model for board start-up hooks, etc. in SPL"
+	help
+	  This adds support for the board uclass and associated functions in
+	  SPL. With this you can create and use BOARD drivers. However unless
+	  BOARD_ENABLE is also set, the existing init sequence will continue
+	  to be used. This is designed to ease migration of boards, since
+	  support for both methods can be provided during the transition
+	  period.
+
+config BOARD_ENABLE
+	bool "Use driver model instead of ad-hoc board init functions"
+	depends on BOARD
+	help
+	  This replaces the ad-hoc start-up functions like board_early_init_f()
+	  with a driver-model-based interface. With this enabled, boards
+	  provide one or more drivers which implement these phases of init as
+	  well as access to the board decription. Existing init functions are
+	  no-longer called.
+
 config ARCH_EARLY_INIT_R
 	bool "Call arch-specific init soon after relocation"
 	default y if X86
@@ -414,6 +444,7 @@ config ARCH_MISC_INIT
 
 config BOARD_EARLY_INIT_F
 	bool "Call board-specific init before relocation"
+	depends on !BOARD_ENABLE
 	default y if X86
 	help
 	  Some boards need to perform initialisation as soon as possible
diff --git a/common/init/Makefile b/common/init/Makefile
index 4902635f53..923ce8e139 100644
--- a/common/init/Makefile
+++ b/common/init/Makefile
@@ -5,3 +5,4 @@
 #
 
 obj-y += board_init.o
+obj-$(CONFIG_$(SPL_)BOARD) += board-uclass.o
diff --git a/common/init/board-uclass.c b/common/init/board-uclass.c
new file mode 100644
index 0000000000..2ca65f44da
--- /dev/null
+++ b/common/init/board-uclass.c
@@ -0,0 +1,108 @@
+/*
+ * Board driver interface
+ *
+ * Copyright (c) 2017 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <board.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int board_phase(struct udevice *dev, enum board_phase_t phase)
+{
+	struct board_uc_priv *priv = dev_get_uclass_priv(dev);
+	struct board_ops *ops = board_get_ops(dev);
+
+	if (!ops->phase)
+		return -ENOSYS;
+	if (!priv->phase_mask && phase == BOARD_PHASE_FIRST) {
+		printf("Device '%s' supports no phases\n", dev->name);
+		return -EINVAL;
+	}
+	if (!(priv->phase_mask & board_phase_mask(phase)))
+		return -ENOSYS;
+
+	return ops->phase(dev, phase);
+}
+
+int board_walk_phase_count(enum board_phase_t phase, bool verbose)
+{
+	struct udevice *dev;
+	int count = 0;
+	int ret;
+
+	for (ret = uclass_first_device(UCLASS_BOARD, &dev);
+	     dev;
+	     uclass_next_device(&dev)) {
+		ret = board_phase(dev, phase);
+		if (ret == 0) {
+			count++;
+		} else if (ret == BOARD_PHASE_CLAIMED) {
+			count++;
+			debug("Phase %d claimed by '%s'\n", phase, dev->name);
+			break;
+		} else if (ret != -ENOSYS) {
+			gd->phase_count[phase] += count;
+			return ret;
+		}
+	}
+
+	if (!count) {
+		if (verbose)
+			printf("Unable to find driver for phase %d\n", phase);
+		return -ENOSYS;
+	}
+	gd->phase_count[phase] += count;
+
+	return count;
+}
+
+int board_walk_phase(enum board_phase_t phase)
+{
+	int ret;
+
+	ret = board_walk_phase_count(phase, true);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+int board_walk_opt_phase(enum board_phase_t phase)
+{
+	int ret;
+
+	ret = board_walk_phase_count(phase, false);
+	if (ret < 0 && ret != -ENOSYS)
+		return ret;
+
+	return 0;
+}
+
+int board_support_phase(struct udevice *dev, enum board_phase_t phase)
+{
+	struct board_uc_priv *priv = dev_get_uclass_priv(dev);
+
+	priv->phase_mask |= board_phase_mask(phase);
+
+	return 0;
+}
+
+int board_support_phase_mask(struct udevice *dev, ulong phase_mask)
+{
+	struct board_uc_priv *priv = dev_get_uclass_priv(dev);
+
+	priv->phase_mask = phase_mask;
+
+	return 0;
+}
+
+UCLASS_DRIVER(board) = {
+	.id		= UCLASS_BOARD,
+	.name		= "board",
+	.per_device_auto_alloc_size	= sizeof(struct board_uc_priv),
+};
diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 5b356dd231..fea1e916ed 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -21,6 +21,7 @@
  */
 
 #ifndef __ASSEMBLY__
+#include <board.h>
 #include <membuff.h>
 #include <linux/list.h>
 
@@ -107,6 +108,10 @@ typedef struct global_data {
 	ulong video_top;		/* Top of video frame buffer area */
 	ulong video_bottom;		/* Bottom of video frame buffer area */
 #endif
+#ifdef CONFIG_BOARD
+	/* number of drivers which handled each phase */
+	uint8_t phase_count[BOARD_PHASE_COUNT];
+#endif
 } gd_t;
 #endif
 
diff --git a/include/board.h b/include/board.h
new file mode 100644
index 0000000000..0975f5ac12
--- /dev/null
+++ b/include/board.h
@@ -0,0 +1,170 @@
+/*
+ * Board driver interface
+ *
+ * Copyright (c) 2017 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __BOARD_H
+#define __BOARD_H
+
+/* Include dm.h here because board.h comes before dm.h in include order */
+#include <dm.h>
+
+/*
+ * This file provides access to board drivers, which are responsible for
+ * initing the board as well as (in future) querying its state.
+ */
+
+/* Phases of init - please update phase_name also */
+enum board_phase_t {
+	/*
+	 * Pre-relocation phases.
+	 * TODO(sjg at chromium.org): At present these are named the same as the
+	 * functions they replace to avoid confusion. However this naming is
+	 * not very consistent. At some point once enough boards uses this
+	 * interface, we could rename some of these.
+	 *
+	 * TODO(sjg at chromium.org): arch_cpu_init() and mach_cpu_init() are
+	 * called before driver model is ready so we cannot yet add them to
+	 * this interface. Hopefully this can be adjusted later:
+	 *   BOARD_F_ARCH_CPU_INIT,
+	 *   BOARD_F_MACH_CPU_INIT,
+	 */
+	BOARD_PHASE_FIRST,
+	BOARD_F_ARCH_CPU_INIT_DM = BOARD_PHASE_FIRST,
+	BOARD_F_EARLY_INIT_F,
+	BOARD_F_CHECKCPU,
+	BOARD_F_MISC_INIT_F,
+	BOARD_F_DRAM_INIT,
+	BOARD_F_RESERVE_ARCH,
+
+	/*
+	 * Post-relocation phases go here:
+	 *
+	 * BOARD_R_...
+	 */
+
+	BOARD_PHASE_TEST,	/* For sandbox testing */
+	BOARD_PHASE_COUNT,
+	BOARD_PHASE_INVALID,	/* For sandbox testing */
+};
+
+static inline ulong board_phase_mask(enum board_phase_t phase)
+{
+	return 1UL << (ulong)phase;
+}
+
+/*
+ * Return this from phase() to indicate that no more devices should handle this
+ * phase
+ */
+#define BOARD_PHASE_CLAIMED EUSERS
+
+/* Operations for the board driver */
+struct board_ops {
+	/**
+	* phase() - Execute a phase of board init
+	*
+	 * @dev:	Device to use
+	* @phase:	Phase to execute
+	* @return 0 if done, -ENOSYS if not supported (which is often fine),
+		BOARD_PHASE_CLAIMED if this was handled and that processing
+		of this phase should stop (i.e. do not send it to other
+		devices), other error if something went wrong
+	*/
+	int (*phase)(struct udevice *dev, enum board_phase_t phase);
+
+	/**
+	 * get_desc() - Get a description string for a board
+	 *
+	 * @dev:	Device to check (UCLASS_BOARD)
+	 * @buf:	Buffer to place string
+	 * @size:	Size of string space
+	 * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
+	 */
+	int (*get_desc)(struct udevice *dev, char *buf, int size);
+};
+
+#define board_get_ops(dev)        ((struct board_ops *)(dev)->driver->ops)
+
+/* Private uclass information about each device */
+struct board_uc_priv {
+	ulong phase_mask;	/* Mask of phases supported by this device */
+};
+
+/**
+ * board_phase() - Execute a phase of board init on a device
+ *
+ * @phase:	Phase to execute
+ * @return 0 if done, -ENOSYS if not supported by this device, other error if
+ *	something went wrong
+ */
+int board_phase(struct udevice *dev, enum board_phase_t phase);
+
+/**
+ * board_walk_phase() - Execute a phase of board init
+ *
+ * This works through the available board devices asking each one to perform
+ * the requested init phase. The process continues until there are no more
+ * board devices.
+ *
+ * If no board device provides the phase, this function returns -ENOSYS.
+ *
+ * @phase:	Phase to execute
+ * @return 0 if done, -ENOSYS if not supported, other error if something went
+ *	wrong
+ */
+int board_walk_phase(enum board_phase_t phase);
+
+/**
+ * board_opt_walk_phase() - Execute an optional phase of board init
+ *
+ * This works through the available board devices asking each one to perform
+ * the requested init phase. The process continues until there are no more
+ * board devices.
+ *
+ * If no board device provides the phase, this function returns 0.
+ *
+ * @phase:	Phase to execute
+ * @return 0 if done, other error if something went wrong
+ */
+int board_walk_opt_phase(enum board_phase_t phase);
+
+/**
+ * board_walk_phase_count() - Execute an optional phase of board init
+ *
+ * This works through the available board devices asking each one to perform
+ * the requested init phase. The process continues until there are no more
+ * board devices.
+ *
+ * If no board provides the phase, this function returns -ENOSYS.
+ *
+ * @phase:	Phase to execute
+ * @return number of devices which handled this phase if done, -ve error if
+ *	something went wrong
+ */
+int board_walk_phase_count(enum board_phase_t phase, bool verbose);
+
+/**
+ * board_support_phase() - Mark a board device as supporting the given phase
+ *
+ * @dev:	Board device
+ * @phase:	Phase to execute
+ * @return 0
+ */
+int board_support_phase(struct udevice *dev, enum board_phase_t phase);
+
+/**
+ * board_support_phase_mask() - Mark a board device as supporting given phases
+ *
+ * @dev:	Board device
+ * @phase_mask:	Mask of phases to execute, built up by ORing board_phase_mask()
+ *		values together
+ * @return 0
+ */
+int board_support_phase_mask(struct udevice *dev, ulong phase_mask);
+
+#endif
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 8c92d0b030..166194fead 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -27,6 +27,7 @@ enum uclass_id {
 	/* U-Boot uclasses start here - in alphabetical order */
 	UCLASS_ADC,		/* Analog-to-digital converter */
 	UCLASS_AHCI,		/* SATA disk controller */
+	UCLASS_BOARD,		/* Board-specific driver */
 	UCLASS_BLK,		/* Block device */
 	UCLASS_CLK,		/* Clock source, e.g. used by peripherals */
 	UCLASS_CPU,		/* CPU, typically part of an SoC */
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 05/16] dm: board: Add a command to view phase info

[Simon Glass]

Add a 'board phases' command ('boa' or 'boa p' for short) which shows
which board phases have been run and how many devices provided an
implementation of each phase. Sample output:

=> board phases
  1 arch_cpu_init_dm
  0 board_early_init_f
  1 checkcpu
  0 misc_init_f
  1 dram_init
  1 reserve_arch

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

 cmd/Kconfig  |  8 ++++++++
 cmd/Makefile |  1 +
 cmd/board.c  | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 cmd/board.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 25e3b783a8..9b93f213b7 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -126,6 +126,14 @@ config CMD_BDI
 	help
 	  Print board info
 
+config CMD_BOARD
+	bool "board"
+	default y if BOARD
+	help
+	  Provides access to board-specific drivers. This includes information
+	  about which board init was completed as well as the board
+	  description.
+
 config CMD_CONFIG
 	bool "config"
 	select BUILD_BIN2C
diff --git a/cmd/Makefile b/cmd/Makefile
index f13bb8c11e..4b1d183514 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -16,6 +16,7 @@ obj-y += version.o
 obj-$(CONFIG_CMD_AES) += aes.o
 obj-$(CONFIG_CMD_AMBAPP) += ambapp.o
 obj-$(CONFIG_CMD_ARMFLASH) += armflash.o
+obj-$(CONFIG_CMD_BOARD) += board.o
 obj-$(CONFIG_SOURCE) += source.o
 obj-$(CONFIG_CMD_SOURCE) += source.o
 obj-$(CONFIG_CMD_BDI) += bdinfo.o
diff --git a/cmd/board.c b/cmd/board.c
new file mode 100644
index 0000000000..3ee91ca25d
--- /dev/null
+++ b/cmd/board.c
@@ -0,0 +1,58 @@
+/*
+ * Board driver commands
+ *
+ * Copyright (c) 2017 Google, Inc
+ * Written by Simon Glass <sjg@chromium.org>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <board.h>
+#include <command.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static const char * const phase_name[] = {
+	[BOARD_F_ARCH_CPU_INIT_DM] = "arch_cpu_init_dm",
+	[BOARD_F_EARLY_INIT_F] = "board_early_init_f",
+	[BOARD_F_CHECKCPU] = "checkcpu",
+	[BOARD_F_MISC_INIT_F] = "misc_init_f",
+	[BOARD_F_DRAM_INIT] = "dram_init",
+	[BOARD_F_RESERVE_ARCH] = "reserve_arch",
+	[BOARD_PHASE_TEST] = "test",
+	[BOARD_PHASE_INVALID] = "invalid",
+
+};
+
+static void board_list_phases(void)
+{
+	enum board_phase_t phase;
+
+	for (phase = BOARD_PHASE_FIRST; phase < BOARD_PHASE_TEST; phase++)
+		printf("%3d %s\n", gd->phase_count[phase], phase_name[phase]);
+}
+
+static int do_board(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	int cmd = 'p';
+
+	if (argc > 1)
+		cmd = argv[1][0];
+
+	switch (cmd) {
+	case 'p': /* phases */
+		board_list_phases();
+		break;
+	default:
+		return CMD_RET_FAILURE;
+	}
+
+	return 0;
+}
+
+U_BOOT_CMD(
+	board,	2,	0,	do_board,
+	"Access board information",
+	"phases\t- Show information about completed board init phases"
+);
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 06/16] dm: board: Adjust pre-relocation init hooks

[Simon Glass]

When CONFIG_BOARD_ENABLED is enabled, replace the existing ad-hoc hooks
with ones based on driver model. These call devices to handle each phase
of init.

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

 common/board_f.c | 47 ++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 42 insertions(+), 5 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c
index 7d1ede0404..df9a64a20f 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -11,7 +11,8 @@
  */
 
 #include <common.h>
-#include <linux/compiler.h>
+#include <board.h>
+#include <cpu.h>
 #include <version.h>
 #include <console.h>
 #include <environment.h>
@@ -819,6 +820,40 @@ static int initf_dm(void)
 	return 0;
 }
 
+#ifdef CONFIG_BOARD_ENABLE
+
+int arch_cpu_init_dm(void)
+{
+	return board_walk_opt_phase(BOARD_F_ARCH_CPU_INIT_DM);
+}
+
+int board_early_init_f(void)
+{
+	return board_walk_opt_phase(BOARD_F_EARLY_INIT_F);
+}
+
+int checkcpu(void)
+{
+	return board_walk_opt_phase(BOARD_F_CHECKCPU);
+}
+
+int misc_init_f(void)
+{
+	return board_walk_opt_phase(BOARD_F_MISC_INIT_F);
+}
+
+int dram_init(void)
+{
+	return board_walk_phase(BOARD_F_DRAM_INIT);
+}
+
+int reserve_arch(void)
+{
+	return board_walk_opt_phase(BOARD_F_RESERVE_ARCH);
+}
+
+#else
+
 /* Architecture-specific memory reservation */
 __weak int reserve_arch(void)
 {
@@ -829,6 +864,7 @@ __weak int arch_cpu_init_dm(void)
 {
 	return 0;
 }
+#endif /* !CONFIG_BOARD_ENABLE_ENABLED */
 
 static const init_fnc_t init_sequence_f[] = {
 #ifdef CONFIG_SANDBOX
@@ -851,7 +887,7 @@ static const init_fnc_t init_sequence_f[] = {
 	initf_dm,
 	arch_cpu_init_dm,
 	mark_bootstage,		/* need timer, go after init dm */
-#if defined(CONFIG_BOARD_EARLY_INIT_F)
+#if defined(CONFIG_BOARD_EARLY_INIT_F) || defined(CONFIG_BOARD_ENABLE)
 	board_early_init_f,
 #endif
 	/* TODO: can any of this go into arch_cpu_init()? */
@@ -898,7 +934,8 @@ static const init_fnc_t init_sequence_f[] = {
 #if defined(CONFIG_MPC83xx)
 	prt_83xx_rsr,
 #endif
-#if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SH)
+#if defined(CONFIG_PPC) || defined(CONFIG_M68K) || defined(CONFIG_SH) || \
+		defined(CONFIG_BOARD_ENABLE)
 	checkcpu,
 #endif
 #if defined(CONFIG_DISPLAY_CPUINFO)
@@ -908,7 +945,7 @@ static const init_fnc_t init_sequence_f[] = {
 	show_board_info,
 #endif
 	INIT_FUNC_WATCHDOG_INIT
-#if defined(CONFIG_MISC_INIT_F)
+#if defined(CONFIG_MISC_INIT_F) || defined(CONFIG_BOARD_ENABLE)
 	misc_init_f,
 #endif
 	INIT_FUNC_WATCHDOG_RESET
@@ -922,7 +959,7 @@ static const init_fnc_t init_sequence_f[] = {
 	/* TODO: unify all these dram functions? */
 #if defined(CONFIG_ARM) || defined(CONFIG_X86) || defined(CONFIG_NDS32) || \
 		defined(CONFIG_MICROBLAZE) || defined(CONFIG_AVR32) || \
-		defined(CONFIG_SH)
+		defined(CONFIG_SH) || defined(CONFIG_SANDBOX)
 	dram_init,		/* configure available RAM banks */
 #endif
 #if defined(CONFIG_MIPS) || defined(CONFIG_PPC) || defined(CONFIG_M68K)
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 07/16] dm: board: Support printing CPU info using the uclass

[Simon Glass]

With driver model we can obtain CPU information by calling the CPU uclass.
This avoids ad-hoc code for each board. Change the code to do this when
CONFIG_BOARD is enabled.

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

 common/board_f.c         | 11 ++++++++++-
 drivers/cpu/cpu-uclass.c | 19 +++++++++++++++++++
 include/cpu.h            |  5 +++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/common/board_f.c b/common/board_f.c
index df9a64a20f..4d9d2f30c7 100644
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -852,6 +852,15 @@ int reserve_arch(void)
 	return board_walk_opt_phase(BOARD_F_RESERVE_ARCH);
 }
 
+int print_cpuinfo(void)
+{
+#ifdef CONFIG_CPU
+	return cpu_print_info();
+#else
+	return 0;
+#endif
+}
+
 #else
 
 /* Architecture-specific memory reservation */
@@ -938,7 +947,7 @@ static const init_fnc_t init_sequence_f[] = {
 		defined(CONFIG_BOARD_ENABLE)
 	checkcpu,
 #endif
-#if defined(CONFIG_DISPLAY_CPUINFO)
+#if defined(CONFIG_DISPLAY_CPUINFO) || defined(CONFIG_BOARD_ENABLED)
 	print_cpuinfo,		/* display cpu info (and speed) */
 #endif
 #if defined(CONFIG_DISPLAY_BOARDINFO)
diff --git a/drivers/cpu/cpu-uclass.c b/drivers/cpu/cpu-uclass.c
index c57ac16b3a..2a69ea3af3 100644
--- a/drivers/cpu/cpu-uclass.c
+++ b/drivers/cpu/cpu-uclass.c
@@ -54,6 +54,25 @@ int cpu_get_vendor(struct udevice *dev, char *buf, int size)
 	return ops->get_vendor(dev, buf, size);
 }
 
+int cpu_print_info(void)
+{
+	struct udevice *dev;
+	char name[60];
+	int ret;
+
+	ret = uclass_first_device(UCLASS_CPU, &dev);
+	if (ret)
+		return ret;
+	if (!dev)
+		return 0;
+	ret = cpu_get_desc(dev, name, sizeof(name));
+	if (ret)
+		return ret;
+	printf("CPU:   %s\n", name);
+
+	return 0;
+}
+
 U_BOOT_DRIVER(cpu_bus) = {
 	.name	= "cpu_bus",
 	.id	= UCLASS_SIMPLE_BUS,
diff --git a/include/cpu.h b/include/cpu.h
index 954257715a..2d5c373343 100644
--- a/include/cpu.h
+++ b/include/cpu.h
@@ -124,4 +124,9 @@ int cpu_get_count(struct udevice *dev);
  */
 int cpu_get_vendor(struct udevice *dev, char *buf, int size);
 
+/**
+ * cpu_print_info() - Print information about the first CPU
+ */
+int cpu_print_info(void);
+
 #endif
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 08/16] dm: sandbox: Convert to using driver-model baord init

[Simon Glass]

Adjust the existing hooks to use a driver instead.

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

 arch/Kconfig            |  3 +++
 board/sandbox/sandbox.c | 44 ++++++++++++++++++++++++++++++++++++++------
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 76c690f667..f07070db18 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -64,6 +64,9 @@ config SANDBOX
 	bool "Sandbox"
 	select BOARD_LATE_INIT
 	select SUPPORT_OF_CONTROL
+	select BOARD
+	select SPL_BOARD
+	select BOARD_ENABLE
 	select DM
 	select DM_KEYBOARD
 	select DM_SPI_FLASH
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index b41e9decb3..1eea20cbcb 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -7,6 +7,7 @@
 #include <cros_ec.h>
 #include <dm.h>
 #include <os.h>
+#include <asm/state.h>
 #include <asm/test.h>
 #include <asm/u-boot-sandbox.h>
 
@@ -41,12 +42,6 @@ unsigned long timer_read_counter(void)
 }
 #endif
 
-int dram_init(void)
-{
-	gd->ram_size = CONFIG_SYS_SDRAM_SIZE;
-	return 0;
-}
-
 #ifdef CONFIG_BOARD_LATE_INIT
 int board_late_init(void)
 {
@@ -63,3 +58,40 @@ int board_late_init(void)
 	return 0;
 }
 #endif
+
+static int sandbox_phase(struct udevice *dev, enum board_phase_t phase)
+{
+	struct sandbox_state *state = state_get_current();
+
+	switch (phase) {
+	case BOARD_F_DRAM_INIT:
+		gd->ram_size = state->ram_size;
+		return 0;
+	default:
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+static int sandbox_board_probe(struct udevice *dev)
+{
+	return board_support_phase(dev, BOARD_F_DRAM_INIT);
+}
+
+static const struct board_ops sandbox_board_ops = {
+	.phase		= sandbox_phase,
+};
+
+/* Name this starting with underscore so it will be called last */
+U_BOOT_DRIVER(_sandbox_board_drv) = {
+	.name		= "sandbox_board",
+	.id		= UCLASS_BOARD,
+	.ops		= &sandbox_board_ops,
+	.probe		= sandbox_board_probe,
+	.flags		= DM_FLAG_PRE_RELOC,
+};
+
+U_BOOT_DEVICE(sandbox_board) = {
+	.name		= "sandbox_board",
+};
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 09/16] dm: board: Add tests for the board uclass

[Simon Glass]

Add some tests which define some devices and check the operation of the
various init functions.

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

 arch/sandbox/dts/test.dts        | 12 +++++++
 arch/sandbox/include/asm/state.h |  3 ++
 arch/sandbox/include/asm/test.h  |  9 +++++
 drivers/misc/Makefile            |  1 +
 drivers/misc/board_sandbox.c     | 44 ++++++++++++++++++++++++
 test/dm/Makefile                 |  1 +
 test/dm/board.c                  | 74 ++++++++++++++++++++++++++++++++++++++++
 7 files changed, 144 insertions(+)
 create mode 100644 drivers/misc/board_sandbox.c
 create mode 100644 test/dm/board.c

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index fff175d1b7..084c3dff63 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -52,6 +52,18 @@
 		reg = <2 1>;
 	};
 
+	board-test0 {
+		compatible = "sandbox,board-test0";
+	};
+
+	board-test1 {
+		compatible = "sandbox,board-test1";
+	};
+
+	board-test2 {
+		compatible = "sandbox,board-test2";
+	};
+
 	b-test {
 		reg = <3 1>;
 		compatible = "denx,u-boot-fdt-test";
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 149f28d873..d531531d72 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -9,6 +9,7 @@
 #include <config.h>
 #include <sysreset.h>
 #include <stdbool.h>
+#include <asm/test.h>
 #include <linux/stringify.h>
 
 /**
@@ -65,6 +66,8 @@ struct sandbox_state {
 	enum state_terminal_raw term_raw;	/* Terminal raw/cooked */
 	bool skip_delays;		/* Ignore any time delays (for test) */
 	bool show_test_output;		/* Don't suppress stdout in tests */
+	/* Return values for board_sandbox */
+	int board_sandbox_ret[BOARD_TEST_COUNT];
 
 	/* Pointer to information for each SPI bus/cs */
 	struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
diff --git a/arch/sandbox/include/asm/test.h b/arch/sandbox/include/asm/test.h
index 451a78e590..12b3b9bc1e 100644
--- a/arch/sandbox/include/asm/test.h
+++ b/arch/sandbox/include/asm/test.h
@@ -79,4 +79,13 @@ long sandbox_i2c_rtc_get_set_base_time(struct udevice *dev, long base_time);
 
 int sandbox_usb_keyb_add_string(struct udevice *dev, const char *str);
 
+/* For testing the BOARD uclass */
+enum {
+	BOARD_TEST0,
+	BOARD_TEST1,
+	BOARD_TEST2,
+
+	BOARD_TEST_COUNT,
+};
+
 #endif
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index e3151ea097..88015cc8af 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -8,6 +8,7 @@
 obj-$(CONFIG_MISC) += misc-uclass.o
 obj-$(CONFIG_ALI152X) += ali512x.o
 obj-$(CONFIG_ALTERA_SYSID) += altera_sysid.o
+obj-$(CONFIG_SANDBOX) += board_sandbox.o
 obj-$(CONFIG_DS4510)  += ds4510.o
 obj-$(CONFIG_CBMEM_CONSOLE) += cbmem_console.o
 ifndef CONFIG_SPL_BUILD
diff --git a/drivers/misc/board_sandbox.c b/drivers/misc/board_sandbox.c
new file mode 100644
index 0000000000..9ccdbfbbe8
--- /dev/null
+++ b/drivers/misc/board_sandbox.c
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2017 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <board.h>
+#include <dm.h>
+#include <asm/state.h>
+#include <asm/test.h>
+
+static int board_sandbox_phase(struct udevice *dev, enum board_phase_t phase)
+{
+	struct sandbox_state *state = state_get_current();
+	int id = dev_get_driver_data(dev);
+
+	return state->board_sandbox_ret[id];
+}
+
+static int board_sandbox_probe(struct udevice *dev)
+{
+	return board_support_phase(dev, BOARD_PHASE_TEST);
+}
+
+static const struct board_ops board_sandbox_ops = {
+	.phase		= board_sandbox_phase,
+};
+
+
+static const struct udevice_id board_sandbox_ids[] = {
+	{ .compatible = "sandbox,board-test0", BOARD_TEST0 },
+	{ .compatible = "sandbox,board-test1", BOARD_TEST1 },
+	{ .compatible = "sandbox,board-test2", BOARD_TEST2 },
+	{ }
+};
+
+U_BOOT_DRIVER(board_sandbox_drv) = {
+	.name		= "board_sandbox",
+	.id		= UCLASS_BOARD,
+	.ops		= &board_sandbox_ops,
+	.of_match	= board_sandbox_ids,
+	.probe		= board_sandbox_probe,
+};
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 1885e17c38..c84a966708 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_UT_DM) += test-uclass.o
 # subsystem you must add sandbox tests here.
 obj-$(CONFIG_UT_DM) += core.o
 ifneq ($(CONFIG_SANDBOX),)
+obj-$(CONFIG_BOARD) += board.o
 obj-$(CONFIG_BLK) += blk.o
 obj-$(CONFIG_CLK) += clk.o
 obj-$(CONFIG_DM_ETH) += eth.o
diff --git a/test/dm/board.c b/test/dm/board.c
new file mode 100644
index 0000000000..986746632b
--- /dev/null
+++ b/test/dm/board.c
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2015 Google, Inc
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <asm/state.h>
+#include <asm/test.h>
+#include <dm/test.h>
+#include <test/ut.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* Test invoking a board phase with three active devices */
+static int dm_test_board(struct unit_test_state *uts)
+{
+	struct sandbox_state *state = state_get_current();
+
+	/* We should start with a count of 0 for our test phase */
+	ut_asserteq(0, gd->phase_count[BOARD_PHASE_TEST]);
+
+	/* Check that we can detect there being no driver */
+	ut_asserteq(-ENOSYS, board_walk_phase_count(BOARD_PHASE_INVALID,
+						    false));
+	ut_asserteq(0, board_walk_opt_phase(BOARD_PHASE_INVALID));
+	ut_asserteq(-ENOSYS, board_walk_phase(BOARD_PHASE_INVALID));
+
+	/* If no devices respond, we should get no action */
+	state->board_sandbox_ret[BOARD_TEST0] = -ENOSYS;
+	state->board_sandbox_ret[BOARD_TEST1] = -ENOSYS;
+	state->board_sandbox_ret[BOARD_TEST2] = -ENOSYS;
+	ut_asserteq(-ENOSYS, board_walk_phase_count(BOARD_PHASE_TEST, false));
+	ut_asserteq(0, board_walk_opt_phase(BOARD_PHASE_TEST));
+	ut_asserteq(0, gd->phase_count[BOARD_PHASE_TEST]);
+
+	/* Enable the first device */
+	state->board_sandbox_ret[BOARD_TEST0] = 0;
+	ut_asserteq(1, board_walk_phase_count(BOARD_PHASE_TEST, false));
+	ut_asserteq(1, gd->phase_count[BOARD_PHASE_TEST]);
+
+	/* Enable the second device too */
+	state->board_sandbox_ret[BOARD_TEST1] = 0;
+	ut_asserteq(2, board_walk_phase_count(BOARD_PHASE_TEST, false));
+	ut_asserteq(3, gd->phase_count[BOARD_PHASE_TEST]);
+
+	/* Enable all three devices */
+	state->board_sandbox_ret[BOARD_TEST2] = 0;
+	ut_asserteq(3, board_walk_phase_count(BOARD_PHASE_TEST, false));
+	ut_asserteq(6, gd->phase_count[BOARD_PHASE_TEST]);
+
+	/*
+	 * Check that the first device can claim the phase and lock out the
+	 * other devices.
+	 */
+	state->board_sandbox_ret[BOARD_TEST0] = BOARD_PHASE_CLAIMED;
+	ut_asserteq(1, board_walk_phase_count(BOARD_PHASE_TEST, false));
+	ut_asserteq(0, board_walk_phase(BOARD_PHASE_TEST));
+	ut_asserteq(0, board_walk_opt_phase(BOARD_PHASE_TEST));
+	ut_asserteq(9, gd->phase_count[BOARD_PHASE_TEST]);
+
+	/* Any error should be reported, but previous devices should still get
+	 * to process the phase.
+	 */
+	state->board_sandbox_ret[BOARD_TEST0] = 0;
+	state->board_sandbox_ret[BOARD_TEST1] = -ENOENT;
+	ut_asserteq(-ENOENT, board_walk_phase_count(BOARD_PHASE_TEST, false));
+	ut_asserteq(-ENOENT, board_walk_phase(BOARD_PHASE_TEST));
+	ut_asserteq(-ENOENT, board_walk_opt_phase(BOARD_PHASE_TEST));
+	ut_asserteq(12, gd->phase_count[BOARD_PHASE_TEST]);
+
+	return 0;
+}
+DM_TEST(dm_test_board, DM_TESTF_SCAN_FDT);
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 10/16] dm: board: Add documentation

[Simon Glass]

Add some documentation for the new board init system. This describes how
it works and how to migrate to it.

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

 doc/driver-model/board-info.txt | 181 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 181 insertions(+)
 create mode 100644 doc/driver-model/board-info.txt

diff --git a/doc/driver-model/board-info.txt b/doc/driver-model/board-info.txt
new file mode 100644
index 0000000000..86db096e28
--- /dev/null
+++ b/doc/driver-model/board-info.txt
@@ -0,0 +1,181 @@
+How to handle board init with Driver Model
+==========================================
+
+Motivation
+----------
+
+At present U-Boot has a lot of ad-hoc init functions related to boards, for
+board_early_init_f, board_misc_init_f() and dram_init().
+
+There are used in different ways by different boards as useful hooks to
+do the required init and sequence it correctly. Some functions are always
+enabled but have a __weak default. Some are controlled by the existence
+of a CONFIG.
+
+There are two main init sequences: board_init_f() (f for running from
+read-only flash) which runs before relocation and board_init_r() (r for
+relocated) which runs afterwards.
+
+One problem with the current sequence is that it has a lot of
+arch-specific #ifdefs around various functions. There are also #ifdefs
+for various features. With a driver-model based approach it should be
+possible to remove these over time since the board-specific code can move into
+drivers and does not need to be called from the init sequence.
+
+
+Design
+------
+
+A new uclass (UCLASS_BOARD) is defined. The uclass has one important method:
+phase(). This is called to handle a particular phase of board init. Phases are
+defined by enum board_phase_t. For example, the existing board_early_init_f()
+function is replaced by the BOARD_F_EARLY_INIT_F phase.
+
+When the init sequence wants to initiate the BOARD_F_EARLY_INIT_F phase it
+calls the phase() method of all the devices that implement that phase. It
+knows which ones implement it because they call board_support_phase() or
+board_support_phase_mask() in their probe method to register which phases they
+support.
+
+Multiple devices can implement the same phase. The init sequence calls these
+devices one by one. It is also possible for a device to 'claim' a phase, such
+that no further devices are called. The ordering of devices is as per the
+usual driver-model approach: the same order as the device tree nodes, or
+ordered by device name in the case of U_BOOT_DEVICE() declarations.
+
+With this approach a call to board_early_init_f() is replaced with a call to
+board_walk_opt_phase() which handles the phase but does not complain if no
+device handles it. For a mandatory phase, board_walk_phase() can be used.
+
+After starting up you can use the 'board phases' command to see what phases
+were executed:
+
+	=> board phases
+	1 arch_cpu_init_dm
+	0 board_early_init_f
+	1 checkcpu
+	0 misc_init_f
+	1 dram_init
+	1 reserve_arch
+
+This shows that four phases were executed and each had a single device which
+handled that phase.
+
+
+Example
+-------
+
+Below is an example with sandbox:
+
+
+static int sandbox_phase(struct udevice *dev, enum board_phase_t phase)
+{
+	struct sandbox_state *state = state_get_current();
+
+	switch (phase) {
+	case BOARD_F_DRAM_INIT:
+		gd->ram_size = state->ram_size;
+		return 0;
+	default:
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+static int sandbox_board_probe(struct udevice *dev)
+{
+	return board_support_phase(dev, BOARD_F_DRAM_INIT);
+}
+
+static const struct board_ops sandbox_board_ops = {
+	.phase		= sandbox_phase,
+};
+
+/* Name this starting with underscore so it will be called last */
+U_BOOT_DRIVER(_sandbox_board_drv) = {
+	.name		= "sandbox_board",
+	.id		= UCLASS_BOARD,
+	.ops		= &sandbox_board_ops,
+	.probe		= sandbox_board_probe,
+	.flags		= DM_FLAG_PRE_RELOC,
+};
+
+U_BOOT_DEVICE(sandbox_board) = {
+	.name		= "sandbox_board",
+};
+
+
+This is a normal driver which supports the phase() method. The U_BOOT_DEVICE()
+declaration instantiates the device. It supports only one phase:
+BOARD_F_DRAM_INIT.
+
+If you look at common/board_f.c you will see how dram_init() uses this:
+
+int dram_init(void)
+{
+	return board_walk_phase(BOARD_F_DRAM_INIT);
+}
+
+That call will end up calling:
+
+	sandbox_phase(dev, BOARD_F_DRAM_INIT)
+
+There is quite a bit of boilerplate with the driver declaration. It would be
+quite easy to replace most of it with a macro, like:
+
+U_BOOT_BOARD_DRV(_sandbox_board, _sandbox_board_drv);
+
+but for now this is not attempted, to keep everything in the open.
+
+
+Porting suggestions
+-------------------
+
+You may find that you have all of your init handlers in a single file in your
+board directory. If so you can simply add a driver similar to the above that
+handles all the phases you use, and make your phase() method call the existing
+functions in that file. You should rename the functions to avoid confusion
+with the legacy method names and make them static since they will not be called
+from outside your file.
+
+If you have handlers in multiple files you can either:
+
+- Add a separate driver in each file
+- Use a single driver and have it call out to functions in other files for some
+    of the phases.
+
+Probably the first approach is better.
+
+You can perform the work in three patches:
+
+- one to add the board drivers and support
+- one to switch on CONFIG_BOARD_ENABLE
+- one to remove the now-unused legacy code
+
+This allows you to bisect easily if you find problems later.
+
+
+Required work
+-------------
+
+Before this feature can be applied to mainline it must support all phases. If
+we do not do thing up front then adding a new method may require porting all
+boards over to use that method. There is only a single control on whether or
+not to use this features (CONFIG_BOARD_ENABLE) so it is all or nothing.
+
+The post-relocation init phases therefore need to be added, and a review of
+pre-relocation phases needs to be completed to find anything that is missing.
+
+
+Future work
+-----------
+
+Apart from the required work, once all boards are ported to this framework we
+should be able to clean up the init sequences to remove all the #ifdefs.
+
+
+--
+Simon Glass
+sjg at chromium.org
+20-Mar-17
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 11/16] dm: x86: board: Enable CONFIG_BOARD

[Simon Glass]

Enable this option so we can use board drivers.

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

 arch/Kconfig | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/Kconfig b/arch/Kconfig
index f07070db18..2133a88c99 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -89,6 +89,8 @@ config X86
 	select CREATE_ARCH_SYMLINK
 	select HAVE_PRIVATE_LIBGCC
 	select SUPPORT_OF_CONTROL
+	select BOARD
+	select SPL_BOARD if SPL
 	select DM
 	select DM_KEYBOARD
 	select DM_SERIAL
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 12/16] dm: x86: board: Add a BOARD_F_RESERVE_ARCH driver

[Simon Glass]

Add support for reserving initial memory.

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

 arch/x86/cpu/cpu.c | 41 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
index 8fa6953588..893bec5c5c 100644
--- a/arch/x86/cpu/cpu.c
+++ b/arch/x86/cpu/cpu.c
@@ -254,7 +254,7 @@ int cpu_init_r(void)
 }
 
 #ifndef CONFIG_EFI_STUB
-int reserve_arch(void)
+static int cpu_x86_reserve_arch(void)
 {
 #ifdef CONFIG_ENABLE_MRC_CACHE
 	mrccache_reserve();
@@ -266,4 +266,43 @@ int reserve_arch(void)
 
 	return 0;
 }
+
+#ifndef CONFIG_BOARD_ENABLE
+int reserve_arch(void)
+{
+	return cpu_x86_reserve_arch();
+}
+#endif
+
+#endif /* !CONFIG_EFI_STUB */
+
+static int cpu_x86_phase(struct udevice *dev, enum board_phase_t phase)
+{
+#ifndef CONFIG_EFI_STUB
+	return cpu_x86_reserve_arch();
+#else
+	return 0;
 #endif
+}
+
+static int cpu_x86_board_probe(struct udevice *dev)
+{
+	return board_support_phase(dev, BOARD_F_RESERVE_ARCH);
+}
+
+static const struct board_ops cpu_x86_board_ops = {
+	.phase		= cpu_x86_phase,
+};
+
+/* Name this starting with underscore so it will be called last */
+U_BOOT_DRIVER(_cpu_x86_board_drv) = {
+	.name		= "cpu_x86_board",
+	.id		= UCLASS_BOARD,
+	.ops		= &cpu_x86_board_ops,
+	.probe		= cpu_x86_board_probe,
+	.flags		= DM_FLAG_PRE_RELOC,
+};
+
+U_BOOT_DEVICE(cpu_x86_board) = {
+	.name		= "cpu_x86_board",
+};
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 13/16] dm: x86: board: ivybridge: Add support for CONFIG_BOARD

[Simon Glass]

Add hooks for ivybridge boards so they can use CONFIG_BOARD for init.

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

 arch/x86/cpu/coreboot/coreboot.c                  |  2 +
 arch/x86/cpu/ivybridge/cpu.c                      | 71 +++++++++++++++++++----
 arch/x86/cpu/ivybridge/sdram.c                    | 13 ++++-
 arch/x86/cpu/ivybridge/sdram_nop.c                | 43 +++++++++++++-
 arch/x86/cpu/x86_64/cpu.c                         |  2 +
 arch/x86/include/asm/arch-ivybridge/sandybridge.h |  7 +++
 arch/x86/lib/spl.c                                | 13 +++++
 board/google/chromebook_link/link.c               |  2 +
 board/google/chromebox_panther/panther.c          |  2 +
 9 files changed, 140 insertions(+), 15 deletions(-)

diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c
index 1b042037bb..609bfcb2da 100644
--- a/arch/x86/cpu/coreboot/coreboot.c
+++ b/arch/x86/cpu/coreboot/coreboot.c
@@ -29,6 +29,7 @@ int arch_cpu_init(void)
 	return x86_cpu_init_f();
 }
 
+#ifndef CONFIG_BOARD_ENABLE
 int board_early_init_f(void)
 {
 	return 0;
@@ -38,6 +39,7 @@ int print_cpuinfo(void)
 {
 	return default_print_cpuinfo();
 }
+#endif
 
 static void board_final_cleanup(void)
 {
diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c
index c4aca08f0d..96e69ef792 100644
--- a/arch/x86/cpu/ivybridge/cpu.c
+++ b/arch/x86/cpu/ivybridge/cpu.c
@@ -12,6 +12,7 @@
  */
 
 #include <common.h>
+#include <board.h>
 #include <dm.h>
 #include <errno.h>
 #include <fdtdec.h>
@@ -50,10 +51,10 @@ int arch_cpu_init(void)
 	return x86_cpu_init_f();
 }
 
-int arch_cpu_init_dm(void)
+static int do_arch_cpu_init_dm(void)
 {
 	struct pci_controller *hose;
-	struct udevice *bus, *dev;
+	struct udevice *bus, *lpc_dev;
 	int ret;
 
 	post_code(0x70);
@@ -67,7 +68,7 @@ int arch_cpu_init_dm(void)
 	/* TODO(sjg at chromium.org): Get rid of gd->hose */
 	gd->hose = hose;
 
-	ret = uclass_first_device_err(UCLASS_LPC, &dev);
+	ret = uclass_first_device_err(UCLASS_LPC, &lpc_dev);
 	if (ret)
 		return ret;
 
@@ -83,6 +84,13 @@ int arch_cpu_init_dm(void)
 	return 0;
 }
 
+#ifndef CONFIG_BOARD_ENABLE
+int arch_cpu_init_dm(void)
+{
+	return do_arch_cpu_init_dm();
+}
+#endif
+
 #define PCH_EHCI0_TEMP_BAR0 0xe8000000
 #define PCH_EHCI1_TEMP_BAR0 0xe8000400
 #define PCH_XHCI_TEMP_BAR0  0xe8001000
@@ -125,12 +133,10 @@ static void enable_usb_bar(struct udevice *bus)
 	pci_bus_write_config(bus, usb3, PCI_COMMAND, cmd, PCI_SIZE_32);
 }
 
-int print_cpuinfo(void)
+static int ivybridge_checkcpu(void)
 {
 	enum pei_boot_mode_t boot_mode = PEI_BOOT_NONE;
-	char processor_name[CPU_MAX_NAME_LEN];
 	struct udevice *dev, *lpc;
-	const char *name;
 	uint32_t pm1_cnt;
 	uint16_t pm1_sts;
 	int ret;
@@ -181,19 +187,62 @@ int print_cpuinfo(void)
 	}
 
 	gd->arch.pei_boot_mode = boot_mode;
-
-	/* Print processor name */
-	name = cpu_get_name(processor_name);
-	printf("CPU:   %s\n", name);
-
 	post_code(POST_CPU_INFO);
 
 	return 0;
 }
 
+#ifndef CONFIG_BOARD_ENABLE
+int print_cpuinfo(void)
+{
+	return ivybridge_checkcpu();
+}
+#endif
+
 void board_debug_uart_init(void)
 {
 	/* This enables the debug UART */
 	pci_x86_write_config(NULL, PCH_LPC_DEV, LPC_EN, COMA_LPC_EN,
 			     PCI_SIZE_16);
 }
+
+static int cpu_x86_ivybridge_phase(struct udevice *dev,
+				   enum board_phase_t phase)
+{
+	switch (phase) {
+	case BOARD_F_ARCH_CPU_INIT_DM:
+		return do_arch_cpu_init_dm();
+	case BOARD_F_CHECKCPU:
+		return ivybridge_checkcpu();
+	case BOARD_F_DRAM_INIT:
+		return ivybridge_dram_init();
+	default:
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+static int cpu_x86_ivybridge_board_probe(struct udevice *dev)
+{
+	return board_support_phase_mask(dev,
+			board_phase_mask(BOARD_F_ARCH_CPU_INIT_DM) |
+			board_phase_mask(BOARD_F_CHECKCPU) |
+			board_phase_mask(BOARD_F_DRAM_INIT));
+}
+
+static const struct board_ops cpu_x86_ivybridge_board_ops = {
+	.phase		= cpu_x86_ivybridge_phase,
+};
+
+U_BOOT_DRIVER(cpu_x86_ivybridge_board_drv) = {
+	.name		= "cpu_x86_ivybridge_board",
+	.id		= UCLASS_BOARD,
+	.ops		= &cpu_x86_ivybridge_board_ops,
+	.probe		= cpu_x86_ivybridge_board_probe,
+	.flags		= DM_FLAG_PRE_RELOC,
+};
+
+U_BOOT_DEVICE(cpu_x86_ivybridge_board) = {
+	.name		= "cpu_x86_ivybridge_board",
+};
diff --git a/arch/x86/cpu/ivybridge/sdram.c b/arch/x86/cpu/ivybridge/sdram.c
index 201368c9c7..2484ed4859 100644
--- a/arch/x86/cpu/ivybridge/sdram.c
+++ b/arch/x86/cpu/ivybridge/sdram.c
@@ -401,7 +401,7 @@ static void rcba_config(void)
 	setbits_le32(RCB_REG(FD), PCH_DISABLE_ALWAYS);
 }
 
-int dram_init(void)
+int ivybridge_dram_init(void)
 {
 	struct pei_data _pei_data __aligned(8) = {
 		.pei_version = PEI_VERSION,
@@ -541,8 +541,8 @@ int dram_init(void)
 	/* S3 resume: don't save scrambler seed or MRC data */
 	if (pei_data->boot_mode != PEI_BOOT_RESUME) {
 		/*
-		 * This will be copied to SDRAM in reserve_arch(), then written
-		 * to SPI flash in mrccache_save()
+		 * This will be copied to SDRAM in the BOARD_F_RESERVE_ARCH
+		 * call, then written to SPI flash in mrccache_save()
 		 */
 		gd->arch.mrc_output = (char *)pei_data->mrc_output;
 		gd->arch.mrc_output_len = pei_data->mrc_output_len;
@@ -559,3 +559,10 @@ int dram_init(void)
 
 	return 0;
 }
+
+#ifndef CONFIG_BOARD_ENABLE
+int dram_init(void)
+{
+	return ivybridge_dram_init();
+}
+#endif
diff --git a/arch/x86/cpu/ivybridge/sdram_nop.c b/arch/x86/cpu/ivybridge/sdram_nop.c
index bd1189e447..641d099bbf 100644
--- a/arch/x86/cpu/ivybridge/sdram_nop.c
+++ b/arch/x86/cpu/ivybridge/sdram_nop.c
@@ -5,10 +5,11 @@
  */
 
 #include <common.h>
+#include <asm/arch/sandybridge.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-int dram_init(void)
+int nop_dram_init(void)
 {
 	gd->ram_size = 1ULL << 31;
 	gd->bd->bi_dram[0].start = 0;
@@ -16,3 +17,43 @@ int dram_init(void)
 
 	return 0;
 }
+
+#ifndef CONFIG_BOARD_ENABLE
+int dram_init(void)
+{
+	return nop_dram_init();
+}
+#endif
+
+static int cpu_x86_nop_phase(struct udevice *dev, enum board_phase_t phase)
+{
+	switch (phase) {
+	case BOARD_F_DRAM_INIT:
+		return nop_dram_init();
+	default:
+		return -ENOSYS;
+	}
+
+	return 0;
+}
+
+static int cpu_x86_nop_board_probe(struct udevice *dev)
+{
+	return board_support_phase(dev, BOARD_F_DRAM_INIT);
+}
+
+static const struct board_ops cpu_x86_nop_board_ops = {
+	.phase		= cpu_x86_nop_phase,
+};
+
+U_BOOT_DRIVER(cpu_x86_nop_board_drv) = {
+	.name		= "cpu_x86_nop_board",
+	.id		= UCLASS_BOARD,
+	.ops		= &cpu_x86_nop_board_ops,
+	.probe		= cpu_x86_nop_board_probe,
+	.flags		= DM_FLAG_PRE_RELOC,
+};
+
+U_BOOT_DEVICE(cpu_x86_nop_board) = {
+	.name		= "cpu_x86_nop_board",
+};
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
index db171f750d..ede46f7957 100644
--- a/arch/x86/cpu/x86_64/cpu.c
+++ b/arch/x86/cpu/x86_64/cpu.c
@@ -67,7 +67,9 @@ int misc_init_r(void)
 	return 0;
 }
 
+#ifndef CONFIG_BOARD_ENABLE
 int print_cpuinfo(void)
 {
 	return 0;
 }
+#endif
diff --git a/arch/x86/include/asm/arch-ivybridge/sandybridge.h b/arch/x86/include/asm/arch-ivybridge/sandybridge.h
index 8e0f668f0b..52f4c17620 100644
--- a/arch/x86/include/asm/arch-ivybridge/sandybridge.h
+++ b/arch/x86/include/asm/arch-ivybridge/sandybridge.h
@@ -113,4 +113,11 @@
  */
 int bridge_silicon_revision(struct udevice *dev);
 
+/**
+ * ivybridge_dram_init() - Set up SDRAM
+ *
+ * @return 0 if OK, -ve on error
+ */
+int ivybridge_dram_init(void);
+
 #endif
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index fa93d64a7a..a4c1a3ac35 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -48,6 +48,18 @@ static int x86_spl_init(void)
 		return ret;
 	}
 	preloader_console_init();
+#ifdef CONFIG_BOARD_ENABLE
+	ret = board_walk_phase(BOARD_F_CHECKCPU);
+	if (ret) {
+		debug("%s: BOARD_F_CHECKCPU failed\n", __func__);
+		return ret;
+	}
+	ret = board_walk_phase(BOARD_F_DRAM_INIT);
+	if (ret) {
+		debug("%s: BOARD_F_DRAM_INIT failed\n", __func__);
+		return ret;
+	}
+#else
 	ret = print_cpuinfo();
 	if (ret) {
 		debug("%s: print_cpuinfo() failed\n", __func__);
@@ -58,6 +70,7 @@ static int x86_spl_init(void)
 		debug("%s: dram_init() failed\n", __func__);
 		return ret;
 	}
+#endif
 	memset(&__bss_start, 0, (ulong)&__bss_end - (ulong)&__bss_start);
 
 	/* TODO(sjg at chromium.org): Consider calling cpu_init_r() here */
diff --git a/board/google/chromebook_link/link.c b/board/google/chromebook_link/link.c
index 42615e1e23..99b1c91edc 100644
--- a/board/google/chromebook_link/link.c
+++ b/board/google/chromebook_link/link.c
@@ -17,7 +17,9 @@ int arch_early_init_r(void)
 	return 0;
 }
 
+#ifndef CONFIG_BOARD_ENABLE
 int board_early_init_f(void)
 {
 	return 0;
 }
+#endif
diff --git a/board/google/chromebox_panther/panther.c b/board/google/chromebox_panther/panther.c
index e3baf88783..151cdd719d 100644
--- a/board/google/chromebox_panther/panther.c
+++ b/board/google/chromebox_panther/panther.c
@@ -12,7 +12,9 @@ int arch_early_init_r(void)
 	return 0;
 }
 
+#ifndef CONFIG_BOARD_ENABLE
 int board_early_init_f(void)
 {
 	return 0;
 }
+#endif
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 14/16] dm: x86: board: ivybridge: Switch to use CONFIG_BOARD

[Simon Glass]

Enable CONFIG_BOARD_ENABLE so that the new board init is used.

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

 configs/chromebook_link64_defconfig | 1 +
 configs/chromebook_link_defconfig   | 1 +
 configs/chromebox_panther_defconfig | 1 +
 3 files changed, 3 insertions(+)

diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig
index 749cfd43b0..074b4a6a1a 100644
--- a/configs/chromebook_link64_defconfig
+++ b/configs/chromebook_link64_defconfig
@@ -19,6 +19,7 @@ CONFIG_SPL_LOAD_FIT=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_BOARD_ENABLE=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_CPU_SUPPORT=y
 CONFIG_SPL_I2C_SUPPORT=y
diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig
index 5ebb556f90..7588bc8f8e 100644
--- a/configs/chromebook_link_defconfig
+++ b/configs/chromebook_link_defconfig
@@ -11,6 +11,7 @@ CONFIG_FIT=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_BOARD_ENABLE=y
 CONFIG_HUSH_PARSER=y
 CONFIG_CMD_CPU=y
 # CONFIG_CMD_IMLS is not set
diff --git a/configs/chromebox_panther_defconfig b/configs/chromebox_panther_defconfig
index 0d65a90eba..fac227cb2a 100644
--- a/configs/chromebox_panther_defconfig
+++ b/configs/chromebox_panther_defconfig
@@ -9,6 +9,7 @@ CONFIG_FIT=y
 CONFIG_BOOTSTAGE=y
 CONFIG_BOOTSTAGE_REPORT=y
 CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_BOARD_ENABLE=y
 CONFIG_HUSH_PARSER=y
 # CONFIG_CMD_IMLS is not set
 # CONFIG_CMD_FLASH is not set
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 15/16] dm: x86: board: ivybridge: Remove old board init code

[Simon Glass]

Remove the legacy board init code since it is no-longer used.

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

 arch/x86/cpu/ivybridge/cpu.c             | 16 +---------------
 arch/x86/cpu/ivybridge/sdram.c           |  7 -------
 arch/x86/cpu/ivybridge/sdram_nop.c       |  7 -------
 arch/x86/lib/spl.c                       | 13 -------------
 board/google/chromebook_link/link.c      |  7 -------
 board/google/chromebox_panther/panther.c |  7 -------
 6 files changed, 1 insertion(+), 56 deletions(-)

diff --git a/arch/x86/cpu/ivybridge/cpu.c b/arch/x86/cpu/ivybridge/cpu.c
index 96e69ef792..e7e6c3168a 100644
--- a/arch/x86/cpu/ivybridge/cpu.c
+++ b/arch/x86/cpu/ivybridge/cpu.c
@@ -84,13 +84,6 @@ static int do_arch_cpu_init_dm(void)
 	return 0;
 }
 
-#ifndef CONFIG_BOARD_ENABLE
-int arch_cpu_init_dm(void)
-{
-	return do_arch_cpu_init_dm();
-}
-#endif
-
 #define PCH_EHCI0_TEMP_BAR0 0xe8000000
 #define PCH_EHCI1_TEMP_BAR0 0xe8000400
 #define PCH_XHCI_TEMP_BAR0  0xe8001000
@@ -133,7 +126,7 @@ static void enable_usb_bar(struct udevice *bus)
 	pci_bus_write_config(bus, usb3, PCI_COMMAND, cmd, PCI_SIZE_32);
 }
 
-static int ivybridge_checkcpu(void)
+int ivybridge_checkcpu(void)
 {
 	enum pei_boot_mode_t boot_mode = PEI_BOOT_NONE;
 	struct udevice *dev, *lpc;
@@ -192,13 +185,6 @@ static int ivybridge_checkcpu(void)
 	return 0;
 }
 
-#ifndef CONFIG_BOARD_ENABLE
-int print_cpuinfo(void)
-{
-	return ivybridge_checkcpu();
-}
-#endif
-
 void board_debug_uart_init(void)
 {
 	/* This enables the debug UART */
diff --git a/arch/x86/cpu/ivybridge/sdram.c b/arch/x86/cpu/ivybridge/sdram.c
index 2484ed4859..eb4d04f8da 100644
--- a/arch/x86/cpu/ivybridge/sdram.c
+++ b/arch/x86/cpu/ivybridge/sdram.c
@@ -559,10 +559,3 @@ int ivybridge_dram_init(void)
 
 	return 0;
 }
-
-#ifndef CONFIG_BOARD_ENABLE
-int dram_init(void)
-{
-	return ivybridge_dram_init();
-}
-#endif
diff --git a/arch/x86/cpu/ivybridge/sdram_nop.c b/arch/x86/cpu/ivybridge/sdram_nop.c
index 641d099bbf..6bf5366410 100644
--- a/arch/x86/cpu/ivybridge/sdram_nop.c
+++ b/arch/x86/cpu/ivybridge/sdram_nop.c
@@ -18,13 +18,6 @@ int nop_dram_init(void)
 	return 0;
 }
 
-#ifndef CONFIG_BOARD_ENABLE
-int dram_init(void)
-{
-	return nop_dram_init();
-}
-#endif
-
 static int cpu_x86_nop_phase(struct udevice *dev, enum board_phase_t phase)
 {
 	switch (phase) {
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index a4c1a3ac35..0d40a6f41e 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -48,7 +48,6 @@ static int x86_spl_init(void)
 		return ret;
 	}
 	preloader_console_init();
-#ifdef CONFIG_BOARD_ENABLE
 	ret = board_walk_phase(BOARD_F_CHECKCPU);
 	if (ret) {
 		debug("%s: BOARD_F_CHECKCPU failed\n", __func__);
@@ -59,18 +58,6 @@ static int x86_spl_init(void)
 		debug("%s: BOARD_F_DRAM_INIT failed\n", __func__);
 		return ret;
 	}
-#else
-	ret = print_cpuinfo();
-	if (ret) {
-		debug("%s: print_cpuinfo() failed\n", __func__);
-		return ret;
-	}
-	ret = dram_init();
-	if (ret) {
-		debug("%s: dram_init() failed\n", __func__);
-		return ret;
-	}
-#endif
 	memset(&__bss_start, 0, (ulong)&__bss_end - (ulong)&__bss_start);
 
 	/* TODO(sjg at chromium.org): Consider calling cpu_init_r() here */
diff --git a/board/google/chromebook_link/link.c b/board/google/chromebook_link/link.c
index 99b1c91edc..64e7c1a08d 100644
--- a/board/google/chromebook_link/link.c
+++ b/board/google/chromebook_link/link.c
@@ -16,10 +16,3 @@ int arch_early_init_r(void)
 {
 	return 0;
 }
-
-#ifndef CONFIG_BOARD_ENABLE
-int board_early_init_f(void)
-{
-	return 0;
-}
-#endif
diff --git a/board/google/chromebox_panther/panther.c b/board/google/chromebox_panther/panther.c
index 151cdd719d..ed60e44264 100644
--- a/board/google/chromebox_panther/panther.c
+++ b/board/google/chromebox_panther/panther.c
@@ -11,10 +11,3 @@ int arch_early_init_r(void)
 {
 	return 0;
 }
-
-#ifndef CONFIG_BOARD_ENABLE
-int board_early_init_f(void)
-{
-	return 0;
-}
-#endif
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 16/16] dm: board: Add information about the new board init to the README

[Simon Glass]

Add a brief note pointing to the documentation.

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

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

diff --git a/README b/README
index aa907ced8a..891da97838 100644
--- a/README
+++ b/README
@@ -222,6 +222,9 @@ See board/sandbox/README.sandbox for more details.
 Board Initialisation Flow:
 --------------------------
 
+Note: An effort to organise and perhaps rationalise the many init hooks in the
+init sequence has started. See doc/driver-model/board-info.txt for details.
+
 This is the intended start-up flow for boards. This should apply for both
 SPL and U-Boot proper (i.e. they both follow the same rules).
 
-- 
2.12.0.367.g23dc2f6d3c-goog

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Tom Rini]

On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
> At present we have a lot of ad-hoc init functions related to boards, for
> example board_early_init_f(), board_misc_init_f() and dram_init().
> 
> There are used in different ways by different boards as useful hooks to
> do the required init and sequence it correctly. Some functions are always
> enabled but have a __weak default. Some are controlled by the existence
> of a CONFIG.
> 
> There are two main init sequences: board_init_f() (f for running from
> read-only flash) which runs before relocation and board_init_r() (r for
> relocated) which runs afterwards.
> 
> One problem with the current sequence is that it has a lot of
> arch-specific #ifdefs around various functions. There are also #ifdefs
> for various features. There has been quite a bit of discussion about how
> to tidy this up and at least one RFC series[1].
> 
> Now that we have driver model we can use this to deal with the init
> sequences. This approach has several advantages:
> 
> - We have a path to remove the #ifdefs
> - It is easy for multiple parts of the code to implement the same hook
> - We can track what is called and what is not
> - We don't need weak functions
> - We can eventually adjust the sequence to improve naming or to add new
> init phases
> - It provides a model for how we might deal with ft_board_setup() and
> friends
> 
> This series starts the process of replacing the pre-relocation init
> sequence with a driver-model solution. It defines a uclass, adds tests
> and converts sandbox and a few x86 boards over to use this new setup.
> 
> This series is not ready for use yet as the rest of the init sequence
> hooks need to be converted. But there is enough here to show the idea.
> 
> Comments welcome.
> 
> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html

How does this look, size wise?  With all of these conversions and
clean-ups, we really need to be size concious as well as it all keeps
adding up.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170319/1248b8f9/attachment.sig>

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Igor Grinberg]

Hi Simon,

On 03/19/17 20:59, Simon Glass wrote:
> At present we have a lot of ad-hoc init functions related to boards, for
> example board_early_init_f(), board_misc_init_f() and dram_init().
> 
> There are used in different ways by different boards as useful hooks to
> do the required init and sequence it correctly. Some functions are always
> enabled but have a __weak default. Some are controlled by the existence
> of a CONFIG.
> 
> There are two main init sequences: board_init_f() (f for running from
> read-only flash) which runs before relocation and board_init_r() (r for
> relocated) which runs afterwards.
> 
> One problem with the current sequence is that it has a lot of
> arch-specific #ifdefs around various functions. There are also #ifdefs
> for various features. There has been quite a bit of discussion about how
> to tidy this up and at least one RFC series[1].
> 
> Now that we have driver model we can use this to deal with the init
> sequences. This approach has several advantages:
> 
> - We have a path to remove the #ifdefs
> - It is easy for multiple parts of the code to implement the same hook
> - We can track what is called and what is not
> - We don't need weak functions
> - We can eventually adjust the sequence to improve naming or to add new
> init phases
> - It provides a model for how we might deal with ft_board_setup() and
> friends

I haven't got this one yet from the patchset.
May be I need to look closer...

> 
> This series starts the process of replacing the pre-relocation init
> sequence with a driver-model solution. It defines a uclass, adds tests
> and converts sandbox and a few x86 boards over to use this new setup.
> 
> This series is not ready for use yet as the rest of the init sequence
> hooks need to be converted. But there is enough here to show the idea.
> 
> Comments welcome.

Overall, it sounds like a great idea!

-- 
Regards,
Igor.

[U-Boot] [PATCH 04/16] dm: board: Add a uclass for init functions

[Igor Grinberg]

On 03/19/17 20:59, Simon Glass wrote:
> Add a uclass to handle board init. This allows drivers to be provided to
> perform the various phases of init. Functions are provided to call all
> devices that can handle a particular phase.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/Kconfig                    |  31 +++++++
>  common/init/Makefile              |   1 +
>  common/init/board-uclass.c        | 108 ++++++++++++++++++++++++
>  include/asm-generic/global_data.h |   5 ++
>  include/board.h                   | 170 ++++++++++++++++++++++++++++++++++++++
>  include/dm/uclass-id.h            |   1 +
>  6 files changed, 316 insertions(+)
>  create mode 100644 common/init/board-uclass.c
>  create mode 100644 include/board.h

[...]

> diff --git a/include/board.h b/include/board.h
> new file mode 100644
> index 0000000000..0975f5ac12
> --- /dev/null
> +++ b/include/board.h

[...]

> +/* Operations for the board driver */
> +struct board_ops {
> +	/**
> +	* phase() - Execute a phase of board init
> +	*
> +	 * @dev:	Device to use
> +	* @phase:	Phase to execute
> +	* @return 0 if done, -ENOSYS if not supported (which is often fine),
> +		BOARD_PHASE_CLAIMED if this was handled and that processing
> +		of this phase should stop (i.e. do not send it to other
> +		devices), other error if something went wrong
> +	*/
> +	int (*phase)(struct udevice *dev, enum board_phase_t phase);

That looks a bit tiny interface.
This will force all the boards to define switch to figure out what the
current phase is... This might cause a problem (probably #ifdefs) in the board
code as some code will be available in SPL and some not.

I would prefer a wider interface instead of a single entry point to any
kind of flexibility to the board driver.

> +
> +	/**
> +	 * get_desc() - Get a description string for a board
> +	 *
> +	 * @dev:	Device to check (UCLASS_BOARD)
> +	 * @buf:	Buffer to place string
> +	 * @size:	Size of string space
> +	 * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
> +	 */
> +	int (*get_desc)(struct udevice *dev, char *buf, int size);
> +};
> +
> +#define board_get_ops(dev)        ((struct board_ops *)(dev)->driver->ops)
> +

[...]


-- 
Regards,
Igor.

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi Tom,

On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
> On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
>> At present we have a lot of ad-hoc init functions related to boards, for
>> example board_early_init_f(), board_misc_init_f() and dram_init().
>>
>> There are used in different ways by different boards as useful hooks to
>> do the required init and sequence it correctly. Some functions are always
>> enabled but have a __weak default. Some are controlled by the existence
>> of a CONFIG.
>>
>> There are two main init sequences: board_init_f() (f for running from
>> read-only flash) which runs before relocation and board_init_r() (r for
>> relocated) which runs afterwards.
>>
>> One problem with the current sequence is that it has a lot of
>> arch-specific #ifdefs around various functions. There are also #ifdefs
>> for various features. There has been quite a bit of discussion about how
>> to tidy this up and at least one RFC series[1].
>>
>> Now that we have driver model we can use this to deal with the init
>> sequences. This approach has several advantages:
>>
>> - We have a path to remove the #ifdefs
>> - It is easy for multiple parts of the code to implement the same hook
>> - We can track what is called and what is not
>> - We don't need weak functions
>> - We can eventually adjust the sequence to improve naming or to add new
>> init phases
>> - It provides a model for how we might deal with ft_board_setup() and
>> friends
>>
>> This series starts the process of replacing the pre-relocation init
>> sequence with a driver-model solution. It defines a uclass, adds tests
>> and converts sandbox and a few x86 boards over to use this new setup.
>>
>> This series is not ready for use yet as the rest of the init sequence
>> hooks need to be converted. But there is enough here to show the idea.
>>
>> Comments welcome.
>>
>> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
>
> How does this look, size wise?  With all of these conversions and
> clean-ups, we really need to be size concious as well as it all keeps
> adding up.  Thanks!

It include size a bit - e.g. x86 808 bytes of text, although that does
include a few extra features.

11: dm: board: Add documentation
16: dm: x86: board: ivybridge: Remove old board init code
       x86: (for 1/1 boards) all +4331.0 bss +2944.0 data +252.0
rodata +327.0 text +808.0
            chromebook_link: all +4331 bss +2944 data +252 rodata +327 text +808
               u-boot: add: 22/0, grow: 1/-4 bytes: 3012/-2183 (829)
                 function                                   old     new   delta
                 ivybridge_dram_init                          -    1956   +1956
                 cpu_x86_ivybridge_phase                      -     151    +151
                 board_walk_phase_count                       -     145    +145
                 ivybridge_checkcpu                           -      96     +96
                 board_phase                                  -      95     +95
                 do_board                                     -      77     +77
                 cpu_print_info                               -      76     +76
                 _u_boot_list_2_uclass_2_board                -      72     +72
                 _u_boot_list_2_driver_2_cpu_x86_ivybridge_board_drv
    -      68     +68
                 _u_boot_list_2_driver_2__cpu_x86_board_drv       -
  68     +68
                 board_walk_phase                             -      28     +28
                 board_walk_opt_phase                         -      28     +28
                 _u_boot_list_2_cmd_2_board                   -      28     +28
                 board_support_phase                          -      27     +27
                 board_support_phase_mask                     -      20     +20
                 cpu_x86_phase                                -      14     +14
                 misc_init_f                                  -      10     +10
                 cpu_x86_ivybridge_board_probe                -      10     +10
                 cpu_x86_board_probe                          -      10     +10
                 checkcpu                                     -      10     +10
                 _u_boot_list_2_driver_info_2_cpu_x86_ivybridge_board
     -       8      +8
                 _u_boot_list_2_driver_info_2_cpu_x86_board       -
   8      +8
                 board_early_init_f                           3      10      +7
                 reserve_arch                                14      10      -4
                 arch_cpu_init_dm                           120       7    -113
                 print_cpuinfo                              125       5    -120
                 dram_init                                 1956      10   -1946
(no errors to report)

I think I can use a linker-list approach to reduce the overhead. But I
still think the driver has value as it provides a means of adding
hooks to do board-specific things from drivers, something that we keep
running into. Also the idea of a board 'driver' seems conceptually
useful.

So one approach would be to have:

1. A linker-list-based board hook setup, where you can declare, for example:

static int ivybridge_dram_init(void)
{
 ...
}
U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);

This should be pretty cheap, perhaps <200 bytes with some care.


2. An optional BOARD uclass which can be used for more involved
situations, but with a higher code size penalty.

Regards,
Simon

[U-Boot] [PATCH 04/16] dm: board: Add a uclass for init functions

[Simon Glass]

Hi Igor,

On 20 March 2017 at 01:54, Igor Grinberg <grinberg@compulab.co.il> wrote:
>
> On 03/19/17 20:59, Simon Glass wrote:
>> Add a uclass to handle board init. This allows drivers to be provided to
>> perform the various phases of init. Functions are provided to call all
>> devices that can handle a particular phase.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  common/Kconfig                    |  31 +++++++
>>  common/init/Makefile              |   1 +
>>  common/init/board-uclass.c        | 108 ++++++++++++++++++++++++
>>  include/asm-generic/global_data.h |   5 ++
>>  include/board.h                   | 170 ++++++++++++++++++++++++++++++++++++++
>>  include/dm/uclass-id.h            |   1 +
>>  6 files changed, 316 insertions(+)
>>  create mode 100644 common/init/board-uclass.c
>>  create mode 100644 include/board.h
>
> [...]
>
>> diff --git a/include/board.h b/include/board.h
>> new file mode 100644
>> index 0000000000..0975f5ac12
>> --- /dev/null
>> +++ b/include/board.h
>
> [...]
>
>> +/* Operations for the board driver */
>> +struct board_ops {
>> +     /**
>> +     * phase() - Execute a phase of board init
>> +     *
>> +      * @dev:        Device to use
>> +     * @phase:       Phase to execute
>> +     * @return 0 if done, -ENOSYS if not supported (which is often fine),
>> +             BOARD_PHASE_CLAIMED if this was handled and that processing
>> +             of this phase should stop (i.e. do not send it to other
>> +             devices), other error if something went wrong
>> +     */
>> +     int (*phase)(struct udevice *dev, enum board_phase_t phase);
>
> That looks a bit tiny interface.
> This will force all the boards to define switch to figure out what the
> current phase is... This might cause a problem (probably #ifdefs) in the board
> code as some code will be available in SPL and some not.
>
> I would prefer a wider interface instead of a single entry point to any
> kind of flexibility to the board driver.

Yes that certainly seems nicer.

But it means we might have ~20 or so methods in the interface. Do you
think most of them would be used? Also I am thinking a linker list
idea might help with code size, and would need some sort of 'phase'
parameter I think.

>
>> +
>> +     /**
>> +      * get_desc() - Get a description string for a board
>> +      *
>> +      * @dev:        Device to check (UCLASS_BOARD)
>> +      * @buf:        Buffer to place string
>> +      * @size:       Size of string space
>> +      * @return 0 if OK, -ENOSPC if buffer is too small, other -ve on error
>> +      */
>> +     int (*get_desc)(struct udevice *dev, char *buf, int size);
>> +};
>> +
>> +#define board_get_ops(dev)        ((struct board_ops *)(dev)->driver->ops)
>> +
>
> [...]
>
>
> --
> Regards,
> Igor.

Regards,
Simon

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi Igor,

On 20 March 2017 at 01:30, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Simon,
>
> On 03/19/17 20:59, Simon Glass wrote:
>> At present we have a lot of ad-hoc init functions related to boards, for
>> example board_early_init_f(), board_misc_init_f() and dram_init().
>>
>> There are used in different ways by different boards as useful hooks to
>> do the required init and sequence it correctly. Some functions are always
>> enabled but have a __weak default. Some are controlled by the existence
>> of a CONFIG.
>>
>> There are two main init sequences: board_init_f() (f for running from
>> read-only flash) which runs before relocation and board_init_r() (r for
>> relocated) which runs afterwards.
>>
>> One problem with the current sequence is that it has a lot of
>> arch-specific #ifdefs around various functions. There are also #ifdefs
>> for various features. There has been quite a bit of discussion about how
>> to tidy this up and at least one RFC series[1].
>>
>> Now that we have driver model we can use this to deal with the init
>> sequences. This approach has several advantages:
>>
>> - We have a path to remove the #ifdefs
>> - It is easy for multiple parts of the code to implement the same hook
>> - We can track what is called and what is not
>> - We don't need weak functions
>> - We can eventually adjust the sequence to improve naming or to add new
>> init phases
>> - It provides a model for how we might deal with ft_board_setup() and
>> friends
>
> I haven't got this one yet from the patchset.
> May be I need to look closer...

It is not included in this patch set. I just mean that we could do a
similar scheme with those functions.

>
>>
>> This series starts the process of replacing the pre-relocation init
>> sequence with a driver-model solution. It defines a uclass, adds tests
>> and converts sandbox and a few x86 boards over to use this new setup.
>>
>> This series is not ready for use yet as the rest of the init sequence
>> hooks need to be converted. But there is enough here to show the idea.
>>
>> Comments welcome.
>
> Overall, it sounds like a great idea!

I'll do another spin and see if I can tighten it up a little.

>
> --
> Regards,
> Igor.

Regards,
Simon

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Tom Rini]

On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
> >> At present we have a lot of ad-hoc init functions related to boards, for
> >> example board_early_init_f(), board_misc_init_f() and dram_init().
> >>
> >> There are used in different ways by different boards as useful hooks to
> >> do the required init and sequence it correctly. Some functions are always
> >> enabled but have a __weak default. Some are controlled by the existence
> >> of a CONFIG.
> >>
> >> There are two main init sequences: board_init_f() (f for running from
> >> read-only flash) which runs before relocation and board_init_r() (r for
> >> relocated) which runs afterwards.
> >>
> >> One problem with the current sequence is that it has a lot of
> >> arch-specific #ifdefs around various functions. There are also #ifdefs
> >> for various features. There has been quite a bit of discussion about how
> >> to tidy this up and at least one RFC series[1].
> >>
> >> Now that we have driver model we can use this to deal with the init
> >> sequences. This approach has several advantages:
> >>
> >> - We have a path to remove the #ifdefs
> >> - It is easy for multiple parts of the code to implement the same hook
> >> - We can track what is called and what is not
> >> - We don't need weak functions
> >> - We can eventually adjust the sequence to improve naming or to add new
> >> init phases
> >> - It provides a model for how we might deal with ft_board_setup() and
> >> friends
> >>
> >> This series starts the process of replacing the pre-relocation init
> >> sequence with a driver-model solution. It defines a uclass, adds tests
> >> and converts sandbox and a few x86 boards over to use this new setup.
> >>
> >> This series is not ready for use yet as the rest of the init sequence
> >> hooks need to be converted. But there is enough here to show the idea.
> >>
> >> Comments welcome.
> >>
> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
> >
> > How does this look, size wise?  With all of these conversions and
> > clean-ups, we really need to be size concious as well as it all keeps
> > adding up.  Thanks!
> 
> It include size a bit - e.g. x86 808 bytes of text, although that does
> include a few extra features.

How about if we don't include some of the extra debug/demo type features
(which are useful at times, certainly) ?  We keep adding things that add
a few bytes here and a few bytes there and it all adds up.

[snip]
> I think I can use a linker-list approach to reduce the overhead. But I
> still think the driver has value as it provides a means of adding
> hooks to do board-specific things from drivers, something that we keep
> running into. Also the idea of a board 'driver' seems conceptually
> useful.
> 
> So one approach would be to have:
> 
> 1. A linker-list-based board hook setup, where you can declare, for example:
> 
> static int ivybridge_dram_init(void)
> {
>  ...
> }
> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
> 
> This should be pretty cheap, perhaps <200 bytes with some care.
> 
> 
> 2. An optional BOARD uclass which can be used for more involved
> situations, but with a higher code size penalty.

OK.  But I also recall that we had talked about trying to condense and
re-factor things.  My worry about an approach like this is it allows for
us to more easily get back into the bad habits of having each
architecture solve similar problems with different solutions.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170322/db509ac6/attachment.sig>

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi Tom,

On 22 March 2017 at 08:37, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
>> Hi Tom,
>>
>> On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
>> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
>> >> At present we have a lot of ad-hoc init functions related to boards, for
>> >> example board_early_init_f(), board_misc_init_f() and dram_init().
>> >>
>> >> There are used in different ways by different boards as useful hooks to
>> >> do the required init and sequence it correctly. Some functions are always
>> >> enabled but have a __weak default. Some are controlled by the existence
>> >> of a CONFIG.
>> >>
>> >> There are two main init sequences: board_init_f() (f for running from
>> >> read-only flash) which runs before relocation and board_init_r() (r for
>> >> relocated) which runs afterwards.
>> >>
>> >> One problem with the current sequence is that it has a lot of
>> >> arch-specific #ifdefs around various functions. There are also #ifdefs
>> >> for various features. There has been quite a bit of discussion about how
>> >> to tidy this up and at least one RFC series[1].
>> >>
>> >> Now that we have driver model we can use this to deal with the init
>> >> sequences. This approach has several advantages:
>> >>
>> >> - We have a path to remove the #ifdefs
>> >> - It is easy for multiple parts of the code to implement the same hook
>> >> - We can track what is called and what is not
>> >> - We don't need weak functions
>> >> - We can eventually adjust the sequence to improve naming or to add new
>> >> init phases
>> >> - It provides a model for how we might deal with ft_board_setup() and
>> >> friends
>> >>
>> >> This series starts the process of replacing the pre-relocation init
>> >> sequence with a driver-model solution. It defines a uclass, adds tests
>> >> and converts sandbox and a few x86 boards over to use this new setup.
>> >>
>> >> This series is not ready for use yet as the rest of the init sequence
>> >> hooks need to be converted. But there is enough here to show the idea.
>> >>
>> >> Comments welcome.
>> >>
>> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
>> >
>> > How does this look, size wise?  With all of these conversions and
>> > clean-ups, we really need to be size concious as well as it all keeps
>> > adding up.  Thanks!
>>
>> It include size a bit - e.g. x86 808 bytes of text, although that does
>> include a few extra features.
>
> How about if we don't include some of the extra debug/demo type features
> (which are useful at times, certainly) ?  We keep adding things that add
> a few bytes here and a few bytes there and it all adds up.

Yes it's very important that the base version doesn't increase size,
or at least only minimally. I should have examined that more closely
in the RFC - my intent was really to get comments on the approach,

>
> [snip]
>> I think I can use a linker-list approach to reduce the overhead. But I
>> still think the driver has value as it provides a means of adding
>> hooks to do board-specific things from drivers, something that we keep
>> running into. Also the idea of a board 'driver' seems conceptually
>> useful.
>>
>> So one approach would be to have:
>>
>> 1. A linker-list-based board hook setup, where you can declare, for example:
>>
>> static int ivybridge_dram_init(void)
>> {
>>  ...
>> }
>> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
>>
>> This should be pretty cheap, perhaps <200 bytes with some care.
>>
>>
>> 2. An optional BOARD uclass which can be used for more involved
>> situations, but with a higher code size penalty.
>
> OK.  But I also recall that we had talked about trying to condense and
> re-factor things.  My worry about an approach like this is it allows for
> us to more easily get back into the bad habits of having each
> architecture solve similar problems with different solutions.

Yes that's true and I've been pushing back on this for a while. For
example there is quite a bit of pressure to add board-specific init
code to drivers with driver model. So far I think we have been able to
avoid this using device tree and other drivers. For example if MMC
needs a clock we can find the required clock by phandle and call the
clock driver.

So are you thinking we should limit this to just a simple hook
approach for now, and then consider the board uclass down the track?

Regards,
Simon

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Tom Rini]

On Wed, Mar 22, 2017 at 08:43:54AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On 22 March 2017 at 08:37, Tom Rini <trini@konsulko.com> wrote:
> > On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
> >> Hi Tom,
> >>
> >> On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
> >> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
> >> >> At present we have a lot of ad-hoc init functions related to boards, for
> >> >> example board_early_init_f(), board_misc_init_f() and dram_init().
> >> >>
> >> >> There are used in different ways by different boards as useful hooks to
> >> >> do the required init and sequence it correctly. Some functions are always
> >> >> enabled but have a __weak default. Some are controlled by the existence
> >> >> of a CONFIG.
> >> >>
> >> >> There are two main init sequences: board_init_f() (f for running from
> >> >> read-only flash) which runs before relocation and board_init_r() (r for
> >> >> relocated) which runs afterwards.
> >> >>
> >> >> One problem with the current sequence is that it has a lot of
> >> >> arch-specific #ifdefs around various functions. There are also #ifdefs
> >> >> for various features. There has been quite a bit of discussion about how
> >> >> to tidy this up and at least one RFC series[1].
> >> >>
> >> >> Now that we have driver model we can use this to deal with the init
> >> >> sequences. This approach has several advantages:
> >> >>
> >> >> - We have a path to remove the #ifdefs
> >> >> - It is easy for multiple parts of the code to implement the same hook
> >> >> - We can track what is called and what is not
> >> >> - We don't need weak functions
> >> >> - We can eventually adjust the sequence to improve naming or to add new
> >> >> init phases
> >> >> - It provides a model for how we might deal with ft_board_setup() and
> >> >> friends
> >> >>
> >> >> This series starts the process of replacing the pre-relocation init
> >> >> sequence with a driver-model solution. It defines a uclass, adds tests
> >> >> and converts sandbox and a few x86 boards over to use this new setup.
> >> >>
> >> >> This series is not ready for use yet as the rest of the init sequence
> >> >> hooks need to be converted. But there is enough here to show the idea.
> >> >>
> >> >> Comments welcome.
> >> >>
> >> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
> >> >
> >> > How does this look, size wise?  With all of these conversions and
> >> > clean-ups, we really need to be size concious as well as it all keeps
> >> > adding up.  Thanks!
> >>
> >> It include size a bit - e.g. x86 808 bytes of text, although that does
> >> include a few extra features.
> >
> > How about if we don't include some of the extra debug/demo type features
> > (which are useful at times, certainly) ?  We keep adding things that add
> > a few bytes here and a few bytes there and it all adds up.
> 
> Yes it's very important that the base version doesn't increase size,
> or at least only minimally. I should have examined that more closely
> in the RFC - my intent was really to get comments on the approach,
> 
> >
> > [snip]
> >> I think I can use a linker-list approach to reduce the overhead. But I
> >> still think the driver has value as it provides a means of adding
> >> hooks to do board-specific things from drivers, something that we keep
> >> running into. Also the idea of a board 'driver' seems conceptually
> >> useful.
> >>
> >> So one approach would be to have:
> >>
> >> 1. A linker-list-based board hook setup, where you can declare, for example:
> >>
> >> static int ivybridge_dram_init(void)
> >> {
> >>  ...
> >> }
> >> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
> >>
> >> This should be pretty cheap, perhaps <200 bytes with some care.
> >>
> >>
> >> 2. An optional BOARD uclass which can be used for more involved
> >> situations, but with a higher code size penalty.
> >
> > OK.  But I also recall that we had talked about trying to condense and
> > re-factor things.  My worry about an approach like this is it allows for
> > us to more easily get back into the bad habits of having each
> > architecture solve similar problems with different solutions.
> 
> Yes that's true and I've been pushing back on this for a while. For
> example there is quite a bit of pressure to add board-specific init
> code to drivers with driver model. So far I think we have been able to
> avoid this using device tree and other drivers. For example if MMC
> needs a clock we can find the required clock by phandle and call the
> clock driver.
> 
> So are you thinking we should limit this to just a simple hook
> approach for now, and then consider the board uclass down the track?

Looking over init_sequence_[rf], I can certainly see the case for "ug,
this is ugly and we need to make it better" (and I now wonder if we
don't have a lot more places where we need INIT_FUNC_WATCHDOG_RESET,
anyhow...).  But for the moment we seem to be able to resist adding more
calls here.  And I would like to see if we can rework / reduce the
current table before we try and simplify and clean-up the mechanism that
we use to handle them.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20170322/bd07d014/attachment.sig>

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[york sun]

On 03/19/2017 11:59 AM, Simon Glass wrote:
> At present we have a lot of ad-hoc init functions related to boards, for
> example board_early_init_f(), board_misc_init_f() and dram_init().
>
> There are used in different ways by different boards as useful hooks to
> do the required init and sequence it correctly. Some functions are always
> enabled but have a __weak default. Some are controlled by the existence
> of a CONFIG.
>
> There are two main init sequences: board_init_f() (f for running from
> read-only flash) which runs before relocation and board_init_r() (r for
> relocated) which runs afterwards.
>
> One problem with the current sequence is that it has a lot of
> arch-specific #ifdefs around various functions. There are also #ifdefs
> for various features. There has been quite a bit of discussion about how
> to tidy this up and at least one RFC series[1].
>
> Now that we have driver model we can use this to deal with the init
> sequences. This approach has several advantages:
>
> - We have a path to remove the #ifdefs
> - It is easy for multiple parts of the code to implement the same hook
> - We can track what is called and what is not
> - We don't need weak functions
> - We can eventually adjust the sequence to improve naming or to add new
> init phases
> - It provides a model for how we might deal with ft_board_setup() and
> friends
>
> This series starts the process of replacing the pre-relocation init
> sequence with a driver-model solution.

I like the idea to be able to use DM earlier.

York

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi York,

On 22 March 2017 at 10:00, york sun <york.sun@nxp.com> wrote:
> On 03/19/2017 11:59 AM, Simon Glass wrote:
>> At present we have a lot of ad-hoc init functions related to boards, for
>> example board_early_init_f(), board_misc_init_f() and dram_init().
>>
>> There are used in different ways by different boards as useful hooks to
>> do the required init and sequence it correctly. Some functions are always
>> enabled but have a __weak default. Some are controlled by the existence
>> of a CONFIG.
>>
>> There are two main init sequences: board_init_f() (f for running from
>> read-only flash) which runs before relocation and board_init_r() (r for
>> relocated) which runs afterwards.
>>
>> One problem with the current sequence is that it has a lot of
>> arch-specific #ifdefs around various functions. There are also #ifdefs
>> for various features. There has been quite a bit of discussion about how
>> to tidy this up and at least one RFC series[1].
>>
>> Now that we have driver model we can use this to deal with the init
>> sequences. This approach has several advantages:
>>
>> - We have a path to remove the #ifdefs
>> - It is easy for multiple parts of the code to implement the same hook
>> - We can track what is called and what is not
>> - We don't need weak functions
>> - We can eventually adjust the sequence to improve naming or to add new
>> init phases
>> - It provides a model for how we might deal with ft_board_setup() and
>> friends
>>
>> This series starts the process of replacing the pre-relocation init
>> sequence with a driver-model solution.
>
> I like the idea to be able to use DM earlier.

Well actually my series does not allow that. Here is the start of the
init sequence with u-boot-dm/rmb-working applied:

static const init_fnc_t init_sequence_f[] = {
        setup_mon_len,
#ifdef CONFIG_OF_CONTROL
        fdtdec_setup,
#endif
#ifdef CONFIG_TRACE
        trace_early_init,
#endif
        initf_malloc,
        initf_console_record,
#if defined(CONFIG_HAVE_FSP)
        arch_fsp_init,
#endif
        arch_cpu_init, /* basic arch cpu dependent setup */
        mach_cpu_init, /* SoC/machine dependent CPU setup */
        initf_dm,

Of these:
- Setting up device tree must happen before DM
- trace could be moved later, but then you lose tracing of DM init
- malloc() is needed
- recording console could move, but as it is it records DM init
console debug output, which could be useful
- arch_fsp_init() and arch_cpu_init() and mach_cpu_init() could
perhaps move later depending on how we define them

Anyway I'm interested in your suggestions.

Regards,
Simon

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi Tom,

On 22 March 2017 at 09:13, Tom Rini <trini@konsulko.com> wrote:
> On Wed, Mar 22, 2017 at 08:43:54AM -0600, Simon Glass wrote:
>> Hi Tom,
>>
>> On 22 March 2017 at 08:37, Tom Rini <trini@konsulko.com> wrote:
>> > On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
>> >> Hi Tom,
>> >>
>> >> On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
>> >> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
>> >> >> At present we have a lot of ad-hoc init functions related to boards, for
>> >> >> example board_early_init_f(), board_misc_init_f() and dram_init().
>> >> >>
>> >> >> There are used in different ways by different boards as useful hooks to
>> >> >> do the required init and sequence it correctly. Some functions are always
>> >> >> enabled but have a __weak default. Some are controlled by the existence
>> >> >> of a CONFIG.
>> >> >>
>> >> >> There are two main init sequences: board_init_f() (f for running from
>> >> >> read-only flash) which runs before relocation and board_init_r() (r for
>> >> >> relocated) which runs afterwards.
>> >> >>
>> >> >> One problem with the current sequence is that it has a lot of
>> >> >> arch-specific #ifdefs around various functions. There are also #ifdefs
>> >> >> for various features. There has been quite a bit of discussion about how
>> >> >> to tidy this up and at least one RFC series[1].
>> >> >>
>> >> >> Now that we have driver model we can use this to deal with the init
>> >> >> sequences. This approach has several advantages:
>> >> >>
>> >> >> - We have a path to remove the #ifdefs
>> >> >> - It is easy for multiple parts of the code to implement the same hook
>> >> >> - We can track what is called and what is not
>> >> >> - We don't need weak functions
>> >> >> - We can eventually adjust the sequence to improve naming or to add new
>> >> >> init phases
>> >> >> - It provides a model for how we might deal with ft_board_setup() and
>> >> >> friends
>> >> >>
>> >> >> This series starts the process of replacing the pre-relocation init
>> >> >> sequence with a driver-model solution. It defines a uclass, adds tests
>> >> >> and converts sandbox and a few x86 boards over to use this new setup.
>> >> >>
>> >> >> This series is not ready for use yet as the rest of the init sequence
>> >> >> hooks need to be converted. But there is enough here to show the idea.
>> >> >>
>> >> >> Comments welcome.
>> >> >>
>> >> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
>> >> >
>> >> > How does this look, size wise?  With all of these conversions and
>> >> > clean-ups, we really need to be size concious as well as it all keeps
>> >> > adding up.  Thanks!
>> >>
>> >> It include size a bit - e.g. x86 808 bytes of text, although that does
>> >> include a few extra features.
>> >
>> > How about if we don't include some of the extra debug/demo type features
>> > (which are useful at times, certainly) ?  We keep adding things that add
>> > a few bytes here and a few bytes there and it all adds up.
>>
>> Yes it's very important that the base version doesn't increase size,
>> or at least only minimally. I should have examined that more closely
>> in the RFC - my intent was really to get comments on the approach,
>>
>> >
>> > [snip]
>> >> I think I can use a linker-list approach to reduce the overhead. But I
>> >> still think the driver has value as it provides a means of adding
>> >> hooks to do board-specific things from drivers, something that we keep
>> >> running into. Also the idea of a board 'driver' seems conceptually
>> >> useful.
>> >>
>> >> So one approach would be to have:
>> >>
>> >> 1. A linker-list-based board hook setup, where you can declare, for example:
>> >>
>> >> static int ivybridge_dram_init(void)
>> >> {
>> >>  ...
>> >> }
>> >> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
>> >>
>> >> This should be pretty cheap, perhaps <200 bytes with some care.
>> >>
>> >>
>> >> 2. An optional BOARD uclass which can be used for more involved
>> >> situations, but with a higher code size penalty.
>> >
>> > OK.  But I also recall that we had talked about trying to condense and
>> > re-factor things.  My worry about an approach like this is it allows for
>> > us to more easily get back into the bad habits of having each
>> > architecture solve similar problems with different solutions.
>>
>> Yes that's true and I've been pushing back on this for a while. For
>> example there is quite a bit of pressure to add board-specific init
>> code to drivers with driver model. So far I think we have been able to
>> avoid this using device tree and other drivers. For example if MMC
>> needs a clock we can find the required clock by phandle and call the
>> clock driver.
>>
>> So are you thinking we should limit this to just a simple hook
>> approach for now, and then consider the board uclass down the track?
>
> Looking over init_sequence_[rf], I can certainly see the case for "ug,
> this is ugly and we need to make it better" (and I now wonder if we
> don't have a lot more places where we need INIT_FUNC_WATCHDOG_RESET,
> anyhow...).  But for the moment we seem to be able to resist adding more
> calls here.  And I would like to see if we can rework / reduce the
> current table before we try and simplify and clean-up the mechanism that
> we use to handle them.

I agree, and I have some concern that making it easier to extend the
init sequence might end up with less consistency between archs as to
the sequence we go through during init.

For now I've done two series to tidy up board_f. There is more to do
though. We can park this series until we get a bit closer (it might be
quite a while).

Regards,
Simon

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Bin Meng]

Hi Simon,

On Sat, Apr 1, 2017 at 12:21 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Tom,
>
> On 22 March 2017 at 09:13, Tom Rini <trini@konsulko.com> wrote:
>> On Wed, Mar 22, 2017 at 08:43:54AM -0600, Simon Glass wrote:
>>> Hi Tom,
>>>
>>> On 22 March 2017 at 08:37, Tom Rini <trini@konsulko.com> wrote:
>>> > On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
>>> >> Hi Tom,
>>> >>
>>> >> On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
>>> >> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
>>> >> >> At present we have a lot of ad-hoc init functions related to boards, for
>>> >> >> example board_early_init_f(), board_misc_init_f() and dram_init().
>>> >> >>
>>> >> >> There are used in different ways by different boards as useful hooks to
>>> >> >> do the required init and sequence it correctly. Some functions are always
>>> >> >> enabled but have a __weak default. Some are controlled by the existence
>>> >> >> of a CONFIG.
>>> >> >>
>>> >> >> There are two main init sequences: board_init_f() (f for running from
>>> >> >> read-only flash) which runs before relocation and board_init_r() (r for
>>> >> >> relocated) which runs afterwards.
>>> >> >>
>>> >> >> One problem with the current sequence is that it has a lot of
>>> >> >> arch-specific #ifdefs around various functions. There are also #ifdefs
>>> >> >> for various features. There has been quite a bit of discussion about how
>>> >> >> to tidy this up and at least one RFC series[1].
>>> >> >>
>>> >> >> Now that we have driver model we can use this to deal with the init
>>> >> >> sequences. This approach has several advantages:
>>> >> >>
>>> >> >> - We have a path to remove the #ifdefs
>>> >> >> - It is easy for multiple parts of the code to implement the same hook
>>> >> >> - We can track what is called and what is not
>>> >> >> - We don't need weak functions
>>> >> >> - We can eventually adjust the sequence to improve naming or to add new
>>> >> >> init phases
>>> >> >> - It provides a model for how we might deal with ft_board_setup() and
>>> >> >> friends
>>> >> >>
>>> >> >> This series starts the process of replacing the pre-relocation init
>>> >> >> sequence with a driver-model solution. It defines a uclass, adds tests
>>> >> >> and converts sandbox and a few x86 boards over to use this new setup.
>>> >> >>
>>> >> >> This series is not ready for use yet as the rest of the init sequence
>>> >> >> hooks need to be converted. But there is enough here to show the idea.
>>> >> >>
>>> >> >> Comments welcome.
>>> >> >>
>>> >> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
>>> >> >
>>> >> > How does this look, size wise?  With all of these conversions and
>>> >> > clean-ups, we really need to be size concious as well as it all keeps
>>> >> > adding up.  Thanks!
>>> >>
>>> >> It include size a bit - e.g. x86 808 bytes of text, although that does
>>> >> include a few extra features.
>>> >
>>> > How about if we don't include some of the extra debug/demo type features
>>> > (which are useful at times, certainly) ?  We keep adding things that add
>>> > a few bytes here and a few bytes there and it all adds up.
>>>
>>> Yes it's very important that the base version doesn't increase size,
>>> or at least only minimally. I should have examined that more closely
>>> in the RFC - my intent was really to get comments on the approach,
>>>
>>> >
>>> > [snip]
>>> >> I think I can use a linker-list approach to reduce the overhead. But I
>>> >> still think the driver has value as it provides a means of adding
>>> >> hooks to do board-specific things from drivers, something that we keep
>>> >> running into. Also the idea of a board 'driver' seems conceptually
>>> >> useful.
>>> >>
>>> >> So one approach would be to have:
>>> >>
>>> >> 1. A linker-list-based board hook setup, where you can declare, for example:
>>> >>
>>> >> static int ivybridge_dram_init(void)
>>> >> {
>>> >>  ...
>>> >> }
>>> >> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
>>> >>
>>> >> This should be pretty cheap, perhaps <200 bytes with some care.
>>> >>
>>> >>
>>> >> 2. An optional BOARD uclass which can be used for more involved
>>> >> situations, but with a higher code size penalty.
>>> >
>>> > OK.  But I also recall that we had talked about trying to condense and
>>> > re-factor things.  My worry about an approach like this is it allows for
>>> > us to more easily get back into the bad habits of having each
>>> > architecture solve similar problems with different solutions.
>>>
>>> Yes that's true and I've been pushing back on this for a while. For
>>> example there is quite a bit of pressure to add board-specific init
>>> code to drivers with driver model. So far I think we have been able to
>>> avoid this using device tree and other drivers. For example if MMC
>>> needs a clock we can find the required clock by phandle and call the
>>> clock driver.
>>>
>>> So are you thinking we should limit this to just a simple hook
>>> approach for now, and then consider the board uclass down the track?
>>
>> Looking over init_sequence_[rf], I can certainly see the case for "ug,
>> this is ugly and we need to make it better" (and I now wonder if we
>> don't have a lot more places where we need INIT_FUNC_WATCHDOG_RESET,
>> anyhow...).  But for the moment we seem to be able to resist adding more
>> calls here.  And I would like to see if we can rework / reduce the
>> current table before we try and simplify and clean-up the mechanism that
>> we use to handle them.
>
> I agree, and I have some concern that making it easier to extend the
> init sequence might end up with less consistency between archs as to
> the sequence we go through during init.
>
> For now I've done two series to tidy up board_f. There is more to do
> though. We can park this series until we get a bit closer (it might be
> quite a while).
>

I did not track this series. What's our next step regarding to this
series? I see some of them are x86-specific which I can apply, no?

Regards,
Bin

[U-Boot] [PATCH 01/16] x86: Drop leading spaces in cpu_x86_get_desc()

[Bin Meng]

On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
> The Intel CPU name can have leading spaces. Remove them since they are not
> useful.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/cpu/cpu_x86.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 02/16] x86: Display the SPL banner only once

[Bin Meng]

Hi Simon,

On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
> At present on a cold reboot we must reset the CPU to get it to full speed.
> With 64-bit U-Boot this happens in SPL. At present we print the banner
> before doing this, the end result being that we print the banner twice.
> Print the banner a little later (after the CPU is ready) to avoid this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/lib/spl.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>

Looks no difference when testing this on QEMU 64-bit. Am I missing anything?

Regards,
Bin

[U-Boot] [PATCH 03/16] x86: config: Enable dhrystone command for link

[Bin Meng]

On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
> Enable this command so we can get an approximate performance measurement.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  configs/chromebook_link64_defconfig | 1 +
>  configs/chromebook_link_defconfig   | 1 +
>  2 files changed, 2 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 02/16] x86: Display the SPL banner only once

[Simon Glass]

Hi Bin,

On 11 April 2017 at 21:09, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
> > At present on a cold reboot we must reset the CPU to get it to full speed.
> > With 64-bit U-Boot this happens in SPL. At present we print the banner
> > before doing this, the end result being that we print the banner twice.
> > Print the banner a little later (after the CPU is ready) to avoid this.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  arch/x86/lib/spl.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> >
>
> Looks no difference when testing this on QEMU 64-bit. Am I missing anything?

I suspect that QEMU doesn't have this problem, but on chromebook_link
we have to reset to change the CPU speed.

Regards,
Simon

[U-Boot] [PATCH 00/16] RFC: Board init using driver model

[Simon Glass]

Hi Bin,

On 11 April 2017 at 20:39, Bin Meng <bmeng.cn@gmail.com> wrote:
> Hi Simon,
>
> On Sat, Apr 1, 2017 at 12:21 PM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Tom,
>>
>> On 22 March 2017 at 09:13, Tom Rini <trini@konsulko.com> wrote:
>>> On Wed, Mar 22, 2017 at 08:43:54AM -0600, Simon Glass wrote:
>>>> Hi Tom,
>>>>
>>>> On 22 March 2017 at 08:37, Tom Rini <trini@konsulko.com> wrote:
>>>> > On Wed, Mar 22, 2017 at 07:05:38AM -0600, Simon Glass wrote:
>>>> >> Hi Tom,
>>>> >>
>>>> >> On 19 March 2017 at 18:47, Tom Rini <trini@konsulko.com> wrote:
>>>> >> > On Sun, Mar 19, 2017 at 12:59:19PM -0600, Simon Glass wrote:
>>>> >> >> At present we have a lot of ad-hoc init functions related to boards, for
>>>> >> >> example board_early_init_f(), board_misc_init_f() and dram_init().
>>>> >> >>
>>>> >> >> There are used in different ways by different boards as useful hooks to
>>>> >> >> do the required init and sequence it correctly. Some functions are always
>>>> >> >> enabled but have a __weak default. Some are controlled by the existence
>>>> >> >> of a CONFIG.
>>>> >> >>
>>>> >> >> There are two main init sequences: board_init_f() (f for running from
>>>> >> >> read-only flash) which runs before relocation and board_init_r() (r for
>>>> >> >> relocated) which runs afterwards.
>>>> >> >>
>>>> >> >> One problem with the current sequence is that it has a lot of
>>>> >> >> arch-specific #ifdefs around various functions. There are also #ifdefs
>>>> >> >> for various features. There has been quite a bit of discussion about how
>>>> >> >> to tidy this up and at least one RFC series[1].
>>>> >> >>
>>>> >> >> Now that we have driver model we can use this to deal with the init
>>>> >> >> sequences. This approach has several advantages:
>>>> >> >>
>>>> >> >> - We have a path to remove the #ifdefs
>>>> >> >> - It is easy for multiple parts of the code to implement the same hook
>>>> >> >> - We can track what is called and what is not
>>>> >> >> - We don't need weak functions
>>>> >> >> - We can eventually adjust the sequence to improve naming or to add new
>>>> >> >> init phases
>>>> >> >> - It provides a model for how we might deal with ft_board_setup() and
>>>> >> >> friends
>>>> >> >>
>>>> >> >> This series starts the process of replacing the pre-relocation init
>>>> >> >> sequence with a driver-model solution. It defines a uclass, adds tests
>>>> >> >> and converts sandbox and a few x86 boards over to use this new setup.
>>>> >> >>
>>>> >> >> This series is not ready for use yet as the rest of the init sequence
>>>> >> >> hooks need to be converted. But there is enough here to show the idea.
>>>> >> >>
>>>> >> >> Comments welcome.
>>>> >> >>
>>>> >> >> [1] https://lists.denx.de/pipermail/u-boot/2011-August/098718.html
>>>> >> >
>>>> >> > How does this look, size wise?  With all of these conversions and
>>>> >> > clean-ups, we really need to be size concious as well as it all keeps
>>>> >> > adding up.  Thanks!
>>>> >>
>>>> >> It include size a bit - e.g. x86 808 bytes of text, although that does
>>>> >> include a few extra features.
>>>> >
>>>> > How about if we don't include some of the extra debug/demo type features
>>>> > (which are useful at times, certainly) ?  We keep adding things that add
>>>> > a few bytes here and a few bytes there and it all adds up.
>>>>
>>>> Yes it's very important that the base version doesn't increase size,
>>>> or at least only minimally. I should have examined that more closely
>>>> in the RFC - my intent was really to get comments on the approach,
>>>>
>>>> >
>>>> > [snip]
>>>> >> I think I can use a linker-list approach to reduce the overhead. But I
>>>> >> still think the driver has value as it provides a means of adding
>>>> >> hooks to do board-specific things from drivers, something that we keep
>>>> >> running into. Also the idea of a board 'driver' seems conceptually
>>>> >> useful.
>>>> >>
>>>> >> So one approach would be to have:
>>>> >>
>>>> >> 1. A linker-list-based board hook setup, where you can declare, for example:
>>>> >>
>>>> >> static int ivybridge_dram_init(void)
>>>> >> {
>>>> >>  ...
>>>> >> }
>>>> >> U_BOOT_BOARD_HOOK(ivybridge_dram_init, BOARD_F_DRAM_INIT);
>>>> >>
>>>> >> This should be pretty cheap, perhaps <200 bytes with some care.
>>>> >>
>>>> >>
>>>> >> 2. An optional BOARD uclass which can be used for more involved
>>>> >> situations, but with a higher code size penalty.
>>>> >
>>>> > OK.  But I also recall that we had talked about trying to condense and
>>>> > re-factor things.  My worry about an approach like this is it allows for
>>>> > us to more easily get back into the bad habits of having each
>>>> > architecture solve similar problems with different solutions.
>>>>
>>>> Yes that's true and I've been pushing back on this for a while. For
>>>> example there is quite a bit of pressure to add board-specific init
>>>> code to drivers with driver model. So far I think we have been able to
>>>> avoid this using device tree and other drivers. For example if MMC
>>>> needs a clock we can find the required clock by phandle and call the
>>>> clock driver.
>>>>
>>>> So are you thinking we should limit this to just a simple hook
>>>> approach for now, and then consider the board uclass down the track?
>>>
>>> Looking over init_sequence_[rf], I can certainly see the case for "ug,
>>> this is ugly and we need to make it better" (and I now wonder if we
>>> don't have a lot more places where we need INIT_FUNC_WATCHDOG_RESET,
>>> anyhow...).  But for the moment we seem to be able to resist adding more
>>> calls here.  And I would like to see if we can rework / reduce the
>>> current table before we try and simplify and clean-up the mechanism that
>>> we use to handle them.
>>
>> I agree, and I have some concern that making it easier to extend the
>> init sequence might end up with less consistency between archs as to
>> the sequence we go through during init.
>>
>> For now I've done two series to tidy up board_f. There is more to do
>> though. We can park this series until we get a bit closer (it might be
>> quite a while).
>>
>
> I did not track this series. What's our next step regarding to this
> series? I see some of them are x86-specific which I can apply, no?

I think it is on hold for now. But yes you can apply the x86 patches.

I may come back to this later after the init sequence is tidied more,
hopefully later in the year.

Regards,
Simon

[U-Boot] [PATCH 02/16] x86: Display the SPL banner only once

[Bin Meng]

On Wed, Apr 12, 2017 at 11:29 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Bin,
>
> On 11 April 2017 at 21:09, Bin Meng <bmeng.cn@gmail.com> wrote:
>>
>> Hi Simon,
>>
>> On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
>> > At present on a cold reboot we must reset the CPU to get it to full speed.
>> > With 64-bit U-Boot this happens in SPL. At present we print the banner
>> > before doing this, the end result being that we print the banner twice.
>> > Print the banner a little later (after the CPU is ready) to avoid this.
>> >
>> > Signed-off-by: Simon Glass <sjg@chromium.org>
>> > ---
>> >
>> >  arch/x86/lib/spl.c | 3 +--
>> >  1 file changed, 1 insertion(+), 2 deletions(-)
>> >
>>
>> Looks no difference when testing this on QEMU 64-bit. Am I missing anything?
>
> I suspect that QEMU doesn't have this problem, but on chromebook_link
> we have to reset to change the CPU speed.
>

That makes sense.

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Tested on QEMU
Tested-by: Bin Meng <bmeng.cn@gmail.com>

[U-Boot] [PATCH 01/16] x86: Drop leading spaces in cpu_x86_get_desc()

[Bin Meng]

On Wed, Apr 12, 2017 at 11:09 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
>> The Intel CPU name can have leading spaces. Remove them since they are not
>> useful.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  arch/x86/cpu/cpu_x86.c | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

[U-Boot] [PATCH 02/16] x86: Display the SPL banner only once

[Bin Meng]

On Tue, Apr 18, 2017 at 3:30 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Wed, Apr 12, 2017 at 11:29 AM, Simon Glass <sjg@chromium.org> wrote:
>> Hi Bin,
>>
>> On 11 April 2017 at 21:09, Bin Meng <bmeng.cn@gmail.com> wrote:
>>>
>>> Hi Simon,
>>>
>>> On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
>>> > At present on a cold reboot we must reset the CPU to get it to full speed.
>>> > With 64-bit U-Boot this happens in SPL. At present we print the banner
>>> > before doing this, the end result being that we print the banner twice.
>>> > Print the banner a little later (after the CPU is ready) to avoid this.
>>> >
>>> > Signed-off-by: Simon Glass <sjg@chromium.org>
>>> > ---
>>> >
>>> >  arch/x86/lib/spl.c | 3 +--
>>> >  1 file changed, 1 insertion(+), 2 deletions(-)
>>> >
>>>
>>> Looks no difference when testing this on QEMU 64-bit. Am I missing anything?
>>
>> I suspect that QEMU doesn't have this problem, but on chromebook_link
>> we have to reset to change the CPU speed.
>>
>
> That makes sense.
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Tested on QEMU
> Tested-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!

[U-Boot] [PATCH 03/16] x86: config: Enable dhrystone command for link

[Bin Meng]

On Wed, Apr 12, 2017 at 11:09 AM, Bin Meng <bmeng.cn@gmail.com> wrote:
> On Mon, Mar 20, 2017 at 2:59 AM, Simon Glass <sjg@chromium.org> wrote:
>> Enable this command so we can get an approximate performance measurement.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>> ---
>>
>>  configs/chromebook_link64_defconfig | 1 +
>>  configs/chromebook_link_defconfig   | 1 +
>>  2 files changed, 2 insertions(+)
>>
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

applied to u-boot-x86, thanks!