[PATCH 0/6] arm: sunxi: v3s: add ethernet support

[PATCH 0/6] arm: sunxi: v3s: add ethernet support

[Andreas Rehn]

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

-- 
2.25.1

[PATCH 1/6] dts: sunxi: add licheepi-zero-dock

[Andreas Rehn]

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
-- 
2.25.1

[PATCH 1/6] dts: sunxi: add licheepi-zero-dock

[Andre Przywara]

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

[PATCH 1/6] dts: sunxi: add licheepi-zero-dock

[Andreas Rehn]

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.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
>
>

-- 
Mit freundlichen Gr??en / kind regards
Andreas Rehn | Master of Engineering (M.Eng)

[PATCH 2/6] clk: sunxi: v3s: Implement EMAC clocks/resets

[Andreas Rehn]

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)),
-- 
2.25.1

[PATCH 2/6] clk: sunxi: v3s: Implement EMAC clocks/resets

[Andre Przywara]

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)),

[PATCH 3/6] clk: sunxi: v3s: fix tabs / spaces

[Andreas Rehn]

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[] = {
-- 
2.25.1

[PATCH 3/6] clk: sunxi: v3s: fix tabs / spaces

[Andre Przywara]

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[] = {

[PATCH 3/6] clk: sunxi: v3s: fix tabs / spaces

[Andreas Rehn]

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[] = {
>
>

-- 
Mit freundlichen Gr??en / kind regards
Andreas Rehn | Master of Engineering (M.Eng)

[PATCH v2 3/6] clk: sunxi: v3s: fix tabs / spaces

[Andreas Rehn]

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[] = {
-- 
2.25.1

Re: [PATCH v2 3/6] clk: sunxi: v3s: fix tabs / spaces

[Andre Przywara]

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[] = {

Re: [PATCH v2 3/6] clk: sunxi: v3s: fix tabs / spaces

[Sean Anderson]

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

[PATCH 4/6] net: sun8i-emac: add v3s pinmux setting

[Andreas Rehn]

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");
 
-- 
2.25.1

[PATCH 4/6] net: sun8i-emac: add v3s pinmux setting

[Andre Przywara]

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;
>  	else
>  		BUILD_BUG_ON_MSG(1, "missing pinmux value for Ethernet pins");
>  

[PATCH v2 4/6] net: sun8i-emac: add v3s variant

[Andreas Rehn]

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

Re: [PATCH v2 4/6] net: sun8i-emac: add v3s variant

[Ramon Fried]

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 >

Re: [PATCH v2 4/6] net: sun8i-emac: add v3s variant

[Andre Przywara]

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
>  		 * 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 },
>  	{ }
>  };
>  

Re: [PATCH v2 4/6] net: sun8i-emac: add v3s variant

[Jagan Teki]

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.

[PATCH 5/6] dts: sunxi: v3s: enable emac support

[Andreas Rehn]

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 at 1c30000 {
-			compatible = "allwinner,sun8i-v3s-emac";
+			compatible = "allwinner,sun8i-h3-emac";
 			syscon = <&syscon>;
 			reg = <0x01c30000 0x10000>;
 			interrupts = <GIC_SPI 82 IRQ_TYPE_LEVEL_HIGH>;
-- 
2.25.1

[PATCH 5/6] dts: sunxi: v3s: enable emac support

[Andre Przywara]

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 at 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>;

[PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andreas Rehn]

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;
-- 
2.25.1

[PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andre Przywara]

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;

[PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andreas Rehn]

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;
>
>

-- 
Mit freundlichen Gr??en / kind regards
Andreas Rehn | Master of Engineering (M.Eng)

[PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andre Przywara]

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 at 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;  
> >
> >  
> 

[PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andre Przywara]

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 at 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;  
> >
> >  
> 

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andreas Rehn]

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;
> > >
> > >
> >
>
>

-- 
Mit freundlichen Grüßen / kind regards
Andreas Rehn | Master of Engineering (M.Eng)

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andre Przywara]

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/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;  
> > > >
> > > >  
> > >  
> >
> >  
> 

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Heinrich Schuchardt]

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:
> 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.

Applying

net: sun8i-emac: v3s: fix soft reset timeout
https://patchwork.ozlabs.org/project/uboot/patch/20210522232340.201471-1-rehn.andreas86@gmail.com/

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-xypron.glpk@gmx.de/

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/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;
>>>>>
>>>>>
>>>>
>>>
>>>
>>
>

Re: [PATCH 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andreas Rehn]

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:
> > 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.
>

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?


> > 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.
>

Agree, I just wanted to give you the exact behaviors/measurements.


> > 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.


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-rehn.andreas86@gmail.com/
>
> 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-xypron.glpk@gmx.de/
>
> 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 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;
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>
> >
>
>

-- 
Mit freundlichen Grüßen / kind regards
Andreas Rehn | Master of Engineering (M.Eng)

[PATCH v2 6/6] net: sun8i-emac: v3s: fix soft reset timeout

[Andreas Rehn]

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 */
-- 
2.25.1

[PATCH v2 3/6] clk: sunxi: v3s: fix tabs / spaces

[Andreas Rehn]

align CLK_USB_PHY0 with tabs

Signed-off-by: Andreas Rehn <rehn.andreas86@gmail.com>
---
 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[] = {
-- 
2.25.1

[PATCH v2 4/6] net: sun8i-emac: add v3s variant

[Andreas Rehn]

Add variant V3S_EMAC.
Skip GPIO setup for V3s because it uses internal phy and don't expose pins.

Signed-off-by: Andreas Rehn <rehn.andreas86@gmail.com>
---
 drivers/net/sun8i_emac.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/net/sun8i_emac.c b/drivers/net/sun8i_emac.c
index 5a1b38bf80..7541d3cf68 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;
@@ -343,7 +344,8 @@ static int sun8i_emac_set_syscon(struct sun8i_eth_pdata *pdata,
 	reg &= ~(SC_ETCS_MASK | SC_EPIT);
 	if (priv->variant == H3_EMAC ||
 	    priv->variant == A64_EMAC ||
-	    priv->variant == H6_EMAC)
+	    priv->variant == H6_EMAC ||
+	    priv->variant == V3S_EMAC)
 		reg &= ~SC_RMII_EN;
 
 	switch (priv->interface) {
@@ -354,7 +356,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 +569,9 @@ 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 - skip gpio setup.
+		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