Hi Jerry,
On Mon, 2023-05-08 at 11:32 -0600, Jerry Hoemann wrote:
> Decode HPE OEM Record 245: Extension Board Inventory Record.
>
> Signed-off-by: Jerry Hoemann <jerry.hoem...@hpe.com>
> ---
> dmioem.c | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 56 insertions(+)
>
> diff --git a/dmioem.c b/dmioem.c
> index 04a89ab..5ccd30e 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -771,6 +771,28 @@ static void dmi_hp_242_speed(const char *attr, u16 speed)
> pr_attr(attr, "%s", "Unknown");
> }
>
> +static void dmi_hp_245_pcie_riser(const struct dmi_header *h)
> +{
> + const char *str = "Reserved";
> + u8 *data = h->data;
> +
> + pr_attr("Board Type", "PCIe Riser");
> + if (h->length < 0x09) return;
> + switch (data[0x05])
> + {
> + case 1: str = "Primary"; break;
> + case 2: str = "Secondary"; break;
> + case 3: str = "Tertiary"; break;
> + case 4: str = "Quaternary"; break;
> + case 10: str = "Front"; break;
> + }
> + pr_attr("Riser Position", "%s", str);
> + pr_attr("Riser ID", "%d", data[0x06]);
In the sample you provided, there are 2 riser records and both have
this field set to 2. I'd expect this ID to be unique within a given
system. The code looks right though, so I suppose the problem is with
the DMI table data?
> + if (data[0x07])
> + pr_attr("CPLD Version", "%c:%x", (data[0x07] >> 7) ? 'B' :
> 'R', (data[0x07] & 0x7F));
Hexadecimal number printed without a prefix can be confusing. Is there
any standard regarding how a CPLD version should be presented?
> + pr_attr("Riser Name", dmi_string(h, data[0x08]));
> +}
> +
> static int dmi_decode_hp(const struct dmi_header *h)
> {
> u8 *data = h->data;
> @@ -1490,6 +1512,40 @@ static int dmi_decode_hp(const struct dmi_header *h)
> dmi_hp_242_speed("Negotiated Speed", WORD(data +
> 0x3A));
> dmi_hp_242_speed("Capable Speed", WORD(data + 0x3C));
> break;
> +
> + case 245:
> + /*
> + * Vendor Specific: HPE Extension Board Inventory
> Record
> + *
> + * This record provides a mechanism for software to
> retrieve installed
> + * Extension Boards in system, such as Riser Cards,
> etc. Each extension
> + * board discovered in system boot time has a
> corresponding record
"at system boot time" would sound better, methinks.
> + * produced in SMBIOS Type 245. This record is
> currently applicable
> + * for ML, DL and Alletra series servers in Gen11 and
> will be backward
> + * compatible with next generations
> + *
> + * This is a variant record. Definition of fields
> 0x05 ... vary based
> + * upon field 0x04 Board Type.
> + *
> + * Offset | Name | Width | Description
> + * ---------------------------------------
> + * 0x00 | Type | BYTE | 0xF5, Extension
> Board Inventory Record
> + * 0x01 | Length | BYTE | Length of structure
> + * 0x02 | Handle | WORD | Unique handle
> + * 0x04 | Board Type | WORD | 0: PCIe Riser, Other
> Reserved
> + *
> + * If Board Type == 0
> + * 0x05 | Riser Pos | WORD |
> + * 0x06 | Riser ID | BYTE |
> + * 0x07 | CPLD Vers | BTYE | 0-> No CPLD. Bits
> [7][6:0] Release:Vers
> + * 0x08 | Riser Name | STRING|
> + */
> + pr_handle_name("%s ProLiant Extension Board Inventory
> Record", company);
> + if (h->length < 0x05) break;
> + if (data[0x04] == 0)
> + dmi_hp_245_pcie_riser(h);
> + break;
> +
> default:
> return 0;
> }
Everything else looks good to me.
--
Jean Delvare
SUSE L3 Support
