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