Angel Pons has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
mb/prodrive/hermes: Update USB 2.0 settings
Test results show that USB signals look better with these settings.
Change-Id: Ib0dafab88d8dcc05388b724f6a7183c13ac64934 Signed-off-by: Angel Pons th3fanbus@gmail.com --- A src/mainboard/prodrive/hermes/devicetree.h M src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb 2 files changed, 30 insertions(+), 13 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/48694/1
diff --git a/src/mainboard/prodrive/hermes/devicetree.h b/src/mainboard/prodrive/hermes/devicetree.h new file mode 100644 index 0000000..d4742b6 --- /dev/null +++ b/src/mainboard/prodrive/hermes/devicetree.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef __MAINBOARD_DEVICETREE_DEF_H__ +#define __MAINBOARD_DEVICETREE_DEF_H__ + +#include <soc/usb.h> + +#define HERMES_USB2_CONFIG(pin) { \ + .enable = 1, \ + .ocpin = (pin), \ + .tx_bias = USB2_BIAS_0MV, \ + .tx_emp_enable = USB2_DE_EMP_ON_PRE_EMP_ON, \ + .pre_emp_bias = USB2_BIAS_28P15MV, \ + .pre_emp_bit = USB2_FULL_BIT_PRE_EMP, \ +} + +#endif /* !__MAINBOARD_DEVICETREE_DEF_H__ */ diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb b/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb index b543a78..9c66f65 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb +++ b/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb @@ -91,37 +91,37 @@ # USB OC0: RP1 register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC0)" register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC0)" - register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC0)" - register "usb2_ports[1]" = "USB2_PORT_TYPE_C(OC0)" + register "usb2_ports[0]" = "HERMES_USB2_CONFIG(OC0)" + register "usb2_ports[1]" = "HERMES_USB2_CONFIG(OC0)"
# USB OC1: RP2 register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC1)" register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC1)" - register "usb2_ports[2]" = "USB2_PORT_TYPE_C(OC1)" - register "usb2_ports[3]" = "USB2_PORT_TYPE_C(OC1)" + register "usb2_ports[2]" = "HERMES_USB2_CONFIG(OC1)" + register "usb2_ports[3]" = "HERMES_USB2_CONFIG(OC1)"
# USB OC2: Internal Header CN_USB3_HDR register "usb3_ports[4]" = "USB3_PORT_DEFAULT(OC2)" register "usb3_ports[5]" = "USB3_PORT_DEFAULT(OC2)" - register "usb2_ports[4]" = "USB2_PORT_TYPE_C(OC2)" - register "usb2_ports[5]" = "USB2_PORT_TYPE_C(OC2)" + register "usb2_ports[4]" = "HERMES_USB2_CONFIG(OC2)" + register "usb2_ports[5]" = "HERMES_USB2_CONFIG(OC2)"
# USB 2.0 # USB OC3: Internal Header USB2_HDR1 - register "usb2_ports[6]" = "USB2_PORT_MID(OC3)" - register "usb2_ports[7]" = "USB2_PORT_MID(OC3)" + register "usb2_ports[6]" = "HERMES_USB2_CONFIG(OC3)" + register "usb2_ports[7]" = "HERMES_USB2_CONFIG(OC3)"
# USB OC4: Internal Header USB2_HDR2 - register "usb2_ports[8]" = "USB2_PORT_MID(OC4)" - register "usb2_ports[9]" = "USB2_PORT_MID(OC4)" + register "usb2_ports[8]" = "HERMES_USB2_CONFIG(OC4)" + register "usb2_ports[9]" = "HERMES_USB2_CONFIG(OC4)"
# USB OC5-7: not connected # BMC - register "usb2_ports[10]" = "USB2_PORT_MID(OC_SKIP)" + register "usb2_ports[10]" = "HERMES_USB2_CONFIG(OC_SKIP)" # piggy-back - register "usb2_ports[12]" = "USB2_PORT_MID(OC_SKIP)" + register "usb2_ports[12]" = "HERMES_USB2_CONFIG(OC_SKIP)" # M2 key E - register "usb2_ports[13]" = "USB2_PORT_MID(OC_SKIP)" + register "usb2_ports[13]" = "HERMES_USB2_CONFIG(OC_SKIP)"
# Thermal register "tcc_offset" = "1" # TCC of 99C
Hello build bot (Jenkins), Patrick Rudolph, Christian Walter,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48694
to look at the new patch set (#2).
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
mb/prodrive/hermes: Update USB 2.0 settings
Test results show that USB signals look better with these settings. Yes, there's a macro in the devicetree now. All ports use the same settings except for the overcurrent pin, so this avoids redundancy.
Change-Id: Ib0dafab88d8dcc05388b724f6a7183c13ac64934 Signed-off-by: Angel Pons th3fanbus@gmail.com --- M src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb 1 file changed, 27 insertions(+), 23 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/94/48694/2
Paul Menzel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
Patch Set 2: Code-Review+1
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48694/2/src/mainboard/prodrive/herm... File src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/48694/2/src/mainboard/prodrive/herm... PS2, Line 109: [10] = HERMES_USB2_CONFIG(OC_SKIP), /* BMC */ : [11] = USB2_PORT_EMPTY, : [12] = HERMES_USB2_CONFIG(OC_SKIP), /* piggy-back */ : [13] = HERMES_USB2_CONFIG(OC_SKIP), /* M.2 key E */ Q: Do these need to be changed as well? I feel `USB2_PORT_MID` makes more sense here
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48694/2/src/mainboard/prodrive/herm... File src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/48694/2/src/mainboard/prodrive/herm... PS2, Line 109: [10] = HERMES_USB2_CONFIG(OC_SKIP), /* BMC */ : [11] = USB2_PORT_EMPTY, : [12] = HERMES_USB2_CONFIG(OC_SKIP), /* piggy-back */ : [13] = HERMES_USB2_CONFIG(OC_SKIP), /* M.2 key E */
Q: Do these need to be changed as well? I feel `USB2_PORT_MID` makes more sense here
yes, all ports should use HERMES_USB2_CONFIG.
Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
Patch Set 2: Code-Review+2
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/48694/2/src/mainboard/prodrive/herm... File src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb:
https://review.coreboot.org/c/coreboot/+/48694/2/src/mainboard/prodrive/herm... PS2, Line 109: [10] = HERMES_USB2_CONFIG(OC_SKIP), /* BMC */ : [11] = USB2_PORT_EMPTY, : [12] = HERMES_USB2_CONFIG(OC_SKIP), /* piggy-back */ : [13] = HERMES_USB2_CONFIG(OC_SKIP), /* M.2 key E */
yes, all ports should use HERMES_USB2_CONFIG.
Ack, thanks
Angel Pons has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48694 )
Change subject: mb/prodrive/hermes: Update USB 2.0 settings ......................................................................
mb/prodrive/hermes: Update USB 2.0 settings
Test results show that USB signals look better with these settings. Yes, there's a macro in the devicetree now. All ports use the same settings except for the overcurrent pin, so this avoids redundancy.
Change-Id: Ib0dafab88d8dcc05388b724f6a7183c13ac64934 Signed-off-by: Angel Pons th3fanbus@gmail.com Reviewed-on: https://review.coreboot.org/c/coreboot/+/48694 Reviewed-by: Paul Menzel paulepanter@users.sourceforge.net Reviewed-by: Patrick Rudolph siro@das-labor.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb 1 file changed, 27 insertions(+), 23 deletions(-)
Approvals: build bot (Jenkins): Verified Paul Menzel: Looks good to me, but someone else must approve Patrick Rudolph: Looks good to me, approved
diff --git a/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb b/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb index b543a78..4d35c47 100644 --- a/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb +++ b/src/mainboard/prodrive/hermes/variants/baseboard/overridetree.cb @@ -85,43 +85,47 @@ register "PcieClkSrcClkReq[4]" = "4" # M2_M_CLK_REQ_n register "PcieClkSrcClkReq[5]" = "5" # M2_E_CLK_REQ_n
+ # USB OC5-7: not connected + register "usb2_ports" = "{ + +#define HERMES_USB2_CONFIG(pin) { \ + .enable = 1, \ + .ocpin = (pin), \ + .tx_bias = USB2_BIAS_0MV, \ + .tx_emp_enable = USB2_DE_EMP_ON_PRE_EMP_ON, \ + .pre_emp_bias = USB2_BIAS_28P15MV, \ + .pre_emp_bit = USB2_FULL_BIT_PRE_EMP, \ +} + [0] = HERMES_USB2_CONFIG(OC0), /* USB3 rear panel 1 */ + [1] = HERMES_USB2_CONFIG(OC0), + [2] = HERMES_USB2_CONFIG(OC1), /* USB3 rear panel 2 */ + [3] = HERMES_USB2_CONFIG(OC1), + [4] = HERMES_USB2_CONFIG(OC2), /* USB3 internal header CN_USB3_HDR */ + [5] = HERMES_USB2_CONFIG(OC2), + [6] = HERMES_USB2_CONFIG(OC3), /* USB2 internal header USB2_HDR1 */ + [7] = HERMES_USB2_CONFIG(OC3), + [8] = HERMES_USB2_CONFIG(OC4), /* USB2 internal header USB2_HDR1 */ + [9] = HERMES_USB2_CONFIG(OC4), + [10] = HERMES_USB2_CONFIG(OC_SKIP), /* BMC */ + [11] = USB2_PORT_EMPTY, + [12] = HERMES_USB2_CONFIG(OC_SKIP), /* piggy-back */ + [13] = HERMES_USB2_CONFIG(OC_SKIP), /* M.2 key E */ + }" + # USB Config 2.0/3.0 # Enumeration starts at 0 # USB 3.0 # USB OC0: RP1 register "usb3_ports[0]" = "USB3_PORT_DEFAULT(OC0)" register "usb3_ports[1]" = "USB3_PORT_DEFAULT(OC0)" - register "usb2_ports[0]" = "USB2_PORT_TYPE_C(OC0)" - register "usb2_ports[1]" = "USB2_PORT_TYPE_C(OC0)"
# USB OC1: RP2 register "usb3_ports[2]" = "USB3_PORT_DEFAULT(OC1)" register "usb3_ports[3]" = "USB3_PORT_DEFAULT(OC1)" - register "usb2_ports[2]" = "USB2_PORT_TYPE_C(OC1)" - register "usb2_ports[3]" = "USB2_PORT_TYPE_C(OC1)"
# USB OC2: Internal Header CN_USB3_HDR register "usb3_ports[4]" = "USB3_PORT_DEFAULT(OC2)" register "usb3_ports[5]" = "USB3_PORT_DEFAULT(OC2)" - register "usb2_ports[4]" = "USB2_PORT_TYPE_C(OC2)" - register "usb2_ports[5]" = "USB2_PORT_TYPE_C(OC2)" - - # USB 2.0 - # USB OC3: Internal Header USB2_HDR1 - register "usb2_ports[6]" = "USB2_PORT_MID(OC3)" - register "usb2_ports[7]" = "USB2_PORT_MID(OC3)" - - # USB OC4: Internal Header USB2_HDR2 - register "usb2_ports[8]" = "USB2_PORT_MID(OC4)" - register "usb2_ports[9]" = "USB2_PORT_MID(OC4)" - - # USB OC5-7: not connected - # BMC - register "usb2_ports[10]" = "USB2_PORT_MID(OC_SKIP)" - # piggy-back - register "usb2_ports[12]" = "USB2_PORT_MID(OC_SKIP)" - # M2 key E - register "usb2_ports[13]" = "USB2_PORT_MID(OC_SKIP)"
# Thermal register "tcc_offset" = "1" # TCC of 99C
-
Angel Pons (Code Review) -
Patrick Rudolph (Code Review)
-
Paul Menzel (Code Review)