| Message ID | 20240209155115.102957-3-heinrich.schuchardt@canonical.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | smbios: fill wake-up type | expand |
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.
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);
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(+)