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

Hi Michal,

On 30 November 2015 at 08:05, Michal Simek michal.simek@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@xilinx.com

arch/arm/Kconfig | 2 + board/xilinx/zynq/board.c | 13 ---- board/xilinx/zynqmp/zynqmp.c | 25 ------- drivers/mmc/zynq_sdhci.c | 17 +++++ drivers/net/zynq_gem.c | 166 ++++++++++++++++++++++++------------------ include/configs/zynq-common.h | 6 -- include/netdev.h | 2 - 7 files changed, 116 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/mmc/zynq_sdhci.c b/drivers/mmc/zynq_sdhci.c index e169b774932a..5ea992a3ce65 100644 --- a/drivers/mmc/zynq_sdhci.c +++ b/drivers/mmc/zynq_sdhci.c @@ -33,6 +33,23 @@ int zynq_sdhci_init(phys_addr_t regbase) return 0; }

+static const struct udevice_id arasan_sdhci_ids[] = {

•   { .compatible = "arasan,sdhci-8.9a" },
  

•   { }
  

+};

+U_BOOT_DRIVER(arasan_sdhci_drv) = {

•   .name           = "rockchip_dwmmc",
  

•   .id             = UCLASS_MMC,
  

•   .of_match       = rockchip_dwmmc_ids,
  

•   .ofdata_to_platdata = rockchip_dwmmc_ofdata_to_platdata,
  

•   .probe          = rockchip_dwmmc_probe,
  

•   .priv_auto_alloc_size = sizeof(struct rockchip_dwmmc_priv),
  

+};

This seems unrelated / also rockchip stuff.

#if CONFIG_IS_ENABLED(OF_CONTROL) int zynq_sdhci_of_init(const void *blob) { diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 4e93707c7ab1..f2a14938036f 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -13,6 +13,7 @@ #include <net.h> #include <netdev.h> #include <config.h> +#include <dm.h>

Can you put <dm.h> up higher so that these are in order?

#include <malloc.h> #include <asm/io.h> #include <phy.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,58 @@ 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 *)dev_get_addr(dev);
  

Better to use:

priv->iobase = (struct zynq_gem_regs *)pdata->iobase

I think. But is pdata->iobase ever used?

•   /* Hardcode for now */
  

•   priv->emio = 0;
  

•   offset = fdtdec_lookup_phandle(gd->fdt_blob, dev->of_offset,
  

•                                  "phy-handle");
  

•   if (offset != -1)
  

I think this should be:

offset > 0

•           priv->phyaddr = fdtdec_get_int(gd->fdt_blob, offset, "reg", 0);
  
  

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

•   .ops    = &zynq_gem_ops,
  

•   .priv_auto_alloc_size = sizeof(struct zynq_gem_priv),
  

+}; 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.

-- 1.9.1

Regards, Simon