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

Submitted by Simon Glass on March 19, 2017, 6:59 p.m.

Details

Message ID 20170319185935.20950-5-sjg@chromium.org
State Changes Requested
Delegated to: Bin Meng
Headers show

Commit Message

Simon Glass March 19, 2017, 6:59 p.m.
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

Comments

Igor Grinberg March 20, 2017, 7:54 a.m.
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)
> +

[...]
Simon Glass March 22, 2017, 1:05 p.m.
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

Patch hide | download patch | download mbox

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@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@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 */