Aamir Bohra has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Allow tigerlake SOC to compile ......................................................................
soc/intel/Kconfig: Allow tigerlake SOC to compile
Change-Id: I25463f1b7b5d8242da3decf3e7a7ca54c699d467 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/37554/1
diff --git a/src/soc/intel/Kconfig b/src/soc/intel/Kconfig index 1eebeb6..e8935b9 100644 --- a/src/soc/intel/Kconfig +++ b/src/soc/intel/Kconfig @@ -8,6 +8,7 @@ source "src/soc/intel/quark/Kconfig" source "src/soc/intel/skylake/Kconfig" source "src/soc/intel/icelake/Kconfig" +source "src/soc/intel/tigerlake/Kconfig"
# Load common config source "src/soc/intel/common/Kconfig"
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Allow tigerlake SOC to compile ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37554/1/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37554/1/src/soc/intel/Kconfig@1 PS1, Line 1: # Load all chipsets : source "src/soc/intel/apollolake/Kconfig" : source "src/soc/intel/baytrail/Kconfig" : source "src/soc/intel/braswell/Kconfig" : source "src/soc/intel/broadwell/Kconfig" : source "src/soc/intel/cannonlake/Kconfig" : source "src/soc/intel/denverton_ns/Kconfig" : source "src/soc/intel/quark/Kconfig" : source "src/soc/intel/skylake/Kconfig" : source "src/soc/intel/icelake/Kconfig" : source "src/soc/intel/tigerlake/Kconfig" : : # Load common config : source "src/soc/intel/common/Kconfig" Can we just change this to: source "src/soc/intel/*/Kconfig"
Hello Patrick Rudolph, Karthik Ramasubramanian, Subrata Banik, Nick Vaccaro, Wonkyu Kim, Maulik V Vaghela, Ravishankar Sarawadi, Rizwan Qureshi, V Sowmya, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37554
to look at the new patch set (#2).
Change subject: soc/intel/Kconfig: Allow to load all Intel SOC Kconfigs ......................................................................
soc/intel/Kconfig: Allow to load all Intel SOC Kconfigs
Currently for each new SOC a Kconfig entry has to be added to use the SOC Kconfig. Inlcuding all SOC Kconfigs would take care of it inherently.
Change-Id: I25463f1b7b5d8242da3decf3e7a7ca54c699d467 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/Kconfig 1 file changed, 2 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/37554/2
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Allow to load all Intel SOC Kconfigs ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37554/1/src/soc/intel/Kconfig File src/soc/intel/Kconfig:
https://review.coreboot.org/c/coreboot/+/37554/1/src/soc/intel/Kconfig@1 PS1, Line 1: # Load all chipsets : source "src/soc/intel/apollolake/Kconfig" : source "src/soc/intel/baytrail/Kconfig" : source "src/soc/intel/braswell/Kconfig" : source "src/soc/intel/broadwell/Kconfig" : source "src/soc/intel/cannonlake/Kconfig" : source "src/soc/intel/denverton_ns/Kconfig" : source "src/soc/intel/quark/Kconfig" : source "src/soc/intel/skylake/Kconfig" : source "src/soc/intel/icelake/Kconfig" : source "src/soc/intel/tigerlake/Kconfig" : : # Load common config : source "src/soc/intel/common/Kconfig"
Can we just change this to: […]
Done, updated the commit message as well
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Allow to load all Intel SOC Kconfigs ......................................................................
Patch Set 2: Code-Review-1
Order might matter here, that `common/Kconfig` is sourced last is likely intentional.
Usually this is done so that the common code can provide a default and SoCs can override it (first `default` line encountered by Kconfig takes precedence).
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Allow to load all Intel SOC Kconfigs ......................................................................
Patch Set 2:
Patch Set 2: Code-Review-1
Order might matter here, that `common/Kconfig` is sourced last is likely intentional.
Usually this is done so that the common code can provide a default and SoCs can override it (first `default` line encountered by Kconfig takes precedence).
Understood, I could see the delta in .config file with PS#1 and PS#2, will revise the patch
Hello Patrick Rudolph, Karthik Ramasubramanian, Subrata Banik, Nick Vaccaro, Wonkyu Kim, Maulik V Vaghela, Ravishankar Sarawadi, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37554
to look at the new patch set (#3).
Change subject: soc/intel/Kconfig: Load tigerlake SOC Kconfig ......................................................................
soc/intel/Kconfig: Load tigerlake SOC Kconfig
Change-Id: I25463f1b7b5d8242da3decf3e7a7ca54c699d467 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/37554/3
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load tigerlake SOC Kconfig ......................................................................
Patch Set 3:
Patch Set 2: Code-Review-1
Order might matter here, that `common/Kconfig` is sourced last is likely intentional.
Usually this is done so that the common code can provide a default and SoCs can override it (first `default` line encountered by Kconfig takes precedence).
That is a valid point.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load tigerlake SOC Kconfig ......................................................................
Patch Set 3:
Patch Set 3:
Patch Set 2: Code-Review-1
Order might matter here, that `common/Kconfig` is sourced last is likely intentional.
Usually this is done so that the common code can provide a default and SoCs can override it (first `default` line encountered by Kconfig takes precedence).
That is a valid point.
What do you think about: Rename: src/soc/intel/common/Kconfig to src/soc/intel/common/Kconfig.common Change: All specific SoC sources to "source src/soc/intel/*/Kconfig" Change: Common source to "source src/soc/intel/common/Kconfig.common"
It just gets rid of the work to add every single Intel SoC to be sourced separately.
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load tigerlake SOC Kconfig ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37554/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37554/3//COMMIT_MSG@7 PS3, Line 7: tigerlake Tiger Lake
Hello Patrick Rudolph, Karthik Ramasubramanian, Subrata Banik, Nick Vaccaro, Wonkyu Kim, Maulik V Vaghela, Ravishankar Sarawadi, Rizwan Qureshi, V Sowmya, build bot (Jenkins), Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/37554
to look at the new patch set (#4).
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
soc/intel/Kconfig: Load Tiger Lake SOC Kconfig
Change-Id: I25463f1b7b5d8242da3decf3e7a7ca54c699d467 Signed-off-by: Aamir Bohra aamir.bohra@intel.com --- M src/soc/intel/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/37554/4
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/37554/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37554/3//COMMIT_MSG@7 PS3, Line 7: tigerlake
Tiger Lake
Done
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
Patch Set 4: Code-Review+2
Order might matter here, that `common/Kconfig` is sourced last is likely intentional.
Usually this is done so that the common code can provide a default and SoCs can override it (first `default` line encountered by Kconfig takes precedence).
That is a valid point.
What do you think about: Rename: src/soc/intel/common/Kconfig to src/soc/intel/common/Kconfig.common Change: All specific SoC sources to "source src/soc/intel/*/Kconfig" Change: Common source to "source src/soc/intel/common/Kconfig.common"
It just gets rid of the work to add every single Intel SoC to be sourced separately.
sgtm
Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
Patch Set 4: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
Patch Set 4: Code-Review+2
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/37554 )
Change subject: soc/intel/Kconfig: Load Tiger Lake SOC Kconfig ......................................................................
soc/intel/Kconfig: Load Tiger Lake SOC Kconfig
Change-Id: I25463f1b7b5d8242da3decf3e7a7ca54c699d467 Signed-off-by: Aamir Bohra aamir.bohra@intel.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/37554 Reviewed-by: Nico Huber nico.h@gmx.de Reviewed-by: Wonkyu Kim wonkyu.kim@intel.com Reviewed-by: Furquan Shaikh furquan@google.com Reviewed-by: Maulik V Vaghela maulik.v.vaghela@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Nico Huber: Looks good to me, approved Furquan Shaikh: Looks good to me, approved Maulik V Vaghela: Looks good to me, approved Wonkyu Kim: Looks good to me, approved
diff --git a/src/soc/intel/Kconfig b/src/soc/intel/Kconfig index 1eebeb6..e8935b9 100644 --- a/src/soc/intel/Kconfig +++ b/src/soc/intel/Kconfig @@ -8,6 +8,7 @@ source "src/soc/intel/quark/Kconfig" source "src/soc/intel/skylake/Kconfig" source "src/soc/intel/icelake/Kconfig" +source "src/soc/intel/tigerlake/Kconfig"
# Load common config source "src/soc/intel/common/Kconfig"
-
Aamir Bohra (Code Review) -
Furquan Shaikh (Code Review)
-
Maulik V Vaghela (Code Review)
-
Nico Huber (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Menzel (Code Review)
-
Wonkyu Kim (Code Review)