[U-Boot] [PATCH v2 12/14] net: gem: Move driver to DM

Bin Meng bmeng.cn at gmail.com
Wed Dec 2 14:16:41 CET 2015


Hi Michal,

On Wed, Dec 2, 2015 at 7:36 PM, Michal Simek <michal.simek at xilinx.com> wrote:
> - Enable DM_ETH by default for Zynq and ZynqMP
> - Remove board_eth_init code
> - Change miiphy_read function to return value instead of error code
>   based on DM requirement
> - Do not enable EMIO DT support by default
>
> Signed-off-by: Michal Simek <michal.simek at xilinx.com>
> ---
>
> Changes in v2:
> - Remove accidentially added mmc code
> - Sort dm.h header
> - add platdata_auto_alloc_size
> - Read iobase from pdata instead of via gev_get_addr
> - Check positive return value from fdtdec_lookup_phandles instead of -1
>
>  arch/arm/Kconfig              |   2 +
>  board/xilinx/zynq/board.c     |  13 ----
>  board/xilinx/zynqmp/zynqmp.c  |  25 -------
>  drivers/net/zynq_gem.c        | 167 +++++++++++++++++++++++++-----------------
>  include/configs/zynq-common.h |   6 --
>  include/netdev.h              |   2 -
>  6 files changed, 100 insertions(+), 115 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 6542c38304a5..f989ab521469 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -551,6 +551,7 @@ config ARCH_ZYNQ
>         select OF_CONTROL
>         select SPL_OF_CONTROL
>         select DM
> +       select DM_ETH
>         select SPL_DM
>         select DM_SPI
>         select DM_SERIAL
> @@ -562,6 +563,7 @@ config ARCH_ZYNQMP
>         select ARM64
>         select DM
>         select OF_CONTROL
> +       select DM_ETH
>         select DM_SERIAL
>
>  config TEGRA
> diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> index 237f2c2a2bf6..572b1468bf51 100644
> --- a/board/xilinx/zynq/board.c
> +++ b/board/xilinx/zynq/board.c
> @@ -119,19 +119,6 @@ int board_eth_init(bd_t *bis)
>         ret |= xilinx_emaclite_initialize(bis, XILINX_EMACLITE_BASEADDR,
>                         txpp, rxpp);
>  #endif
> -
> -#if defined(CONFIG_ZYNQ_GEM)
> -# if defined(CONFIG_ZYNQ_GEM0)
> -       ret |= zynq_gem_initialize(bis, ZYNQ_GEM_BASEADDR0,
> -                                  CONFIG_ZYNQ_GEM_PHY_ADDR0,
> -                                  CONFIG_ZYNQ_GEM_EMIO0);
> -# endif
> -# if defined(CONFIG_ZYNQ_GEM1)
> -       ret |= zynq_gem_initialize(bis, ZYNQ_GEM_BASEADDR1,
> -                                  CONFIG_ZYNQ_GEM_PHY_ADDR1,
> -                                  CONFIG_ZYNQ_GEM_EMIO1);
> -# endif
> -#endif
>         return ret;
>  }
>
> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
> index d105bb4de32f..51dc30f90d7e 100644
> --- a/board/xilinx/zynqmp/zynqmp.c
> +++ b/board/xilinx/zynqmp/zynqmp.c
> @@ -65,31 +65,6 @@ void scsi_init(void)
>  }
>  #endif
>
> -int board_eth_init(bd_t *bis)
> -{
> -       u32 ret = 0;
> -
> -#if defined(CONFIG_ZYNQ_GEM)
> -# if defined(CONFIG_ZYNQ_GEM0)
> -       ret |= zynq_gem_initialize(bis, ZYNQ_GEM_BASEADDR0,
> -                                               CONFIG_ZYNQ_GEM_PHY_ADDR0, 0);
> -# endif
> -# if defined(CONFIG_ZYNQ_GEM1)
> -       ret |= zynq_gem_initialize(bis, ZYNQ_GEM_BASEADDR1,
> -                                               CONFIG_ZYNQ_GEM_PHY_ADDR1, 0);
> -# endif
> -# if defined(CONFIG_ZYNQ_GEM2)
> -       ret |= zynq_gem_initialize(bis, ZYNQ_GEM_BASEADDR2,
> -                                               CONFIG_ZYNQ_GEM_PHY_ADDR2, 0);
> -# endif
> -# if defined(CONFIG_ZYNQ_GEM3)
> -       ret |= zynq_gem_initialize(bis, ZYNQ_GEM_BASEADDR3,
> -                                               CONFIG_ZYNQ_GEM_PHY_ADDR3, 0);
> -# endif
> -#endif
> -       return ret;
> -}
> -
>  #ifdef CONFIG_CMD_MMC
>  int board_mmc_init(bd_t *bd)
>  {
> diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c
> index 4e93707c7ab1..d800f58c0229 100644
> --- a/drivers/net/zynq_gem.c
> +++ b/drivers/net/zynq_gem.c
> @@ -10,6 +10,7 @@
>   */
>
>  #include <common.h>
> +#include <dm.h>
>  #include <net.h>
>  #include <netdev.h>
>  #include <config.h>
> @@ -23,6 +24,8 @@
>  #include <asm/arch/sys_proto.h>
>  #include <asm-generic/errno.h>
>
> +DECLARE_GLOBAL_DATA_PTR;
> +
>  #if !defined(CONFIG_PHYLIB)
>  # error XILINX_GEM_ETHERNET requires PHYLIB
>  #endif
> @@ -241,7 +244,7 @@ static u32 phywrite(struct zynq_gem_priv *priv, u32 phy_addr,
>                             ZYNQ_GEM_PHYMNTNC_OP_W_MASK, &data);
>  }
>
> -static int phy_detection(struct eth_device *dev)
> +static int phy_detection(struct udevice *dev)
>  {
>         int i;
>         u16 phyreg;
> @@ -280,20 +283,22 @@ static int phy_detection(struct eth_device *dev)
>         return -1;
>  }
>
> -static int zynq_gem_setup_mac(struct eth_device *dev)
> +static int zynq_gem_setup_mac(struct udevice *dev)
>  {
>         u32 i, macaddrlow, macaddrhigh;
> -       struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> +       struct eth_pdata *pdata = dev_get_platdata(dev);
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       struct zynq_gem_regs *regs = priv->iobase;
>
>         /* Set the MAC bits [31:0] in BOT */
> -       macaddrlow = dev->enetaddr[0];
> -       macaddrlow |= dev->enetaddr[1] << 8;
> -       macaddrlow |= dev->enetaddr[2] << 16;
> -       macaddrlow |= dev->enetaddr[3] << 24;
> +       macaddrlow = pdata->enetaddr[0];
> +       macaddrlow |= pdata->enetaddr[1] << 8;
> +       macaddrlow |= pdata->enetaddr[2] << 16;
> +       macaddrlow |= pdata->enetaddr[3] << 24;
>
>         /* Set MAC bits [47:32] in TOP */
> -       macaddrhigh = dev->enetaddr[4];
> -       macaddrhigh |= dev->enetaddr[5] << 8;
> +       macaddrhigh = pdata->enetaddr[4];
> +       macaddrhigh |= pdata->enetaddr[5] << 8;
>
>         for (i = 0; i < 4; i++) {
>                 writel(0, &regs->laddr[i][LADDR_LOW]);
> @@ -308,11 +313,11 @@ static int zynq_gem_setup_mac(struct eth_device *dev)
>         return 0;
>  }
>
> -static int zynq_phy_init(struct eth_device *dev)
> +static int zynq_phy_init(struct udevice *dev)
>  {
>         int ret;
> -       struct zynq_gem_priv *priv = dev->priv;
> -       struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       struct zynq_gem_regs *regs = priv->iobase;
>         const u32 supported = SUPPORTED_10baseT_Half |
>                         SUPPORTED_10baseT_Full |
>                         SUPPORTED_100baseT_Half |
> @@ -342,12 +347,12 @@ static int zynq_phy_init(struct eth_device *dev)
>         return 0;
>  }
>
> -static int zynq_gem_init(struct eth_device *dev, bd_t *bis)
> +static int zynq_gem_init(struct udevice *dev)
>  {
>         u32 i;
>         unsigned long clk_rate = 0;
> -       struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> -       struct zynq_gem_priv *priv = dev->priv;
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       struct zynq_gem_regs *regs = priv->iobase;
>         struct emac_bd *dummy_tx_bd = &priv->tx_bd[TX_FREE_DESC];
>         struct emac_bd *dummy_rx_bd = &priv->tx_bd[TX_FREE_DESC + 2];
>
> @@ -437,7 +442,7 @@ static int zynq_gem_init(struct eth_device *dev, bd_t *bis)
>
>         /* Change the rclk and clk only not using EMIO interface */
>         if (!priv->emio)
> -               zynq_slcr_gem_clk_setup(dev->iobase !=
> +               zynq_slcr_gem_clk_setup((ulong)priv->iobase !=
>                                         ZYNQ_GEM_BASEADDR0, clk_rate);
>
>         setbits_le32(&regs->nwctrl, ZYNQ_GEM_NWCTRL_RXEN_MASK |
> @@ -473,11 +478,11 @@ static int wait_for_bit(const char *func, u32 *reg, const u32 mask,
>         return -ETIMEDOUT;
>  }
>
> -static int zynq_gem_send(struct eth_device *dev, void *ptr, int len)
> +static int zynq_gem_send(struct udevice *dev, void *ptr, int len)
>  {
>         u32 addr, size;
> -       struct zynq_gem_priv *priv = dev->priv;
> -       struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       struct zynq_gem_regs *regs = priv->iobase;
>         struct emac_bd *current_bd = &priv->tx_bd[1];
>
>         /* Setup Tx BD */
> @@ -518,10 +523,10 @@ static int zynq_gem_send(struct eth_device *dev, void *ptr, int len)
>  }
>
>  /* Do not check frame_recd flag in rx_status register 0x20 - just poll BD */
> -static int zynq_gem_recv(struct eth_device *dev)
> +static int zynq_gem_recv(struct udevice *dev, int flags, uchar **packetp)
>  {
>         int frame_len;
> -       struct zynq_gem_priv *priv = dev->priv;
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
>         struct emac_bd *current_bd = &priv->rx_bd[priv->rxbd_current];
>         struct emac_bd *first_bd;
>
> @@ -561,54 +566,41 @@ static int zynq_gem_recv(struct eth_device *dev)
>         return frame_len;
>  }
>
> -static void zynq_gem_halt(struct eth_device *dev)
> +static void zynq_gem_halt(struct udevice *dev)
>  {
> -       struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       struct zynq_gem_regs *regs = priv->iobase;
>
>         clrsetbits_le32(&regs->nwctrl, ZYNQ_GEM_NWCTRL_RXEN_MASK |
>                                                 ZYNQ_GEM_NWCTRL_TXEN_MASK, 0);
>  }
>
> -static int zynq_gem_miiphy_read(const char *devname, uchar addr,
> -                                                       uchar reg, ushort *val)
> +static int zynq_gem_miiphy_read(struct mii_dev *bus, int addr,
> +                               int devad, int reg)
>  {
> -       struct eth_device *dev = eth_get_dev();
> -       struct zynq_gem_priv *priv = dev->priv;
> +       struct zynq_gem_priv *priv = bus->priv;
>         int ret;
> +       u16 val;
>
> -       ret = phyread(priv, addr, reg, val);
> -       debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, *val);
> -       return ret;
> +       ret = phyread(priv, addr, reg, &val);
> +       debug("%s 0x%x, 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, val, ret);
> +       return val;
>  }
>
> -static int zynq_gem_miiphy_write(const char *devname, uchar addr,
> -                                                       uchar reg, ushort val)
> +static int zynq_gem_miiphy_write(struct mii_dev *bus, int addr, int devad,
> +                                int reg, u16 value)
>  {
> -       struct eth_device *dev = eth_get_dev();
> -       struct zynq_gem_priv *priv = dev->priv;
> +       struct zynq_gem_priv *priv = bus->priv;
>
> -       debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, val);
> -       return phywrite(priv, addr, reg, val);
> +       debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, value);
> +       return phywrite(priv, addr, reg, value);
>  }
>
> -int zynq_gem_initialize(bd_t *bis, phys_addr_t base_addr,
> -                       int phy_addr, u32 emio)
> +static int zynq_gem_probe(struct udevice *dev)
>  {
> -       int ret;
> -       struct eth_device *dev;
> -       struct zynq_gem_priv *priv;
>         void *bd_space;
> -
> -       dev = calloc(1, sizeof(*dev));
> -       if (dev == NULL)
> -               return -1;
> -
> -       dev->priv = calloc(1, sizeof(struct zynq_gem_priv));
> -       if (dev->priv == NULL) {
> -               free(dev);
> -               return -1;
> -       }
> -       priv = dev->priv;
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       int ret;
>
>         /* Align rxbuffers to ARCH_DMA_MINALIGN */
>         priv->rxbuffers = memalign(ARCH_DMA_MINALIGN, RX_BUF * PKTSIZE_ALIGN);
> @@ -623,8 +615,11 @@ int zynq_gem_initialize(bd_t *bis, phys_addr_t base_addr,
>         priv->tx_bd = (struct emac_bd *)bd_space;
>         priv->rx_bd = (struct emac_bd *)((ulong)bd_space + BD_SEPRN_SPACE);
>
> -       priv->phyaddr = phy_addr;
> -       priv->emio = emio;
> +       priv->bus = mdio_alloc();
> +       priv->bus->read = zynq_gem_miiphy_read;
> +       priv->bus->write = zynq_gem_miiphy_write;
> +       priv->bus->priv = priv;
> +       strcpy(priv->bus->name, "gem");
>
>  #ifndef CONFIG_ZYNQ_GEM_INTERFACE
>         priv->interface = PHY_INTERFACE_MODE_MII;
> @@ -632,25 +627,59 @@ int zynq_gem_initialize(bd_t *bis, phys_addr_t base_addr,
>         priv->interface = CONFIG_ZYNQ_GEM_INTERFACE;
>  #endif
>
> -       sprintf(dev->name, "Gem.%lx", base_addr);
> +       ret = mdio_register(priv->bus);
> +       if (ret)
> +               return ret;
>
> -       dev->iobase = base_addr;
> -       priv->iobase = (struct zynq_gem_regs *)base_addr;
> +       zynq_phy_init(dev);
>
> -       dev->init = zynq_gem_init;
> -       dev->halt = zynq_gem_halt;
> -       dev->send = zynq_gem_send;
> -       dev->recv = zynq_gem_recv;
> -       dev->write_hwaddr = zynq_gem_setup_mac;
> +       return 0;
> +}
>
> -       eth_register(dev);
> +static const struct eth_ops zynq_gem_ops = {
> +       .start                  = zynq_gem_init,
> +       .send                   = zynq_gem_send,
> +       .recv                   = zynq_gem_recv,
> +       .stop                   = zynq_gem_halt,
> +       .write_hwaddr           = zynq_gem_setup_mac,
> +};
>
> -       miiphy_register(dev->name, zynq_gem_miiphy_read, zynq_gem_miiphy_write);
> -       priv->bus = miiphy_get_dev_by_name(dev->name);
> +static int zynq_gem_ofdata_to_platdata(struct udevice *dev)
> +{
> +       struct eth_pdata *pdata = dev_get_platdata(dev);
> +       struct zynq_gem_priv *priv = dev_get_priv(dev);
> +       int offset = 0;
>
> -       ret = zynq_phy_init(dev);
> -       if (ret)
> -               return ret;
> +       pdata->iobase = (phys_addr_t)dev_get_addr(dev);
> +       priv->iobase = (struct zynq_gem_regs *)pdata->iobase;
> +       /* Hardcode for now */
> +       priv->emio = 0;
> +
> +       offset = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
> +                                      "phy-handle");
> +       if (offset > 0)
> +               priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", 0);

I don't see where is this priv->phyaddr used in this driver?

>
> -       return 1;
> +       printf("ZYNQ GEM: %lx, phyaddr %d\n", (ulong)priv->iobase,
> +              priv->phyaddr);
> +
> +       return 0;
>  }
> +
> +static const struct udevice_id zynq_gem_ids[] = {
> +       { .compatible = "cdns,zynqmp-gem" },
> +       { .compatible = "cdns,zynq-gem" },
> +       { .compatible = "cdns,gem" },
> +       { }
> +};
> +
> +U_BOOT_DRIVER(zynq_gem) = {
> +       .name   = "zynq_gem",
> +       .id     = UCLASS_ETH,
> +       .of_match = zynq_gem_ids,
> +       .ofdata_to_platdata = zynq_gem_ofdata_to_platdata,
> +       .probe  = zynq_gem_probe,

Please add .remove function, otherwise there will be memory leak when
removing the device. See designware.c for example.

> +       .ops    = &zynq_gem_ops,
> +       .priv_auto_alloc_size = sizeof(struct zynq_gem_priv),
> +       .platdata_auto_alloc_size = sizeof(struct eth_pdata),
> +};
> diff --git a/include/configs/zynq-common.h b/include/configs/zynq-common.h
> index 5db501188b18..faff0e9b70f7 100644
> --- a/include/configs/zynq-common.h
> +++ b/include/configs/zynq-common.h
> @@ -56,12 +56,6 @@
>  # define CONFIG_BOOTP_GATEWAY
>  # define CONFIG_BOOTP_HOSTNAME
>  # define CONFIG_BOOTP_MAY_FAIL
> -# if !defined(CONFIG_ZYNQ_GEM_EMIO0)
> -#  define CONFIG_ZYNQ_GEM_EMIO0        0
> -# endif
> -# if !defined(CONFIG_ZYNQ_GEM_EMIO1)
> -#  define CONFIG_ZYNQ_GEM_EMIO1        0
> -# endif
>  #endif
>
>  /* SPI */
> diff --git a/include/netdev.h b/include/netdev.h
> index 5c6ae5b5624e..de74b9a534b1 100644
> --- a/include/netdev.h
> +++ b/include/netdev.h
> @@ -87,8 +87,6 @@ int xilinx_emaclite_initialize(bd_t *bis, unsigned long base_addr,
>                                                         int txpp, int rxpp);
>  int xilinx_ll_temac_eth_init(bd_t *bis, unsigned long base_addr, int flags,
>                                                 unsigned long ctrl_addr);
> -int zynq_gem_initialize(bd_t *bis, phys_addr_t base_addr,
> -                       int phy_addr, u32 emio);
>  /*
>   * As long as the Xilinx xps_ll_temac ethernet driver has not its own interface
>   * exported by a public hader file, we need a global definition at this point.
> --

Regards,
Bin


More information about the U-Boot mailing list