Patch Set 2:

To me, this way of organizing the code is more confusing. The chip.h and soc_intel_xeon_sp_ops are reloaded for different Xeon-SP processors. This will cause confusions for code review and debugging.

Another option is to keep different names (eg. soc_intel_xeon_sp_skx_ops and soc_intel_xeon_sp_cpx_ops), and use build time switch (ifdef) to pick appropriate struct for specific Xeon-SP processor.

I am trying to minimize ifdefs that do the same thing, especially on silicon. This should have less confusion as more silicon is added. There is a generic chip.h that includes the correct silicon file. The chip config files look like they are 95% the same, so common code can use the common structure names and the uncommon things should break at build time if there is a code mistake. It is harder to read and review definitions that are duplicated in each silicon version with the same file names and very similar code. After unwinding the SKx/CPX, there is very little difference, just a lot of things that have been moved around and renamed.

Going forward, common defines can be de-duplicated and moved here. I think that a common config struct that includes a silicon specific structure would work.

That said, this change is larger than the one specific need to check a config option in ACPI generation. The assumption is that there would be further de-duplication in the future.

The change we are fixing duplicating SILICON for each version in a function:
#if SILICON
const struct soc_intel_xeon_sp_SILICON_config *const config = config_of(device);
#endif

...
if (config->vtd_support)


A simpler fix is to ignore the silicon typedef as the other code does. I am going to remove the dependency on this patch group for now.

View Change

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I1c5e3dec3ee24328b1e987fdae856e9e77fe5499
Gerrit-Change-Number: 45969
Gerrit-PatchSet: 2
Gerrit-Owner: Marc Jones <marc@marcjonesconsulting.com>
Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com>
Gerrit-Reviewer: Jonathan Zhang <jonzhang@fb.com>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer@coreboot.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-Comment-Date: Mon, 05 Oct 2020 18:21:09 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment