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