Mostly small stuff. The main thing is to drop the dummy IFD and ME files by removing HAVE_IFD_BIN and HAVE_ME_BIN from src/mainboard/ocp/tiogapass/Kconfig.
Patch set 38:Code-Review -1
28 comments:
File configs/config.ocp_tiogapass:
CONFIG_CPU_MICROCODE_CBFS_LOC=0xfff0fdc0
CONFIG_CPU_MICROCODE_CBFS_LEN=0x7C00
No need to specify these, they are the defaults as per src/soc/intel/xeon_sp/Kconfig
File src/mainboard/ocp/tiogapass/Kconfig:
select HAVE_IFD_BIN
select HAVE_ME_BIN
Remove these and coreboot will not try to add an IFD or ME image. Then we can also remove those fake binary files.
For real builds, the user can either enable these with a pre-made .config file or `make menuconfig and Chipset --> Add Intel descriptor.bin file (which will also allow an ME path to be specified)
File src/mainboard/ocp/tiogapass/Makefile.inc:
Patch Set #38, Line 18: romstage-y += boardid.c
remove (see comment in tiogapass_boardid.h)
Patch Set #38, Line 21: ramstage-y += boardid.c
same as previous comment
File src/mainboard/ocp/tiogapass/acpi/platform.asl:
Please indent this file uniformly using tabs.
File src/mainboard/ocp/tiogapass/acpi_tables.c:
Patch Set #38, Line 20: #include <soc/nvs.h>
nit: this is not needed
File src/mainboard/ocp/tiogapass/boardid.c:
This is currently unused, let's get rid of it for now and reintroduce it later if needed.
File src/mainboard/ocp/tiogapass/devicetree.cb:
Please indent this file uniformly using tabs.
File src/mainboard/ocp/tiogapass/romstage.c:
Patch Set #38, Line 18: #include "tiogapass_boardid.h"
remove (see comment in tiogapass_boardid.h)
Patch Set #38, Line 20: #include <fsp/soc_binding.h>
unnecessary #include
Patch Set #38, Line 21: #include <FspmUpd.h>
#include <soc/romstage.h>
(see comment below about mainboard_* function prototypes)
void mainboard_config_gpios(FSPM_UPD * mupd);
void mainboard_config_iio(FSPM_UPD *mupd);
void mainboard_memory_init_params(FSPM_UPD *mupd);
I'm surprised that the compiler doesn't err out on these... mainboard_config_gpios() and mainboard_memory_init_params() are defined in <soc/romstage.h> (part of the previous patch)
Patch Set #38, Line 33: void mainboard_config_gpios(FSPM_UPD *mupd)
I think this is only called from mainboard_memory_init_params() in this file, so it can be static.
Also, there is a function declaration in the SoC's romstage.h file (in the previous patch) that should be removed.
Patch Set #38, Line 40: void mainboard_config_iio(FSPM_UPD *mupd)
this can be static since it's only called from mainboard_memory_init_params()
Patch Set #38, Line 45: sizeof(tp_iio_bifur_table)/sizeof(UPD_IIO_BIFURCATION_DATA_ENTRY);
ARRAY_SIZE(tp_iio_bifur_table)
Patch Set #38, Line 50: sizeof(tp_iio_pci_port_skt0)/sizeof(UPD_PCI_PORT_CONFIG);
ARRAY_SIZE(tp_iio_pci_port_skt0);
Patch Set #38, Line 55: sizeof(tp_pch_pci_port_skt0)/sizeof(UPD_PCH_PCIE_PORT);
ARRAY_SIZE(tp_pch_pci_port_skt0);
File src/mainboard/ocp/tiogapass/skxsp_tp_gpio.h:
nit: extra asterix
nit: extra asterix
Needs an #endif
File src/mainboard/ocp/tiogapass/skxsp_tp_iio.h:
nit: Use /* (without the extra asterix)
Nit: Use */ (without the extra asterix)
Nit: extra asterix
Nit: extra asterix
indent
Nit: extra asterix
Nit: extra asterix
File src/mainboard/ocp/tiogapass/tiogapass_boardid.h:
This seems copy+pasted from harcuvar. Let's get rid of it for now, though we may introduce it in a more proper form if we need logic to tell apart board revisions / vendors for whatever reason.
To view, visit change 38549. To unsubscribe, or for help writing mail filters, visit settings.