Hello Chris Wang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/46041
to review the following change.
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
soc/amd/picasso: Remove xhci0_force_gen1 from soc config
To remvoe the xhci0_force_gen1 and use usb3_port_force_gen1 instead. The xhci0_force_gen1 is used for force all port on xhci0 to USB3 GEN1. Now variant can use the usb3_port_force_gen1 to custmzied which port it needs to limit.
BUG=b:167651308 BRANCH=zork TEST=Build, verify the USB3 speed in gen1
Signed-off-by: Chris Wang chris.wang@amd.corp-partner.google.com Change-Id: If5f0c1f22d8c98c4461f09d074bf082c340b14d9 --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 4 files changed, 1 insertion(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/46041/1
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb index 4004243..0346fd2 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb @@ -44,8 +44,6 @@ .timing = SD_EMMC_EMMC_HS400, }"
- register "xhci0_force_gen1" = "0" - register "has_usb2_phy_tune_params" = "1"
# Controller0 Port0 Default diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb index 8d475e9..f5cdc2f 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb @@ -44,8 +44,6 @@ .timing = SD_EMMC_EMMC_HS400, }"
- register "xhci0_force_gen1" = "0" - register "has_usb2_phy_tune_params" = "1"
# Controller0 Port0 Default diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index 0039479..2b85f11 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -169,8 +169,7 @@ SD_EMMC_EMMC_HS300, } timing; } emmc_config; - /* set xhci0 from gen2 to gen1 */ - uint8_t xhci0_force_gen1; + /* Force USB3 port to gen1, bit0 - controller0 Port0, bit1 - Port1 */ struct usb3_port_force_gen1 usb3_port_force_gen1;
diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index bacdbb0..1473d0b 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -105,7 +105,6 @@ /* each OC mapping in xhci_oc_pin_select is 4 bit per USB port */ ASSERT(2 * sizeof(scfg->xhci_oc_pin_select) >= USB_PORT_COUNT);
- scfg->xhci0_force_gen1 = cfg->xhci0_force_gen1; scfg->fch_usb_3_port_force_gen1 = cfg->usb3_port_force_gen1.usb3_port_force_gen1_en;
if (cfg->has_usb2_phy_tune_params) {
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46041 )
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
Patch Set 1:
(2 comments)
Some people would probably ask you to split this into two patches - one for the mainboard and another for the chipset changes. Personally, I don't think that's needed as it's just the removal of code that's no longer needed, but I'll leave that up to you.
https://review.coreboot.org/c/coreboot/+/46041/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46041/1//COMMIT_MSG@9 PS1, Line 9: remvoe remove
https://review.coreboot.org/c/coreboot/+/46041/1//COMMIT_MSG@11 PS1, Line 11: custmzied customize
Hello build bot (Jenkins), Furquan Shaikh, Marshall Dawson, Paul Menzel, Chris Wang, Aaron Durbin, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/46041
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
soc/amd/picasso: Remove xhci0_force_gen1 from soc config
To remove the xhci0_force_gen1 and use usb3_port_force_gen1 instead. The xhci0_force_gen1 is used for force all port on xhci0 to USB3 GEN1. Now variant can use the usb3_port_force_gen1 to customize which port it needs to limit.
BUG=b:167651308 BRANCH=zork TEST=Build, verify the USB3 speed in gen1
Signed-off-by: Chris Wang chris.wang@amd.corp-partner.google.com Change-Id: If5f0c1f22d8c98c4461f09d074bf082c340b14d9 --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 4 files changed, 1 insertion(+), 7 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/46041/2
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46041 )
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46041/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46041/1//COMMIT_MSG@9 PS1, Line 9: remvoe
remove
Done
https://review.coreboot.org/c/coreboot/+/46041/1//COMMIT_MSG@11 PS1, Line 11: custmzied
customize
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46041 )
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
Patch Set 2: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46041 )
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
Patch Set 4: Code-Review+2
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46041 )
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
Patch Set 4: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/46041 )
Change subject: soc/amd/picasso: Remove xhci0_force_gen1 from soc config ......................................................................
soc/amd/picasso: Remove xhci0_force_gen1 from soc config
To remove the xhci0_force_gen1 and use usb3_port_force_gen1 instead. The xhci0_force_gen1 is used for force all port on xhci0 to USB3 GEN1. Now variant can use the usb3_port_force_gen1 to customize which port it needs to limit.
BUG=b:167651308 BRANCH=zork TEST=Build, verify the USB3 speed in gen1
Signed-off-by: Chris Wang chris.wang@amd.corp-partner.google.com Change-Id: If5f0c1f22d8c98c4461f09d074bf082c340b14d9 Reviewed-on: https://review.coreboot.org/c/coreboot/+/46041 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Sam McNally sammc@google.com Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb M src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 4 files changed, 1 insertion(+), 7 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved Edward O'Callaghan: Looks good to me, approved Sam McNally: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb index ae712ee..cbb812d 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_dalboz.cb @@ -54,8 +54,6 @@ .init_khz_preset = 1, }"
- register "xhci0_force_gen1" = "0" - register "has_usb2_phy_tune_params" = "1"
# Controller0 Port0 Default diff --git a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb index 69179ec..7288d6e 100644 --- a/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb +++ b/src/mainboard/google/zork/variants/baseboard/devicetree_trembyle.cb @@ -54,8 +54,6 @@ .init_khz_preset = 1, }"
- register "xhci0_force_gen1" = "0" - register "has_usb2_phy_tune_params" = "1"
# Controller0 Port0 Default diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index f4233de..3098a81 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -201,8 +201,7 @@ */ uint16_t init_khz_preset; } emmc_config; - /* set xhci0 from gen2 to gen1 */ - uint8_t xhci0_force_gen1; + /* Force USB3 port to gen1, bit0 - controller0 Port0, bit1 - Port1 */ union usb3_force_gen1 usb3_port_force_gen1;
diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index 973bb87d4..a08a209 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -110,7 +110,6 @@ /* each OC mapping in xhci_oc_pin_select is 4 bit per USB port */ ASSERT(2 * sizeof(scfg->xhci_oc_pin_select) >= USB_PORT_COUNT);
- scfg->xhci0_force_gen1 = cfg->xhci0_force_gen1; scfg->fch_usb_3_port_force_gen1 = cfg->usb3_port_force_gen1.usb3_port_force_gen1_en;
if (cfg->has_usb2_phy_tune_params) {
-
chris wang (Code Review) -
Edward O'Callaghan (Code Review)
-
Felix Held (Code Review)
-
Martin Roth (Code Review)
-
Sam McNally (Code Review)