[PATCH 0/5] sysinfo: Add gpio sysinfo driver
This series adds a GPIO sysinfo driver using the dm_gpio_get_values_as_int_base3 function. The board revision is mapped based in devicetree properties. This series is based on Simon's GPIO series [1].
[1] https://patchwork.ozlabs.org/project/uboot/list/?series=228126
Sean Anderson (5): dm: gpio: Fix gpio_get_list_count failing with livetree sysinfo: Provide some global/default IDs sysinfo: Require that sysinfo_detect be called before other methods sysinfo: Add gpio-sysinfo driver test: Add gpio-sysinfo test
arch/sandbox/dts/test.dts | 7 + common/spl/spl_fit.c | 4 + .../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/gpio/gpio-uclass.c | 4 +- drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gazerbeam.h | 10 +- drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ drivers/sysinfo/sandbox.h | 10 +- include/sysinfo.h | 42 ++++-- test/dm/Makefile | 1 + test/dm/sysinfo-gpio.c | 69 +++++++++ 12 files changed, 309 insertions(+), 22 deletions(-) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c create mode 100644 test/dm/sysinfo-gpio.c
of_parse_phandle_with_args (called by dev_read_phandle_with_args) does not support getting the length of a phandle list by using the index -1. Instead, use dev_count_phandle_with_args which supports exactly this use-case.
Fixes: 3669e0e759 ("dm: gpio: Add better functions to request GPIOs")
Signed-off-by: Sean Anderson sean.anderson@seco.com ---
drivers/gpio/gpio-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpio/gpio-uclass.c b/drivers/gpio/gpio-uclass.c index 8dc647dc9f..8de6fe58a4 100644 --- a/drivers/gpio/gpio-uclass.c +++ b/drivers/gpio/gpio-uclass.c @@ -1214,8 +1214,8 @@ int gpio_get_list_count(struct udevice *dev, const char *list_name) { int ret;
- ret = dev_read_phandle_with_args(dev, list_name, "#gpio-cells", 0, -1, - NULL); + ret = dev_count_phandle_with_args(dev, list_name, "#gpio-cells", + -ENOENT); if (ret) { debug("%s: Node '%s', property '%s', GPIO count failed: %d\n", __func__, dev->name, list_name, ret);
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
of_parse_phandle_with_args (called by dev_read_phandle_with_args) does not support getting the length of a phandle list by using the index -1. Instead, use dev_count_phandle_with_args which supports exactly this use-case.
Fixes: 3669e0e759 ("dm: gpio: Add better functions to request GPIOs")
Signed-off-by: Sean Anderson sean.anderson@seco.com
drivers/gpio/gpio-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
This adds an ID for a board revision. Existing IDs have been moved above SYSINFO_ID_END to allow for future expansion.
Signed-off-by: Sean Anderson sean.anderson@seco.com ---
drivers/sysinfo/gazerbeam.h | 10 +++++----- drivers/sysinfo/sandbox.h | 10 ++++------ include/sysinfo.h | 13 +++++++++++++ 3 files changed, 22 insertions(+), 11 deletions(-)
diff --git a/drivers/sysinfo/gazerbeam.h b/drivers/sysinfo/gazerbeam.h index 171729d203..d8e3c48955 100644 --- a/drivers/sysinfo/gazerbeam.h +++ b/drivers/sysinfo/gazerbeam.h @@ -5,11 +5,11 @@ * */
-enum { - BOARD_MULTICHANNEL, - BOARD_VARIANT, - BOARD_HWVERSION, -}; +#include <sysinfo.h> + +#define BOARD_MULTICHANNEL (SYSINFO_ID_END + 1) +#define BOARD_VARIANT (SYSINFO_ID_END + 2) +#define BOARD_HWVERSION (SYSINFO_ID_END + 3)
enum { VAR_CON, diff --git a/drivers/sysinfo/sandbox.h b/drivers/sysinfo/sandbox.h index 2cff494f56..dc7c15e738 100644 --- a/drivers/sysinfo/sandbox.h +++ b/drivers/sysinfo/sandbox.h @@ -4,9 +4,7 @@ * Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc */
-enum { - BOOL_CALLED_DETECT, - INT_TEST1, - INT_TEST2, - STR_VACATIONSPOT, -}; +#define BOOL_CALLED_DETECT (SYSINFO_ID_END + 1) +#define INT_TEST1 (SYSINFO_ID_END + 2) +#define INT_TEST2 (SYSINFO_ID_END + 3) +#define STR_VACATIONSPOT (SYSINFO_ID_END + 4) diff --git a/include/sysinfo.h b/include/sysinfo.h index c045d316b0..9386bdf49a 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -32,6 +32,19 @@ */
#if CONFIG_IS_ENABLED(SYSINFO) +/** + * enum sysinfo_id - IDs which may be passed to sysinfo accessors + */ +enum sysinfo_id { + /** @SYSINFO_ID_REVISION: Use this ID to access the board revision. */ + SYSINFO_ID_REVISION, + /** + * @SYSINFO_ID_END: The last global sysinfo id. If you need to return + * custom information, use SYSINFO_ID_END+1 as the base offset. + */ + SYSINFO_ID_END = 127, +}; + struct sysinfo_ops { /** * detect() - Run the hardware info detection procedure for this
Hi Sean,
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
This adds an ID for a board revision. Existing IDs have been moved above SYSINFO_ID_END to allow for future expansion.
Signed-off-by: Sean Anderson sean.anderson@seco.com
drivers/sysinfo/gazerbeam.h | 10 +++++----- drivers/sysinfo/sandbox.h | 10 ++++------ include/sysinfo.h | 13 +++++++++++++ 3 files changed, 22 insertions(+), 11 deletions(-)
Can we keep the enums? They are easier to maintain that #define and the debugger understands them.
Regards, Simon
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
This adds an ID for a board revision. Existing IDs have been moved above SYSINFO_ID_END to allow for future expansion.
Signed-off-by: Sean Anderson sean.anderson@seco.com
drivers/sysinfo/gazerbeam.h | 10 +++++----- drivers/sysinfo/sandbox.h | 10 ++++------ include/sysinfo.h | 13 +++++++++++++ 3 files changed, 22 insertions(+), 11 deletions(-)
Can we keep the enums? They are easier to maintain that #define and the debugger understands them.
Yes, I will change that for v2.
--Sean
Regards, Simon
This updates sysinfo documentation to document that detect() must be called first. This allows drivers to cache information in detect() and perform (cheaper) retrieval in the other accessors. This also modifies the only instance where this sequencing was not followed.
Signed-off-by: Sean Anderson sean.anderson@seco.com ---
common/spl/spl_fit.c | 4 ++++ include/sysinfo.h | 29 ++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index a6ad094e91..4d17582af5 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -116,6 +116,10 @@ static int spl_fit_get_image_name(const void *fit, int images, * no string in the property for this index. Check if the * sysinfo-level code can supply one. */ + rc = sysinfo_detect(sysinfo); + if (rc) + return rc; + rc = sysinfo_get_fit_loadable(sysinfo, index - i - 1, type, &str); if (rc && rc != -ENOENT) diff --git a/include/sysinfo.h b/include/sysinfo.h index 9386bdf49a..14c25fa15c 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -54,7 +54,8 @@ struct sysinfo_ops { * This operation might take a long time (e.g. read from EEPROM, * check the presence of a device on a bus etc.), hence this is not * done in the probe() method, but later during operation in this - * dedicated method. + * dedicated method. This method must be called before any other + * methods. * * Return: 0 if OK, -ve on error. */ @@ -67,7 +68,8 @@ struct sysinfo_ops { * @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. + * Return: 0 if OK, -ENOENT if called before @detect, else -ve on + * error. */ int (*get_bool)(struct udevice *dev, int id, bool *val);
@@ -78,7 +80,8 @@ struct sysinfo_ops { * @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. + * Return: 0 if OK, -ENOENT if called before @detect, else -ve on + * error. */ int (*get_int)(struct udevice *dev, int id, int *val);
@@ -90,7 +93,8 @@ struct sysinfo_ops { * @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. + * Return: 0 if OK, -ENOENT if called before @detect, else -ve on + * error. */ int (*get_str)(struct udevice *dev, int id, size_t size, char *val);
@@ -98,7 +102,7 @@ struct sysinfo_ops { * get_fit_loadable - Get the name of an image to load from FIT * This function can be used to provide the image names based on runtime * detection. A classic use-case would when DTBOs are used to describe - * additionnal daughter cards. + * additional daughter cards. * * @dev: The sysinfo instance to gather the data. * @index: Index of the image. Starts at 0 and gets incremented @@ -120,6 +124,9 @@ struct sysinfo_ops { * * @dev: The device containing the information * + * This function must be called before any other accessor function for this + * device. + * * Return: 0 if OK, -ve on error. */ int sysinfo_detect(struct udevice *dev); @@ -131,7 +138,8 @@ int sysinfo_detect(struct udevice *dev); * @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. + * Return: 0 if OK, -ENOENT if called before sysinfo_detect(), else -ve on + * error. */ int sysinfo_get_bool(struct udevice *dev, int id, bool *val);
@@ -142,7 +150,8 @@ int sysinfo_get_bool(struct udevice *dev, int id, bool *val); * @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. + * Return: 0 if OK, -ENOENT if called before sysinfo_detect(), else -ve on + * error. */ int sysinfo_get_int(struct udevice *dev, int id, int *val);
@@ -154,7 +163,8 @@ int sysinfo_get_int(struct udevice *dev, int id, int *val); * @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. + * Return: 0 if OK, -ENOENT if called before sysinfo_detect(), else -ve on + * error. */ int sysinfo_get_str(struct udevice *dev, int id, size_t size, char *val);
@@ -166,7 +176,8 @@ int sysinfo_get_str(struct udevice *dev, int id, size_t size, char *val); * function that returns the unique device. This is especially useful for use * in sysinfo files. * - * Return: 0 if OK, -ve on error. + * Return: 0 if OK, -ENOENT if called before sysinfo_detect(), else -ve on + * error. */ int sysinfo_get(struct udevice **devp);
Hi Sean,
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
This updates sysinfo documentation to document that detect() must be called first. This allows drivers to cache information in detect() and perform (cheaper) retrieval in the other accessors. This also modifies the only instance where this sequencing was not followed.
Signed-off-by: Sean Anderson sean.anderson@seco.com
common/spl/spl_fit.c | 4 ++++ include/sysinfo.h | 29 ++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Is it possible to enforce this in the uclass? Also I think -EPERM or -EACCES might be better than -ENOENT.
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
This updates sysinfo documentation to document that detect() must be called first. This allows drivers to cache information in detect() and perform (cheaper) retrieval in the other accessors. This also modifies the only instance where this sequencing was not followed.
Signed-off-by: Sean Anderson sean.anderson@seco.com
common/spl/spl_fit.c | 4 ++++ include/sysinfo.h | 29 ++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Is it possible to enforce this in the uclass?
So have the uclass call detect() if it hasn't been called? Should there be a way to re-detect?
Also I think -EPERM or -EACCES might be better than -ENOENT.
Hm, I thought I saw gazerbeam using ENOENT for this purpose, but on further review it looks like it does not. I think EPERM is good for this.
--Sean
Hi Sean,
On Fri, 5 Mar 2021 at 08:12, Sean Anderson sean.anderson@seco.com wrote:
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
This updates sysinfo documentation to document that detect() must be called first. This allows drivers to cache information in detect() and perform (cheaper) retrieval in the other accessors. This also modifies the only instance where this sequencing was not followed.
Signed-off-by: Sean Anderson sean.anderson@seco.com
common/spl/spl_fit.c | 4 ++++ include/sysinfo.h | 29 ++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Is it possible to enforce this in the uclass?
So have the uclass call detect() if it hasn't been called? Should there be a way to re-detect?
I mean have a check function (that all off the uclass methods call) that checks it has been called and returns an error immediately if not.
I don't think automatically detecting is a great idea - as you say it makes it hard to re-detect. But just a bool flag in the device's uclass-private data should be enough.
Also I think -EPERM or -EACCES might be better than -ENOENT.
Hm, I thought I saw gazerbeam using ENOENT for this purpose, but on further review it looks like it does not. I think EPERM is good for this.
OK.
Regards, Simon
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com ---
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device + +This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties. + +Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property + + gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>; + +If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be + + 0t201 = 2*9 + 0*3 + 1*3 = 19 + +If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be + + 0t012 = 0*9 + 1*3 + 2*1 = 5 + +Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number, + least-significant-digit first +- revisions: a list of known revisions; any revisions not present will have the + name "unknown" +- names: the name of each revision in revisions + +Example: +sysinfo { + compatible = "gpio-sysinfo"; + gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>; + revisions = <19>, <5>; + names = "rev_a", "foo"; +}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO + bool "Enable gpio sysinfo driver" + help + Support querying gpios to determine board revision. This uses gpios to + form a ternary number (when they are pulled-up, -down, or floating). + This ternary number is then mapped to a board revision name using + device tree properties. + endif diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 Sean Anderson sean.anderson@seco.com + */ + +#include <common.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <log.h> +#include <sysinfo.h> +#include <asm/gpio.h> + +struct sysinfo_gpio_priv { + struct gpio_desc *gpios; + int gpio_num, revision; +}; + +static int sysinfo_gpio_detect(struct udevice *dev) +{ + struct sysinfo_gpio_priv *priv = dev_get_priv(dev); + + priv->revision = dm_gpio_get_values_as_int_base3(priv->gpios, + priv->gpio_num); + return priv->revision < 0 ? priv->revision : 0; +} + +static int sysinfo_gpio_get_int(struct udevice *dev, int id, int *val) +{ + struct sysinfo_gpio_priv *priv = dev_get_priv(dev); + + switch (id) { + case SYSINFO_ID_REVISION: + if (priv->revision < 0) { + return priv->revision; + *val = priv->revision; + return 0; + default: + return -EINVAL; + }; +} + +static int sysinfo_gpio_get_str(struct udevice *dev, int id, size_t size, char *val) +{ + struct sysinfo_gpio_priv *priv = dev_get_priv(dev); + + switch (id) { + case SYSINFO_ID_REVISION: { + const char *name = NULL; + int i, ret; + u32 revision; + + if (priv->revision < 0) + return priv->revision; + + for (i = 0; i < priv->gpio_num; i++) { + ret = dev_read_u32_index(dev, "revisions", i, + &revision); + if (ret) { + if (ret != -EOVERFLOW) + return ret; + break; + } + + if (revision == priv->revision) { + ret = dev_read_string_index(dev, "names", i, + &name); + if (ret < 0) + return ret; + break; + } + } + if (!name) + name = "unknown"; + + strncpy(val, name, size); + val[size - 1] = '\0'; + return 0; + } default: + return -EINVAL; + }; +} + +static const struct sysinfo_ops sysinfo_gpio_ops = { + .detect = sysinfo_gpio_detect, + .get_int = sysinfo_gpio_get_int, + .get_str = sysinfo_gpio_get_str, +}; + +static int sysinfo_gpio_probe(struct udevice *dev) +{ + int ret; + struct sysinfo_gpio_priv *priv = dev_get_priv(dev); + + priv->gpio_num = gpio_get_list_count(dev, "gpios"); + if (priv->gpio_num < 0) { + dev_err(dev, "could not get gpios length (err = %d)\n", + priv->gpio_num); + return priv->gpio_num; + } + + priv->gpios = calloc(priv->gpio_num, sizeof(*priv->gpios)); + if (!priv->gpios) { + dev_err(dev, "could not allocate memory for %d gpios\n", + priv->gpio_num); + return -ENOMEM; + } + + ret = gpio_request_list_by_name(dev, "gpios", priv->gpios, + priv->gpio_num, GPIOD_IS_IN); + if (ret != priv->gpio_num) { + dev_err(dev, "could not get gpios (err = %d)\n", + priv->gpio_num); + return ret; + } + + if (!dev_read_bool(dev, "revisions") || !dev_read_bool(dev, "names")) { + dev_err(dev, "revisions or names properties missing\n"); + return -ENOENT; + } + + priv->revision = -ENOENT; + + return 0; +} + +static const struct udevice_id sysinfo_gpio_ids[] = { + { .compatible = "gpio-sysinfo" }, + { /* sentinel */ } +}; + +U_BOOT_DRIVER(sysinfo_gpio) = { + .name = "sysinfo_gpio", + .id = UCLASS_SYSINFO, + .of_match = sysinfo_gpio_ids, + .ops = &sysinfo_gpio_ops, + .priv_auto = sizeof(struct sysinfo_gpio_priv), + .probe = sysinfo_gpio_probe, +};
On 3/1/21 3:46 PM, Sean Anderson wrote:
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device
+This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties.
+Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property
- gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>;
+If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be
- 0t201 = 2*9 + 0*3 + 1*3 = 19
+If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be
- 0t012 = 0*9 + 1*3 + 2*1 = 5
+Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number,
- least-significant-digit first
+- revisions: a list of known revisions; any revisions not present will have the
- name "unknown"
+- names: the name of each revision in revisions
+Example: +sysinfo {
- compatible = "gpio-sysinfo";
- gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;
- revisions = <19>, <5>;
- names = "rev_a", "foo";
+}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO
- bool "Enable gpio sysinfo driver"
- help
Support querying gpios to determine board revision. This uses gpios toform a ternary number (when they are pulled-up, -down, or floating).This ternary number is then mapped to a board revision name usingdevice tree properties.- endif
diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h> +#include <log.h> +#include <sysinfo.h> +#include <asm/gpio.h>
+struct sysinfo_gpio_priv {
- struct gpio_desc *gpios;
- int gpio_num, revision;
+};
+static int sysinfo_gpio_detect(struct udevice *dev) +{
- struct sysinfo_gpio_priv *priv = dev_get_priv(dev);
- priv->revision = dm_gpio_get_values_as_int_base3(priv->gpios,
priv->gpio_num);- return priv->revision < 0 ? priv->revision : 0;
+}
+static int sysinfo_gpio_get_int(struct udevice *dev, int id, int *val) +{
- struct sysinfo_gpio_priv *priv = dev_get_priv(dev);
- switch (id) {
- case SYSINFO_ID_REVISION:
if (priv->revision < 0) {
Looks like I forgot to remove this brace. Will be fixed in v2.
--Sean
return priv->revision;*val = priv->revision;return 0;- default:
return -EINVAL;- };
+}
+static int sysinfo_gpio_get_str(struct udevice *dev, int id, size_t size, char *val) +{
- struct sysinfo_gpio_priv *priv = dev_get_priv(dev);
- switch (id) {
- case SYSINFO_ID_REVISION: {
const char *name = NULL;int i, ret;u32 revision;if (priv->revision < 0)return priv->revision;for (i = 0; i < priv->gpio_num; i++) {ret = dev_read_u32_index(dev, "revisions", i,&revision);if (ret) {if (ret != -EOVERFLOW)return ret;break;}if (revision == priv->revision) {ret = dev_read_string_index(dev, "names", i,&name);if (ret < 0)return ret;break;}}if (!name)name = "unknown";strncpy(val, name, size);val[size - 1] = '\0';return 0;- } default:
return -EINVAL;- };
+}
+static const struct sysinfo_ops sysinfo_gpio_ops = {
- .detect = sysinfo_gpio_detect,
- .get_int = sysinfo_gpio_get_int,
- .get_str = sysinfo_gpio_get_str,
+};
+static int sysinfo_gpio_probe(struct udevice *dev) +{
- int ret;
- struct sysinfo_gpio_priv *priv = dev_get_priv(dev);
- priv->gpio_num = gpio_get_list_count(dev, "gpios");
- if (priv->gpio_num < 0) {
dev_err(dev, "could not get gpios length (err = %d)\n",priv->gpio_num);return priv->gpio_num;- }
- priv->gpios = calloc(priv->gpio_num, sizeof(*priv->gpios));
- if (!priv->gpios) {
dev_err(dev, "could not allocate memory for %d gpios\n",priv->gpio_num);return -ENOMEM;- }
- ret = gpio_request_list_by_name(dev, "gpios", priv->gpios,
priv->gpio_num, GPIOD_IS_IN);- if (ret != priv->gpio_num) {
dev_err(dev, "could not get gpios (err = %d)\n",priv->gpio_num);return ret;- }
- if (!dev_read_bool(dev, "revisions") || !dev_read_bool(dev, "names")) {
dev_err(dev, "revisions or names properties missing\n");return -ENOENT;- }
- priv->revision = -ENOENT;
- return 0;
+}
+static const struct udevice_id sysinfo_gpio_ids[] = {
- { .compatible = "gpio-sysinfo" },
- { /* sentinel */ }
+};
+U_BOOT_DRIVER(sysinfo_gpio) = {
- .name = "sysinfo_gpio",
- .id = UCLASS_SYSINFO,
- .of_match = sysinfo_gpio_ids,
- .ops = &sysinfo_gpio_ops,
- .priv_auto = sizeof(struct sysinfo_gpio_priv),
- .probe = sysinfo_gpio_probe,
+};
Hi Sean,
On Mon, 1 Mar 2021 at 16:08, Sean Anderson sean.anderson@seco.com wrote:
On 3/1/21 3:46 PM, Sean Anderson wrote:
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see below
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device
+This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties.
+Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property
gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>;+If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be
0t201 = 2*9 + 0*3 + 1*3 = 19+If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be
0t012 = 0*9 + 1*3 + 2*1 = 5+Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number,
- least-significant-digit first
+- revisions: a list of known revisions; any revisions not present will have the
- name "unknown"
+- names: the name of each revision in revisions
+Example: +sysinfo {
compatible = "gpio-sysinfo";gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;revisions = <19>, <5>;names = "rev_a", "foo";+}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO
bool "Enable gpio sysinfo driver"
depends on DM_GPIO ?
helpSupport querying gpios to determine board revision. This uses gpios toform a ternary number (when they are pulled-up, -down, or floating).This ternary number is then mapped to a board revision name usingdevice tree properties.- endif
diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h>
should be at end
+#include <log.h> +#include <sysinfo.h> +#include <asm/gpio.h>
+struct sysinfo_gpio_priv {
needs comment
struct gpio_desc *gpios;int gpio_num, revision;+};
+static int sysinfo_gpio_detect(struct udevice *dev) +{
struct sysinfo_gpio_priv *priv = dev_get_priv(dev);priv->revision = dm_gpio_get_values_as_int_base3(priv->gpios,priv->gpio_num);return priv->revision < 0 ? priv->revision : 0;+}
+static int sysinfo_gpio_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_gpio_priv *priv = dev_get_priv(dev);switch (id) {case SYSINFO_ID_REVISION:if (priv->revision < 0) {Looks like I forgot to remove this brace. Will be fixed in v2.
--Sean
return priv->revision;*val = priv->revision;return 0;default:return -EINVAL;};+}
+static int sysinfo_gpio_get_str(struct udevice *dev, int id, size_t size, char *val) +{
struct sysinfo_gpio_priv *priv = dev_get_priv(dev);switch (id) {case SYSINFO_ID_REVISION: {const char *name = NULL;int i, ret;u32 revision;if (priv->revision < 0)return priv->revision;for (i = 0; i < priv->gpio_num; i++) {ret = dev_read_u32_index(dev, "revisions", i,&revision);if (ret) {if (ret != -EOVERFLOW)return ret;break;}if (revision == priv->revision) {ret = dev_read_string_index(dev, "names", i,&name);if (ret < 0)return ret;break;}}if (!name)name = "unknown";strncpy(val, name, size);val[size - 1] = '\0';return 0;} default:return -EINVAL;};+}
+static const struct sysinfo_ops sysinfo_gpio_ops = {
.detect = sysinfo_gpio_detect,.get_int = sysinfo_gpio_get_int,.get_str = sysinfo_gpio_get_str,+};
+static int sysinfo_gpio_probe(struct udevice *dev) +{
int ret;struct sysinfo_gpio_priv *priv = dev_get_priv(dev);priv->gpio_num = gpio_get_list_count(dev, "gpios");if (priv->gpio_num < 0) {dev_err(dev, "could not get gpios length (err = %d)\n",priv->gpio_num);return priv->gpio_num;}priv->gpios = calloc(priv->gpio_num, sizeof(*priv->gpios));if (!priv->gpios) {dev_err(dev, "could not allocate memory for %d gpios\n",priv->gpio_num);return -ENOMEM;}ret = gpio_request_list_by_name(dev, "gpios", priv->gpios,priv->gpio_num, GPIOD_IS_IN);if (ret != priv->gpio_num) {dev_err(dev, "could not get gpios (err = %d)\n",priv->gpio_num);return ret;}if (!dev_read_bool(dev, "revisions") || !dev_read_bool(dev, "names")) {
I think this is a bit grubby since they are not bool properties...can you use dev_read_prop() instead?
dev_err(dev, "revisions or names properties missing\n");return -ENOENT;}priv->revision = -ENOENT;return 0;+}
+static const struct udevice_id sysinfo_gpio_ids[] = {
{ .compatible = "gpio-sysinfo" },{ /* sentinel */ }+};
+U_BOOT_DRIVER(sysinfo_gpio) = {
.name = "sysinfo_gpio",.id = UCLASS_SYSINFO,.of_match = sysinfo_gpio_ids,.ops = &sysinfo_gpio_ops,.priv_auto = sizeof(struct sysinfo_gpio_priv),.probe = sysinfo_gpio_probe,+};
Regards, Simon
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 16:08, Sean Anderson sean.anderson@seco.com wrote:
On 3/1/21 3:46 PM, Sean Anderson wrote:
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see below
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device
+This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties.
+Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property
gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>;+If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be
0t201 = 2*9 + 0*3 + 1*3 = 19+If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be
0t012 = 0*9 + 1*3 + 2*1 = 5+Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number,
- least-significant-digit first
+- revisions: a list of known revisions; any revisions not present will have the
- name "unknown"
+- names: the name of each revision in revisions
+Example: +sysinfo {
compatible = "gpio-sysinfo";gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;revisions = <19>, <5>;names = "rev_a", "foo";+}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO
bool "Enable gpio sysinfo driver"depends on DM_GPIO ?
helpSupport querying gpios to determine board revision. This uses gpios toform a ternary number (when they are pulled-up, -down, or floating).This ternary number is then mapped to a board revision name usingdevice tree properties.- endif
diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h>
should be at end
Can you clarify the ordering rules? I was following [1] which perscribes
<common.h> <others.h> <asm/...> <arch/arm/...> <linux/...> "local.h"
[1] https://www.denx.de/wiki/U-Boot/CodingStyle#Include_files
+#include <log.h> +#include <sysinfo.h> +#include <asm/gpio.h>
+struct sysinfo_gpio_priv {
needs comment
struct gpio_desc *gpios;int gpio_num, revision;+};
+static int sysinfo_gpio_detect(struct udevice *dev) +{
struct sysinfo_gpio_priv *priv = dev_get_priv(dev);priv->revision = dm_gpio_get_values_as_int_base3(priv->gpios,priv->gpio_num);return priv->revision < 0 ? priv->revision : 0;+}
+static int sysinfo_gpio_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_gpio_priv *priv = dev_get_priv(dev);switch (id) {case SYSINFO_ID_REVISION:if (priv->revision < 0) {Looks like I forgot to remove this brace. Will be fixed in v2.
--Sean
return priv->revision;*val = priv->revision;return 0;default:return -EINVAL;};+}
+static int sysinfo_gpio_get_str(struct udevice *dev, int id, size_t size, char *val) +{
struct sysinfo_gpio_priv *priv = dev_get_priv(dev);switch (id) {case SYSINFO_ID_REVISION: {const char *name = NULL;int i, ret;u32 revision;if (priv->revision < 0)return priv->revision;for (i = 0; i < priv->gpio_num; i++) {ret = dev_read_u32_index(dev, "revisions", i,&revision);if (ret) {if (ret != -EOVERFLOW)return ret;break;}if (revision == priv->revision) {ret = dev_read_string_index(dev, "names", i,&name);if (ret < 0)return ret;break;}}if (!name)name = "unknown";strncpy(val, name, size);val[size - 1] = '\0';return 0;} default:return -EINVAL;};+}
+static const struct sysinfo_ops sysinfo_gpio_ops = {
.detect = sysinfo_gpio_detect,.get_int = sysinfo_gpio_get_int,.get_str = sysinfo_gpio_get_str,+};
+static int sysinfo_gpio_probe(struct udevice *dev) +{
int ret;struct sysinfo_gpio_priv *priv = dev_get_priv(dev);priv->gpio_num = gpio_get_list_count(dev, "gpios");if (priv->gpio_num < 0) {dev_err(dev, "could not get gpios length (err = %d)\n",priv->gpio_num);return priv->gpio_num;}priv->gpios = calloc(priv->gpio_num, sizeof(*priv->gpios));if (!priv->gpios) {dev_err(dev, "could not allocate memory for %d gpios\n",prhttps://www.denx.de/wiki/U-Boot/CodingStyleiv->gpio_num);return -ENOMEM;}ret = gpio_request_list_by_name(dev, "gpios", priv->gpios,priv->gpio_num, GPIOD_IS_IN);if (ret != priv->gpio_num) {dev_err(dev, "could not get gpios (err = %d)\n",priv->gpio_num);return ret;}if (!dev_read_bool(dev, "revisions") || !dev_read_bool(dev, "names")) {I think this is a bit grubby since they are not bool properties...can you use dev_read_prop() instead?
I suppose. I think this is cleaner, since dev_read_bool is really an alternatively-named dev_prop_exists.
--Sean
dev_err(dev, "revisions or names properties missing\n");return -ENOENT;}priv->revision = -ENOENT;return 0;+}
+static const struct udevice_id sysinfo_gpio_ids[] = {
{ .compatible = "gpio-sysinfo" },{ /* sentinel */ }+};
+U_BOOT_DRIVER(sysinfo_gpio) = {
.name = "sysinfo_gpio",.id = UCLASS_SYSINFO,.of_match = sysinfo_gpio_ids,.ops = &sysinfo_gpio_ops,.priv_auto = sizeof(struct sysinfo_gpio_priv),.probe = sysinfo_gpio_probe,+};
Regards, Simon
Hi Sean,
On Fri, 5 Mar 2021 at 08:19, Sean Anderson sean.anderson@seco.com wrote:
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 16:08, Sean Anderson sean.anderson@seco.com wrote:
On 3/1/21 3:46 PM, Sean Anderson wrote:
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see below
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device
+This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties.
+Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property
gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>;+If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be
0t201 = 2*9 + 0*3 + 1*3 = 19+If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be
0t012 = 0*9 + 1*3 + 2*1 = 5+Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number,
- least-significant-digit first
+- revisions: a list of known revisions; any revisions not present will have the
- name "unknown"
+- names: the name of each revision in revisions
+Example: +sysinfo {
compatible = "gpio-sysinfo";gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;revisions = <19>, <5>;names = "rev_a", "foo";+}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO
bool "Enable gpio sysinfo driver"depends on DM_GPIO ?
helpSupport querying gpios to determine board revision. This uses gpios toform a ternary number (when they are pulled-up, -down, or floating).This ternary number is then mapped to a board revision name usingdevice tree properties.- endif
diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h>
should be at end
Can you clarify the ordering rules? I was following [1] which perscribes
<common.h> <others.h> <asm/...> <arch/arm/...> <linux/...> "local.h"[1] https://www.denx.de/wiki/U-Boot/CodingStyle#Include_files
Yes that's right, but dm/ is a directory so it goes with the other dirs at the end. I'll update the page to include dm/ [..]
if (!dev_read_bool(dev, "revisions") || !dev_read_bool(dev, "names")) {I think this is a bit grubby since they are not bool properties...can you use dev_read_prop() instead?
I suppose. I think this is cleaner, since dev_read_bool is really an alternatively-named dev_prop_exists.
Well, adding that would be a nice step and I agree it would be even better!
Regards, Simon
On 3/5/21 11:39 AM, Simon Glass wrote:
Hi Sean,
On Fri, 5 Mar 2021 at 08:19, Sean Anderson sean.anderson@seco.com wrote:
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 16:08, Sean Anderson sean.anderson@seco.com wrote:
On 3/1/21 3:46 PM, Sean Anderson wrote:
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see below
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device
+This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties.
+Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property
gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>;+If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be
0t201 = 2*9 + 0*3 + 1*3 = 19+If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be
0t012 = 0*9 + 1*3 + 2*1 = 5+Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number,
- least-significant-digit first
+- revisions: a list of known revisions; any revisions not present will have the
- name "unknown"
+- names: the name of each revision in revisions
+Example: +sysinfo {
compatible = "gpio-sysinfo";gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;revisions = <19>, <5>;names = "rev_a", "foo";+}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO
bool "Enable gpio sysinfo driver"depends on DM_GPIO ?
helpSupport querying gpios to determine board revision. This uses gpios toform a ternary number (when they are pulled-up, -down, or floating).This ternary number is then mapped to a board revision name usingdevice tree properties.- endif
diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h>
should be at end
Can you clarify the ordering rules? I was following [1] which perscribes
<common.h> <others.h> <asm/...> <arch/arm/...> <linux/...> "local.h"[1] https://www.denx.de/wiki/U-Boot/CodingStyle#Include_files
Yes that's right, but dm/ is a directory so it goes with the other dirs at the end. I'll update the page to include dm/
Ok, so the general rule should be that directories go last?
--Sean
[..]
if (!dev_read_bool(dev, "revisions") || !dev_read_bool(dev, "names")) {I think this is a bit grubby since they are not bool properties...can you use dev_read_prop() instead?
I suppose. I think this is cleaner, since dev_read_bool is really an alternatively-named dev_prop_exists.
Well, adding that would be a nice step and I agree it would be even better!
Regards, Simon
Hi Sean,
On Fri, 5 Mar 2021 at 10:24, Sean Anderson sean.anderson@seco.com wrote:
On 3/5/21 11:39 AM, Simon Glass wrote:
Hi Sean,
On Fri, 5 Mar 2021 at 08:19, Sean Anderson sean.anderson@seco.com wrote:
On 3/4/21 11:08 PM, Simon Glass wrote:
Hi Sean,
On Mon, 1 Mar 2021 at 16:08, Sean Anderson sean.anderson@seco.com wrote:
On 3/1/21 3:46 PM, Sean Anderson wrote:
This uses the newly-added dm_gpio_get_values_as_int_base3 function to implement a sysinfo device. The revision map is stored in the device tree.
Signed-off-by: Sean Anderson sean.anderson@seco.com
.../sysinfo/gpio-sysinfo.txt | 37 +++++ drivers/sysinfo/Kconfig | 8 + drivers/sysinfo/Makefile | 1 + drivers/sysinfo/gpio.c | 138 ++++++++++++++++++ 4 files changed, 184 insertions(+) create mode 100644 doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt create mode 100644 drivers/sysinfo/gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
Please see below
diff --git a/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt new file mode 100644 index 0000000000..b5739d94e9 --- /dev/null +++ b/doc/device-tree-bindings/sysinfo/gpio-sysinfo.txt @@ -0,0 +1,37 @@ +GPIO-based Sysinfo device
+This binding describes several GPIOs which specify a board revision. Each GPIO +forms a digit in a ternary revision number. This revision is then mapped to a +name using the revisions and names properties.
+Each GPIO may be floating, pulled-up, or pulled-down, mapping to digits 2, 1, +and 0, respectively. The first GPIO forms the least-significant digit of the +revision. For example, consider the property
gpios = <&gpio 0>, <&gpio 1>, <&gpio 2>;+If GPIO 0 is pulled-up, GPIO 1 is pulled-down, and GPIO 2 is floating, then the +revision would be
0t201 = 2*9 + 0*3 + 1*3 = 19+If instead GPIO 0 is floating, GPIO 1 is pulled-up, and GPIO 2 is pulled-down, +then the revision would be
0t012 = 0*9 + 1*3 + 2*1 = 5+Required properties: +- compatible: should be "gpio-sysinfo". +- gpios: should be a list of gpios forming the revision number,
- least-significant-digit first
+- revisions: a list of known revisions; any revisions not present will have the
- name "unknown"
+- names: the name of each revision in revisions
+Example: +sysinfo {
compatible = "gpio-sysinfo";gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;revisions = <19>, <5>;names = "rev_a", "foo";+}; diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig index 85c1e81e41..381dcd8844 100644 --- a/drivers/sysinfo/Kconfig +++ b/drivers/sysinfo/Kconfig @@ -30,4 +30,12 @@ config SYSINFO_SMBIOS one which provides a way to specify this SMBIOS information in the devicetree, without needing any board-specific functionality.
+config SYSINFO_GPIO
bool "Enable gpio sysinfo driver"depends on DM_GPIO ?
helpSupport querying gpios to determine board revision. This uses gpios toform a ternary number (when they are pulled-up, -down, or floating).This ternary number is then mapped to a board revision name usingdevice tree properties.- endif
diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile index 6d04fcba1d..d9f708b7ea 100644 --- a/drivers/sysinfo/Makefile +++ b/drivers/sysinfo/Makefile @@ -4,5 +4,6 @@ # Mario Six, Guntermann & Drunck GmbH, mario.six@gdsys.cc obj-y += sysinfo-uclass.o obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o +obj-$(CONFIG_SYSINFO_GPIO) += gpio.o obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o diff --git a/drivers/sysinfo/gpio.c b/drivers/sysinfo/gpio.c new file mode 100644 index 0000000000..6a0eff3ec9 --- /dev/null +++ b/drivers/sysinfo/gpio.c @@ -0,0 +1,138 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <dm/device_compat.h>
should be at end
Can you clarify the ordering rules? I was following [1] which perscribes
<common.h> <others.h> <asm/...> <arch/arm/...> <linux/...> "local.h"[1] https://www.denx.de/wiki/U-Boot/CodingStyle#Include_files
Yes that's right, but dm/ is a directory so it goes with the other dirs at the end. I'll update the page to include dm/
Ok, so the general rule should be that directories go last?
Yes that's right.
Regards, SImon
This adds a test for the gpio-sysinfo driver.
Signed-off-by: Sean Anderson sean.anderson@seco.com ---
arch/sandbox/dts/test.dts | 7 ++++ test/dm/Makefile | 1 + test/dm/sysinfo-gpio.c | 69 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 test/dm/sysinfo-gpio.c
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index e95f4631bf..6137061d7a 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1340,6 +1340,13 @@ compatible = "sandbox,sysinfo-sandbox"; };
+ sysinfo-gpio { + compatible = "gpio-sysinfo"; + gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>; + revisions = <19>, <5>; + names = "rev_a", "foo"; + }; + some_regmapped-bus { #address-cells = <0x1>; #size-cells = <0x1>; diff --git a/test/dm/Makefile b/test/dm/Makefile index e70e50f402..2ba81ee76b 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -93,5 +93,6 @@ obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o ifneq ($(CONFIG_PINMUX),) obj-$(CONFIG_PINCONF) += pinmux.o endif +obj-$(CONFIG_SYSINFO_GPIO) += sysinfo-gpio.o endif endif # !SPL diff --git a/test/dm/sysinfo-gpio.c b/test/dm/sysinfo-gpio.c new file mode 100644 index 0000000000..a700352ff8 --- /dev/null +++ b/test/dm/sysinfo-gpio.c @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 Sean Anderson sean.anderson@seco.com + */ + +#include <common.h> +#include <dm.h> +#include <log.h> +#include <dm/test.h> +#include <sysinfo.h> +#include <test/test.h> +#include <test/ut.h> +#include <asm/gpio.h> + +static int dm_test_sysinfo_gpio(struct unit_test_state *uts) +{ + char buf[64]; + int val; + struct udevice *sysinfo, *gpio; + + ut_assertok(uclass_get_device_by_name(UCLASS_SYSINFO, "sysinfo-gpio", + &sysinfo)); + ut_assertok(uclass_get_device_by_name(UCLASS_GPIO, "base-gpios", &gpio)); + + /* + * Set up pins: pull-up (1), pull-down (0) and floating (2). This should + * result in digits 2 0 1, i.e. 2 * 9 + 1 * 3 = 19 + */ + sandbox_gpio_set_flags(gpio, 15, GPIOD_EXT_PULL_UP); + sandbox_gpio_set_flags(gpio, 16, GPIOD_EXT_PULL_DOWN); + sandbox_gpio_set_flags(gpio, 17, 0); + ut_assertok(sysinfo_detect(sysinfo)); + ut_assertok(sysinfo_get_int(sysinfo, SYSINFO_ID_REVISION, &val)); + ut_asserteq(19, val); + ut_assertok(sysinfo_get_str(sysinfo, SYSINFO_ID_REVISION, sizeof(buf), + buf)); + ut_asserteq_str("rev_a", buf); + + /* + * Set up pins: floating (2), pull-up (1) and pull-down (0). This should + * result in digits 0 1 2, i.e. 1 * 3 + 2 = 5 + */ + sandbox_gpio_set_flags(gpio, 15, 0); + sandbox_gpio_set_flags(gpio, 16, GPIOD_EXT_PULL_UP); + sandbox_gpio_set_flags(gpio, 17, GPIOD_EXT_PULL_DOWN); + ut_assertok(sysinfo_detect(sysinfo)); + ut_assertok(sysinfo_get_int(sysinfo, SYSINFO_ID_REVISION, &val)); + ut_asserteq(5, val); + ut_assertok(sysinfo_get_str(sysinfo, SYSINFO_ID_REVISION, sizeof(buf), + buf)); + ut_asserteq_str("foo", buf); + + /* + * Set up pins: floating (2), pull-up (1) and pull-down (0). This should + * result in digits 1 2 0, i.e. 1 * 9 + 2 * 3 = 15 + */ + sandbox_gpio_set_flags(gpio, 15, GPIOD_EXT_PULL_DOWN); + sandbox_gpio_set_flags(gpio, 16, 0); + sandbox_gpio_set_flags(gpio, 17, GPIOD_EXT_PULL_UP); + ut_assertok(sysinfo_detect(sysinfo)); + ut_assertok(sysinfo_get_int(sysinfo, SYSINFO_ID_REVISION, &val)); + ut_asserteq(15, val); + ut_assertok(sysinfo_get_str(sysinfo, SYSINFO_ID_REVISION, sizeof(buf), + buf)); + ut_asserteq_str("unknown", buf); + + return 0; +} +DM_TEST(dm_test_sysinfo_gpio, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
On Mon, 1 Mar 2021 at 15:46, Sean Anderson sean.anderson@seco.com wrote:
This adds a test for the gpio-sysinfo driver.
Signed-off-by: Sean Anderson sean.anderson@seco.com
arch/sandbox/dts/test.dts | 7 ++++ test/dm/Makefile | 1 + test/dm/sysinfo-gpio.c | 69 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+) create mode 100644 test/dm/sysinfo-gpio.c
Reviewed-by: Simon Glass sjg@chromium.org
nit below
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index e95f4631bf..6137061d7a 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -1340,6 +1340,13 @@ compatible = "sandbox,sysinfo-sandbox"; };
sysinfo-gpio {compatible = "gpio-sysinfo";gpios = <&gpio_a 15>, <&gpio_a 16>, <&gpio_a 17>;revisions = <19>, <5>;names = "rev_a", "foo";};some_regmapped-bus { #address-cells = <0x1>; #size-cells = <0x1>;diff --git a/test/dm/Makefile b/test/dm/Makefile index e70e50f402..2ba81ee76b 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -93,5 +93,6 @@ obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o ifneq ($(CONFIG_PINMUX),) obj-$(CONFIG_PINCONF) += pinmux.o endif +obj-$(CONFIG_SYSINFO_GPIO) += sysinfo-gpio.o endif endif # !SPL diff --git a/test/dm/sysinfo-gpio.c b/test/dm/sysinfo-gpio.c new file mode 100644 index 0000000000..a700352ff8 --- /dev/null +++ b/test/dm/sysinfo-gpio.c @@ -0,0 +1,69 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (C) 2021 Sean Anderson sean.anderson@seco.com
- */
+#include <common.h> +#include <dm.h> +#include <log.h> +#include <dm/test.h>
put at end
+#include <sysinfo.h> +#include <test/test.h>
then this after dm/test.h
+#include <test/ut.h> +#include <asm/gpio.h>
participants (2)
-
Sean Anderson -
Simon Glass