Re: [PATCH v2 1/1] dmioem: Update HPE OEM Record 216

[Jean Delvare]

Hi Jerry,

On Thu, 26 Jan 2023 18:12:52 -0700, Jerry Hoemann wrote:
> Decode HPE OEM Record 216: Version Indicator Record
> 
> Signed-off-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> ---
>  dmioem.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 200 insertions(+)
> 
> diff --git a/dmioem.c b/dmioem.c
> index b791118..48c023a 100644
> --- a/dmioem.c
> +++ b/dmioem.c
> @@ -300,6 +300,161 @@ static void dmi_hp_203_devloc(const char *fname, 
> unsigned int code)
>       pr_attr(fname, "%s", str);
>  }
>  
> +static void dmi_hp_216_fw_type(u16 code)
> +{
> +     const char *str = "Reserved";
> +     static const char * const type[] = {
> +             "Reserved", /* 0x00 */
> +             "System ROM",
> (...)
> +             "Reserved", /* 0x2F */
> +             "Embedded Video Controller",
> +             "PCIe Riser N Programmable Logic Device (N is the riser 
> number)",

Thanks for clarifying the meaning of N. However I don't think there is
much value including it in the string with the further need to explain
its meaning. For one thing, the riser number is not fundamentally part
of the firmware type. For another, the "Firmware Name String" field
which immediately follows will provide the information with N "solved".
For example:

Handle 0x00B6, DMI type 216, 23 bytes
HPE Version Indicator
        Firmware Type: PCIe Riser N Programmable Logic Device (N is the riser 
number)
        Firmware Name String: PCIe Riser 1 Programmable Logic Device
        (...)

Therefore I believe it would be less confusing to just use "PCIe Riser
Programmable Logic Device" as the firmware type string. Is that OK with
you?

Everything else looks good to me now.

-- 
Jean Delvare
SUSE L3 Support