Hi Werner,
thanks for getting the discussion back on track :-)
Am Fr., 30. Nov. 2018 um 09:23 Uhr schrieb Zeh, Werner < werner.zeh@siemens.com>:
Speaking of the two chipsets in question in this thread I do not see the real demand of getting rid of them yet.
Why is coreboot not able to move forward if we keep fsp_baytrail and
fsp_broadwell_de in the tree? Sure, we cannot expect to get all the fancy new features in them, but why should these chipsets stop working? What kind of source tree refactoring can hit theses chipsets that hard?
First things first: Arthur proposed making a number of features mandatory so that the bootflow becomes more predictable across chipsets and boards. Mandatory features imply that chipsets or boards that can't comply to these new requirements for whatever reason will be dropped. There was no decision yet whether to make anything mandatory, and on what schedule and with what effect.
As I understand Arthur's first analysis, he considered postcar to be impossible to do with FSP 1.0 due to FSP 1.0's design. Now he's providing patches, so apparently it's not that impossible after all?
In that case, the main concern is finding people who are willing to test such changes (and more ideally: help lifting older chipsets to new standards). Theoretically, and we did that in the past, we could just review patches like Arthur's on a best effort basis and get them in because they look reasonable and pass the build test. I'm not fundamentally opposed to such a treatment for inactive code (ie. where no maintainer is known), but I'd rather not see that happen all that often going forward.
In that light I'd like to bring up our MAINTAINERS file again: It's now used to automatically add people as reviewers to patches, and it has two tiers of reviewers: "maintainers" (declared with an M: prefix) and "reviewers" (declared with an R: prefix). I encourage people who want/need to keep up to speed on chipsets to add themselves as reviewer for these subdirectories, so they're notified when significant changes are proposed to the code they rely on.
So maybe people like you and Jay want to register yourselves as reviewers to fsp_baytrail and fsp_broadwell_de, so you'll notice changes early and can test and comment on them?
I am completely with you Ron that it is a bad idea of keeping boards in the
tree which are not relay tested on hardware for a long time.
And just because of this reason I have a test setup around where every of
these boards it tested on real hardware several times a day with the latest master tree.
I guess that's approaching better coverage than 90% of the tree (unless there's another huge test operation out there that doesn't report to the public). So if we were to review changes to make these fsp 1.0 boards "somehow" postcar-capable, you'd notice shortly after we checked them in if that broke anything?
This setup ensures that the mainboards can boot without issues into the OS.
So the argument that the chipsets in question are not tested on real hardware is not valid now.
Indeed, but it's not visible.
I'll work on improving the board-status infrastructure "soon". Is publicly reporting which commits on master work on your boards something you could do, or is that off-limits for some operational reason? (not talking about any implementation details, since those are subject to change, just if there's a problem with sending coreboot and kernel logs of test machines, likely with some pruning of sensitive data, from some corporate environment to a public system).
If we some time should hit the point where a special feature cannot be
ported to one of this chipsets because of the boundaries that FSP1.0 dictates I will vote for keeping the chipsets nevertheless in the tree. In the end this chips are working fine so far and I guess we can keep them working with a small effort. And I am willing to do whatever is needed to keep this chipsets in the tree...in the scope of FSP1.0 boundaries.
"It depends". romstage is in a much better place now that we dropped the requirement that it needs to be compilable by romcc. It also left CPUs without cache-as-RAM out in the cold. I don't see anything that might require a similar drastic rework though.
Going forward, to keep everybody's blood pressure at a sensible level, maybe we should insert another escalation level before "make stuff mandatory (at the threat of dropping it)"? A call for making such adjustments on a voluntary basis, eg "We have this new way of doing things that makes stuff better/faster/securer/shinier. It's already supported on these chipsets: a, b, c. coreboot could benefit if everything moved to that. Any takers?"
Patrick