[coreboot-gerrit] Change in coreboot[master]: soc/intel/xeon_sp: Add common chip.h

[Jonathan Zhang (Code Review)]

Jonathan Zhang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45969 )

Change subject: soc/intel/xeon_sp: Add common chip.h
......................................................................

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.