Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40335 )
Change subject: soc/intel/common/block/acpi: Add provision for multiple PCI segments ......................................................................
Patch Set 4:
(4 comments)
https://review.coreboot.org/c/coreboot/+/40335/4/src/soc/intel/common/block/... File src/soc/intel/common/block/acpi/acpi.c:
https://review.coreboot.org/c/coreboot/+/40335/4/src/soc/intel/common/block/... PS4, Line 38: uint16_t seg_nr = 0; Assignment is not needed. See below.
https://review.coreboot.org/c/coreboot/+/40335/4/src/soc/intel/common/block/... PS4, Line 39: uint16_t pci_ex_len = (CONFIG_SA_PCIEX_LENGTH >> 20) - 1; Why is the conversion needed? pci_ex_len and PCIEX_LENGTH sounds the same.
Maybe also use `pciex_len` as name?
https://review.coreboot.org/c/coreboot/+/40335/4/src/soc/intel/common/block/... PS4, Line 44: seg_nr = pci_ex_len / 256; Move this out of the if statement, as x / 256 with x <= 255, is 0 for integers.
https://review.coreboot.org/c/coreboot/+/40335/4/src/soc/intel/common/block/... PS4, Line 48: bus_end_nr = pci_ex_len; That’s the same as above with `seg_nr = 0`. So, the else branch, and therefore the if statement is not needed.