Francois Toguo Fotso has posted comments on this change. (
Change subject: smbios: Add and adapt fields for version 3.2
Patch Set 6:
Patch Set 6:
tl;dr; Please focus on a commit message that provides a
reasoning for the change, what it gains us and the scope
of the changes. I still don't see the incentive for all
Francois, I think you are experiencing a lot of conflicting
feedback from reviewers here, because you leave too much
room for interpretation. For instance I guess, Paul asked you
to properly refer to the other change because he might have
thought it was a patch to coreboot. But it can't work out,
as you don't know yet what commit hash the other change will
Also, I don't see a reason to mention the change to mosys.
The SMBIOS spec is the interface between coreboot and mosys,
both have to comply to SMBIOS, period. Otherwise, you might
break compatibility to other programs whilst adding conflic-
Reference to the mosys change in the commit message have been removed.
If I tell you, you shouldn't do this or that in
I merely interpret the scope of this change that you set with
the commit message. And I'm still unsure what that should be.
It is very important to define the scope particularly in this
case because SMBIOS allows many things to be optional. I
can't say that I read the full document, but so far I don't
see anything that would have to change for 3.2 compatibility.
The 64-bit entry point is optional too, and I guess, again,
Patrick wouldn't have asked for it if you had provided a clear
line around what this change should comprise and what not.
So please, before making any more changes to the code, tell
us exactly what you want to achieve and why. e.g.
No more change(s) to the code is planned in this review.
* Add optional fields, as a programming excercise?
* SMBIOS 3.2 compliance in coreboot, because?
* SMBIOS 3.2 compliance plus optional fields, because?
* Keep things in sync with mosys?
A particular reason is project or test case related, we have some internal testing that
will require to support SMBIOS 3.2 to make the test pass.
And regarding existing bugs (wrong core counts, clock
MTs etc.), if they make it harder to update the SMBIOS code,
I suggest to fix them first.
Please understand that you create a lot of reviewing work
with this -- a change that doesn't gain us anything at the
moment (anticipation of future changes?). So this has pro-
bably lowest priority for most of us.
To view, visit https://review.coreboot.org/c/coreboot/+/32090
To unsubscribe, or for help writing mail filters, visit
Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com>
Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com>
Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org>
Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Francois Toguo Fotso
Gerrit-CC: Lijian Zhao <lijian.zhao(a)intel.com>
Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com>
Gerrit-Comment-Date: Mon, 01 Apr 2019 20:09:03 +0000