Hello Chris Wang,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to review the following change.
Change subject: soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port
add upd usb3_port_force_gen1 for support USB3 port to gen1
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/1
diff --git a/src/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index e3da255..9868220 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -168,7 +168,7 @@ USB_OC_PIN_5 = 0x5, USB_OC_NONE = 0xf, } usb_port_overcurrent_pin[USB_PORT_COUNT]; - + uint32_t usb3_port_force_gen1; /* The array index is the general purpose PCIe clock output number. */ enum gpp_clk_req_setting gpp_clk_config[GPP_CLK_OUTPUT_COUNT]; }; diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index b21f237..e36ebd0 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -106,6 +106,7 @@ 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;
if (cfg->has_usb2_phy_tune_params) { for (i = 0; i < FSPS_UPD_USB2_PORT_COUNT; i++) {
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port ......................................................................
Patch Set 1:
(4 comments)
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@7 PS1, Line 7: soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port Maybe:
Add UPD to force USB3 to Gen1 by port
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@9 PS1, Line 9: add upd usb3_port_force_gen1 for support USB3 port to gen1 Please start sentences with a capital letter and finish with a dot/period.
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@13 PS1, Line 13: TEST=Build,verify the USB3 speed in gen1 Please add a space after the comma.
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h@... PS1, Line 171: uint32_t usb3_port_force_gen1; Keep the blank line before the comment?
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@7 PS1, Line 7: soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port
Maybe: […]
Note that `UPD` should be spelled in uppercase
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h@... PS1, Line 171: uint32_t The FSP UPD seems to be an uint8_t, so this should be uint8_t as well.
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#2).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
add upd usb3_port_force_gen1 for support USB3 port to gen1
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 2 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/2
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#3).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
add UPD usb3_port_force_gen1 for support USB3 port force to gen1
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/3
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#4).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 2 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/4
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 4:
(3 comments)
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@7 PS1, Line 7: soc/amd/picasso: Add Upd for support force USB3 to Gen1 by port
Note that `UPD` should be spelled in uppercase
Done
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h@... PS1, Line 171: uint32_t usb3_port_force_gen1;
Keep the blank line before the comment?
Done
https://review.coreboot.org/c/coreboot/+/45333/1/src/soc/amd/picasso/chip.h@... PS1, Line 171: uint32_t
The FSP UPD seems to be an uint8_t, so this should be uint8_t as well.
Done
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h@... PS4, Line 172: usb3_port_force_gen1 Can you add some documentation on this? Or better yet, is it possible to make it a bit field?
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h@... PS4, Line 172: usb3_port_force_gen1
Can you add some documentation on this? Or better yet, is it possible to make it a bit field?
Ya. this doesn't seem per port unless it's a bitfield.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h@... PS4, Line 158: uint8_t xhci0_force_gen1; How is this different than the newly added one?
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#5).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 14 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/5
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h@... PS4, Line 158: uint8_t xhci0_force_gen1;
How is this different than the newly added one?
for xhci0_force_gen1,it will set all port on xhci0 to gen1 but usb3_port_force_gen1 will set it by individual port.
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h@... PS4, Line 172: usb3_port_force_gen1
Ya. this doesn't seem per port unless it's a bitfield.
Done
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 5:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h@... PS5, Line 41: typedef no typedefs please.
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h@... PS5, Line 47: xhci1_port0 Dali doesn't have an xhci1 right?
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#6).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 16 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/6
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#7).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 17 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/7
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 7:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h@... PS5, Line 41: typedef
no typedefs please.
Done
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h@... PS5, Line 47: xhci1_port0
Dali doesn't have an xhci1 right?
yes, I add a comment for noticed the USB usage.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 7: Code-Review+1
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/7/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/7/src/soc/amd/picasso/chip.h@... PS7, Line 173: uint8_t xhci0_force_gen1; You should probably comment as to what this does, and the field below is likely the correct one to use for policy modifications. If anything, I think we should remove xhci0_force_gen1 from chip.h.
Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/7/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/7/src/soc/amd/picasso/chip.h@... PS7, Line 173: uint8_t xhci0_force_gen1;
You should probably comment as to what this does, and the field below is likely the correct one to u […]
+1 some documentation here would be helpful.
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#8).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/8
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/7/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/7/src/soc/amd/picasso/chip.h@... PS7, Line 173: uint8_t xhci0_force_gen1;
+1 some documentation here would be helpful.
Add comment for xhci0_force_gen1 and usb3_port_froce_gen1.
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 8:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/8/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/8/src/soc/amd/picasso/chip.h@... PS8, Line 173: xhci0_force_gen1 Shouldn't this be dropped now?
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 8:
(5 comments)
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@9 PS1, Line 9: add upd usb3_port_force_gen1 for support USB3 port to gen1
Please start sentences with a capital letter and finish with a dot/period.
Done
https://review.coreboot.org/c/coreboot/+/45333/1//COMMIT_MSG@13 PS1, Line 13: TEST=Build,verify the USB3 speed in gen1
Please add a space after the comma.
Done
https://review.coreboot.org/c/coreboot/+/45333/8/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/8/src/soc/amd/picasso/chip.h@... PS8, Line 173: xhci0_force_gen1
Shouldn't this be dropped now?
I can separate a CL to remove this and use the usb3_port_force_gen1 instead.
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/5/src/soc/amd/picasso/chip.h@... PS5, Line 47: xhci1_port0
yes, I add a comment for noticed the USB usage.
Done
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/4/src/soc/amd/picasso/chip.h@... PS4, Line 158: uint8_t xhci0_force_gen1;
for xhci0_force_gen1,it will set all port on xhci0 to gen1 but usb3_port_force_gen1 will set it by i […]
Done
Hello build bot (Jenkins), Furquan Shaikh, Chris Wang, Keith Tzeng, Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#9).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 19 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/9
Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 9:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h@... PS9, Line 175: usb3_port_force_gen1 usb3_port_force_gen1; Could you rename either the struct or the variable?
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 9:
I had the same thought about the identical structure and variable names. If you change it in your daytime, Felix might approve it before Martin or I see it.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 9:
(1 comment)
I had the same thought about the identical structure and variable names. If you change it in your daytime, Felix might approve it before Martin or I see it.
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h@... PS9, Line 49: xhci1_port0 Did you check that this one works? Maybe I'm missing it, but I only see AGESA checking bits 0-3.
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Marshall Dawson, Chris Wang, Keith Tzeng, Aaron Durbin, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#10).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 17 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/10
chris wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 10:
(2 comments)
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h@... PS9, Line 49: xhci1_port0
Did you check that this one works? Maybe I'm missing it, but I only see AGESA checking bits 0-3.
yes, it only disable to port3. so remove the xhci1_port_0.
https://review.coreboot.org/c/coreboot/+/45333/9/src/soc/amd/picasso/chip.h@... PS9, Line 175: usb3_port_force_gen1 usb3_port_force_gen1;
Could you rename either the struct or the variable?
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 10:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/10/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/10/src/soc/amd/picasso/chip.h... PS10, Line 41: struct __packed usb3_force_gen1 { I wonder if it's needed to wrap the union in a struct that only contains the union or if the outermost struct can be omitted
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Marshall Dawson, Chris Wang, Keith Tzeng, Aaron Durbin, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#11).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 15 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/11
Chris Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 11:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/10/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/10/src/soc/amd/picasso/chip.h... PS10, Line 41: struct __packed usb3_force_gen1 {
I wonder if it's needed to wrap the union in a struct that only contains the union or if the outermo […]
Done
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 11: Code-Review+2
Hello build bot (Jenkins), Martin Roth, Furquan Shaikh, Marshall Dawson, Chris Wang, Keith Tzeng, Aaron Durbin, Felix Held,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/45333
to look at the new patch set (#12).
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 15 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/45333/12
Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 12: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 12: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/12/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/12/src/soc/amd/picasso/chip.h... PS12, Line 41: usb3_force_gen1 feel free to make this as a follow up as its a bit of a nit but I think you could do a typedef here and have `usb3_force_gen1_t`.
Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 12: Code-Review+2
Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
Patch Set 12:
(1 comment)
https://review.coreboot.org/c/coreboot/+/45333/12/src/soc/amd/picasso/chip.h File src/soc/amd/picasso/chip.h:
https://review.coreboot.org/c/coreboot/+/45333/12/src/soc/amd/picasso/chip.h... PS12, Line 41: usb3_force_gen1
feel free to make this as a follow up as its a bit of a nit but I think you could do a typedef here […]
.
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/45333 )
Change subject: soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port ......................................................................
soc/amd/picasso: Add UPD for support force USB3 to Gen1 by port
Add UPD usb3_port_force_gen1 for support USB3 port force to gen1.
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: I896c185988c3ea5dbdd72957b363ebdaa2747cff Reviewed-on: https://review.coreboot.org/c/coreboot/+/45333 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de Reviewed-by: Edward O'Callaghan quasisec@chromium.org Reviewed-by: Sam McNally sammc@google.com --- M src/soc/amd/picasso/chip.h M src/soc/amd/picasso/fsp_params.c 2 files changed, 15 insertions(+), 1 deletion(-)
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/soc/amd/picasso/chip.h b/src/soc/amd/picasso/chip.h index a39549e..f4233de 100644 --- a/src/soc/amd/picasso/chip.h +++ b/src/soc/amd/picasso/chip.h @@ -37,6 +37,17 @@ uint8_t tx_res_tune; };
+/* force USB3 port to gen1, bit0 - controller0 Port0, bit1 - Port1, etc */ +union __packed usb3_force_gen1 { + struct { + uint8_t xhci0_port0:1; + uint8_t xhci0_port1:1; + uint8_t xhci0_port2:1; + uint8_t xhci0_port3:1; + } ports; + uint8_t usb3_port_force_gen1_en; +}; + #define USB_PORT_COUNT 6
enum sd_emmc_driver_strength { @@ -190,8 +201,10 @@ */ 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;
uint8_t has_usb2_phy_tune_params; struct usb2_phy_tune usb_2_port_tune_params[USB_PORT_COUNT]; diff --git a/src/soc/amd/picasso/fsp_params.c b/src/soc/amd/picasso/fsp_params.c index 95e691d..973bb87d4 100644 --- a/src/soc/amd/picasso/fsp_params.c +++ b/src/soc/amd/picasso/fsp_params.c @@ -111,6 +111,7 @@ 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) { for (i = 0; i < FSPS_UPD_USB2_PORT_COUNT; i++) {