* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
@ 2017-04-20 8:15 Kever Yang
2017-04-20 8:21 ` Dr. Philipp Tomsich
2017-04-24 3:38 ` Simon Glass
0 siblings, 2 replies; 7+ messages in thread
From: Kever Yang @ 2017-04-20 8:15 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
2017-04-20 8:15 [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support Kever Yang
@ 2017-04-20 8:21 ` Dr. Philipp Tomsich
2017-04-20 8:44 ` Kever Yang
2017-04-24 3:38 ` Simon Glass
1 sibling, 1 reply; 7+ messages in thread
From: Dr. Philipp Tomsich @ 2017-04-20 8:21 UTC (permalink / raw)
To: u-boot
>
> 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 at theobroma-systems.com <mailto:philipp.tomsich@theobroma-systems.com>>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
2017-04-20 8:21 ` Dr. Philipp Tomsich
@ 2017-04-20 8:44 ` Kever Yang
2017-04-20 8:48 ` Dr. Philipp Tomsich
0 siblings, 1 reply; 7+ messages in thread
From: Kever Yang @ 2017-04-20 8:44 UTC (permalink / raw)
To: u-boot
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>>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
2017-04-20 8:44 ` Kever Yang
@ 2017-04-20 8:48 ` Dr. Philipp Tomsich
2017-04-20 11:00 ` Klaus Goger
0 siblings, 1 reply; 7+ messages in thread
From: Dr. Philipp Tomsich @ 2017-04-20 8:48 UTC (permalink / raw)
To: u-boot
> 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 at 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 at 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
2017-04-20 8:48 ` Dr. Philipp Tomsich
@ 2017-04-20 11:00 ` Klaus Goger
0 siblings, 0 replies; 7+ messages in thread
From: Klaus Goger @ 2017-04-20 11:00 UTC (permalink / raw)
To: u-boot
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
2017-04-20 8:15 [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support Kever Yang
2017-04-20 8:21 ` Dr. Philipp Tomsich
@ 2017-04-24 3:38 ` Simon Glass
2017-05-02 11:12 ` sjg at google.com
1 sibling, 1 reply; 7+ messages in thread
From: Simon Glass @ 2017-04-24 3:38 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
2017-04-24 3:38 ` Simon Glass
@ 2017-05-02 11:12 ` sjg at google.com
0 siblings, 0 replies; 7+ messages in thread
From: sjg at google.com @ 2017-05-02 11:12 UTC (permalink / raw)
To: u-boot
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!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-05-02 11:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-20 8:15 [U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support Kever Yang
2017-04-20 8:21 ` Dr. Philipp Tomsich
2017-04-20 8:44 ` Kever Yang
2017-04-20 8:48 ` Dr. Philipp Tomsich
2017-04-20 11:00 ` Klaus Goger
2017-04-24 3:38 ` Simon Glass
2017-05-02 11:12 ` sjg at google.com
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.