[PATCH v2 00/12] smbios: Enhancements for more flexibility

[PATCH v2 00/12] smbios: Enhancements for more flexibility

[Simon Glass]

This series includes various patches to allow more flexibility as to where
the data for SMBIOS tables comes from:

- introduces some standard sysinfo options as a source, e.g. to read
   strapping pins to determine the board revision
- allows the U-Boot version number to be included
- allows the version number to be provided programmatically, e.g. to
   support the build system adding information after U-Boot is built

Documentation is added for how to obtain version information.

The code is also refactored a little to make it easier to maintain.

Changes in v2:
- Add a comment about dropping the century
- Zero the context's dev pointer if not used
- Correct documentation format
- Add new patch to fix sysinfo with CONFIG_IS_ENABLED()
- Add new patch to fix crash on coral

Simon Glass (12):
  README: Add doumentation for version information
  Makefile: Provide numeric versions
  smbios: Move smbios_write_type to the C file
  smbios: Use char consistently for the eos member
  smbios: Set BIOS release version
  smbios: Use a struct to keep track of context
  smbios: Drop the eos parameter
  smbios: Track the end of the string table
  smbios: Add more options for the BIOS version string
  sysinfo: Move #ifdef so that operations are always defined
  x86: coral: Add sysinfo ops
  smbios: Allow a few values to come from sysinfo

 Makefile                              |   4 +
 README                                |  92 ++++++++++
 board/google/chromebook_coral/coral.c |   5 +
 include/asm-generic/global_data.h     |   6 +
 include/smbios.h                      |  26 +--
 include/sysinfo.h                     |  13 +-
 lib/smbios.c                          | 243 +++++++++++++++++++-------
 7 files changed, 315 insertions(+), 74 deletions(-)

-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 01/12] README: Add doumentation for version information

[Simon Glass]

There are quite a few available version options in U-Boot. Add a list of
the available Makefile variables and #defines, along with examples.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 README | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/README b/README
index 89606c8adde..f32c69c8d77 100644
--- a/README
+++ b/README
@@ -1830,6 +1830,90 @@ The following options need to be configured:
 		Time to wait after FPGA configuration. The default is
 		200 ms.
 
+- Version identification:
+
+		U-Boot releases are named by year and patch level, for example
+		2020.10 means the release that came out in October 2020.
+		Release candidates are tagged every few weeks as the project
+		heads to the next release. So 2020.10-rc1 was the first
+		release candidate (RC), tagged soon after 2020.07 was released.
+
+		See here for full details:
+		  https://www.denx.de/wiki/view/U-Boot/ReleaseCycle
+
+		Within the build system, various Makefile variables are created,
+		making use of VERSION, PATCHLEVEL and EXTRAVERSION defined at
+		the top of 'Makefile'. There is also SUBLEVEL available for
+		downstream use. See also CONFIG_IDENT_STRING.
+
+		Some variables end up in a generated header file at
+		include/generated/version_autogenerated.h and can be accessed
+		from C source by including <version.h>
+
+		The following are available:
+
+		  UBOOTRELEASE (Makefile)
+			  Full release version as a string. If this is not a
+			  tagged release, it also includes the number of commits
+			  since the last tag as well as the the git hash. If
+			  there are uncommitted changes a '-dirty' suffix is
+			  added too.
+			  This is written by scripts/setlocalversion
+			  (maintained by Linux) to include/config/uboot.release
+			  and ends up in the UBOOTRELEASE Makefile variable.
+
+			  Examples:
+			     2020.10-rc3
+			     2021.01-rc5-00248-g60dd854f3ba-dirty
+
+		  PLAIN_VERSION (string #define)
+			  This is UBOOTRELEASE but available in C source.
+
+			  Examples:
+			     2020.10
+			     2021.01-rc5-00248-g60dd854f3ba-dirty
+
+		  UBOOTVERSION (Makefile)
+			  This holds just the first three components of
+			  UBOOTRELEASE (i.e. not the git hash, etc.)
+
+			  Examples:
+			     2020.10
+			     2021.01-rc5
+
+		  U_BOOT_VERSION (string #define)
+			  "U-Boot " followed by UBOOTRELEASE, for example:
+
+			     U-Boot 2020.10
+			     U-Boot 2021.01-rc5
+
+			  This is used as part of the banner string when U-Boot
+			  starts.
+
+		  U_BOOT_VERSION_STRING (string #define)
+			  U_BOOT_VERSION followed by build-time information
+			  and CONFIG_IDENT_STRING.
+
+			  Examples:
+			     U-Boot 2020.10 (Jan 06 2021 - 08:50:36 -0700)
+                             U-Boot 2021.01-rc5-00248-g60dd854f3ba-dirty (Jan 06 2021 - 08:50:36 -0700) for spring
+
+		Build date/time is also included. See the generated file
+		include/generated/timestamp_autogenerated.h for the available
+		fields. For example:
+
+		  #define U_BOOT_DATE "Jan 06 2021"     (US format only)
+		  #define U_BOOT_TIME "08:50:36"        (24-hour clock)
+		  #define U_BOOT_TZ "-0700"             (Time zone in hours)
+		  #define U_BOOT_DMI_DATE "01/06/2021"  (US format only)
+		  #define U_BOOT_BUILD_DATE 0x20210106  (hex yyyymmdd format)
+		  #define U_BOOT_EPOCH 1609948236
+
+		The Epoch is the number of seconds since midnight on 1/1/70.
+		Every time you build U-Boot this will update based on the time
+		on your build machine. See 'Reproducible builds' if you want to
+		avoid that.
+
 - Configuration Management:
 
 		CONFIG_IDENT_STRING
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 02/12] Makefile: Provide numeric versions

[Simon Glass]

For SMBIOS we want to store the numeric version numbers in the tables. It
does not make sense to parse the strings. Instead, add new #defines with
the version and patchlevel.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 Makefile | 4 ++++
 README   | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/Makefile b/Makefile
index cef149dec97..67972dbdb9b 100644
--- a/Makefile
+++ b/Makefile
@@ -1849,9 +1849,13 @@ prepare: prepare0
 # Generate some files
 # ---------------------------------------------------------------------------
 
+# Use sed to remove leading zeros from PATCHLEVEL to avoid using octal numbers
 define filechk_version.h
 	(echo \#define PLAIN_VERSION \"$(UBOOTRELEASE)\"; \
 	echo \#define U_BOOT_VERSION \"U-Boot \" PLAIN_VERSION; \
+	echo \#define U_BOOT_VERSION_NUM $(VERSION); \
+	echo \#define U_BOOT_VERSION_NUM_PATCH $$(echo $(PATCHLEVEL) | \
+		sed -e "s/^0*//"); \
 	echo \#define CC_VERSION_STRING \"$$(LC_ALL=C $(CC) --version | head -n 1)\"; \
 	echo \#define LD_VERSION_STRING \"$$(LC_ALL=C $(LD) --version | head -n 1)\"; )
 endef
diff --git a/README b/README
index f32c69c8d77..37d21f18ca6 100644
--- a/README
+++ b/README
@@ -1898,6 +1898,14 @@ The following options need to be configured:
 			     U-Boot 2020.10 (Jan 06 2021 - 08:50:36 -0700)
                              U-Boot 2021.01-rc5-00248-g60dd854f3ba-dirty (Jan 06 2021 - 08:50:36 -0700) for spring
 
+		  U_BOOT_VERSION_NUM (integer #define)
+			  Release year, e.g. 2021 for release 2021.01. Note
+			  this is an integer, not a string.
+
+		  U_BOOT_VERSION_NUM_PATCH (integer #define)
+			  Patch number, e.g. 1 for release 2020.01. Note
+			  this is an integer, not a string.
+
 		Build date/time is also included. See the generated file
 		include/generated/timestamp_autogenerated.h for the available
 		fields. For example:
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 03/12] smbios: Move smbios_write_type to the C file

[Simon Glass]

This type is not used outside the smbios.c file so there is no need for it
to be in the header file. Move it.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---

(no changes since v1)

 include/smbios.h | 10 ----------
 lib/smbios.c     | 10 ++++++++++
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index 1846607c3cf..fc69188a8fe 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -219,16 +219,6 @@ static inline void fill_smbios_header(void *table, int type,
 	header->handle = handle;
 }
 
-/**
- * Function prototype to write a specific type of SMBIOS structure
- *
- * @addr:	start address to write the structure
- * @handle:	the structure's handle, a unique 16-bit number
- * @node:	node containing the information to write (ofnode_null() if none)
- * @return:	size of the structure
- */
-typedef int (*smbios_write_type)(ulong *addr, int handle, ofnode node);
-
 /**
  * write_smbios_table() - Write SMBIOS table
  *
diff --git a/lib/smbios.c b/lib/smbios.c
index 1e10fa84207..b1171f544a8 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -17,6 +17,16 @@
 #include <dm/uclass-internal.h>
 #endif
 
+/**
+ * Function prototype to write a specific type of SMBIOS structure
+ *
+ * @addr:	start address to write the structure
+ * @handle:	the structure's handle, a unique 16-bit number
+ * @node:	node containing the information to write (ofnode_null() if none)
+ * @return:	size of the structure
+ */
+typedef int (*smbios_write_type)(ulong *addr, int handle, ofnode node);
+
 /**
  * struct smbios_write_method - Information about a table-writing function
  *
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 04/12] smbios: Use char consistently for the eos member

[Simon Glass]

At present a few of the structs use u8 instead of char. This is a string,
so char is better. Update them.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
---

(no changes since v1)

 include/smbios.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index fc69188a8fe..1cbeabf9522 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -183,14 +183,14 @@ struct __packed smbios_type32 {
 	u16 handle;
 	u8 reserved[6];
 	u8 boot_status;
-	u8 eos[SMBIOS_STRUCT_EOS_BYTES];
+	char eos[SMBIOS_STRUCT_EOS_BYTES];
 };
 
 struct __packed smbios_type127 {
 	u8 type;
 	u8 length;
 	u16 handle;
-	u8 eos[SMBIOS_STRUCT_EOS_BYTES];
+	char eos[SMBIOS_STRUCT_EOS_BYTES];
 };
 
 struct __packed smbios_header {
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 05/12] smbios: Set BIOS release version

[Simon Glass]

We may as well include the U-Boot release information in the type-0 table
since it is designed for that purpose.

U-Boot uses release versions based on the year and month. The year cannot
fit in a byte, so drop the century.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add a comment about dropping the century

 lib/smbios.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index b1171f544a8..a072d9ec10b 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -143,8 +143,9 @@ static int smbios_write_type0(ulong *current, int handle, ofnode node)
 #endif
 	t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
 
-	t->bios_major_release = 0xff;
-	t->bios_minor_release = 0xff;
+	/* bios_major_release has only one byte, so drop century */
+	t->bios_major_release = U_BOOT_VERSION_NUM % 100;
+	t->bios_minor_release = U_BOOT_VERSION_NUM_PATCH;
 	t->ec_major_release = 0xff;
 	t->ec_minor_release = 0xff;
 
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 06/12] smbios: Use a struct to keep track of context

[Simon Glass]

At present we pass the ofnode to each function. We also pass the 'eos'
pointer for adding new strings. We don't track the current end of the
string table, so have smbios_string_table_len() to find that.

The code can be made more efficient if it keeps information in a
context struct. This also makes it easier to add more features.

As a first step, switch the ofnode parameter to be a context pointer.
Update smbios_add_prop() at the same time to avoid changing the same
lines of code in consecutive patches.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Zero the context's dev pointer if not used

 lib/smbios.c | 87 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 55 insertions(+), 32 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index a072d9ec10b..4d2cb0f85e2 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -17,15 +17,27 @@
 #include <dm/uclass-internal.h>
 #endif
 
+/**
+ * struct smbios_ctx - context for writing SMBIOS tables
+ *
+ * @node:	node containing the information to write (ofnode_null() if none)
+ * @dev:	sysinfo device to use (NULL if none)
+ */
+struct smbios_ctx {
+	ofnode node;
+	struct udevice *dev;
+};
+
 /**
  * Function prototype to write a specific type of SMBIOS structure
  *
  * @addr:	start address to write the structure
  * @handle:	the structure's handle, a unique 16-bit number
- * @node:	node containing the information to write (ofnode_null() if none)
+ * @ctx:	context for writing the tables
  * @return:	size of the structure
  */
-typedef int (*smbios_write_type)(ulong *addr, int handle, ofnode node);
+typedef int (*smbios_write_type)(ulong *addr, int handle,
+				 struct smbios_ctx *ctx);
 
 /**
  * struct smbios_write_method - Information about a table-writing function
@@ -78,17 +90,18 @@ static int smbios_add_string(char *start, const char *str)
  * smbios_add_prop() - Add a property from the device tree
  *
  * @start:	string area start address
- * @node:	node containing the information to write (ofnode_null() if none)
+ * @ctx:	context for writing the tables
  * @prop:	property to write
  * @return 0 if not found, else SMBIOS string number (1 or more)
  */
-static int smbios_add_prop(char *start, ofnode node, const char *prop)
+static int smbios_add_prop(char *start, struct smbios_ctx *ctx,
+			   const char *prop)
 {
 
 	if (IS_ENABLED(CONFIG_OF_CONTROL)) {
 		const char *str;
 
-		str = ofnode_read_string(node, prop);
+		str = ofnode_read_string(ctx->node, prop);
 		if (str)
 			return smbios_add_string(start, str);
 	}
@@ -118,7 +131,8 @@ static int smbios_string_table_len(char *start)
 	return len + 1;
 }
 
-static int smbios_write_type0(ulong *current, int handle, ofnode node)
+static int smbios_write_type0(ulong *current, int handle,
+			      struct smbios_ctx *ctx)
 {
 	struct smbios_type0 *t;
 	int len = sizeof(struct smbios_type0);
@@ -156,7 +170,8 @@ static int smbios_write_type0(ulong *current, int handle, ofnode node)
 	return len;
 }
 
-static int smbios_write_type1(ulong *current, int handle, ofnode node)
+static int smbios_write_type1(ulong *current, int handle,
+			      struct smbios_ctx *ctx)
 {
 	struct smbios_type1 *t;
 	int len = sizeof(struct smbios_type1);
@@ -165,17 +180,17 @@ static int smbios_write_type1(ulong *current, int handle, ofnode node)
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type1));
 	fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
-	t->manufacturer = smbios_add_prop(t->eos, node, "manufacturer");
-	t->product_name = smbios_add_prop(t->eos, node, "product");
-	t->version = smbios_add_prop(t->eos, node, "version");
+	t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
+	t->product_name = smbios_add_prop(t->eos, ctx, "product");
+	t->version = smbios_add_prop(t->eos, ctx, "version");
 	if (serial_str) {
 		t->serial_number = smbios_add_string(t->eos, serial_str);
 		strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
 	} else {
-		t->serial_number = smbios_add_prop(t->eos, node, "serial");
+		t->serial_number = smbios_add_prop(t->eos, ctx, "serial");
 	}
-	t->sku_number = smbios_add_prop(t->eos, node, "sku");
-	t->family = smbios_add_prop(t->eos, node, "family");
+	t->sku_number = smbios_add_prop(t->eos, ctx, "sku");
+	t->family = smbios_add_prop(t->eos, ctx, "family");
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
@@ -184,7 +199,8 @@ static int smbios_write_type1(ulong *current, int handle, ofnode node)
 	return len;
 }
 
-static int smbios_write_type2(ulong *current, int handle, ofnode node)
+static int smbios_write_type2(ulong *current, int handle,
+			      struct smbios_ctx *ctx)
 {
 	struct smbios_type2 *t;
 	int len = sizeof(struct smbios_type2);
@@ -192,9 +208,9 @@ static int smbios_write_type2(ulong *current, int handle, ofnode node)
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type2));
 	fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
-	t->manufacturer = smbios_add_prop(t->eos, node, "manufacturer");
-	t->product_name = smbios_add_prop(t->eos, node, "product");
-	t->asset_tag_number = smbios_add_prop(t->eos, node, "asset-tag");
+	t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
+	t->product_name = smbios_add_prop(t->eos, ctx, "product");
+	t->asset_tag_number = smbios_add_prop(t->eos, ctx, "asset-tag");
 	t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
 	t->board_type = SMBIOS_BOARD_MOTHERBOARD;
 
@@ -205,7 +221,8 @@ static int smbios_write_type2(ulong *current, int handle, ofnode node)
 	return len;
 }
 
-static int smbios_write_type3(ulong *current, int handle, ofnode node)
+static int smbios_write_type3(ulong *current, int handle,
+			      struct smbios_ctx *ctx)
 {
 	struct smbios_type3 *t;
 	int len = sizeof(struct smbios_type3);
@@ -213,7 +230,7 @@ static int smbios_write_type3(ulong *current, int handle, ofnode node)
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type3));
 	fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
-	t->manufacturer = smbios_add_prop(t->eos, node, "manufacturer");
+	t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
 	t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP;
 	t->bootup_state = SMBIOS_STATE_SAFE;
 	t->power_supply_state = SMBIOS_STATE_SAFE;
@@ -227,7 +244,8 @@ static int smbios_write_type3(ulong *current, int handle, ofnode node)
 	return len;
 }
 
-static void smbios_write_type4_dm(struct smbios_type4 *t, ofnode node)
+static void smbios_write_type4_dm(struct smbios_type4 *t,
+				  struct smbios_ctx *ctx)
 {
 	u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN;
 	const char *vendor = "Unknown";
@@ -259,7 +277,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t, ofnode node)
 	t->processor_version = smbios_add_string(t->eos, name);
 }
 
-static int smbios_write_type4(ulong *current, int handle, ofnode node)
+static int smbios_write_type4(ulong *current, int handle,
+			      struct smbios_ctx *ctx)
 {
 	struct smbios_type4 *t;
 	int len = sizeof(struct smbios_type4);
@@ -268,7 +287,7 @@ static int smbios_write_type4(ulong *current, int handle, ofnode node)
 	memset(t, 0, sizeof(struct smbios_type4));
 	fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
 	t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
-	smbios_write_type4_dm(t, node);
+	smbios_write_type4_dm(t, ctx);
 	t->status = SMBIOS_PROCESSOR_STATUS_ENABLED;
 	t->processor_upgrade = SMBIOS_PROCESSOR_UPGRADE_NONE;
 	t->l1_cache_handle = 0xffff;
@@ -283,7 +302,8 @@ static int smbios_write_type4(ulong *current, int handle, ofnode node)
 	return len;
 }
 
-static int smbios_write_type32(ulong *current, int handle, ofnode node)
+static int smbios_write_type32(ulong *current, int handle,
+			       struct smbios_ctx *ctx)
 {
 	struct smbios_type32 *t;
 	int len = sizeof(struct smbios_type32);
@@ -298,7 +318,8 @@ static int smbios_write_type32(ulong *current, int handle, ofnode node)
 	return len;
 }
 
-static int smbios_write_type127(ulong *current, int handle, ofnode node)
+static int smbios_write_type127(ulong *current, int handle,
+				struct smbios_ctx *ctx)
 {
 	struct smbios_type127 *t;
 	int len = sizeof(struct smbios_type127);
@@ -327,7 +348,7 @@ ulong write_smbios_table(ulong addr)
 {
 	ofnode parent_node = ofnode_null();
 	struct smbios_entry *se;
-	struct udevice *dev;
+	struct smbios_ctx ctx;
 	ulong table_addr;
 	ulong tables;
 	int len = 0;
@@ -337,10 +358,13 @@ ulong write_smbios_table(ulong addr)
 	int isize;
 	int i;
 
+	ctx.node = ofnode_null();
 	if (IS_ENABLED(CONFIG_OF_CONTROL)) {
-		uclass_first_device(UCLASS_SYSINFO, &dev);
-		if (dev)
-			parent_node = dev_read_subnode(dev, "smbios");
+		uclass_first_device(UCLASS_SYSINFO, &ctx.dev);
+		if (ctx.dev)
+			parent_node = dev_read_subnode(ctx.dev, "smbios");
+	} else {
+		ctx.dev = NULL;
 	}
 
 	/* 16 byte align the table address */
@@ -356,14 +380,13 @@ ulong write_smbios_table(ulong addr)
 	/* populate minimum required tables */
 	for (i = 0; i < ARRAY_SIZE(smbios_write_funcs); i++) {
 		const struct smbios_write_method *method;
-		ofnode node = ofnode_null();
 		int tmp;
 
 		method = &smbios_write_funcs[i];
 		if (IS_ENABLED(CONFIG_OF_CONTROL) && method->subnode_name)
-			node = ofnode_find_subnode(parent_node,
-						   method->subnode_name);
-		tmp = method->write((ulong *)&addr, handle++, node);
+			ctx.node = ofnode_find_subnode(parent_node,
+						       method->subnode_name);
+		tmp = method->write((ulong *)&addr, handle++, &ctx);
 
 		max_struct_size = max(max_struct_size, tmp);
 		len += tmp;
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 07/12] smbios: Drop the eos parameter

[Simon Glass]

We can store this in the context and avoid passing it to each function.
This makes it easier to follow and will also allow keeping track of the
end of the string table (in future patches).

Add an 'eos' field to the context and create a function to set it up.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/smbios.c | 61 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index 4d2cb0f85e2..1d9bde0c3c2 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -22,10 +22,13 @@
  *
  * @node:	node containing the information to write (ofnode_null() if none)
  * @dev:	sysinfo device to use (NULL if none)
+ * @eos:	end-of-string pointer for the table being processed. This is set
+ *		up when we start processing a table
  */
 struct smbios_ctx {
 	ofnode node;
 	struct udevice *dev;
+	char *eos;
 };
 
 /**
@@ -57,14 +60,15 @@ struct smbios_write_method {
  * This adds a string to the string area which is appended directly after
  * the formatted portion of an SMBIOS structure.
  *
- * @start:	string area start address
+ * @ctx:	SMBIOS context
  * @str:	string to add
  * @return:	string number in the string area (1 or more)
  */
-static int smbios_add_string(char *start, const char *str)
+static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
 {
 	int i = 1;
-	char *p = start;
+	char *p = ctx->eos;
+
 	if (!*str)
 		str = "Unknown";
 
@@ -90,25 +94,28 @@ static int smbios_add_string(char *start, const char *str)
  * smbios_add_prop() - Add a property from the device tree
  *
  * @start:	string area start address
- * @ctx:	context for writing the tables
+ * @node:	node containing the information to write (ofnode_null() if none)
  * @prop:	property to write
  * @return 0 if not found, else SMBIOS string number (1 or more)
  */
-static int smbios_add_prop(char *start, struct smbios_ctx *ctx,
-			   const char *prop)
+static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
 {
-
 	if (IS_ENABLED(CONFIG_OF_CONTROL)) {
 		const char *str;
 
 		str = ofnode_read_string(ctx->node, prop);
 		if (str)
-			return smbios_add_string(start, str);
+			return smbios_add_string(ctx, str);
 	}
 
 	return 0;
 }
 
+static void set_eos(struct smbios_ctx *ctx, char *eos)
+{
+	ctx->eos = eos;
+}
+
 /**
  * smbios_string_table_len() - compute the string area size
  *
@@ -140,9 +147,10 @@ static int smbios_write_type0(ulong *current, int handle,
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type0));
 	fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
-	t->vendor = smbios_add_string(t->eos, "U-Boot");
-	t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
-	t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
+	set_eos(ctx, t->eos);
+	t->vendor = smbios_add_string(ctx, "U-Boot");
+	t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
+	t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE);
 #ifdef CONFIG_ROM_SIZE
 	t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
 #endif
@@ -180,17 +188,18 @@ static int smbios_write_type1(ulong *current, int handle,
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type1));
 	fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
-	t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
-	t->product_name = smbios_add_prop(t->eos, ctx, "product");
-	t->version = smbios_add_prop(t->eos, ctx, "version");
+	set_eos(ctx, t->eos);
+	t->manufacturer = smbios_add_prop(ctx, "manufacturer");
+	t->product_name = smbios_add_prop(ctx, "product");
+	t->version = smbios_add_prop(ctx, "version");
 	if (serial_str) {
-		t->serial_number = smbios_add_string(t->eos, serial_str);
+		t->serial_number = smbios_add_string(ctx, serial_str);
 		strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
 	} else {
-		t->serial_number = smbios_add_prop(t->eos, ctx, "serial");
+		t->serial_number = smbios_add_prop(ctx, "serial");
 	}
-	t->sku_number = smbios_add_prop(t->eos, ctx, "sku");
-	t->family = smbios_add_prop(t->eos, ctx, "family");
+	t->sku_number = smbios_add_prop(ctx, "sku");
+	t->family = smbios_add_prop(ctx, "family");
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
@@ -208,9 +217,10 @@ static int smbios_write_type2(ulong *current, int handle,
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type2));
 	fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
-	t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
-	t->product_name = smbios_add_prop(t->eos, ctx, "product");
-	t->asset_tag_number = smbios_add_prop(t->eos, ctx, "asset-tag");
+	set_eos(ctx, t->eos);
+	t->manufacturer = smbios_add_prop(ctx, "manufacturer");
+	t->product_name = smbios_add_prop(ctx, "product");
+	t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
 	t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
 	t->board_type = SMBIOS_BOARD_MOTHERBOARD;
 
@@ -230,7 +240,8 @@ static int smbios_write_type3(ulong *current, int handle,
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type3));
 	fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
-	t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
+	set_eos(ctx, t->eos);
+	t->manufacturer = smbios_add_prop(ctx, "manufacturer");
 	t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP;
 	t->bootup_state = SMBIOS_STATE_SAFE;
 	t->power_supply_state = SMBIOS_STATE_SAFE;
@@ -273,8 +284,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t,
 #endif
 
 	t->processor_family = processor_family;
-	t->processor_manufacturer = smbios_add_string(t->eos, vendor);
-	t->processor_version = smbios_add_string(t->eos, name);
+	t->processor_manufacturer = smbios_add_string(ctx, vendor);
+	t->processor_version = smbios_add_string(ctx, name);
 }
 
 static int smbios_write_type4(ulong *current, int handle,
@@ -286,6 +297,7 @@ static int smbios_write_type4(ulong *current, int handle,
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type4));
 	fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
+	set_eos(ctx, t->eos);
 	t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
 	smbios_write_type4_dm(t, ctx);
 	t->status = SMBIOS_PROCESSOR_STATUS_ENABLED;
@@ -311,6 +323,7 @@ static int smbios_write_type32(ulong *current, int handle,
 	t = map_sysmem(*current, len);
 	memset(t, 0, sizeof(struct smbios_type32));
 	fill_smbios_header(t, SMBIOS_SYSTEM_BOOT_INFORMATION, len, handle);
+	set_eos(ctx, t->eos);
 
 	*current += len;
 	unmap_sysmem(t);
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 08/12] smbios: Track the end of the string table

[Simon Glass]

Add a new member to the context struct which tracks the end of the string
table. This allows us to avoid recalculating this at the end.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 lib/smbios.c | 32 +++++++++++++++-----------------
 1 file changed, 15 insertions(+), 17 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index 1d9bde0c3c2..7090460bc02 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -24,11 +24,15 @@
  * @dev:	sysinfo device to use (NULL if none)
  * @eos:	end-of-string pointer for the table being processed. This is set
  *		up when we start processing a table
+ * @next_ptr:	pointer to the start of the next string to be added. When the
+ *		table is nopt empty, this points to the byte after the \0 of the
+ *		previous string.
  */
 struct smbios_ctx {
 	ofnode node;
 	struct udevice *dev;
 	char *eos;
+	char *next_ptr;
 };
 
 /**
@@ -77,6 +81,7 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
 			strcpy(p, str);
 			p += strlen(str);
 			*p++ = '\0';
+			ctx->next_ptr = p;
 			*p++ = '\0';
 
 			return i;
@@ -114,6 +119,7 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
 static void set_eos(struct smbios_ctx *ctx, char *eos)
 {
 	ctx->eos = eos;
+	ctx->next_ptr = eos;
 }
 
 /**
@@ -121,21 +127,13 @@ static void set_eos(struct smbios_ctx *ctx, char *eos)
  *
  * This computes the size of the string area including the string terminator.
  *
- * @start:	string area start address
+ * @ctx:	SMBIOS context
  * @return:	string area size
  */
-static int smbios_string_table_len(char *start)
+static int smbios_string_table_len(const struct smbios_ctx *ctx)
 {
-	char *p = start;
-	int i, len = 0;
-
-	while (*p) {
-		i = strlen(p) + 1;
-		p += i;
-		len += i;
-	}
-
-	return len + 1;
+	/* Allow for the final \0 after all strings */
+	return (ctx->next_ptr + 1) - ctx->eos;
 }
 
 static int smbios_write_type0(ulong *current, int handle,
@@ -171,7 +169,7 @@ static int smbios_write_type0(ulong *current, int handle,
 	t->ec_major_release = 0xff;
 	t->ec_minor_release = 0xff;
 
-	len = t->length + smbios_string_table_len(t->eos);
+	len = t->length + smbios_string_table_len(ctx);
 	*current += len;
 	unmap_sysmem(t);
 
@@ -201,7 +199,7 @@ static int smbios_write_type1(ulong *current, int handle,
 	t->sku_number = smbios_add_prop(ctx, "sku");
 	t->family = smbios_add_prop(ctx, "family");
 
-	len = t->length + smbios_string_table_len(t->eos);
+	len = t->length + smbios_string_table_len(ctx);
 	*current += len;
 	unmap_sysmem(t);
 
@@ -224,7 +222,7 @@ static int smbios_write_type2(ulong *current, int handle,
 	t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
 	t->board_type = SMBIOS_BOARD_MOTHERBOARD;
 
-	len = t->length + smbios_string_table_len(t->eos);
+	len = t->length + smbios_string_table_len(ctx);
 	*current += len;
 	unmap_sysmem(t);
 
@@ -248,7 +246,7 @@ static int smbios_write_type3(ulong *current, int handle,
 	t->thermal_state = SMBIOS_STATE_SAFE;
 	t->security_status = SMBIOS_SECURITY_NONE;
 
-	len = t->length + smbios_string_table_len(t->eos);
+	len = t->length + smbios_string_table_len(ctx);
 	*current += len;
 	unmap_sysmem(t);
 
@@ -307,7 +305,7 @@ static int smbios_write_type4(ulong *current, int handle,
 	t->l3_cache_handle = 0xffff;
 	t->processor_family2 = t->processor_family;
 
-	len = t->length + smbios_string_table_len(t->eos);
+	len = t->length + smbios_string_table_len(ctx);
 	*current += len;
 	unmap_sysmem(t);
 
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 09/12] smbios: Add more options for the BIOS version string

[Simon Glass]

At present the version string is obtained from PLAIN_VERSION. Some boards
may want to configure this using the device tree, since the build system
can more easily insert things there after U-Boot itself is built. Add this
option to the code.

Also in some cases the version needs to be generated programmatically,
such as when it is stored elsewhere in the ROM and must be read first.
To handle this, keep a pointer around so that it can be updated later.
This works by storing the last string in the context, since it is easier
than passing out a little-used extra parameter.

Provide a function to update the version string.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Correct documentation format

 include/asm-generic/global_data.h |  6 ++++
 include/smbios.h                  | 12 +++++++
 lib/smbios.c                      | 56 +++++++++++++++++++++++++++++--
 3 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
index 19f70393b45..750e998d885 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -439,6 +439,12 @@ struct global_data {
 	 */
 	struct acpi_ctx *acpi_ctx;
 #endif
+#if CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE)
+	/**
+	 * @smbios_version: Points to SMBIOS type 0 version
+	 */
+	char *smbios_version;
+#endif
 };
 
 /**
diff --git a/include/smbios.h b/include/smbios.h
index 1cbeabf9522..ecc4fd1de3b 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -257,4 +257,16 @@ const struct smbios_header *smbios_header(const struct smbios_entry *entry, int
  */
 const char *smbios_string(const struct smbios_header *header, int index);
 
+/**
+ * smbios_update_version() - Update the version string
+ *
+ * This can be called after the SMBIOS tables are written (e.g. after the U-Boot
+ * main loop has started) to update the BIOS version string (SMBIOS table 0).
+ *
+ * @version: New version string to use
+ * @return 0 if OK, -ENOENT if no version string was previously written,
+ *	-ENOSPC if the new string is too large to fit
+ */
+int smbios_update_version(const char *version);
+
 #endif /* _SMBIOS_H_ */
diff --git a/lib/smbios.c b/lib/smbios.c
index 7090460bc02..d46569b09f4 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -17,6 +17,10 @@
 #include <dm/uclass-internal.h>
 #endif
 
+enum {
+	SMBIOS_STR_MAX	= 64,	/* Maximum length allowed for a string */
+};
+
 /**
  * struct smbios_ctx - context for writing SMBIOS tables
  *
@@ -27,12 +31,15 @@
  * @next_ptr:	pointer to the start of the next string to be added. When the
  *		table is nopt empty, this points to the byte after the \0 of the
  *		previous string.
+ * @last_str:	points to the last string that was written to the table, or NULL
+ *		if none
  */
 struct smbios_ctx {
 	ofnode node;
 	struct udevice *dev;
 	char *eos;
 	char *next_ptr;
+	char *last_str;
 };
 
 /**
@@ -78,6 +85,7 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
 
 	for (;;) {
 		if (!*p) {
+			ctx->last_str = p;
 			strcpy(p, str);
 			p += strlen(str);
 			*p++ = '\0';
@@ -87,8 +95,10 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
 			return i;
 		}
 
-		if (!strcmp(p, str))
+		if (!strcmp(p, str)) {
+			ctx->last_str = p;
 			return i;
+		}
 
 		p += strlen(p) + 1;
 		i++;
@@ -120,6 +130,35 @@ static void set_eos(struct smbios_ctx *ctx, char *eos)
 {
 	ctx->eos = eos;
 	ctx->next_ptr = eos;
+	ctx->last_str = NULL;
+}
+
+int smbios_update_version(const char *version)
+{
+	char *ptr = gd->smbios_version;
+	uint old_len, len;
+
+	if (!ptr)
+		return log_ret(-ENOENT);
+
+	/*
+	 * This string is supposed to have at least enough bytes and is
+	 * padded with spaces. Update it, taking care not to move the
+	 * \0 terminator, so that other strings in the string table
+	 * are not disturbed. See smbios_add_string()
+	 */
+	old_len = strnlen(ptr, SMBIOS_STR_MAX);
+	len = strnlen(version, SMBIOS_STR_MAX);
+	if (len > old_len)
+		return log_ret(-ENOSPC);
+
+	log_debug("Replacing SMBIOS type 0 version string '%s'\n", ptr);
+	memcpy(ptr, version, len);
+#ifdef LOG_DEBUG
+	print_buffer((ulong)ptr, ptr, 1, old_len + 1, 0);
+#endif
+
+	return 0;
 }
 
 /**
@@ -147,7 +186,18 @@ static int smbios_write_type0(ulong *current, int handle,
 	fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
 	set_eos(ctx, t->eos);
 	t->vendor = smbios_add_string(ctx, "U-Boot");
-	t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
+
+	t->bios_ver = smbios_add_prop(ctx, "version");
+	if (!t->bios_ver)
+		t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
+	if (t->bios_ver)
+		gd->smbios_version = ctx->last_str;
+	log_debug("smbios_version = %p: '%s'\n", gd->smbios_version,
+		  gd->smbios_version);
+#ifdef LOG_DEBUG
+	print_buffer((ulong)gd->smbios_version, gd->smbios_version,
+		     1, strlen(gd->smbios_version) + 1, 0);
+#endif
 	t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE);
 #ifdef CONFIG_ROM_SIZE
 	t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
@@ -346,7 +396,7 @@ static int smbios_write_type127(ulong *current, int handle,
 }
 
 static struct smbios_write_method smbios_write_funcs[] = {
-	{ smbios_write_type0, },
+	{ smbios_write_type0, "bios", },
 	{ smbios_write_type1, "system", },
 	{ smbios_write_type2, "baseboard", },
 	{ smbios_write_type3, "chassis", },
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 10/12] sysinfo: Move #ifdef so that operations are always defined

[Simon Glass]

At present the struct is not available unless SYSINFO is enabled. This is
annoying since code it is not possible to use compile-time checks like
CONFIG_IS_ENABLED(SYSINFO) with this header.

Fix it by moving the #ifdef.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to fix sysinfo with CONFIG_IS_ENABLED()

 include/sysinfo.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/sysinfo.h b/include/sysinfo.h
index c045d316b07..6e021253524 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -31,7 +31,6 @@
  * to read the serial number.
  */
 
-#if CONFIG_IS_ENABLED(SYSINFO)
 struct sysinfo_ops {
 	/**
 	 * detect() - Run the hardware info detection procedure for this
@@ -102,6 +101,7 @@ struct sysinfo_ops {
 
 #define sysinfo_get_ops(dev)	((struct sysinfo_ops *)(dev)->driver->ops)
 
+#if CONFIG_IS_ENABLED(SYSINFO)
 /**
  * sysinfo_detect() - Run the hardware info detection procedure for this device.
  *
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 11/12] x86: coral: Add sysinfo ops

[Simon Glass]

These ops are missing at present which is not permitted. Add an empty
operation struct.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to fix crash on coral

 board/google/chromebook_coral/coral.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c
index 34b2c2ac5d5..f9fb3f163f0 100644
--- a/board/google/chromebook_coral/coral.c
+++ b/board/google/chromebook_coral/coral.c
@@ -8,6 +8,7 @@
 #include <command.h>
 #include <dm.h>
 #include <log.h>
+#include <sysinfo.h>
 #include <acpi/acpigen.h>
 #include <asm-generic/gpio.h>
 #include <asm/acpi_nhlt.h>
@@ -143,6 +144,9 @@ struct acpi_ops coral_acpi_ops = {
 	.inject_dsdt	= chromeos_acpi_gpio_generate,
 };
 
+struct sysinfo_ops coral_sysinfo_ops = {
+};
+
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
 static const struct udevice_id coral_ids[] = {
 	{ .compatible = "google,coral" },
@@ -154,5 +158,6 @@ U_BOOT_DRIVER(coral_drv) = {
 	.name		= "coral",
 	.id		= UCLASS_SYSINFO,
 	.of_match	= of_match_ptr(coral_ids),
+	.ops		= &coral_sysinfo_ops,
 	ACPI_OPS_PTR(&coral_acpi_ops)
 };
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 12/12] smbios: Allow a few values to come from sysinfo

[Simon Glass]

While static configuration is useful it cannot cover every case. Sometimes
board revisions are encoded in resistor straps and must be read at
runtime.

The easiest way to provide this information is via sysinfo, since the
board can then provide a driver to read whatever is needed.

Add some standard sysinfo options for this, and use them to obtain the
required information.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 include/sysinfo.h | 11 +++++++++++
 lib/smbios.c      | 32 +++++++++++++++++++++++++++++---
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/include/sysinfo.h b/include/sysinfo.h
index 6e021253524..743f3554659 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -31,6 +31,17 @@
  * to read the serial number.
  */
 
+/** enum sysinfo_id - Standard IDs defined by U-Boot */
+enum sysinfo_id {
+	SYSINFO_ID_NONE,
+
+	SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
+	SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
+
+	/* First value available for downstream/board used */
+	SYSINFO_ID_USER = 0x1000,
+};
+
 struct sysinfo_ops {
 	/**
 	 * detect() - Run the hardware info detection procedure for this
diff --git a/lib/smbios.c b/lib/smbios.c
index d46569b09f4..9bdde0b953f 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -10,6 +10,7 @@
 #include <env.h>
 #include <mapmem.h>
 #include <smbios.h>
+#include <sysinfo.h>
 #include <tables_csum.h>
 #include <version.h>
 #ifdef CONFIG_CPU
@@ -106,15 +107,26 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
 }
 
 /**
- * smbios_add_prop() - Add a property from the device tree
+ * smbios_add_prop_si() - Add a property from the devicetree or sysinfo
+ *
+ * Sysinfo is used if available, with a fallback to devicetree
  *
  * @start:	string area start address
  * @node:	node containing the information to write (ofnode_null() if none)
  * @prop:	property to write
  * @return 0 if not found, else SMBIOS string number (1 or more)
  */
-static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
+static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
+			      int sysinfo_id)
 {
+	if (sysinfo_id && ctx->dev) {
+		char val[80];
+		int ret;
+
+		ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val);
+		if (!ret)
+			return smbios_add_string(ctx, val);
+	}
 	if (IS_ENABLED(CONFIG_OF_CONTROL)) {
 		const char *str;
 
@@ -126,6 +138,17 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
 	return 0;
 }
 
+/**
+ * smbios_add_prop() - Add a property from the devicetree
+ *
+ * @prop:	property to write
+ * @return 0 if not found, else SMBIOS string number (1 or more)
+ */
+static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
+{
+	return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE);
+}
+
 static void set_eos(struct smbios_ctx *ctx, char *eos)
 {
 	ctx->eos = eos;
@@ -239,7 +262,8 @@ static int smbios_write_type1(ulong *current, int handle,
 	set_eos(ctx, t->eos);
 	t->manufacturer = smbios_add_prop(ctx, "manufacturer");
 	t->product_name = smbios_add_prop(ctx, "product");
-	t->version = smbios_add_prop(ctx, "version");
+	t->version = smbios_add_prop_si(ctx, "version",
+					SYSINFO_ID_SMBIOS_SYSTEM_VERSION);
 	if (serial_str) {
 		t->serial_number = smbios_add_string(ctx, serial_str);
 		strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
@@ -268,6 +292,8 @@ static int smbios_write_type2(ulong *current, int handle,
 	set_eos(ctx, t->eos);
 	t->manufacturer = smbios_add_prop(ctx, "manufacturer");
 	t->product_name = smbios_add_prop(ctx, "product");
+	t->version = smbios_add_prop_si(ctx, "version",
+					SYSINFO_ID_SMBIOS_BASEBOARD_VERSION);
 	t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
 	t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
 	t->board_type = SMBIOS_BOARD_MOTHERBOARD;
-- 
2.30.0.296.g2bfb1c46d8-goog

[PATCH v2 01/12] README: Add doumentation for version information

[Bin Meng]

Hi Simon,

On Thu, Jan 21, 2021 at 10:06 AM Simon Glass <sjg@chromium.org> wrote:
>
> There are quite a few available version options in U-Boot. Add a list of
> the available Makefile variables and #defines, along with examples.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  README | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
>

Good write-up.

However I believe going forward we should write documents using reST.
Please consider moving this version to something like develop/version.rst.

Regards,
Bin

[PATCH v2 02/12] Makefile: Provide numeric versions

[Bin Meng]

On Thu, Jan 21, 2021 at 10:06 AM Simon Glass <sjg@chromium.org> wrote:
>
> For SMBIOS we want to store the numeric version numbers in the tables. It
> does not make sense to parse the strings. Instead, add new #defines with
> the version and patchlevel.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  Makefile | 4 ++++
>  README   | 8 ++++++++
>  2 files changed, 12 insertions(+)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 03/12] smbios: Move smbios_write_type to the C file

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> This type is not used outside the smbios.c file so there is no need for it
> to be in the header file. Move it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>
> (no changes since v1)
>
>  include/smbios.h | 10 ----------
>  lib/smbios.c     | 10 ++++++++++
>  2 files changed, 10 insertions(+), 10 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 04/12] smbios: Use char consistently for the eos member

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present a few of the structs use u8 instead of char. This is a string,
> so char is better. Update them.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
> ---
>
> (no changes since v1)
>
>  include/smbios.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 05/12] smbios: Set BIOS release version

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> We may as well include the U-Boot release information in the type-0 table
> since it is designed for that purpose.
>
> U-Boot uses release versions based on the year and month. The year cannot
> fit in a byte, so drop the century.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Add a comment about dropping the century
>
>  lib/smbios.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 06/12] smbios: Use a struct to keep track of context

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present we pass the ofnode to each function. We also pass the 'eos'
> pointer for adding new strings. We don't track the current end of the
> string table, so have smbios_string_table_len() to find that.
>
> The code can be made more efficient if it keeps information in a
> context struct. This also makes it easier to add more features.
>
> As a first step, switch the ofnode parameter to be a context pointer.
> Update smbios_add_prop() at the same time to avoid changing the same
> lines of code in consecutive patches.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Zero the context's dev pointer if not used
>
>  lib/smbios.c | 87 +++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 55 insertions(+), 32 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 07/12] smbios: Drop the eos parameter

[Bin Meng]

Hi Simon,

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> We can store this in the context and avoid passing it to each function.
> This makes it easier to follow and will also allow keeping track of the
> end of the string table (in future patches).
>
> Add an 'eos' field to the context and create a function to set it up.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  lib/smbios.c | 61 +++++++++++++++++++++++++++++++---------------------
>  1 file changed, 37 insertions(+), 24 deletions(-)
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 4d2cb0f85e2..1d9bde0c3c2 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -22,10 +22,13 @@
>   *
>   * @node:      node containing the information to write (ofnode_null() if none)
>   * @dev:       sysinfo device to use (NULL if none)
> + * @eos:       end-of-string pointer for the table being processed. This is set
> + *             up when we start processing a table
>   */
>  struct smbios_ctx {
>         ofnode node;
>         struct udevice *dev;
> +       char *eos;
>  };
>
>  /**
> @@ -57,14 +60,15 @@ struct smbios_write_method {
>   * This adds a string to the string area which is appended directly after
>   * the formatted portion of an SMBIOS structure.
>   *
> - * @start:     string area start address
> + * @ctx:       SMBIOS context
>   * @str:       string to add
>   * @return:    string number in the string area (1 or more)
>   */
> -static int smbios_add_string(char *start, const char *str)
> +static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
>  {
>         int i = 1;
> -       char *p = start;
> +       char *p = ctx->eos;
> +
>         if (!*str)
>                 str = "Unknown";
>
> @@ -90,25 +94,28 @@ static int smbios_add_string(char *start, const char *str)
>   * smbios_add_prop() - Add a property from the device tree
>   *
>   * @start:     string area start address
> - * @ctx:       context for writing the tables
> + * @node:      node containing the information to write (ofnode_null() if none)

Instead of adding @node, we should remove @start.


>   * @prop:      property to write
>   * @return 0 if not found, else SMBIOS string number (1 or more)
>   */
> -static int smbios_add_prop(char *start, struct smbios_ctx *ctx,
> -                          const char *prop)
> +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
>  {
> -
>         if (IS_ENABLED(CONFIG_OF_CONTROL)) {
>                 const char *str;
>
>                 str = ofnode_read_string(ctx->node, prop);
>                 if (str)
> -                       return smbios_add_string(start, str);
> +                       return smbios_add_string(ctx, str);
>         }
>
>         return 0;
>  }
>
> +static void set_eos(struct smbios_ctx *ctx, char *eos)

nits: call it smbios_ctx_set_eos()?

> +{
> +       ctx->eos = eos;
> +}
> +
>  /**
>   * smbios_string_table_len() - compute the string area size
>   *
> @@ -140,9 +147,10 @@ static int smbios_write_type0(ulong *current, int handle,
>         t = map_sysmem(*current, len);
>         memset(t, 0, sizeof(struct smbios_type0));
>         fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
> -       t->vendor = smbios_add_string(t->eos, "U-Boot");
> -       t->bios_ver = smbios_add_string(t->eos, PLAIN_VERSION);
> -       t->bios_release_date = smbios_add_string(t->eos, U_BOOT_DMI_DATE);
> +       set_eos(ctx, t->eos);
> +       t->vendor = smbios_add_string(ctx, "U-Boot");
> +       t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
> +       t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE);
>  #ifdef CONFIG_ROM_SIZE
>         t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
>  #endif
> @@ -180,17 +188,18 @@ static int smbios_write_type1(ulong *current, int handle,
>         t = map_sysmem(*current, len);
>         memset(t, 0, sizeof(struct smbios_type1));
>         fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
> -       t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
> -       t->product_name = smbios_add_prop(t->eos, ctx, "product");
> -       t->version = smbios_add_prop(t->eos, ctx, "version");
> +       set_eos(ctx, t->eos);
> +       t->manufacturer = smbios_add_prop(ctx, "manufacturer");
> +       t->product_name = smbios_add_prop(ctx, "product");
> +       t->version = smbios_add_prop(ctx, "version");
>         if (serial_str) {
> -               t->serial_number = smbios_add_string(t->eos, serial_str);
> +               t->serial_number = smbios_add_string(ctx, serial_str);
>                 strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
>         } else {
> -               t->serial_number = smbios_add_prop(t->eos, ctx, "serial");
> +               t->serial_number = smbios_add_prop(ctx, "serial");
>         }
> -       t->sku_number = smbios_add_prop(t->eos, ctx, "sku");
> -       t->family = smbios_add_prop(t->eos, ctx, "family");
> +       t->sku_number = smbios_add_prop(ctx, "sku");
> +       t->family = smbios_add_prop(ctx, "family");
>
>         len = t->length + smbios_string_table_len(t->eos);
>         *current += len;
> @@ -208,9 +217,10 @@ static int smbios_write_type2(ulong *current, int handle,
>         t = map_sysmem(*current, len);
>         memset(t, 0, sizeof(struct smbios_type2));
>         fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
> -       t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
> -       t->product_name = smbios_add_prop(t->eos, ctx, "product");
> -       t->asset_tag_number = smbios_add_prop(t->eos, ctx, "asset-tag");
> +       set_eos(ctx, t->eos);
> +       t->manufacturer = smbios_add_prop(ctx, "manufacturer");
> +       t->product_name = smbios_add_prop(ctx, "product");
> +       t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
>         t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
>         t->board_type = SMBIOS_BOARD_MOTHERBOARD;
>
> @@ -230,7 +240,8 @@ static int smbios_write_type3(ulong *current, int handle,
>         t = map_sysmem(*current, len);
>         memset(t, 0, sizeof(struct smbios_type3));
>         fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
> -       t->manufacturer = smbios_add_prop(t->eos, ctx, "manufacturer");
> +       set_eos(ctx, t->eos);
> +       t->manufacturer = smbios_add_prop(ctx, "manufacturer");
>         t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP;
>         t->bootup_state = SMBIOS_STATE_SAFE;
>         t->power_supply_state = SMBIOS_STATE_SAFE;
> @@ -273,8 +284,8 @@ static void smbios_write_type4_dm(struct smbios_type4 *t,
>  #endif
>
>         t->processor_family = processor_family;
> -       t->processor_manufacturer = smbios_add_string(t->eos, vendor);
> -       t->processor_version = smbios_add_string(t->eos, name);
> +       t->processor_manufacturer = smbios_add_string(ctx, vendor);
> +       t->processor_version = smbios_add_string(ctx, name);
>  }
>
>  static int smbios_write_type4(ulong *current, int handle,
> @@ -286,6 +297,7 @@ static int smbios_write_type4(ulong *current, int handle,
>         t = map_sysmem(*current, len);
>         memset(t, 0, sizeof(struct smbios_type4));
>         fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
> +       set_eos(ctx, t->eos);
>         t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
>         smbios_write_type4_dm(t, ctx);
>         t->status = SMBIOS_PROCESSOR_STATUS_ENABLED;
> @@ -311,6 +323,7 @@ static int smbios_write_type32(ulong *current, int handle,
>         t = map_sysmem(*current, len);
>         memset(t, 0, sizeof(struct smbios_type32));
>         fill_smbios_header(t, SMBIOS_SYSTEM_BOOT_INFORMATION, len, handle);
> +       set_eos(ctx, t->eos);
>
>         *current += len;
>         unmap_sysmem(t);
> --

Regards,
Bin

[PATCH v2 08/12] smbios: Track the end of the string table

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> Add a new member to the context struct which tracks the end of the string
> table. This allows us to avoid recalculating this at the end.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  lib/smbios.c | 32 +++++++++++++++-----------------
>  1 file changed, 15 insertions(+), 17 deletions(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 09/12] smbios: Add more options for the BIOS version string

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present the version string is obtained from PLAIN_VERSION. Some boards
> may want to configure this using the device tree, since the build system
> can more easily insert things there after U-Boot itself is built. Add this
> option to the code.
>
> Also in some cases the version needs to be generated programmatically,
> such as when it is stored elsewhere in the ROM and must be read first.
> To handle this, keep a pointer around so that it can be updated later.
> This works by storing the last string in the context, since it is easier
> than passing out a little-used extra parameter.
>
> Provide a function to update the version string.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Correct documentation format
>
>  include/asm-generic/global_data.h |  6 ++++
>  include/smbios.h                  | 12 +++++++
>  lib/smbios.c                      | 56 +++++++++++++++++++++++++++++--
>  3 files changed, 71 insertions(+), 3 deletions(-)
>
> diff --git a/include/asm-generic/global_data.h b/include/asm-generic/global_data.h
> index 19f70393b45..750e998d885 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -439,6 +439,12 @@ struct global_data {
>          */
>         struct acpi_ctx *acpi_ctx;
>  #endif
> +#if CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE)
> +       /**
> +        * @smbios_version: Points to SMBIOS type 0 version
> +        */
> +       char *smbios_version;
> +#endif
>  };
>
>  /**
> diff --git a/include/smbios.h b/include/smbios.h
> index 1cbeabf9522..ecc4fd1de3b 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -257,4 +257,16 @@ const struct smbios_header *smbios_header(const struct smbios_entry *entry, int
>   */
>  const char *smbios_string(const struct smbios_header *header, int index);
>
> +/**
> + * smbios_update_version() - Update the version string
> + *
> + * This can be called after the SMBIOS tables are written (e.g. after the U-Boot
> + * main loop has started) to update the BIOS version string (SMBIOS table 0).
> + *
> + * @version: New version string to use
> + * @return 0 if OK, -ENOENT if no version string was previously written,
> + *     -ENOSPC if the new string is too large to fit
> + */
> +int smbios_update_version(const char *version);
> +
>  #endif /* _SMBIOS_H_ */
> diff --git a/lib/smbios.c b/lib/smbios.c
> index 7090460bc02..d46569b09f4 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -17,6 +17,10 @@
>  #include <dm/uclass-internal.h>
>  #endif
>
> +enum {
> +       SMBIOS_STR_MAX  = 64,   /* Maximum length allowed for a string */
> +};
> +
>  /**
>   * struct smbios_ctx - context for writing SMBIOS tables
>   *
> @@ -27,12 +31,15 @@
>   * @next_ptr:  pointer to the start of the next string to be added. When the
>   *             table is nopt empty, this points to the byte after the \0 of the
>   *             previous string.
> + * @last_str:  points to the last string that was written to the table, or NULL
> + *             if none
>   */
>  struct smbios_ctx {
>         ofnode node;
>         struct udevice *dev;
>         char *eos;
>         char *next_ptr;
> +       char *last_str;
>  };
>
>  /**
> @@ -78,6 +85,7 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
>
>         for (;;) {
>                 if (!*p) {
> +                       ctx->last_str = p;
>                         strcpy(p, str);
>                         p += strlen(str);
>                         *p++ = '\0';
> @@ -87,8 +95,10 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
>                         return i;
>                 }
>
> -               if (!strcmp(p, str))
> +               if (!strcmp(p, str)) {
> +                       ctx->last_str = p;
>                         return i;
> +               }
>
>                 p += strlen(p) + 1;
>                 i++;
> @@ -120,6 +130,35 @@ static void set_eos(struct smbios_ctx *ctx, char *eos)
>  {
>         ctx->eos = eos;
>         ctx->next_ptr = eos;
> +       ctx->last_str = NULL;
> +}
> +
> +int smbios_update_version(const char *version)
> +{
> +       char *ptr = gd->smbios_version;

Missing DECLARE_GLOBAL_DATA_PTR?

> +       uint old_len, len;
> +
> +       if (!ptr)
> +               return log_ret(-ENOENT);
> +
> +       /*
> +        * This string is supposed to have at least enough bytes and is
> +        * padded with spaces. Update it, taking care not to move the
> +        * \0 terminator, so that other strings in the string table
> +        * are not disturbed. See smbios_add_string()
> +        */
> +       old_len = strnlen(ptr, SMBIOS_STR_MAX);
> +       len = strnlen(version, SMBIOS_STR_MAX);
> +       if (len > old_len)
> +               return log_ret(-ENOSPC);
> +
> +       log_debug("Replacing SMBIOS type 0 version string '%s'\n", ptr);
> +       memcpy(ptr, version, len);
> +#ifdef LOG_DEBUG
> +       print_buffer((ulong)ptr, ptr, 1, old_len + 1, 0);
> +#endif
> +
> +       return 0;
>  }
>
>  /**
> @@ -147,7 +186,18 @@ static int smbios_write_type0(ulong *current, int handle,
>         fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
>         set_eos(ctx, t->eos);
>         t->vendor = smbios_add_string(ctx, "U-Boot");
> -       t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
> +
> +       t->bios_ver = smbios_add_prop(ctx, "version");
> +       if (!t->bios_ver)
> +               t->bios_ver = smbios_add_string(ctx, PLAIN_VERSION);
> +       if (t->bios_ver)
> +               gd->smbios_version = ctx->last_str;
> +       log_debug("smbios_version = %p: '%s'\n", gd->smbios_version,
> +                 gd->smbios_version);
> +#ifdef LOG_DEBUG
> +       print_buffer((ulong)gd->smbios_version, gd->smbios_version,
> +                    1, strlen(gd->smbios_version) + 1, 0);
> +#endif
>         t->bios_release_date = smbios_add_string(ctx, U_BOOT_DMI_DATE);
>  #ifdef CONFIG_ROM_SIZE
>         t->bios_rom_size = (CONFIG_ROM_SIZE / 65536) - 1;
> @@ -346,7 +396,7 @@ static int smbios_write_type127(ulong *current, int handle,
>  }
>
>  static struct smbios_write_method smbios_write_funcs[] = {
> -       { smbios_write_type0, },
> +       { smbios_write_type0, "bios", },
>         { smbios_write_type1, "system", },
>         { smbios_write_type2, "baseboard", },
>         { smbios_write_type3, "chassis", },
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 10/12] sysinfo: Move #ifdef so that operations are always defined

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present the struct is not available unless SYSINFO is enabled. This is
> annoying since code it is not possible to use compile-time checks like
> CONFIG_IS_ENABLED(SYSINFO) with this header.
>
> Fix it by moving the #ifdef.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Add new patch to fix sysinfo with CONFIG_IS_ENABLED()
>
>  include/sysinfo.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 11/12] x86: coral: Add sysinfo ops

[Bin Meng]

Hi Simon,

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> These ops are missing at present which is not permitted. Add an empty
> operation struct.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> Changes in v2:
> - Add new patch to fix crash on coral
>
>  board/google/chromebook_coral/coral.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c
> index 34b2c2ac5d5..f9fb3f163f0 100644
> --- a/board/google/chromebook_coral/coral.c
> +++ b/board/google/chromebook_coral/coral.c
> @@ -8,6 +8,7 @@
>  #include <command.h>
>  #include <dm.h>
>  #include <log.h>
> +#include <sysinfo.h>
>  #include <acpi/acpigen.h>
>  #include <asm-generic/gpio.h>
>  #include <asm/acpi_nhlt.h>
> @@ -143,6 +144,9 @@ struct acpi_ops coral_acpi_ops = {
>         .inject_dsdt    = chromeos_acpi_gpio_generate,
>  };
>
> +struct sysinfo_ops coral_sysinfo_ops = {
> +};
> +
>  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
>  static const struct udevice_id coral_ids[] = {
>         { .compatible = "google,coral" },
> @@ -154,5 +158,6 @@ U_BOOT_DRIVER(coral_drv) = {
>         .name           = "coral",
>         .id             = UCLASS_SYSINFO,
>         .of_match       = of_match_ptr(coral_ids),
> +       .ops            = &coral_sysinfo_ops,
>         ACPI_OPS_PTR(&coral_acpi_ops)
>  };

Shouldn't we fix sysinfo-uclass to test op against NULL? That way we
relax the driver a little bit.

Regards,
Bin

[PATCH v2 12/12] smbios: Allow a few values to come from sysinfo

[Bin Meng]

On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
>
> While static configuration is useful it cannot cover every case. Sometimes
> board revisions are encoded in resistor straps and must be read at
> runtime.
>
> The easiest way to provide this information is via sysinfo, since the
> board can then provide a driver to read whatever is needed.
>
> Add some standard sysinfo options for this, and use them to obtain the
> required information.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  include/sysinfo.h | 11 +++++++++++
>  lib/smbios.c      | 32 +++++++++++++++++++++++++++++---
>  2 files changed, 40 insertions(+), 3 deletions(-)
>
> diff --git a/include/sysinfo.h b/include/sysinfo.h
> index 6e021253524..743f3554659 100644
> --- a/include/sysinfo.h
> +++ b/include/sysinfo.h
> @@ -31,6 +31,17 @@
>   * to read the serial number.
>   */
>
> +/** enum sysinfo_id - Standard IDs defined by U-Boot */
> +enum sysinfo_id {
> +       SYSINFO_ID_NONE,
> +
> +       SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
> +       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
> +
> +       /* First value available for downstream/board used */
> +       SYSINFO_ID_USER = 0x1000,
> +};
> +
>  struct sysinfo_ops {
>         /**
>          * detect() - Run the hardware info detection procedure for this
> diff --git a/lib/smbios.c b/lib/smbios.c
> index d46569b09f4..9bdde0b953f 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -10,6 +10,7 @@
>  #include <env.h>
>  #include <mapmem.h>
>  #include <smbios.h>
> +#include <sysinfo.h>
>  #include <tables_csum.h>
>  #include <version.h>
>  #ifdef CONFIG_CPU
> @@ -106,15 +107,26 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
>  }
>
>  /**
> - * smbios_add_prop() - Add a property from the device tree
> + * smbios_add_prop_si() - Add a property from the devicetree or sysinfo
> + *
> + * Sysinfo is used if available, with a fallback to devicetree
>   *
>   * @start:     string area start address
>   * @node:      node containing the information to write (ofnode_null() if none)
>   * @prop:      property to write
>   * @return 0 if not found, else SMBIOS string number (1 or more)
>   */
> -static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> +static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> +                             int sysinfo_id)
>  {
> +       if (sysinfo_id && ctx->dev) {
> +               char val[80];

Is 80 the limitation defined by sysinfo drivers? Can this be a macro?

> +               int ret;
> +
> +               ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val);
> +               if (!ret)
> +                       return smbios_add_string(ctx, val);
> +       }
>         if (IS_ENABLED(CONFIG_OF_CONTROL)) {
>                 const char *str;
>
> @@ -126,6 +138,17 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
>         return 0;
>  }
>
> +/**
> + * smbios_add_prop() - Add a property from the devicetree
> + *
> + * @prop:      property to write
> + * @return 0 if not found, else SMBIOS string number (1 or more)
> + */
> +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> +{
> +       return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE);
> +}
> +
>  static void set_eos(struct smbios_ctx *ctx, char *eos)
>  {
>         ctx->eos = eos;
> @@ -239,7 +262,8 @@ static int smbios_write_type1(ulong *current, int handle,
>         set_eos(ctx, t->eos);
>         t->manufacturer = smbios_add_prop(ctx, "manufacturer");
>         t->product_name = smbios_add_prop(ctx, "product");
> -       t->version = smbios_add_prop(ctx, "version");
> +       t->version = smbios_add_prop_si(ctx, "version",
> +                                       SYSINFO_ID_SMBIOS_SYSTEM_VERSION);
>         if (serial_str) {
>                 t->serial_number = smbios_add_string(ctx, serial_str);
>                 strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
> @@ -268,6 +292,8 @@ static int smbios_write_type2(ulong *current, int handle,
>         set_eos(ctx, t->eos);
>         t->manufacturer = smbios_add_prop(ctx, "manufacturer");
>         t->product_name = smbios_add_prop(ctx, "product");
> +       t->version = smbios_add_prop_si(ctx, "version",
> +                                       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION);
>         t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
>         t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
>         t->board_type = SMBIOS_BOARD_MOTHERBOARD;
> --

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH v2 12/12] smbios: Allow a few values to come from sysinfo

[Simon Glass]

Hi Bin,

On Wed, 20 Jan 2021 at 23:55, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > While static configuration is useful it cannot cover every case. Sometimes
> > board revisions are encoded in resistor straps and must be read at
> > runtime.
> >
> > The easiest way to provide this information is via sysinfo, since the
> > board can then provide a driver to read whatever is needed.
> >
> > Add some standard sysinfo options for this, and use them to obtain the
> > required information.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  include/sysinfo.h | 11 +++++++++++
> >  lib/smbios.c      | 32 +++++++++++++++++++++++++++++---
> >  2 files changed, 40 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/sysinfo.h b/include/sysinfo.h
> > index 6e021253524..743f3554659 100644
> > --- a/include/sysinfo.h
> > +++ b/include/sysinfo.h
> > @@ -31,6 +31,17 @@
> >   * to read the serial number.
> >   */
> >
> > +/** enum sysinfo_id - Standard IDs defined by U-Boot */
> > +enum sysinfo_id {
> > +       SYSINFO_ID_NONE,
> > +
> > +       SYSINFO_ID_SMBIOS_SYSTEM_VERSION,
> > +       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION,
> > +
> > +       /* First value available for downstream/board used */
> > +       SYSINFO_ID_USER = 0x1000,
> > +};
> > +
> >  struct sysinfo_ops {
> >         /**
> >          * detect() - Run the hardware info detection procedure for this
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index d46569b09f4..9bdde0b953f 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -10,6 +10,7 @@
> >  #include <env.h>
> >  #include <mapmem.h>
> >  #include <smbios.h>
> > +#include <sysinfo.h>
> >  #include <tables_csum.h>
> >  #include <version.h>
> >  #ifdef CONFIG_CPU
> > @@ -106,15 +107,26 @@ static int smbios_add_string(struct smbios_ctx *ctx, const char *str)
> >  }
> >
> >  /**
> > - * smbios_add_prop() - Add a property from the device tree
> > + * smbios_add_prop_si() - Add a property from the devicetree or sysinfo
> > + *
> > + * Sysinfo is used if available, with a fallback to devicetree
> >   *
> >   * @start:     string area start address
> >   * @node:      node containing the information to write (ofnode_null() if none)
> >   * @prop:      property to write
> >   * @return 0 if not found, else SMBIOS string number (1 or more)
> >   */
> > -static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> > +static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> > +                             int sysinfo_id)
> >  {
> > +       if (sysinfo_id && ctx->dev) {
> > +               char val[80];
>
> Is 80 the limitation defined by sysinfo drivers? Can this be a macro?

I found this limitation in the SMBIOS spec.

Yes I will make it a macro.

>
> > +               int ret;
> > +
> > +               ret = sysinfo_get_str(ctx->dev, sysinfo_id, sizeof(val), val);
> > +               if (!ret)
> > +                       return smbios_add_string(ctx, val);
> > +       }
> >         if (IS_ENABLED(CONFIG_OF_CONTROL)) {
> >                 const char *str;
> >
> > @@ -126,6 +138,17 @@ static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> >         return 0;
> >  }
> >
> > +/**
> > + * smbios_add_prop() - Add a property from the devicetree
> > + *
> > + * @prop:      property to write
> > + * @return 0 if not found, else SMBIOS string number (1 or more)
> > + */
> > +static int smbios_add_prop(struct smbios_ctx *ctx, const char *prop)
> > +{
> > +       return smbios_add_prop_si(ctx, prop, SYSINFO_ID_NONE);
> > +}
> > +
> >  static void set_eos(struct smbios_ctx *ctx, char *eos)
> >  {
> >         ctx->eos = eos;
> > @@ -239,7 +262,8 @@ static int smbios_write_type1(ulong *current, int handle,
> >         set_eos(ctx, t->eos);
> >         t->manufacturer = smbios_add_prop(ctx, "manufacturer");
> >         t->product_name = smbios_add_prop(ctx, "product");
> > -       t->version = smbios_add_prop(ctx, "version");
> > +       t->version = smbios_add_prop_si(ctx, "version",
> > +                                       SYSINFO_ID_SMBIOS_SYSTEM_VERSION);
> >         if (serial_str) {
> >                 t->serial_number = smbios_add_string(ctx, serial_str);
> >                 strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
> > @@ -268,6 +292,8 @@ static int smbios_write_type2(ulong *current, int handle,
> >         set_eos(ctx, t->eos);
> >         t->manufacturer = smbios_add_prop(ctx, "manufacturer");
> >         t->product_name = smbios_add_prop(ctx, "product");
> > +       t->version = smbios_add_prop_si(ctx, "version",
> > +                                       SYSINFO_ID_SMBIOS_BASEBOARD_VERSION);
> >         t->asset_tag_number = smbios_add_prop(ctx, "asset-tag");
> >         t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
> >         t->board_type = SMBIOS_BOARD_MOTHERBOARD;
> > --
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

Regards,
Simon

[PATCH v2 11/12] x86: coral: Add sysinfo ops

[Simon Glass]

Hi Bin,

On Wed, 20 Jan 2021 at 23:54, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Thu, Jan 21, 2021 at 10:07 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > These ops are missing at present which is not permitted. Add an empty
> > operation struct.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > Changes in v2:
> > - Add new patch to fix crash on coral
> >
> >  board/google/chromebook_coral/coral.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/board/google/chromebook_coral/coral.c b/board/google/chromebook_coral/coral.c
> > index 34b2c2ac5d5..f9fb3f163f0 100644
> > --- a/board/google/chromebook_coral/coral.c
> > +++ b/board/google/chromebook_coral/coral.c
> > @@ -8,6 +8,7 @@
> >  #include <command.h>
> >  #include <dm.h>
> >  #include <log.h>
> > +#include <sysinfo.h>
> >  #include <acpi/acpigen.h>
> >  #include <asm-generic/gpio.h>
> >  #include <asm/acpi_nhlt.h>
> > @@ -143,6 +144,9 @@ struct acpi_ops coral_acpi_ops = {
> >         .inject_dsdt    = chromeos_acpi_gpio_generate,
> >  };
> >
> > +struct sysinfo_ops coral_sysinfo_ops = {
> > +};
> > +
> >  #if !CONFIG_IS_ENABLED(OF_PLATDATA)
> >  static const struct udevice_id coral_ids[] = {
> >         { .compatible = "google,coral" },
> > @@ -154,5 +158,6 @@ U_BOOT_DRIVER(coral_drv) = {
> >         .name           = "coral",
> >         .id             = UCLASS_SYSINFO,
> >         .of_match       = of_match_ptr(coral_ids),
> > +       .ops            = &coral_sysinfo_ops,
> >         ACPI_OPS_PTR(&coral_acpi_ops)
> >  };
>
> Shouldn't we fix sysinfo-uclass to test op against NULL? That way we
> relax the driver a little bit.

In general I don't like missing out the operations struct. If the
uclass requires operations then the drivers should provide them. It is
true that the operation struct is currently empty, but I've sent
patches to add to it.

Checking for missing operations must be done in every uclass
operation, so it adds to code size.

Regards,
Simon