Ronak Kanabar has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/30917
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CML,WHL and CFL is not using midfixed option in SaGv so keeping it for CNL and removing it for CML,WHL and CFL.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/1
diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index 6517b9e..11cddc9 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -105,7 +105,9 @@ enum { SaGv_Disabled, SaGv_FixedLow, +#if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE_PCH_H) SaGv_FixedMid, +#endif SaGv_FixedHigh, SaGv_Enabled, } SaGv;
Ronak Kanabar has removed Patrick Rudolph from this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Removed reviewer Patrick Rudolph.
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG@9 PS1, Line 9: CML What does CML stand for?
Hello Naresh Solanki, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#2).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG@9 PS1, Line 9: CML
What does CML stand for?
Got it figured out, thanks for your explanation.
https://review.coreboot.org/#/c/30917/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/2//COMMIT_MSG@9 PS2, Line 9: CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for : CNL only Does CNL use the midfixed option or not?
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/30917/2/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/2/src/soc/intel/cannonlake/chip.h@109 PS2, Line 109: SaGv_FixedMid, this means that SaGv is passed different numbers depending on compile time options. Is that deliberate (ie. Are there different FSPs, is the FSP parsing these numbers differently depending on the SoC it runs on, or is the SoC interpreting the number differently depending on its type)?
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 2:
(3 comments)
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG@9 PS1, Line 9: CML
What does CML stand for?
typo mistake
https://review.coreboot.org/#/c/30917/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/2//COMMIT_MSG@9 PS2, Line 9: CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for : CNL only
Does CNL use the midfixed option or not?
yes CNL is using midfixed so keeping for it and removing for others.
https://review.coreboot.org/#/c/30917/2/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/2/src/soc/intel/cannonlake/chip.h@109 PS2, Line 109: SaGv_FixedMid,
this means that SaGv is passed different numbers depending on compile time options. […]
yes so CNL soc can train at 3 different frequency so CNL FSP UPD SaGv can have all this 5 valuse, 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled
but WHL and CFL can train at 2 different frequency so SaGv UPD don't have the option of fixedMid. in that case we need to remove FixedMid option. It is depends on FSP and ultimately on Soc support.
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#3).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/3
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG@9 PS1, Line 9: CML
typo mistake
why CML added here ?
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/1//COMMIT_MSG@9 PS1, Line 9: CML
why CML added here ?
saw latest patch set, understood the purpose. no issue
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 3: Code-Review+1
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/30917/3/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/3/src/soc/intel/cannonlake/chip.h@104 PS3, Line 104: 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled This would not hold true for CFL/WHL, you can remove this as options are clear from enum declaration or add CFL/WHL specific options too.
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 3:
(1 comment)
Patch Set 2:
(3 comments)
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/30917/3/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/3/src/soc/intel/cannonlake/chip.h@104 PS3, Line 104: 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled
This would not hold true for CFL/WHL, you can remove this as options are clear from enum declaration […]
I will change description.
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#4).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/4
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 4:
(7 comments)
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@103 PS4, Line 103: * When enabled memory will be training at two or three different frequencies. line over 80 characters
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@104 PS4, Line 104: * for CNL options are as following trailing whitespace
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@104 PS4, Line 104: * for CNL options are as following code indent should use tabs where possible
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@105 PS4, Line 105: * 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled trailing whitespace
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@106 PS4, Line 106: * for others options are as following trailing whitespace
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@106 PS4, Line 106: * for others options are as following code indent should use tabs where possible
https://review.coreboot.org/#/c/30917/4/src/soc/intel/cannonlake/chip.h@107 PS4, Line 107: * 0:Disabled, 1:FixedLow, 2:FixedHigh, 3:Enabled*/ code indent should use tabs where possible
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#5).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 7 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/5
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 5:
(4 comments)
https://review.coreboot.org/#/c/30917/5/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/5/src/soc/intel/cannonlake/chip.h@103 PS5, Line 103: * When enabled memory will be training at two or three different frequencies. line over 80 characters
https://review.coreboot.org/#/c/30917/5/src/soc/intel/cannonlake/chip.h@104 PS5, Line 104: * for CNL options are as following trailing whitespace
https://review.coreboot.org/#/c/30917/5/src/soc/intel/cannonlake/chip.h@105 PS5, Line 105: * 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled trailing whitespace
https://review.coreboot.org/#/c/30917/5/src/soc/intel/cannonlake/chip.h@106 PS5, Line 106: * for others options are as following trailing whitespace
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#6).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 8 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/6
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 6: Code-Review+1
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 6:
(1 comment)
https://review.coreboot.org/#/c/30917/6/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/6/src/soc/intel/cannonlake/chip.h@106 PS6, Line 106: others can you please name the other platforms here.
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 6:
(1 comment)
Patch Set 5:
(4 comments)
https://review.coreboot.org/#/c/30917/6/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/6/src/soc/intel/cannonlake/chip.h@106 PS6, Line 106: others
can you please name the other platforms here.
okay
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, Lijian Zhao, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#7).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/7
Aamir Bohra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 7: Code-Review+1
Maulik V Vaghela has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 7: Code-Review+1
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/#/c/30917/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/7//COMMIT_MSG@9 PS7, Line 9: keeping keep
https://review.coreboot.org/#/c/30917/7//COMMIT_MSG@10 PS7, Line 10: removing remove
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, Lijian Zhao, build bot (Jenkins),
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#8).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keep it for CNL only and remove it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/8
Hello Naresh Solanki, Patrick Rudolph, Subrata Banik, Aamir Bohra, Maulik V Vaghela, Rizwan Qureshi, build bot (Jenkins), Lijian Zhao,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/30917
to look at the new patch set (#9).
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com --- M src/soc/intel/cannonlake/chip.h 1 file changed, 7 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/17/30917/9
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 9:
(2 comments)
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/30917/7//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30917/7//COMMIT_MSG@9 PS7, Line 9: keeping
keep
done
https://review.coreboot.org/#/c/30917/7//COMMIT_MSG@10 PS7, Line 10: removing
remove
done
Lijian Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 9: Code-Review+2
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 9: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
soc/intel/cannonlake: Change in SaGv options
CNL,WHL and CFL all are not using midfixed option in SaGv so keeping it for CNL only and removing it for others.
Change-Id: I754515c2f8e249479c603872c61ac9a006e962ff Signed-off-by: Ronak Kanabar ronak.kanabar@intel.com Reviewed-on: https://review.coreboot.org/c/30917 Reviewed-by: Lijian Zhao lijian.zhao@intel.com Reviewed-by: Subrata Banik subrata.banik@intel.com Reviewed-by: Aamir Bohra aamir.bohra@intel.com Reviewed-by: Maulik V Vaghela maulik.v.vaghela@intel.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/soc/intel/cannonlake/chip.h 1 file changed, 7 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Lijian Zhao: Looks good to me, approved Subrata Banik: Looks good to me, approved Aamir Bohra: Looks good to me, but someone else must approve Maulik V Vaghela: Looks good to me, but someone else must approve
diff --git a/src/soc/intel/cannonlake/chip.h b/src/soc/intel/cannonlake/chip.h index 947cd65..a877ec1 100644 --- a/src/soc/intel/cannonlake/chip.h +++ b/src/soc/intel/cannonlake/chip.h @@ -100,12 +100,18 @@ uint16_t FreqSaGvMid;
/* System Agent dynamic frequency support. Only effects ULX/ULT CPUs. + * for CNL options are as following + * When enabled memory will be training at three different frequencies. + * 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled + * for WHL/CFL options are as following * When enabled memory will be training at two different frequencies. - * 0:Disabled, 1:FixedLow, 2:FixedMid, 3:FixedHigh, 4:Enabled */ + * 0:Disabled, 1:FixedLow, 2:FixedHigh, 3:Enabled*/ enum { SaGv_Disabled, SaGv_FixedLow, +#if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE) SaGv_FixedMid, +#endif SaGv_FixedHigh, SaGv_Enabled, } SaGv;
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h@112 PS10, Line 112: #if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE) This check is not correct. SOC_INTEL_CANNONLAKE is selected for all: CNL, CFL, WHL. So, the values for CFL and WHL are not going to be correct.
Reference: https://review.coreboot.org/cgit/coreboot.git/tree/src/soc/intel/cannonlake/...
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h@112 PS10, Line 112: #if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE)
This check is not correct. SOC_INTEL_CANNONLAKE is selected for all: CNL, CFL, WHL. […]
Definitely. Would it make sense to add a Kconfig option to distinguish between CNL/CFL/WHL common and CNL-specific stuff?
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h@112 PS10, Line 112: #if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE)
Definitely. […]
You can just check #if !IS_ENABLED(CONFIG_SOC_INTEL_COFFEELAKE)
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h@112 PS10, Line 112: #if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE)
You can just check #if !IS_ENABLED(CONFIG_SOC_INTEL_COFFEELAKE)
Ack, refer to CB:31095
Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 10:
(1 comment)
please hold the other CL to merge, i will ask Ronak to look why he made this change.
Ronak Kanabar has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30917 )
Change subject: soc/intel/cannonlake: Change in SaGv options ......................................................................
Patch Set 10:
(1 comment)
Patch Set 10:
(1 comment)
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h File src/soc/intel/cannonlake/chip.h:
https://review.coreboot.org/#/c/30917/10/src/soc/intel/cannonlake/chip.h@112 PS10, Line 112: #if IS_ENABLED(CONFIG_SOC_INTEL_CANNONLAKE)
Ack, refer to CB:31095
At this point we can use this as CB:31095 but I think we should have different Kconfig options for all SOC if all 3 SOC are default selecting CNL CPU then it is not good.
-
Aamir Bohra (Code Review) -
Angel Pons (Code Review)
-
build bot (Jenkins) (Code Review)
-
Furquan Shaikh (Code Review)
-
Lijian Zhao (Code Review)
-
Maulik V Vaghela (Code Review)
-
Patrick Georgi (Code Review)
-
Ronak Kanabar (Code Review)
-
Subrata Banik (Code Review)