[PATCH v2 12/13] sysinfo: Add driver for IOT2050 boards

Simon Glass sjg at chromium.org
Wed Oct 23 05:39:09 CEST 2024


Hi Jan,

On Tue, 22 Oct 2024 at 21:59, Jan Kiszka <jan.kiszka at siemens.com> wrote:
>
> On 22.10.24 19:00, Simon Glass wrote:
> > On Tue, 22 Oct 2024 at 08:06, Jan Kiszka <jan.kiszka at siemens.com> wrote:
> >>
> >> From: Li Hua Qian <huaqian.li at siemens.com>
> >>
> >> This brings a sysinfo driver and DT entry for the IOT2050 board series.
> >> It translates the board information passed from SE-Boot to SPL into
> >> values that can be retrieved via the sysinfo API. Will is already used
> >> to fill the SMBIOS table when booting via EFI.
> >>
> >> Signed-off-by: Li Hua Qian <huaqian.li at siemens.com>
> >> [Jan: split-off as separate patch, cleanup]
> >> Signed-off-by: Jan Kiszka <jan.kiszka at siemens.com>
> >> ---
> >>  .../dts/k3-am65-iot2050-common-u-boot.dtsi    |  18 +++
> >>  configs/iot2050_defconfig                     |   1 +
> >>  drivers/sysinfo/Kconfig                       |   7 +
> >>  drivers/sysinfo/Makefile                      |   1 +
> >>  drivers/sysinfo/iot2050.c                     | 143 ++++++++++++++++++
> >>  drivers/sysinfo/iot2050.h                     |  26 ++++
> >>  6 files changed, 196 insertions(+)
> >>  create mode 100644 drivers/sysinfo/iot2050.c
> >>  create mode 100644 drivers/sysinfo/iot2050.h
> >
> > I think strlcpy() might be better than strncpy() for this case
> >
>
> Ack.
>
> > The idea with sysinfo is that we use the same enum for all boards, so
> > please add your new things to sysinfo.h
> >
>
> See below.
>
> > Do you actually want all the sysinfo info in capitals? If so, that's
> > fine, just checking...
> >
>
> This is how it is being shipped already, yes.
>
> >>
> >> diff --git a/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi b/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
> >> index b6d2c816acc..55337179f9f 100644
> >> --- a/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
> >> +++ b/arch/arm/dts/k3-am65-iot2050-common-u-boot.dtsi
> >> @@ -14,6 +14,24 @@
> >>                 spi0 = &ospi0;
> >>         };
> >>
> >> +       sysinfo {
> >> +               compatible = "siemens,sysinfo-iot2050";
> >> +               /* TI_SRAM_SCRATCH_BOARD_EEPROM_START */
> >> +               offset = <0x40280000>;
> >> +               bootph-all;
> >> +
> >> +               smbios {
> >> +                       system {
> >> +                               manufacturer = "SIEMENS AG";
> >> +                               product = "SIMATIC IOT2050";
> >> +                       };
> >> +
> >> +                       baseboard {
> >> +                               manufacturer = "SIEMENS AG";
> >> +                       };
> >> +               };
> >> +       };
> >> +
> >>         leds {
> >>                 bootph-all;
> >>                 status-led-red {
> >> diff --git a/configs/iot2050_defconfig b/configs/iot2050_defconfig
> >> index 2624f0cb573..574e232d119 100644
> >> --- a/configs/iot2050_defconfig
> >> +++ b/configs/iot2050_defconfig
> >> @@ -141,6 +141,7 @@ CONFIG_SOC_TI=y
> >>  CONFIG_SPI=y
> >>  CONFIG_DM_SPI=y
> >>  CONFIG_CADENCE_QSPI=y
> >> +CONFIG_SYSINFO=y
> >>  CONFIG_SYSRESET=y
> >>  CONFIG_SPL_SYSRESET=y
> >>  CONFIG_SYSRESET_TI_SCI=y
> >> diff --git a/drivers/sysinfo/Kconfig b/drivers/sysinfo/Kconfig
> >> index 2030e4babc9..df83df69ffb 100644
> >> --- a/drivers/sysinfo/Kconfig
> >> +++ b/drivers/sysinfo/Kconfig
> >> @@ -31,6 +31,13 @@ config SYSINFO_RCAR3
> >>         help
> >>           Support querying SoC version information for Renesas R-Car Gen3.
> >>
> >> +config SYSINFO_IOT2050
> >> +       bool "Enable sysinfo driver for the Siemens IOT2050"
> >> +       depends on TARGET_IOT2050_A53
> >> +       default y if TARGET_IOT2050_A53
> >> +       help
> >> +         Support querying device information for Siemens IOT2050.
> >> +
> >>  config SYSINFO_SANDBOX
> >>         bool "Enable sysinfo driver for the Sandbox board"
> >>         help
> >> diff --git a/drivers/sysinfo/Makefile b/drivers/sysinfo/Makefile
> >> index 680dde77fe8..26ca3150999 100644
> >> --- a/drivers/sysinfo/Makefile
> >> +++ b/drivers/sysinfo/Makefile
> >> @@ -5,6 +5,7 @@
> >>  obj-y += sysinfo-uclass.o
> >>  obj-$(CONFIG_SYSINFO_GAZERBEAM) += gazerbeam.o
> >>  obj-$(CONFIG_SYSINFO_GPIO) += gpio.o
> >> +obj-$(CONFIG_SYSINFO_IOT2050) += iot2050.o
> >>  obj-$(CONFIG_SYSINFO_RCAR3) += rcar3.o
> >>  obj-$(CONFIG_SYSINFO_SANDBOX) += sandbox.o
> >>  obj-$(CONFIG_SYSINFO_SMBIOS) += smbios.o
> >> diff --git a/drivers/sysinfo/iot2050.c b/drivers/sysinfo/iot2050.c
> >> new file mode 100644
> >> index 00000000000..5359d6b8d62
> >> --- /dev/null
> >> +++ b/drivers/sysinfo/iot2050.c
> >> @@ -0,0 +1,143 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/*
> >> + * Copyright (c) Siemens AG, 2024
> >> + */
> >> +
> >> +#include <dm.h>
> >> +#include <sysinfo.h>
> >> +#include <net.h>
> >> +#include <asm/arch/hardware.h>
> >> +
> >> +#include "iot2050.h"
> >> +
> >> +#define IOT2050_INFO_MAGIC             0x20502050
> >> +
> >> +struct iot2050_info {
> >> +       u32 magic;
> >> +       u16 size;
> >> +       char name[20 + 1];
> >> +       char serial[16 + 1];
> >> +       char mlfb[18 + 1];
> >> +       char uuid[32 + 1];
> >> +       char a5e[18 + 1];
> >> +       u8 mac_addr_cnt;
> >> +       u8 mac_addr[8][ARP_HLEN];
> >> +       char seboot_version[40 + 1];
> >> +       u8 padding[3];
> >> +       u32 ddr_size_mb;
> >> +} __packed;
> >> +
> >> +/**
> >> + * struct sysinfo_iot2050_priv - sysinfo private data
> >> + * @info: iot2050 board info
> >> + */
> >> +struct sysinfo_iot2050_priv {
> >> +       struct iot2050_info *info;
> >> +};
> >> +
> >> +static int sysinfo_iot2050_detect(struct udevice *dev)
> >> +{
> >> +       struct sysinfo_iot2050_priv *priv = dev_get_priv(dev);
> >> +
> >> +       if (priv->info == NULL || priv->info->magic != IOT2050_INFO_MAGIC)
> >> +               return -EFAULT;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static int sysinfo_iot2050_get_str(struct udevice *dev, int id, size_t size,
> >> +                                  char *val)
> >> +{
> >> +       struct sysinfo_iot2050_priv *priv = dev_get_priv(dev);
> >> +       char byte_str[3] = {0};
> >> +       unsigned int n;
> >> +
> >> +       switch (id) {
> >> +       case BOARD_NAME:
> >> +               strncpy(val, priv->info->name, size);
> >> +               break;
> >> +       case BOARD_SERIAL:
> >> +               strncpy(val, priv->info->serial, size);
> >> +               break;
> >> +       case BOARD_MLFB:
> >> +               strncpy(val, priv->info->mlfb, size);
> >> +               break;
> >> +       case BOARD_UUID:
> >> +               for (n = 0; n < min(size, (size_t)16); n++) {
> >> +                       memcpy(byte_str, priv->info->uuid + n * 2, 2);
> >> +                       val[n] = (char)hextoul(byte_str, NULL);
> >> +               }
> >> +               break;
> >> +       case BOARD_A5E:
> >> +               strncpy(val, priv->info->a5e, size);
> >> +               break;
> >> +       case BOARD_SEBOOT_VER:
> >> +               strncpy(val, priv->info->seboot_version, size);
> >> +               break;
> >> +       case BOARD_MAC_ADDR_1:
> >> +       case BOARD_MAC_ADDR_2:
> >> +       case BOARD_MAC_ADDR_3:
> >> +       case BOARD_MAC_ADDR_4:
> >> +       case BOARD_MAC_ADDR_5:
> >> +       case BOARD_MAC_ADDR_6:
> >> +       case BOARD_MAC_ADDR_7:
> >> +       case BOARD_MAC_ADDR_8:
> >> +               memcpy(val, priv->info->mac_addr[id - BOARD_MAC_ADDR_START],
> >> +                      ARP_HLEN);
> >
> > For this, we really need another parameter to get_str(), i.e. the
> > index, since we don't know how many MACs there will be. So how about
> > adding a few new operations?
> >
> > int (*get_item_count)(struct udevice *dev, int id);
> > int (*get_str_item)(struct udevice *dev, int id, int index, size_t
> > size, char *val);
>
> Ok, makes sense.
>
> >
> >> +               return 0;
> >> +       case BOARD_DDR_SIZE:
> >> +               memcpy(val, &priv->info->ddr_size_mb,
> >> +                      sizeof(priv->info->ddr_size_mb));
> >> +               return 0;
> >> +       default:
> >> +               return -EINVAL;
> >> +       };
> >> +
> >> +       val[size - 1] = '\0';
> >> +       return 0;
> >> +}
> >> +
> >> +static int sysinfo_iot2050_get_int(struct udevice *dev, int id, int *val)
> >> +{
> >> +       struct sysinfo_iot2050_priv *priv = dev_get_priv(dev);
> >> +
> >> +       switch (id) {
> >> +       case BOARD_MAC_ADDR_CNT:
> >> +               *val = priv->info->mac_addr_cnt;
> >> +               return 0;
> >> +       default:
> >> +               return -EINVAL;
> >> +       };
> >> +}
> >> +
> >> +static const struct sysinfo_ops sysinfo_iot2050_ops = {
> >> +       .detect = sysinfo_iot2050_detect,
> >> +       .get_str = sysinfo_iot2050_get_str,
> >> +       .get_int = sysinfo_iot2050_get_int,
> >> +};
> >> +
> >> +static int sysinfo_iot2050_probe(struct udevice *dev)
> >> +{
> >> +       struct sysinfo_iot2050_priv *priv = dev_get_priv(dev);
> >> +       unsigned long offset;
> >> +
> >> +       offset = dev_read_u32_default(dev, "offset",
> >> +                                     TI_SRAM_SCRATCH_BOARD_EEPROM_START);
> >> +       priv->info = (struct iot2050_info *)offset;
> >> +
> >> +       return 0;
> >> +}
> >> +
> >> +static const struct udevice_id sysinfo_iot2050_ids[] = {
> >> +       { .compatible = "siemens,sysinfo-iot2050" },
> >> +       { /* sentinel */ }
> >> +};
> >> +
> >> +U_BOOT_DRIVER(sysinfo_iot2050) = {
> >> +       .name           = "sysinfo_iot2050",
> >> +       .id             = UCLASS_SYSINFO,
> >> +       .of_match       = sysinfo_iot2050_ids,
> >> +       .ops            = &sysinfo_iot2050_ops,
> >> +       .priv_auto      = sizeof(struct sysinfo_iot2050_priv),
> >> +       .probe          = sysinfo_iot2050_probe,
> >> +};
> >> diff --git a/drivers/sysinfo/iot2050.h b/drivers/sysinfo/iot2050.h
> >> new file mode 100644
> >> index 00000000000..f659a8282b5
> >> --- /dev/null
> >> +++ b/drivers/sysinfo/iot2050.h
> >> @@ -0,0 +1,26 @@
> >> +/* SPDX-License-Identifier: GPL-2.0 */
> >> +/*
> >> + * Copyright (c) Siemens AG, 2024
> >> + */
> >> +
> >> +#include <sysinfo.h>
> >> +
> >> +enum sysinfo_id_iot2050 {
> >> +       BOARD_MLFB              = SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
> >> +       BOARD_SERIAL            = SYSINFO_ID_SMBIOS_SYSTEM_SERIAL,
> >> +       BOARD_UUID              = SYSINFO_ID_SMBIOS_SYSTEM_UUID,
> >> +       BOARD_A5E               = SYSINFO_ID_SMBIOS_BASEBOARD_PRODUCT,
> >> +       BOARD_NAME              = SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
> >
> > Do you need to rename these? It seems better to use the standard
> > names, although I do think they are quite long.
>
> We don't rename, we map pre-existing field names in EEPROM and
> pre-existing board-specific U-Boot env vars to sysinfo fields here.

Yes, that's what I mean. Can you just use the SYSINFO_ID... values
directory, rather than creating mapping?

>
> >
> >> +       BOARD_SEBOOT_VER        = SYSINFO_ID_USER + 0,
> >> +       BOARD_MAC_ADDR_CNT      = SYSINFO_ID_USER + 1,
> >> +       BOARD_MAC_ADDR_1        = SYSINFO_ID_USER + 2,
> >> +       BOARD_MAC_ADDR_START    = BOARD_MAC_ADDR_1,
> >> +       BOARD_MAC_ADDR_2        = SYSINFO_ID_USER + 3,
> >> +       BOARD_MAC_ADDR_3        = SYSINFO_ID_USER + 4,
> >> +       BOARD_MAC_ADDR_4        = SYSINFO_ID_USER + 5,
> >> +       BOARD_MAC_ADDR_5        = SYSINFO_ID_USER + 6,
> >> +       BOARD_MAC_ADDR_6        = SYSINFO_ID_USER + 7,
> >> +       BOARD_MAC_ADDR_7        = SYSINFO_ID_USER + 8,
> >> +       BOARD_MAC_ADDR_8        = SYSINFO_ID_USER + 9,
> >> +       BOARD_DDR_SIZE          = SYSINFO_ID_USER + 10,
> >
> > These are the ones which should be added as standard. Be sure to
> > comment them as necessary.
>
> You mean that BOARD_MAC_ADDR and BOARD_DDR_SIZE should become
> SYSINFO_ID_BOARD_MAC_ADDR and SYSINFO_ID_BOARD_DDR_SIZE? But
> BOARD_SEBOOT_VER is nothing that has any meaning beyond our board.

For BOARD_SEBOOT_VER, I don't know what it is, but just add a comment
and put it in sysinfo.h. It is OK to add board-specific stufff there.

Is BOARD_DDR_SIZE the memory size? Perhaps rename it to RAM_SIZE ?

Regards,
Simon


More information about the U-Boot mailing list