[RFC PATCH v2] arm: kirkwood: nsa310s: Use Marvell uclass mvgbe and PHY driver for DM Ethernet

Tony Dinh mibodhi at gmail.com
Mon Apr 11 22:31:05 CEST 2022


Hi Stefan,

I think I can see the problem with this NSA310s board now.  In the old
ad-hoc code to set RGMII delay (same for all Kirkwood boards), I
noticed the last thing was the jump back to page 0. Apparently it is
necessary to do that before doing a soft reset.

And I looked at the MV88E1116R datasheet (Table 55 Page Address).
Indeed, register 22 is Page Select and its value is applicable to
register 0 to 28.

For the Sheevaplug, it was already at page 0 (register 22 is
0x1fb50000), but the NSA310s board was not (register 22 is 0xfb50011).
That 0x11 seems to be garbage, because the only PHY pages that are
used in this 1116R/1318 chips are page 0 to 6, and 255 (REF: Table 35
Register Map Summary). Please see the debug logs below after the
break.

So I will send a separate patch for drivers/net/mvgbe.c to add the
jump to page 0 before phy_connect is called. This way it can be
reviewed by network maintainers. And then send a real patch for the
NSA310S after that.

Thanks,
Tony

=================

Sheevaplug Test

U-Boot 2022.04-rc5-00004-g20d2aad075-dirty (Apr 10 2022 - 20:20:24 -0700)
Marvell-Sheevaplug

=> ping 192.168.0.249

mvgbe_start UCLASS_ETH  mvgbe
__mvgbe_phy_init  MV88E1xxx_PGADR_REG current value = 0x1fb50000
__mvgbe_phy_init called miiphy_write to clear MV88E1318_PGADR_REG (set
register 22  to 0) # this is the jump back to page 0
__mvgbe_phy_init  MV88E1xxx_PGADR_REG current value = 0x1fb50000
phy.c phy_connect:
phy.c get_phy_driver PHY driver found - PHY id 1410e90
phy.c phy_connect_dev: ethernet-controller at 72000 connected to Marvell 88E1310
phy.c board_phy_config
m88e1310_config
m88e1011s_startup
ethernet-controller at 72000 Waiting for PHY auto negotiation to
complete....... done
host 192.168.0.249 is alive


NSA310S Test

U-Boot 2022.04-rc5-00004-g81f1e4b6de-dirty (Apr 10 2022 - 21:58:39 -0700)
ZyXEL NSA310S/320S 1/2-Bay Power Media Server

nsa310s => ping 192.168.0.249

mvgbe_start UCLASS_ETH  mvgbe
mvgbe_start PHY addr 1
__mvgbe_init
__mvgbe_phy_init phy name = ethernet-controller at 72000
__mvgbe_phy_init phyid = 1
__mvgbe_phy_init phy_interface = rgmii
__mvgbe_phy_init  MV88E1xxx_PGADR_REG current value = 0xfb50011
__mvgbe_phy_init called miiphy_write to clear MV88E1318_PGADR_REG (set
register 22  to 0)
__mvgbe_phy_init  MV88E1xxx_PGADR_REG current value = 0xfb50000
phy.c phy_connect:
phy.c get_phy_driver PHY driver found - PHY id 1410e90
phy.c phy_connect_dev: ethernet-controller at 72000 connected to Marvell 88E1310
phy.c board_phy_config
m88e1310_config
m88e1011s_startup

ethernet-controller at 72000 Waiting for PHY auto negotiation to complete..... done
host 192.168.0.249 is alive




On Fri, Apr 8, 2022 at 5:57 PM Tony Dinh <mibodhi at gmail.com> wrote:
>
> Hi all,
>
> This is a work-in-progress patch, to clean up the
> DM Ethernet code for the Zyxel NSA310S board (Kirkwood 88F6702 A1).
>
> This NSA310s board Ethernet has some quirks that it does not work
> quite the same way as other Kirwood boards with the similar network
> chip (MV88E1318), such as the Sheevaplug and the Dreamplug.
>
> Currently, in the NSA310S board file we use CONFIG_RESET_PHY_R to
> execute reset_phy() during initialization. And the reset_phy() code in
> this board file is ad-hoc, does not involve  Marvell PHY driver (which
> it should be). So I'm following the same pattern that I've done for
> the Sheevaplug: use the uclass drivers/net/mvgbe.c (CONFIG_MVGBE) to
> bring up Ethernet, removing the reset_phy() code, and enable
> CONFIG_PHY_MARVEL.
>
> With this board, I could not get it to work the same way as for the
> Sheevaplug to bring up the PHY automatically. I had to insert the
> ad-hoc code to set RGMII delay in the __mvgbe_phy_init() function in
> mvgbe.c, so the phy_connect() call will find the PHY. The PHY Id is
> 1410e90, same as in the Sheevaplug and Dreamplug. Please see this in
> the patch below (it's only a hack to see the PHY can be brought up
> this way).
>
> With the Sheevaplug and Dreamplug, there is no need to do this hack.
> The uclass mvgbe did all the work, by the time phy_connect() is
> executed, the PHY was already up and working fine. And then the
> Marvell PHY driver kicks in, setting up the rest.
>
> Perhaps we need code in the uclass MVGBE that handles this in a
> generic way? These are the Marvell PHY  driver functions invoked after
> phy_connect() was successful. This happens in all 3 boards
> (Sheevaplug, Dreamplug, and NSA310s).
>
> m88e1310_config
> m88e1011s_startup
>
> I would appreciate hearing some explanation and perhaps some
> suggestion/guidance about this topic.
>
> Thanks,
> Tony
>
> Signed-off-by: Tony Dinh <mibodhi at gmail.com>
> ---
>
> Changes in v2:
> Cleanup patch formatting
>
>  arch/arm/dts/kirkwood-nsa310s.dts |   1 +
>  board/zyxel/nsa310s/nsa310s.c     | 120 +++++++++---------------------
>  board/zyxel/nsa310s/nsa310s.h     |  46 ------------
>  configs/nsa310s_defconfig         |   3 +
>  drivers/net/mvgbe.c               |  18 +++++
>  include/configs/nsa310s.h         |  15 ++--
>  6 files changed, 62 insertions(+), 141 deletions(-)
>  delete mode 100644 board/zyxel/nsa310s/nsa310s.h
>
> diff --git a/arch/arm/dts/kirkwood-nsa310s.dts b/arch/arm/dts/kirkwood-nsa310s.dts
> index e1c9c9080c..09ee76c2a2 100644
> --- a/arch/arm/dts/kirkwood-nsa310s.dts
> +++ b/arch/arm/dts/kirkwood-nsa310s.dts
> @@ -306,6 +306,7 @@
>         status = "okay";
>         ethernet0-port at 0 {
>                 phy-handle = <&ethphy0>;
> +               phy-mode = "rgmii";
>         };
>  };
>
> diff --git a/board/zyxel/nsa310s/nsa310s.c b/board/zyxel/nsa310s/nsa310s.c
> index b71de4e11f..8a45020bfa 100644
> --- a/board/zyxel/nsa310s/nsa310s.c
> +++ b/board/zyxel/nsa310s/nsa310s.c
> @@ -1,22 +1,49 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * Copyright (C) 2015, 2021 Tony Dinh <mibodhi at gmail.com>
> + * Copyright (C) 2015, 2021-2022 Tony Dinh <mibodhi at gmail.com>
>   * Copyright (C) 2015 Gerald Kerma <dreagle at doukki.net>
>   */
>
>  #include <common.h>
>  #include <init.h>
> -#include <miiphy.h>
> -#include <net.h>
> +#include <netdev.h>
>  #include <asm/arch/cpu.h>
>  #include <asm/arch/soc.h>
>  #include <asm/arch/mpp.h>
>  #include <asm/global_data.h>
>  #include <asm/io.h>
> -#include "nsa310s.h"
> +#include <linux/bitops.h>
>
>  DECLARE_GLOBAL_DATA_PTR;
>
> +/*
> + * low GPIO's
> + */
> +#define HDD1_GREEN_LED         BIT(16)
> +#define HDD1_RED_LED           BIT(13)
> +#define USB_GREEN_LED          BIT(15)
> +#define USB_POWER              BIT(21)
> +#define SYS_GREEN_LED          BIT(28)
> +#define SYS_ORANGE_LED         BIT(29)
> +
> +#define COPY_GREEN_LED         BIT(22)
> +#define COPY_RED_LED           BIT(23)
> +
> +#define PIN_USB_GREEN_LED      15
> +#define PIN_USB_POWER          21
> +
> +#define NSA310S_OE_LOW         (~(0))
> +#define NSA310S_VAL_LOW                (SYS_GREEN_LED | USB_POWER)
> +
> +/*
> + * high GPIO's
> + */
> +#define HDD2_GREEN_LED         BIT(2)
> +#define HDD2_POWER             BIT(1)
> +
> +#define NSA310S_OE_HIGH                (~(0))
> +#define NSA310S_VAL_HIGH       (HDD2_POWER)
> +
>  int board_early_init_f(void)
>  {
>         /*
> @@ -80,87 +107,8 @@ int board_init(void)
>         return 0;
>  }
>
> -static int fdt_get_phy_addr(const char *path)
> -{
> -       const void *fdt = gd->fdt_blob;
> -       const u32 *reg;
> -       const u32 *val;
> -       int node, phandle, addr;
> -
> -       /* Find the node by its full path */
> -       node = fdt_path_offset(fdt, path);
> -       if (node >= 0) {
> -               /* Look up phy-handle */
> -               val = fdt_getprop(fdt, node, "phy-handle", NULL);
> -               if (val) {
> -                       phandle = fdt32_to_cpu(*val);
> -                       if (!phandle)
> -                               return -1;
> -                       /* Follow it to its node */
> -                       node = fdt_node_offset_by_phandle(fdt, phandle);
> -                       if (node) {
> -                               /* Look up reg */
> -                               reg = fdt_getprop(fdt, node, "reg", NULL);
> -                               if (reg) {
> -                                       addr = fdt32_to_cpu(*reg);
> -                                       return addr;
> -                               }
> -                       }
> -               }
> -       }
> -       return -1;
> -}
> -
> -#ifdef CONFIG_RESET_PHY_R
> -void reset_phy(void)
> +int board_eth_init(struct bd_info *bis)
>  {
> -       u16 reg;
> -       u16 phyaddr;
> -       char *name = "ethernet-controller at 72000";
> -       char *eth0_path = "/ocp at f1000000/ethernet-controller at 72000/ethernet0-port at 0";
> -
> -       if (miiphy_set_current_dev(name))
> -               return;
> -
> -       phyaddr = fdt_get_phy_addr(eth0_path);
> -       if (phyaddr < 0)
> -               return;
> -
> -       /* set RGMII delay */
> -       miiphy_write(name, phyaddr, MV88E1318_PGADR_REG, MV88E1318_MAC_CTRL_PG);
> -       miiphy_read(name, phyaddr, MV88E1318_MAC_CTRL_REG, &reg);
> -       reg |= (MV88E1318_RGMII_RX_CTRL | MV88E1318_RGMII_TX_CTRL);
> -       miiphy_write(name, phyaddr, MV88E1318_MAC_CTRL_REG, reg);
> -       miiphy_write(name, phyaddr, MV88E1318_PGADR_REG, 0);
> -
> -       /* reset PHY */
> -       if (miiphy_reset(name, phyaddr))
> -               return;
> -
> -       /*
> -        * ZyXEL NSA310S uses the 88E1310S Alaska (interface identical to 88E1318)
> -        * and has an MCU attached to the LED[2] via tristate interrupt
> -        */
> -
> -       /* switch to LED register page */
> -       miiphy_write(name, phyaddr, MV88E1318_PGADR_REG, MV88E1318_LED_PG);
> -       /* read out LED polarity register */
> -       miiphy_read(name, phyaddr, MV88E1318_LED_POL_REG, &reg);
> -       /* clear 4, set 5 - LED2 low, tri-state */
> -       reg &= ~(MV88E1318_LED2_4);
> -       reg |= (MV88E1318_LED2_5);
> -       /* write back LED polarity register */
> -       miiphy_write(name, phyaddr, MV88E1318_LED_POL_REG, reg);
> -       /* jump back to page 0, per the PHY chip documenation. */
> -       miiphy_write(name, phyaddr, MV88E1318_PGADR_REG, 0);
> -
> -       /* set PHY back to auto-negotiation mode */
> -       miiphy_write(name, phyaddr, 0x4, 0x1e1);
> -       miiphy_write(name, phyaddr, 0x9, 0x300);
> -       /* downshift */
> -       miiphy_write(name, phyaddr, 0x10, 0x3860);
> -       miiphy_write(name, phyaddr, 0x0, 0x9140);
> -
> -       printf("MV88E1318 PHY initialized on %s\n", name);
> +       printf("nsa310s.c: %s\n", __func__);
> +       return cpu_eth_init(bis);
>  }
> -#endif /* CONFIG_RESET_PHY_R */
> diff --git a/board/zyxel/nsa310s/nsa310s.h b/board/zyxel/nsa310s/nsa310s.h
> deleted file mode 100644
> index d8bd9a586f..0000000000
> --- a/board/zyxel/nsa310s/nsa310s.h
> +++ /dev/null
> @@ -1,46 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0+ */
> -/*
> - * Copyright (C) 2015
> - * Gerald Kerma <dreagle at doukki.net>
> - * Tony Dinh <mibodhi at gmail.com>
> - */
> -
> -#ifndef __NSA310S_H
> -#define __NSA310S_H
> -
> -/* low GPIO's */
> -#define HDD1_GREEN_LED         (1 << 16)
> -#define HDD1_RED_LED           (1 << 13)
> -#define USB_GREEN_LED          (1 << 15)
> -#define USB_POWER                      (1 << 21)
> -#define SYS_GREEN_LED          (1 << 28)
> -#define SYS_ORANGE_LED         (1 << 29)
> -
> -#define COPY_GREEN_LED         (1 << 22)
> -#define COPY_RED_LED           (1 << 23)
> -
> -#define PIN_USB_GREEN_LED      15
> -#define PIN_USB_POWER          21
> -
> -#define NSA310S_OE_LOW         (~(0))
> -#define NSA310S_VAL_LOW                (SYS_GREEN_LED | USB_POWER)
> -
> -/* high GPIO's */
> -#define HDD2_GREEN_LED         (1 << 2)
> -#define HDD2_POWER                     (1 << 1)
> -
> -#define NSA310S_OE_HIGH                (~(0))
> -#define NSA310S_VAL_HIGH       (HDD2_POWER)
> -
> -/* PHY related */
> -#define MV88E1318_PGADR_REG            22
> -#define MV88E1318_MAC_CTRL_PG  2
> -#define MV88E1318_MAC_CTRL_REG 21
> -#define MV88E1318_RGMII_TX_CTRL        (1 << 4)
> -#define MV88E1318_RGMII_RX_CTRL        (1 << 5)
> -#define MV88E1318_LED_PG               3
> -#define MV88E1318_LED_POL_REG  17
> -#define MV88E1318_LED2_4               (1 << 4)
> -#define MV88E1318_LED2_5               (1 << 5)
> -
> -#endif /* __NSA310S_H */
> diff --git a/configs/nsa310s_defconfig b/configs/nsa310s_defconfig
> index 05a6761854..0745c2820c 100644
> --- a/configs/nsa310s_defconfig
> +++ b/configs/nsa310s_defconfig
> @@ -2,6 +2,7 @@ CONFIG_ARM=y
>  CONFIG_SKIP_LOWLEVEL_INIT=y
>  CONFIG_SYS_DCACHE_OFF=y
>  CONFIG_ARCH_CPU_INIT=y
> +CONFIG_SYS_THUMB_BUILD=y
>  CONFIG_ARCH_KIRKWOOD=y
>  CONFIG_SYS_KWD_CONFIG="board/zyxel/nsa310s/kwbimage.cfg"
>  CONFIG_SYS_TEXT_BASE=0x600000
> @@ -30,6 +31,7 @@ CONFIG_CMD_MII=y
>  CONFIG_CMD_PING=y
>  CONFIG_CMD_EXT2=y
>  CONFIG_CMD_FAT=y
> +CONFIG_CMD_FS_GENERIC=y
>  CONFIG_CMD_JFFS2=y
>  CONFIG_CMD_MTDPARTS=y
>  CONFIG_MTDPARTS_DEFAULT="mtdparts=orion_nand:0xe0000 at 0x0(uboot),0x20000 at 0xe0000(uboot_env),0x100000 at 0x100000(second_stage_uboot),- at 0x200000(root)"
> @@ -47,6 +49,7 @@ CONFIG_SYS_SATA_MAX_DEVICE=1
>  # CONFIG_MMC is not set
>  CONFIG_MTD=y
>  CONFIG_MTD_RAW_NAND=y
> +CONFIG_PHY_MARVELL=y
>  CONFIG_DM_ETH=y
>  CONFIG_MVGBE=y
>  CONFIG_MII=y
> diff --git a/drivers/net/mvgbe.c b/drivers/net/mvgbe.c
> index 954bf86121..9429c2913b 100644
> --- a/drivers/net/mvgbe.c
> +++ b/drivers/net/mvgbe.c
> @@ -726,6 +726,15 @@ static int mvgbe_recv(struct eth_device *dev)
>  }
>  #endif
>
> +#ifdef CONFIG_TARGET_NSA310S
> +/* PHY related */
> +#define MV88E1318_PGADR_REG    22
> +#define MV88E1318_MAC_CTRL_PG  2
> +#define MV88E1318_MAC_CTRL_REG 21
> +#define MV88E1318_RGMII_TX_CTRL        BIT(4)
> +#define MV88E1318_RGMII_RX_CTRL        BIT(5)
> +#endif
> +
>  #if defined(CONFIG_PHYLIB) || defined(CONFIG_DM_ETH)
>  #if defined(CONFIG_DM_ETH)
>  static struct phy_device *__mvgbe_phy_init(struct udevice *dev,
> @@ -740,11 +749,20 @@ static struct phy_device *__mvgbe_phy_init(struct eth_device *dev,
>  #endif
>  {
>         struct phy_device *phydev;
> +       int reg;
>
>         /* Set phy address of the port */
>         miiphy_write(dev->name, MV_PHY_ADR_REQUEST, MV_PHY_ADR_REQUEST,
>                      phyid);
>
> +#ifdef CONFIG_TARGET_NSA310S
> +       miiphy_write(dev->name, phyid, MV88E1318_PGADR_REG, MV88E1318_MAC_CTRL_PG);
> +       miiphy_read(dev->name, phyid, MV88E1318_MAC_CTRL_REG, &reg);
> +       reg |= (MV88E1318_RGMII_RX_CTRL | MV88E1318_RGMII_TX_CTRL);
> +       miiphy_write(dev->name, phyid, MV88E1318_MAC_CTRL_REG, reg);
> +       miiphy_write(dev->name, phyid, MV88E1318_PGADR_REG, 0);
> +#endif
> +
>         phydev = phy_connect(bus, phyid, dev, phy_interface);
>         if (!phydev) {
>                 printf("phy_connect failed\n");
> diff --git a/include/configs/nsa310s.h b/include/configs/nsa310s.h
> index ccf4519119..09a747f8af 100644
> --- a/include/configs/nsa310s.h
> +++ b/include/configs/nsa310s.h
> @@ -1,6 +1,6 @@
>  /* SPDX-License-Identifier: GPL-2.0+ */
>  /*
> - * Copyright (C) 2015, 2021 Tony Dinh <mibodhi at gmail.com>
> + * Copyright (C) 2015, 2021-2022 Tony Dinh <mibodhi at gmail.com>
>   * Copyright (C) 2015
>   * Gerald Kerma <dreagle at doukki.net>
>   * Luka Perkov <luka.perkov at sartura.hr>
> @@ -11,8 +11,6 @@
>
>  #include "mv-common.h"
>
> -/* environment variables configuration */
> -
>  /* default environment variables */
>
>  #define CONFIG_EXTRA_ENV_SETTINGS \
> @@ -24,15 +22,14 @@
>         "bootargs_root=ubi.mtd=3 root=ubi0:rootfs rootfstype=ubifs rw\0"
>
>  /* Ethernet driver configuration */
> -#ifdef CONFIG_CMD_NET
>  #define CONFIG_MVGBE_PORTS     {1, 0}  /* enable port 0 only */
>  #define CONFIG_PHY_BASE_ADR    1
> -#define CONFIG_RESET_PHY_R
> -#endif /* CONFIG_CMD_NET */
> +#ifdef CONFIG_RESET_PHY_R
> +#undef CONFIG_RESET_PHY_R       /* remove legacy reset_phy() */
> +#endif
>
> -/* SATA driver configuration */
> -#ifdef CONFIG_SATA
> +/* Support large HDDs for USB and SATA */
>  #define CONFIG_LBA48
> -#endif /* CONFIG_SATA */
> +#define CONFIG_SYS_64BIT_LBA
>
>  #endif /* _CONFIG_NSA310S_H */
> --
> 2.30.2
>


More information about the U-Boot mailing list