[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