Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38125 )
Change subject: soc/intel/{apl,cnl,icl,skl,tgl}: Make above 4GB MMIO resource proper ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/apollolake/in... File src/soc/intel/apollolake/include/soc/nvs.h:
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/apollolake/in... PS4, Line 50: uint8_t e4gm; /* 0x3D - Enable above 4GB MMIO Resource */ : uint64_t a4gb; /* 0x3E - 0x45 Base of above 4GB MMIO Resource */ : uint64_t a4gs; /* 0x46 - 0x4D Length of above 4GB MMIO Resource */
Right now they are listed under "Miscellaneous" since these are specific to Intel SoCs, we probably […]
Yes Rizwan, we already created common NVS structure since CNL onwards [https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/common/block/...]
making it common since SKL and APL is little difficult hence left it as is without adding into common part
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent.c:
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/common/block/... PS4, Line 19: nclude <device/pci_ops.h>
required?
Done
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/common/block/... PS4, Line 82: gnvs->a4gb
should be gnvs->a4gs?
Done
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/skylake/inclu... File src/soc/intel/skylake/include/soc/iomap.h:
https://review.coreboot.org/c/coreboot/+/38125/4/src/soc/intel/skylake/inclu... PS4, Line 78: 0x2000000000
nit: 128*GiB here and everywhere would be nice
Yes, i also wished to do the same. But weird things happen and you can see compilation issue here, i believe its due to 32bit IASL compiler
https://qa.coreboot.org/job/coreboot-gerrit/113502/testReport/junit/board/ch...