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-
ting dependencies.

Reference to the mosys change in the commit message have been removed.

If I tell you, you shouldn't do this or that in this change,
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.

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 vs.
> 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.

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: 6
Gerrit-Owner: Francois Toguo Fotso <>
Gerrit-Reviewer: Francois Toguo Fotso <>
Gerrit-Reviewer: Nico Huber <>
Gerrit-Reviewer: Patrick Georgi <>
Gerrit-Reviewer: Patrick Rudolph <>
Gerrit-Reviewer: Paul Menzel <>
Gerrit-Reviewer: build bot (Jenkins) <>
Gerrit-CC: Francois Toguo Fotso <>
Gerrit-CC: Lijian Zhao <>
Gerrit-CC: Patrick Rudolph <>
Gerrit-Comment-Date: Mon, 01 Apr 2019 20:09:03 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment