[PATCH 0/2] arm: mvebu: a38x: improve USB3 serdes configuration

This patchset improves the USB3 serdes configuration for the Armada 38x SoC from Marvell. The changes are based on the version from Marvell found here: https://github.com/MarvellEmbeddedProcessors/u-boot-marvell
Stefan Eichenberger (2): arm: mvebu: a38x: serdes: fix serdes config for USB3 arm: mvebu: a38x: serdes: Improve USB3 electrical configuration
.../serdes/a38x/high_speed_env_spec.c | 38 ++++++++++++++----- arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h | 2 + 2 files changed, 31 insertions(+), 9 deletions(-)

The electrical serdes configuration for USB3 expects an array as data argument. For USB3 the second value is used (see data_arr_idx = USB3 = 1). However, because only one value is inside the array mv_seq_exec is accessing an invalid element and the serdes is configured wrongly.
This wrong initialization is leading to an unreliable detection mechanism for some USB3 devices. We were able to reproduce the issue regularly with an LTE modem from Sierra Wireless (SM7455) where it was not detected as USB3 device in 1/3 of all tests.
This commit fixes the issue by setting data_arr_idx to 0. This is the same value as the original U-Boot from Marvell is using. There it is called FIRST_CELL which is a define for 0. See: https://github.com/MarvellEmbeddedProcessors/u-boot-marvell commit 56f963ce4c ("fix: serdes: a38x, a39x: Fix USB3 serdes DB initialization")
Signed-off-by: Stefan Eichenberger eichest@gmail.com Signed-off-by: René Straub rene.straub@netmodule.com --- arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index d2bc3ab25c..11b5824232 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1204,7 +1204,7 @@ int hws_serdes_seq_db_init(void) sizeof(usb3_electrical_config_serdes_rev2_params) / sizeof(struct op_params); } - serdes_seq_db[USB3_ELECTRICAL_CONFIG_SEQ].data_arr_idx = USB3; + serdes_seq_db[USB3_ELECTRICAL_CONFIG_SEQ].data_arr_idx = 0;
/* USB3_TX_CONFIG_SEQ sequence init */ serdes_seq_db[USB3_TX_CONFIG_SEQ1].op_params_ptr =

On 12/9/21 11:19, Stefan Eichenberger wrote:
The electrical serdes configuration for USB3 expects an array as data argument. For USB3 the second value is used (see data_arr_idx = USB3 = 1). However, because only one value is inside the array mv_seq_exec is accessing an invalid element and the serdes is configured wrongly.
This wrong initialization is leading to an unreliable detection mechanism for some USB3 devices. We were able to reproduce the issue regularly with an LTE modem from Sierra Wireless (SM7455) where it was not detected as USB3 device in 1/3 of all tests.
This commit fixes the issue by setting data_arr_idx to 0. This is the same value as the original U-Boot from Marvell is using. There it is called FIRST_CELL which is a define for 0. See: https://github.com/MarvellEmbeddedProcessors/u-boot-marvell commit 56f963ce4c ("fix: serdes: a38x, a39x: Fix USB3 serdes DB initialization")
Signed-off-by: Stefan Eichenberger eichest@gmail.com Signed-off-by: René Straub rene.straub@netmodule.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index d2bc3ab25c..11b5824232 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -1204,7 +1204,7 @@ int hws_serdes_seq_db_init(void) sizeof(usb3_electrical_config_serdes_rev2_params) / sizeof(struct op_params); }
- serdes_seq_db[USB3_ELECTRICAL_CONFIG_SEQ].data_arr_idx = USB3;
serdes_seq_db[USB3_ELECTRICAL_CONFIG_SEQ].data_arr_idx = 0;
/* USB3_TX_CONFIG_SEQ sequence init */ serdes_seq_db[USB3_TX_CONFIG_SEQ1].op_params_ptr =
Viele Grüße, Stefan Roese

This is a backport from Marvell U-Boot: https://github.com/MarvellEmbeddedProcessors/u-boot-marvell commit 381d029e7a ("fix: serdes: a38x, a39x: Improve USB3 electrical configuration")
Improves electrical USB3 receiver jitter tolerance test:
- De-Emphasize force, in functional mode the transmitter should always have 3.5db de-emphasize, so we are forcing it.
- After forcing De-Emphasize, choose 3.5db (After forcing, default is 6dB so need to change it to 3.5dB).
- Align90 set to 0x58 - this is the sample point in the receiver, after the clock is recovered this sampler samples at the chosen value, usually it is supposed to be 0x60(which is the center of the eye), but sometimes after adding jitter and ISI the center of the eye can move slightly and the sample point is not necessarily the exact center, and after optimization (searching the middle of the eye manually) it was seen that the center of the eye is actually 0x58 and not 0x60.
- FFE Res and FFE Cap set to 0xE & 0xF respectively: improves this settings is adequate according to how the USB3 spec defines the interconnect, thus improves USB3 jitter tolerance settings.
- Change the resolution of the DFE to 0x3 which is 6mV(highest resolution) , this avoids the DFE to saturate and cease to work.
- HPF set to 0x3 which is 5Khz high pass filter, the function of the HPF is to filter the low frequency patterns(below 5Khz) to make sure that the signal is not a noise, the setting before was 0x1(205Khz), and the change came since the USB3 CP0 pattern, that is used in the USB3 jitter tolerance testing, is similar to PRBS15, which has 2^15=32768bits which is 32768*200ps (200ps is one Unit interval in USB3(5Gbps)) = 6.5us, which is in frequency terms: 152Khz. since the PRBS15 is a random pattern and can theoretically have once in a while a pattern that will be at frequency of 152Khz, hence the previous setting (205khz HPF) can possibly filter this pattern which can cause to an error in the receiver, thus this change to avoid such scenarios.
Signed-off-by: Stefan Eichenberger eichest@gmail.com Signed-off-by: René Straub rene.straub@netmodule.com --- .../serdes/a38x/high_speed_env_spec.c | 39 +++++++++++++++---- arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h | 2 + 2 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index 11b5824232..9ba60b57aa 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -459,18 +459,41 @@ struct op_params usb3_electrical_config_serdes_rev1_params[] = { };
struct op_params usb3_electrical_config_serdes_rev2_params[] = { - /* Spread Spectrum Clock Enable */ - {LANE_CFG4_REG, 0x800, 0x80, {0x80}, 0, 0}, + /* Spread Spectrum Clock Enable, CFG_DFE_OVERRIDE and PIN_DFE_PAT_DIS */ + {LANE_CFG4_REG, 0x800, 0xc2, {0xc0}, 0, 0}, + /* CFG_SQ_DET_SEL and CFG_RX_INIT_SEL */ + {LANE_CFG5_REG, 0x800, 0x3, {0x3}, 0, 0}, /* G2_TX_SSC_AMP[6:0]=4.5k_p_pM and TX emphasis mode=m_v */ {G2_SETTINGS_2_REG, 0x800, 0xfe40, {0x4440}, 0, 0}, - /* G2_RX SELMUFF, SELMUFI, SELMUPF and SELMUPI */ + /* FFE Setting Force, FFE_RES[2:0]=0x6 and FFE_CAP[3:0]=0xf */ + {G2_SETTINGS_3_REG, 0x800, 0xff, {0xef}, 0, 0}, + /* G2_DFE_RES[1:0]=0x0(3mV)*/ + {G2_SETTINGS_4_REG, 0x800, 0x300, {0x300}, 0, 0}, + /* HPF_Bw[1:0]=0x3 */ + {PLLINTP_REG1, 0x800, 0x300, {0x300}, 0, 0}, + /* TXIMPCAL_TH[3:0]=0x3, RXIMPCAL_TH[3:0]=0x0 */ + {VTHIMPCAL_CTRL_REG, 0x800, 0xff00, {0x3000}, 0, 0}, + /* CFG_SQ_DET_SEL and CFG_RX_INIT_SEL*/ + {LANE_CFG5_REG, 0x800, 0x3, {0x3}, 0, 0}, + /* REFCLK_SEL(25Mhz), ICP_FORCE, ICP[3:0]=0xa(210uA); */ + {MISC_REG, 0x800, 0x42f, {0x42a}, 0, 0}, + /* REF_FREF_SEL[4:0]=0x2(25Mhz) */ + {POWER_AND_PLL_CTRL_REG, 0x800, 0x1f, {0x02}, 0, 0}, + /* + * G2_RX SELMUFF[1:0]=3, G2_RX_SELMUFI[1:0]=3, G2_RX_SELMUPF[2:0]=2 + * and G2_RX_SELMUPI[2:0]=2 + */ {G2_SETTINGS_1_REG, 0x800, 0x3ff, {0x3d2}, 0, 0}, /* Dtl Clamping disable and Dtl-clamping-Sel(6000ppm) */ {RX_REG2, 0x800, 0xf0, {0x70}, 0, 0}, + /* tx_amp_pipe_v0[4:0]=0x1a */ + {PCIE_REG1, 0x800, 0xf80, {0xd00}, 0, 0}, /* vco_cal_vth_sel */ {REF_REG0, 0x800, 0x38, {0x20}, 0, 0}, - /* Spread Spectrum Clock Enable */ - {LANE_CFG5_REG, 0x800, 0x4, {0x4}, 0, 0}, + /* PRD_TXDEEMPH0 */ + {LANE_CFG0_REG, 0x800, 0x1, {0x1}, 0, 0}, + /* MODE_MARGIN_OVERRIDE */ + {GLOBAL_TEST_CTRL, 0x800, 0x4, {0x4}, 0, 0}, };
/* PEX and USB3 - TX config seq */ @@ -490,11 +513,11 @@ struct op_params pex_and_usb3_tx_config_params1[] = { /* 10ms delay */ {0x0, 0x0, 0x0, {0x0, 0x0}, 10, 0}, /* os_ph_offset_force (align 90) */ - {RX_REG3, 0x800, 0xff, {0xdc, NO_DATA}, 0, 0}, + {RX_REG3, 0x800, 0xff, {0xdc, 0xd8}, 0, 0}, /* Set os_ph_valid */ - {RX_REG3, 0x800, 0x100, {0x100, NO_DATA}, 0, 0}, + {RX_REG3, 0x800, 0x100, {0x100, 0x100}, 0, 0}, /* Unset os_ph_valid */ - {RX_REG3, 0x800, 0x100, {0x0, NO_DATA}, 0, 0}, + {RX_REG3, 0x800, 0x100, {0x0, 0x0}, 0, 0}, };
struct op_params pex_and_usb3_tx_config_params2[] = { diff --git a/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h b/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h index 118bf56602..94c43b4daf 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h +++ b/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h @@ -50,6 +50,7 @@ #define MISC_REG 0xa013c #define GLUE_REG 0xa0140 #define GENERATION_DIVIDER_FORCE_REG 0xa0144 +#define PLLINTP_REG1 0xa0150 #define PCIE_REG0 0xa0120 #define LANE_ALIGN_REG0 0xa0124 #define SQUELCH_FFE_SETTING_REG 0xa0018 @@ -78,6 +79,7 @@ #define LANE_CFG4_REG 0xa0620 #define LANE_CFG5_REG 0xa0624 #define GLOBAL_CLK_CTRL 0xa0704 +#define GLOBAL_TEST_CTRL 0xa0708 #define GLOBAL_MISC_CTRL 0xa0718 #define GLOBAL_CLK_SRC_HI 0xa0710

On 12/9/21 11:19, Stefan Eichenberger wrote:
This is a backport from Marvell U-Boot: https://github.com/MarvellEmbeddedProcessors/u-boot-marvell commit 381d029e7a ("fix: serdes: a38x, a39x: Improve USB3 electrical configuration")
Improves electrical USB3 receiver jitter tolerance test:
- De-Emphasize force, in functional mode the transmitter should always
have 3.5db de-emphasize, so we are forcing it.
- After forcing De-Emphasize, choose 3.5db (After forcing, default is
6dB so need to change it to 3.5dB).
- Align90 set to 0x58 - this is the sample point in the receiver, after
the clock is recovered this sampler samples at the chosen value, usually it is supposed to be 0x60(which is the center of the eye), but sometimes after adding jitter and ISI the center of the eye can move slightly and the sample point is not necessarily the exact center, and after optimization (searching the middle of the eye manually) it was seen that the center of the eye is actually 0x58 and not 0x60.
- FFE Res and FFE Cap set to 0xE & 0xF respectively: improves this
settings is adequate according to how the USB3 spec defines the interconnect, thus improves USB3 jitter tolerance settings.
- Change the resolution of the DFE to 0x3 which is 6mV(highest
resolution) , this avoids the DFE to saturate and cease to work.
- HPF set to 0x3 which is 5Khz high pass filter, the function of the HPF
is to filter the low frequency patterns(below 5Khz) to make sure that the signal is not a noise, the setting before was 0x1(205Khz), and the change came since the USB3 CP0 pattern, that is used in the USB3 jitter tolerance testing, is similar to PRBS15, which has 2^15=32768bits which is 32768*200ps (200ps is one Unit interval in USB3(5Gbps)) = 6.5us, which is in frequency terms: 152Khz. since the PRBS15 is a random pattern and can theoretically have once in a while a pattern that will be at frequency of 152Khz, hence the previous setting (205khz HPF) can possibly filter this pattern which can cause to an error in the receiver, thus this change to avoid such scenarios.
Signed-off-by: Stefan Eichenberger eichest@gmail.com Signed-off-by: René Straub rene.straub@netmodule.com
Reviewed-by: Stefan Roese sr@denx.de
Thanks, Stefan
.../serdes/a38x/high_speed_env_spec.c | 39 +++++++++++++++---- arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h | 2 + 2 files changed, 33 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c index 11b5824232..9ba60b57aa 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c +++ b/arch/arm/mach-mvebu/serdes/a38x/high_speed_env_spec.c @@ -459,18 +459,41 @@ struct op_params usb3_electrical_config_serdes_rev1_params[] = { };
struct op_params usb3_electrical_config_serdes_rev2_params[] = {
- /* Spread Spectrum Clock Enable */
- {LANE_CFG4_REG, 0x800, 0x80, {0x80}, 0, 0},
- /* Spread Spectrum Clock Enable, CFG_DFE_OVERRIDE and PIN_DFE_PAT_DIS */
- {LANE_CFG4_REG, 0x800, 0xc2, {0xc0}, 0, 0},
- /* CFG_SQ_DET_SEL and CFG_RX_INIT_SEL */
- {LANE_CFG5_REG, 0x800, 0x3, {0x3}, 0, 0}, /* G2_TX_SSC_AMP[6:0]=4.5k_p_pM and TX emphasis mode=m_v */ {G2_SETTINGS_2_REG, 0x800, 0xfe40, {0x4440}, 0, 0},
- /* G2_RX SELMUFF, SELMUFI, SELMUPF and SELMUPI */
- /* FFE Setting Force, FFE_RES[2:0]=0x6 and FFE_CAP[3:0]=0xf */
- {G2_SETTINGS_3_REG, 0x800, 0xff, {0xef}, 0, 0},
- /* G2_DFE_RES[1:0]=0x0(3mV)*/
- {G2_SETTINGS_4_REG, 0x800, 0x300, {0x300}, 0, 0},
- /* HPF_Bw[1:0]=0x3 */
- {PLLINTP_REG1, 0x800, 0x300, {0x300}, 0, 0},
- /* TXIMPCAL_TH[3:0]=0x3, RXIMPCAL_TH[3:0]=0x0 */
- {VTHIMPCAL_CTRL_REG, 0x800, 0xff00, {0x3000}, 0, 0},
- /* CFG_SQ_DET_SEL and CFG_RX_INIT_SEL*/
- {LANE_CFG5_REG, 0x800, 0x3, {0x3}, 0, 0},
- /* REFCLK_SEL(25Mhz), ICP_FORCE, ICP[3:0]=0xa(210uA); */
- {MISC_REG, 0x800, 0x42f, {0x42a}, 0, 0},
- /* REF_FREF_SEL[4:0]=0x2(25Mhz) */
- {POWER_AND_PLL_CTRL_REG, 0x800, 0x1f, {0x02}, 0, 0},
- /*
* G2_RX SELMUFF[1:0]=3, G2_RX_SELMUFI[1:0]=3, G2_RX_SELMUPF[2:0]=2
* and G2_RX_SELMUPI[2:0]=2
{G2_SETTINGS_1_REG, 0x800, 0x3ff, {0x3d2}, 0, 0}, /* Dtl Clamping disable and Dtl-clamping-Sel(6000ppm) */ {RX_REG2, 0x800, 0xf0, {0x70}, 0, 0},*/
- /* tx_amp_pipe_v0[4:0]=0x1a */
- {PCIE_REG1, 0x800, 0xf80, {0xd00}, 0, 0}, /* vco_cal_vth_sel */ {REF_REG0, 0x800, 0x38, {0x20}, 0, 0},
- /* Spread Spectrum Clock Enable */
- {LANE_CFG5_REG, 0x800, 0x4, {0x4}, 0, 0},
/* PRD_TXDEEMPH0 */
{LANE_CFG0_REG, 0x800, 0x1, {0x1}, 0, 0},
/* MODE_MARGIN_OVERRIDE */
{GLOBAL_TEST_CTRL, 0x800, 0x4, {0x4}, 0, 0}, };
/* PEX and USB3 - TX config seq */
@@ -490,11 +513,11 @@ struct op_params pex_and_usb3_tx_config_params1[] = { /* 10ms delay */ {0x0, 0x0, 0x0, {0x0, 0x0}, 10, 0}, /* os_ph_offset_force (align 90) */
- {RX_REG3, 0x800, 0xff, {0xdc, NO_DATA}, 0, 0},
- {RX_REG3, 0x800, 0xff, {0xdc, 0xd8}, 0, 0}, /* Set os_ph_valid */
- {RX_REG3, 0x800, 0x100, {0x100, NO_DATA}, 0, 0},
- {RX_REG3, 0x800, 0x100, {0x100, 0x100}, 0, 0}, /* Unset os_ph_valid */
- {RX_REG3, 0x800, 0x100, {0x0, NO_DATA}, 0, 0},
{RX_REG3, 0x800, 0x100, {0x0, 0x0}, 0, 0}, };
struct op_params pex_and_usb3_tx_config_params2[] = {
diff --git a/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h b/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h index 118bf56602..94c43b4daf 100644 --- a/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h +++ b/arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h @@ -50,6 +50,7 @@ #define MISC_REG 0xa013c #define GLUE_REG 0xa0140 #define GENERATION_DIVIDER_FORCE_REG 0xa0144 +#define PLLINTP_REG1 0xa0150 #define PCIE_REG0 0xa0120 #define LANE_ALIGN_REG0 0xa0124 #define SQUELCH_FFE_SETTING_REG 0xa0018 @@ -78,6 +79,7 @@ #define LANE_CFG4_REG 0xa0620 #define LANE_CFG5_REG 0xa0624 #define GLOBAL_CLK_CTRL 0xa0704 +#define GLOBAL_TEST_CTRL 0xa0708 #define GLOBAL_MISC_CTRL 0xa0718 #define GLOBAL_CLK_SRC_HI 0xa0710
Viele Grüße, Stefan Roese

On 12/9/21 11:19, Stefan Eichenberger wrote:
This patchset improves the USB3 serdes configuration for the Armada 38x SoC from Marvell. The changes are based on the version from Marvell found here: https://github.com/MarvellEmbeddedProcessors/u-boot-marvell
Stefan Eichenberger (2): arm: mvebu: a38x: serdes: fix serdes config for USB3 arm: mvebu: a38x: serdes: Improve USB3 electrical configuration
.../serdes/a38x/high_speed_env_spec.c | 38 ++++++++++++++----- arch/arm/mach-mvebu/serdes/a38x/sys_env_lib.h | 2 + 2 files changed, 31 insertions(+), 9 deletions(-)
Applied to u-boot-marvell/next
Thanks, Stefan
participants (2)
-
Stefan Eichenberger
-
Stefan Roese