Attention is currently required from: Elyes Haouas, Felix Singer, Jérémy Compostella, Shuo Liu, Vasiliy Khoruzhick.
yuchi.chen@intel.com has posted comments on this change by yuchi.chen@intel.com. ( https://review.coreboot.org/c/coreboot/+/83321?usp=email )
Change subject: soc/intel/snowridge: Add support for Intel Atom Snow Ridge SoC ......................................................................
Patch Set 25:
(5 comments)
File src/soc/intel/snowridge/chip.c:
https://review.coreboot.org/c/coreboot/+/83321/comment/94086692_7b65b240?usp... : PS15, Line 323: res = new_resource(dev, IOINDEX_SUBTRACTIVE(index++, 0));
domain non-legacy IO/MMIO ranges should be regarded as non-subtractive
The `IOINDEX_SUBTRACTIVE` are used in `pci_domain_read_resources()` in `src/device/pci_device.c`, should we follow with it?
https://review.coreboot.org/c/coreboot/+/83321/comment/b296be68_c3a0e46a?usp... : PS15, Line 418: }
will the common pci codes do this again?
I searched the whole source tree, but there is no explicit setting of SERR. `src/device/pci_device.c` has a strange comment sayiny `v3 has command |= (PCI_COMMAND_PARITY + PCI_COMMAND_SERR)`, but I don't know what it means.
https://review.coreboot.org/c/coreboot/+/83321/comment/59040e25_7f3e514c?usp... : PS15, Line 434:
in SNR, are there both south cluster PCIe and north cluster PCIe? […]
SNR south cluster is on domain 0, with device function number >= 6.
File src/soc/intel/snowridge/romstage/romstage.c:
https://review.coreboot.org/c/coreboot/+/83321/comment/71847f16_3eb7f080?usp... : PS15, Line 108: const FSP_SMBIOS_MEMORY_INFO *fsp_smbios_memory_info;
to declare variable along with using will help the readability of the codes.
Done
File src/soc/intel/snowridge/systemagent.c:
https://review.coreboot.org/c/coreboot/+/83321/comment/fe23a08f_2bace14e?usp... : PS15, Line 32: void soc_add_fixed_mmio_resources(struct device *dev, int *resource_cnt)
The parameter in the declaration is resource_cnt, but all the SoC implementations renamed it to inde […]
Done