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:
Repost as gerrit messed up the message:
There's one file making use of struct smbios_entry:
Of course you would add a second struct for the
64bit entry point
and keep the 32bit one.
If the callers should provide the value in the correct unit the
defines, you should update all callers instead.
I don't see how chromium patches relate to coreboot and why you
them in the commit message.
|> I don't see how chromium patches relate to coreboot
Because these SMBIOS information are ultimatly consumed in
chromium, more specifically in mosys, and smbios related changes
are going there.
I don't see how it can be the second patch, if the first never will be in this
|> why you would mention them in the commit
Please see the earlier request/comment from Paul.
|> There's one file making use of struct smbios_entry:
See my comment above. That information is consumed outside of
coreboot and the smbios_entry struct is also defined at the
caller's side and elsewhere:
Those definitions have to be consistent, otherwise the system
I don't care about 3rdparty software and it's totally unrelated.
Nico seems to be in agreement that we shouldn't be breaking compatibility to other
> |> Of course you would add a second struct for the 64bit entry
> point and keep the 32bit one.
> |> If the callers should provide the value in the correct unit the
> SMBIOS spec defines, you should update all callers instead.
> Maybe I am missing something, but not sure I understand the 2
> sentences above. If you have 2 formats of smbios_entry which one
> would you return? The caller's only passes one variable into which
> this function "concatenate" the fields info, and the caller
> retrieves the information using the same standard definition of
> smbios_entry. Why would you need 2 definitions?
One struct for each entrypoint.
It's not returned, so it wouldn't matter for the caller.
The whole SMBIOS table including the entry point is returned.
Correct, the whole SMBIOS table, including the entry point is returned.
My question was: which one of the two formats for the entry will be included to the able
to be returned. Because the caller expects one format, not two.
That is why in order to maintain the compatibility, that Nico is also refering to, I
suggested not making that change now in this same review.
Are you asking
to change all the functions through which this
information passes through? And if so, how would you do that in
this commit? Since those files are not include here.
Even if that is your request it conflict's with Nico's who wants to
reduce the amount of changes going in here.
In short, having one definition of smbios_entry on one side of the
system and a different one on the end, would break the whole flow.
Please let me know if I am missing something
For the type 17 CLK units, you should provide the value in the correct unit the SMBIOS
spec defines, as it was also the case before the update. That means you should update all
providers of CBMEM_ID_MEMINFO.
The changes already present in here will take care of the type 17 new clock units. Another
alternative would be to remove the change here and push it down in the FSP codes, the
ultimate provider of these information. Which will need to be done for all SOCs since the
FSP varies per SOC. Requests to various FSP teams for furture SOCs to update their type17
info to 3.2 spec are already in the work. Past SOCs and GLK (current) for example
won't be making the change. Hence the decision to implement it here.
PS1, Line 279: t->clock_speed = dimm->ddr_frequency;
clock_speed and speed are now in units of MT/s instead
of Mhz. […]
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:01:51 +0000
Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>