Tim Chu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45138 )
Change subject: arch/x86/smbios: Update SMBIOS type 0 ec version ......................................................................
Patch Set 4:
(4 comments)
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.
https://review.coreboot.org/c/coreboot/+/45138/2/src/arch/x86/smbios.c File src/arch/x86/smbios.c:
https://review.coreboot.org/c/coreboot/+/45138/2/src/arch/x86/smbios.c@428 PS2, 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.
https://review.coreboot.org/c/coreboot/+/45138/2/src/arch/x86/smbios_default... File src/arch/x86/smbios_defaults.c:
https://review.coreboot.org/c/coreboot/+/45138/2/src/arch/x86/smbios_default... PS2, 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
https://review.coreboot.org/c/coreboot/+/45138/2/src/drivers/ipmi/ipmi_kcs_o... File src/drivers/ipmi/ipmi_kcs_ops.c:
https://review.coreboot.org/c/coreboot/+/45138/2/src/drivers/ipmi/ipmi_kcs_o... PS2, 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).
https://review.coreboot.org/c/coreboot/+/45138/3/src/mainboard/ocp/deltalake... File src/mainboard/ocp/deltalake/ramstage.c:
https://review.coreboot.org/c/coreboot/+/45138/3/src/mainboard/ocp/deltalake... PS3, Line 26: /* Update SMBIOS type 0 ec version. In deltalake, BMC version is used to represent ec version. */
line over 96 characters
Done