Patch Set 2:

(3 comments)

Patch Set 2:

Not sure it is ok to use BMC version directly?

I've no idea if it's correct to use the BMC version here. If SMBIOS says this is for the Embedded Controller, then we shouldn't write the BMC information

In YV3, there's no EC. BMC is used to represent EC in YV3, so I think maybe it is fine to use BMC version here. If it's not suitable I'll remove it.

View Change

4 comments:

• File src/arch/x86/smbios.c:

  • Patch Set #2, Line 428:

    smbios_ec_revision(&ipmi_bmc_major_revision, &ipmi_bmc_minor_revision);
    	t->ec_major_release = ipmi_bmc_major_revision;
    	t->ec_minor_release = ipmi_bmc_minor_revision;
    

    If SMBIOS says this is EC, why use `ipmi_bmc` here? […]

    Because there's no EC in deltalake, we use BMC to represent EC.

• File src/arch/x86/smbios_defaults.c:

  • Patch Set #2, Line 66:

    __weak void smbios_ec_revision(uint8_t *ipmi_bmc_major_revision,
    	uint8_t *ipmi_bmc_minor_revision)
    

    the parameter names should be `ec_major_release` and `ec_minor_release`

    Done

• File src/drivers/ipmi/ipmi_kcs_ops.c:

  • Patch Set #2, Line 283: ((bmc_revision_major / 16) * 10) + (bmc_revision_major % 16)

    Huh? What is this math needed for?

    The value received from OpenBMC is hexadecimal, however it should be decimal. It need to be transferred to decimal so that we can get right value(such as 0x33 -> 33).

• File src/mainboard/ocp/deltalake/ramstage.c:

  • Patch Set #3, Line 26: /* Update SMBIOS type 0 ec version. In deltalake, BMC version is used to represent ec version. */

    line over 96 characters

    Done

To view, visit change 45138. To unsubscribe, or for help writing mail filters, visit settings.