On Thu, Mar 13, 2025 at 10:39:07AM +0100, Jean Delvare wrote: > Hi Jerry, > > On Mon, 24 Feb 2025 17:32:01 -0700, Jerry Hoemann wrote: > > Update Enumerated Firmware Types. > > Update Enumerated Version Data Format: > > > > Signed-off-by: Jerry Hoemann <jerry.hoem...@hpe.com> > > --- > > dmioem.c | 12 +++++++++++- > > 1 file changed, 11 insertions(+), 1 deletion(-) > > > > diff --git a/dmioem.c b/dmioem.c > > index 1df77ec..c7af5d6 100644 > > --- a/dmioem.c > > +++ b/dmioem.c > > @@ -467,7 +467,7 @@ static void dmi_hp_216_fw_type(u16 code) > > "SPI Descriptor Version", > > "Innovation Engine Firmware (IE Firmware)", > > "UMB Backplane Firmware", > > - "Reserved", /* 0x14 */ > > + "Embedded Diagnostics", /* 0x14 */ > > "Reserved", > > I'd move the comment as /* 0x15 */ on the following line, as apparently > we mark the boundaries of the Reserved ranges.
Done. > > > "Reserved", > > "Reserved", > > @@ -511,6 +511,10 @@ static void dmi_hp_216_fw_type(u16 code) > > "Power Distribution Board CPLD", > > "PCIe Switch Board CPLD", > > "Sideband Board CPLD", > > + "PCIe Riser MCU Firmware", /* 0x40 */ > > + "PCIe Switch Board Firmware", > > + "Power Supply Firmware", > > + "BMC Firmware", > > }; > > > > if (code < ARRAY_SIZE(type)) > > @@ -597,6 +601,12 @@ static void dmi_hp_216_version(u8 format, u8 *data) > > case 18: > > pr_attr(name, "%d.%2d", data[0], data[1]); > > Not new in this patch, but the format above looks suspicious. %2d would > left-pad numbers from 0 to 9 with a space after the dot. I suspect you > want to pad with a 0 instead, so the format should be "%d.%02d" (so for > example [1, 2] would be displayed as "1.02")? Correct, format should have been: "%d.%02d", i.e. Dollars and cents. Will send a separate patch to fix this issue. > > > break; > > + case 19: > > + pr_attr(name, "%0x.%0x.%0x", data[0], data[1], data[2]); > > + break; > > Not sure what is the intent here, but as far as I know, a 0 flag > without a field width has no effect in a printf format. Sorry, just wrong. fixed. > > > + case 20: > > + pr_attr(name, "%d.%d.%d.%d", data[0], data[1], data[2], > > data[3]); > > + break; > > This is the very same format as case 14. Please check if this is > expected. If it is, then please merge them to avoid code duplication. > > (Note: 20 is 0x14, this could have caused confusion.) The decode for Entry 14 is incorrect. Entry 14 is for VROC version number which is also of the form A.B.C.D, but there D is two bytes making 5 bytes total. I'll send a separate patch to fix that error. > > > case 3: /* fall through */ > > default: > > pr_attr(name, "%s", reserved); > > Thanks, > -- > Jean Delvare > SUSE L3 Support -- ----------------------------------------------------------------------------- Jerry Hoemann Software Engineer Hewlett Packard Enterprise -----------------------------------------------------------------------------
