John Zhao has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/31131
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
soc/intel/apollolake: Sync fsp upd structure update
FSP 2.0.9 provides UPD interface to adjust integrated filter value, usb3 LDO and pmic vdd2 voltage. Change coreboot upd structure to sync with fsp 2.0.9 release.
BUG=b:123398358 CQ-DEPEND=CL:*817128 TEST=Verified yorp boots to kernel.
Change-Id: I3d17dfbe58bdc5222378459723da8e9ac0573510 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h 3 files changed, 57 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/31131/1
diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index afbb45c..dcd5d93 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -623,6 +623,21 @@ * Options to disable XHCI Link Compliance Mode. */ silconfig->DisableComplianceMode = cfg->DisableComplianceMode; + + /* + * Options to change USB3 ModPhy setting for Integrated Filter value. + */ + silconfig->ModPhyIfValue = cfg->ModPhyIfValue; + + /* + * Options to bump USB3 LDO voltage with 40mv. + */ + silconfig->ModPhyVoltageBump = cfg->ModPhyVoltageBump; + + /* + * Options to adjust PMIC Vdd2 voltage. + */ + silconfig->PmicVdd2Voltage = cfg->PmicVdd2Voltage; #endif }
diff --git a/src/soc/intel/apollolake/chip.h b/src/soc/intel/apollolake/chip.h index b9c9dc5..5e9df67 100644 --- a/src/soc/intel/apollolake/chip.h +++ b/src/soc/intel/apollolake/chip.h @@ -168,6 +168,26 @@ * 0:FALSE(Default), 1:True. */ uint8_t DisableComplianceMode; + + /* Options to change USB3 ModPhy setting for the Integrated Filter (IF) + * value. Default is 0 to not changing IF value (default: 0x12). Set + * value to change IF value for successful Chrome OS installation. + */ + uint8_t ModPhyIfValue; + + /* Options to bump USB3 LDO voltage with 40mv. Default is FALSE to not + * increasing LDO voltage. Set TRUE to increase LDO voltage with 40mv. + * 0:FALSE (default), 1:True. + */ + uint8_t ModPhyVoltageBump; + + /* Options to adjust PMIC Vdd2 voltage (default 1.2v) . Default is 0 to + * not adjusting Vdd2 voltage. Set value for Vdd2 voltage with 1.24v. + * PMIC Buck 5 control register configuration - IPC Configuration + * Upd for changing Vdd2 voltage configuration : I2C_Slave_Address + * (31:24) + Register_Offset (23:16) + OR Value (15:8) + AND Value (7:0) + */ + uint32_t PmicVdd2Voltage; };
typedef struct soc_intel_apollolake_config config_t; diff --git a/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h b/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h index 18a43e2..97a40b6 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h +++ b/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h @@ -1,6 +1,6 @@ /** @file
-Copyright (c) 2018, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: @@ -1721,7 +1721,25 @@ **/ UINT32 PmicPmcIpcCtrl;
-/** Offset 0x03AF +/** Offset 0x03AF - ModPhyIfValue + Upd To modify the Integrated Filter (IF) value as 0x12(Default) for WIN and 0x16 + for Chrome +**/ + UINT8 ModPhyIfValue; + +/** Offset 0x03B0 - ModPhyVoltageBump + ModPhyVoltageBump. 1: enable, 0: disable + $EN_DIS +**/ + UINT8 ModPhyVoltageBump; + +/** Offset 0x03B1 - Vdd2 Voltage configuration + Upd for changing Vdd2 Voltage configuration : I2C_Slave_Address (31:23) + Register_Offset + (23:16) + OR Value (15:8) + AND Value (7:0) +**/ + UINT32 PmicVdd2Voltage; + +/** Offset 0x03B5 **/ UINT8 ReservedFspsUpd[1]; } FSP_S_CONFIG; @@ -1792,9 +1810,9 @@ **/ FSP_S_CONFIG FspsConfig;
-/** Offset 0x03B0 +/** Offset 0x03B6 **/ - UINT8 UnusedUpdSpace7[16]; + UINT8 UnusedUpdSpace7[10];
/** Offset 0x03C0 **/
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/31131/1/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/1/src/soc/intel/apollolake/chip.h@173 PS1, Line 173: Set : * value to change IF value for successful Chrome OS installation Let's not put mainboard specific comment in common code. Instead can you add details to indicate what are valid values for ModPhyIfValue?
https://review.coreboot.org/#/c/31131/1/src/soc/intel/apollolake/chip.h@185 PS1, Line 185: Set value for Vdd2 voltage with 1.24v. What are the valid values?
Hello Aaron Durbin, Patrick Rudolph, Justin TerAvest, Shamile Khan, build bot (Jenkins), Hannah Williams, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31131
to look at the new patch set (#2).
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
soc/intel/apollolake: Sync fsp upd structure update
FSP 2.0.9 provides UPD interface to adjust integrated filter value, usb3 LDO and pmic vdd2 voltage. Change coreboot upd structure to sync with fsp 2.0.9 release.
BUG=b:123398358 CQ-DEPEND=CL:*817128 TEST=Verified yorp boots to kernel.
Change-Id: I3d17dfbe58bdc5222378459723da8e9ac0573510 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h 3 files changed, 53 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/31131/2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@173 PS2, Line 173: * value. Default is 0 to not changing default IF value. What are the valid values? And what do the values mean?
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@178 PS2, Line 178: . by 40mV?
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@184 PS2, Line 184: * the PMIC Vdd2 voltage. What are the valid values?
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 2:
(3 comments)
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@173 PS2, Line 173: * value. Default is 0 to not changing default IF value.
What are the valid values? And what do the values mean?
The change is sync to fsp 2.0.9 upd update. Can we specify value to mainboard code? Final value will be shared along with validation results. Note: fsp currently applies IF value 0x12.
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@178 PS2, Line 178: .
by 40mV?
Can we specify value to mainboard code? Note: If set TRUE, the usb3 LDO is increased by 40mV.
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@184 PS2, Line 184: * the PMIC Vdd2 voltage.
What are the valid values?
Can we specify value to mainboard code? Note: PMIC vdd2 is by default with 1.2v. PmicVdd2Voltage can be configured to adjust vdd2 through BUCK5_VID[3:2]: 00=1.0v, 01=1.15v, 10=1.24v, 11=1.20v(default). It is intended to up to 1.24v as
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@173 PS2, Line 173: * value. Default is 0 to not changing default IF value.
The change is sync to fsp 2.0.9 upd update. […]
I mean what are the values that any mainboard user can set for these fields? Mainboard can have an explanation of any specific value that is chosen. But these fields should provide some help on what is the set of valid values that can be used.
Hello Aaron Durbin, Patrick Rudolph, Justin TerAvest, Shamile Khan, build bot (Jenkins), Hannah Williams, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31131
to look at the new patch set (#3).
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
soc/intel/apollolake: Sync fsp upd structure update
FSP 2.0.9 provides UPD interface to adjust integrated filter value, usb3 LDO and pmic vdd2 voltage. Change coreboot upd structure to sync with fsp 2.0.9 release.
BUG=b:123398358 CQ-DEPEND=CL:*817128 TEST=Verified yorp boots to kernel.
Change-Id: I3d17dfbe58bdc5222378459723da8e9ac0573510 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h 3 files changed, 57 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/31131/3
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/2/src/soc/intel/apollolake/chip.h@173 PS2, Line 173: * value. Default is 0 to not changing default IF value.
I mean what are the values that any mainboard user can set for these fields? Mainboard can have an e […]
updated.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 3: Code-Review+1
(1 comment)
https://review.coreboot.org/#/c/31131/3/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/3/src/soc/intel/apollolake/chip.h@174 PS3, Line 174: 0x00 to 0xff How do the value 0x1 - 0xff impact the IF value?
Hello Aaron Durbin, Patrick Rudolph, Justin TerAvest, Shamile Khan, build bot (Jenkins), Hannah Williams, Furquan Shaikh,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/31131
to look at the new patch set (#4).
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
soc/intel/apollolake: Sync fsp upd structure update
FSP 2.0.9 provides UPD interface to adjust integrated filter value, usb3 LDO and pmic vdd2 voltage. Change coreboot upd structure to sync with fsp 2.0.9 release.
BUG=b:123398358 CQ-DEPEND=CL:*817128 TEST=Verified yorp boots to kernel.
Change-Id: I3d17dfbe58bdc5222378459723da8e9ac0573510 Signed-off-by: John Zhao john.zhao@intel.com --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h 3 files changed, 57 insertions(+), 4 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/31/31131/4
John Zhao has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/#/c/31131/3/src/soc/intel/apollolake/chip.h File src/soc/intel/apollolake/chip.h:
https://review.coreboot.org/#/c/31131/3/src/soc/intel/apollolake/chip.h@174 PS3, Line 174: 0x00 to 0xff
How do the value 0x1 - 0xff impact the IF value?
fsp will take the input value as IF value.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
soc/intel/apollolake: Sync fsp upd structure update
FSP 2.0.9 provides UPD interface to adjust integrated filter value, usb3 LDO and pmic vdd2 voltage. Change coreboot upd structure to sync with fsp 2.0.9 release.
BUG=b:123398358 CQ-DEPEND=CL:*817128 TEST=Verified yorp boots to kernel.
Change-Id: I3d17dfbe58bdc5222378459723da8e9ac0573510 Signed-off-by: John Zhao john.zhao@intel.com Reviewed-on: https://review.coreboot.org/c/31131 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Furquan Shaikh furquan@google.com --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/chip.h M src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h 3 files changed, 57 insertions(+), 4 deletions(-)
Approvals: build bot (Jenkins): Verified Furquan Shaikh: Looks good to me, approved
diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index a3ce4838..cddfe44 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -639,6 +639,21 @@ * Options to disable XHCI Link Compliance Mode. */ silconfig->DisableComplianceMode = cfg->DisableComplianceMode; + + /* + * Options to change USB3 ModPhy setting for Integrated Filter value. + */ + silconfig->ModPhyIfValue = cfg->ModPhyIfValue; + + /* + * Options to bump USB3 LDO voltage with 40mv. + */ + silconfig->ModPhyVoltageBump = cfg->ModPhyVoltageBump; + + /* + * Options to adjust PMIC Vdd2 voltage. + */ + silconfig->PmicVdd2Voltage = cfg->PmicVdd2Voltage; #endif }
diff --git a/src/soc/intel/apollolake/chip.h b/src/soc/intel/apollolake/chip.h index b9c9dc5..6c2404a 100644 --- a/src/soc/intel/apollolake/chip.h +++ b/src/soc/intel/apollolake/chip.h @@ -168,6 +168,26 @@ * 0:FALSE(Default), 1:True. */ uint8_t DisableComplianceMode; + + /* Options to change USB3 ModPhy setting for the Integrated Filter (IF) + * value. Default is 0 to not changing default IF value (0x12). Set + * value with the range from 0x01 to 0xff to change IF value. + */ + uint8_t ModPhyIfValue; + + /* Options to bump USB3 LDO voltage. Default is FALSE to not increasing + * LDO voltage. Set TRUE to increase LDO voltage with 40mV. + * 0:FALSE (default), 1:True. + */ + uint8_t ModPhyVoltageBump; + + /* Options to adjust PMIC Vdd2 voltage. Default is 0 to not adjusting + * the PMIC Vdd2 default voltage 1.20v. Upd for changing Vdd2 Voltage + * configuration: I2C_Slave_Address (31:23) + Register_Offset (23:16) + * + OR Value (15:8) + AND Value (7:0) through BUCK5_VID[3:2]: + * 00=1.10v, 01=1.15v, 10=1.24v, 11=1.20v (default). + */ + uint32_t PmicVdd2Voltage; };
typedef struct soc_intel_apollolake_config config_t; diff --git a/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h b/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h index 18a43e2..97a40b6 100644 --- a/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h +++ b/src/vendorcode/intel/fsp/fsp2_0/glk/FspsUpd.h @@ -1,6 +1,6 @@ /** @file
-Copyright (c) 2018, Intel Corporation. All rights reserved.<BR> +Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met: @@ -1721,7 +1721,25 @@ **/ UINT32 PmicPmcIpcCtrl;
-/** Offset 0x03AF +/** Offset 0x03AF - ModPhyIfValue + Upd To modify the Integrated Filter (IF) value as 0x12(Default) for WIN and 0x16 + for Chrome +**/ + UINT8 ModPhyIfValue; + +/** Offset 0x03B0 - ModPhyVoltageBump + ModPhyVoltageBump. 1: enable, 0: disable + $EN_DIS +**/ + UINT8 ModPhyVoltageBump; + +/** Offset 0x03B1 - Vdd2 Voltage configuration + Upd for changing Vdd2 Voltage configuration : I2C_Slave_Address (31:23) + Register_Offset + (23:16) + OR Value (15:8) + AND Value (7:0) +**/ + UINT32 PmicVdd2Voltage; + +/** Offset 0x03B5 **/ UINT8 ReservedFspsUpd[1]; } FSP_S_CONFIG; @@ -1792,9 +1810,9 @@ **/ FSP_S_CONFIG FspsConfig;
-/** Offset 0x03B0 +/** Offset 0x03B6 **/ - UINT8 UnusedUpdSpace7[16]; + UINT8 UnusedUpdSpace7[10];
/** Offset 0x03C0 **/
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31131 )
Change subject: soc/intel/apollolake: Sync fsp upd structure update ......................................................................
Patch Set 5:
Just food for thought (I guess Intel probably has to revise their release processes for a while longer, given that the ability to publish trouble-free FSP releases on github is still rather new):
It may be best to see such updates go to https://github.com/IntelFsp/FSP. We automatically mirror that repo on coreboot.org, ensuring availability. With that, you only need to update the 3rdparty/fsp submodule pointer and use the files there instead of keeping copies of FSP headers in the coreboot repo synchronized.
By standardizing on such an approach every FSP user benefits from updates and everybody can be sure that their FSP binary (that they get from IntelFSP/FSP) matches the headers (since they get them from the same place).
-
Furquan Shaikh (Code Review) -
John Zhao (Code Review)
-
Patrick Georgi (Code Review)