Re: [PATCH v4 04/13] smbios: Allow properties to come from the device tree

2020-11-03 Thread Bin Meng
Hi Simon,

On Thu, Oct 22, 2020 at 10:21 PM Simon Glass  wrote:
>
> 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 
> ---
>
> Changes in v4:
> - Fix build error with vexpress_ca9x4
>
> Changes in v3:
> - Use a different binding with subnodes for each table type
>
>  lib/smbios.c | 98 +++-
>  1 file changed, 81 insertions(+), 17 deletions(-)
>
> diff --git a/lib/smbios.c b/lib/smbios.c
> index b0f5e936044..be72a98c49d 100644
> --- a/lib/smbios.c
> +++ b/lib/smbios.c
> @@ -17,6 +17,18 @@
>  #include 
>  #endif
>
> +/**
> + * struct smbios_write_method - Informaiton about a table-writing function

typo: Information

> + *
> + * @write: Function to call
> + * @subnode_name: Name of subnode which has the information for this 
> function,
> + * NULL if none
> + */
> +struct smbios_write_method {
> +   smbios_write_type write;
> +   const char *subnode_name;
> +};
> +
>  /**
>   * smbios_add_string() - add a string to the string area
>   *
> @@ -52,6 +64,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 +169,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",

typo: manufacturer

> + 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 +195,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",

ditto

> + 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 +217,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 = 

[PATCH v4 04/13] smbios: Allow properties to come from the device tree

2020-10-22 Thread 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 
---

Changes in v4:
- Fix build error with vexpress_ca9x4

Changes in v3:
- Use a different binding with subnodes for each table type

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

diff --git a/lib/smbios.c b/lib/smbios.c
index b0f5e936044..be72a98c49d 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -17,6 +17,18 @@
 #include 
 #endif
 
+/**
+ * struct smbios_write_method - Informaiton about a table-writing function
+ *
+ * @write: Function to call
+ * @subnode_name: Name of subnode which has the information for this function,
+ * NULL if none
+ */
+struct smbios_write_method {
+   smbios_write_type write;
+   const char *subnode_name;
+};
+
 /**
  * smbios_add_string() - add a string to the string area
  *
@@ -52,6 +64,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 +169,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 +195,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 +217,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;
@@