Patch Set 2:

Patch Set 2:

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 think the plan is to end up using the same struct for both flavors of Xeon-SP.

Realistically the flavors of Xeon-SP will not have same struct, due to FSP differences. Different processors of Xeon-SP are developed by different Intel product teams at different schedule, so the interfaces of FSPs will be different. Same struct is possible if we are okay with using a superset of configuration items in the struct.

Even though soc/intel uses the devicetree as a mirror for FSP configuration, this is not how the devicetree is meant to be used. While some FSP UPDs can be set through the devicetree, others are best set using Kconfig symbols, and others can be set depending on whether a PCI device is enabled or not. Differences in FSP UPDs can be handled in FSP-specific files, while keeping the same devicetree struct.

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 08:47:08 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment