[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [dmidecode] [PATCH] dmioem: Reflect HPE's new company name.
|
From: |
Jerry Hoemann |
|
Subject: |
Re: [dmidecode] [PATCH] dmioem: Reflect HPE's new company name. |
|
Date: |
Wed, 13 Sep 2017 11:09:41 -0600 |
|
User-agent: |
Mutt/1.8.3 (2017-05-23) |
On Wed, Sep 06, 2017 at 02:29:44PM +0200, Jean Delvare wrote:
> Hi Jerry,
>
> Sorry for the delay, I was on vacation. Thanks Erwan for the
> preliminary review - overall I agree, the patch looks essentially good.
> To the details now...
>
> On Fri, 25 Aug 2017 17:54:43 -0600, Jerry Hoemann wrote:
> > After Hewlett Packard Enterprise split from Hewlett-Packard, DMI OEM
> > tables reflect the new company name. Gen10 and subsequent systems will
> > use HPE. Gen9 and prior systems continue to use the old "HP" name.
> >
> > Signed-off-by: Jerry Hoemann <address@hidden>
> > Reviewed-by: Randy Wright <address@hidden>
> > ---
> > dmioem.c | 17 +++++++++++------
> > 1 file changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/dmioem.c b/dmioem.c
> > index 034ad9f..a09b6e5 100644
> > --- a/dmioem.c
> > +++ b/dmioem.c
> > @@ -35,6 +35,7 @@ enum DMI_VENDORS
> > VENDOR_UNKNOWN,
> > VENDOR_HP,
> > VENDOR_ACER,
> > + VENDOR_HPE,
> > };
> >
> > static enum DMI_VENDORS dmi_vendor = VENDOR_UNKNOWN;
> > @@ -58,6 +59,8 @@ void dmi_set_vendor(const char *s)
> >
> > if (strncmp(s, "HP", len) == 0 || strncmp(s, "Hewlett-Packard", len) ==
> > 0)
> > dmi_vendor = VENDOR_HP;
> > + else if (strncmp(s, "HPE",len) == 0 || strncmp(s, "Hewlett Packard
> > Enterprise", len) == 0)
>
> Coding style: missing space after comma.
Will fix.
>
> > + dmi_vendor = VENDOR_HPE;
> > else if (strncmp(s, "Acer", len) == 0)
> > dmi_vendor = VENDOR_ACER;
> > }
> > @@ -98,14 +101,15 @@ static int dmi_decode_hp(const struct dmi_header *h)
> > u8 *data = h->data;
> > int nic, ptr;
> > u32 feat;
> > + const char *company = (dmi_vendor == VENDOR_HP) ? "HP": "HPE";
>
> Coding style: missing space before colon.
Will fix.
>
> >
> > switch (h->type)
> > {
> > case 204:
> > /*
> > - * Vendor Specific: HP ProLiant System/Rack Locator
> > + * Vendor Specific: HPE ProLiant System/Rack Locator
> > */
> > - printf("HP ProLiant System/Rack Locator\n");
> > + printf("%s ProLiant System/Rack Locator\n", company);
> > if (h->length < 0x0B) break;
> > printf("\tRack Name: %s\n", dmi_string(h, data[0x04]));
> > printf("\tEnclosure Name: %s\n", dmi_string(h,
> > data[0x05]));
> > @@ -155,7 +159,7 @@ static int dmi_decode_hp(const struct dmi_header *h)
> >
>
> Did you leave cases 209 and 221 out on purpose?
Yes, intentional. But, upon reflection, not what we want.
Will fix.
>
> > case 233:
> > /*
> > - * Vendor Specific: HP ProLiant NIC MAC Information
> > + * Vendor Specific: HPE ProLiant NIC MAC Information
> > *
> > * This prints the BIOS NIC number,
> > * PCI bus/device/function, and MAC address
> > @@ -171,7 +175,7 @@ static int dmi_decode_hp(const struct dmi_header *h)
> > * 0x08 | MAC | 32B | MAC addr padded w/ 0s
> > * 0x28 | Port No| BYTE | Each NIC maps to a Port
> > */
> > - printf("HP BIOS PXE NIC PCI and MAC Information\n");
> > + printf("%s BIOS PXE NIC PCI and MAC Information\n",
> > company);
> > if (h->length < 0x0E) break;
> > /* If the record isn't long enough, we don't have an ID
> > * use 0xFF to use the internal counter.
> > @@ -187,7 +191,7 @@ static int dmi_decode_hp(const struct dmi_header *h)
> > *
> > * Source: hpwdt kernel driver
> > */
> > - printf("HP 64-bit CRU Information\n");
> > + printf("%s 64-bit CRU Information\n", company);
> > if (h->length < 0x18) break;
> > printf("\tSignature: 0x%08x", DWORD(data + 0x04));
> > if (is_printable(data + 0x04, 4))
> > @@ -212,7 +216,7 @@ static int dmi_decode_hp(const struct dmi_header *h)
> > *
> > * Source: hpwdt kernel driver
> > */
> > - printf("HP ProLiant Information\n");
> > + printf("%s ProLiant Information\n", company);
> > if (h->length < 0x08) break;
> > printf("\tPower Features: 0x%08x\n", DWORD(data +
> > 0x04));
> > if (h->length < 0x0C) break;
> > @@ -281,6 +285,7 @@ int dmi_decode_oem(const struct dmi_header *h)
> > switch (dmi_vendor)
> > {
> > case VENDOR_HP:
> > + case VENDOR_HPE:
> > return dmi_decode_hp(h);
> > case VENDOR_ACER:
> > return dmi_decode_acer(h);
>
> For the first 2, you updated "HP" to "HPE" in the heading comment. For
> the last 2 you did not. Is it on purpose, or an overlook?
Missed. Will update comments to HPE in file.
>
> (I don't really care if we update these comments or not, but I like
> consistency.)
>
> --
> Jean Delvare
> SUSE L3 Support
--
-----------------------------------------------------------------------------
Jerry Hoemann Software Engineer Hewlett Packard Enterprise
-----------------------------------------------------------------------------