build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38985 )
Change subject: WIP: skylake-sp: Add uncore and memory map ......................................................................
Patch Set 10:
(8 comments)
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... File src/cpu/intel/xeonsp/cpu/skylake-sp/include/soc/pci_devs.h:
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 37: #define SKXSP_MMAP_VTD_CFG_REG_DEVID 0x2024 please, no space before tabs
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... File src/cpu/intel/xeonsp/cpu/skylake-sp/include/soc/skx_log_utils.h:
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 20: printk(BIOS_SPEW, "%s:%d res: %s, dev: %s, index: 0x%x, base: 0x%llx, end: 0x%llx, size_kb: 0x%llx\n", \ line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 21: __func__, __LINE__, type, dev_path(dev), index, (base_kb << 10), (base_kb << 10) + (size_kb << 10) - 1, size_kb) line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 24: printk(BIOS_SPEW, "%s:%d res: %s, dev: %s, index: 0x%x, base: 0x%llx, end: 0x%llx, size: 0x%llx\n", \ line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 27: #define ADD_MMIO_RESOURCE(dev, index, base, size) \ Macros with multiple statements should be enclosed in a do - while loop
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 28: mmio_resource(dev, index, (uint64_t) ((uint64_t)base >> 10), (uint64_t) ((uint64_t)size >> 10)); \ line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 29: LOG_MEM_RESOURCE("mmio", dev, index, (uint64_t) ((uint64_t)base >> 10), (uint64_t) ((uint64_t)size >> 10)) line over 96 characters
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... File src/cpu/intel/xeonsp/cpu/skylake-sp/uncore.c:
https://review.coreboot.org/c/coreboot/+/38985/10/src/cpu/intel/xeonsp/cpu/s... PS10, Line 331: printk(BIOS_DEBUG, "mmapvtd_init\n"); Prefer using '"%s...", __func__' to using 'mmapvtd_init', this function's name, in a string