Change in coreboot[master]: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place include/device/pci_mmio_cfg.h now has a compile time error if MMCONF_BASE_ADDRESS is not defined. This requires soc/intel target to explicitly set this in the SOC dir, instead of potentially using a possibly wrong common default. Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/skylake/Kconfig 5 files changed, 16 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/35884/1 diff --git a/src/soc/intel/apollolake/Kconfig b/src/soc/intel/apollolake/Kconfig index a1d3c07..24a4b7a 100644 --- a/src/soc/intel/apollolake/Kconfig +++ b/src/soc/intel/apollolake/Kconfig @@ -128,6 +128,10 @@ TPM part is conntected on Fast SPI interface, but the LPC MMIO TPM transactions are decoded and serialized over the SPI interface. +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config PCR_BASE_ADDRESS hex default 0xd0000000 diff --git a/src/soc/intel/cannonlake/Kconfig b/src/soc/intel/cannonlake/Kconfig index c1fda95..aeef75c 100644 --- a/src/soc/intel/cannonlake/Kconfig +++ b/src/soc/intel/cannonlake/Kconfig @@ -109,6 +109,10 @@ select FSP_T_XIP if FSP_CAR select HECI_DISABLE_USING_SMM if !SOC_INTEL_COFFEELAKE && !SOC_INTEL_WHISKEYLAKE && !SOC_INTEL_COMETLAKE +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config DCACHE_RAM_BASE default 0xfef00000 diff --git a/src/soc/intel/common/block/systemagent/Kconfig b/src/soc/intel/common/block/systemagent/Kconfig index 1222573..d7619a0 100644 --- a/src/soc/intel/common/block/systemagent/Kconfig +++ b/src/soc/intel/common/block/systemagent/Kconfig @@ -3,10 +3,6 @@ help Intel Processor common System Agent support -config MMCONF_BASE_ADDRESS - hex - default 0xe0000000 - config SA_PCIEX_LENGTH hex default 0x10000000 if (PCIEX_LENGTH_256MB) diff --git a/src/soc/intel/denverton_ns/Kconfig b/src/soc/intel/denverton_ns/Kconfig index 2aadcae..d12e4e3 100644 --- a/src/soc/intel/denverton_ns/Kconfig +++ b/src/soc/intel/denverton_ns/Kconfig @@ -91,6 +91,10 @@ int default 16 +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config PCR_BASE_ADDRESS hex default 0xfd000000 diff --git a/src/soc/intel/skylake/Kconfig b/src/soc/intel/skylake/Kconfig index 901e5f9..a9b8aaa 100644 --- a/src/soc/intel/skylake/Kconfig +++ b/src/soc/intel/skylake/Kconfig @@ -120,6 +120,10 @@ hex default 0x200000 +config MMCONF_BASE_ADDRESS + hex + default 0xe0000000 + config DCACHE_RAM_BASE hex default 0xfef00000 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-MessageType: newchange

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@10 PS1, Line 10: This requires soc/intel target to : explicitly set this in the SOC dir I see the sanity, but don't follow conclusion, why is it require by "this"? https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@12 PS1, Line 12: wrong How can it be wrong? does the FSP binary depend on a specific location? -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Tue, 08 Oct 2019 12:56:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@10 PS1, Line 10: This requires soc/intel target to : explicitly set this in the SOC dir
I see the sanity, but don't follow conclusion, why is it require by […] It's a bit like the overuse of __weak in soc/intel to make stuff build without being correct. Requiring things to de explicitly done in the soc dir could avoid that? I'll try to better state that.
https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@12 PS1, Line 12: wrong
How can it be wrong? does the FSP binary depend on a specific location?
The integration guide recommends (I looked at kabylake) against using non default values and if FSP-T is used it needs to match what is programmed in that UPD, so I think it sufficiently safe to say there are values that should not be tried out. -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Tue, 08 Oct 2019 13:28:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 1: (2 comments) https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@10 PS1, Line 10: This requires soc/intel target to : explicitly set this in the SOC dir
It's a bit like the overuse of __weak in soc/intel to make stuff build without being correct. […] To be clear, I'm just bikeshedding the relation to the previous sentence: if the fact that you added the compile time error would really require this change, Jenkins wouldn't have agreed on the original change :-p
What I read between the lines is roughly that "[To be really useful] this requires [...]". https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@12 PS1, Line 12: wrong
How can it be wrong? does the FSP binary depend on a specific […] I agree but still wonder: With a change like this, that has no technical requirement, how can we prevent that it's not turned back. e.g. somebody sees there are 8 out of 9 platforms that use the same value and puts it back into common/...
I don't like comments, but in this case it could help to add something like Needs to be what individual FSP versions expect. Keep this in the individual soc/ directory. -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Tue, 08 Oct 2019 13:44:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment

David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 2: Code-Review+1 (1 comment) https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG Commit Message: https://review.coreboot.org/c/coreboot/+/35884/1//COMMIT_MSG@12 PS1, Line 12: wrong
I agree but still wonder: With a change like this, that has no technical […] Agreed, SOC specific FSP-T will set a value for this, and no guarantee that all SOCs will use the same value.
-- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Thu, 31 Oct 2019 15:58:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Arthur Heymans <arthur@aheymans.xyz> Comment-In-Reply-To: Nico Huber <nico.h@gmx.de> Gerrit-MessageType: comment

Vanny E has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Tue, 05 Nov 2019 16:08:02 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 2: off-topic: Is there easy way to replace Name(PCBA, CONFIG_MMCONF_BASE_ADDRESS) in asl? Surely, there must be way to pass runtime C variable value in there instead? -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-Comment-Date: Tue, 05 Nov 2019 16:16:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 2:
Patch Set 2:
off-topic: Is there easy way to replace Name(PCBA, CONFIG_MMCONF_BASE_ADDRESS) in asl?
Surely, there must be way to pass runtime C variable value in there instead?
We either build the tables at runtime or pull the value from gnvs (assuming Name() allows that). -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Aaron Durbin <adurbin@chromium.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 06 Nov 2019 16:54:17 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanny E <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Kyösti Mälkki <kyosti.malkki@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 06 Nov 2019 16:54:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes has uploaded a new patch set (#3) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place include/device/pci_mmio_cfg.h now has a compile time error if MMCONF_BASE_ADDRESS is not defined. This requires soc/intel target to explicitly set this in the SOC dir, instead of potentially using a possibly wrong common default. Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/denverton_ns/Kconfig M src/soc/intel/skylake/Kconfig 4 files changed, 16 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/35884/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset

Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 3: (1 comment) https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/denverton_ns/... File src/soc/intel/denverton_ns/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/denverton_ns/... PS3, Line 45: config MMCONF_BASE_ADDRESS : hex : default 0xe0000000 See this. -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 22 Jul 2020 21:59:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

Andrey Petrov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 3: Code-Review+2 thank you for doing this -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 22 Jul 2020 22:18:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 3: (3 comments) https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/apollolake/Kc... File src/soc/intel/apollolake/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/apollolake/Kc... PS3, Line 84: SOC_INTEL_COMMON_BLOCK_SA This already defines MMCONF_BASE_ADDRESS https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/cannonlake/Kc... PS3, Line 98: SOC_INTEL_COMMON_BLOCK_SA This already defines MMCONF_BASE_ADDRESS https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/skylake/Kconf... File src/soc/intel/skylake/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/skylake/Kconf... PS3, Line 63: SOC_INTEL_COMMON_BLOCK_SA This already defines MMCONF_BASE_ADDRESS -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Wed, 22 Jul 2020 22:24:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 3: (4 comments) https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/apollolake/Kc... File src/soc/intel/apollolake/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/apollolake/Kc... PS3, Line 84: SOC_INTEL_COMMON_BLOCK_SA
This already defines MMCONF_BASE_ADDRESS Done
https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/cannonlake/Kc... File src/soc/intel/cannonlake/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/cannonlake/Kc... PS3, Line 98: SOC_INTEL_COMMON_BLOCK_SA
This already defines MMCONF_BASE_ADDRESS Done
https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/denverton_ns/... File src/soc/intel/denverton_ns/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/denverton_ns/... PS3, Line 45: config MMCONF_BASE_ADDRESS : hex : default 0xe0000000
See this. Done
https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/skylake/Kconf... File src/soc/intel/skylake/Kconfig: https://review.coreboot.org/c/coreboot/+/35884/3/src/soc/intel/skylake/Kconf... PS3, Line 63: SOC_INTEL_COMMON_BLOCK_SA
This already defines MMCONF_BASE_ADDRESS Done
-- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 23 Jul 2020 05:54:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus@gmail.com> Comment-In-Reply-To: Aaron Durbin <adurbin@chromium.org> Gerrit-MessageType: comment

HAOUAS Elyes has uploaded a new patch set (#4) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place include/device/pci_mmio_cfg.h now has a compile time error if MMCONF_BASE_ADDRESS is not defined. This requires soc/intel target to explicitly set this in the SOC dir, instead of potentially using a possibly wrong common default. Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Signed-off-by: Arthur Heymans <arthur@aheymans.xyz> Signed-off-by: Elyes HAOUAS <ehaouas@noos.fr> --- M src/soc/intel/apollolake/Kconfig M src/soc/intel/cannonlake/Kconfig M src/soc/intel/common/block/systemagent/Kconfig M src/soc/intel/skylake/Kconfig 4 files changed, 12 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/84/35884/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-MessageType: newpatchset

David Guckian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Angel Pons <th3fanbus@gmail.com> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 23 Jul 2020 08:04:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 4: Code-Review-1 No. It doesn't make sense. -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 23 Jul 2020 08:24:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 4:
Patch Set 4: Code-Review-1
No. It doesn't make sense.
not sure to understand anyway: src/soc/intel/icelake/Kconfig will select SOC_INTEL_COMMON_BLOCK_SA but MMCONF_BASE_ADDRESS = 0xc0000000 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 23 Jul 2020 08:48:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Patch Set 4:
Patch Set 4:
Patch Set 4: Code-Review-1
No. It doesn't make sense.
not sure to understand
anyway: src/soc/intel/icelake/Kconfig will select SOC_INTEL_COMMON_BLOCK_SA but MMCONF_BASE_ADDRESS = 0xc0000000
See the reason why `soc/intel/common/Kconfig.common` is named like that in "soc/intel/Kconfig": the common Kconfig options will be picked up last 😊 -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: HAOUAS Elyes <ehaouas@noos.fr> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@users.sourceforge.net> Gerrit-Comment-Date: Thu, 23 Jul 2020 09:09:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

Arthur Heymans has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35884 ) Change subject: soc/intel/Kconfig: Move MMCONF_BASE_ADDRESS out of a common place ...................................................................... Abandoned Fixed in master. -- To view, visit https://review.coreboot.org/c/coreboot/+/35884 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcfea1718cc36ae16b491604786c26c6ed320f06 Gerrit-Change-Number: 35884 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org> Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus@gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur@aheymans.xyz> Gerrit-Reviewer: David Guckian <d.guckian20@gmail.com> Gerrit-Reviewer: David Guckian <david.guckian@intel.com> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio@intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> Gerrit-CC: Nico Huber <nico.h@gmx.de> Gerrit-CC: Paul Menzel <paulepanter@mailbox.org> Gerrit-MessageType: abandon
participants (9)
-
Aaron Durbin (Code Review)
-
Andrey Petrov (Code Review)
-
Angel Pons (Code Review)
-
Arthur Heymans (Code Review)
-
David Guckian (Code Review)
-
HAOUAS Elyes (Code Review)
-
Kyösti Mälkki (Code Review)
-
Nico Huber (Code Review)
-
Vanny E (Code Review)