[U-Boot] [PATCH v2 1/3] drivers: Add board uclass
Since there is no canonical "board device" that can be used in board files, it is difficult to use DM function for board initialization in these cases.
Hence, add a uclass that implements a simple "board device", which can hold devices not suitable anywhere else in the device tree, and is also able to read encoded information, e.g. hard-wired GPIOs on a GPIO expander, read-only memory ICs, etc. that carry information about the hardware.
The devices of this uclass expose methods to read generic data types (integers, strings, booleans) to encode the information provided by the hardware.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v1 -> v2: * Corrected description of dev parameter of devinfo_detect * Added size parameter to devinfo_get_str * Expanded uclass documentation * Added function to get devinfo instance * Renamed the uclass from devinfo to board
--- drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/board/Kconfig | 17 +++++++ drivers/board/Makefile | 9 ++++ drivers/board/board-uclass.c | 61 +++++++++++++++++++++++ include/board.h | 115 +++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + 7 files changed, 206 insertions(+) create mode 100644 drivers/board/Kconfig create mode 100644 drivers/board/Makefile create mode 100644 drivers/board/board-uclass.c create mode 100644 include/board.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index c2e813f5ad..19c7c448c0 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -22,6 +22,8 @@ source "drivers/ddr/Kconfig"
source "drivers/demo/Kconfig"
+source "drivers/board/Kconfig" + source "drivers/ddr/fsl/Kconfig"
source "drivers/dfu/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 6846d181aa..fecf64021c 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -72,6 +72,7 @@ obj-y += block/ obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ +obj-y += board/ obj-y += firmware/ obj-$(CONFIG_FPGA) += fpga/ obj-y += misc/ diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig new file mode 100644 index 0000000000..cc1cf27205 --- /dev/null +++ b/drivers/board/Kconfig @@ -0,0 +1,17 @@ +menuconfig BOARD + bool "Device Information" + help + Support methods to query hardware configurations from internal + mechanisms (e.g. reading GPIO values, determining the presence of + devices on busses, etc.). This enables the usage of U-Boot with + modular board architectures. + +if BOARD + + +config BOARD_GAZERBEAM + bool "Enable device information for the Gazerbeam board" + help + Support querying device information for the gdsys Gazerbeam board. + +endif diff --git a/drivers/board/Makefile b/drivers/board/Makefile new file mode 100644 index 0000000000..397706245a --- /dev/null +++ b/drivers/board/Makefile @@ -0,0 +1,9 @@ +# +# (C) Copyright 2017 +# Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc +# +# SPDX-License-Identifier: GPL-2.0+ +# + +obj-$(CONFIG_BOARD) += board-uclass.o +obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o diff --git a/drivers/board/board-uclass.c b/drivers/board/board-uclass.c new file mode 100644 index 0000000000..3137437245 --- /dev/null +++ b/drivers/board/board-uclass.c @@ -0,0 +1,61 @@ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <board.h> + +int get_board(struct udevice **devp) +{ + return uclass_first_device_err(UCLASS_BOARD, devp); +} + +int board_detect(struct udevice *dev) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->detect) + return -ENOSYS; + + return ops->detect(dev); +} + +int board_get_bool(struct udevice *dev, int id, bool *val) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->get_bool) + return -ENOSYS; + + return ops->get_bool(dev, id, val); +} + +int board_get_int(struct udevice *dev, int id, int *val) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->get_int) + return -ENOSYS; + + return ops->get_int(dev, id, val); +} + +int board_get_str(struct udevice *dev, int id, size_t size, char *val) +{ + struct board_ops *ops = board_get_ops(dev); + + if (!ops->get_str) + return -ENOSYS; + + return ops->get_str(dev, id, size, val); +} + +UCLASS_DRIVER(board) = { + .id = UCLASS_BOARD, + .name = "board", + .post_bind = dm_scan_fdt_dev, +}; diff --git a/include/board.h b/include/board.h new file mode 100644 index 0000000000..0e64a3dfce --- /dev/null +++ b/include/board.h @@ -0,0 +1,115 @@ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +/** + * This uclass encapsulates hardware methods to gather information about a + * board or a specific device such as hard-wired GPIOs on GPIO expanders, + * read-only data in flash ICs, or similar. + * + * The interface offers functions to read the usual standard data types (bool, + * int, string) from the device, each of which is identified by a static + * numeric ID (which will usually be defined as a enum in a header file). + * + * If for example dev was a board device representing a read-only serial + * number flash IC, we could call + * + * board_detect(dev); + * board_get_int(dev, ID_SERIAL_NUMBER, &serial); + * + * to read the serial number from the device. + */ + +struct board_ops { + /** + * detect() - Run the hardware info detection procedure for this + * device. + * + * @dev: The device containing the information + * @return 0 if OK, -ve on error. + */ + int (*detect)(struct udevice *dev); + + /** + * get_bool() - Read a specific bool data value that describes the + * hardware setup. + * + * @dev: The board instance to gather the data. + * @id: A unique identifier for the bool value to be read. + * @val: Pointer to a buffer that receives the value read. + * @return 0 if OK, -ve on error. + */ + int (*get_bool)(struct udevice *dev, int id, bool *val); + + /** + * get_int() - Read a specific int data value that describes the + * hardware setup. + * + * @dev: The board instance to gather the data. + * @id: A unique identifier for the int value to be read. + * @val: Pointer to a buffer that receives the value read. + * @return 0 if OK, -ve on error. + */ + int (*get_int)(struct udevice *dev, int id, int *val); + + /** + * get_str() - Read a specific string data value that describes the + * hardware setup. + * + * @dev: The board instance to gather the data. + * @id: A unique identifier for the string value to be read. + * @size: The size of the buffer to receive the string data. + * @val: Pointer to a buffer that receives the value read. + * @return 0 if OK, -ve on error. + */ + int (*get_str)(struct udevice *dev, int id, size_t size, char *val); +}; + +#define board_get_ops(dev) ((struct board_ops *)(dev)->driver->ops) + +/** + * board_detect() - Run the hardware info detection procedure for this device. + * + * @dev: The device containing the information + * @return 0 if OK, -ve on error. + */ +int board_detect(struct udevice *dev); + +/** + * board_get_bool() - Read a specific bool data value that describes the + * hardware setup. + * + * @dev: The board instance to gather the data. + * @id: A unique identifier for the bool value to be read. + * @val: Pointer to a buffer that receives the value read. + * @return 0 if OK, -ve on error. + */ +int board_get_bool(struct udevice *dev, int id, bool *val); + +/** + * board_get_int() - Read a specific int data value that describes the + * hardware setup. + * + * @dev: The board instance to gather the data. + * @id: A unique identifier for the int value to be read. + * @val: Pointer to a buffer that receives the value read. + * @return 0 if OK, -ve on error. + */ +int board_get_int(struct udevice *dev, int id, int *val); + +/** + * board_get_str() - Read a specific string data value that describes the + * hardware setup. + * + * @dev: The board instance to gather the data. + * @id: A unique identifier for the string value to be read. + * @size: The size of the buffer to receive the string data. + * @val: Pointer to a buffer that receives the value read. + * @return 0 if OK, -ve on error. + */ +int board_get_str(struct udevice *dev, int id, size_t size, char *val); + +int get_board(struct udevice **devp); diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index d28fb3e23f..97327194b4 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -30,6 +30,7 @@ enum uclass_id { UCLASS_ADC, /* Analog-to-digital converter */ UCLASS_AHCI, /* SATA disk controller */ UCLASS_BLK, /* Block device */ + UCLASS_BOARD, /* Device information from hardware */ UCLASS_CLK, /* Clock source, e.g. used by peripherals */ UCLASS_CPU, /* CPU, typically part of an SoC */ UCLASS_CROS_EC, /* Chrome OS EC */
Add a board driver for the upcoming gdsys Gazerbeam board.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v1 -> v2: * Improved error handling * Renamed DT properties * Moved the driver over to the board uclass
--- drivers/board/gazerbeam.c | 173 ++++++++++++++++++++++++++++++++++++++++++++++ drivers/board/gazerbeam.h | 17 +++++ 2 files changed, 190 insertions(+) create mode 100644 drivers/board/gazerbeam.c create mode 100644 drivers/board/gazerbeam.h
diff --git a/drivers/board/gazerbeam.c b/drivers/board/gazerbeam.c new file mode 100644 index 0000000000..b4c2a01d76 --- /dev/null +++ b/drivers/board/gazerbeam.c @@ -0,0 +1,173 @@ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <board.h> +#include <i2c.h> +#include <asm/gpio.h> + +#include "gazerbeam.h" + +const int VER_GPIOS_COUNT = 4; + +struct board_gazerbeam_priv { + struct gpio_desc var_gpios[2]; + struct gpio_desc ver_gpios[4]; + int variant; + int multichannel; + int hwversion; +}; + +static int _read_multichannel_variant(struct udevice *dev) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + struct udevice *i2c_bus; + struct udevice *dummy; + char *listname; + int mc4, mc2, sc, con; + int gpio_num; + int res; + + res = uclass_get_device_by_seq(UCLASS_I2C, 1, &i2c_bus); + if (res) { + debug("Could not get I2C bus\n"); + return res; + } + + if (!i2c_bus) { + debug("Could not get I2C bus\n"); + return -EIO; + } + + mc2 = !dm_i2c_probe(i2c_bus, 0x20, 0, &dummy); + mc4 = !dm_i2c_probe(i2c_bus, 0x22, 0, &dummy); + + if (mc2 && mc4) { + debug("Board hardware configuration inconsistent.\n"); + return -EINVAL; + } + + listname = mc2 ? "var-gpios-mc2" : "var-gpios-mc4"; + + gpio_num = gpio_request_list_by_name(dev, listname, priv->var_gpios, 2, + GPIOD_IS_IN); + if (gpio_num < 0) { + debug("Requesting gpio list %s failed.\n", listname); + return gpio_num; + } + + sc = dm_gpio_get_value(&priv->var_gpios[0]); + if (sc < 0) { + debug("Error while reading sc GPIO"); + return sc; + } + + con = dm_gpio_get_value(&priv->var_gpios[1]); + if (con < 0) { + debug("Error while reading con GPIO"); + return con; + } + + if ((sc && mc2) || (sc && mc4) || (!sc && !mc2 && !mc4)) { + debug("Board hardware configuration inconsistent.\n"); + return -EINVAL; + } + + priv->variant = con ? VAR_CON : VAR_CPU; + + priv->multichannel = mc4 ? 4 : (mc2 ? 2 : (sc ? 1 : 0)); + + return 0; +} + +static int _read_hwversion(struct udevice *dev) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + int res; + + if (!gpio_request_list_by_name(dev, "ver-gpios", priv->ver_gpios, + VER_GPIOS_COUNT, GPIOD_IS_IN)) + return -ENODEV; + + res = dm_gpio_get_values_as_int(priv->ver_gpios, VER_GPIOS_COUNT); + + if (res < 0) { + debug("Error while reading HW version from GPIO expander\n"); + return res; + } + + priv->hwversion = res; + + res = gpio_free_list(dev, priv->ver_gpios, VER_GPIOS_COUNT); + if (res < 0) { + debug("Error while freeing HW version GPIO list\n"); + return res; + } + + return 0; +} + +int board_gazerbeam_detect(struct udevice *dev) +{ + int res; + + res = _read_multichannel_variant(dev); + if (res) + return res; + + res = _read_hwversion(dev); + if (res) + return res; + + return 0; +} + +int board_gazerbeam_get_int(struct udevice *dev, int id, int *val) +{ + struct board_gazerbeam_priv *priv = dev_get_priv(dev); + + switch (id) { + case BOARD_MULTICHANNEL: + *val = priv->multichannel; + break; + case BOARD_VARIANT: + *val = priv->variant; + break; + case BOARD_HWVERSION: + *val = priv->hwversion; + break; + default: + return -EINVAL; + } + + return 0; +} + +static const struct udevice_id board_gazerbeam_ids[] = { + { .compatible = "gdsys,board_gazerbeam" }, + { /* sentinel */ } +}; + +static const struct board_ops board_gazerbeam_ops = { + .detect = board_gazerbeam_detect, + .get_int = board_gazerbeam_get_int, +}; + +int board_gazerbeam_probe(struct udevice *dev) +{ + return 0; +} + +U_BOOT_DRIVER(board_gazerbeam) = { + .name = "board_gazerbeam", + .id = UCLASS_BOARD, + .of_match = board_gazerbeam_ids, + .ops = &board_gazerbeam_ops, + .priv_auto_alloc_size = sizeof(struct board_gazerbeam_priv), + .probe = board_gazerbeam_probe, +}; diff --git a/drivers/board/gazerbeam.h b/drivers/board/gazerbeam.h new file mode 100644 index 0000000000..532001a053 --- /dev/null +++ b/drivers/board/gazerbeam.h @@ -0,0 +1,17 @@ +/* + * (C) Copyright 2017 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +enum { + BOARD_MULTICHANNEL, + BOARD_VARIANT, + BOARD_HWVERSION, +}; + +enum { + VAR_CON, + VAR_CPU, +};
Add tests for the new board uclass.
Signed-off-by: Mario Six mario.six@gdsys.cc ---
v1 -> v2: New in v2
--- arch/sandbox/dts/test.dts | 4 ++ configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 2 + configs/sandbox_flattree_defconfig | 2 + configs/sandbox_noblk_defconfig | 2 + configs/sandbox_spl_defconfig | 2 + drivers/board/Kconfig | 7 ++- drivers/board/Makefile | 1 + drivers/board/sandbox.c | 108 +++++++++++++++++++++++++++++++++++++ drivers/board/sandbox.h | 13 +++++ test/dm/Makefile | 1 + test/dm/board.c | 57 ++++++++++++++++++++ 12 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 drivers/board/sandbox.c create mode 100644 drivers/board/sandbox.h create mode 100644 test/dm/board.c
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 06d0e8ce85..4f5f1c8474 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -535,6 +535,10 @@ }; }; }; + + board { + compatible = "sandbox,board_sandbox"; + }; };
#include "sandbox_pmic.dtsi" diff --git a/configs/sandbox64_defconfig b/configs/sandbox64_defconfig index ff60508735..8e665c2073 100644 --- a/configs/sandbox64_defconfig +++ b/configs/sandbox64_defconfig @@ -87,6 +87,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index fd39519bed..3a2b9755bd 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -87,6 +87,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 9b8d033838..d64b018510 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -69,6 +69,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig index 8bdd4edcda..5e8a448547 100644 --- a/configs/sandbox_noblk_defconfig +++ b/configs/sandbox_noblk_defconfig @@ -76,6 +76,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index c308628199..d0e58dbffa 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -90,6 +90,8 @@ CONFIG_CPU=y CONFIG_DM_DEMO=y CONFIG_DM_DEMO_SIMPLE=y CONFIG_DM_DEMO_SHAPE=y +CONFIG_BOARD=y +CONFIG_BOARD_SANDBOX=y CONFIG_PM8916_GPIO=y CONFIG_SANDBOX_GPIO=y CONFIG_DM_I2C_COMPAT=y diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig index cc1cf27205..2a3fc9c049 100644 --- a/drivers/board/Kconfig +++ b/drivers/board/Kconfig @@ -10,8 +10,13 @@ if BOARD
config BOARD_GAZERBEAM - bool "Enable device information for the Gazerbeam board" + bool "Enable board driver for the Gazerbeam board" help Support querying device information for the gdsys Gazerbeam board.
+config BOARD_SANDBOX + bool "Enable board driver for the Sandbox board" + help + Support querying device information for the Sandbox boards. + endif diff --git a/drivers/board/Makefile b/drivers/board/Makefile index 397706245a..5a6e782e3b 100644 --- a/drivers/board/Makefile +++ b/drivers/board/Makefile @@ -7,3 +7,4 @@
obj-$(CONFIG_BOARD) += board-uclass.o obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_BOARD_SANDBOX) += sandbox.o diff --git a/drivers/board/sandbox.c b/drivers/board/sandbox.c new file mode 100644 index 0000000000..3fd6d77780 --- /dev/null +++ b/drivers/board/sandbox.c @@ -0,0 +1,108 @@ +/* + * (C) Copyright 2018 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <board.h> + +#include "sandbox.h" + +struct board_sandbox_priv { + bool called_detect; + int test_i1; + int test_i2; +}; + +char vacation_spots[][64] = {"R'lyeh", "Dreamlands", "Plateau of Leng", + "Carcosa", "Yuggoth", "The Nameless City"}; + +int board_sandbox_detect(struct udevice *dev) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + + priv->called_detect = true; + priv->test_i2 = 100; + + return 0; +} + +int board_sandbox_get_bool(struct udevice *dev, int id, bool *val) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + + switch (id) { + case BOOL_CALLED_DETECT: + /* Checks if the dectect method has been called */ + *val = priv->called_detect; + return 0; + } + + return -ENOENT; +} + +int board_sandbox_get_int(struct udevice *dev, int id, int *val) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + + switch (id) { + case INT_TEST1: + *val = priv->test_i1; + /* Increments with every call */ + priv->test_i1++; + return 0; + case INT_TEST2: + *val = priv->test_i2; + /* Decrements with every call */ + priv->test_i2--; + return 0; + } + + return -ENOENT; +} + +int board_sandbox_get_str(struct udevice *dev, int id, size_t size, char *val) +{ + struct board_sandbox_priv *priv = dev_get_priv(dev); + int i1 = priv->test_i1; + int i2 = priv->test_i2; + int index = (i1 * i2) % ARRAY_SIZE(vacation_spots); + + switch (id) { + case STR_VACATIONSPOT: + /* Picks a vacation spot depending on i1 and i2 */ + snprintf(val, size, vacation_spots[index]); + return 0; + } + + return -ENOENT; +} + +static const struct udevice_id board_sandbox_ids[] = { + { .compatible = "sandbox,board_sandbox" }, + { /* sentinel */ } +}; + +static const struct board_ops board_sandbox_ops = { + .detect = board_sandbox_detect, + .get_bool = board_sandbox_get_bool, + .get_int = board_sandbox_get_int, + .get_str = board_sandbox_get_str, +}; + +int board_sandbox_probe(struct udevice *dev) +{ + return 0; +} + +U_BOOT_DRIVER(board_sandbox) = { + .name = "board_sandbox", + .id = UCLASS_BOARD, + .of_match = board_sandbox_ids, + .ops = &board_sandbox_ops, + .priv_auto_alloc_size = sizeof(struct board_sandbox_priv), + .probe = board_sandbox_probe, +}; diff --git a/drivers/board/sandbox.h b/drivers/board/sandbox.h new file mode 100644 index 0000000000..4f5a02d13a --- /dev/null +++ b/drivers/board/sandbox.h @@ -0,0 +1,13 @@ +/* + * (C) Copyright 2018 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +enum { + BOOL_CALLED_DETECT, + INT_TEST1, + INT_TEST2, + STR_VACATIONSPOT, +}; diff --git a/test/dm/Makefile b/test/dm/Makefile index 513c4561ad..0583f45c7b 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..700c280193 --- /dev/null +++ b/test/dm/board.c @@ -0,0 +1,57 @@ +/* + * (C) Copyright 2018 + * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc + * + * SPDX-License-Identifier: GPL-2.0+ + */ + +#include <common.h> +#include <dm.h> +#include <dm/test.h> +#include <board.h> +#include <test/ut.h> + +#include "../../drivers/board/sandbox.h" + +static int dm_test_board(struct unit_test_state *uts) +{ + struct udevice *board; + bool called_detect; + char str[64]; + int i; + + get_board(&board); + ut_assert(board); + + board_get_bool(board, BOOL_CALLED_DETECT, &called_detect); + ut_assert(!called_detect); + + board_detect(board); + + board_get_bool(board, BOOL_CALLED_DETECT, &called_detect); + ut_assert(called_detect); + + board_get_str(board, STR_VACATIONSPOT, sizeof(str), str); + ut_assertok(strcmp(str, "R'lyeh")); + + board_get_int(board, INT_TEST1, &i); + ut_asserteq(0, i); + + board_get_int(board, INT_TEST2, &i); + ut_asserteq(100, i); + + board_get_str(board, STR_VACATIONSPOT, sizeof(str), str); + ut_assertok(strcmp(str, "Carcosa")); + + board_get_int(board, INT_TEST1, &i); + ut_asserteq(1, i); + + board_get_int(board, INT_TEST2, &i); + ut_asserteq(99, i); + + board_get_str(board, STR_VACATIONSPOT, sizeof(str), str); + ut_assertok(strcmp(str, "Yuggoth")); + + return 0; +} +DM_TEST(dm_test_board, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
On 27 April 2018 at 06:51, Mario Six mario.six@gdsys.cc wrote:
Add tests for the new board uclass.
Signed-off-by: Mario Six mario.six@gdsys.cc
v1 -> v2: New in v2
arch/sandbox/dts/test.dts | 4 ++ configs/sandbox64_defconfig | 2 + configs/sandbox_defconfig | 2 + configs/sandbox_flattree_defconfig | 2 + configs/sandbox_noblk_defconfig | 2 + configs/sandbox_spl_defconfig | 2 + drivers/board/Kconfig | 7 ++- drivers/board/Makefile | 1 + drivers/board/sandbox.c | 108 +++++++++++++++++++++++++++++++++++++ drivers/board/sandbox.h | 13 +++++ test/dm/Makefile | 1 + test/dm/board.c | 57 ++++++++++++++++++++ 12 files changed, 200 insertions(+), 1 deletion(-) create mode 100644 drivers/board/sandbox.c create mode 100644 drivers/board/sandbox.h create mode 100644 test/dm/board.c
Reviewed-by: Simon Glass sjg@chromium.org
On 27 Apr 2018, at 14:51, Mario Six mario.six@gdsys.cc wrote:
Since there is no canonical "board device" that can be used in board files, it is difficult to use DM function for board initialization in these cases.
Hence, add a uclass that implements a simple "board device", which can hold devices not suitable anywhere else in the device tree, and is also able to read encoded information, e.g. hard-wired GPIOs on a GPIO expander, read-only memory ICs, etc. that carry information about the hardware.
A board-uclass should model a board (i.e. provide implementations for the same key abstractions that we have in place today: e.g., board_init).
This seems more like a specialised form of a misc-device.
The devices of this uclass expose methods to read generic data types (integers, strings, booleans) to encode the information provided by the hardware.
Again, this is what a misc-device is intended for… and extending the misc-device APIs (or having a convenience layer on top of its ioctl interface?) might be more appropriate.
After reviewing the below, the similarities to the misc-device are even more apparent: if you need typed access to a key-value pair, this can be easily implemented through a common ioctl with semantic sugar at the misc-uclass level.
Signed-off-by: Mario Six mario.six@gdsys.cc
v1 -> v2:
- Corrected description of dev parameter of devinfo_detect
- Added size parameter to devinfo_get_str
- Expanded uclass documentation
- Added function to get devinfo instance
- Renamed the uclass from devinfo to board
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/board/Kconfig | 17 +++++++ drivers/board/Makefile | 9 ++++ drivers/board/board-uclass.c | 61 +++++++++++++++++++++++ include/board.h | 115 +++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + 7 files changed, 206 insertions(+) create mode 100644 drivers/board/Kconfig create mode 100644 drivers/board/Makefile create mode 100644 drivers/board/board-uclass.c create mode 100644 include/board.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index c2e813f5ad..19c7c448c0 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -22,6 +22,8 @@ source "drivers/ddr/Kconfig"
source "drivers/demo/Kconfig"
+source "drivers/board/Kconfig"
source "drivers/ddr/fsl/Kconfig"
source "drivers/dfu/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 6846d181aa..fecf64021c 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -72,6 +72,7 @@ obj-y += block/ obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ +obj-y += board/ obj-y += firmware/ obj-$(CONFIG_FPGA) += fpga/ obj-y += misc/ diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig new file mode 100644 index 0000000000..cc1cf27205 --- /dev/null +++ b/drivers/board/Kconfig @@ -0,0 +1,17 @@ +menuconfig BOARD
- bool "Device Information"
- help
Support methods to query hardware configurations from internalmechanisms (e.g. reading GPIO values, determining the presence ofdevices on busses, etc.). This enables the usage of U-Boot withmodular board architectures.+if BOARD
+config BOARD_GAZERBEAM
- bool "Enable device information for the Gazerbeam board"
- help
Support querying device information for the gdsys Gazerbeam board.+endif diff --git a/drivers/board/Makefile b/drivers/board/Makefile new file mode 100644 index 0000000000..397706245a --- /dev/null +++ b/drivers/board/Makefile @@ -0,0 +1,9 @@ +# +# (C) Copyright 2017 +# Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc +# +# SPDX-License-Identifier: GPL-2.0+ +#
+obj-$(CONFIG_BOARD) += board-uclass.o +obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o diff --git a/drivers/board/board-uclass.c b/drivers/board/board-uclass.c new file mode 100644 index 0000000000..3137437245 --- /dev/null +++ b/drivers/board/board-uclass.c @@ -0,0 +1,61 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <board.h>
+int get_board(struct udevice **devp) +{
- return uclass_first_device_err(UCLASS_BOARD, devp);
+}
+int board_detect(struct udevice *dev) +{
- struct board_ops *ops = board_get_ops(dev);
- if (!ops->detect)
return -ENOSYS;- return ops->detect(dev);
+}
+int board_get_bool(struct udevice *dev, int id, bool *val) +{
- struct board_ops *ops = board_get_ops(dev);
- if (!ops->get_bool)
return -ENOSYS;- return ops->get_bool(dev, id, val);
+}
+int board_get_int(struct udevice *dev, int id, int *val) +{
- struct board_ops *ops = board_get_ops(dev);
- if (!ops->get_int)
return -ENOSYS;- return ops->get_int(dev, id, val);
+}
+int board_get_str(struct udevice *dev, int id, size_t size, char *val) +{
- struct board_ops *ops = board_get_ops(dev);
- if (!ops->get_str)
return -ENOSYS;- return ops->get_str(dev, id, size, val);
+}
+UCLASS_DRIVER(board) = {
- .id = UCLASS_BOARD,
- .name = "board",
- .post_bind = dm_scan_fdt_dev,
+}; diff --git a/include/board.h b/include/board.h new file mode 100644 index 0000000000..0e64a3dfce --- /dev/null +++ b/include/board.h @@ -0,0 +1,115 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+/**
- This uclass encapsulates hardware methods to gather information about a
- board or a specific device such as hard-wired GPIOs on GPIO expanders,
- read-only data in flash ICs, or similar.
- The interface offers functions to read the usual standard data types (bool,
- int, string) from the device, each of which is identified by a static
- numeric ID (which will usually be defined as a enum in a header file).
- If for example dev was a board device representing a read-only serial
- number flash IC, we could call
- board_detect(dev);
- board_get_int(dev, ID_SERIAL_NUMBER, &serial);
- to read the serial number from the device.
- */
+struct board_ops {
- /**
* detect() - Run the hardware info detection procedure for this* device.** @dev: The device containing the information* @return 0 if OK, -ve on error.*/- int (*detect)(struct udevice *dev);
This doesn’t make any sense, as the probe entry-point is intended to probe for a device.
- /**
* get_bool() - Read a specific bool data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the bool value to be read.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/- int (*get_bool)(struct udevice *dev, int id, bool *val);
- /**
* get_int() - Read a specific int data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the int value to be read.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/- int (*get_int)(struct udevice *dev, int id, int *val);
- /**
* get_str() - Read a specific string data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the string value to be read.* @size: The size of the buffer to receive the string data.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/- int (*get_str)(struct udevice *dev, int id, size_t size, char *val);
These could all be handled as ioctl()-calls to a misc-device by defining a common protocol and convenience layer.
+};
+#define board_get_ops(dev) ((struct board_ops *)(dev)->driver->ops)
+/**
- board_detect() - Run the hardware info detection procedure for this device.
- @dev: The device containing the information
- @return 0 if OK, -ve on error.
- */
+int board_detect(struct udevice *dev);
+/**
- board_get_bool() - Read a specific bool data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the bool value to be read.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_bool(struct udevice *dev, int id, bool *val);
+/**
- board_get_int() - Read a specific int data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the int value to be read.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_int(struct udevice *dev, int id, int *val);
+/**
- board_get_str() - Read a specific string data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the string value to be read.
- @size: The size of the buffer to receive the string data.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_str(struct udevice *dev, int id, size_t size, char *val);
+int get_board(struct udevice **devp); diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index d28fb3e23f..97327194b4 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -30,6 +30,7 @@ enum uclass_id { UCLASS_ADC, /* Analog-to-digital converter */ UCLASS_AHCI, /* SATA disk controller */ UCLASS_BLK, /* Block device */
- UCLASS_BOARD, /* Device information from hardware */ UCLASS_CLK, /* Clock source, e.g. used by peripherals */ UCLASS_CPU, /* CPU, typically part of an SoC */ UCLASS_CROS_EC, /* Chrome OS EC */
-- 2.16.1
Hi,
On 27 April 2018 at 07:02, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
On 27 Apr 2018, at 14:51, Mario Six mario.six@gdsys.cc wrote:
Since there is no canonical "board device" that can be used in board files, it is difficult to use DM function for board initialization in these cases.
Hence, add a uclass that implements a simple "board device", which can hold devices not suitable anywhere else in the device tree, and is also able to read encoded information, e.g. hard-wired GPIOs on a GPIO expander, read-only memory ICs, etc. that carry information about the hardware.
With comments below:
Reviewed-by: Simon Glass sjg@chromium.org
A board-uclass should model a board (i.e. provide implementations for the same key abstractions that we have in place today: e.g., board_init).
This seems more like a specialised form of a misc-device.
The devices of this uclass expose methods to read generic data types (integers, strings, booleans) to encode the information provided by the hardware.
Again, this is what a misc-device is intended for… and extending the misc-device APIs (or having a convenience layer on top of its ioctl interface?) might be more appropriate.
After reviewing the below, the similarities to the misc-device are even more apparent: if you need typed access to a key-value pair, this can be easily implemented through a common ioctl with semantic sugar at the misc-uclass level.
A misc device might be similar in action but it is not the same in terms of concept. For example it would be very strange to have a misc device which points to lots of other devices that are on the board. For example, consider this hypothetical case:
board { identity-eeprom = <&eeprom>; id-gpios = <&gpio0 2>, <&gpio0 4>; ec = <&cros_ec>; };
Here we point to a few devices which may otherwise require specific device-tree references to locate. Already we are seeing this sort of code in U-Boot:
/* find the grf node */ node = fdt_node_offset_by_compatible(blob, -1, "rockchip,rk3288-grf");
ret = regulator_get_by_platname("vdd_arm", &dev); if (ret) { debug("Cannot set regulator name\n"); return ret; }
These are the sorts of things which suggest that some sort of 'board directory', pointing to the things that the board needs to init, would be useful.
Signed-off-by: Mario Six mario.six@gdsys.cc
v1 -> v2:
- Corrected description of dev parameter of devinfo_detect
- Added size parameter to devinfo_get_str
- Expanded uclass documentation
- Added function to get devinfo instance
- Renamed the uclass from devinfo to board
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/board/Kconfig | 17 +++++++ drivers/board/Makefile | 9 ++++ drivers/board/board-uclass.c | 61 +++++++++++++++++++++++ include/board.h | 115 +++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + 7 files changed, 206 insertions(+) create mode 100644 drivers/board/Kconfig create mode 100644 drivers/board/Makefile create mode 100644 drivers/board/board-uclass.c create mode 100644 include/board.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index c2e813f5ad..19c7c448c0 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -22,6 +22,8 @@ source "drivers/ddr/Kconfig"
source "drivers/demo/Kconfig"
+source "drivers/board/Kconfig"
source "drivers/ddr/fsl/Kconfig"
source "drivers/dfu/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 6846d181aa..fecf64021c 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -72,6 +72,7 @@ obj-y += block/ obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ +obj-y += board/ obj-y += firmware/ obj-$(CONFIG_FPGA) += fpga/ obj-y += misc/ diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig new file mode 100644 index 0000000000..cc1cf27205 --- /dev/null +++ b/drivers/board/Kconfig @@ -0,0 +1,17 @@ +menuconfig BOARD
bool "Device Information"helpSupport methods to query hardware configurations from internalmechanisms (e.g. reading GPIO values, determining the presence ofdevices on busses, etc.). This enables the usage of U-Boot withmodular board architectures.+if BOARD
+config BOARD_GAZERBEAM
bool "Enable device information for the Gazerbeam board"helpSupport querying device information for the gdsys Gazerbeam board.+endif diff --git a/drivers/board/Makefile b/drivers/board/Makefile new file mode 100644 index 0000000000..397706245a --- /dev/null +++ b/drivers/board/Makefile @@ -0,0 +1,9 @@ +# +# (C) Copyright 2017 +# Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc +# +# SPDX-License-Identifier: GPL-2.0+ +#
+obj-$(CONFIG_BOARD) += board-uclass.o +obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o diff --git a/drivers/board/board-uclass.c b/drivers/board/board-uclass.c new file mode 100644 index 0000000000..3137437245 --- /dev/null +++ b/drivers/board/board-uclass.c @@ -0,0 +1,61 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <board.h>
+int get_board(struct udevice **devp)
board_get() ?
That way we can use a consistent prefix for all functions.
+{
return uclass_first_device_err(UCLASS_BOARD, devp);+}
+int board_detect(struct udevice *dev) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->detect)return -ENOSYS;return ops->detect(dev);+}
+int board_get_bool(struct udevice *dev, int id, bool *val) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->get_bool)return -ENOSYS;return ops->get_bool(dev, id, val);+}
+int board_get_int(struct udevice *dev, int id, int *val) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->get_int)return -ENOSYS;return ops->get_int(dev, id, val);+}
+int board_get_str(struct udevice *dev, int id, size_t size, char *val) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->get_str)return -ENOSYS;return ops->get_str(dev, id, size, val);+}
+UCLASS_DRIVER(board) = {
.id = UCLASS_BOARD,.name = "board",.post_bind = dm_scan_fdt_dev,+}; diff --git a/include/board.h b/include/board.h new file mode 100644 index 0000000000..0e64a3dfce --- /dev/null +++ b/include/board.h @@ -0,0 +1,115 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+/**
- This uclass encapsulates hardware methods to gather information about a
- board or a specific device such as hard-wired GPIOs on GPIO expanders,
- read-only data in flash ICs, or similar.
- The interface offers functions to read the usual standard data types (bool,
- int, string) from the device, each of which is identified by a static
- numeric ID (which will usually be defined as a enum in a header file).
- If for example dev was a board device representing a read-only serial
- number flash IC, we could call
- board_detect(dev);
- board_get_int(dev, ID_SERIAL_NUMBER, &serial);
Please add error checking code.
- to read the serial number from the device.
- */
+struct board_ops {
/*** detect() - Run the hardware info detection procedure for this* device.** @dev: The device containing the information* @return 0 if OK, -ve on error.*/int (*detect)(struct udevice *dev);This doesn’t make any sense, as the probe entry-point is intended to probe for a device.
I can see your point, but I feel that it does make some sense.
Probing the board device could be used to simply locate all the dependent devices and set them up in a data structure. This might be a fairly fast operation.
Detecting the hardware could take a lot longer: - reading from an EEPROM - calling out to an EC to ask for details - checking for the present of a device on an I2C device
I don't like the idea of probe() taking a really long time (e.g. 50ms). It should be possible to get access to a device without incurring that delay. This way, we can access the devices without necessarily knowing exactly what type of board it is.
The comment above definitely needs expanding to explain why this doesn't happen in probe()
/*** get_bool() - Read a specific bool data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the bool value to be read.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/int (*get_bool)(struct udevice *dev, int id, bool *val);/*** get_int() - Read a specific int data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the int value to be read.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/int (*get_int)(struct udevice *dev, int id, int *val);/*** get_str() - Read a specific string data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the string value to be read.* @size: The size of the buffer to receive the string data.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/int (*get_str)(struct udevice *dev, int id, size_t size, char *val);These could all be handled as ioctl()-calls to a misc-device by defining a common protocol and convenience layer.
It could, and perhaps we should add an ioctl() as well. But I don't see a lot of overhead in adding type-specific methods, and they are convenient to use in the driver and in the caller. For the caller we can add a convenience layer of course, but in the driver that is a bit more of a pain.
+};
+#define board_get_ops(dev) ((struct board_ops *)(dev)->driver->ops)
+/**
- board_detect() - Run the hardware info detection procedure for this device.
- @dev: The device containing the information
- @return 0 if OK, -ve on error.
- */
+int board_detect(struct udevice *dev);
+/**
- board_get_bool() - Read a specific bool data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the bool value to be read.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_bool(struct udevice *dev, int id, bool *val);
+/**
- board_get_int() - Read a specific int data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the int value to be read.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_int(struct udevice *dev, int id, int *val);
+/**
- board_get_str() - Read a specific string data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the string value to be read.
- @size: The size of the buffer to receive the string data.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_str(struct udevice *dev, int id, size_t size, char *val);
+int get_board(struct udevice **devp); diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index d28fb3e23f..97327194b4 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -30,6 +30,7 @@ enum uclass_id { UCLASS_ADC, /* Analog-to-digital converter */ UCLASS_AHCI, /* SATA disk controller */ UCLASS_BLK, /* Block device */
UCLASS_BOARD, /* Device information from hardware */ UCLASS_CLK, /* Clock source, e.g. used by peripherals */ UCLASS_CPU, /* CPU, typically part of an SoC */ UCLASS_CROS_EC, /* Chrome OS EC */-- 2.16.1
Regards, SImon
Simon,
On 1 May 2018, at 01:12, Simon Glass sjg@chromium.org wrote:
Hi,
On 27 April 2018 at 07:02, Dr. Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com> wrote:
On 27 Apr 2018, at 14:51, Mario Six <mario.six@gdsys.cc mailto:mario.six@gdsys.cc> wrote:
Since there is no canonical "board device" that can be used in board files, it is difficult to use DM function for board initialization in these cases.
Hence, add a uclass that implements a simple "board device", which can hold devices not suitable anywhere else in the device tree, and is also able to read encoded information, e.g. hard-wired GPIOs on a GPIO expander, read-only memory ICs, etc. that carry information about the hardware.
With comments below:
Reviewed-by: Simon Glass <sjg@chromium.org mailto:sjg@chromium.org>
A board-uclass should model a board (i.e. provide implementations for the same key abstractions that we have in place today: e.g., board_init).
This seems more like a specialised form of a misc-device.
The devices of this uclass expose methods to read generic data types (integers, strings, booleans) to encode the information provided by the hardware.
Again, this is what a misc-device is intended for… and extending the misc-device APIs (or having a convenience layer on top of its ioctl interface?) might be more appropriate.
After reviewing the below, the similarities to the misc-device are even more apparent: if you need typed access to a key-value pair, this can be easily implemented through a common ioctl with semantic sugar at the misc-uclass level.
A misc device might be similar in action but it is not the same in terms of concept. For example it would be very strange to have a misc device which points to lots of other devices that are on the board. For example, consider this hypothetical case:
board { identity-eeprom = <&eeprom>; id-gpios = <&gpio0 2>, <&gpio0 4>; ec = <&cros_ec>; };
Maybe I am getting hung up on the ‘board’ name, as in my mind the board selection should be based on the top-level .compatible: e.g. compatible = "tsd,rk3399-q7", "tsd,puma", "rockchip,rk3399"; and the selected board-classes could then encapsulate the various init hooks and inquire into devices defined via /aliases, /config and /chosen.
A board-class should restructure existing board.c-files into the .ops that are used there (i.e. init, late-init, usb-init, debug-uart-init, etc.) ... this is different from what’s being proposed here.
The hypothetical case you give is already covered wholly by today’s device model. Let me explain with a counter example: aliases { identity-eeprom = <&eeprom>; id-gpios = <&board-id>; ec = <&cros_ec>; }
board-id { /* a good use for a hypothetical misc-device that exports the values of a variable-length list of GPIOs */ .compatible = “identity-gpios”; id-gpios = <&gpio0 2>, <&gpio0 4>; }
Here we point to a few devices which may otherwise require specific device-tree references to locate. Already we are seeing this sort of code in U-Boot:
/* find the grf node */ node = fdt_node_offset_by_compatible(blob, -1, "rockchip,rk3288-grf”);
This is usually caused by badly-modelled device-trees or missing device tree support in drivers. In this specific case it’s a wart caused by dwc2_udc_probe(…) and that this is not called from the device-tree.
I would hope that this will eventually be included converted to the device model and that the info will be processed from the dwc2_udc driver.
ret = regulator_get_by_platname("vdd_arm", &dev); if (ret) { debug("Cannot set regulator name\n"); return ret; }
I know this specific example, and it’s from a use case, where a voltage needs to be slowly raised. The reason for this being there is that we don’t have the same DTS as in Linux and the modelling (and driver) for the PMU subsystem are not there.
So I don’t seem to follow on how this would resolve itself with the proposed board-uclass.
These are the sorts of things which suggest that some sort of 'board directory', pointing to the things that the board needs to init, would be useful.
In fact, I have been raising the topic of having a ‘board’ and a ‘soc’ uclass a few weeks back in order to structure the boot-up through the device-model.
This is exactly where my comments are coming from: the proposed board uclass (as in this patch-series) does not aim to break up the existing board-files and put them into a framework of .ops in a new board-uclass.
In the current code-base I see cases like
int rk3288_qos_init(void) { int val = 2 << PRIORITY_HIGH_SHIFT | 2 << PRIORITY_LOW_SHIFT; /* set vop qos to higher priority */ writel(val, CPU_AXI_QOS_PRIORITY + VIO0_VOP_QOS); writel(val, CPU_AXI_QOS_PRIORITY + VIO1_VOP_QOS);
if (!fdt_node_check_compatible(gd->fdt_blob, 0, "rockchip,rk3288-tinker")) { /* set isp qos to higher priority */ writel(val, CPU_AXI_QOS_PRIORITY + VIO1_ISP_R_QOS); writel(val, CPU_AXI_QOS_PRIORITY + VIO1_ISP_W0_QOS); writel(val, CPU_AXI_QOS_PRIORITY + VIO1_ISP_W1_QOS); } return 0; }
which clearly requires a multi-step init-process for a soc->init(...) board->init(...) with both derived from compatible = "rockchip,rk3288-tinker", "rockchip,rk3288";
Existing entry points like the debug_uart_init, etc. should get their own .ops-entry …
Signed-off-by: Mario Six mario.six@gdsys.cc
v1 -> v2:
- Corrected description of dev parameter of devinfo_detect
- Added size parameter to devinfo_get_str
- Expanded uclass documentation
- Added function to get devinfo instance
- Renamed the uclass from devinfo to board
drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/board/Kconfig | 17 +++++++ drivers/board/Makefile | 9 ++++ drivers/board/board-uclass.c | 61 +++++++++++++++++++++++ include/board.h | 115 +++++++++++++++++++++++++++++++++++++++++++ include/dm/uclass-id.h | 1 + 7 files changed, 206 insertions(+) create mode 100644 drivers/board/Kconfig create mode 100644 drivers/board/Makefile create mode 100644 drivers/board/board-uclass.c create mode 100644 include/board.h
diff --git a/drivers/Kconfig b/drivers/Kconfig index c2e813f5ad..19c7c448c0 100644 --- a/drivers/Kconfig +++ b/drivers/Kconfig @@ -22,6 +22,8 @@ source "drivers/ddr/Kconfig"
source "drivers/demo/Kconfig"
+source "drivers/board/Kconfig"
source "drivers/ddr/fsl/Kconfig"
source "drivers/dfu/Kconfig" diff --git a/drivers/Makefile b/drivers/Makefile index 6846d181aa..fecf64021c 100644 --- a/drivers/Makefile +++ b/drivers/Makefile @@ -72,6 +72,7 @@ obj-y += block/ obj-$(CONFIG_BOOTCOUNT_LIMIT) += bootcount/ obj-$(CONFIG_CPU) += cpu/ obj-y += crypto/ +obj-y += board/ obj-y += firmware/ obj-$(CONFIG_FPGA) += fpga/ obj-y += misc/ diff --git a/drivers/board/Kconfig b/drivers/board/Kconfig new file mode 100644 index 0000000000..cc1cf27205 --- /dev/null +++ b/drivers/board/Kconfig @@ -0,0 +1,17 @@ +menuconfig BOARD
bool "Device Information"helpSupport methods to query hardware configurations from internalmechanisms (e.g. reading GPIO values, determining the presence ofdevices on busses, etc.). This enables the usage of U-Boot withmodular board architectures.+if BOARD
+config BOARD_GAZERBEAM
bool "Enable device information for the Gazerbeam board"helpSupport querying device information for the gdsys Gazerbeam board.+endif diff --git a/drivers/board/Makefile b/drivers/board/Makefile new file mode 100644 index 0000000000..397706245a --- /dev/null +++ b/drivers/board/Makefile @@ -0,0 +1,9 @@ +# +# (C) Copyright 2017 +# Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc +# +# SPDX-License-Identifier: GPL-2.0+ +#
+obj-$(CONFIG_BOARD) += board-uclass.o +obj-$(CONFIG_BOARD_GAZERBEAM) += gazerbeam.o diff --git a/drivers/board/board-uclass.c b/drivers/board/board-uclass.c new file mode 100644 index 0000000000..3137437245 --- /dev/null +++ b/drivers/board/board-uclass.c @@ -0,0 +1,61 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+#include <common.h> +#include <dm.h> +#include <board.h>
+int get_board(struct udevice **devp)
board_get() ?
That way we can use a consistent prefix for all functions.
+{
return uclass_first_device_err(UCLASS_BOARD, devp);+}
+int board_detect(struct udevice *dev) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->detect)return -ENOSYS;return ops->detect(dev);+}
+int board_get_bool(struct udevice *dev, int id, bool *val) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->get_bool)return -ENOSYS;return ops->get_bool(dev, id, val);+}
+int board_get_int(struct udevice *dev, int id, int *val) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->get_int)return -ENOSYS;return ops->get_int(dev, id, val);+}
+int board_get_str(struct udevice *dev, int id, size_t size, char *val) +{
struct board_ops *ops = board_get_ops(dev);if (!ops->get_str)return -ENOSYS;return ops->get_str(dev, id, size, val);+}
+UCLASS_DRIVER(board) = {
.id = UCLASS_BOARD,.name = "board",.post_bind = dm_scan_fdt_dev,+}; diff --git a/include/board.h b/include/board.h new file mode 100644 index 0000000000..0e64a3dfce --- /dev/null +++ b/include/board.h @@ -0,0 +1,115 @@ +/*
- (C) Copyright 2017
- Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc
- SPDX-License-Identifier: GPL-2.0+
- */
+/**
- This uclass encapsulates hardware methods to gather information about a
- board or a specific device such as hard-wired GPIOs on GPIO expanders,
- read-only data in flash ICs, or similar.
- The interface offers functions to read the usual standard data types (bool,
- int, string) from the device, each of which is identified by a static
- numeric ID (which will usually be defined as a enum in a header file).
- If for example dev was a board device representing a read-only serial
- number flash IC, we could call
- board_detect(dev);
- board_get_int(dev, ID_SERIAL_NUMBER, &serial);
Please add error checking code.
- to read the serial number from the device.
- */
+struct board_ops {
/*** detect() - Run the hardware info detection procedure for this* device.** @dev: The device containing the information* @return 0 if OK, -ve on error.*/int (*detect)(struct udevice *dev);This doesn’t make any sense, as the probe entry-point is intended to probe for a device.
I can see your point, but I feel that it does make some sense.
Probing the board device could be used to simply locate all the dependent devices and set them up in a data structure. This might be a fairly fast operation.
Detecting the hardware could take a lot longer:
- reading from an EEPROM
- calling out to an EC to ask for details
- checking for the present of a device on an I2C device
I don't like the idea of probe() taking a really long time (e.g. 50ms). It should be possible to get access to a device without incurring that delay. This way, we can access the devices without necessarily knowing exactly what type of board it is.
The comment above definitely needs expanding to explain why this doesn't happen in probe()]
Maybe I have been misreading the device-model all along, but I thought this was the difference between the bind and the probe operations:
* @bind: Called to bind a device to its driver * @probe: Called to probe a device, i.e. activate it
So the bind should be blindingly fast, whereas the probe should activate the device and thus might be slower (if activating the device might take longer).
/*** get_bool() - Read a specific bool data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the bool value to be read.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/int (*get_bool)(struct udevice *dev, int id, bool *val);/*** get_int() - Read a specific int data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the int value to be read.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/int (*get_int)(struct udevice *dev, int id, int *val);/*** get_str() - Read a specific string data value that describes the* hardware setup.** @dev: The board instance to gather the data.* @id: A unique identifier for the string value to be read.* @size: The size of the buffer to receive the string data.* @val: Pointer to a buffer that receives the value read.* @return 0 if OK, -ve on error.*/int (*get_str)(struct udevice *dev, int id, size_t size, char *val);These could all be handled as ioctl()-calls to a misc-device by defining a common protocol and convenience layer.
It could, and perhaps we should add an ioctl() as well. But I don't see a lot of overhead in adding type-specific methods, and they are convenient to use in the driver and in the caller. For the caller we can add a convenience layer of course, but in the driver that is a bit more of a pain.
+};
+#define board_get_ops(dev) ((struct board_ops *)(dev)->driver->ops)
+/**
- board_detect() - Run the hardware info detection procedure for this device.
- @dev: The device containing the information
- @return 0 if OK, -ve on error.
- */
+int board_detect(struct udevice *dev);
+/**
- board_get_bool() - Read a specific bool data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the bool value to be read.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_bool(struct udevice *dev, int id, bool *val);
+/**
- board_get_int() - Read a specific int data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the int value to be read.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_int(struct udevice *dev, int id, int *val);
+/**
- board_get_str() - Read a specific string data value that describes the
hardware setup.
- @dev: The board instance to gather the data.
- @id: A unique identifier for the string value to be read.
- @size: The size of the buffer to receive the string data.
- @val: Pointer to a buffer that receives the value read.
- @return 0 if OK, -ve on error.
- */
+int board_get_str(struct udevice *dev, int id, size_t size, char *val);
+int get_board(struct udevice **devp); diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h index d28fb3e23f..97327194b4 100644 --- a/include/dm/uclass-id.h +++ b/include/dm/uclass-id.h @@ -30,6 +30,7 @@ enum uclass_id { UCLASS_ADC, /* Analog-to-digital converter */ UCLASS_AHCI, /* SATA disk controller */ UCLASS_BLK, /* Block device */
UCLASS_BOARD, /* Device information from hardware */-- 2.16.1
Regards, SImon
Hi Philipp,
[somehow the quoting is messed up here - so it's hard to distinguish my response from yours.
On 1 May 2018 at 07:14, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
Simon,
On 1 May 2018, at 01:12, Simon Glass sjg@chromium.org wrote:
Hi,
On 27 April 2018 at 07:02, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
On 27 Apr 2018, at 14:51, Mario Six mario.six@gdsys.cc wrote:
Since there is no canonical "board device" that can be used in board files, it is difficult to use DM function for board initialization in these cases.
Hence, add a uclass that implements a simple "board device", which can hold devices not suitable anywhere else in the device tree, and is also able to read encoded information, e.g. hard-wired GPIOs on a GPIO expander, read-only memory ICs, etc. that carry information about the hardware.
With comments below:
Reviewed-by: Simon Glass sjg@chromium.org
A board-uclass should model a board (i.e. provide implementations for the same key abstractions that we have in place today: e.g., board_init).
This seems more like a specialised form of a misc-device.
The devices of this uclass expose methods to read generic data types (integers, strings, booleans) to encode the information provided by the hardware.
Again, this is what a misc-device is intended for… and extending the misc-device APIs (or having a convenience layer on top of its ioctl interface?) might be more appropriate.
We could certainly use a misc device, but I feel that a board device is a better abstraction for lots of reasons. It is the board that collects together all the different devices that U-Boot has access to.
To me a misc device is useful as a parent device to collect a few things together, perhaps a GPIO expander which has a clock output. But using a misc device to model a board seems like a short-term strategy.
Addressing your idea about implementing the board init in this uclass, yes I think that would be a good idea. In fact I did a series for that some time ago:
https://lists.denx.de/pipermail/u-boot/2017-March/284086.html
But, baby steps.
After reviewing the below, the similarities to the misc-device are even more apparent: if you need typed access to a key-value pair, this can be easily implemented through a common ioctl with semantic sugar at the misc-uclass level.
A misc device might be similar in action but it is not the same in terms of concept. For example it would be very strange to have a misc device which points to lots of other devices that are on the board. For example, consider this hypothetical case:
board { identity-eeprom = <&eeprom>; id-gpios = <&gpio0 2>, <&gpio0 4>; ec = <&cros_ec>; };
Maybe I am getting hung up on the ‘board’ name, as in my mind the board selection should be based on the top-level .compatible: e.g. compatible = "tsd,rk3399-q7", "tsd,puma", "rockchip,rk3399"; and the selected board-classes could then encapsulate the various init hooks and inquire into devices defined via /aliases, /config and /chosen.
A board-class should restructure existing board.c-files into the .ops that are used there (i.e. init, late-init, usb-init, debug-uart-init, etc.) ... this is different from what’s being proposed here.
See above. I agree the compatible string should be used.
The hypothetical case you give is already covered wholly by today’s device model. Let me explain with a counter example: aliases { identity-eeprom = <&eeprom>; id-gpios = <&board-id>; ec = <&cros_ec>; }
board-id { /* a good use for a hypothetical misc-device that exports the values of a variable-length list of GPIOs */ .compatible = “identity-gpios”; id-gpios = <&gpio0 2>, <&gpio0 4>; }
Yes, agreed, that will work. But I want to push this a bit further and I think a board uclass makes sense overall.
Here we point to a few devices which may otherwise require specific device-tree references to locate. Already we are seeing this sort of code in U-Boot:
/* find the grf node */ node = fdt_node_offset_by_compatible(blob, -1, "rockchip,rk3288-grf”);
This is usually caused by badly-modelled device-trees or missing device tree support in drivers. In this specific case it’s a wart caused by dwc2_udc_probe(…) and that this is not called from the device-tree.
I would hope that this will eventually be included converted to the device model and that the info will be processed from the dwc2_udc driver.
ret = regulator_get_by_platname("vdd_arm", &dev); if (ret) { debug("Cannot set regulator name\n"); return ret; }
I know this specific example, and it’s from a use case, where a voltage needs to be slowly raised. The reason for this being there is that we don’t have the same DTS as in Linux and the modelling (and driver) for the PMU subsystem are not there.
So I don’t seem to follow on how this would resolve itself with the proposed board-uclass.
These are the sorts of things which suggest that some sort of 'board directory', pointing to the things that the board needs to init, would be useful.
In fact, I have been raising the topic of having a ‘board’ and a ‘soc’ uclass a few weeks back in order to structure the boot-up through the device-model.
This is exactly where my comments are coming from: the proposed board uclass (as in this patch-series) does not aim to break up the existing board-files and put them into a framework of .ops in a new board-uclass.
In the current code-base I see cases like
int rk3288_qos_init(void) { int val = 2 << PRIORITY_HIGH_SHIFT | 2 << PRIORITY_LOW_SHIFT; /* set vop qos to higher priority */ writel(val, CPU_AXI_QOS_PRIORITY + VIO0_VOP_QOS); writel(val, CPU_AXI_QOS_PRIORITY + VIO1_VOP_QOS);
if (!fdt_node_check_compatible(gd->fdt_blob, 0, "rockchip,rk3288-tinker")) { /* set isp qos to higher priority */ writel(val, CPU_AXI_QOS_PRIORITY + VIO1_ISP_R_QOS); writel(val, CPU_AXI_QOS_PRIORITY + VIO1_ISP_W0_QOS); writel(val, CPU_AXI_QOS_PRIORITY + VIO1_ISP_W1_QOS); } return 0;}
which clearly requires a multi-step init-process for a soc->init(...) board->init(...) with both derived from compatible = "rockchip,rk3288-tinker", "rockchip,rk3288";
Existing entry points like the debug_uart_init, etc. should get their own .ops-entry …
Yes, I agree that would be a good thing to do. See my series referenced above.
[..]
- to read the serial number from the device.
- */
+struct board_ops {
/*** detect() - Run the hardware info detection procedure for this* device.** @dev: The device containing the information* @return 0 if OK, -ve on error.*/int (*detect)(struct udevice *dev);This doesn’t make any sense, as the probe entry-point is intended to probe for a device.
I can see your point, but I feel that it does make some sense.
Probing the board device could be used to simply locate all the dependent devices and set them up in a data structure. This might be a fairly fast operation.
Detecting the hardware could take a lot longer:
- reading from an EEPROM
- calling out to an EC to ask for details
- checking for the present of a device on an I2C device
I don't like the idea of probe() taking a really long time (e.g. 50ms). It should be possible to get access to a device without incurring that delay. This way, we can access the devices without necessarily knowing exactly what type of board it is.
The comment above definitely needs expanding to explain why this doesn't happen in probe()]
Maybe I have been misreading the device-model all along, but I thought this was the difference between the bind and the probe operations:
- @bind: Called to bind a device to its driver
- @probe: Called to probe a device, i.e. activate it
So the bind should be blindingly fast, whereas the probe should activate the device and thus might be slower (if activating the device might take longer).
That's right, but even so I want most devices to probe quickly where possible. For example an I2C bus does not scan itself in probe(). A display port peripheral does not start up the display in probe(). There is clearly a need in some cases to separate probe() from enable() or whatever you call it. Otherwise we risk breaking the 'lazy-init' requirement of U-Boot. Probing all the devices in the system should not take a long time.
Regards, Simon
participants (3)
-
Dr. Philipp Tomsich -
Mario Six -
Simon Glass