[U-Boot] [PATCH] rockchip: pinctrl: rk3399: add gmac io strength support
Dr. Philipp Tomsich
philipp.tomsich at theobroma-systems.com
Thu Apr 20 08:48:00 UTC 2017
> On 20 Apr 2017, at 10:44, Kever Yang <kever.yang at 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 at 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 at 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.
More information about the U-Boot
mailing list