[PATCH 0/6] arm: sunxi: v3s: add ethernet support
This patchset enables ethernet on Allwinner V3/Sochip S3 SoCs. It is tested with an LicheePi Zero Dock board in combination with different switches and one direct link.
Test scenario: Download mainline kernel and device-tree over tftp and start kernel with nfs rootfs.
Without [PATCH 6/6], i wasn't able to get a stable connection on 100 Mb full duplex switches with autonegation enabled. Maybe the internal phy has a different behavior on softreset then others which results in a delayed established link.
Andreas Rehn (6): dts: sunxi: add licheepi-zero-dock clk: sunxi: v3s: Implement EMAC clocks/resets clk: sunxi: v3s: fix tabs / spaces net: sun8i-emac: add v3s pinmux setting dts: sunxi: v3s: enable emac support net: sun8i-emac: v3s: fix soft reset timeout
arch/arm/dts/Makefile | 3 ++- arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts | 11 +++++++++++ arch/arm/dts/sun8i-v3s.dtsi | 10 +++++++++- board/sunxi/MAINTAINERS | 5 +++++ configs/LicheePi_Zero_dock_defconfig | 7 +++++++ drivers/clk/sunxi/clk_v3s.c | 10 ++++++++-- drivers/net/sun8i_emac.c | 5 ++++- 7 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 configs/LicheePi_Zero_dock_defconfig
licheepi-zero dock is the second gen licheepi-zero board and brings addtional periperals like mic, speaker, ethernet, MIPI Camera Interface, 4 push buttons, second TF Card slot for Wifi or SD.
As the device tree is synchronized in a previous commit, just add it to Makefile, create a new MAINTAINER item and provide a defconfig.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- arch/arm/dts/Makefile | 3 ++- board/sunxi/MAINTAINERS | 5 +++++ configs/LicheePi_Zero_dock_defconfig | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 configs/LicheePi_Zero_dock_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 9c601a5c98..a5253ac112 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -609,7 +609,8 @@ dtb-$(CONFIG_MACH_SUN8I_R40) += \ sun8i-v40-bananapi-m2-berry.dtb dtb-$(CONFIG_MACH_SUN8I_V3S) += \ sun8i-s3-pinecube.dtb \ - sun8i-v3s-licheepi-zero.dtb + sun8i-v3s-licheepi-zero.dtb \ + sun8i-v3s-licheepi-zero-dock.dtb dtb-$(CONFIG_MACH_SUN50I_H5) += \ sun50i-h5-bananapi-m2-plus.dtb \ sun50i-h5-emlid-neutis-n5-devboard.dtb \ diff --git a/board/sunxi/MAINTAINERS b/board/sunxi/MAINTAINERS index 76eba2ad20..e956087b76 100644 --- a/board/sunxi/MAINTAINERS +++ b/board/sunxi/MAINTAINERS @@ -266,6 +266,11 @@ M: Icenowy Zheng icenowy@aosc.xyz S: Maintained F: configs/LicheePi_Zero_defconfig
+LICHEEPI-ZERO-DOCK BOARD +M: Icenowy Zheng icenowy@aosc.xyz +S: Maintained +F: configs/LicheePi_Zero_dock_defconfig + LINKSPRITE-PCDUINO BOARD M: Zoltan Herpai wigyori@uid0.hu S: Maintained diff --git a/configs/LicheePi_Zero_dock_defconfig b/configs/LicheePi_Zero_dock_defconfig new file mode 100644 index 0000000000..d890151f80 --- /dev/null +++ b/configs/LicheePi_Zero_dock_defconfig @@ -0,0 +1,7 @@ +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_SPL=y +CONFIG_MACH_SUN8I_V3S=y +CONFIG_DRAM_CLK=360 +CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero-dock" +CONFIG_SUN8I_EMAC=y
On Wed, 19 May 2021 21:42:03 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi Andreas,
licheepi-zero dock is the second gen licheepi-zero board and brings addtional periperals like mic, speaker, ethernet, MIPI Camera Interface, 4 push buttons, second TF Card slot for Wifi or SD.
As the device tree is synchronized in a previous commit, just add it to Makefile, create a new MAINTAINER item and provide a defconfig.
Thanks for upstreaming this!
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
arch/arm/dts/Makefile | 3 ++- board/sunxi/MAINTAINERS | 5 +++++ configs/LicheePi_Zero_dock_defconfig | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 configs/LicheePi_Zero_dock_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 9c601a5c98..a5253ac112 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -609,7 +609,8 @@ dtb-$(CONFIG_MACH_SUN8I_R40) += \ sun8i-v40-bananapi-m2-berry.dtb dtb-$(CONFIG_MACH_SUN8I_V3S) += \ sun8i-s3-pinecube.dtb \
- sun8i-v3s-licheepi-zero.dtb
- sun8i-v3s-licheepi-zero.dtb \
- sun8i-v3s-licheepi-zero-dock.dtb
dtb-$(CONFIG_MACH_SUN50I_H5) += \ sun50i-h5-bananapi-m2-plus.dtb \ sun50i-h5-emlid-neutis-n5-devboard.dtb \ diff --git a/board/sunxi/MAINTAINERS b/board/sunxi/MAINTAINERS index 76eba2ad20..e956087b76 100644 --- a/board/sunxi/MAINTAINERS +++ b/board/sunxi/MAINTAINERS @@ -266,6 +266,11 @@ M: Icenowy Zheng icenowy@aosc.xyz S: Maintained F: configs/LicheePi_Zero_defconfig
+LICHEEPI-ZERO-DOCK BOARD +M: Icenowy Zheng icenowy@aosc.xyz
Does she know about this? How it normally works is that the submitter of the defconfig takes "maintainership". Which mostly means that he or she owns the board and can be reached for questions or testing requests.
Cheers, Andre
+S: Maintained +F: configs/LicheePi_Zero_dock_defconfig
LINKSPRITE-PCDUINO BOARD M: Zoltan Herpai wigyori@uid0.hu S: Maintained diff --git a/configs/LicheePi_Zero_dock_defconfig b/configs/LicheePi_Zero_dock_defconfig new file mode 100644 index 0000000000..d890151f80 --- /dev/null +++ b/configs/LicheePi_Zero_dock_defconfig @@ -0,0 +1,7 @@ +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_SPL=y +CONFIG_MACH_SUN8I_V3S=y +CONFIG_DRAM_CLK=360 +CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero-dock" +CONFIG_SUN8I_EMAC=y
hey andre,
thx for the fast response!
I thought this would be the right choice since she did the initial work a long time ago. also, she maintains the lichee-zero (without dock) already.
sorry, this is my first patchset to u-boot and I'm not aware of the process.
greetings Andreas
Am Mi., 19. Mai 2021 um 23:42 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:03 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi Andreas,
licheepi-zero dock is the second gen licheepi-zero board and brings addtional periperals like mic, speaker, ethernet, MIPI Camera Interface, 4 push buttons, second TF Card slot for Wifi or SD.
As the device tree is synchronized in a previous commit, just add it to Makefile, create a new MAINTAINER item and provide a defconfig.
Thanks for upstreaming this!
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
arch/arm/dts/Makefile | 3 ++- board/sunxi/MAINTAINERS | 5 +++++ configs/LicheePi_Zero_dock_defconfig | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 configs/LicheePi_Zero_dock_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 9c601a5c98..a5253ac112 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -609,7 +609,8 @@ dtb-$(CONFIG_MACH_SUN8I_R40) += \ sun8i-v40-bananapi-m2-berry.dtb dtb-$(CONFIG_MACH_SUN8I_V3S) += \ sun8i-s3-pinecube.dtb \
sun8i-v3s-licheepi-zero.dtb
sun8i-v3s-licheepi-zero.dtb \sun8i-v3s-licheepi-zero-dock.dtbdtb-$(CONFIG_MACH_SUN50I_H5) += \ sun50i-h5-bananapi-m2-plus.dtb \ sun50i-h5-emlid-neutis-n5-devboard.dtb \ diff --git a/board/sunxi/MAINTAINERS b/board/sunxi/MAINTAINERS index 76eba2ad20..e956087b76 100644 --- a/board/sunxi/MAINTAINERS +++ b/board/sunxi/MAINTAINERS @@ -266,6 +266,11 @@ M: Icenowy Zheng icenowy@aosc.xyz S: Maintained F: configs/LicheePi_Zero_defconfig
+LICHEEPI-ZERO-DOCK BOARD +M: Icenowy Zheng icenowy@aosc.xyz
Does she know about this? How it normally works is that the submitter of the defconfig takes "maintainership". Which mostly means that he or she owns the board and can be reached for questions or testing requests.
Cheers, Andre
+S: Maintained +F: configs/LicheePi_Zero_dock_defconfig
LINKSPRITE-PCDUINO BOARD M: Zoltan Herpai wigyori@uid0.hu S: Maintained diff --git a/configs/LicheePi_Zero_dock_defconfig
b/configs/LicheePi_Zero_dock_defconfig
new file mode 100644 index 0000000000..d890151f80 --- /dev/null +++ b/configs/LicheePi_Zero_dock_defconfig @@ -0,0 +1,7 @@ +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_SPL=y +CONFIG_MACH_SUN8I_V3S=y +CONFIG_DRAM_CLK=360 +CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero-dock" +CONFIG_SUN8I_EMAC=y
于 2021年5月20日 GMT+08:00 上午5:42:38, Andre Przywara andre.przywara@arm.com 写到:
On Wed, 19 May 2021 21:42:03 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi Andreas,
licheepi-zero dock is the second gen licheepi-zero board and brings addtional periperals like mic, speaker, ethernet, MIPI Camera Interface, 4 push buttons, second TF Card slot for Wifi or SD.
As the device tree is synchronized in a previous commit, just add it
to
Makefile, create a new MAINTAINER item and provide a defconfig.
Thanks for upstreaming this!
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
arch/arm/dts/Makefile | 3 ++- board/sunxi/MAINTAINERS | 5 +++++ configs/LicheePi_Zero_dock_defconfig | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 configs/LicheePi_Zero_dock_defconfig
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index 9c601a5c98..a5253ac112 100644 --- a/arch/arm/dts/Makefile +++ b/arch/arm/dts/Makefile @@ -609,7 +609,8 @@ dtb-$(CONFIG_MACH_SUN8I_R40) += \ sun8i-v40-bananapi-m2-berry.dtb dtb-$(CONFIG_MACH_SUN8I_V3S) += \ sun8i-s3-pinecube.dtb \
- sun8i-v3s-licheepi-zero.dtb
- sun8i-v3s-licheepi-zero.dtb \
- sun8i-v3s-licheepi-zero-dock.dtb
dtb-$(CONFIG_MACH_SUN50I_H5) += \ sun50i-h5-bananapi-m2-plus.dtb \ sun50i-h5-emlid-neutis-n5-devboard.dtb \ diff --git a/board/sunxi/MAINTAINERS b/board/sunxi/MAINTAINERS index 76eba2ad20..e956087b76 100644 --- a/board/sunxi/MAINTAINERS +++ b/board/sunxi/MAINTAINERS @@ -266,6 +266,11 @@ M: Icenowy Zheng icenowy@aosc.xyz S: Maintained F: configs/LicheePi_Zero_defconfig
+LICHEEPI-ZERO-DOCK BOARD +M: Icenowy Zheng icenowy@aosc.xyz
Does she know about this? How it normally works is that the submitter of the defconfig takes "maintainership". Which mostly means that he or she owns the board and can be reached for questions or testing requests.
Well I do have it, and I can be responsible for it.
Cheers, Andre
+S: Maintained +F: configs/LicheePi_Zero_dock_defconfig
LINKSPRITE-PCDUINO BOARD M: Zoltan Herpai wigyori@uid0.hu S: Maintained diff --git a/configs/LicheePi_Zero_dock_defconfig
b/configs/LicheePi_Zero_dock_defconfig
new file mode 100644 index 0000000000..d890151f80 --- /dev/null +++ b/configs/LicheePi_Zero_dock_defconfig @@ -0,0 +1,7 @@ +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_SPL=y +CONFIG_MACH_SUN8I_V3S=y +CONFIG_DRAM_CLK=360 +CONFIG_DEFAULT_DEVICE_TREE="sun8i-v3s-licheepi-zero-dock" +CONFIG_SUN8I_EMAC=y
Add emac clock and reset register/bits.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- drivers/clk/sunxi/clk_v3s.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 29622199fd..55fc597043 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -17,6 +17,7 @@ static struct ccu_clk_gate v3s_gates[] = { [CLK_BUS_MMC0] = GATE(0x060, BIT(8)), [CLK_BUS_MMC1] = GATE(0x060, BIT(9)), [CLK_BUS_MMC2] = GATE(0x060, BIT(10)), + [CLK_BUS_EMAC] = GATE(0x060, BIT(17)), [CLK_BUS_SPI0] = GATE(0x060, BIT(20)), [CLK_BUS_OTG] = GATE(0x060, BIT(24)),
@@ -24,6 +25,8 @@ static struct ccu_clk_gate v3s_gates[] = { [CLK_BUS_UART1] = GATE(0x06c, BIT(17)), [CLK_BUS_UART2] = GATE(0x06c, BIT(18)),
+ [CLK_BUS_EPHY] = GATE(0x070, BIT(0)), + [CLK_SPI0] = GATE(0x0a0, BIT(31)),
[CLK_USB_PHY0] = GATE(0x0cc, BIT(8)), @@ -35,9 +38,12 @@ static struct ccu_reset v3s_resets[] = { [RST_BUS_MMC0] = RESET(0x2c0, BIT(8)), [RST_BUS_MMC1] = RESET(0x2c0, BIT(9)), [RST_BUS_MMC2] = RESET(0x2c0, BIT(10)), + [RST_BUS_EMAC] = RESET(0x2c0, BIT(17)), [RST_BUS_SPI0] = RESET(0x2c0, BIT(20)), [RST_BUS_OTG] = RESET(0x2c0, BIT(24)),
+ [RST_BUS_EPHY] = RESET(0x2c8, BIT(2)), + [RST_BUS_UART0] = RESET(0x2d8, BIT(16)), [RST_BUS_UART1] = RESET(0x2d8, BIT(17)), [RST_BUS_UART2] = RESET(0x2d8, BIT(18)),
On Wed, 19 May 2021 21:42:04 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Add emac clock and reset register/bits.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
Compared against the manual and the Linux driver.
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
drivers/clk/sunxi/clk_v3s.c | 6 ++++++ 1 file changed, 6 insertions(+)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 29622199fd..55fc597043 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -17,6 +17,7 @@ static struct ccu_clk_gate v3s_gates[] = { [CLK_BUS_MMC0] = GATE(0x060, BIT(8)), [CLK_BUS_MMC1] = GATE(0x060, BIT(9)), [CLK_BUS_MMC2] = GATE(0x060, BIT(10)),
- [CLK_BUS_EMAC] = GATE(0x060, BIT(17)), [CLK_BUS_SPI0] = GATE(0x060, BIT(20)), [CLK_BUS_OTG] = GATE(0x060, BIT(24)),
@@ -24,6 +25,8 @@ static struct ccu_clk_gate v3s_gates[] = { [CLK_BUS_UART1] = GATE(0x06c, BIT(17)), [CLK_BUS_UART2] = GATE(0x06c, BIT(18)),
[CLK_BUS_EPHY] = GATE(0x070, BIT(0)),
[CLK_SPI0] = GATE(0x0a0, BIT(31)),
[CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
@@ -35,9 +38,12 @@ static struct ccu_reset v3s_resets[] = { [RST_BUS_MMC0] = RESET(0x2c0, BIT(8)), [RST_BUS_MMC1] = RESET(0x2c0, BIT(9)), [RST_BUS_MMC2] = RESET(0x2c0, BIT(10)),
[RST_BUS_EMAC] = RESET(0x2c0, BIT(17)), [RST_BUS_SPI0] = RESET(0x2c0, BIT(20)), [RST_BUS_OTG] = RESET(0x2c0, BIT(24)),
[RST_BUS_EPHY] = RESET(0x2c8, BIT(2)),
[RST_BUS_UART0] = RESET(0x2d8, BIT(16)), [RST_BUS_UART1] = RESET(0x2d8, BIT(17)), [RST_BUS_UART2] = RESET(0x2d8, BIT(18)),
align CLK_SPI0 and CLK_USB_PHY0 with tabs
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- drivers/clk/sunxi/clk_v3s.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 55fc597043..9c2717bfab 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -27,9 +27,9 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_EPHY] = GATE(0x070, BIT(0)),
- [CLK_SPI0] = GATE(0x0a0, BIT(31)), + [CLK_SPI0] = GATE(0x0a0, BIT(31)),
- [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)), + [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)), };
static struct ccu_reset v3s_resets[] = {
On Wed, 19 May 2021 21:42:05 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
align CLK_SPI0 and CLK_USB_PHY0 with tabs
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/clk/sunxi/clk_v3s.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 55fc597043..9c2717bfab 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -27,9 +27,9 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_EPHY] = GATE(0x070, BIT(0)),
- [CLK_SPI0] = GATE(0x0a0, BIT(31)),
- [CLK_SPI0] = GATE(0x0a0, BIT(31)),
What is the problem with this line? This is already using tabs and is correctly aligned already? Are you using a tab length other than 8, by any chance?
- [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
- [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
This change looks fine.
Cheers, Andre.
};
static struct ccu_reset v3s_resets[] = {
Both didn't align with the rest of the list on my side.
this patch is not important for the functionality so we can drop this if this only apppears on my machine.
greetings Andreas
Am Mi., 19. Mai 2021 um 23:43 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:05 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
align CLK_SPI0 and CLK_USB_PHY0 with tabs
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/clk/sunxi/clk_v3s.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 55fc597043..9c2717bfab 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -27,9 +27,9 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_BUS_EPHY] = GATE(0x070, BIT(0)),
[CLK_SPI0] = GATE(0x0a0, BIT(31)),
[CLK_SPI0] = GATE(0x0a0, BIT(31)),What is the problem with this line? This is already using tabs and is correctly aligned already? Are you using a tab length other than 8, by any chance?
[CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
[CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),This change looks fine.
Cheers, Andre.
};
static struct ccu_reset v3s_resets[] = {
align CLK_USB_PHY0 with tabs
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- Changes in v2: - revert CLK_SPI0 extra tab
drivers/clk/sunxi/clk_v3s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 55fc597043..bc6b7b4870 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -29,7 +29,7 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_SPI0] = GATE(0x0a0, BIT(31)),
- [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)), + [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)), };
static struct ccu_reset v3s_resets[] = {
On Sun, 23 May 2021 01:17:29 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
align CLK_USB_PHY0 with tabs
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
P.S. Please send a whole v2 series next time, to make this easier to sort out which patch still applies and which not.
Changes in v2:
- revert CLK_SPI0 extra tab
drivers/clk/sunxi/clk_v3s.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/sunxi/clk_v3s.c b/drivers/clk/sunxi/clk_v3s.c index 55fc597043..bc6b7b4870 100644 --- a/drivers/clk/sunxi/clk_v3s.c +++ b/drivers/clk/sunxi/clk_v3s.c @@ -29,7 +29,7 @@ static struct ccu_clk_gate v3s_gates[] = {
[CLK_SPI0] = GATE(0x0a0, BIT(31)),
- [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
- [CLK_USB_PHY0] = GATE(0x0cc, BIT(8)),
};
static struct ccu_reset v3s_resets[] = {
Hi Andre,
On 5/26/21 7:16 PM, Andre Przywara wrote:
On Sun, 23 May 2021 01:17:29 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
align CLK_USB_PHY0 with tabs
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
P.S. Please send a whole v2 series next time, to make this easier to sort out which patch still applies and which not.
It looks like this never got applied (despite being marked as "accepted" in patchwork). Do you want me to pick it up?
--Sean
Driver uses pinmux instead of emac type. Add v3s pinmux to support SoC.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- drivers/net/sun8i_emac.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..0e7ad3b0d4 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -89,6 +89,7 @@ #define SUN8I_IOMUX_R40 5 #define SUN8I_IOMUX_H6 5 #define SUN8I_IOMUX_H616 2 +#define SUN8I_IOMUX_V3S 2 #define SUN8I_IOMUX 4
/* H3/A64 EMAC Register's offset */ @@ -566,6 +567,8 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX; + else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) + iomux = SUN8I_IOMUX_V3S; else BUILD_BUG_ON_MSG(1, "missing pinmux value for Ethernet pins");
On Wed, 19 May 2021 21:42:06 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
Driver uses pinmux instead of emac type. Add v3s pinmux to support SoC.
So if I understand this correctly, then the v3s does NOT expose the MAC pins (MII/RMII/RGMII) on its package (only the V3 does this)? Instead there are internal pins, connecting the MAC directly to the internal PHY only. Those don't need to be muxed, so do not require a pinctrl-0 property.
So this whole patch would not be necessary.
If you want to avoid the message, please send a patch for that.
Cheers, Andre
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..0e7ad3b0d4 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -89,6 +89,7 @@ #define SUN8I_IOMUX_R40 5 #define SUN8I_IOMUX_H6 5 #define SUN8I_IOMUX_H616 2 +#define SUN8I_IOMUX_V3S 2 #define SUN8I_IOMUX 4
/* H3/A64 EMAC Register's offset */ @@ -566,6 +567,8 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX;
- else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S))
iomux = SUN8I_IOMUX_V3S;
于 2021年5月20日 GMT+08:00 上午5:44:07, Andre Przywara andre.przywara@arm.com 写到:
On Wed, 19 May 2021 21:42:06 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
Driver uses pinmux instead of emac type. Add v3s pinmux to support SoC.
So if I understand this correctly, then the v3s does NOT expose the MAC pins (MII/RMII/RGMII) on its package (only the V3 does this)?
Yes. *MII is at PD GPIO bank and PD is just not wired out on V3s.
(on V3/S3 it is, but I haven't seen a board that utilizes external PHY.)
Instead there are internal pins, connecting the MAC directly to the internal PHY only. Those don't need to be muxed, so do not require a pinctrl-0 property.
So this whole patch would not be necessary.
If you want to avoid the message, please send a patch for that.
Cheers, Andre
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..0e7ad3b0d4 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -89,6 +89,7 @@ #define SUN8I_IOMUX_R40 5 #define SUN8I_IOMUX_H6 5 #define SUN8I_IOMUX_H616 2 +#define SUN8I_IOMUX_V3S 2 #define SUN8I_IOMUX 4
/* H3/A64 EMAC Register's offset */ @@ -566,6 +567,8 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX;
- else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S))
iomux = SUN8I_IOMUX_V3S;
Add variant V3S_EMAC. Handle pinmux compile time error by skipping goio setup, because V3s uses internal phy and don't expose pins.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- Changes in v2: - skip pinmux and add proper description - Add V3S variant add it to compatible list - Skip (R)GMII flags and handle sun8i_handle_internal_phy
drivers/net/sun8i_emac.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..ab9f61994c 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -145,6 +145,7 @@ enum emac_variant { A64_EMAC, R40_GMAC, H6_EMAC, + V3S_EMAC, };
struct emac_dma_desc { @@ -303,7 +304,7 @@ static void sun8i_adjust_link(struct emac_eth_dev *priv, static u32 sun8i_emac_set_syscon_ephy(struct emac_eth_dev *priv, u32 reg) { if (priv->use_internal_phy) { - /* H3 based SoC's that has an Internal 100MBit PHY + /* H3 and V3s based SoC's that has an Internal 100MBit PHY * needs to be configured and powered up before use */ reg &= ~H3_EPHY_DEFAULT_MASK; @@ -354,7 +355,8 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID: - reg |= SC_EPIT | SC_ETCS_INT_GMII; + if (priv->variant != V3S_EMAC) + reg |= SC_EPIT | SC_ETCS_INT_GMII; break; case PHY_INTERFACE_MODE_RMII: if (priv->variant == H3_EMAC || @@ -566,6 +568,10 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX; + else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) + // V3s does not expose any MAC pins, + // but case is required to handle BUILD_BUG_ON_MSG. + return 0; else BUILD_BUG_ON_MSG(1, "missing pinmux value for Ethernet pins");
@@ -956,7 +962,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) return -EINVAL; }
- if (priv->variant == H3_EMAC) { + if (priv->variant == H3_EMAC || + priv->variant == V3S_EMAC) { ret = sun8i_handle_internal_phy(dev, priv); if (ret) return ret; @@ -1009,6 +1016,8 @@ static const struct udevice_id sun8i_emac_eth_ids[] = { .data = (uintptr_t)R40_GMAC }, {.compatible = "allwinner,sun50i-h6-emac", .data = (uintptr_t)H6_EMAC }, + {.compatible = "allwinner,sun8i-v3s-emac", + .data = (uintptr_t)V3S_EMAC }, { } };
On Sun, May 23, 2021 at 2:23 AM Andreas Rehn rehn.andreas86@gmail.com wrote:
Add variant V3S_EMAC. Handle pinmux compile time error by skipping goio setup, because V3s uses internal phy and don't expose pins.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
Changes in v2: - skip pinmux and add proper description - Add V3S variant add it to compatible list - Skip (R)GMII flags and handle sun8i_handle_internal_phy
drivers/net/sun8i_emac.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..ab9f61994c 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -145,6 +145,7 @@ enum emac_variant { A64_EMAC, R40_GMAC, H6_EMAC,
V3S_EMAC,};
struct emac_dma_desc { @@ -303,7 +304,7 @@ static void sun8i_adjust_link(struct emac_eth_dev *priv, static u32 sun8i_emac_set_syscon_ephy(struct emac_eth_dev *priv, u32 reg) { if (priv->use_internal_phy) {
/* H3 based SoC's that has an Internal 100MBit PHY
/* H3 and V3s based SoC's that has an Internal 100MBit PHY * needs to be configured and powered up before use */ reg &= ~H3_EPHY_DEFAULT_MASK;@@ -354,7 +355,8 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID:
reg |= SC_EPIT | SC_ETCS_INT_GMII;
if (priv->variant != V3S_EMAC)reg |= SC_EPIT | SC_ETCS_INT_GMII; break; case PHY_INTERFACE_MODE_RMII: if (priv->variant == H3_EMAC ||@@ -566,6 +568,10 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX;
else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S))// V3s does not expose any MAC pins,// but case is required to handle BUILD_BUG_ON_MSG.return 0; else BUILD_BUG_ON_MSG(1, "missing pinmux value for Ethernet pins");@@ -956,7 +962,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) return -EINVAL; }
if (priv->variant == H3_EMAC) {
if (priv->variant == H3_EMAC ||priv->variant == V3S_EMAC) { ret = sun8i_handle_internal_phy(dev, priv); if (ret) return ret;@@ -1009,6 +1016,8 @@ static const struct udevice_id sun8i_emac_eth_ids[] = { .data = (uintptr_t)R40_GMAC }, {.compatible = "allwinner,sun50i-h6-emac", .data = (uintptr_t)H6_EMAC },
{.compatible = "allwinner,sun8i-v3s-emac",.data = (uintptr_t)V3S_EMAC }, { }};
-- 2.25.1
Reviewed-by: Ramon Fried <rfried.dev@gmail.com >
On Sun, 23 May 2021 01:22:38 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
Add variant V3S_EMAC. Handle pinmux compile time error by skipping goio setup, because V3s uses internal phy and don't expose pins.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
Reviewed-by: Andre Przywara andre.przywara@arm.com
Cheers, Andre
Changes in v2:
- skip pinmux and add proper description
- Add V3S variant add it to compatible list
- Skip (R)GMII flags and handle sun8i_handle_internal_phy
drivers/net/sun8i_emac.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..ab9f61994c 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -145,6 +145,7 @@ enum emac_variant { A64_EMAC, R40_GMAC, H6_EMAC,
- V3S_EMAC,
};
struct emac_dma_desc { @@ -303,7 +304,7 @@ static void sun8i_adjust_link(struct emac_eth_dev *priv, static u32 sun8i_emac_set_syscon_ephy(struct emac_eth_dev *priv, u32 reg) { if (priv->use_internal_phy) {
/* H3 based SoC's that has an Internal 100MBit PHY
/* H3 and V3s based SoC's that has an Internal 100MBit PHY*/ reg &= ~H3_EPHY_DEFAULT_MASK;
- needs to be configured and powered up before use
@@ -354,7 +355,8 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID:
reg |= SC_EPIT | SC_ETCS_INT_GMII;
if (priv->variant != V3S_EMAC)reg |= SC_EPIT | SC_ETCS_INT_GMII;@@ -566,6 +568,10 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX;
- else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S))
// V3s does not expose any MAC pins,// but case is required to handle BUILD_BUG_ON_MSG.return 0;@@ -956,7 +962,8 @@ static int sun8i_emac_eth_of_to_plat(struct udevice *dev) return -EINVAL; }
- if (priv->variant == H3_EMAC) {
- if (priv->variant == H3_EMAC ||
priv->variant == V3S_EMAC) {@@ -1009,6 +1016,8 @@ static const struct udevice_id sun8i_emac_eth_ids[] = { .data = (uintptr_t)R40_GMAC }, {.compatible = "allwinner,sun50i-h6-emac", .data = (uintptr_t)H6_EMAC },
- {.compatible = "allwinner,sun8i-v3s-emac",
.data = (uintptr_t)V3S_EMAC },};
On Sun, May 23, 2021 at 4:52 AM Andreas Rehn rehn.andreas86@gmail.com wrote:
Add variant V3S_EMAC. Handle pinmux compile time error by skipping goio setup, because V3s uses internal phy and don't expose pins.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
Changes in v2: - skip pinmux and add proper description - Add V3S variant add it to compatible list - Skip (R)GMII flags and handle sun8i_handle_internal_phy
drivers/net/sun8i_emac.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 5a1b38bf80..ab9f61994c 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -145,6 +145,7 @@ enum emac_variant { A64_EMAC, R40_GMAC, H6_EMAC,
V3S_EMAC,};
struct emac_dma_desc { @@ -303,7 +304,7 @@ static void sun8i_adjust_link(struct emac_eth_dev *priv, static u32 sun8i_emac_set_syscon_ephy(struct emac_eth_dev *priv, u32 reg) { if (priv->use_internal_phy) {
/* H3 based SoC's that has an Internal 100MBit PHY
/* H3 and V3s based SoC's that has an Internal 100MBit PHY * needs to be configured and powered up before use */ reg &= ~H3_EPHY_DEFAULT_MASK;@@ -354,7 +355,8 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata, case PHY_INTERFACE_MODE_RGMII_ID: case PHY_INTERFACE_MODE_RGMII_RXID: case PHY_INTERFACE_MODE_RGMII_TXID:
reg |= SC_EPIT | SC_ETCS_INT_GMII;
if (priv->variant != V3S_EMAC)reg |= SC_EPIT | SC_ETCS_INT_GMII; break; case PHY_INTERFACE_MODE_RMII: if (priv->variant == H3_EMAC ||@@ -566,6 +568,10 @@ static int parse_phy_pins(struct udevice *dev) iomux = SUN8I_IOMUX; else if (IS_ENABLED(CONFIG_MACH_SUN50I)) iomux = SUN8I_IOMUX;
else if (IS_ENABLED(CONFIG_MACH_SUN8I_V3S))// V3s does not expose any MAC pins,// but case is required to handle BUILD_BUG_ON_MSG.
Wrong multi-line comment. please fix it?
Jagan.
h3 and v3s have internal phys and can share the same driver. Furthermore sun8i-v3s-emac is not available, use sun8i-h3-emac instead - add emac pins - enable emac for licheepi-zero-dock as it provides a ethernet port
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts | 11 +++++++++++ arch/arm/dts/sun8i-v3s.dtsi | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts index db5cd0b857..083ac11b94 100644 --- a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts +++ b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts @@ -49,6 +49,10 @@ compatible = "licheepi,licheepi-zero-dock", "licheepi,licheepi-zero", "allwinner,sun8i-v3s";
+ aliases { + ethernet0 = &emac; + }; + leds { /* The LEDs use PG0~2 pins, which conflict with MMC1 */ status = "disabled"; @@ -94,3 +98,10 @@ voltage = <800000>; }; }; + +&emac { + allwinner,leds-active-low; + status = "okay"; + pinctrl-names = "default"; + pinctrl-0 = <&emac_rgmii_pins>; +}; diff --git a/arch/arm/dts/sun8i-v3s.dtsi b/arch/arm/dts/sun8i-v3s.dtsi index 0c73416769..35cc4d63f7 100644 --- a/arch/arm/dts/sun8i-v3s.dtsi +++ b/arch/arm/dts/sun8i-v3s.dtsi @@ -342,6 +342,14 @@ function = "csi"; };
+ emac_rgmii_pins: emac-rgmii-pins { + pins = "PD0", "PD1", "PD2", "PD3", "PD4", + "PD5", "PD7", "PD8", "PD9", "PD10", + "PD12", "PD13", "PD15", "PD16", "PD17"; + function = "emac"; + drive-strength = <40>; + }; + i2c0_pins: i2c0-pins { pins = "PB6", "PB7"; function = "i2c0"; @@ -468,7 +476,7 @@ };
emac: ethernet@1c30000 { - compatible = "allwinner,sun8i-v3s-emac"; + compatible = "allwinner,sun8i-h3-emac"; syscon = <&syscon>; reg = <0x01c30000 0x10000>; interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
On Wed, 19 May 2021 21:42:07 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
h3 and v3s have internal phys and can share the same driver. Furthermore sun8i-v3s-emac is not available, use sun8i-h3-emac instead
- add emac pins
- enable emac for licheepi-zero-dock as it provides a ethernet port
So first, this is not how we handle DT changes in U-Boot. They would need to go through the Linux tree first, then can be synced back to U-Boot. Sorry.
Looking more into the details:
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts | 11 +++++++++++ arch/arm/dts/sun8i-v3s.dtsi | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts index db5cd0b857..083ac11b94 100644 --- a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts +++ b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts @@ -49,6 +49,10 @@ compatible = "licheepi,licheepi-zero-dock", "licheepi,licheepi-zero", "allwinner,sun8i-v3s";
- aliases {
ethernet0 = &emac;- };
- leds { /* The LEDs use PG0~2 pins, which conflict with MMC1 */ status = "disabled";
@@ -94,3 +98,10 @@ voltage = <800000>; }; };
+&emac {
- allwinner,leds-active-low;
- status = "okay";
- pinctrl-names = "default";
- pinctrl-0 = <&emac_rgmii_pins>;
I don't think this is correct. If I understand correctly, the V3s does not expose any MAC pins, instead relies entirely on the internal PHY. Those pins are not muxed, so don't need any pinctrl properties.
+}; diff --git a/arch/arm/dts/sun8i-v3s.dtsi b/arch/arm/dts/sun8i-v3s.dtsi index 0c73416769..35cc4d63f7 100644 --- a/arch/arm/dts/sun8i-v3s.dtsi +++ b/arch/arm/dts/sun8i-v3s.dtsi @@ -342,6 +342,14 @@ function = "csi"; };
emac_rgmii_pins: emac-rgmii-pins {pins = "PD0", "PD1", "PD2", "PD3", "PD4","PD5", "PD7", "PD8", "PD9", "PD10","PD12", "PD13", "PD15", "PD16", "PD17";function = "emac";drive-strength = <40>;};
This is wrong (and not needed): The V3s does not expose MAC pins. If I understand correctly, the V3 and V3s share the same die, so the pin controller has those registers, but the whole port is connected nowhere.
i2c0_pins: i2c0-pins { pins = "PB6", "PB7"; function = "i2c0";@@ -468,7 +476,7 @@ };
emac: ethernet@1c30000 {
compatible = "allwinner,sun8i-v3s-emac";
compatible = "allwinner,sun8i-h3-emac";
You can't just change the compatible string this way, the original one is there for a reason. In this case the difference is that the V3s does not support Gigabit Ethernet - because the only MAC pins connected are the internal MII ones. I believe the MAC itself could probably still handle GBit, but it can't talk to the outside in this mode.
Instead just add the v3s compatible string to the sun8i-emac driver. Assign a new type and add this new type wherever you see H3_EMAC, but not in the RGMII part.
Cheers, Andre
syscon = <&syscon>; reg = <0x01c30000 0x10000>; interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
于 2021年5月20日 GMT+08:00 上午5:44:30, Andre Przywara andre.przywara@arm.com 写到:
On Wed, 19 May 2021 21:42:07 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
h3 and v3s have internal phys and can share the same driver. Furthermore sun8i-v3s-emac is not available, use sun8i-h3-emac
instead
- add emac pins
- enable emac for licheepi-zero-dock as it provides a ethernet port
So first, this is not how we handle DT changes in U-Boot. They would need to go through the Linux tree first, then can be synced back to U-Boot. Sorry.
Looking more into the details:
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts | 11 +++++++++++ arch/arm/dts/sun8i-v3s.dtsi | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts
b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts
index db5cd0b857..083ac11b94 100644 --- a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts +++ b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts @@ -49,6 +49,10 @@ compatible = "licheepi,licheepi-zero-dock",
"licheepi,licheepi-zero",
"allwinner,sun8i-v3s";
- aliases {
ethernet0 = &emac;- };
- leds { /* The LEDs use PG0~2 pins, which conflict with MMC1 */ status = "disabled";
@@ -94,3 +98,10 @@ voltage = <800000>; }; };
+&emac {
- allwinner,leds-active-low;
- status = "okay";
- pinctrl-names = "default";
- pinctrl-0 = <&emac_rgmii_pins>;
I don't think this is correct. If I understand correctly, the V3s does not expose any MAC pins, instead relies entirely on the internal PHY. Those pins are not muxed, so don't need any pinctrl properties.
+}; diff --git a/arch/arm/dts/sun8i-v3s.dtsi
b/arch/arm/dts/sun8i-v3s.dtsi
index 0c73416769..35cc4d63f7 100644 --- a/arch/arm/dts/sun8i-v3s.dtsi +++ b/arch/arm/dts/sun8i-v3s.dtsi @@ -342,6 +342,14 @@ function = "csi"; };
emac_rgmii_pins: emac-rgmii-pins {pins = "PD0", "PD1", "PD2", "PD3", "PD4","PD5", "PD7", "PD8", "PD9", "PD10","PD12", "PD13", "PD15", "PD16", "PD17";function = "emac";drive-strength = <40>;};This is wrong (and not needed): The V3s does not expose MAC pins. If I understand correctly, the V3 and V3s share the same die, so the pin controller has those registers, but the whole port is connected nowhere.
i2c0_pins: i2c0-pins { pins = "PB6", "PB7"; function = "i2c0";@@ -468,7 +476,7 @@ };
emac: ethernet@1c30000 {
compatible = "allwinner,sun8i-v3s-emac";
compatible = "allwinner,sun8i-h3-emac";You can't just change the compatible string this way, the original one is there for a reason. In this case the difference is that the V3s does not support Gigabit Ethernet - because the only MAC pins connected are the internal MII ones. I believe the MAC itself could probably still handle GBit, but it can't talk to the outside in this mode.
You got it right. However, we choose to just use v3s compatible on V3/S3 too.
Instead just add the v3s compatible string to the sun8i-emac driver. Assign a new type and add this new type wherever you see H3_EMAC, but not in the RGMII part.
Maybe we can just make v3s situation a copy of h3 one.
Cheers, Andre
syscon = <&syscon>; reg = <0x01c30000 0x10000>; interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
On Fri, 21 May 2021 17:41:37 +0800 Icenowy Zheng icenowy@aosc.io wrote:
Hi Icenowy,
于 2021年5月20日 GMT+08:00 上午5:44:30, Andre Przywara andre.przywara@arm.com 写到:
On Wed, 19 May 2021 21:42:07 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
h3 and v3s have internal phys and can share the same driver. Furthermore sun8i-v3s-emac is not available, use sun8i-h3-emac
instead
- add emac pins
- enable emac for licheepi-zero-dock as it provides a ethernet port
So first, this is not how we handle DT changes in U-Boot. They would need to go through the Linux tree first, then can be synced back to U-Boot. Sorry.
Looking more into the details:
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts | 11 +++++++++++ arch/arm/dts/sun8i-v3s.dtsi | 10 +++++++++- 2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts
b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts
index db5cd0b857..083ac11b94 100644 --- a/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts +++ b/arch/arm/dts/sun8i-v3s-licheepi-zero-dock.dts @@ -49,6 +49,10 @@ compatible = "licheepi,licheepi-zero-dock",
"licheepi,licheepi-zero",
"allwinner,sun8i-v3s";
- aliases {
ethernet0 = &emac;- };
- leds { /* The LEDs use PG0~2 pins, which conflict with MMC1 */ status = "disabled";
@@ -94,3 +98,10 @@ voltage = <800000>; }; };
+&emac {
- allwinner,leds-active-low;
- status = "okay";
- pinctrl-names = "default";
- pinctrl-0 = <&emac_rgmii_pins>;
I don't think this is correct. If I understand correctly, the V3s does not expose any MAC pins, instead relies entirely on the internal PHY. Those pins are not muxed, so don't need any pinctrl properties.
+}; diff --git a/arch/arm/dts/sun8i-v3s.dtsi
b/arch/arm/dts/sun8i-v3s.dtsi
index 0c73416769..35cc4d63f7 100644 --- a/arch/arm/dts/sun8i-v3s.dtsi +++ b/arch/arm/dts/sun8i-v3s.dtsi @@ -342,6 +342,14 @@ function = "csi"; };
emac_rgmii_pins: emac-rgmii-pins {pins = "PD0", "PD1", "PD2", "PD3", "PD4","PD5", "PD7", "PD8", "PD9", "PD10","PD12", "PD13", "PD15", "PD16", "PD17";function = "emac";drive-strength = <40>;};This is wrong (and not needed): The V3s does not expose MAC pins. If I understand correctly, the V3 and V3s share the same die, so the pin controller has those registers, but the whole port is connected nowhere.
i2c0_pins: i2c0-pins { pins = "PB6", "PB7"; function = "i2c0";@@ -468,7 +476,7 @@ };
emac: ethernet@1c30000 {
compatible = "allwinner,sun8i-v3s-emac";
compatible = "allwinner,sun8i-h3-emac";You can't just change the compatible string this way, the original one is there for a reason. In this case the difference is that the V3s does not support Gigabit Ethernet - because the only MAC pins connected are the internal MII ones. I believe the MAC itself could probably still handle GBit, but it can't talk to the outside in this mode.
You got it right. However, we choose to just use v3s compatible on V3/S3 too.
Thanks for the confirmation!
Instead just add the v3s compatible string to the sun8i-emac driver. Assign a new type and add this new type wherever you see H3_EMAC, but not in the RGMII part.
Maybe we can just make v3s situation a copy of h3 one.
Yes, that would work in practice, since we realistically would just never request RGMII through V3s DTs. But it's also easy to solve this properly: case PHY_INTERFACE_MODE_RGMII: .... if (priv->variant != V3S_EMAC) reg |= SC_EPIT | SC_ETCS_INT_GMII;
Cheers, Andre.
Cheers, Andre
syscon = <&syscon>; reg = <0x01c30000 0x10000>; interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev) /* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1, - EMAC_CTL1_SOFT_RST, false, 10, true); + EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev) /* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true);
hey,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp download. after a manual retry, it works fine but retry is not a valid production behavior.
greetings Andreas
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev) /* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
On Thu, 20 May 2021 00:10:47 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp download. after a manual retry, it works fine but retry is not a valid production behavior.
Well, I see occasional TFTP hiccups as well, across different boards. They are never fatal, the TFTP protocol just triggers a re-transmission. It's mostly an annoyance.
Some time ago I tried to debug this further, but couldn't find the real reason for this. I was always tempted to shorten the TFTP timeout, as packet loss can happen anyway (this is UDP!), and the default timeout of 5 secs sounds ridiculously long (but is mentioned in the original RFC, IIRC).
Anyway I doubt that this timeout value has any real impact: the soft reset bit automatically clears when it's reset, so this wait is just a safety measure to avoid waiting forever in case something goes wrong. So when we don't reset within 10ms, the whole MAC won't start. And keep in mind, this is just the MAC soft reset, it's not negotiating anything on the PHY side or over the wire.
So do you have any deeper insight here? If the 10ms are too short, and you can show me numbers that it needs longer, I am happy to change that.
Cheers, Andre
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev) /* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
On Thu, 20 May 2021 00:10:47 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
hey,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp download. after a manual retry, it works fine but retry is not a valid production behavior.
Just read the arch timer after the SOFT_RST write and after the first read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at 24MHz this is less than a *micro*second for the MAC to reset. So the 10 ms are already plenty. Are you sure that it's this timeout value that is improving things for you?
Cheers, Andre
greetings Andreas
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice *dev) /* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
sorry for the late response.
I run some test runs and maybe there is something with the phy itself or something is missing on sun8i_emac_eth_stop/start?
if you have any patches/ideas to test - let me know! maybe someone has an idea how I can try to force the Linux mainline driver in the same situation? just want to know if there is the same behavior.
test-scenario: download 10 times zImage and dtb over tftp, static ip, no reset, timeout = 1000 10 duplex half: soft reset time 0us with 3 tftp timeouts and recover lowest speed: 369.1 KiB/s max speed: 779.3 KiB/s 10 duplex full: soft reset time 0us with 0 tftp timeouts and recover lowest speed: 656.3 KiB/s max speed: 752.9 KiB/s 100 duplex half: soft reset time 0us with 1 tftp timeout and recover lowest speed: 1.6 MiB/s max speed: 2.7 MiB/s
100 duplex full: try1: 0us, 630000 us with 0 tftp timeout and recover try2: 1001000 us sun8i_emac_eth_start: Timeout -> 5 times -> reconnect cable try3: 382000us, 502000us with 0 tftp timeout and recover try4: 330000 us, 1001000 us sun8i_emac_eth_start: Timeout -> 2 times -> 192000 us try5: power up with cable pluged in: 58000 us, 373000 us with 0 tftp timeout and recover try6: 354000 us, 494000 us with 0 tftp timeout and recover try7: 1001000 us sun8i_emac_eth_start: Timeout -> 3 times -> 1001000 us sun8i_emac_eth_start: Timeout, 626000 us next tries with fresh startup try8: 845000 us, 594000 us try9: 903000 us, 479000 us try10: 638000 us, 500000 us try11: 1001000 us sun8i_emac_eth_start: Timeout, 333000 us try12: 63000 us, 489000 us lowest speed: 1.6 MiB/s max speed: 2.7 MiB/s
when switching from 100 duplex half to full and try to run tftp download for zImage and dtb try1: reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to complete......... TIMEOUT ! reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to complete......... TIMEOUT ! try2: reset MAC done after: 0 us Using ethernet@1c30000 device TFTP from server 192.168.5.80; our IP address is 192.168.5.78 Filename 'zImage'. Load address: 0x42000000 Loading: ################################################################# ################################################################# ################################################################# ################################################################ 2.4 MiB/s done Bytes transferred = 3790520 (39d6b8 hex) reset MAC done after: 1001000 us sun8i_emac_eth_start: Timeout
Am Do., 20. Mai 2021 um 02:18 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Thu, 20 May 2021 00:10:47 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
hey,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp
download.
after a manual retry, it works fine but retry is not a valid production behavior.
Just read the arch timer after the SOFT_RST write and after the first read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at 24MHz this is less than a *micro*second for the MAC to reset. So the 10 ms are already plenty. Are you sure that it's this timeout value that is improving things for you?
Cheers, Andre
greetings Andreas
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice
*dev)
/* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
On Fri, 21 May 2021 22:14:00 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
sorry for the late response.
same ;-)
I run some test runs and maybe there is something with the phy itself or something is missing on sun8i_emac_eth_stop/start?
if you have any patches/ideas to test - let me know! maybe someone has an idea how I can try to force the Linux mainline driver in the same situation? just want to know if there is the same behavior.
So... I think there are at least three different problems at play here: 1) EMAC soft reset timeout: as mentioned, I believe the timeout value itself is a red herring, as it is an automatic operation (the bit flips back to 0 once the reset is done). Waiting much longer sounds weird, the MAC should reset immediately, since at this point it doesn't talk to anyone: it just pushes the "reset switch" on its internal state. However there might be more to it, see below. 2) TFTP timeout and resulting slow transfer speed: This is a totally unrelated and somewhat normal behaviour: TFTP uses UDP, so it's not connection oriented. UDP packets might get lost, for instance due to collisions on the wire. TCP handles those loses transparently and swiftly, that's why you don't notice them there. What makes this so annoying is the long timeout value of 5 seconds, which drastically reduces the overall transfer rate. You can tweak this value by changing TIMEOUT at the beginning of net/tftp.c. If you put 100 there, you will probably barely notice them anymore. The 5 seconds seem to come from the TFTP RFC, so it's hard to argue against it. 3) PHY autonegotiation timeout: This is again independent from the others, especially the MAC soft reset timeout. U-Boot's network stack tries to speak to the PHY via the MDIO bus: PHY_ANEG_TIMEOUT is the macro putting a limit here. There is currently the default 4 seconds fallback value in effect for sunxi here: this might be too short for some situations. Grep for that value to find much longer timeouts for some other platforms. You can try to define this for the V3s in include/configs/sunxi-common.h, and see if that improves things. Happy to take a patch to that effect.
Regarding 1): Heinrich reported the same problem on a H3 board, and bisected it down to some other patch. This again does not seem to be related, and I start to wonder if we indeed handle the soft reset wrongly, as mentioned in you v2 patch. I will have a closer look later at when exactly we issue the soft reset, maybe we do it too often? We probably should do it only once, and not on every new network request. Or maybe we need some delay after the soft reset returns, because it's doing so prematurely? But just dropping it does not sound right, although it's interesting that Linux doesn't need it.
test-scenario: download 10 times zImage and dtb over tftp, static ip, no reset, timeout = 1000 10 duplex half: soft reset time 0us with 3 tftp timeouts and recover lowest speed: 369.1 KiB/s max speed: 779.3 KiB/s 10 duplex full: soft reset time 0us with 0 tftp timeouts and recover lowest speed: 656.3 KiB/s max speed: 752.9 KiB/s 100 duplex half: soft reset time 0us with 1 tftp timeout and recover lowest speed: 1.6 MiB/s max speed: 2.7 MiB/s
100 duplex full:
what are those values before and after the comma below?
Cheers, Andre
try1: 0us, 630000 us with 0 tftp timeout and recover try2: 1001000 us sun8i_emac_eth_start: Timeout -> 5 times -> reconnect cable try3: 382000us, 502000us with 0 tftp timeout and recover try4: 330000 us, 1001000 us sun8i_emac_eth_start: Timeout -> 2 times -> 192000 us try5: power up with cable pluged in: 58000 us, 373000 us with 0 tftp timeout and recover try6: 354000 us, 494000 us with 0 tftp timeout and recover try7: 1001000 us sun8i_emac_eth_start: Timeout -> 3 times -> 1001000 us sun8i_emac_eth_start: Timeout, 626000 us next tries with fresh startup try8: 845000 us, 594000 us try9: 903000 us, 479000 us try10: 638000 us, 500000 us try11: 1001000 us sun8i_emac_eth_start: Timeout, 333000 us try12: 63000 us, 489000 us lowest speed: 1.6 MiB/s max speed: 2.7 MiB/swhen switching from 100 duplex half to full and try to run tftp download for zImage and dtb try1: reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to complete......... TIMEOUT ! reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to complete......... TIMEOUT ! try2: reset MAC done after: 0 us Using ethernet@1c30000 device TFTP from server 192.168.5.80; our IP address is 192.168.5.78 Filename 'zImage'. Load address: 0x42000000 Loading: ################################################################# ################################################################# ################################################################# ################################################################ 2.4 MiB/s done Bytes transferred = 3790520 (39d6b8 hex) reset MAC done after: 1001000 us sun8i_emac_eth_start: Timeout
Am Do., 20. Mai 2021 um 02:18 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Thu, 20 May 2021 00:10:47 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
hey,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp
download.
after a manual retry, it works fine but retry is not a valid production behavior.
Just read the arch timer after the SOFT_RST write and after the first read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at 24MHz this is less than a *micro*second for the MAC to reset. So the 10 ms are already plenty. Are you sure that it's this timeout value that is improving things for you?
Cheers, Andre
greetings Andreas
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice
*dev)
/* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
On 6/3/21 3:56 PM, Andre Przywara wrote:
On Fri, 21 May 2021 22:14:00 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
sorry for the late response.
same ;-)
I run some test runs and maybe there is something with the phy itself or something is missing on sun8i_emac_eth_stop/start?
if you have any patches/ideas to test - let me know! maybe someone has an idea how I can try to force the Linux mainline driver in the same situation? just want to know if there is the same behavior.
So... I think there are at least three different problems at play here:
- EMAC soft reset timeout: as mentioned, I believe the timeout value itself is a red herring, as it is an automatic operation (the bit flips back to 0 once the reset is done). Waiting much longer sounds weird, the MAC should reset immediately, since at this point it doesn't talk to anyone: it just pushes the "reset switch" on its internal state. However there might be more to it, see below.
- TFTP timeout and resulting slow transfer speed: This is a totally unrelated and somewhat normal behaviour: TFTP uses UDP, so it's not connection oriented. UDP packets might get lost, for instance due to collisions on the wire. TCP handles those loses transparently and swiftly, that's why you don't notice them there. What makes this so annoying is the long timeout value of 5 seconds, which drastically reduces the overall transfer rate. You can tweak this value by changing TIMEOUT at the beginning of net/tftp.c. If you put 100 there, you will probably barely notice them anymore. The 5 seconds seem to come from the TFTP RFC, so it's hard to argue against it.
- PHY autonegotiation timeout: This is again independent from the others, especially the MAC soft reset timeout. U-Boot's network stack tries to speak to the PHY via the MDIO bus: PHY_ANEG_TIMEOUT is the macro putting a limit here. There is currently the default 4 seconds fallback value in effect for sunxi here: this might be too short for some situations. Grep for that value to find much longer timeouts for some other platforms. You can try to define this for the V3s in include/configs/sunxi-common.h, and see if that improves things. Happy to take a patch to that effect.
Regarding 1): Heinrich reported the same problem on a H3 board, and bisected it down to some other patch. This again does not seem to be related, and I start to wonder if we indeed handle the soft reset wrongly, as mentioned in you v2 patch. I will have a closer look later at when exactly we issue the soft reset, maybe we do it too often? We probably should do it only once, and not on every new network request. Or maybe we need some delay after the soft reset returns, because it's doing so prematurely? But just dropping it does not sound right, although it's interesting that Linux doesn't need it.
Applying
net: sun8i-emac: v3s: fix soft reset timeout https://patchwork.ozlabs.org/project/uboot/patch/20210522232340.201471-1-reh...
and
/* Soft reset MAC */ - if (!IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) { + if (1 || !IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {
does not solve the problem I see on the OrangePi PC:
=> dhcp sun8i_emac_eth_start: Timeout
So it seems we are talking about different issues.
Applying "net: sun8i-emac: v3s: fix soft reset timeout" on top of
"net: sun8i-emac: fix MDIO frequency" https://patchwork.ozlabs.org/project/uboot/patch/20210603075242.96527-1-xypr...
does not do any harm nor does it show any benefit for tFTP transfer on the OrangePi PC.
Best regards
Heinrich
test-scenario: download 10 times zImage and dtb over tftp, static ip, no reset, timeout = 1000 10 duplex half: soft reset time 0us with 3 tftp timeouts and recover lowest speed: 369.1 KiB/s max speed: 779.3 KiB/s 10 duplex full: soft reset time 0us with 0 tftp timeouts and recover lowest speed: 656.3 KiB/s max speed: 752.9 KiB/s 100 duplex half: soft reset time 0us with 1 tftp timeout and recover lowest speed: 1.6 MiB/s max speed: 2.7 MiB/s
100 duplex full:
what are those values before and after the comma below?
Cheers, Andre
try1: 0us, 630000 us with 0 tftp timeout and recover try2: 1001000 us sun8i_emac_eth_start: Timeout -> 5 times -> reconnect cable try3: 382000us, 502000us with 0 tftp timeout and recover try4: 330000 us, 1001000 us sun8i_emac_eth_start: Timeout -> 2 times -> 192000 us try5: power up with cable pluged in: 58000 us, 373000 us with 0 tftp timeout and recover try6: 354000 us, 494000 us with 0 tftp timeout and recover try7: 1001000 us sun8i_emac_eth_start: Timeout -> 3 times -> 1001000 us sun8i_emac_eth_start: Timeout, 626000 us next tries with fresh startup try8: 845000 us, 594000 us try9: 903000 us, 479000 us try10: 638000 us, 500000 us try11: 1001000 us sun8i_emac_eth_start: Timeout, 333000 us try12: 63000 us, 489000 us lowest speed: 1.6 MiB/s max speed: 2.7 MiB/swhen switching from 100 duplex half to full and try to run tftp download for zImage and dtb try1: reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to complete......... TIMEOUT ! reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to complete......... TIMEOUT ! try2: reset MAC done after: 0 us Using ethernet@1c30000 device TFTP from server 192.168.5.80; our IP address is 192.168.5.78 Filename 'zImage'. Load address: 0x42000000 Loading: ################################################################# ################################################################# ################################################################# ################################################################ 2.4 MiB/s done Bytes transferred = 3790520 (39d6b8 hex) reset MAC done after: 1001000 us sun8i_emac_eth_start: Timeout
Am Do., 20. Mai 2021 um 02:18 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Thu, 20 May 2021 00:10:47 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
hey,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp
download.
after a manual retry, it works fine but retry is not a valid production behavior.
Just read the arch timer after the SOFT_RST write and after the first read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at 24MHz this is less than a *micro*second for the MAC to reset. So the 10 ms are already plenty. Are you sure that it's this timeout value that is improving things for you?
Cheers, Andre
greetings Andreas
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the time it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup do not effect current behavior but reduces unexpected behaviors (e.g. timeouts on tftp download).
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com
drivers/net/sun8i_emac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index 0e7ad3b0d4..23fd35f9e1 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice
*dev)
/* Soft reset MAC */ writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1,
EMAC_CTL1_SOFT_RST, false, 10, true);
EMAC_CTL1_SOFT_RST, false, 500, true); if (ret) { printf("%s: Timeout\n", __func__); return ret;
Am Do., 3. Juni 2021 um 16:43 Uhr schrieb Heinrich Schuchardt < xypron.glpk@gmx.de>:
On 6/3/21 3:56 PM, Andre Przywara wrote:
On Fri, 21 May 2021 22:14:00 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
sorry for the late response.
same ;-)
I run some test runs and maybe there is something with the phy itself or something is missing on sun8i_emac_eth_stop/start?
if you have any patches/ideas to test - let me know! maybe someone has an idea how I can try to force the Linux mainline
driver
in the same situation? just want to know if there is the same behavior.
So... I think there are at least three different problems at play here:
- EMAC soft reset timeout: as mentioned, I believe the timeout value itself is a red herring, as it is an automatic operation (the bit flips back to 0 once the reset is done). Waiting much longer sounds weird, the MAC should reset immediately, since at this point it doesn't talk to anyone: it just pushes the "reset switch" on its internal state. However there might be more to it, see below.
I totally agree. Disabling the soft-reset results not in a timeout at 100 MB/s full-duplex and the download starts immediately. For me, this looks like a wrong usage of the softreset too. Maybe this occurs only if the die supports an internal phy?
- TFTP timeout and resulting slow transfer speed: This is a totally unrelated and somewhat normal behaviour: TFTP uses UDP, so it's not connection oriented. UDP packets might get lost, for instance due to collisions on the wire. TCP handles those loses transparently and swiftly, that's why you don't notice them there. What makes this so annoying is the long timeout value of 5 seconds, which drastically reduces the overall transfer rate. You can tweak this value by changing TIMEOUT at the beginning of net/tftp.c. If you put 100 there, you will probably barely notice them anymore. The 5 seconds seem to come from the TFTP RFC, so it's hard to argue against it.
Agree, I just wanted to give you the exact behaviors/measurements.
- PHY autonegotiation timeout: This is again independent from the others, especially the MAC soft reset timeout. U-Boot's network stack tries to speak to the PHY via the MDIO bus: PHY_ANEG_TIMEOUT is the macro putting a limit here. There is currently the default 4 seconds fallback value in effect for sunxi here: this might be too short for some situations. Grep for that value to find much longer timeouts for some other platforms. You can try to define this for the V3s in include/configs/sunxi-common.h, and see if that improves things. Happy to take a patch to that effect.
I didn't run into this after i disabled the soft-reset for 100 MB/s full-duplex! And yes, I expect a timeout if the cable is not connected but when you connect the cable until the timeout is reached, the link will be established.
Regarding 1): Heinrich reported the same problem on a H3 board, and bisected it down to some other patch. This again does not seem to be related, and I start to wonder if we indeed handle the soft reset wrongly, as mentioned in you v2 patch. I will have a closer look later at when exactly we issue the soft reset, maybe we do it too often? We probably should do it only once, and not on every new network request. Or maybe we need some delay after the soft reset returns, because it's doing so prematurely? But just dropping it does not sound right, although it's interesting that Linux doesn't need it.
I was also wondering about the linux driver but i didn't see any wrong behaviors if i disable it on u-boot too.
Applying
net: sun8i-emac: v3s: fix soft reset timeout
https://patchwork.ozlabs.org/project/uboot/patch/20210522232340.201471-1-reh...
and
/* Soft reset MAC */
if (!IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {
if (1 || !IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) {does not solve the problem I see on the OrangePi PC:
=> dhcp sun8i_emac_eth_start: Timeout
So it seems we are talking about different issues.
Applying "net: sun8i-emac: v3s: fix soft reset timeout" on top of
"net: sun8i-emac: fix MDIO frequency"
https://patchwork.ozlabs.org/project/uboot/patch/20210603075242.96527-1-xypr...
does not do any harm nor does it show any benefit for tFTP transfer on the OrangePi PC.
Best regards
Heinrich
test-scenario: download 10 times zImage and dtb over tftp, static ip, no reset, timeout = 1000 10 duplex half: soft reset time 0us with 3 tftp timeouts and recover lowest speed: 369.1 KiB/s max speed: 779.3 KiB/s 10 duplex full: soft reset time 0us with 0 tftp timeouts and recover lowest speed: 656.3 KiB/s max speed: 752.9 KiB/s 100 duplex half: soft reset time 0us with 1 tftp timeout and recover lowest speed: 1.6 MiB/s max speed: 2.7 MiB/s
100 duplex full:
what are those values before and after the comma below?
The first one is the kernel, the second one the device-tree download. just to give you the numbers which are not deterministic for different tftp download sizes.
Cheers, Andre
try1: 0us, 630000 us with 0 tftp timeout and recover try2: 1001000 us sun8i_emac_eth_start: Timeout -> 5 times -> reconnect cable try3: 382000us, 502000us with 0 tftp timeout and recover try4: 330000 us, 1001000 us sun8i_emac_eth_start: Timeout -> 2 times -> 192000 us try5: power up with cable pluged in: 58000 us, 373000 us with 0 tftp timeout and recover try6: 354000 us, 494000 us with 0 tftp timeout andrecover
try7: 1001000 us sun8i_emac_eth_start: Timeout -> 3 times -> 1001000 us sun8i_emac_eth_start: Timeout,626000 us
next tries with fresh startup try8: 845000 us, 594000 us try9: 903000 us, 479000 us try10: 638000 us, 500000 us try11: 1001000 us sun8i_emac_eth_start: Timeout, 333000 us try12: 63000 us, 489000 us lowest speed: 1.6 MiB/s max speed: 2.7 MiB/swhen switching from 100 duplex half to full and try to run tftp download for zImage and dtb try1: reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to
complete.........
TIMEOUT ! reset MAC done after: 0 us ethernet@1c30000 Waiting for PHY auto negotiation to
complete.........
TIMEOUT ! try2: reset MAC done after: 0 us Using ethernet@1c30000 device TFTP from server 192.168.5.80; our IP address is 192.168.5.78 Filename 'zImage'. Load address: 0x42000000 Loading: ################################################################# ################################################################# ################################################################# ################################################################ 2.4 MiB/s done Bytes transferred = 3790520 (39d6b8 hex) reset MAC done after: 1001000 us sun8i_emac_eth_start: Timeout
Am Do., 20. Mai 2021 um 02:18 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Thu, 20 May 2021 00:10:47 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
hey,
sure. I give it a try tomorrow. with 250 ms, for example, I ran into timeouts after the first tftp
download.
after a manual retry, it works fine but retry is not a valid
production
behavior.
Just read the arch timer after the SOFT_RST write and after the first read of 0 again, and I got 17-18 ticks on my OrangePi Zero (H2+). So at 24MHz this is less than a *micro*second for the MAC to reset. So the 10 ms are already plenty. Are you sure that it's this timeout value that is improving things for you?
Cheers, Andre
greetings Andreas
Am Mi., 19. Mai 2021 um 23:45 Uhr schrieb Andre Przywara < andre.przywara@arm.com>:
On Wed, 19 May 2021 21:42:08 +0200 Andreas Rehn rehn.andreas86@gmail.com wrote:
Hi,
> v3s emac soft reset tooks quit longer if autonegation is active > on 100 Mbit full duplex pairs what can result in > `sun8i_emac_eth_start: Timeout` error
Mmmh, why the 500ms? Can you figure out how long it typically takes for you? By open-coding wait_for_bit_le32() and printing the
time
it took to flip the bit back?
Happy to change this then when we have some data.
Cheers, Andre
> wait_for_bit_le32 polls register value each ms. > Increasing the timeout for setup do not effect current behavior > but reduces unexpected behaviors (e.g. timeouts on tftp download). > > Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com > --- > drivers/net/sun8i_emac.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c > index 0e7ad3b0d4..23fd35f9e1 100644 > --- a/drivers/net/sun8i_emac.c > +++ b/drivers/net/sun8i_emac.c > @@ -475,7 +475,7 @@ static int sun8i_emac_eth_start(struct udevice
*dev)
> /* Soft reset MAC */ > writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); > ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1, > - EMAC_CTL1_SOFT_RST, false, 10, true); > + EMAC_CTL1_SOFT_RST, false, 500, true); > if (ret) { > printf("%s: Timeout\n", __func__); > return ret;
v3s emac soft reset tooks quit longer if autonegation is active on 100 Mbit full duplex pairs what can result in `sun8i_emac_eth_start: Timeout` error
wait_for_bit_le32 polls register value each ms. Increasing the timeout for setup to 1000 ms and above results still in start timeouts.
Linux kernel driver dwmac-sun8i work very nice and don't provide a soft reset. Skip soft reset on u-boot for V3s provide the expected behavior on all connection permutations. If cable is not plugged in, the timeout comes form the phy driver itself.
Signed-off-by: Andreas Rehn rehn.andreas86@gmail.com --- Changes in v2: - skip soft reset if MACH_SUN8I_V3S is enabled - depends on PATCH v2 4/6
drivers/net/sun8i_emac.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c index ab9f61994c..403e9b9d31 100644 --- a/drivers/net/sun8i_emac.c +++ b/drivers/net/sun8i_emac.c @@ -474,12 +474,14 @@ static int sun8i_emac_eth_start(struct udevice *dev) int ret;
/* Soft reset MAC */ - writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); - ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1, - EMAC_CTL1_SOFT_RST, false, 10, true); - if (ret) { - printf("%s: Timeout\n", __func__); - return ret; + if (!IS_ENABLED(CONFIG_MACH_SUN8I_V3S)) { + writel(EMAC_CTL1_SOFT_RST, priv->mac_reg + EMAC_CTL1); + ret = wait_for_bit_le32(priv->mac_reg + EMAC_CTL1, + EMAC_CTL1_SOFT_RST, false, 10, true); + if (ret) { + printf("%s: Timeout\n", __func__); + return ret; + } }
/* Rewrite mac address after reset */
participants (7)
-
Andre Przywara -
Andreas Rehn -
Heinrich Schuchardt -
Icenowy Zheng -
Jagan Teki -
Ramon Fried -
Sean Anderson