[PATCH 4/5] sysinfo: Add gpio-sysinfo driver

[Simon Glass]

Hi Sean,

On Fri, 5 Mar 2021 at 10:24, Sean Anderson <sean.anderson at 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 at 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 at 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 at 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 at 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 ?
> >>   >
> >>   >>> +     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 at 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 at 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