Change in coreboot[master]: soc/intel/common/systemagent: Change PCIEX_LENGTH to PCIEX_LENGTH_MIB
Usha P has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40235 )
Change subject: soc/intel/common/systemagent: Change PCIEX_LENGTH to PCIEX_LENGTH_MIB ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/c/coreboot/+/40235/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/40235/1//COMMIT_MSG@16 PS1, Line 16: a new Kconfig option
not relevant after latest patch set
Done
https://review.coreboot.org/c/coreboot/+/40235/1//COMMIT_MSG@22 PS1, Line 22: Rename CONFIG_PCIEX_LENGTH to CONFIG_PCIEX_LENGTH_MIB to avoid ugly : typecasting.
Can we split this into 2 changes - first for changing length to MiB and second for adding more lengt […]
Yes Furquan. Modified this patch to hold the length to MiB changes and https://review.coreboot.org/c/coreboot/+/40363/1 CL is created to accommodate additional length values.
https://review.coreboot.org/c/coreboot/+/40235/1/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/Kconfig:
https://review.coreboot.org/c/coreboot/+/40235/1/src/soc/intel/common/block/... PS1, Line 12: SA_PCIEX_LENGTH
I meant wouldn't you have to change this: https://review.coreboot.org/cgit/coreboot. […]
Latest patchset includes the SoC specific changes needed.
https://review.coreboot.org/c/coreboot/+/40235/1/src/soc/intel/common/block/... PS1, Line 25: config PCIEX_LENGTH_4096MB : bool : : config PCIEX_LENGTH_2048MB : bool : : config PCIEX_LENGTH_1024MB : bool : : config PCIEX_LENGTH_512MB : bool : : config PCIEX_LENGTH_256MB : bool : : config PCIEX_LENGTH_128MB : bool : : config PCIEX_LENGTH_64MB : bool
Shouldn't this be a choice? We wouldn't want multiple ones to be selected at the same time, right?
Taken care in https://review.coreboot.org/c/coreboot/+/40363/
https://review.coreboot.org/c/coreboot/+/40235/3/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/Kconfig:
https://review.coreboot.org/c/coreboot/+/40235/3/src/soc/intel/common/block/... PS3, Line 14: default 0x10000000 if (PCIEX_LENGTH_256MB) : default 0x8000000 if (PCIEX_LENGTH_128MB) : default 0x4000000 if (PCIEX_LENGTH_64MB) : default 0x10000000
u missed this https://review.coreboot.org/c/coreboot/+/40235/2.. […]
Done
https://review.coreboot.org/c/coreboot/+/40235/1/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/40235/1/src/soc/intel/common/block/... PS1, Line 46: /* TODO- include condition check for New SoC implememtation */
How do you plan to do that?
Planning to include condition like #if (CONFIG_SA_PCIEXBAR_LENGTH_BIT_WIDTH == 3). Still exploring to see which is the best way to put a check for New SoC.
https://review.coreboot.org/c/coreboot/+/40235/3/src/soc/intel/common/block/... File src/soc/intel/common/block/systemagent/systemagent_early.c:
https://review.coreboot.org/c/coreboot/+/40235/3/src/soc/intel/common/block/... PS3, Line 31: case 256 * MiB:
Done
-
Usha P (Code Review)