[PATCH v2 0/2] smbios: fill wake-up type
We should not use the reserved value 0x00 for the wake up type but use 0x06 (Power Switch).
Correctly display wake-up type in smbios command.
Display SMBIOS type 1 field family in smbios command.
v2: use wake-up type 0x02 as our default
Heinrich Schuchardt (2): cmd: smbios: type 1 wake-up time, family smbios: fill wake-up type
cmd/smbios.c | 25 +++++++++++++++++++++++-- include/smbios.h | 10 ++++++++++ lib/smbios.c | 1 + 3 files changed, 34 insertions(+), 2 deletions(-)
Correct type 1 output
* render wake up time as string * print family string * remove duplicate serial number output
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: no change --- cmd/smbios.c | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-)
diff --git a/cmd/smbios.c b/cmd/smbios.c index 66f6b761378..d3bd8b12a67 100644 --- a/cmd/smbios.c +++ b/cmd/smbios.c @@ -14,6 +14,18 @@
DECLARE_GLOBAL_DATA_PTR;
+static const char * const wakeup_type_strings[] = { + "Reserved", /* 0x00 */ + "Other", /* 0x01 */ + "Unknown", /* 0x02 */ + "APM Timer", /* 0x03 */ + "Modem Ring", /* 0x04 */ + "Lan Remote", /* 0x05 */ + "Power Switch", /* 0x06 */ + "PCI PME#", /* 0x07 */ + "AC Power Restored", /* 0x08 */ +}; + /** * smbios_get_string() - get SMBIOS string from table * @@ -72,6 +84,14 @@ void smbios_print_str(const char *label, void *table, u8 index) printf("\t%s: %s\n", label, smbios_get_string(table, index)); }
+const char *smbios_wakeup_type_str(u8 wakeup_type) +{ + if (wakeup_type >= ARRAY_SIZE(wakeup_type_strings)) + /* Values over 0x08 are reserved. */ + wakeup_type = 0; + return wakeup_type_strings[wakeup_type]; +} + static void smbios_print_type1(struct smbios_type1 *table) { printf("System Information\n"); @@ -81,11 +101,12 @@ static void smbios_print_type1(struct smbios_type1 *table) smbios_print_str("Serial Number", table, table->serial_number); if (table->length >= 0x19) { printf("\tUUID: %pUl\n", table->uuid); - smbios_print_str("Wake Up Type", table, table->serial_number); + printf("\tWake-up Type: %s\n", + smbios_wakeup_type_str(table->wakeup_type)); } if (table->length >= 0x1b) { - smbios_print_str("Serial Number", table, table->serial_number); smbios_print_str("SKU Number", table, table->sku_number); + smbios_print_str("Family", table, table->family); } }
We should not use the reserved value 0x00 for the wake up type but use 0x02 (Unknown).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- v2: use wake-up type 'Unknown' as our default add more SMBIOS wake-up type constants --- include/smbios.h | 10 ++++++++++ lib/smbios.c | 1 + 2 files changed, 11 insertions(+)
diff --git a/include/smbios.h b/include/smbios.h index 3df8827b60d..990e37b4d2b 100644 --- a/include/smbios.h +++ b/include/smbios.h @@ -108,6 +108,16 @@ struct __packed smbios_type0 { char eos[SMBIOS_STRUCT_EOS_BYTES]; };
+#define SMBIOS_WAKEUP_TYPE_RESERVED 0x00 +#define SMBIOS_WAKEUP_TYPE_OTHER 0x01 +#define SMBIOS_WAKEUP_TYPE_UNKNOWN 0x02 +#define SMBIOS_WAKEUP_TYPE_APM_TIME 0x03 +#define SMBIOS_WAKEUP_TYPE_MODEM_RING 0x04 +#define SMBIOS_WAKEUP_TYPE_LAN_REMOTE 0x05 +#define SMBIOS_WAKEUP_TYPE_POWER_SWITCH 0x06 +#define SMBIOS_WAKEUP_TYPE_PCI_PME 0x07 +#define SMBIOS_WAKEUP_TYPE_AC_POWER_RESTORED 0x08 + struct __packed smbios_type1 { u8 type; u8 length; diff --git a/lib/smbios.c b/lib/smbios.c index c83af730a91..b190b010f30 100644 --- a/lib/smbios.c +++ b/lib/smbios.c @@ -394,6 +394,7 @@ static int smbios_write_type1(ulong *current, int handle, } else { t->serial_number = smbios_add_prop(ctx, "serial", NULL); } + t->wakeup_type = SMBIOS_WAKEUP_TYPE_UNKNOWN; t->sku_number = smbios_add_prop(ctx, "sku", NULL); t->family = smbios_add_prop(ctx, "family", NULL);
On Fri, Feb 09, 2024 at 04:51:15PM +0100, Heinrich Schuchardt wrote:
We should not use the reserved value 0x00 for the wake up type but use 0x02 (Unknown).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
[snip]
@@ -108,6 +108,16 @@ struct __packed smbios_type0 { char eos[SMBIOS_STRUCT_EOS_BYTES]; };
+#define SMBIOS_WAKEUP_TYPE_RESERVED 0x00 +#define SMBIOS_WAKEUP_TYPE_OTHER 0x01 +#define SMBIOS_WAKEUP_TYPE_UNKNOWN 0x02 +#define SMBIOS_WAKEUP_TYPE_APM_TIME 0x03 +#define SMBIOS_WAKEUP_TYPE_MODEM_RING 0x04 +#define SMBIOS_WAKEUP_TYPE_LAN_REMOTE 0x05 +#define SMBIOS_WAKEUP_TYPE_POWER_SWITCH 0x06 +#define SMBIOS_WAKEUP_TYPE_PCI_PME 0x07 +#define SMBIOS_WAKEUP_TYPE_AC_POWER_RESTORED 0x08
Shouldn't we do this as an enum these days?
On 2/9/24 19:12, Tom Rini wrote:
On Fri, Feb 09, 2024 at 04:51:15PM +0100, Heinrich Schuchardt wrote:
We should not use the reserved value 0x00 for the wake up type but use 0x02 (Unknown).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
[snip]
@@ -108,6 +108,16 @@ struct __packed smbios_type0 { char eos[SMBIOS_STRUCT_EOS_BYTES]; };
+#define SMBIOS_WAKEUP_TYPE_RESERVED 0x00 +#define SMBIOS_WAKEUP_TYPE_OTHER 0x01 +#define SMBIOS_WAKEUP_TYPE_UNKNOWN 0x02 +#define SMBIOS_WAKEUP_TYPE_APM_TIME 0x03 +#define SMBIOS_WAKEUP_TYPE_MODEM_RING 0x04 +#define SMBIOS_WAKEUP_TYPE_LAN_REMOTE 0x05 +#define SMBIOS_WAKEUP_TYPE_POWER_SWITCH 0x06 +#define SMBIOS_WAKEUP_TYPE_PCI_PME 0x07 +#define SMBIOS_WAKEUP_TYPE_AC_POWER_RESTORED 0x08
Shouldn't we do this as an enum these days?
The field in the SMBIOS is of type u8 and cannot be an enum. Defining an enum would only make a difference if we had a function using it.
Do you want me to resend the patch with an enum?
Best regards
Heinrich
On Fri, Feb 09, 2024 at 07:37:28PM +0100, Heinrich Schuchardt wrote:
On 2/9/24 19:12, Tom Rini wrote:
On Fri, Feb 09, 2024 at 04:51:15PM +0100, Heinrich Schuchardt wrote:
We should not use the reserved value 0x00 for the wake up type but use 0x02 (Unknown).
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
[snip]
@@ -108,6 +108,16 @@ struct __packed smbios_type0 { char eos[SMBIOS_STRUCT_EOS_BYTES]; }; +#define SMBIOS_WAKEUP_TYPE_RESERVED 0x00 +#define SMBIOS_WAKEUP_TYPE_OTHER 0x01 +#define SMBIOS_WAKEUP_TYPE_UNKNOWN 0x02 +#define SMBIOS_WAKEUP_TYPE_APM_TIME 0x03 +#define SMBIOS_WAKEUP_TYPE_MODEM_RING 0x04 +#define SMBIOS_WAKEUP_TYPE_LAN_REMOTE 0x05 +#define SMBIOS_WAKEUP_TYPE_POWER_SWITCH 0x06 +#define SMBIOS_WAKEUP_TYPE_PCI_PME 0x07 +#define SMBIOS_WAKEUP_TYPE_AC_POWER_RESTORED 0x08
Shouldn't we do this as an enum these days?
The field in the SMBIOS is of type u8 and cannot be an enum. Defining an enum would only make a difference if we had a function using it.
Do you want me to resend the patch with an enum?
Since I think this is going to be the start of adding ore and similar values (which to be clear, is good), yes please lets get in the habit of generally using enums here. Thanks.
participants (2)
-
Heinrich Schuchardt -
Tom Rini