diff mbox

[v3,2/3] ARM: shmobile: Koelsch: add Ether support

Message ID 201312082352.45265.sergei.shtylyov@cogentembedded.com (mailing list archive)
State Accepted
Commit 974faba70550409049ee349939f4479ad98908ae
Headers show

Commit Message

Sergei Shtylyov Dec. 8, 2013, 8:52 p.m. UTC
Register Ether platform device and pin data on  the  Koelsch board. 
Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

---
Changes in version 3:
- changed "r8a779x-ether" wildcard device name to "r8a7791-ether";
- added PFC data for IRQ0 pin used by KSZ8041 PHY;
- resolved rejects, refreshed the patch.

Changes in version 2:
- added *if* (IS_ENABLED(CONFIG_PHYLIB)) around phy_register_fixup_for_id()
  call;
- changed Ether device name to "r8a779x-ether".

 arch/arm/mach-shmobile/board-koelsch.c |   57 ++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Geert Uytterhoeven Dec. 9, 2013, 6:52 p.m. UTC | #1
Hi Sergei,

On Sun, Dec 8, 2013 at 9:52 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Register Ether platform device and pin data on  the  Koelsch board.
> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

> --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c
> +++ renesas/arch/arm/mach-shmobile/board-koelsch.c

> +/* Ether */
> +static const struct sh_eth_plat_data ether_pdata __initconst = {
> +       .phy                    = 0x1,
> +       .edmac_endian           = EDMAC_LITTLE_ENDIAN,
> +       .phy_interface          = PHY_INTERFACE_MODE_RMII,
> +       .ether_link_active_low  = 1,
> +};

With the above, NFS root is terrible slow, and ping indicates more than
10% packet loss. I was trying to debootstrap Debian jessie, but gave up
after a few hours.

Based on earlier discussions I found in the mailing list archives, I
replaced the
".ether_link_active_low = 1" by ".no_ether_link = 1", and the problem went away.
Debootstrap completed in about one hour.  No more noticable packet loss
during normal operation.
I do still get "net eth0: Receive FIFO Overflow" once in a while, though (e.g.
when running nuttcp).

# nuttcp -r amd64box
  112.8022 MB /  10.14 sec =   93.3131 Mbps 2 %TX 15 %RX 0 retrans 0.28 msRTT
# nuttcp -t amd64box
   94.1981 MB /  10.04 sec =   78.7000 Mbps 3 %TX 5 %RX 0 retrans

With ".ether_link_active_low = 1", performance is halved:

# nuttcp -r amd64box
   56.0736 MB /  10.11 sec =   46.5290 Mbps 0 %TX 8 %RX 0 retrans 996.39 msRTT
# nuttcp -t amd64box
   47.7191 MB /  10.03 sec =   39.8991 Mbps 1 %TX 2 %RX 0 retrans

BTW, I'm using renesas-devel-v3.13-rc2-20131205, with the following 4 patches
from you applied on top:
      sh_eth: add R8A7791 support
      ARM: shmobile: r8a7791: add Ether clock
      ARM: shmobile: Koelsch: add Ether support
      ARM: shmobile: Koelsch: enable Ether in defconfig
Anything I'm missing?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Dec. 9, 2013, 7:40 p.m. UTC | #2
On Mon, Dec 9, 2013 at 9:25 PM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>>> --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c
>>> +++ renesas/arch/arm/mach-shmobile/board-koelsch.c
>>
>>
>>> +/* Ether */
>>> +static const struct sh_eth_plat_data ether_pdata __initconst = {
>>> +       .phy                    = 0x1,
>>> +       .edmac_endian           = EDMAC_LITTLE_ENDIAN,
>>> +       .phy_interface          = PHY_INTERFACE_MODE_RMII,
>>> +       .ether_link_active_low  = 1,
>>> +};
>
>> With the above, NFS root is terrible slow,
>
>    Do you get NFS timeout(s) on booting? I do. It's a known issue.

I only saw "NFS server not responding" once.

>    You can try my simplistic 'sh_eth' driver patch:
>
> http://marc.info/?l=linux-sh&m=137911743723032

Thanks, that also fixes the problem!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Dec. 9, 2013, 8:25 p.m. UTC | #3
Hello.

On 12/09/2013 09:52 PM, Geert Uytterhoeven wrote:

>> Register Ether platform device and pin data on  the  Koelsch board.
>> Register platform fixup for Micrel KSZ8041 PHY, just like on the Lager board.

>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>

>> --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c
>> +++ renesas/arch/arm/mach-shmobile/board-koelsch.c
>
>> +/* Ether */
>> +static const struct sh_eth_plat_data ether_pdata __initconst = {
>> +       .phy                    = 0x1,
>> +       .edmac_endian           = EDMAC_LITTLE_ENDIAN,
>> +       .phy_interface          = PHY_INTERFACE_MODE_RMII,
>> +       .ether_link_active_low  = 1,
>> +};

> With the above, NFS root is terrible slow,

    Do you get NFS timeout(s) on booting? I do. It's a known issue.

> and ping indicates more than 10% packet loss.

    Hm, that I didn't see...

> I was trying to debootstrap Debian jessie, but gave up
> after a few hours.

> Based on earlier discussions I found in the mailing list archives, I
> replaced the
> ".ether_link_active_low = 1" by ".no_ether_link = 1", and the problem went away.

    That's one way to deal with the issue, but it's not quite correct because 
it'll make Ether LEDs function in a way not matching to the intended one.
I've added PHY platform fixup to force the LED mode to the intended one after 
PHY reset that the driver does. I've also submitted a somewhat naive patch to 
call the fixups after that PHY reset but it got refused by David Miller. 
That's why we have what we have now. Hopefully, this should get resolved in 
3.14 if Florian Fainelli's patch series gets accepted:

http://marc.info/?l=linux-netdev&m=138636384109141

> Debootstrap completed in about one hour.  No more noticable packet loss
> during normal operation.
> I do still get "net eth0: Receive FIFO Overflow" once in a while, though (e.g.
> when running nuttcp).

> # nuttcp -r amd64box
>    112.8022 MB /  10.14 sec =   93.3131 Mbps 2 %TX 15 %RX 0 retrans 0.28 msRTT
> # nuttcp -t amd64box
>     94.1981 MB /  10.04 sec =   78.7000 Mbps 3 %TX 5 %RX 0 retrans

> With ".ether_link_active_low = 1", performance is halved:

> # nuttcp -r amd64box
>     56.0736 MB /  10.11 sec =   46.5290 Mbps 0 %TX 8 %RX 0 retrans 996.39 msRTT
> # nuttcp -t amd64box
>     47.7191 MB /  10.03 sec =   39.8991 Mbps 1 %TX 2 %RX 0 retrans

> BTW, I'm using renesas-devel-v3.13-rc2-20131205, with the following 4 patches
> from you applied on top:
>        sh_eth: add R8A7791 support
>        ARM: shmobile: r8a7791: add Ether clock
>        ARM: shmobile: Koelsch: add Ether support
>        ARM: shmobile: Koelsch: enable Ether in defconfig
> Anything I'm missing?

    You can try my simplistic 'sh_eth' driver patch:

http://marc.info/?l=linux-sh&m=137911743723032

> Thanks!

> Gr{oetje,eeting}s,

>                          Geert

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sergei Shtylyov Dec. 9, 2013, 11:10 p.m. UTC | #4
On 12/09/2013 10:40 PM, Geert Uytterhoeven wrote:

>>>> --- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c
>>>> +++ renesas/arch/arm/mach-shmobile/board-koelsch.c

>>>> +/* Ether */
>>>> +static const struct sh_eth_plat_data ether_pdata __initconst = {
>>>> +       .phy                    = 0x1,
>>>> +       .edmac_endian           = EDMAC_LITTLE_ENDIAN,
>>>> +       .phy_interface          = PHY_INTERFACE_MODE_RMII,
>>>> +       .ether_link_active_low  = 1,
>>>> +};

>>> With the above, NFS root is terrible slow,

>>     Do you get NFS timeout(s) on booting? I do. It's a known issue.

> I only saw "NFS server not responding" once.

    Yes, I meant that by NFS timeout. Sometimes I see 2 of them in a row.

>>     You can try my simplistic 'sh_eth' driver patch:

>> http://marc.info/?l=linux-sh&m=137911743723032

> Thanks, that also fixes the problem!

    Let's hope Florian's patches get queued for 3.14. You can also test them 
meanwhile...

> Gr{oetje,eeting}s,
>
>                          Geert

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Geert Uytterhoeven Dec. 10, 2013, 1:22 p.m. UTC | #5
On Tue, Dec 10, 2013 at 12:10 AM, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
>>>     You can try my simplistic 'sh_eth' driver patch:
>
>>> http://marc.info/?l=linux-sh&m=137911743723032
>
>> Thanks, that also fixes the problem!
>
>    Let's hope Florian's patches get queued for 3.14. You can also test them
> meanwhile...

I applied the whole series "net: phy: consolidate PHY reset", and sh_eth
now works fine on Koelsch.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Florian Fainelli Dec. 10, 2013, 5:05 p.m. UTC | #6
2013/12/10 Geert Uytterhoeven <geert@linux-m68k.org>:
> On Tue, Dec 10, 2013 at 12:10 AM, Sergei Shtylyov
> <sergei.shtylyov@cogentembedded.com> wrote:
>>>>     You can try my simplistic 'sh_eth' driver patch:
>>
>>>> http://marc.info/?l=linux-sh&m=137911743723032
>>
>>> Thanks, that also fixes the problem!
>>
>>    Let's hope Florian's patches get queued for 3.14. You can also test them
>> meanwhile...
>
> I applied the whole series "net: phy: consolidate PHY reset", and sh_eth
> now works fine on Koelsch.

Great! Thanks Geert.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: renesas/arch/arm/mach-shmobile/board-koelsch.c
===================================================================
--- renesas.orig/arch/arm/mach-shmobile/board-koelsch.c
+++ renesas/arch/arm/mach-shmobile/board-koelsch.c
@@ -24,15 +24,31 @@ 
 #include <linux/input.h>
 #include <linux/kernel.h>
 #include <linux/leds.h>
+#include <linux/phy.h>
 #include <linux/pinctrl/machine.h>
 #include <linux/platform_data/gpio-rcar.h>
 #include <linux/platform_device.h>
+#include <linux/sh_eth.h>
 #include <mach/common.h>
+#include <mach/irqs.h>
 #include <mach/r8a7791.h>
 #include <mach/rcar-gen2.h>
 #include <asm/mach-types.h>
 #include <asm/mach/arch.h>
 
+/* Ether */
+static const struct sh_eth_plat_data ether_pdata __initconst = {
+	.phy			= 0x1,
+	.edmac_endian		= EDMAC_LITTLE_ENDIAN,
+	.phy_interface		= PHY_INTERFACE_MODE_RMII,
+	.ether_link_active_low	= 1,
+};
+
+static const struct resource ether_resources[] __initconst = {
+	DEFINE_RES_MEM(0xee700000, 0x400),
+	DEFINE_RES_IRQ(gic_spi(162)),
+};
+
 /* LEDS */
 static struct gpio_led koelsch_leds[] = {
 	{
@@ -80,6 +96,15 @@  static const struct gpio_keys_platform_d
 };
 
 static const struct pinctrl_map koelsch_pinctrl_map[] = {
+	/* Ether */
+	PIN_MAP_MUX_GROUP_DEFAULT("r8a7791-ether", "pfc-r8a7791",
+				  "eth_link", "eth"),
+	PIN_MAP_MUX_GROUP_DEFAULT("r8a7791-ether", "pfc-r8a7791",
+				  "eth_mdio", "eth"),
+	PIN_MAP_MUX_GROUP_DEFAULT("r8a7791-ether", "pfc-r8a7791",
+				  "eth_rmii", "eth"),
+	PIN_MAP_MUX_GROUP_DEFAULT("r8a7791-ether", "pfc-r8a7791",
+				  "intc_irq0", "intc"),
 	/* SCIF0 (CN19: DEBUG SERIAL0) */
 	PIN_MAP_MUX_GROUP_DEFAULT("sh-sci.6", "pfc-r8a7791",
 				  "scif0_data_d", "scif0"),
@@ -95,6 +120,10 @@  static void __init koelsch_add_standard_
 				  ARRAY_SIZE(koelsch_pinctrl_map));
 	r8a7791_pinmux_init();
 	r8a7791_add_standard_devices();
+	platform_device_register_resndata(&platform_bus, "r8a7791-ether", -1,
+					  ether_resources,
+					  ARRAY_SIZE(ether_resources),
+					  &ether_pdata, sizeof(ether_pdata));
 	platform_device_register_data(&platform_bus, "leds-gpio", -1,
 				      &koelsch_leds_pdata,
 				      sizeof(koelsch_leds_pdata));
@@ -103,6 +132,32 @@  static void __init koelsch_add_standard_
 				      sizeof(koelsch_keys_pdata));
 }
 
+/*
+ * Ether LEDs on the Koelsch board are named LINK and ACTIVE which corresponds
+ * to non-default 01 setting of the Micrel KSZ8041 PHY control register 1 bits
+ * 14-15. We have to set them back to 01 from the default 00 value each time
+ * the PHY is reset. It's also important because the PHY's LED0 signal is
+ * connected to SoC's ETH_LINK signal and in the PHY's default mode it will
+ * bounce on and off after each packet, which we apparently want to avoid.
+ */
+static int koelsch_ksz8041_fixup(struct phy_device *phydev)
+{
+	u16 phyctrl1 = phy_read(phydev, 0x1e);
+
+	phyctrl1 &= ~0xc000;
+	phyctrl1 |= 0x4000;
+	return phy_write(phydev, 0x1e, phyctrl1);
+}
+
+static void __init koelsch_init(void)
+{
+	koelsch_add_standard_devices();
+
+	if (IS_ENABLED(CONFIG_PHYLIB))
+		phy_register_fixup_for_id("r8a7791-ether-ff:01",
+					  koelsch_ksz8041_fixup);
+}
+
 static const char * const koelsch_boards_compat_dt[] __initconst = {
 	"renesas,koelsch",
 	NULL,
@@ -112,7 +167,7 @@  DT_MACHINE_START(KOELSCH_DT, "koelsch")
 	.smp		= smp_ops(r8a7791_smp_ops),
 	.init_early	= r8a7791_init_early,
 	.init_time	= rcar_gen2_timer_init,
-	.init_machine	= koelsch_add_standard_devices,
+	.init_machine	= koelsch_init,
 	.init_late	= shmobile_init_late,
 	.dt_compat	= koelsch_boards_compat_dt,
 MACHINE_END