Patch Set 5:

(3 comments)

Patch Set 5:

(6 comments)

Patch Set 1:

(6 comments)

Patrick,
- Line 561: "_weak" so that it can be if needed for som platform/soc
- Line 829: I agree that the entry point should be added as as specified in SMBIOS 3.0. it would be more disruptive in this CL, as the change will also need to go in other other files where the table entry are used.
I suggest having a later patch and review exclusively for the table entry update.


|> It's not disruptive. Just a few lines to add in this file. No need to update the callers.
It is disruptive indeed. For example the new "anchor" field is 5 bytes whereas the current one is 4 bytes. The 5 bytes of "formated area" , the 5 bytes of "intermediate anchor string" and the 1 byte of "intermediate checksum" all need to go away in 3.0 and one "reserve" byte need to come in. Even if it was just about updating the anchor string alone, changing its size here and not on the callers side will not work. Furthermore that smbios_entry struct is defined and used in at least two other different filee, hence my previous suggestion.

|> Commit message: Line 10: I can't find it on Gerrit.
The hash was requested. Here is the full CL path, it hasn't been merge yet:
https://chromium-review.googlesource.com/c/chromiumos/platform/mosys/+/1542758

|> Commit message: Line 13: It does more than that.
Yes, but that was just asummary of what the procedure does. If you have a better suggestion like Paul, I am happy to add it.

|>src/arch/x86/smbios.c: Line 279: Are you sure that all callers provide the value in MHz?
They should. Per previous SMBIOS version specs, that speed have consistently been defined in MHZ. Even if other callers were mistakenly not providing it in MHz, then the fix most likely in other files, should be provided in other patches. Please sync up your comments with Nico's, he wants to reduce the number of changes in the patch. This cannot be achieved while fixing other stuffs as well.

Regards,
Francois

View Change

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1171c20ac85a2231d949d4ac1c3e3c544ba3e5ef
Gerrit-Change-Number: 32090
Gerrit-PatchSet: 5
Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso@intel.com>
Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso@intel.com>
Gerrit-Reviewer: Nico Huber <nico.h@gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Francois Toguo Fotso <francois.toguo.fotso@intel.corp-partner.google.com>
Gerrit-CC: Lijian Zhao <lijian.zhao@intel.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph@9elements.com>
Gerrit-Comment-Date: Fri, 29 Mar 2019 17:03:13 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment