Discussion:
Can we please remove the FSP TempRamInit option (where applicable)
(too old to reply)
Nico Huber
2018-11-06 18:11:22 UTC
Permalink
Hi coreboot fellows,

I have always been confused that we have the option to use FSP's
TempRamInit (CAR setup) even when a native coreboot implementation is
available. Now, what I'm really concerned about is the low quality of
the code in coreboot surrounding it. There are often Kconfig prompts
that don't add up, and about every related, merged commit I've been
looking at today seemed somehow flawed.

So if we can't keep the quality we are used to when trying to maintain
two (or even more) CAR options, why not focus on a single one? After
all, coreboot is a firmware framework, not an FSP testing framework.

Here's just one weird example of what I was confronted with today:

default USE_CANNONLAKE_CAR_NEM_ENHANCED if
MAINBOARD_HAS_CHROMEOS

I don't understand it, but somehow feel offended. Does that mean I have
to work with ChromeOS now to get reasonable defaults?

Nico
--
coreboot mailing list: ***@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot
Lance Zhao
2018-11-06 22:39:56 UTC
Permalink
Yes, I believe we should let mainboard to select CAR implementation instead
of force selection in soc Kconfig. I will suggest to remove that line.
Post by Nico Huber
Hi coreboot fellows,
I have always been confused that we have the option to use FSP's
TempRamInit (CAR setup) even when a native coreboot implementation is
available. Now, what I'm really concerned about is the low quality of
the code in coreboot surrounding it. There are often Kconfig prompts
that don't add up, and about every related, merged commit I've been
looking at today seemed somehow flawed.
So if we can't keep the quality we are used to when trying to maintain
two (or even more) CAR options, why not focus on a single one? After
all, coreboot is a firmware framework, not an FSP testing framework.
default USE_CANNONLAKE_CAR_NEM_ENHANCED if
MAINBOARD_HAS_CHROMEOS
I don't understand it, but somehow feel offended. Does that mean I have
to work with ChromeOS now to get reasonable defaults?
Nico
--
https://mail.coreboot.org/mailman/listinfo/coreboot
Nico Huber
2018-11-07 08:20:22 UTC
Permalink
Hi Lance,
Post by Lance Zhao
Yes, I believe we should let mainboard to select CAR implementation instead
of force selection in soc Kconfig. I will suggest to remove that line.
having a `select` for the CAR implementation is not what I had in mind
but definitely something we can discuss. The current situation is _not_
that the SoC selects it, but rather that the SoC offers a choice with
questionable defaults and possible settings that don't even compile.

So, if we'd want to set the CAR implementation in stone per mainboard,
I think we should discuss first, how to decide (as a developer that adds
a mainboard) what to choose. And there is the problem: I myself don't
see a choice and would always take the open-source implementation.

Take FSP2.0 for instance, most SoCs currently offer a choice between:

o "Enhanced Non-evict mode"
o "Use FSP CAR"

While I can find many arguments to select the former, I can't find any
for the latter. Can you help me there?

Nico

PS. I presume that both options technically offer the same feature.
Is there something I'm missing?
--
coreboot mailing list: ***@coreboot.org
https://mail.coreboot.org/mailman/listinfo/coreboot
Loading...