[PATCH v2 0/4] x86: Updates to SMBIOS

[PATCH v2 0/4] x86: Updates to SMBIOS

[Simon Glass]

At present there are a few Kconfig options which allow SMBIOS fields to
be specified at build time.

Not all fields are supported. Also, defining these at build-time is
limiting since a factory system cannot insert values for particular boards
or models without rebuilding U-Boot.

This series adds a way to set SMBIOS properties using the devicetree.
With this approach, more fields are supported and it is easy to update
values in the devicetree in the factory.

Changes in v2:
- Move dm.h header file to avoid build error on qemu-arm
- Deal with boards that don't use of-control

Simon Glass (4):
  x86: Pass an ofnode into each SMBIOS function
  smbios: Allow properties to come from the device tree
  smbios: Add more properties
  smbios: Add documentation and devicetree binding

 doc/arch/x86.rst                             |   8 ++
 doc/device-tree-bindings/board/board_x86.txt |  36 +++++++
 include/smbios.h                             |   5 +-
 lib/smbios.c                                 | 104 ++++++++++++++-----
 4 files changed, 128 insertions(+), 25 deletions(-)
 create mode 100644 doc/device-tree-bindings/board/board_x86.txt

-- 
2.28.0.806.g8561365e88-goog

[PATCH v2 1/4] x86: Pass an ofnode into each SMBIOS function

[Simon Glass]

As a first step to obtaining SMBIOS information from the devicetree, add
an ofnode parameter to the writing functions.

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

Changes in v2:
- Move dm.h header file to avoid build error on qemu-arm
- Deal with boards that don't use of-control

 include/smbios.h |  5 ++++-
 lib/smbios.c     | 44 ++++++++++++++++++++++++++------------------
 2 files changed, 30 insertions(+), 19 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index 97b9ddce237..c19afbe16c7 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -8,6 +8,8 @@
 #ifndef _SMBIOS_H_
 #define _SMBIOS_H_
 
+#include <dm/ofnode.h>
+
 /* SMBIOS spec version implemented */
 #define SMBIOS_MAJOR_VER	3
 #define SMBIOS_MINOR_VER	0
@@ -222,9 +224,10 @@ static inline void fill_smbios_header(void *table, int type,
  *
  * @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);
+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 11790443e1a..4bb3bac56b4 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -6,6 +6,7 @@
  */
 
 #include <common.h>
+#include <dm.h>
 #include <env.h>
 #include <mapmem.h>
 #include <smbios.h>
@@ -13,7 +14,6 @@
 #include <version.h>
 #ifdef CONFIG_CPU
 #include <cpu.h>
-#include <dm.h>
 #include <dm/uclass-internal.h>
 #endif
 
@@ -25,7 +25,7 @@
  *
  * @start:	string area start address
  * @str:	string to add
- * @return:	string number in the string area
+ * @return:	string number in the string area (1 or more)
  */
 static int smbios_add_string(char *start, const char *str)
 {
@@ -74,7 +74,7 @@ static int smbios_string_table_len(char *start)
 	return len + 1;
 }
 
-static int smbios_write_type0(ulong *current, int handle)
+static int smbios_write_type0(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type0 *t;
 	int len = sizeof(struct smbios_type0);
@@ -111,7 +111,7 @@ static int smbios_write_type0(ulong *current, int handle)
 	return len;
 }
 
-static int smbios_write_type1(ulong *current, int handle)
+static int smbios_write_type1(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type1 *t;
 	int len = sizeof(struct smbios_type1);
@@ -134,7 +134,7 @@ static int smbios_write_type1(ulong *current, int handle)
 	return len;
 }
 
-static int smbios_write_type2(ulong *current, int handle)
+static int smbios_write_type2(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type2 *t;
 	int len = sizeof(struct smbios_type2);
@@ -154,7 +154,7 @@ static int smbios_write_type2(ulong *current, int handle)
 	return len;
 }
 
-static int smbios_write_type3(ulong *current, int handle)
+static int smbios_write_type3(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type3 *t;
 	int len = sizeof(struct smbios_type3);
@@ -176,7 +176,7 @@ static int smbios_write_type3(ulong *current, int handle)
 	return len;
 }
 
-static void smbios_write_type4_dm(struct smbios_type4 *t)
+static void smbios_write_type4_dm(struct smbios_type4 *t, ofnode node)
 {
 	u16 processor_family = SMBIOS_PROCESSOR_FAMILY_UNKNOWN;
 	const char *vendor = "Unknown";
@@ -185,20 +185,20 @@ static void smbios_write_type4_dm(struct smbios_type4 *t)
 #ifdef CONFIG_CPU
 	char processor_name[49];
 	char vendor_name[49];
-	struct udevice *dev = NULL;
+	struct udevice *cpu = NULL;
 
-	uclass_find_first_device(UCLASS_CPU, &dev);
-	if (dev) {
-		struct cpu_platdata *plat = dev_get_parent_platdata(dev);
+	uclass_find_first_device(UCLASS_CPU, &cpu);
+	if (cpu) {
+		struct cpu_platdata *plat = dev_get_parent_platdata(cpu);
 
 		if (plat->family)
 			processor_family = plat->family;
 		t->processor_id[0] = plat->id[0];
 		t->processor_id[1] = plat->id[1];
 
-		if (!cpu_get_vendor(dev, vendor_name, sizeof(vendor_name)))
+		if (!cpu_get_vendor(cpu, vendor_name, sizeof(vendor_name)))
 			vendor = vendor_name;
-		if (!cpu_get_desc(dev, processor_name, sizeof(processor_name)))
+		if (!cpu_get_desc(cpu, processor_name, sizeof(processor_name)))
 			name = processor_name;
 	}
 #endif
@@ -208,7 +208,7 @@ static void smbios_write_type4_dm(struct smbios_type4 *t)
 	t->processor_version = smbios_add_string(t->eos, name);
 }
 
-static int smbios_write_type4(ulong *current, int handle)
+static int smbios_write_type4(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type4 *t;
 	int len = sizeof(struct smbios_type4);
@@ -217,7 +217,7 @@ static int smbios_write_type4(ulong *current, int handle)
 	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);
+	smbios_write_type4_dm(t, node);
 	t->status = SMBIOS_PROCESSOR_STATUS_ENABLED;
 	t->processor_upgrade = SMBIOS_PROCESSOR_UPGRADE_NONE;
 	t->l1_cache_handle = 0xffff;
@@ -232,7 +232,7 @@ static int smbios_write_type4(ulong *current, int handle)
 	return len;
 }
 
-static int smbios_write_type32(ulong *current, int handle)
+static int smbios_write_type32(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type32 *t;
 	int len = sizeof(struct smbios_type32);
@@ -247,7 +247,7 @@ static int smbios_write_type32(ulong *current, int handle)
 	return len;
 }
 
-static int smbios_write_type127(ulong *current, int handle)
+static int smbios_write_type127(ulong *current, int handle, ofnode node)
 {
 	struct smbios_type127 *t;
 	int len = sizeof(struct smbios_type127);
@@ -274,7 +274,9 @@ static smbios_write_type smbios_write_funcs[] = {
 
 ulong write_smbios_table(ulong addr)
 {
+	ofnode node = ofnode_null();
 	struct smbios_entry *se;
+	struct udevice *dev;
 	ulong table_addr;
 	ulong tables;
 	int len = 0;
@@ -284,6 +286,12 @@ ulong write_smbios_table(ulong addr)
 	int isize;
 	int i;
 
+	if (IS_ENABLED(CONFIG_OF_CONTROL)) {
+		uclass_first_device(UCLASS_BOARD, &dev);
+		if (dev)
+			node = dev_read_subnode(dev, "smbios");
+	}
+
 	/* 16 byte align the table address */
 	addr = ALIGN(addr, 16);
 
@@ -296,7 +304,7 @@ ulong write_smbios_table(ulong addr)
 
 	/* populate minimum required tables */
 	for (i = 0; i < ARRAY_SIZE(smbios_write_funcs); i++) {
-		int tmp = smbios_write_funcs[i]((ulong *)&addr, handle++);
+		int tmp = smbios_write_funcs[i]((ulong *)&addr, handle++, node);
 
 		max_struct_size = max(max_struct_size, tmp);
 		len += tmp;
-- 
2.28.0.806.g8561365e88-goog

[PATCH v2 2/4] smbios: Allow properties to come from the device tree

[Simon Glass]

Support a way to put SMBIOS properties in the device tree. These can be
placed in a 'board' device in an 'smbios' subnode.

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

(no changes since v1)

 lib/smbios.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 50 insertions(+), 6 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index 4bb3bac56b4..28a2ddf7a69 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -52,6 +52,43 @@ static int smbios_add_string(char *start, const char *str)
 	}
 }
 
+/**
+ * smbios_add_prop_default() - Add a property from the device tree or default
+ *
+ * @start:	string area start address
+ * @node:	node containing the information to write (ofnode_null() if none)
+ * @prop:	property to write
+ * @def:	default string if the node has no such property
+ * @return 0 if not found, else SMBIOS string number (1 or more)
+ */
+static int smbios_add_prop_default(char *start, ofnode node, const char *prop,
+				   const char *def)
+{
+	const char *str = NULL;
+
+	if (IS_ENABLED(CONFIG_OF_CONTROL))
+		str = ofnode_read_string(node, prop);
+	if (str)
+		return smbios_add_string(start, str);
+	else if (def)
+		return smbios_add_string(start, def);
+
+	return 0;
+}
+
+/**
+ * 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)
+ * @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)
+{
+	return smbios_add_prop_default(start, node, prop, NULL);
+}
+
 /**
  * smbios_string_table_len() - compute the string area size
  *
@@ -120,11 +157,15 @@ 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_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
-	t->product_name = smbios_add_string(t->eos, CONFIG_SMBIOS_PRODUCT_NAME);
+	t->manufacturer = smbios_add_prop_default(t->eos, node, "manufactuer",
+						  CONFIG_SMBIOS_MANUFACTURER);
+	t->product_name = smbios_add_prop_default(t->eos, node, "product",
+						  CONFIG_SMBIOS_PRODUCT_NAME);
 	if (serial_str) {
-		strncpy((char *)t->uuid, serial_str, sizeof(t->uuid));
 		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");
 	}
 
 	len = t->length + smbios_string_table_len(t->eos);
@@ -142,8 +183,10 @@ 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_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
-	t->product_name = smbios_add_string(t->eos, CONFIG_SMBIOS_PRODUCT_NAME);
+	t->manufacturer = smbios_add_prop_default(t->eos, node, "manufactuer",
+						  CONFIG_SMBIOS_MANUFACTURER);
+	t->product_name = smbios_add_prop_default(t->eos, node, "product",
+						  CONFIG_SMBIOS_PRODUCT_NAME);
 	t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
 	t->board_type = SMBIOS_BOARD_MOTHERBOARD;
 
@@ -162,7 +205,8 @@ 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_string(t->eos, CONFIG_SMBIOS_MANUFACTURER);
+	t->manufacturer = smbios_add_prop_default(t->eos, node, "manufactuer",
+						  CONFIG_SMBIOS_MANUFACTURER);
 	t->chassis_type = SMBIOS_ENCLOSURE_DESKTOP;
 	t->bootup_state = SMBIOS_STATE_SAFE;
 	t->power_supply_state = SMBIOS_STATE_SAFE;
-- 
2.28.0.806.g8561365e88-goog

[PATCH v2 3/4] smbios: Add more properties

[Simon Glass]

The current tables only support a subset of the available fields defined
by the SMBIOS spec. Add a few more.

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

(no changes since v1)

 lib/smbios.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/smbios.c b/lib/smbios.c
index 28a2ddf7a69..2b0d6ea8528 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -161,12 +161,15 @@ static int smbios_write_type1(ulong *current, int handle, ofnode node)
 						  CONFIG_SMBIOS_MANUFACTURER);
 	t->product_name = smbios_add_prop_default(t->eos, node, "product",
 						  CONFIG_SMBIOS_PRODUCT_NAME);
+	t->version = smbios_add_prop(t->eos, node, "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->sku_number = smbios_add_prop(t->eos, node, "sku");
+	t->family = smbios_add_prop(t->eos, node, "family");
 
 	len = t->length + smbios_string_table_len(t->eos);
 	*current += len;
@@ -187,6 +190,7 @@ static int smbios_write_type2(ulong *current, int handle, ofnode node)
 						  CONFIG_SMBIOS_MANUFACTURER);
 	t->product_name = smbios_add_prop_default(t->eos, node, "product",
 						  CONFIG_SMBIOS_PRODUCT_NAME);
+	t->asset_tag_number = smbios_add_prop(t->eos, node, "asset-tag");
 	t->feature_flags = SMBIOS_BOARD_FEATURE_HOSTING;
 	t->board_type = SMBIOS_BOARD_MOTHERBOARD;
 
-- 
2.28.0.806.g8561365e88-goog

[PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Simon Glass]

Add information about how to set SMBIOS properties using the devicetree.

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

(no changes since v1)

 doc/arch/x86.rst                             |  8 +++++
 doc/device-tree-bindings/board/board_x86.txt | 36 ++++++++++++++++++++
 2 files changed, 44 insertions(+)
 create mode 100644 doc/device-tree-bindings/board/board_x86.txt

diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
index c6b70ce61a3..45f9ba308c7 100644
--- a/doc/arch/x86.rst
+++ b/doc/arch/x86.rst
@@ -740,6 +740,14 @@ Note that this is a development feature only. It is not intended for use in
 production environments. Also it is not currently part of the automated tests
 so may break in the future.
 
+SMBIOS tables
+-------------
+
+To generate SMBIOS tables in U-Boot, for use by the OS, enable the
+CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values to
+use is via the device tree. For details see
+device-tree-bindings/board/board_x86.txt
+
 TODO List
 ---------
 - Audio
diff --git a/doc/device-tree-bindings/board/board_x86.txt b/doc/device-tree-bindings/board/board_x86.txt
new file mode 100644
index 00000000000..3766751896a
--- /dev/null
+++ b/doc/device-tree-bindings/board/board_x86.txt
@@ -0,0 +1,36 @@
+x86 board driver
+================
+
+This driver allows providing board-specific features such as power control
+GPIOs. In addition, the SMBIOS values can be specified in the device tree,
+as below:
+
+An optional 'smbios' subnode can be used to provide these properties.
+
+Optional properties:
+  - manufacturer: Product manufacturer for system / baseboard
+  - product:      Product name
+  - version:      Product version string
+  - serial:       Serial number for system (note that this can be overridden by
+                      the serial# environment variable)
+  - sku:          Product SKU (Stock-Keeping Unit)
+  - family:       Product family
+  - asset-tag:    Asset tag for the motherboard, sometimes used in organisations
+                      to track devices
+
+
+Example:
+
+	board {
+		compatible = "google,coral";
+
+		smbios {
+			manufacturer = "Google";
+			product = "Coral";
+			version = "rev2";
+			serial = "123456789";
+			sku = "sku3";
+			family = "Google_Coral";
+			asset-tag = "ABC123";
+		};
+	};
-- 
2.28.0.806.g8561365e88-goog

[PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Heinrich Schuchardt]

On 02.10.20 16:23, Simon Glass wrote:
> Add information about how to set SMBIOS properties using the devicetree.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> (no changes since v1)
>
>  doc/arch/x86.rst                             |  8 +++++
>  doc/device-tree-bindings/board/board_x86.txt | 36 ++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
>
> diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
> index c6b70ce61a3..45f9ba308c7 100644
> --- a/doc/arch/x86.rst
> +++ b/doc/arch/x86.rst
> @@ -740,6 +740,14 @@ Note that this is a development feature only. It is not intended for use in
>  production environments. Also it is not currently part of the automated tests
>  so may break in the future.
>
> +SMBIOS tables
> +-------------
> +
> +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
> +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values to
> +use is via the device tree. For details see
> +device-tree-bindings/board/board_x86.txt
> +
>  TODO List
>  ---------
>  - Audio
> diff --git a/doc/device-tree-bindings/board/board_x86.txt b/doc/device-tree-bindings/board/board_x86.txt
> new file mode 100644
> index 00000000000..3766751896a
> --- /dev/null
> +++ b/doc/device-tree-bindings/board/board_x86.txt
> @@ -0,0 +1,36 @@
> +x86 board driver
> +================
> +
> +This driver allows providing board-specific features such as power control
> +GPIOs. In addition, the SMBIOS values can be specified in the device tree,
> +as below:
> +
> +An optional 'smbios' subnode can be used to provide these properties.
> +
> +Optional properties:
> +  - manufacturer: Product manufacturer for system / baseboard
> +  - product:      Product name
> +  - version:      Product version string
> +  - serial:       Serial number for system (note that this can be overridden by
> +                      the serial# environment variable)
> +  - sku:          Product SKU (Stock-Keeping Unit)
> +  - family:       Product family
> +  - asset-tag:    Asset tag for the motherboard, sometimes used in organisations
> +                      to track devices
> +
> +
> +Example:
> +
> +	board {
> +		compatible = "google,coral";
> +
> +		smbios {

I think board is the wrong place for the new node.

SMBIOS also includes chassis, processor and other information. We may
opt to provide non-board information in future via the device-tree.

If this is for U-Boot internal only usage, we should indicate this in
the label name.

So it could be like:

/u-boot-smbios {
   bios {};
   system {};
   board {
      manufacturer = "Hardkernel Co., Ltd.";
      product = "ODROID-C2";
      ...
   };
   chassis {};
   processor {};
   boot {};
};

> +			manufacturer = "Google";
> +			product = "Coral";
> +			version = "rev2";
> +			serial = "123456789";
> +			sku = "sku3";
> +			family = "Google_Coral";
> +			asset-tag = "ABC123";
> +		};
> +	};
>

SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.

On my Odroid C2 dmidecode shows the information provided by U-Boot:

....
Handle 0x0001, DMI type 1, 27 bytes
System Information
        Manufacturer: Hardkernel Co., Ltd.
        Product Name: ODROID-C2
        Version: Not Specified
...
        Wake-up Type: Reserved
        SKU Number: Not Specified
        Family: Not Specified
....


So doc/device-tree-bindings/board/board_x86.txt is the wrong place to
document the new device tree node.

Why not create a document doc/device-tree-bindings/smbios.rst for it?

Best regards

Heinrich

[PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Simon Glass]

Hi Heinrich,

On Fri, 2 Oct 2020 at 08:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 02.10.20 16:23, Simon Glass wrote:
> > Add information about how to set SMBIOS properties using the devicetree.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  doc/arch/x86.rst                             |  8 +++++
> >  doc/device-tree-bindings/board/board_x86.txt | 36 ++++++++++++++++++++
> >  2 files changed, 44 insertions(+)
> >  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
> >
> > diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
> > index c6b70ce61a3..45f9ba308c7 100644
> > --- a/doc/arch/x86.rst
> > +++ b/doc/arch/x86.rst
> > @@ -740,6 +740,14 @@ Note that this is a development feature only. It is not intended for use in
> >  production environments. Also it is not currently part of the automated tests
> >  so may break in the future.
> >
> > +SMBIOS tables
> > +-------------
> > +
> > +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
> > +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values to
> > +use is via the device tree. For details see
> > +device-tree-bindings/board/board_x86.txt
> > +
> >  TODO List
> >  ---------
> >  - Audio
> > diff --git a/doc/device-tree-bindings/board/board_x86.txt b/doc/device-tree-bindings/board/board_x86.txt
> > new file mode 100644
> > index 00000000000..3766751896a
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/board/board_x86.txt
> > @@ -0,0 +1,36 @@
> > +x86 board driver
> > +================
> > +
> > +This driver allows providing board-specific features such as power control
> > +GPIOs. In addition, the SMBIOS values can be specified in the device tree,
> > +as below:
> > +
> > +An optional 'smbios' subnode can be used to provide these properties.
> > +
> > +Optional properties:
> > +  - manufacturer: Product manufacturer for system / baseboard
> > +  - product:      Product name
> > +  - version:      Product version string
> > +  - serial:       Serial number for system (note that this can be overridden by
> > +                      the serial# environment variable)
> > +  - sku:          Product SKU (Stock-Keeping Unit)
> > +  - family:       Product family
> > +  - asset-tag:    Asset tag for the motherboard, sometimes used in organisations
> > +                      to track devices
> > +
> > +
> > +Example:
> > +
> > +     board {
> > +             compatible = "google,coral";
> > +
> > +             smbios {
>
> I think board is the wrong place for the new node.
>
> SMBIOS also includes chassis, processor and other information. We may
> opt to provide non-board information in future via the device-tree.

I think of board as the whole thing. Are you thinking of it just being
the circuit board?

>
> If this is for U-Boot internal only usage, we should indicate this in
> the label name.

All drivers are for U-Boot-internal use, right? Or are you saying you
don't want this information to appear in the upstream bindings?

>
> So it could be like:
>
> /u-boot-smbios {

compatible string?

>    bios {};
>    system {};
>    board {
>       manufacturer = "Hardkernel Co., Ltd.";
>       product = "ODROID-C2";
>       ...
>    };
>    chassis {};
>    processor {};
>    boot {};
> };
>
> > +                     manufacturer = "Google";
> > +                     product = "Coral";
> > +                     version = "rev2";
> > +                     serial = "123456789";
> > +                     sku = "sku3";
> > +                     family = "Google_Coral";
> > +                     asset-tag = "ABC123";
> > +             };
> > +     };
> >
>
> SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.
>
> On my Odroid C2 dmidecode shows the information provided by U-Boot:

Oh dear. Legacy never dies. It never occurred to me that this would be
used on ARM.

>
> ....
> Handle 0x0001, DMI type 1, 27 bytes
> System Information
>         Manufacturer: Hardkernel Co., Ltd.
>         Product Name: ODROID-C2
>         Version: Not Specified
> ...
>         Wake-up Type: Reserved
>         SKU Number: Not Specified
>         Family: Not Specified
> ....
>
>
> So doc/device-tree-bindings/board/board_x86.txt is the wrong place to
> document the new device tree node.
>
> Why not create a document doc/device-tree-bindings/smbios.rst for it?

OK I can do that. I'll await your thoughts on the above.

Regards,
Simon

[PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Heinrich Schuchardt]

On 10/3/20 5:40 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 2 Oct 2020 at 08:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> On 02.10.20 16:23, Simon Glass wrote:
>>> Add information about how to set SMBIOS properties using the devicetree.
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>  doc/arch/x86.rst                             |  8 +++++
>>>  doc/device-tree-bindings/board/board_x86.txt | 36 ++++++++++++++++++++
>>>  2 files changed, 44 insertions(+)
>>>  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
>>>
>>> diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
>>> index c6b70ce61a3..45f9ba308c7 100644
>>> --- a/doc/arch/x86.rst
>>> +++ b/doc/arch/x86.rst
>>> @@ -740,6 +740,14 @@ Note that this is a development feature only. It is not intended for use in
>>>  production environments. Also it is not currently part of the automated tests
>>>  so may break in the future.
>>>
>>> +SMBIOS tables
>>> +-------------
>>> +
>>> +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
>>> +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values to
>>> +use is via the device tree. For details see
>>> +device-tree-bindings/board/board_x86.txt
>>> +
>>>  TODO List
>>>  ---------
>>>  - Audio
>>> diff --git a/doc/device-tree-bindings/board/board_x86.txt b/doc/device-tree-bindings/board/board_x86.txt
>>> new file mode 100644
>>> index 00000000000..3766751896a
>>> --- /dev/null
>>> +++ b/doc/device-tree-bindings/board/board_x86.txt
>>> @@ -0,0 +1,36 @@
>>> +x86 board driver
>>> +================
>>> +
>>> +This driver allows providing board-specific features such as power control
>>> +GPIOs. In addition, the SMBIOS values can be specified in the device tree,
>>> +as below:
>>> +
>>> +An optional 'smbios' subnode can be used to provide these properties.
>>> +
>>> +Optional properties:
>>> +  - manufacturer: Product manufacturer for system / baseboard
>>> +  - product:      Product name
>>> +  - version:      Product version string
>>> +  - serial:       Serial number for system (note that this can be overridden by
>>> +                      the serial# environment variable)
>>> +  - sku:          Product SKU (Stock-Keeping Unit)
>>> +  - family:       Product family
>>> +  - asset-tag:    Asset tag for the motherboard, sometimes used in organisations
>>> +                      to track devices
>>> +
>>> +
>>> +Example:
>>> +
>>> +     board {
>>> +             compatible = "google,coral";
>>> +
>>> +             smbios {
>>
>> I think board is the wrong place for the new node.
>>
>> SMBIOS also includes chassis, processor and other information. We may
>> opt to provide non-board information in future via the device-tree.
>
> I think of board as the whole thing. Are you thinking of it just being
> the circuit board?

The whole I would call a system.

dmidecode shows bios, system, board, chassis, processor, boot. So board
seems to be only a small aspect of the SMBIOS system description.

>> If this is for U-Boot internal only usage, we should indicate this in
>> the label name.
>
> All drivers are for U-Boot-internal use, right? Or are you saying you
> don't want this information to appear in the upstream bindings?

I don't won't name collisions of U-Boot with Linux when using U-Boot's
device tree for booting an operating system on ARM and RISC-V systems.

>> So it could be like:
>>
>> /u-boot-smbios {
>
> compatible string?

Yes there should be one. So you know that this is the node you are
looking for.

    compatible = "u-boot,smbios";

>
>>    bios {};
>>    system {};
>>    board {
>>       manufacturer = "Hardkernel Co., Ltd.";
>>       product = "ODROID-C2";
>>       ...
>>    };
>>    chassis {};
>>    processor {};
>>    boot {};
>> };
>>
>>> +                     manufacturer = "Google";
>>> +                     product = "Coral";
>>> +                     version = "rev2";
>>> +                     serial = "123456789";
>>> +                     sku = "sku3";
>>> +                     family = "Google_Coral";
>>> +                     asset-tag = "ABC123";
>>> +             };
>>> +     };
>>>
>>
>> SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.
>>
>> On my Odroid C2 dmidecode shows the information provided by U-Boot:
>
> Oh dear. Legacy never dies. It never occurred to me that this would be
> used on ARM.

Don't forget about RISC-V :)

https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.3.0.pdf:

"The following processor architectures are now supported:

* IA-32 (x86),386
* x64 (x86-64, Intel64, AMD64, EM64T),
* Intel Itanium architecture,
* 32-bit ARM (Aarch32),
* 64-bit ARM (Aarch64),
* RISC-V 32 (RV32),
* RISC-V 64 (RV64),
* RISC-V 128 (RV128)"

If you think of data centers, it may be valuable to be able to use the
same management tools for amd64, aarch64 and rv64.

Best regards

Heinrich

>
>>
>> ....
>> Handle 0x0001, DMI type 1, 27 bytes
>> System Information
>>         Manufacturer: Hardkernel Co., Ltd.
>>         Product Name: ODROID-C2
>>         Version: Not Specified
>> ...
>>         Wake-up Type: Reserved
>>         SKU Number: Not Specified
>>         Family: Not Specified
>> ....
>>
>>
>> So doc/device-tree-bindings/board/board_x86.txt is the wrong place to
>> document the new device tree node.
>>
>> Why not create a document doc/device-tree-bindings/smbios.rst for it?
>
> OK I can do that. I'll await your thoughts on the above.
>
> Regards,
> Simon
>

[PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Simon Glass]

Hi Heinrich,

On Sat, 3 Oct 2020 at 11:54, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/3/20 5:40 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 2 Oct 2020 at 08:57, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> On 02.10.20 16:23, Simon Glass wrote:
> >>> Add information about how to set SMBIOS properties using the devicetree.
> >>>
> >>> Signed-off-by: Simon Glass <sjg@chromium.org>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>>  doc/arch/x86.rst                             |  8 +++++
> >>>  doc/device-tree-bindings/board/board_x86.txt | 36 ++++++++++++++++++++
> >>>  2 files changed, 44 insertions(+)
> >>>  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
> >>>
> >>> diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
> >>> index c6b70ce61a3..45f9ba308c7 100644
> >>> --- a/doc/arch/x86.rst
> >>> +++ b/doc/arch/x86.rst
> >>> @@ -740,6 +740,14 @@ Note that this is a development feature only. It is not intended for use in
> >>>  production environments. Also it is not currently part of the automated tests
> >>>  so may break in the future.
> >>>
> >>> +SMBIOS tables
> >>> +-------------
> >>> +
> >>> +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
> >>> +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values to
> >>> +use is via the device tree. For details see
> >>> +device-tree-bindings/board/board_x86.txt
> >>> +
> >>>  TODO List
> >>>  ---------
> >>>  - Audio
> >>> diff --git a/doc/device-tree-bindings/board/board_x86.txt b/doc/device-tree-bindings/board/board_x86.txt
> >>> new file mode 100644
> >>> index 00000000000..3766751896a
> >>> --- /dev/null
> >>> +++ b/doc/device-tree-bindings/board/board_x86.txt
> >>> @@ -0,0 +1,36 @@
> >>> +x86 board driver
> >>> +================
> >>> +
> >>> +This driver allows providing board-specific features such as power control
> >>> +GPIOs. In addition, the SMBIOS values can be specified in the device tree,
> >>> +as below:
> >>> +
> >>> +An optional 'smbios' subnode can be used to provide these properties.
> >>> +
> >>> +Optional properties:
> >>> +  - manufacturer: Product manufacturer for system / baseboard
> >>> +  - product:      Product name
> >>> +  - version:      Product version string
> >>> +  - serial:       Serial number for system (note that this can be overridden by
> >>> +                      the serial# environment variable)
> >>> +  - sku:          Product SKU (Stock-Keeping Unit)
> >>> +  - family:       Product family
> >>> +  - asset-tag:    Asset tag for the motherboard, sometimes used in organisations
> >>> +                      to track devices
> >>> +
> >>> +
> >>> +Example:
> >>> +
> >>> +     board {
> >>> +             compatible = "google,coral";
> >>> +
> >>> +             smbios {
> >>
> >> I think board is the wrong place for the new node.
> >>
> >> SMBIOS also includes chassis, processor and other information. We may
> >> opt to provide non-board information in future via the device-tree.
> >
> > I think of board as the whole thing. Are you thinking of it just being
> > the circuit board?
>
> The whole I would call a system.
>
> dmidecode shows bios, system, board, chassis, processor, boot. So board
> seems to be only a small aspect of the SMBIOS system description.

Well that is using smbios terminology. For U-Boot there is only the
board at present.

We could rename the board uclass to system, I suppose. But at least
for now, the terminology within U-Boot is to use the word 'board' to
describe a build target.

>
> >> If this is for U-Boot internal only usage, we should indicate this in
> >> the label name.
> >
> > All drivers are for U-Boot-internal use, right? Or are you saying you
> > don't want this information to appear in the upstream bindings?
>
> I don't won't name collisions of U-Boot with Linux when using U-Boot's
> device tree for booting an operating system on ARM and RISC-V systems.
>
> >> So it could be like:
> >>
> >> /u-boot-smbios {
> >
> > compatible string?
>
> Yes there should be one. So you know that this is the node you are
> looking for.
>
>     compatible = "u-boot,smbios";

OK good.

>
> >
> >>    bios {};
> >>    system {};
> >>    board {
> >>       manufacturer = "Hardkernel Co., Ltd.";
> >>       product = "ODROID-C2";
> >>       ...
> >>    };
> >>    chassis {};
> >>    processor {};
> >>    boot {};

I think this makes sense because here we are describing smbios tables
so we should use the same terminology.

> >> };
> >>
> >>> +                     manufacturer = "Google";
> >>> +                     product = "Coral";
> >>> +                     version = "rev2";
> >>> +                     serial = "123456789";
> >>> +                     sku = "sku3";
> >>> +                     family = "Google_Coral";
> >>> +                     asset-tag = "ABC123";
> >>> +             };
> >>> +     };
> >>>
> >>
> >> SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.
> >>
> >> On my Odroid C2 dmidecode shows the information provided by U-Boot:
> >
> > Oh dear. Legacy never dies. It never occurred to me that this would be
> > used on ARM.
>
> Don't forget about RISC-V :)
>
> https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.3.0.pdf:
>
> "The following processor architectures are now supported:
>
> * IA-32 (x86),386
> * x64 (x86-64, Intel64, AMD64, EM64T),
> * Intel Itanium architecture,
> * 32-bit ARM (Aarch32),
> * 64-bit ARM (Aarch64),
> * RISC-V 32 (RV32),
> * RISC-V 64 (RV64),
> * RISC-V 128 (RV128)"
>
> If you think of data centers, it may be valuable to be able to use the
> same management tools for amd64, aarch64 and rv64.

Yes, long live legacy!

Regards,
SImon