Hello Kevin Chiu,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/47857
to review the following change.
Change subject: mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz ......................................................................
mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz
Enhance USB 2.0 M/B C0, DB C1 A1 port: HS DC Voltage Level(TXVREFTUNE0): 0x6 COMPDISTUNE(COMPDISTUNE0): 0x5
BUG=b:165209698 BRANCH=zork TEST=emerge-zork coreboot
Change-Id: I371e4295c2ee161096f0a277c0c649bf217269b2 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com --- M src/mainboard/google/zork/variants/dirinboz/overridetree.cb 1 file changed, 39 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/57/47857/1
diff --git a/src/mainboard/google/zork/variants/dirinboz/overridetree.cb b/src/mainboard/google/zork/variants/dirinboz/overridetree.cb index efd1dfc..7b63079 100644 --- a/src/mainboard/google/zork/variants/dirinboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/dirinboz/overridetree.cb @@ -21,6 +21,45 @@ register "telemetry_vddcr_soc_offset" = "167" # End : OPN Performance Configuration
+ # USB 2.0 strength + register "usb_2_port_tune_params[0]" = "{ + .com_pds_tune = 0x05, + .sq_rx_tune = 0x3, + .tx_fsls_tune = 0x3, + .tx_pre_emp_amp_tune = 0x03, + .tx_pre_emp_pulse_tune = 0x0, + .tx_rise_tune = 0x1, + .tx_vref_tune = 0x6, + .tx_hsxv_tune = 0x3, + .tx_res_tune = 0x01, + }" + + # USB 2.0 strength + register "usb_2_port_tune_params[2]" = "{ + .com_pds_tune = 0x05, + .sq_rx_tune = 0x3, + .tx_fsls_tune = 0x3, + .tx_pre_emp_amp_tune = 0x03, + .tx_pre_emp_pulse_tune = 0x0, + .tx_rise_tune = 0x1, + .tx_vref_tune = 0x6, + .tx_hsxv_tune = 0x3, + .tx_res_tune = 0x01, + }" + + # USB 2.0 strength + register "usb_2_port_tune_params[3]" = "{ + .com_pds_tune = 0x05, + .sq_rx_tune = 0x3, + .tx_fsls_tune = 0x3, + .tx_pre_emp_amp_tune = 0x03, + .tx_pre_emp_pulse_tune = 0x0, + .tx_rise_tune = 0x1, + .tx_vref_tune = 0x6, + .tx_hsxv_tune = 0x3, + .tx_res_tune = 0x01, + }" + # I2C2 for touchscreen and trackpad register "i2c[2]" = "{ .speed = I2C_SPEED_FAST,
Attention is currently required from: Martin Roth, Furquan Shaikh, Bhanu Prakash Maiya, Eric Peers, Felix Held. Kevin Chiu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47857 )
Change subject: mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz ......................................................................
Patch Set 2:
This change is ready for review.
Attention is currently required from: Kevin Chiu, Martin Roth, Furquan Shaikh, Bhanu Prakash Maiya, Eric Peers, Felix Held. Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47857 )
Change subject: mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz ......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/47857/comment/8c8a7591_93b16485 PS2, Line 11: COMPDISTUNE(COMPDISTUNE0): 0x7 Where did you get these from?
https://review.coreboot.org/c/coreboot/+/47857/comment/683a83bd_008737ec PS2, Line 15: TEST=emerge-zork coreboot How did you verify those?
Attention is currently required from: Kevin Chiu, Martin Roth, Furquan Shaikh, Bhanu Prakash Maiya, Eric Peers. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47857 )
Change subject: mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz ......................................................................
Patch Set 2: Code-Review+2
Attention is currently required from: Kevin Chiu, Furquan Shaikh, Bhanu Prakash Maiya, Paul Menzel, Eric Peers. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/47857 )
Change subject: mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz ......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/47857/comment/0092a507_33adec86 PS2, Line 11: COMPDISTUNE(COMPDISTUNE0): 0x7
Where did you get these from?
Looking at the bug, these were calculated through a very lengthy process starting with a failing eye diagram. The EEs and AMD calculated the needed changes from the default values which were verified through testing.
https://review.coreboot.org/c/coreboot/+/47857/comment/f2220325_ad2cb772 PS2, Line 15: TEST=emerge-zork coreboot
How did you verify those?
The values were tested empirically with an eye diagram mf the USB signals.
Martin Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/47857 )
Change subject: mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz ......................................................................
mb/google/zork: update USB 2.0 controller Lane Parameter for dirinboz
Enhance USB 2.0 M/B C0, DB C1 A1 port: HS DC Voltage Level(TXVREFTUNE0): 0xe COMPDISTUNE(COMPDISTUNE0): 0x7
BUG=b:165209698 BRANCH=zork TEST=emerge-zork coreboot
Change-Id: I371e4295c2ee161096f0a277c0c649bf217269b2 Signed-off-by: Kevin Chiu kevin.chiu@quantatw.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/47857 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Felix Held felix-coreboot@felixheld.de --- M src/mainboard/google/zork/variants/dirinboz/overridetree.cb 1 file changed, 39 insertions(+), 0 deletions(-)
Approvals: build bot (Jenkins): Verified Felix Held: Looks good to me, approved
diff --git a/src/mainboard/google/zork/variants/dirinboz/overridetree.cb b/src/mainboard/google/zork/variants/dirinboz/overridetree.cb index 3032984..6416a5c 100644 --- a/src/mainboard/google/zork/variants/dirinboz/overridetree.cb +++ b/src/mainboard/google/zork/variants/dirinboz/overridetree.cb @@ -21,6 +21,45 @@ register "telemetry_vddcr_soc_offset" = "167" # End : OPN Performance Configuration
+ # USB 2.0 strength + register "usb_2_port_tune_params[0]" = "{ + .com_pds_tune = 0x07, + .sq_rx_tune = 0x3, + .tx_fsls_tune = 0x3, + .tx_pre_emp_amp_tune = 0x03, + .tx_pre_emp_pulse_tune = 0x0, + .tx_rise_tune = 0x1, + .tx_vref_tune = 0xe, + .tx_hsxv_tune = 0x3, + .tx_res_tune = 0x01, + }" + + # USB 2.0 strength + register "usb_2_port_tune_params[2]" = "{ + .com_pds_tune = 0x07, + .sq_rx_tune = 0x3, + .tx_fsls_tune = 0x3, + .tx_pre_emp_amp_tune = 0x03, + .tx_pre_emp_pulse_tune = 0x0, + .tx_rise_tune = 0x1, + .tx_vref_tune = 0xe, + .tx_hsxv_tune = 0x3, + .tx_res_tune = 0x01, + }" + + # USB 2.0 strength + register "usb_2_port_tune_params[3]" = "{ + .com_pds_tune = 0x07, + .sq_rx_tune = 0x3, + .tx_fsls_tune = 0x3, + .tx_pre_emp_amp_tune = 0x03, + .tx_pre_emp_pulse_tune = 0x0, + .tx_rise_tune = 0x1, + .tx_vref_tune = 0xe, + .tx_hsxv_tune = 0x3, + .tx_res_tune = 0x01, + }" + # I2C2 for touchscreen and trackpad register "i2c[2]" = "{ .speed = I2C_SPEED_FAST,