[U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
GMAC controller need to init the tx io driver strength to 13mA, just like the description in dts pinctrl node, or else the controller may only work in 100MHz Mode, and fail to work at 1000MHz mode.
Signed-off-by: Kever Yang kever.yang@rock-chips.com ---
arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 75 +++++++++++++++++++++++-- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 18 ++++++ 2 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h index b340b05..e8a6f71 100644 --- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h @@ -151,10 +151,11 @@ struct rk3399_grf_regs { u32 gpio2_sr[3][4]; u32 reserved23[4]; u32 gpio2_smt[3][4]; - u32 reserved24[(0xe130 - 0xe0ec)/4 - 1]; - u32 gpio4b_e01; - u32 gpio4b_e2; - u32 reserved24a[(0xe200 - 0xe134)/4 - 1]; + u32 reserved24[(0xe100 - 0xe0ec)/4 - 1]; + u32 gpio2_e[4]; + u32 gpio3_e[7]; + u32 gpio4_e[5]; + u32 reserved24a[(0xe200 - 0xe13c)/4 - 1]; u32 soc_con0; u32 soc_con1; u32 soc_con2; @@ -435,6 +436,72 @@ enum { GRF_GPIO4C6_SEL_MASK = 3 << GRF_GPIO4C6_SEL_SHIFT, GRF_PWM_1 = 1,
+ /* GRF_GPIO3A_E01 */ + GRF_GPIO3A0_E_SHIFT = 0, + GRF_GPIO3A0_E_MASK = 7 << GRF_GPIO3A0_E_SHIFT, + GRF_GPIO3A1_E_SHIFT = 3, + GRF_GPIO3A1_E_MASK = 7 << GRF_GPIO3A1_E_SHIFT, + GRF_GPIO3A2_E_SHIFT = 6, + GRF_GPIO3A2_E_MASK = 7 << GRF_GPIO3A2_E_SHIFT, + GRF_GPIO3A3_E_SHIFT = 9, + GRF_GPIO3A3_E_MASK = 7 << GRF_GPIO3A3_E_SHIFT, + GRF_GPIO3A4_E_SHIFT = 12, + GRF_GPIO3A4_E_MASK = 7 << GRF_GPIO3A4_E_SHIFT, + GRF_GPIO3A5_E0_SHIFT = 15, + GRF_GPIO3A5_E0_MASK = 1 << GRF_GPIO3A5_E0_SHIFT, + + /* GRF_GPIO3A_E2 */ + GRF_GPIO3A5_E12_SHIFT = 0, + GRF_GPIO3A5_E12_MASK = 3 << GRF_GPIO3A5_E12_SHIFT, + GRF_GPIO3A6_E_SHIFT = 2, + GRF_GPIO3A6_E_MASK = 7 << GRF_GPIO3A6_E_SHIFT, + GRF_GPIO3A7_E_SHIFT = 5, + GRF_GPIO3A7_E_MASK = 7 << GRF_GPIO3A7_E_SHIFT, + + /* GRF_GPIO3B_E01 */ + GRF_GPIO3B0_E_SHIFT = 0, + GRF_GPIO3B0_E_MASK = 7 << GRF_GPIO3B0_E_SHIFT, + GRF_GPIO3B1_E_SHIFT = 3, + GRF_GPIO3B1_E_MASK = 7 << GRF_GPIO3B1_E_SHIFT, + GRF_GPIO3B2_E_SHIFT = 6, + GRF_GPIO3B2_E_MASK = 7 << GRF_GPIO3B2_E_SHIFT, + GRF_GPIO3B3_E_SHIFT = 9, + GRF_GPIO3B3_E_MASK = 7 << GRF_GPIO3B3_E_SHIFT, + GRF_GPIO3B4_E_SHIFT = 12, + GRF_GPIO3B4_E_MASK = 7 << GRF_GPIO3B4_E_SHIFT, + GRF_GPIO3B5_E0_SHIFT = 15, + GRF_GPIO3B5_E0_MASK = 1 << GRF_GPIO3B5_E0_SHIFT, + + /* GRF_GPIO3A_E2 */ + GRF_GPIO3B5_E12_SHIFT = 0, + GRF_GPIO3B5_E12_MASK = 3 << GRF_GPIO3B5_E12_SHIFT, + GRF_GPIO3B6_E_SHIFT = 2, + GRF_GPIO3B6_E_MASK = 7 << GRF_GPIO3B6_E_SHIFT, + GRF_GPIO3B7_E_SHIFT = 5, + GRF_GPIO3B7_E_MASK = 7 << GRF_GPIO3B7_E_SHIFT, + + /* GRF_GPIO3C_E01 */ + GRF_GPIO3C0_E_SHIFT = 0, + GRF_GPIO3C0_E_MASK = 7 << GRF_GPIO3C0_E_SHIFT, + GRF_GPIO3C1_E_SHIFT = 3, + GRF_GPIO3C1_E_MASK = 7 << GRF_GPIO3C1_E_SHIFT, + GRF_GPIO3C2_E_SHIFT = 6, + GRF_GPIO3C2_E_MASK = 7 << GRF_GPIO3C2_E_SHIFT, + GRF_GPIO3C3_E_SHIFT = 9, + GRF_GPIO3C3_E_MASK = 7 << GRF_GPIO3C3_E_SHIFT, + GRF_GPIO3C4_E_SHIFT = 12, + GRF_GPIO3C4_E_MASK = 7 << GRF_GPIO3C4_E_SHIFT, + GRF_GPIO3C5_E0_SHIFT = 15, + GRF_GPIO3C5_E0_MASK = 1 << GRF_GPIO3C5_E0_SHIFT, + + /* GRF_GPIO3C_E2 */ + GRF_GPIO3C5_E12_SHIFT = 0, + GRF_GPIO3C5_E12_MASK = 3 << GRF_GPIO3C5_E12_SHIFT, + GRF_GPIO3C6_E_SHIFT = 2, + GRF_GPIO3C6_E_MASK = 7 << GRF_GPIO3C6_E_SHIFT, + GRF_GPIO3C7_E_SHIFT = 5, + GRF_GPIO3C7_E_MASK = 7 << GRF_GPIO3C7_E_SHIFT, + /* GRF_SOC_CON7 */ GRF_UART_DBG_SEL_SHIFT = 10, GRF_UART_DBG_SEL_MASK = 3 << GRF_UART_DBG_SEL_SHIFT, diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3399.c b/drivers/pinctrl/rockchip/pinctrl_rk3399.c index 507bec4..6b62a0c 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3399.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3399.c @@ -232,6 +232,24 @@ static void pinctrl_rk3399_gmac_config(struct rk3399_grf_regs *grf, int mmc_id) rk_clrsetreg(&grf->gpio3c_iomux, GRF_GPIO3C1_SEL_MASK, GRF_MAC_TXCLK << GRF_GPIO3C1_SEL_SHIFT); + + /* Set drive strength for GMAC tx io, value 3 means 13mA */ + rk_clrsetreg(&grf->gpio3_e[0], + GRF_GPIO3A0_E_MASK | GRF_GPIO3A1_E_MASK | + GRF_GPIO3A4_E_MASK | GRF_GPIO3A5_E0_MASK, + 3 << GRF_GPIO3A0_E_SHIFT | + 3 << GRF_GPIO3A1_E_SHIFT | + 3 << GRF_GPIO3A4_E_SHIFT | + 1 << GRF_GPIO3A5_E0_SHIFT); + rk_clrsetreg(&grf->gpio3_e[1], + GRF_GPIO3A5_E12_MASK, + 1 << GRF_GPIO3A5_E12_SHIFT); + rk_clrsetreg(&grf->gpio3_e[2], + GRF_GPIO3B4_E_MASK, + 3 << GRF_GPIO3B4_E_SHIFT); + rk_clrsetreg(&grf->gpio3_e[4], + GRF_GPIO3C1_E_MASK, + 3 << GRF_GPIO3C1_E_SHIFT); } #endif
On 20 Apr 2017, at 10:15, Kever Yang kever.yang@rock-chips.com wrote:
GMAC controller need to init the tx io driver strength to 13mA, just like the description in dts pinctrl node, or else the controller may only work in 100MHz Mode, and fail to work at 1000MHz mode.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 75 +++++++++++++++++++++++-- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 18 ++++++ 2 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h index b340b05..e8a6f71 100644 --- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h @@ -151,10 +151,11 @@ struct rk3399_grf_regs { u32 gpio2_sr[3][4]; u32 reserved23[4]; u32 gpio2_smt[3][4];
- u32 reserved24[(0xe130 - 0xe0ec)/4 - 1];
- u32 gpio4b_e01;
- u32 gpio4b_e2;
- u32 reserved24a[(0xe200 - 0xe134)/4 - 1];
- u32 reserved24[(0xe100 - 0xe0ec)/4 - 1];
- u32 gpio2_e[4];
- u32 gpio3_e[7];
- u32 gpio4_e[5];
- u32 reserved24a[(0xe200 - 0xe13c)/4 - 1]; u32 soc_con0; u32 soc_con1; u32 soc_con2;
@@ -435,6 +436,72 @@ enum { GRF_GPIO4C6_SEL_MASK = 3 << GRF_GPIO4C6_SEL_SHIFT, GRF_PWM_1 = 1,
- /* GRF_GPIO3A_E01 */
- GRF_GPIO3A0_E_SHIFT = 0,
- GRF_GPIO3A0_E_MASK = 7 << GRF_GPIO3A0_E_SHIFT,
- GRF_GPIO3A1_E_SHIFT = 3,
- GRF_GPIO3A1_E_MASK = 7 << GRF_GPIO3A1_E_SHIFT,
- GRF_GPIO3A2_E_SHIFT = 6,
- GRF_GPIO3A2_E_MASK = 7 << GRF_GPIO3A2_E_SHIFT,
- GRF_GPIO3A3_E_SHIFT = 9,
- GRF_GPIO3A3_E_MASK = 7 << GRF_GPIO3A3_E_SHIFT,
- GRF_GPIO3A4_E_SHIFT = 12,
- GRF_GPIO3A4_E_MASK = 7 << GRF_GPIO3A4_E_SHIFT,
- GRF_GPIO3A5_E0_SHIFT = 15,
- GRF_GPIO3A5_E0_MASK = 1 << GRF_GPIO3A5_E0_SHIFT,
- /* GRF_GPIO3A_E2 */
- GRF_GPIO3A5_E12_SHIFT = 0,
- GRF_GPIO3A5_E12_MASK = 3 << GRF_GPIO3A5_E12_SHIFT,
- GRF_GPIO3A6_E_SHIFT = 2,
- GRF_GPIO3A6_E_MASK = 7 << GRF_GPIO3A6_E_SHIFT,
- GRF_GPIO3A7_E_SHIFT = 5,
- GRF_GPIO3A7_E_MASK = 7 << GRF_GPIO3A7_E_SHIFT,
- /* GRF_GPIO3B_E01 */
- GRF_GPIO3B0_E_SHIFT = 0,
- GRF_GPIO3B0_E_MASK = 7 << GRF_GPIO3B0_E_SHIFT,
- GRF_GPIO3B1_E_SHIFT = 3,
- GRF_GPIO3B1_E_MASK = 7 << GRF_GPIO3B1_E_SHIFT,
- GRF_GPIO3B2_E_SHIFT = 6,
- GRF_GPIO3B2_E_MASK = 7 << GRF_GPIO3B2_E_SHIFT,
- GRF_GPIO3B3_E_SHIFT = 9,
- GRF_GPIO3B3_E_MASK = 7 << GRF_GPIO3B3_E_SHIFT,
- GRF_GPIO3B4_E_SHIFT = 12,
- GRF_GPIO3B4_E_MASK = 7 << GRF_GPIO3B4_E_SHIFT,
- GRF_GPIO3B5_E0_SHIFT = 15,
- GRF_GPIO3B5_E0_MASK = 1 << GRF_GPIO3B5_E0_SHIFT,
- /* GRF_GPIO3A_E2 */
- GRF_GPIO3B5_E12_SHIFT = 0,
- GRF_GPIO3B5_E12_MASK = 3 << GRF_GPIO3B5_E12_SHIFT,
- GRF_GPIO3B6_E_SHIFT = 2,
- GRF_GPIO3B6_E_MASK = 7 << GRF_GPIO3B6_E_SHIFT,
- GRF_GPIO3B7_E_SHIFT = 5,
- GRF_GPIO3B7_E_MASK = 7 << GRF_GPIO3B7_E_SHIFT,
- /* GRF_GPIO3C_E01 */
- GRF_GPIO3C0_E_SHIFT = 0,
- GRF_GPIO3C0_E_MASK = 7 << GRF_GPIO3C0_E_SHIFT,
- GRF_GPIO3C1_E_SHIFT = 3,
- GRF_GPIO3C1_E_MASK = 7 << GRF_GPIO3C1_E_SHIFT,
- GRF_GPIO3C2_E_SHIFT = 6,
- GRF_GPIO3C2_E_MASK = 7 << GRF_GPIO3C2_E_SHIFT,
- GRF_GPIO3C3_E_SHIFT = 9,
- GRF_GPIO3C3_E_MASK = 7 << GRF_GPIO3C3_E_SHIFT,
- GRF_GPIO3C4_E_SHIFT = 12,
- GRF_GPIO3C4_E_MASK = 7 << GRF_GPIO3C4_E_SHIFT,
- GRF_GPIO3C5_E0_SHIFT = 15,
- GRF_GPIO3C5_E0_MASK = 1 << GRF_GPIO3C5_E0_SHIFT,
- /* GRF_GPIO3C_E2 */
- GRF_GPIO3C5_E12_SHIFT = 0,
- GRF_GPIO3C5_E12_MASK = 3 << GRF_GPIO3C5_E12_SHIFT,
- GRF_GPIO3C6_E_SHIFT = 2,
- GRF_GPIO3C6_E_MASK = 7 << GRF_GPIO3C6_E_SHIFT,
- GRF_GPIO3C7_E_SHIFT = 5,
- GRF_GPIO3C7_E_MASK = 7 << GRF_GPIO3C7_E_SHIFT,
- /* GRF_SOC_CON7 */ GRF_UART_DBG_SEL_SHIFT = 10, GRF_UART_DBG_SEL_MASK = 3 << GRF_UART_DBG_SEL_SHIFT,
diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3399.c b/drivers/pinctrl/rockchip/pinctrl_rk3399.c index 507bec4..6b62a0c 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3399.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3399.c @@ -232,6 +232,24 @@ static void pinctrl_rk3399_gmac_config(struct rk3399_grf_regs *grf, int mmc_id) rk_clrsetreg(&grf->gpio3c_iomux, GRF_GPIO3C1_SEL_MASK, GRF_MAC_TXCLK << GRF_GPIO3C1_SEL_SHIFT);
- /* Set drive strength for GMAC tx io, value 3 means 13mA */
- rk_clrsetreg(&grf->gpio3_e[0],
GRF_GPIO3A0_E_MASK | GRF_GPIO3A1_E_MASK |GRF_GPIO3A4_E_MASK | GRF_GPIO3A5_E0_MASK,3 << GRF_GPIO3A0_E_SHIFT |3 << GRF_GPIO3A1_E_SHIFT |3 << GRF_GPIO3A4_E_SHIFT |1 << GRF_GPIO3A5_E0_SHIFT);- rk_clrsetreg(&grf->gpio3_e[1],
GRF_GPIO3A5_E12_MASK,1 << GRF_GPIO3A5_E12_SHIFT);- rk_clrsetreg(&grf->gpio3_e[2],
GRF_GPIO3B4_E_MASK,3 << GRF_GPIO3B4_E_SHIFT);- rk_clrsetreg(&grf->gpio3_e[4],
GRF_GPIO3C1_E_MASK,3 << GRF_GPIO3C1_E_SHIFT);} #endif
-- 1.9.1
Do you know if this is required for all board designs? We have a total run length of less than 2cm to the KSZ9031 PHY and wondered about this ourselves—our testing has shown that with these small distances (and the PHY we use) the setting doesn’t seem to be required.
Instead of hard-coding this: shouldn't we parse the drive-strength setting from the DTS?
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com>
Hi Philipp,
On 04/20/2017 04:21 PM, Dr. Philipp Tomsich wrote:
On 20 Apr 2017, at 10:15, Kever Yang <kever.yang@rock-chips.com mailto:kever.yang@rock-chips.com> wrote:
GMAC controller need to init the tx io driver strength to 13mA, just like the description in dts pinctrl node, or else the controller may only work in 100MHz Mode, and fail to work at 1000MHz mode.
Signed-off-by: Kever Yang <kever.yang@rock-chips.com
mailto:kever.yang@rock-chips.com>
arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 75 +++++++++++++++++++++++-- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 18 ++++++ 2 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h index b340b05..e8a6f71 100644 --- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h @@ -151,10 +151,11 @@ struct rk3399_grf_regs { u32 gpio2_sr[3][4]; u32 reserved23[4]; u32 gpio2_smt[3][4]; -u32 reserved24[(0xe130 - 0xe0ec)/4 - 1]; -u32 gpio4b_e01; -u32 gpio4b_e2; -u32 reserved24a[(0xe200 - 0xe134)/4 - 1]; +u32 reserved24[(0xe100 - 0xe0ec)/4 - 1]; +u32 gpio2_e[4]; +u32 gpio3_e[7]; +u32 gpio4_e[5]; +u32 reserved24a[(0xe200 - 0xe13c)/4 - 1]; u32 soc_con0; u32 soc_con1; u32 soc_con2; @@ -435,6 +436,72 @@ enum { GRF_GPIO4C6_SEL_MASK = 3 << GRF_GPIO4C6_SEL_SHIFT, GRF_PWM_1 = 1,
+/* GRF_GPIO3A_E01 */ +GRF_GPIO3A0_E_SHIFT = 0, +GRF_GPIO3A0_E_MASK = 7 << GRF_GPIO3A0_E_SHIFT, +GRF_GPIO3A1_E_SHIFT = 3, +GRF_GPIO3A1_E_MASK = 7 << GRF_GPIO3A1_E_SHIFT, +GRF_GPIO3A2_E_SHIFT = 6, +GRF_GPIO3A2_E_MASK = 7 << GRF_GPIO3A2_E_SHIFT, +GRF_GPIO3A3_E_SHIFT = 9, +GRF_GPIO3A3_E_MASK = 7 << GRF_GPIO3A3_E_SHIFT, +GRF_GPIO3A4_E_SHIFT = 12, +GRF_GPIO3A4_E_MASK = 7 << GRF_GPIO3A4_E_SHIFT, +GRF_GPIO3A5_E0_SHIFT = 15, +GRF_GPIO3A5_E0_MASK = 1 << GRF_GPIO3A5_E0_SHIFT,
+/* GRF_GPIO3A_E2 */ +GRF_GPIO3A5_E12_SHIFT = 0, +GRF_GPIO3A5_E12_MASK = 3 << GRF_GPIO3A5_E12_SHIFT, +GRF_GPIO3A6_E_SHIFT = 2, +GRF_GPIO3A6_E_MASK = 7 << GRF_GPIO3A6_E_SHIFT, +GRF_GPIO3A7_E_SHIFT = 5, +GRF_GPIO3A7_E_MASK = 7 << GRF_GPIO3A7_E_SHIFT,
+/* GRF_GPIO3B_E01 */ +GRF_GPIO3B0_E_SHIFT = 0, +GRF_GPIO3B0_E_MASK = 7 << GRF_GPIO3B0_E_SHIFT, +GRF_GPIO3B1_E_SHIFT = 3, +GRF_GPIO3B1_E_MASK = 7 << GRF_GPIO3B1_E_SHIFT, +GRF_GPIO3B2_E_SHIFT = 6, +GRF_GPIO3B2_E_MASK = 7 << GRF_GPIO3B2_E_SHIFT, +GRF_GPIO3B3_E_SHIFT = 9, +GRF_GPIO3B3_E_MASK = 7 << GRF_GPIO3B3_E_SHIFT, +GRF_GPIO3B4_E_SHIFT = 12, +GRF_GPIO3B4_E_MASK = 7 << GRF_GPIO3B4_E_SHIFT, +GRF_GPIO3B5_E0_SHIFT = 15, +GRF_GPIO3B5_E0_MASK = 1 << GRF_GPIO3B5_E0_SHIFT,
+/* GRF_GPIO3A_E2 */ +GRF_GPIO3B5_E12_SHIFT = 0, +GRF_GPIO3B5_E12_MASK = 3 << GRF_GPIO3B5_E12_SHIFT, +GRF_GPIO3B6_E_SHIFT = 2, +GRF_GPIO3B6_E_MASK = 7 << GRF_GPIO3B6_E_SHIFT, +GRF_GPIO3B7_E_SHIFT = 5, +GRF_GPIO3B7_E_MASK = 7 << GRF_GPIO3B7_E_SHIFT,
+/* GRF_GPIO3C_E01 */ +GRF_GPIO3C0_E_SHIFT = 0, +GRF_GPIO3C0_E_MASK = 7 << GRF_GPIO3C0_E_SHIFT, +GRF_GPIO3C1_E_SHIFT = 3, +GRF_GPIO3C1_E_MASK = 7 << GRF_GPIO3C1_E_SHIFT, +GRF_GPIO3C2_E_SHIFT = 6, +GRF_GPIO3C2_E_MASK = 7 << GRF_GPIO3C2_E_SHIFT, +GRF_GPIO3C3_E_SHIFT = 9, +GRF_GPIO3C3_E_MASK = 7 << GRF_GPIO3C3_E_SHIFT, +GRF_GPIO3C4_E_SHIFT = 12, +GRF_GPIO3C4_E_MASK = 7 << GRF_GPIO3C4_E_SHIFT, +GRF_GPIO3C5_E0_SHIFT = 15, +GRF_GPIO3C5_E0_MASK = 1 << GRF_GPIO3C5_E0_SHIFT,
+/* GRF_GPIO3C_E2 */ +GRF_GPIO3C5_E12_SHIFT = 0, +GRF_GPIO3C5_E12_MASK = 3 << GRF_GPIO3C5_E12_SHIFT, +GRF_GPIO3C6_E_SHIFT = 2, +GRF_GPIO3C6_E_MASK = 7 << GRF_GPIO3C6_E_SHIFT, +GRF_GPIO3C7_E_SHIFT = 5, +GRF_GPIO3C7_E_MASK = 7 << GRF_GPIO3C7_E_SHIFT,
/* GRF_SOC_CON7 */ GRF_UART_DBG_SEL_SHIFT= 10, GRF_UART_DBG_SEL_MASK= 3 << GRF_UART_DBG_SEL_SHIFT, diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3399.c b/drivers/pinctrl/rockchip/pinctrl_rk3399.c index 507bec4..6b62a0c 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3399.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3399.c @@ -232,6 +232,24 @@ static void pinctrl_rk3399_gmac_config(struct rk3399_grf_regs *grf, int mmc_id) rk_clrsetreg(&grf->gpio3c_iomux, GRF_GPIO3C1_SEL_MASK, GRF_MAC_TXCLK << GRF_GPIO3C1_SEL_SHIFT);
+/* Set drive strength for GMAC tx io, value 3 means 13mA */ +rk_clrsetreg(&grf->gpio3_e[0],
- GRF_GPIO3A0_E_MASK | GRF_GPIO3A1_E_MASK |
- GRF_GPIO3A4_E_MASK | GRF_GPIO3A5_E0_MASK,
- 3 << GRF_GPIO3A0_E_SHIFT |
- 3 << GRF_GPIO3A1_E_SHIFT |
- 3 << GRF_GPIO3A4_E_SHIFT |
- 1 << GRF_GPIO3A5_E0_SHIFT);
+rk_clrsetreg(&grf->gpio3_e[1],
- GRF_GPIO3A5_E12_MASK,
- 1 << GRF_GPIO3A5_E12_SHIFT);
+rk_clrsetreg(&grf->gpio3_e[2],
- GRF_GPIO3B4_E_MASK,
- 3 << GRF_GPIO3B4_E_SHIFT);
+rk_clrsetreg(&grf->gpio3_e[4],
- GRF_GPIO3C1_E_MASK,
- 3 << GRF_GPIO3C1_E_SHIFT);
} #endif
-- 1.9.1
Do you know if this is required for all board designs? We have a total run length of less than 2cm to the KSZ9031 PHY and wondered about this ourselves—our testing has shown that with these small distances (and the PHY we use) the setting doesn’t seem to be required.
If your layout is very good, it might work without this patch, did you test with 1000M Ethernet on many boards? With patch, we can keep the setting with kernel and make sure all the hardware able to work at 1000M mode. The firefly-rk3399 and rockchip rk3399-evb can't work at 1000M Ethernet mode without this patch.
Instead of hard-coding this: shouldn't we parse the drive-strength setting from the DTS?
Sync with the kernel pinctrl-rockchip can parse the drive-strength, but I don't think it's a good idea to sync that more than 2000 line file, this patch should be enough for U-Boot now.
Thanks, - Kever
Reviewed-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com mailto:philipp.tomsich@theobroma-systems.com>
On 20 Apr 2017, at 10:44, Kever Yang kever.yang@rock-chips.com wrote:
Hi Philipp,
On 04/20/2017 04:21 PM, Dr. Philipp Tomsich wrote:
On 20 Apr 2017, at 10:15, Kever Yang <kever.yang@rock-chips.com mailto:kever.yang@rock-chips.com> wrote:
GMAC controller need to init the tx io driver strength to 13mA, just like the description in dts pinctrl node, or else the controller may only work in 100MHz Mode, and fail to work at 1000MHz mode.
Signed-off-by: Kever Yang <kever.yang@rock-chips.com mailto:kever.yang@rock-chips.com>
arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 75 +++++++++++++++++++++++-- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 18 ++++++ 2 files changed, 89 insertions(+), 4 deletions(-)
diff --git a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h index b340b05..e8a6f71 100644 --- a/arch/arm/include/asm/arch-rockchip/grf_rk3399.h +++ b/arch/arm/include/asm/arch-rockchip/grf_rk3399.h @@ -151,10 +151,11 @@ struct rk3399_grf_regs { u32 gpio2_sr[3][4]; u32 reserved23[4]; u32 gpio2_smt[3][4];
- u32 reserved24[(0xe130 - 0xe0ec)/4 - 1];
- u32 gpio4b_e01;
- u32 gpio4b_e2;
- u32 reserved24a[(0xe200 - 0xe134)/4 - 1];
- u32 reserved24[(0xe100 - 0xe0ec)/4 - 1];
- u32 gpio2_e[4];
- u32 gpio3_e[7];
- u32 gpio4_e[5];
- u32 reserved24a[(0xe200 - 0xe13c)/4 - 1]; u32 soc_con0; u32 soc_con1; u32 soc_con2;
@@ -435,6 +436,72 @@ enum { GRF_GPIO4C6_SEL_MASK = 3 << GRF_GPIO4C6_SEL_SHIFT, GRF_PWM_1 = 1,
- /* GRF_GPIO3A_E01 */
- GRF_GPIO3A0_E_SHIFT = 0,
- GRF_GPIO3A0_E_MASK = 7 << GRF_GPIO3A0_E_SHIFT,
- GRF_GPIO3A1_E_SHIFT = 3,
- GRF_GPIO3A1_E_MASK = 7 << GRF_GPIO3A1_E_SHIFT,
- GRF_GPIO3A2_E_SHIFT = 6,
- GRF_GPIO3A2_E_MASK = 7 << GRF_GPIO3A2_E_SHIFT,
- GRF_GPIO3A3_E_SHIFT = 9,
- GRF_GPIO3A3_E_MASK = 7 << GRF_GPIO3A3_E_SHIFT,
- GRF_GPIO3A4_E_SHIFT = 12,
- GRF_GPIO3A4_E_MASK = 7 << GRF_GPIO3A4_E_SHIFT,
- GRF_GPIO3A5_E0_SHIFT = 15,
- GRF_GPIO3A5_E0_MASK = 1 << GRF_GPIO3A5_E0_SHIFT,
- /* GRF_GPIO3A_E2 */
- GRF_GPIO3A5_E12_SHIFT = 0,
- GRF_GPIO3A5_E12_MASK = 3 << GRF_GPIO3A5_E12_SHIFT,
- GRF_GPIO3A6_E_SHIFT = 2,
- GRF_GPIO3A6_E_MASK = 7 << GRF_GPIO3A6_E_SHIFT,
- GRF_GPIO3A7_E_SHIFT = 5,
- GRF_GPIO3A7_E_MASK = 7 << GRF_GPIO3A7_E_SHIFT,
- /* GRF_GPIO3B_E01 */
- GRF_GPIO3B0_E_SHIFT = 0,
- GRF_GPIO3B0_E_MASK = 7 << GRF_GPIO3B0_E_SHIFT,
- GRF_GPIO3B1_E_SHIFT = 3,
- GRF_GPIO3B1_E_MASK = 7 << GRF_GPIO3B1_E_SHIFT,
- GRF_GPIO3B2_E_SHIFT = 6,
- GRF_GPIO3B2_E_MASK = 7 << GRF_GPIO3B2_E_SHIFT,
- GRF_GPIO3B3_E_SHIFT = 9,
- GRF_GPIO3B3_E_MASK = 7 << GRF_GPIO3B3_E_SHIFT,
- GRF_GPIO3B4_E_SHIFT = 12,
- GRF_GPIO3B4_E_MASK = 7 << GRF_GPIO3B4_E_SHIFT,
- GRF_GPIO3B5_E0_SHIFT = 15,
- GRF_GPIO3B5_E0_MASK = 1 << GRF_GPIO3B5_E0_SHIFT,
- /* GRF_GPIO3A_E2 */
- GRF_GPIO3B5_E12_SHIFT = 0,
- GRF_GPIO3B5_E12_MASK = 3 << GRF_GPIO3B5_E12_SHIFT,
- GRF_GPIO3B6_E_SHIFT = 2,
- GRF_GPIO3B6_E_MASK = 7 << GRF_GPIO3B6_E_SHIFT,
- GRF_GPIO3B7_E_SHIFT = 5,
- GRF_GPIO3B7_E_MASK = 7 << GRF_GPIO3B7_E_SHIFT,
- /* GRF_GPIO3C_E01 */
- GRF_GPIO3C0_E_SHIFT = 0,
- GRF_GPIO3C0_E_MASK = 7 << GRF_GPIO3C0_E_SHIFT,
- GRF_GPIO3C1_E_SHIFT = 3,
- GRF_GPIO3C1_E_MASK = 7 << GRF_GPIO3C1_E_SHIFT,
- GRF_GPIO3C2_E_SHIFT = 6,
- GRF_GPIO3C2_E_MASK = 7 << GRF_GPIO3C2_E_SHIFT,
- GRF_GPIO3C3_E_SHIFT = 9,
- GRF_GPIO3C3_E_MASK = 7 << GRF_GPIO3C3_E_SHIFT,
- GRF_GPIO3C4_E_SHIFT = 12,
- GRF_GPIO3C4_E_MASK = 7 << GRF_GPIO3C4_E_SHIFT,
- GRF_GPIO3C5_E0_SHIFT = 15,
- GRF_GPIO3C5_E0_MASK = 1 << GRF_GPIO3C5_E0_SHIFT,
- /* GRF_GPIO3C_E2 */
- GRF_GPIO3C5_E12_SHIFT = 0,
- GRF_GPIO3C5_E12_MASK = 3 << GRF_GPIO3C5_E12_SHIFT,
- GRF_GPIO3C6_E_SHIFT = 2,
- GRF_GPIO3C6_E_MASK = 7 << GRF_GPIO3C6_E_SHIFT,
- GRF_GPIO3C7_E_SHIFT = 5,
- GRF_GPIO3C7_E_MASK = 7 << GRF_GPIO3C7_E_SHIFT,
- /* GRF_SOC_CON7 */ GRF_UART_DBG_SEL_SHIFT = 10, GRF_UART_DBG_SEL_MASK = 3 << GRF_UART_DBG_SEL_SHIFT,
diff --git a/drivers/pinctrl/rockchip/pinctrl_rk3399.c b/drivers/pinctrl/rockchip/pinctrl_rk3399.c index 507bec4..6b62a0c 100644 --- a/drivers/pinctrl/rockchip/pinctrl_rk3399.c +++ b/drivers/pinctrl/rockchip/pinctrl_rk3399.c @@ -232,6 +232,24 @@ static void pinctrl_rk3399_gmac_config(struct rk3399_grf_regs *grf, int mmc_id) rk_clrsetreg(&grf->gpio3c_iomux, GRF_GPIO3C1_SEL_MASK, GRF_MAC_TXCLK << GRF_GPIO3C1_SEL_SHIFT);
- /* Set drive strength for GMAC tx io, value 3 means 13mA */
- rk_clrsetreg(&grf->gpio3_e[0],
GRF_GPIO3A0_E_MASK | GRF_GPIO3A1_E_MASK |GRF_GPIO3A4_E_MASK | GRF_GPIO3A5_E0_MASK,3 << GRF_GPIO3A0_E_SHIFT |3 << GRF_GPIO3A1_E_SHIFT |3 << GRF_GPIO3A4_E_SHIFT |1 << GRF_GPIO3A5_E0_SHIFT);- rk_clrsetreg(&grf->gpio3_e[1],
GRF_GPIO3A5_E12_MASK,1 << GRF_GPIO3A5_E12_SHIFT);- rk_clrsetreg(&grf->gpio3_e[2],
GRF_GPIO3B4_E_MASK,3 << GRF_GPIO3B4_E_SHIFT);- rk_clrsetreg(&grf->gpio3_e[4],
GRF_GPIO3C1_E_MASK,3 << GRF_GPIO3C1_E_SHIFT);} #endif
-- 1.9.1
Do you know if this is required for all board designs? We have a total run length of less than 2cm to the KSZ9031 PHY and wondered about this ourselves—our testing has shown that with these small distances (and the PHY we use) the setting doesn’t seem to be required.
If your layout is very good, it might work without this patch, did you test with 1000M Ethernet on many boards? With patch, we can keep the setting with kernel and make sure all the hardware able to work at 1000M mode. The firefly-rk3399 and rockchip rk3399-evb can't work at 1000M Ethernet mode without this patch.
Yes, we have full GbE in U-Boot (without this change) across the entire board population of our initial batch. This is most likely due to the very short distance between the RK3399 and PHY (there isn’t really an alternative to having it close due to the size constraints of the module).
Thanks for the background info on the EVB and Firefly.
Instead of hard-coding this: shouldn't we parse the drive-strength setting from the DTS?
Sync with the kernel pinctrl-rockchip can parse the drive-strength, but I don't think it's a good idea to sync that more than 2000 line file, this patch should be enough for U-Boot now.
Ok.
Regards, Philipp.
On 20 Apr 2017, at 10:48, Dr. Philipp Tomsich philipp.tomsich@theobroma-systems.com wrote:
On 20 Apr 2017, at 10:44, Kever Yang kever.yang@rock-chips.com wrote:
Hi Philipp,
Do you know if this is required for all board designs? We have a total run length of less than 2cm to the KSZ9031 PHY and wondered about this ourselves—our testing has shown that with these small distances (and the PHY we use) the setting doesn’t seem to be required.
If your layout is very good, it might work without this patch, did you test with 1000M Ethernet on many boards? With patch, we can keep the setting with kernel and make sure all the hardware able to work at 1000M mode. The firefly-rk3399 and rockchip rk3399-evb can't work at 1000M Ethernet mode without this patch.
Yes, we have full GbE in U-Boot (without this change) across the entire board population of our initial batch. This is most likely due to the very short distance between the RK3399 and PHY (there isn’t really an alternative to having it close due to the size constraints of the module).
Just for completeness, Linux with default drive strength (&pcfg_pull_none) works on puma-rk3399 too. Tested with iperf3 we get about 935Mbit with zero RX/TX errors.
Regards, Klaus
On 20 April 2017 at 02:15, Kever Yang kever.yang@rock-chips.com wrote:
GMAC controller need to init the tx io driver strength to 13mA, just like the description in dts pinctrl node, or else the controller may only work in 100MHz Mode, and fail to work at 1000MHz mode.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 75 +++++++++++++++++++++++-- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 18 ++++++ 2 files changed, 89 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On 20 April 2017 at 02:15, Kever Yang kever.yang@rock-chips.com wrote:
GMAC controller need to init the tx io driver strength to 13mA, just like the description in dts pinctrl node, or else the controller may only work in 100MHz Mode, and fail to work at 1000MHz mode.
Signed-off-by: Kever Yang kever.yang@rock-chips.com
arch/arm/include/asm/arch-rockchip/grf_rk3399.h | 75 +++++++++++++++++++++++-- drivers/pinctrl/rockchip/pinctrl_rk3399.c | 18 ++++++ 2 files changed, 89 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot-rockchip/next, thanks!
participants (5)
-
Dr. Philipp Tomsich -
Kever Yang -
Klaus Goger -
Simon Glass -
sjg＠google.com