Patch Set 2:

Patch Set 1: Code-Review-1

Why is the change useful?

I hope commit msg has adequate details.

Is there a customer that don't want postcar?

I thought we are working in "open source environment" and should be open for exploratory activity. and yes, FW design should have flexibility to pick and drop it's stages. But looks like on latest socs. Post car is no more an optional stage so system won't boot if soc kconfig just drops POSTCAR_STAGE selection.

Same for other stages like bootblock, romstage, payload ...

It was introduced to get rid of platform specific CAR tear down. I don't see why adding platform specific CAR tear down would help the "open source environment".

As this is an "open source environment", can't you pick and drop FSP, as it's the biggest and slowest?
Why do your start with the fastest and slimmest?


Additionally https://review.coreboot.org/c/coreboot/+/34476
comments from Aaron as below

FWIW, putting CAR teardown on the front of ramstage would get rid of the extra stage load. That was my point.
>
> Do you need additional space for FSP?

May be yes, sometime we are ending up with FW size being ~16.3MB and due to that recommend user to move to 32 MB SPI, hence its better to optimize the space to see what we need and what we can drop

Start with FSP?


> Beside the minor booting time and minor flash size, it only increases code complexity and adds unmaintained code.

Looking at the MAINTAINERS file Intel doesn't even maintain soc/intel.

what is "unmaintained " code here, for that matter postcar once added was "unmaintained" code ? but later every soc just migrated into that. Also what is additional "complexity" here, it uses exact same car tear down logic that postcar use. rather tearing down in front of ramstage, we are tearing in ramstage front when postcar not selected

It adds more preprocessor code and conditionals, making it harder to read and understand.

Also there's no documentation.

To best postcar documenation we have is in "Documenation/getting_started/architecture.md"
Feel free to write a better one.

can you please point me to postcar implementation document so i can "just" add commit msg line there to say it does same work in ramstage in absence of postcar with those additional benefits?

View Change

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Ibc88e6727b2fa692585dffe576e69f6d4d0b349d
Gerrit-Change-Number: 34752
Gerrit-PatchSet: 2
Gerrit-Owner: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki@gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth@google.com>
Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Subrata Banik <subrata.banik@intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus@gmail.com>
Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net>
Gerrit-Comment-Date: Tue, 06 Aug 2019 17:14:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment