Attention is currently required from: chris wang. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50240 )
Change subject: soc/amd/picasso: add UPD for USB3 phy setting adjsut ......................................................................
Patch Set 4:
(4 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50240/comment/c56b8a4d_a93cc7c2 PS4, Line 7: adjsut *adjust*
https://review.coreboot.org/c/coreboot/+/50240/comment/fd2679b5_8d933a23 PS4, Line 12: 1. RX_EQ_DELTA_IQ_OVRD_VAL- Override value for rx_eq_delta_iq. Range 0-0xF : 2. RX_EQ_DELTA_IQ_OVRD_EN - Enable override value for rx_eq_delta_iq. Range 0-0x1 Nit: Split these to 2 lines each?
File src/soc/amd/picasso/fsp_params.c:
https://review.coreboot.org/c/coreboot/+/50240/comment/29dabeb9_2dc03f2a PS4, Line 130: cfg->usb3_phy_en Don't you want to copy this too? It's in the UPD.
https://review.coreboot.org/c/coreboot/+/50240/comment/0e1dc428_d5d6eb1d PS4, Line 131: memcpy(&scfg->usb_3_port0_phy_tune, &cfg->usb3_phy_tune_params[0], : sizeof(scfg->usb_3_port0_phy_tune[0])); Is there a case where we might want to set the values for only one port or is it required that all 4 be updated at a time? Should the usb3_phy_en value be a bitfield?
I realize it would have to be the same in agesa.
-
Martin Roth (Code Review)