[U-Boot] [PATCH v3] net: ll_temac: Add LL TEMAC driver to u-boot
LL Temac driver can be used by microblaze, xilinx ppc405/440 in sdma and fifo mode. DCR or XPS bus can be used.
The driver uses and requires PHYLIB.
Signed-off-by: Michal Simek monstr@monstr.eu
--- v2: Remove helper function for access to temac Remove SDMA/FIFO/DCR macros and configure it in board Setup mac by write_hwaddr
v3: Use helper functions for fifo mode Use helper functions for indirect accesses Code cleanup Add comments for MAGIC values Simplify code in fifo mode --- drivers/net/Makefile | 1 + drivers/net/xilinx_ll_temac.c | 666 +++++++++++++++++++++++++++++++++++++++++ include/netdev.h | 2 + 3 files changed, 669 insertions(+), 0 deletions(-) create mode 100644 drivers/net/xilinx_ll_temac.c
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 819b197..4541eaf 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -84,6 +84,7 @@ COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o COBJS-$(CONFIG_ULI526X) += uli526x.o COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o +COBJS-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
COBJS := $(sort $(COBJS-y)) SRCS := $(COBJS:.o=.c) diff --git a/drivers/net/xilinx_ll_temac.c b/drivers/net/xilinx_ll_temac.c new file mode 100644 index 0000000..1639363 --- /dev/null +++ b/drivers/net/xilinx_ll_temac.c @@ -0,0 +1,666 @@ +/* + * Xilinx xps_ll_temac ethernet driver for u-boot + * + * Copyright (C) 2008 - 2011 Michal Simek monstr@monstr.eu + * Copyright (C) 2008 - 2011 PetaLogix + * + * Based on Yoshio Kashiwagi kashiwagi@co-nss.co.jp driver + * Copyright (C) 2008 Nissin Systems Co.,Ltd. + * March 2008 created + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + */ + +#include <config.h> +#include <common.h> +#include <net.h> +#include <malloc.h> +#include <asm/processor.h> +#include <asm/io.h> +#include <phy.h> +#include <miiphy.h> + +#undef ETH_HALTING + +#if !defined(CONFIG_PHYLIB) +# error LL_TEMAC requires PHYLIB +#endif + +#define XTE_EMMC_LINKSPEED_MASK 0xC0000000 /* Link speed */ +/* XTE_EMCFG_LINKSPD_MASK */ +#define XTE_EMMC_LINKSPD_10 0x00000000 /* for 10 Mbit */ +#define XTE_EMMC_LINKSPD_100 0x40000000 /* for 100 Mbit */ +#define XTE_EMMC_LINKSPD_1000 0x80000000 /* forr 1000 Mbit */ + +#define XTE_RSE_MIIM_RR_MASK 0x0002 +#define XTE_RSE_MIIM_WR_MASK 0x0004 +#define XTE_RSE_CFG_RR_MASK 0x0020 +#define XTE_RSE_CFG_WR_MASK 0x0040 + +/* XPS_LL_TEMAC indirect registers offset definition */ +#define RCW1 0x240 +#define TC 0x280 +#define EMMC 0x300 +#define MC 0x340 +#define UAW0 0x380 +#define UAW1 0x384 +#define AFM 0x390 +#define MIIMWD 0x3b0 +#define MIIMAI 0x3b4 + +#define CNTLREG_WRITE_ENABLE_MASK 0x8000 + +#define MDIO_ENABLE_MASK 0x40 +#define MDIO_CLOCK_DIV_100MHz 0x28 + +/* XPS_LL_TEMAC SDMA registers definition */ +#define TX_CURDESC_PTR 0x03 +#define TX_TAILDESC_PTR 0x04 +#define TX_CHNL_CTRL 0x05 +#define TX_IRQ_REG 0x06 +#define TX_CHNL_STS 0x07 +#define RX_NXTDESC_PTR 0x08 +#define RX_CURDESC_PTR 0x0b +#define RX_TAILDESC_PTR 0x0c +#define RX_CHNL_CTRL 0x0d +#define RX_IRQ_REG 0x0e +#define RX_CHNL_STS 0x0f +#define DMA_CONTROL_REG 0x10 + +/* DMA control bit */ +#define DMA_CONTROL_RESET 0x1 + +/* CDMAC descriptor status bit definitions */ +# define BDSTAT_STOP_ON_END_MASK 0x20 +# define BDSTAT_COMPLETED_MASK 0x10 +# define BDSTAT_SOP_MASK 0x08 +# define BDSTAT_EOP_MASK 0x04 + +# define CHNL_STS_ERROR_MASK 0x80 + +/* All interrupt enable bits */ +#define XLLDMA_CR_IRQ_ALL_EN_MASK 0x00000087 +/* All interrupt bits */ +#define XLLDMA_IRQ_ALL_MASK 0x0000001F +/* Disable error when 2 or 4 bit coalesce counter overflows */ +#define XLLDMA_DMACR_RX_OVERFLOW_ERR_DIS_MASK 0x00000010 +/* Disable error when 2 or 4 bit coalesce counter overflows */ +#define XLLDMA_DMACR_TX_OVERFLOW_ERR_DIS_MASK 0x00000008 +/* Enable use of tail pointer register */ +#define XLLDMA_DMACR_TAIL_PTR_EN_MASK 0x00000004 + +#define LL_FIFO_ISR_RC_COMPLETE 0x04000000 + +#define SDMA_BIT 1 +#define DCR_BIT 2 + +#define DMAALIGN 32 + +/* SDMA Buffer Descriptor */ +struct cdmac_bd_t { + struct cdmac_bd_t *next_p; + unsigned char *phys_buf_p; + unsigned long buf_len; + unsigned char stat; + unsigned char app1_1; + unsigned short app1_2; + unsigned long app2; + unsigned long app3; + unsigned long app4; + unsigned long app5; +}; + +static struct cdmac_bd_t tx_bd __attribute((aligned(DMAALIGN))); +static struct cdmac_bd_t rx_bd __attribute((aligned(DMAALIGN))); + +struct ll_fifo_s { + u32 isr; /* Interrupt Status Register 0x0 */ + u32 ier; /* Interrupt Enable Register 0x4 */ + u32 tdfr; /* Transmit data FIFO reset 0x8 */ + u32 tdfv; /* Transmit data FIFO Vacancy 0xC */ + u32 tdfd; /* Transmit data FIFO 32bit wide data write port 0x10 */ + u32 tlf; /* Write Transmit Length FIFO 0x14 */ + u32 rdfr; /* Read Receive data FIFO reset 0x18 */ + u32 rdfo; /* Receive data FIFO Occupancy 0x1C */ + u32 rdfd; /* Read Receive data FIFO 32bit wide data read port 0x20 */ + u32 rlf; /* Read Receive Length FIFO 0x24 */ + u32 llr; /* Read LocalLink reset 0x28 */ +}; + +static u8 tx_buffer[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); +static u8 rx_buffer[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); + +struct temac_reg { + u32 reserved[8]; + u32 msw; /* Hard TEMAC MSW Data Register */ + u32 lsw; /* Hard TEMAC LSW Data Register */ + u32 ctl; /* Hard TEMAC Control Register */ + u32 rdy; /* Hard TEMAC Ready Status */ +}; + +struct ll_priv { + u32 ctrl; + u32 mode; + int phyaddr; + + struct phy_device *phydev; + struct mii_dev *bus; +}; + +#define XILINX_INDIRECT_DCR_ADDRESS_REG 0 +#define XILINX_INDIRECT_DCR_ACCESS_REG 1 + +static void mtdcr_local(u32 reg, u32 val) +{ +#if defined(CONFIG_PPC) + mtdcr(XILINX_INDIRECT_DCR_ADDRESS_REG, reg); + mtdcr(XILINX_INDIRECT_DCR_ACCESS_REG, val); +#endif +} + +static u32 mfdcr_local(u32 reg) +{ + u32 val = 0; +#if defined(CONFIG_PPC) + mtdcr(XILINX_INDIRECT_DCR_ADDRESS_REG, reg); + val = mfdcr(XILINX_INDIRECT_DCR_ACCESS_REG); +#endif + return val; +} + +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{ + if (priv->mode & DCR_BIT) + mtdcr_local(priv->ctrl + offset, val); + else + out_be32((u32 *)(priv->ctrl + offset * 4), val); +} + +static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) +{ + if (priv->mode & DCR_BIT) + return mfdcr_local(priv->ctrl + offset); + + return in_be32((u32 *)(priv->ctrl + offset * 4)); +} + +static void xps_ll_temac_check_status(struct temac_reg *regs, int mask) +{ + u32 timeout = 2000; + while (timeout && (!(in_be32(®s->rdy) & mask))) + timeout--; + + if (!timeout) + printf("%s: Timeout\n", __func__); +} + +#if defined(DEBUG) || defined(CONFIG_MII) || defined(CONFIG_CMD_MII) +/* undirect hostif write to ll_temac */ +static void xps_ll_temac_hostif_set(struct eth_device *dev, int emac, + int phy_addr, int reg_addr, int phy_data) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->lsw, phy_data); + out_be32(®s->ctl, CNTLREG_WRITE_ENABLE_MASK | MIIMWD); + out_be32(®s->lsw, (phy_addr << 5) | reg_addr); + out_be32(®s->ctl, CNTLREG_WRITE_ENABLE_MASK | MIIMAI | (emac << 10)); + xps_ll_temac_check_status(regs, XTE_RSE_MIIM_WR_MASK); +} +#endif + +/* undirect hostif read from ll_temac */ +static unsigned int xps_ll_temac_hostif_get(struct eth_device *dev, + int emac, int phy_addr, int reg_addr) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->lsw, (phy_addr << 5) | reg_addr); + out_be32(®s->ctl, MIIMAI | (emac << 10)); + xps_ll_temac_check_status(regs, XTE_RSE_MIIM_RR_MASK); + return in_be32(®s->lsw); +} + +/* undirect write to ll_temac */ +static void xps_ll_temac_indirect_set(struct eth_device *dev, + int emac, int reg_offset, int reg_data) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->lsw, reg_data); + out_be32(®s->ctl, + CNTLREG_WRITE_ENABLE_MASK | (emac << 10) | reg_offset); + xps_ll_temac_check_status(regs, XTE_RSE_CFG_WR_MASK); +} + +/* undirect read from ll_temac */ +static int xps_ll_temac_indirect_get(struct eth_device *dev, + int emac, int reg_offset) +{ + struct temac_reg *regs = (struct temac_reg *)dev->iobase; + + out_be32(®s->ctl, (emac << 10) | reg_offset); + xps_ll_temac_check_status(regs, XTE_RSE_CFG_RR_MASK); + return in_be32(®s->lsw); +} + +#ifdef DEBUG +/* read from phy */ +static void read_phy_reg(struct eth_device *dev, int phy_addr) +{ + int j, result; + debug("phy%d ", phy_addr); + for (j = 0; j < 32; j++) { + result = xps_ll_temac_hostif_get(dev, 0, phy_addr, j); + debug("%d: 0x%x ", j, result); + } + debug("\n"); +} +#endif + +/* setting ll_temac and phy to proper setting */ +static int xps_ll_temac_phy_ctrl(struct eth_device *dev) +{ + int i; + unsigned int temp, speed; + struct ll_priv *priv = dev->priv; + struct phy_device *phydev; + + u32 supported = SUPPORTED_10baseT_Half | + SUPPORTED_10baseT_Full | + SUPPORTED_100baseT_Half | + SUPPORTED_100baseT_Full | + SUPPORTED_1000baseT_Half | + SUPPORTED_1000baseT_Full; + + if (priv->phyaddr == -1) { + for (i = 31; i >= 0; i--) { + temp = xps_ll_temac_hostif_get(dev, 0, i, 1); + if ((temp & 0x0ffff) != 0x0ffff) { + debug("phy %x result %x\n", i, temp); + priv->phyaddr = i; + break; + } + } + } + + /* interface - look at tsec */ + phydev = phy_connect(priv->bus, priv->phyaddr, dev, 0); + + phydev->supported &= supported; + phydev->advertising = phydev->supported; + priv->phydev = phydev; + phy_config(phydev); + phy_startup(phydev); + + switch (phydev->speed) { + case 1000: + speed = XTE_EMMC_LINKSPD_1000; + break; + case 100: + speed = XTE_EMMC_LINKSPD_100; + break; + case 10: + speed = XTE_EMMC_LINKSPD_10; + break; + default: + return 0; + } + temp = xps_ll_temac_indirect_get(dev, 0, EMMC); + temp &= ~XTE_EMMC_LINKSPEED_MASK; + temp |= speed; + xps_ll_temac_indirect_set(dev, 0, EMMC, temp); + + return 1; +} + +static inline int xps_ll_temac_dma_error(struct eth_device *dev) +{ + int err; + struct ll_priv *priv = dev->priv; + + /* Check for TX and RX channel errrors. */ + err = sdma_in_be32(priv, TX_CHNL_STS) & CHNL_STS_ERROR_MASK; + err |= sdma_in_be32(priv, RX_CHNL_STS) & CHNL_STS_ERROR_MASK; + return err; +} + +static void xps_ll_temac_reset_dma(struct eth_device *dev) +{ + u32 r; + struct ll_priv *priv = dev->priv; + + /* Soft reset the DMA. */ + sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET); + while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET) + ; + + /* Now clear the interrupts. */ + r = sdma_in_be32(priv, TX_CHNL_CTRL); + r &= ~XLLDMA_CR_IRQ_ALL_EN_MASK; + sdma_out_be32(priv, TX_CHNL_CTRL, r); + + r = sdma_in_be32(priv, RX_CHNL_CTRL); + r &= ~XLLDMA_CR_IRQ_ALL_EN_MASK; + sdma_out_be32(priv, RX_CHNL_CTRL, r); + + /* Now ACK pending IRQs. */ + sdma_out_be32(priv, TX_IRQ_REG, XLLDMA_IRQ_ALL_MASK); + sdma_out_be32(priv, RX_IRQ_REG, XLLDMA_IRQ_ALL_MASK); + + /* Set tail-ptr mode, disable errors for both channels. */ + sdma_out_be32(priv, DMA_CONTROL_REG, + XLLDMA_DMACR_TAIL_PTR_EN_MASK | + XLLDMA_DMACR_RX_OVERFLOW_ERR_DIS_MASK | + XLLDMA_DMACR_TX_OVERFLOW_ERR_DIS_MASK); +} + +/* bd init */ +static void xps_ll_temac_bd_init(struct eth_device *dev) +{ + struct ll_priv *priv = dev->priv; + memset(&tx_bd, 0, sizeof(tx_bd)); + memset(&rx_bd, 0, sizeof(rx_bd)); + + rx_bd.phys_buf_p = rx_buffer; + + rx_bd.next_p = &rx_bd; + rx_bd.buf_len = PKTSIZE_ALIGN; + flush_cache((u32)&rx_bd, sizeof(tx_bd)); + flush_cache((u32)rx_bd.phys_buf_p, PKTSIZE_ALIGN); + + sdma_out_be32(priv, RX_CURDESC_PTR, (u32)&rx_bd); + sdma_out_be32(priv, RX_TAILDESC_PTR, (u32)&rx_bd); + sdma_out_be32(priv, RX_NXTDESC_PTR, (u32)&rx_bd); /* setup first fd */ + + tx_bd.phys_buf_p = tx_buffer; + tx_bd.next_p = &tx_bd; + + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + sdma_out_be32(priv, TX_CURDESC_PTR, (u32)&tx_bd); +} + +static int ll_temac_send_sdma(struct eth_device *dev, + volatile void *buffer, int length) +{ + struct ll_priv *priv = dev->priv; + + if (xps_ll_temac_dma_error(dev)) { + xps_ll_temac_reset_dma(dev); + xps_ll_temac_bd_init(dev); + } + + memcpy(tx_buffer, (void *)buffer, length); + flush_cache((u32)tx_buffer, length); + + tx_bd.stat = BDSTAT_SOP_MASK | BDSTAT_EOP_MASK | + BDSTAT_STOP_ON_END_MASK; + tx_bd.buf_len = length; + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + + sdma_out_be32(priv, TX_CURDESC_PTR, (u32)&tx_bd); + sdma_out_be32(priv, TX_TAILDESC_PTR, (u32)&tx_bd); /* DMA start */ + + do { + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + } while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK)); + + return 0; +} + +static int ll_temac_recv_sdma(struct eth_device *dev) +{ + int length; + struct ll_priv *priv = dev->priv; + + if (xps_ll_temac_dma_error(dev)) { + xps_ll_temac_reset_dma(dev); + xps_ll_temac_bd_init(dev); + } + + flush_cache((u32)&rx_bd, sizeof(rx_bd)); + + if (!(rx_bd.stat & BDSTAT_COMPLETED_MASK)) + return 0; + + /* Read out the packet info and start the DMA + onto the second buffer to enable the ethernet rx + path to run in parallel with sw processing + packets. */ + length = rx_bd.app5 & 0x3FFF; /* max length mask */ + if (length > 0) + NetReceive(rx_bd.phys_buf_p, length); + + /* flip the buffer and re-enable the DMA. */ + flush_cache((u32)rx_bd.phys_buf_p, length); + + rx_bd.buf_len = PKTSIZE_ALIGN; + rx_bd.stat = 0; + rx_bd.app5 = 0; + + flush_cache((u32)&rx_bd, sizeof(rx_bd)); + sdma_out_be32(priv, RX_TAILDESC_PTR, (u32)&rx_bd); + + return length; +} + +#ifdef DEBUG +static void debugll(struct eth_device *dev, int count) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + printf("%d fifo isr 0x%08x, fifo_ier 0x%08x, fifo_rdfr 0x%08x, " + "fifo_rdfo 0x%08x fifo_rlr 0x%08x\n", count, + in_be32(&ll_fifo->isr), in_be32(&ll_fifo->ier), + in_be32(&ll_fifo->rdfr), in_be32(&ll_fifo->rdfo), + in_be32(&ll_fifo->rlf)); +} +#endif + +static int ll_temac_send_fifo(struct eth_device *dev, + volatile void *buffer, int length) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + u32 *buf = (u32 *)buffer; + u32 i; + + for (i = 0; i < length; i += 4) + out_be32(&ll_fifo->tdfd, *buf++); + + out_be32(&ll_fifo->tlf, length); + return 0; +} + +static int ll_temac_recv_fifo(struct eth_device *dev) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + u32 i, len = 0; + u32 *buf = (u32 *)&rx_buffer; + + if (in_be32(&ll_fifo->isr) & LL_FIFO_ISR_RC_COMPLETE) { + out_be32(&ll_fifo->isr, 0xffffffff); /* reset isr */ + + /* while (ll_fifo->isr); */ + len = in_be32(&ll_fifo->rlf) & 0x7FF; + + for (i = 0; i < len; i += 4) + *buf++ = in_be32(&ll_fifo->rdfd); + +#ifdef DEBUG + debugll(dev, 1); +#endif + NetReceive((uchar *)&rx_buffer, len); + } + return len; +} + +/* setup mac addr */ +static int ll_temac_addr_setup(struct eth_device *dev) +{ + int val; + + /* set up unicast MAC address filter */ + val = ((dev->enetaddr[3] << 24) | (dev->enetaddr[2] << 16) | + (dev->enetaddr[1] << 8) | (dev->enetaddr[0])); + xps_ll_temac_indirect_set(dev, 0, UAW0, val); + val = (dev->enetaddr[5] << 8) | dev->enetaddr[4] ; + xps_ll_temac_indirect_set(dev, 0, UAW1, val); + + return 0; +} + +static int xps_ll_temac_init(struct eth_device *dev, bd_t *bis) +{ + struct ll_priv *priv = dev->priv; + struct ll_fifo_s *ll_fifo = (void *)priv->ctrl; + + if (priv->mode & SDMA_BIT) { + xps_ll_temac_reset_dma(dev); + xps_ll_temac_bd_init(dev); + } else { + out_be32(&ll_fifo->tdfr, 0x000000a5); /* Fifo reset key */ + out_be32(&ll_fifo->rdfr, 0x000000a5); /* Fifo reset key */ + out_be32(&ll_fifo->isr, 0xFFFFFFFF); /* Reset status register */ + out_be32(&ll_fifo->ier, 0); /* Disable all IRQs */ + } + + xps_ll_temac_indirect_set(dev, 0, MC, + MDIO_ENABLE_MASK | MDIO_CLOCK_DIV_100MHz); + + /* Promiscuous mode disable */ + xps_ll_temac_indirect_set(dev, 0, AFM, 0); + /* Enable Receiver - RX bit */ + xps_ll_temac_indirect_set(dev, 0, RCW1, 0x10000000); + /* Enable Transmitter - TX bit */ + xps_ll_temac_indirect_set(dev, 0, TC, 0x10000000); + return 0; +} + +/* halt device */ +static void ll_temac_halt(struct eth_device *dev) +{ +#ifdef ETH_HALTING + struct ll_priv *priv = dev->priv; + /* Disable Receiver */ + xps_ll_temac_indirect_set(dev, 0, RCW1, 0); + /* Disable Transmitter */ + xps_ll_temac_indirect_set(dev, 0, TC, 0); + + if (priv->mode & SDMA_BIT) { + sdma_out_be32(priv->ctrl, DMA_CONTROL_REG, DMA_CONTROL_RESET); + while (sdma_in_be32(priv->ctrl, DMA_CONTROL_REG) + & DMA_CONTROL_RESET) + ; + } +#endif +} + +static int ll_temac_init(struct eth_device *dev, bd_t *bis) +{ +#if DEBUG + int i; +#endif + xps_ll_temac_init(dev, bis); + + printf("%s: Xilinx XPS LocalLink Tri-Mode Ether MAC #%d at 0x%08X.\n", + dev->name, 0, dev->iobase); + +#if DEBUG + for (i = 0; i < 32; i++) + read_phy_reg(dev, i); +#endif + + if (!xps_ll_temac_phy_ctrl(dev)) { + ll_temac_halt(dev); + return -1; + } + + return 0; +} + +static int ll_temac_miiphy_read(const char *devname, uchar addr, + uchar reg, ushort *val) +{ + struct eth_device *dev = eth_get_dev(); + + *val = xps_ll_temac_hostif_get(dev, 0, addr, reg); /* emac = 0 */ + + debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, *val); + return 0; +} + +static int ll_temac_miiphy_write(const char *devname, uchar addr, + uchar reg, ushort val) +{ + struct eth_device *dev = eth_get_dev(); + debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, val); + + xps_ll_temac_hostif_set(dev, 0, addr, reg, val); + + return 0; +} + +static int ll_temac_bus_reset(struct mii_dev *bus) +{ + debug("Just bus reset\n"); + return 0; +} + +/* mode bits: 0bit - fifo(0)/sdma(1):SDMA_BIT, 1bit - no dcr(0)/dcr(1):DCR_BIT + * ctrl - control address for file/sdma */ +int xilinx_ll_temac_initialize(bd_t *bis, unsigned long base_addr, + int mode, unsigned long ctrl) +{ + struct eth_device *dev; + struct ll_priv *priv; + + dev = calloc(1, sizeof(*dev)); + if (dev == NULL) + return -1; + + dev->priv = calloc(1, sizeof(struct ll_priv)); + if (dev->priv == NULL) { + free(dev); + return -1; + } + + priv = dev->priv; + + sprintf(dev->name, "Xlltem.%lx", base_addr); + + dev->iobase = base_addr; + priv->ctrl = ctrl; + priv->mode = mode; + +#ifdef CONFIG_PHY_ADDR + priv->phyaddr = CONFIG_PHY_ADDR; +#else + priv->phyaddr = -1; +#endif + + dev->init = ll_temac_init; + dev->halt = ll_temac_halt; + dev->write_hwaddr = ll_temac_addr_setup; + + if (priv->mode & SDMA_BIT) { + dev->send = ll_temac_send_sdma; + dev->recv = ll_temac_recv_sdma; + } else { + dev->send = ll_temac_send_fifo; + dev->recv = ll_temac_recv_fifo; + } + + eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) + miiphy_register(dev->name, ll_temac_miiphy_read, ll_temac_miiphy_write); + priv->bus = miiphy_get_dev_by_name(dev->name); + priv->bus->reset = ll_temac_bus_reset; +#endif + return 1; +} diff --git a/include/netdev.h b/include/netdev.h index 26ce13d..e976659 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -92,6 +92,8 @@ int uec_standard_init(bd_t *bis); int uli526x_initialize(bd_t *bis); int xilinx_emaclite_initialize(bd_t *bis, unsigned long base_addr, int txpp, int rxpp); +int xilinx_ll_temac_initialize(bd_t *bis, unsigned long base_addr, + int mode, unsigned long ctrl); int sh_eth_initialize(bd_t *bis); int dm9000_initialize(bd_t *bis); int fecmxc_initialize(bd_t *bis);
Add axi_ethernet driver for little-endian Microblaze.
RX/TX BDs and rxframe buffer are shared among all axi_ethernet MACs. Only one MAC can work in one time.
Signed-off-by: Michal Simek monstr@monstr.eu
---
v2: Fix cammelcase weirdness Create mdio_wait function with timeouts Synchronize debug messages Remove base address + offset notation -> use struct instead --- drivers/net/Makefile | 1 + drivers/net/xilinx_axi_emac.c | 629 +++++++++++++++++++++++++++++++++++++++++ include/netdev.h | 2 + 3 files changed, 632 insertions(+), 0 deletions(-) create mode 100644 drivers/net/xilinx_axi_emac.c
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 4541eaf..ae9d4cb 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -83,6 +83,7 @@ COBJS-$(CONFIG_TSEC_ENET) += tsec.o fsl_mdio.o COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o COBJS-$(CONFIG_ULI526X) += uli526x.o COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o +COBJS-$(CONFIG_XILINX_AXIEMAC) += xilinx_axi_emac.o COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o COBJS-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c new file mode 100644 index 0000000..1c90b0e --- /dev/null +++ b/drivers/net/xilinx_axi_emac.c @@ -0,0 +1,629 @@ +/* + * Copyright (C) 2011 Michal Simek monstr@monstr.eu + * Copyright (C) 2011 PetaLogix + * Copyright (C) 2010 Xilinx, Inc. All rights reserved. + * + * See file CREDITS for list of people who contributed to this + * project. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License as + * published by the Free Software Foundation; either version 2 of + * the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, + * MA 02111-1307 USA + */ + +#include <config.h> +#include <common.h> +#include <net.h> +#include <malloc.h> +#include <asm/io.h> +#include <phy.h> +#include <miiphy.h> + +#if !defined(CONFIG_PHYLIB) +# error AXI_ETHERNET requires PHYLIB +#endif + +/* Link setup */ +#define XAE_EMMC_LINKSPEED_MASK 0xC0000000 /* Link speed */ +#define XAE_EMMC_LINKSPD_10 0x00000000 /* Link Speed mask for 10 Mbit */ +#define XAE_EMMC_LINKSPD_100 0x40000000 /* Link Speed mask for 100 Mbit */ +#define XAE_EMMC_LINKSPD_1000 0x80000000 /* Link Speed mask for 1000 Mbit */ + +/* Interrupt Status/Enable/Mask Registers bit definitions */ +#define XAE_INT_RXRJECT_MASK 0x00000008 /* Rx frame rejected */ +#define XAE_INT_MGTRDY_MASK 0x00000080 /* MGT clock Lock */ + +/* Receive Configuration Word 1 (RCW1) Register bit definitions */ +#define XAE_RCW1_RX_MASK 0x10000000 /* Receiver enable */ + +/* Transmitter Configuration (TC) Register bit definitions */ +#define XAE_TC_TX_MASK 0x10000000 /* Transmitter enable */ + +#define XAE_UAW1_UNICASTADDR_MASK 0x0000FFFF + +/* MDIO Management Configuration (MC) Register bit definitions */ +#define XAE_MDIO_MC_MDIOEN_MASK 0x00000040 /* MII management enable*/ + +/* MDIO Management Control Register (MCR) Register bit definitions */ +#define XAE_MDIO_MCR_PHYAD_MASK 0x1F000000 /* Phy Address Mask */ +#define XAE_MDIO_MCR_PHYAD_SHIFT 24 /* Phy Address Shift */ +#define XAE_MDIO_MCR_REGAD_MASK 0x001F0000 /* Reg Address Mask */ +#define XAE_MDIO_MCR_REGAD_SHIFT 16 /* Reg Address Shift */ +#define XAE_MDIO_MCR_OP_READ_MASK 0x00008000 /* Op Code Read Mask */ +#define XAE_MDIO_MCR_OP_WRITE_MASK 0x00004000 /* Op Code Write Mask */ +#define XAE_MDIO_MCR_INITIATE_MASK 0x00000800 /* Ready Mask */ +#define XAE_MDIO_MCR_READY_MASK 0x00000080 /* Ready Mask */ + +#define XAE_MDIO_DIV_DFT 29 /* Default MDIO clock divisor */ + +/* DMA macros */ +/* Bitmasks of XAXIDMA_CR_OFFSET register */ +#define XAXIDMA_CR_RUNSTOP_MASK 0x00000001 /* Start/stop DMA channel */ +#define XAXIDMA_CR_RESET_MASK 0x00000004 /* Reset DMA engine */ + +/* Bitmasks of XAXIDMA_SR_OFFSET register */ +#define XAXIDMA_HALTED_MASK 0x00000001 /* DMA channel halted */ + +/* Bitmask for interrupts */ +#define XAXIDMA_IRQ_IOC_MASK 0x00001000 /* Completion intr */ +#define XAXIDMA_IRQ_DELAY_MASK 0x00002000 /* Delay interrupt */ +#define XAXIDMA_IRQ_ALL_MASK 0x00007000 /* All interrupts */ + +/* Bitmasks of XAXIDMA_BD_CTRL_OFFSET register */ +#define XAXIDMA_BD_CTRL_TXSOF_MASK 0x08000000 /* First tx packet */ +#define XAXIDMA_BD_CTRL_TXEOF_MASK 0x04000000 /* Last tx packet */ + +#define DMAALIGN 128 + +static u8 rxframe[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); + +/* reflect dma offsets */ +struct axidma_reg { + u32 control; /* DMACR */ + u32 status; /* DMASR */ + u32 current; /* CURDESC */ + u32 reserved; + u32 tail; /* TAILDESC */ +}; + +/* Private driver structures */ +struct axidma_priv { + struct axidma_reg *dmatx; + struct axidma_reg *dmarx; + int phyaddr; + + struct phy_device *phydev; + struct mii_dev *bus; +}; + +/* BD descriptors */ +struct axidma_bd { + u32 next; /* Next descriptor pointer */ + u32 reserved1; + u32 phys; /* Buffer address */ + u32 reserved2; + u32 reserved3; + u32 reserved4; + u32 cntrl; /* Control */ + u32 status; /* Status */ + u32 app0; + u32 app1; /* TX start << 16 | insert */ + u32 app2; /* TX csum seed */ + u32 app3; + u32 app4; + u32 sw_id_offset; + u32 reserved5; + u32 reserved6; +}; + +/* Static BDs - driver uses only one BD */ +static struct axidma_bd tx_bd __attribute((aligned(DMAALIGN))); +static struct axidma_bd rx_bd __attribute((aligned(DMAALIGN))); + +struct axi_regs { + u32 reserved[3]; + u32 is; /* 0xC: Interrupt status */ + u32 reserved2; + u32 ie; /* 0x14: Interrupt enable */ + u32 reserved3[251]; + u32 rcw1; /* 0x404: Rx Configuration Word 1 */ + u32 tc; /* 0x408: Tx Configuration */ + u32 reserved4; + u32 emmc; /* 0x410: EMAC mode configuration */ + u32 reserved5[59]; + u32 mdio_mc; /* 0x500: MII Management Config */ + u32 mdio_mcr; /* 0x504: MII Management Control */ + u32 mdio_mwd; /* 0x508: MII Management Write Data */ + u32 mdio_mrd; /* 0x50C: MII Management Read Data */ + u32 reserved6[124]; + u32 uaw0; /* 0x700: Unicast address word 0 */ + u32 uaw1; /* 0x704: Unicast address word 1 */ +}; + +/* Use MII register 1 (MII status register) to detect PHY */ +#define PHY_DETECT_REG 1 + +/* Mask used to verify certain PHY features (or register contents) + * in the register above: + * 0x1000: 10Mbps full duplex support + * 0x0800: 10Mbps half duplex support + * 0x0008: Auto-negotiation support + */ +#define PHY_DETECT_MASK 0x1808 + +static inline int mdio_wait(struct eth_device *dev) +{ + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + + u32 timeout = 200; + /* Wait till MDIO interface is ready to accept a new transaction. */ + while (timeout && (!(in_be32(®s->mdio_mcr) + & XAE_MDIO_MCR_READY_MASK))) + timeout--; + + if (!timeout) { + printf("%s: Timeout\n", __func__); + return 1; + } + return 0; +} + +static u16 phyread(struct eth_device *dev, u32 phyaddress, u32 registernum) +{ + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + u32 mdioctrlreg = 0; + + if (mdio_wait(dev)) + return 0; + + mdioctrlreg = ((phyaddress << XAE_MDIO_MCR_PHYAD_SHIFT) & + XAE_MDIO_MCR_PHYAD_MASK) | + ((registernum << XAE_MDIO_MCR_REGAD_SHIFT) + & XAE_MDIO_MCR_REGAD_MASK) | + XAE_MDIO_MCR_INITIATE_MASK | + XAE_MDIO_MCR_OP_READ_MASK; + + out_be32(®s->mdio_mcr, mdioctrlreg); + + if (mdio_wait(dev)) + return 0; + + /* Read data */ + return in_be32(®s->mdio_mrd); +} + +static void phywrite(struct eth_device *dev, u32 phyaddress, u32 registernum, + u32 data) +{ + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + u32 mdioctrlreg = 0; + + if (mdio_wait(dev)) + return; + + mdioctrlreg = ((phyaddress << XAE_MDIO_MCR_PHYAD_SHIFT) & + XAE_MDIO_MCR_PHYAD_MASK) | + ((registernum << XAE_MDIO_MCR_REGAD_SHIFT) + & XAE_MDIO_MCR_REGAD_MASK) | + XAE_MDIO_MCR_INITIATE_MASK | + XAE_MDIO_MCR_OP_WRITE_MASK; + + /* Write data */ + out_be32(®s->mdio_mwd, data); + + out_be32(®s->mdio_mcr, mdioctrlreg); + + mdio_wait(dev); +} + + +/* setting axi emac and phy to proper setting */ +static int setup_phy(struct eth_device *dev) +{ + int i; + unsigned int speed; + u16 phyreg; + u32 emmc_reg; + struct axidma_priv *priv = dev->priv; + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + struct phy_device *phydev; + + u32 supported = SUPPORTED_10baseT_Half | + SUPPORTED_10baseT_Full | + SUPPORTED_100baseT_Half | + SUPPORTED_100baseT_Full | + SUPPORTED_1000baseT_Half | + SUPPORTED_1000baseT_Full; + + if (priv->phyaddr == -1) { + /* detect the PHY address */ + for (i = 31; i >= 0; i--) { + phyreg = phyread(dev, i, PHY_DETECT_REG); + if ((phyreg != 0xFFFF) && + ((phyreg & PHY_DETECT_MASK) == PHY_DETECT_MASK)) { + /* Found a valid PHY address */ + priv->phyaddr = i; + debug("axiemac: Found valid phy address, %d\n", + phyreg); + break; + } + } + } + + /* interface - look at tsec */ + phydev = phy_connect(priv->bus, priv->phyaddr, dev, 0); + + phydev->supported &= supported; + phydev->advertising = phydev->supported; + priv->phydev = phydev; + phy_config(phydev); + phy_startup(phydev); + + switch (phydev->speed) { + case 1000: + speed = XAE_EMMC_LINKSPD_1000; + break; + case 100: + speed = XAE_EMMC_LINKSPD_100; + break; + case 10: + speed = XAE_EMMC_LINKSPD_10; + break; + default: + return 0; + } + + /* Setup the emac for the phy speed */ + emmc_reg = in_be32(®s->emmc); + emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK; + emmc_reg |= speed; + + /* Write new speed setting out to Axi Ethernet */ + out_be32(®s->emmc, emmc_reg); + + /* + * Setting the operating speed of the MAC needs a delay. There + * doesn't seem to be register to poll, so please consider this + * during your application design. + */ + udelay(1); + + return 1; +} + +/* STOP DMA transfers */ +static void axiemac_halt(struct eth_device *dev) +{ + struct axidma_priv *priv = dev->priv; + + /* Stop the hardware */ + priv->dmatx->control &= ~XAXIDMA_CR_RUNSTOP_MASK; + priv->dmarx->control &= ~XAXIDMA_CR_RUNSTOP_MASK; + + debug("axiemac: Halted\n"); +} + +static int axi_ethernet_init(struct eth_device *dev) +{ + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + /* + * Check the status of the MgtRdy bit in the interrupt status + * registers. This must be done to allow the MGT clock to become stable + * for the Sgmii and 1000BaseX PHY interfaces. No other register reads + * will be valid until this bit is valid. + * The bit is always a 1 for all other PHY interfaces. + */ + u32 timeout = 200; + while (timeout && (!(in_be32(®s->is) & XAE_INT_MGTRDY_MASK))) + timeout--; + + if (!timeout) { + printf("%s: Timeout\n", __func__); + return 1; + } + + /* Stop the device and reset HW */ + /* Disable interrupts */ + out_be32(®s->ie, 0); + + /* Disable the receiver */ + out_be32(®s->rcw1, in_be32(®s->rcw1) & ~XAE_RCW1_RX_MASK); + + /* + * Stopping the receiver in mid-packet causes a dropped packet + * indication from HW. Clear it. + */ + /* set the interrupt status register to clear the interrupt */ + out_be32(®s->is, XAE_INT_RXRJECT_MASK); + + /* Setup HW */ + /* Set default MDIO divisor */ + out_be32(®s->mdio_mc, XAE_MDIO_DIV_DFT | XAE_MDIO_MC_MDIOEN_MASK); + + debug("axiemac: InitHw done\n"); + return 0; +} + +static int axiemac_setup_mac(struct eth_device *dev) +{ + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + + /* Set the MAC address */ + int val = ((dev->enetaddr[3] << 24) | (dev->enetaddr[2] << 16) | + (dev->enetaddr[1] << 8) | (dev->enetaddr[0])); + out_be32(®s->uaw0, val); + + val = (dev->enetaddr[5] << 8) | dev->enetaddr[4] ; + val |= in_be32(®s->uaw1) & ~XAE_UAW1_UNICASTADDR_MASK; + out_be32(®s->uaw1, val); + return 0; +} + +/* Reset DMA engine */ +static void axi_dma_init(struct eth_device *dev) +{ + struct axidma_priv *priv = dev->priv; + + /* Reset the engine so the hardware starts from a known state */ + priv->dmatx->control = XAXIDMA_CR_RESET_MASK; + priv->dmarx->control = XAXIDMA_CR_RESET_MASK; + + /* At the initialization time, hardware should finish reset quickly */ + u32 timeout = 500; + while (timeout--) { + /* Check transmit/receive channel */ + /* Reset is done when the reset bit is low */ + if (!(priv->dmatx->control | priv->dmarx->control) + & XAXIDMA_CR_RESET_MASK) + break; + } + + if (!timeout) + printf("%s: Timeout\n", __func__); +} + +static int axiemac_init(struct eth_device *dev, bd_t * bis) +{ + struct axidma_priv *priv = dev->priv; + struct axi_regs *regs = (struct axi_regs *)dev->iobase; + debug("axiemac: Init started\n"); + + /* + * Initialize AXIDMA engine. AXIDMA engine must be initialized before + * AxiEthernet. During AXIDMA engine initialization, AXIDMA hardware is + * reset, and since AXIDMA reset line is connected to AxiEthernet, this + * would ensure a reset of AxiEthernet. + */ + axi_dma_init(dev); + + /* Initialize AxiEthernet hardware. */ + if (axi_ethernet_init(dev)) + return -1; + + /* Start the hardware */ + priv->dmarx->control |= XAXIDMA_CR_RUNSTOP_MASK; + /* Start DMA RX channel. Now it's ready to receive data.*/ + priv->dmarx->current = (u32)&rx_bd; + + /* Disable all RX interrupts before RxBD space setup */ + priv->dmarx->control &= ~XAXIDMA_IRQ_ALL_MASK; + + /* Setup the BD. */ + memset(&rx_bd, 0, sizeof(rx_bd)); + rx_bd.next = (u32)&rx_bd; + rx_bd.phys = (u32)&rxframe; + rx_bd.cntrl = sizeof(rxframe); + /* Flush the last BD so DMA core could see the updates */ + flush_cache((u32)&rx_bd, sizeof(rx_bd)); + + /* it is necessary to flush rxframe because if you don't do it + * then cache can contain uninitialized data */ + flush_cache((u32)&rxframe, sizeof(rxframe)); + + /* Rx BD is ready - start */ + priv->dmarx->tail = (u32)&rx_bd; + + /* enable TX */ + out_be32(®s->tc, XAE_TC_TX_MASK); + /* enable RX */ + out_be32(®s->rcw1, XAE_RCW1_RX_MASK); + + /* PHY setup */ + if (!setup_phy(dev)) { + axiemac_halt(dev); + return -1; + } + + debug("axiemac: Init complete\n"); + return 0; +} + +static int axiemac_send(struct eth_device *dev, volatile void *ptr, int len) +{ + struct axidma_priv *priv = dev->priv; + + if (len > PKTSIZE_ALIGN) + len = PKTSIZE_ALIGN; + + /* Flush packet to main memory to be trasfered by DMA */ + flush_cache((u32)ptr, len); + + /* Setup Tx BD */ + memset(&tx_bd, 0, sizeof(tx_bd)); + /* At the end of the ring, link the last BD back to the top */ + tx_bd.next = (u32)&tx_bd; + tx_bd.phys = (u32)ptr; + /* save len */ + tx_bd.cntrl = len | XAXIDMA_BD_CTRL_TXSOF_MASK | + XAXIDMA_BD_CTRL_TXEOF_MASK; + + /* Flush the last BD so DMA core could see the updates */ + flush_cache((u32)&tx_bd, sizeof(tx_bd)); + + if (priv->dmatx->status & XAXIDMA_HALTED_MASK) { + priv->dmatx->current = (u32)&tx_bd; + /* Start the hardware */ + priv->dmatx->control |= XAXIDMA_CR_RUNSTOP_MASK; + } + + /* start transfer */ + priv->dmatx->tail = (u32)&tx_bd; + + /* Wait for transmission to complete */ + debug("axiemac: Waiting for tx to be done\n"); + while (!priv->dmatx->status & + (XAXIDMA_IRQ_DELAY_MASK | XAXIDMA_IRQ_IOC_MASK)) + ; + + debug("axiemac: Sending complete\n"); + return 0; +} + +static int IsRxReady(struct eth_device *dev) +{ + u32 status; + struct axidma_priv *priv = dev->priv; + + /* Read pending interrupts */ + status = priv->dmarx->status; + + /* Acknowledge pending interrupts */ + priv->dmarx->status &= XAXIDMA_IRQ_ALL_MASK; + + /* + * If Reception done interrupt is asserted, call RX call back function + * to handle the processed BDs and then raise the according flag. + */ + if ((status & (XAXIDMA_IRQ_DELAY_MASK | XAXIDMA_IRQ_IOC_MASK))) + return 1; + + return 0; +} + +static int axiemac_recv(struct eth_device *dev) +{ + u32 length; + struct axidma_priv *priv = dev->priv; + + /* wait for an incoming packet */ + if (!IsRxReady(dev)) + return 0; + + debug("axiemac: RX data ready\n"); + + /* Disable IRQ for a moment till packet is handled */ + priv->dmarx->control &= ~XAXIDMA_IRQ_ALL_MASK; + + length = rx_bd.app4 & 0xFFFF; /* max length mask */ +#ifdef DEBUG + print_buffer(&rxframe, &rxframe[0], 1, length, 16); +#endif + /* pass the received frame up for processing */ + if (length) + NetReceive(rxframe, length); + +#ifdef DEBUG + /* It is useful to clear buffer to be sure that it is consistent */ + memset(rxframe, 0, sizeof(rxframe)); +#endif + /* Setup RxBD */ + /* Clear the whole buffer and setup it again - all flags are cleared */ + memset(&rx_bd, 0, sizeof(rx_bd)); + rx_bd.next = (u32)&rx_bd; + rx_bd.phys = (u32)&rxframe; + rx_bd.cntrl = sizeof(rxframe); + + /* write bd to HW */ + flush_cache((u32)&rx_bd, sizeof(rx_bd)); + + /* it is necessary to flush rxframe because if you don't do it + * then cache will contain previous packet */ + flush_cache((u32)&rxframe, sizeof(rxframe)); + + /* Rx BD is ready - start again */ + priv->dmarx->tail = (u32)&rx_bd; + + debug("axiemac: RX completed, framelength = %d\n", length); + + return length; +} + +static int axiemac_miiphy_read(const char *devname, uchar addr, + uchar reg, ushort *val) +{ + struct eth_device *dev = eth_get_dev(); + *val = phyread(dev, addr, reg); + debug("axiemac: Read MII 0x%x, 0x%x, 0x%x\n", addr, reg, *val); + return 0; +} + +static int axiemac_miiphy_write(const char *devname, uchar addr, + uchar reg, ushort val) +{ + struct eth_device *dev = eth_get_dev(); + debug("axiemac: Write MII 0x%x, 0x%x, 0x%x\n", addr, reg, val); + phywrite(dev, addr, reg, val); + return 0; +} + +static int axiemac_bus_reset(struct mii_dev *bus) +{ + debug("axiemac: Bus reset\n"); + return 0; +} + +int xilinx_axiemac_initialize(bd_t *bis, unsigned long base_addr, + unsigned long dma_addr) +{ + struct eth_device *dev; + struct axidma_priv *priv; + + dev = calloc(1, sizeof(struct eth_device)); + if (dev == NULL) + return -1; + + dev->priv = calloc(1, sizeof(struct axidma_priv)); + if (dev->priv == NULL) { + free(dev); + return -1; + } + priv = dev->priv; + + sprintf(dev->name, "aximac.%lx", base_addr); + + dev->iobase = base_addr; + priv->dmatx = (struct axidma_reg *)dma_addr; + /* rx channel offset */ + priv->dmarx = (struct axidma_reg *)(dma_addr + 0x30); + dev->init = axiemac_init; + dev->halt = axiemac_halt; + dev->send = axiemac_send; + dev->recv = axiemac_recv; + dev->write_hwaddr = axiemac_setup_mac; + +#ifdef CONFIG_PHY_ADDR + priv->phyaddr = CONFIG_PHY_ADDR; +#else + priv->phyaddr = -1; +#endif + + eth_register(dev); + +#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII) || defined(CONFIG_PHYLIB) + miiphy_register(dev->name, axiemac_miiphy_read, axiemac_miiphy_write); + priv->bus = miiphy_get_dev_by_name(dev->name); + priv->bus->reset = axiemac_bus_reset; +#endif + return 1; +} diff --git a/include/netdev.h b/include/netdev.h index e976659..3125b4a 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -90,6 +90,8 @@ int smc91111_initialize(u8 dev_num, int base_addr); int tsi108_eth_initialize(bd_t *bis); int uec_standard_init(bd_t *bis); int uli526x_initialize(bd_t *bis); +int xilinx_axiemac_initialize(bd_t *bis, unsigned long base_addr, + unsigned long dma_addr); int xilinx_emaclite_initialize(bd_t *bis, unsigned long base_addr, int txpp, int rxpp); int xilinx_ll_temac_initialize(bd_t *bis, unsigned long base_addr,
Dear Michal Simek,
In message 1314877154-14536-2-git-send-email-monstr@monstr.eu you wrote:
Add axi_ethernet driver for little-endian Microblaze.
RX/TX BDs and rxframe buffer are shared among all axi_ethernet MACs. Only one MAC can work in one time.
Signed-off-by: Michal Simek monstr@monstr.eu
+/* Mask used to verify certain PHY features (or register contents)
- in the register above:
- 0x1000: 10Mbps full duplex support
- 0x0800: 10Mbps half duplex support
- 0x0008: Auto-negotiation support
- */
Incorrect multiline comment style. Please fix globally.
- u32 timeout = 200;
- /* Wait till MDIO interface is ready to accept a new transaction. */
- while (timeout && (!(in_be32(®s->mdio_mcr)
& XAE_MDIO_MCR_READY_MASK)))timeout--;
Multiline statement - braches needed.
Also, add some udelay() or similar to have a defined length of the timeout. Please fix globally.
+/* STOP DMA transfers */ +static void axiemac_halt(struct eth_device *dev) +{
- struct axidma_priv *priv = dev->priv;
- /* Stop the hardware */
- priv->dmatx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
- priv->dmarx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
Please ALWAYS use I/O accessors to access device registers. Please fix globally.
+static int IsRxReady(struct eth_device *dev)
As requested before: Please get rid of these CamelCaps identifiers!!!
+static int axiemac_miiphy_read(const char *devname, uchar addr,
uchar reg, ushort *val)+{
- struct eth_device *dev = eth_get_dev();
- *val = phyread(dev, addr, reg);
- debug("axiemac: Read MII 0x%x, 0x%x, 0x%x\n", addr, reg, *val);
- return 0;
Please separate declarations and code by a single blank line. Please fix globally.
Why does this function return int when you uncondsitionally always return 0 only?
+}
+static int axiemac_miiphy_write(const char *devname, uchar addr,
uchar reg, ushort val)+{
- struct eth_device *dev = eth_get_dev();
- debug("axiemac: Write MII 0x%x, 0x%x, 0x%x\n", addr, reg, val);
- phywrite(dev, addr, reg, val);
- return 0;
+}
Ditto.
+static int axiemac_bus_reset(struct mii_dev *bus) +{
- debug("axiemac: Bus reset\n");
- return 0;
+}
Ditto.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Wolfgang Denk wrote:
Dear Michal Simek,
In message 1314877154-14536-2-git-send-email-monstr@monstr.eu you wrote:
Add axi_ethernet driver for little-endian Microblaze.
RX/TX BDs and rxframe buffer are shared among all axi_ethernet MACs. Only one MAC can work in one time.
Signed-off-by: Michal Simek monstr@monstr.eu
+/* Mask used to verify certain PHY features (or register contents)
- in the register above:
- 0x1000: 10Mbps full duplex support
- 0x0800: 10Mbps half duplex support
- 0x0008: Auto-negotiation support
- */
Incorrect multiline comment style. Please fix globally.
Fixed.
- u32 timeout = 200;
- /* Wait till MDIO interface is ready to accept a new transaction. */
- while (timeout && (!(in_be32(®s->mdio_mcr)
& XAE_MDIO_MCR_READY_MASK)))timeout--;Multiline statement - braches needed.
Also, add some udelay() or similar to have a defined length of the timeout. Please fix globally.
Fixed.
+/* STOP DMA transfers */ +static void axiemac_halt(struct eth_device *dev) +{
- struct axidma_priv *priv = dev->priv;
- /* Stop the hardware */
- priv->dmatx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
- priv->dmarx->control &= ~XAXIDMA_CR_RUNSTOP_MASK;
Please ALWAYS use I/O accessors to access device registers. Please fix globally.
Done
+static int IsRxReady(struct eth_device *dev)
As requested before: Please get rid of these CamelCaps identifiers!!!
Fixed this.
+static int axiemac_miiphy_read(const char *devname, uchar addr,
uchar reg, ushort *val)+{
- struct eth_device *dev = eth_get_dev();
- *val = phyread(dev, addr, reg);
- debug("axiemac: Read MII 0x%x, 0x%x, 0x%x\n", addr, reg, *val);
- return 0;
Please separate declarations and code by a single blank line. Please fix globally.
Done
Why does this function return int when you uncondsitionally always return 0 only?
I have fixed phywrite/phyread functions.
+}
+static int axiemac_miiphy_write(const char *devname, uchar addr,
uchar reg, ushort val)+{
- struct eth_device *dev = eth_get_dev();
- debug("axiemac: Write MII 0x%x, 0x%x, 0x%x\n", addr, reg, val);
- phywrite(dev, addr, reg, val);
- return 0;
+}
Ditto.
+static int axiemac_bus_reset(struct mii_dev *bus) +{
- debug("axiemac: Bus reset\n");
- return 0;
+}
Ditto.
Because of miiphyutil code requires to return 0 and I am not aware about doing anything useful for bus_reset.
Regards, Michal
Dear Michal Simek,
In message 1314877154-14536-1-git-send-email-monstr@monstr.eu you wrote:
LL Temac driver can be used by microblaze, xilinx ppc405/440 in sdma and fifo mode. DCR or XPS bus can be used.
The driver uses and requires PHYLIB.
+static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{
- if (priv->mode & DCR_BIT)
mtdcr_local(priv->ctrl + offset, val);- else
out_be32((u32 *)(priv->ctrl + offset * 4), val);+}
+static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) +{
- if (priv->mode & DCR_BIT)
return mfdcr_local(priv->ctrl + offset);- return in_be32((u32 *)(priv->ctrl + offset * 4));
+}
Can we please get rid of these functions? As mentioned many, many times before, we discourage all use of "base address plus offset" to access any device registers etc.
These functions here re-introduce such accesses, and this is something I will not accept.
- /* Soft reset the DMA. */
- sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET);
- while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET)
;
I think this has been pointed out before: we do not accept such potentially endless loops. Please fix this (and integrate theother previously made review remarks). Thanks.
- do {
flush_cache((u32)&tx_bd, sizeof(tx_bd));- } while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK));
Ditto. Please fix globally.
- /* Read out the packet info and start the DMA
onto the second buffer to enable the ethernet rxpath to run in parallel with sw processingpackets. */
Incorrect multiline comment style; please fix globally.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Michal Simek,
In message 1314877154-14536-1-git-send-email-monstr@monstr.eu you wrote:
LL Temac driver can be used by microblaze, xilinx ppc405/440 in sdma and fifo mode. DCR or XPS bus can be used.
The driver uses and requires PHYLIB.
+static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{
- if (priv->mode & DCR_BIT)
mtdcr_local(priv->ctrl + offset, val);- else
out_be32((u32 *)(priv->ctrl + offset * 4), val);+}
+static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) +{
- if (priv->mode & DCR_BIT)
return mfdcr_local(priv->ctrl + offset);- return in_be32((u32 *)(priv->ctrl + offset * 4));
+}
Can we please get rid of these functions? As mentioned many, many times before, we discourage all use of "base address plus offset" to access any device registers etc.
These functions here re-introduce such accesses, and this is something I will not accept.
Ok. How to do it?
For bus access it is necessary to use 4B offsets for DCR just 1B and one system can contains two MACs where the first use 4B offset and the second 1B.
- /* Soft reset the DMA. */
- sdma_out_be32(priv, DMA_CONTROL_REG, DMA_CONTROL_RESET);
- while (sdma_in_be32(priv, DMA_CONTROL_REG) & DMA_CONTROL_RESET)
;I think this has been pointed out before: we do not accept such potentially endless loops. Please fix this (and integrate theother previously made review remarks). Thanks.
- do {
flush_cache((u32)&tx_bd, sizeof(tx_bd));- } while (!(tx_bd.stat & BDSTAT_COMPLETED_MASK));
Ditto. Please fix globally.
I forget. Sorry. Will fix it.
- /* Read out the packet info and start the DMA
onto the second buffer to enable the ethernet rxpath to run in parallel with sw processingpackets. */Incorrect multiline comment style; please fix globally.
Fixed and checked.
Thanks, Michal
Dear Michal Simek,
In message 4E5F85DA.4080403@monstr.eu you wrote:
+static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{
- if (priv->mode & DCR_BIT)
mtdcr_local(priv->ctrl + offset, val);- else
out_be32((u32 *)(priv->ctrl + offset * 4), val);+}
+static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) +{
- if (priv->mode & DCR_BIT)
return mfdcr_local(priv->ctrl + offset);- return in_be32((u32 *)(priv->ctrl + offset * 4));
+}
Can we please get rid of these functions? As mentioned many, many times before, we discourage all use of "base address plus offset" to access any device registers etc.
These functions here re-introduce such accesses, and this is something I will not accept.
Ok. How to do it?
For bus access it is necessary to use 4B offsets for DCR just 1B and one system can contains two MACs where the first use 4B offset and the second 1B.
I don't think your description here matches the code above. With "1 byte offsets" you would be doing all unaligned bus accesses.
I think you should use an array of structs and an index to select the right one.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Wolfgang Denk wrote:
Dear Michal Simek,
In message 4E5F85DA.4080403@monstr.eu you wrote:
+static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{
- if (priv->mode & DCR_BIT)
mtdcr_local(priv->ctrl + offset, val);- else
out_be32((u32 *)(priv->ctrl + offset * 4), val);+}
+static u32 sdma_in_be32(struct ll_priv *priv, u32 offset) +{
- if (priv->mode & DCR_BIT)
return mfdcr_local(priv->ctrl + offset);- return in_be32((u32 *)(priv->ctrl + offset * 4));
+}
Can we please get rid of these functions? As mentioned many, many times before, we discourage all use of "base address plus offset" to access any device registers etc.
These functions here re-introduce such accesses, and this is something I will not accept.
Ok. How to do it?
For bus access it is necessary to use 4B offsets for DCR just 1B and one system can contains two MACs where the first use 4B offset and the second 1B.
I don't think your description here matches the code above. With "1 byte offsets" you would be doing all unaligned bus accesses.
Let me show it on example. on MB or PPC system with bus: DMA is in memory controller on 32bit address + sdma DMA port offset.
DMA : Sdma offset(www.xilinx.com/support/documentation/ip_documentation/mpmc.pdf page 54 and 55) 0 : 0x0 1 : 0x80 2 : 0x100 3 : 0x180 ... 7 : 0x380
Let me assume that MPMC is at 0x12340000 The first reg for DMA2 accessed through bus is at 0x12340100 offset the second at 0x12340104, the third 0x12340108 etc.
On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done with mfdcr_local and mtdcr_local functions.
DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299) 0 : 0x80-0x90 1 : 0x98-0xA8 2 : 0xB0-0xC0 3 : 0xC8-0xD8
The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc..
Regards, Michal
Dear Michal Simek,
In message 4E607A0B.3040608@monstr.eu you wrote:
+static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{
- if (priv->mode & DCR_BIT)
mtdcr_local(priv->ctrl + offset, val);- else
out_be32((u32 *)(priv->ctrl + offset * 4), val);+}
...
On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done with mfdcr_local and mtdcr_local functions.
DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299) 0 : 0x80-0x90 1 : 0x98-0xA8 2 : 0xB0-0xC0 3 : 0xC8-0xD8
The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc..
This is indeed a good example, as it shows how terribly broken your code is.
See function sdma_out_be32() above. It is suppose to write a 32 bit value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to some device register - but the register addresses are (1) not aligned to 32 bit boundaries and (2) not even 32 bits apart.
Oh! You are NOT writing 32 bit data? You are writing only 8 bit? But the function claims to be writing 32 bit!!!
And your function mtdcr_local() is actually a NOOP for anything but PPC? Without even a warning?
This code is obfuscated in multiple levels.
I will not accept that.
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Michal Simek,
In message 4E607A0B.3040608@monstr.eu you wrote:
+static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) +{
- if (priv->mode & DCR_BIT)
mtdcr_local(priv->ctrl + offset, val);- else
out_be32((u32 *)(priv->ctrl + offset * 4), val);+}
...
On PPC system with DCR is special connection between memory controller through DCR bus. Handling is done with mfdcr_local and mtdcr_local functions.
DMA : Sdma address ranges (www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 and 299) 0 : 0x80-0x90 1 : 0x98-0xA8 2 : 0xB0-0xC0 3 : 0xC8-0xD8
The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc..
This is indeed a good example, as it shows how terribly broken your code is.
See function sdma_out_be32() above. It is suppose to write a 32 bit value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to some device register - but the register addresses are (1) not aligned to 32 bit boundaries and (2) not even 32 bits apart.
I think you misunderstand what there is written. For DCR is register address with offset + 0x1 but still you are writing there 32bit values. It is not direct access. and it is not any unaligned access at all. But bus access it will be unaligned access but not for DCR.
DCR is defined just for PPC right now because none wanted to do it for Microblaze.
Regards, Michal
Dear Michal Simek,
In message 4E609682.8030701@monstr.eu you wrote:
> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) > +{ > + if (priv->mode & DCR_BIT) > + mtdcr_local(priv->ctrl + offset, val); > + else > + out_be32((u32 *)(priv->ctrl + offset * 4), val); > +}
...
...
The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc..
This is indeed a good example, as it shows how terribly broken your code is.
See function sdma_out_be32() above. It is suppose to write a 32 bit value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to some device register - but the register addresses are (1) not aligned to 32 bit boundaries and (2) not even 32 bits apart.
I think you misunderstand what there is written.
Maybe. Maybe even I want to misunderstand it. The problem is that the code does not prevent such misunderstanding.
There are many shortcomings of that code.
DCR is defined just for PPC right now because none wanted to do it for Microblaze.
Actually even this is incorrect - AFAIK Device Control Registers (DCR) exist not on all PPC systems, but only on 4xx (and even there only on some models). So your code works on a few systems, silently does not do anything on others, and crashes on yet others with an illegal instruction.
How do you call code that exposes such behaviour?
I don't want to have this in mainline.
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
Wolfgang Denk wrote:
Dear Michal Simek,
In message 4E609682.8030701@monstr.eu you wrote:
>> +static void sdma_out_be32(struct ll_priv *priv, u32 offset, u32 val) >> +{ >> + if (priv->mode & DCR_BIT) >> + mtdcr_local(priv->ctrl + offset, val); >> + else >> + out_be32((u32 *)(priv->ctrl + offset * 4), val); >> +}
...
...
The first reg for DMA2 accessed trough DCR is at 0xB0, the second at 0xB1, etc..
This is indeed a good example, as it shows how terribly broken your code is.
See function sdma_out_be32() above. It is suppose to write a 32 bit value ("u32 val") as a 32 bit entity in big endian mode ("_be32") to some device register - but the register addresses are (1) not aligned to 32 bit boundaries and (2) not even 32 bits apart.
I think you misunderstand what there is written.
Maybe. Maybe even I want to misunderstand it. The problem is that the code does not prevent such misunderstanding.
There are many shortcomings of that code.
I think that this is the reason why we have review process, don't we?
Especially this driver is 2-3 years old and it is used by many our customers. It is only my effort to clear xilinx drivers/platforms.
As I see there is still ugly board/xilinx/common folder and ancient enet driver and i2c driver.
DCR is defined just for PPC right now because none wanted to do it for Microblaze.
Actually even this is incorrect - AFAIK Device Control Registers (DCR) exist not on all PPC systems, but only on 4xx (and even there only on some models). So your code works on a few systems, silently does not do anything on others, and crashes on yet others with an illegal instruction.
That driver is not definitely for all ppc systems. That IP is available just for Xilinx FPGA where can be possible to use it with Microblaze and xilinx ppc440 (maybe ppc405). That DCR handling, which is implemented in this driver, fits to xilinx ppc440 implementation. Which means that none can add this IP to any other PPC system out of Xilinx FPGA.
How do you call code that exposes such behaviour?
If I look at drivers/net folder there are a lot of examples like that. None expect that altera_tse/bfin_mac/tsec will be possible to use with all systems. Maybe you expect that they can be use on the same architecture but this is Xilinx FPGA. It is up to you how you want to compose your system.
IRC tsec is used just for Freescale PPC and this ll_temac driver is just used for xilinx microblaze and xilinx ppc.
Sorry I can't see any problem to have driver for specific platform or even to have one driver which supports two completely different platform.
I saw that there are some drivers in arch/<cpu>/ folders but this is not that case because can be possible to use it for microblaze and xilinx ppc.
This ll_temac driver is just another net IP like emaclite is. Emaclite driver is possible to use on Microblaze and xilinx ppc systems and in near future with arm on Xilinx zynq platform.
I don't want to have this in mainline.
If this is your decision, I won't send any updated version.
Regards, Michal
Dear Michal Simek,
In message 4E60B220.6010309@monstr.eu you wrote:
As I see there is still ugly board/xilinx/common folder and ancient enet driver and i2c driver.
Indeed, and improvementrs are more than welcome.
Actually even this is incorrect - AFAIK Device Control Registers (DCR) exist not on all PPC systems, but only on 4xx (and even there only on some models). So your code works on a few systems, silently does not do anything on others, and crashes on yet others with an illegal instruction.
That driver is not definitely for all ppc systems. That IP is available just for Xilinx FPGA where can be possible to use it with Microblaze and xilinx ppc440 (maybe ppc405). That DCR handling, which is implemented in this driver, fits to xilinx ppc440 implementation. Which means that none can add this IP to any other PPC system out of Xilinx FPGA.
So why not use something like CONFIG_440 in this test, and add an #error for anything else?
Do we actually need this m{f,t}dcr_local() then?
Sorry I can't see any problem to have driver for specific platform or even to have one driver which supports two completely different platform.
My issue is that this code silently breaks or crashes when certain (undocumented) conditions are not met. Preventing this seems not to bee too difficult: add a comment, make it depend on the right CONFIG_ settings, and bail out with a clear error message when conditions are not met.
As for the other part of the problem - you try to mix two different cases: one where you refer to an index, and one where you refer to an address. This obviously doesn't mix well. If there is no better way of doing this, I'd still prefer deriving the index from the offset in a struct than deriving the address from an offset - the intention is to enable the compiler to perform type checking, which is impossible with a typeless base+offset address.
I don't want to have this in mainline.
If this is your decision, I won't send any updated version.
Attempted extortion?
Best regards,
Wolfgang Denk
Dear Wolfgang Denk,
In message 4E60B220.6010309@monstr.eu you wrote:
As I see there is still ugly board/xilinx/common folder and ancient enet driver and i2c driver.
Indeed, and improvementrs are more than welcome.
Actually even this is incorrect - AFAIK Device Control Registers (DCR) exist not on all PPC systems, but only on 4xx (and even there only on some models). So your code works on a few systems, silently does not do anything on others, and crashes on yet others with an illegal instruction.
That driver is not definitely for all ppc systems. That IP is available just for Xilinx FPGA where can be possible to use it with Microblaze and xilinx ppc440 (maybe ppc405). That DCR handling, which is implemented in this driver, fits to xilinx ppc440 implementation. Which means that none can add this IP to any other PPC system out of Xilinx FPGA.
So why not use something like CONFIG_440 in this test, and add an #error for anything else?
Do we actually need this m{f,t}dcr_local() then?
DCR handling is specific for Xilinx ppc440 which means that not all PPC440 can use it. As you see m{f,t}dcr_local setup handling for it that's why it is neeeded.
Sorry I can't see any problem to have driver for specific platform or even to have one driver which supports two completely different platform.
My issue is that this code silently breaks or crashes when certain (undocumented) conditions are not met. Preventing this seems not to bee too difficult: add a comment, make it depend on the right CONFIG_ settings, and bail out with a clear error message when conditions are not met.
Driver is protected by CONFIG_XILINX_LL_TEMAC option which means that any platform is not silently breaks. You can use it with Microblaze and PPC and configuration is done (xparameters.h and config.mk files) by u-boot BSP in connection to Xilinx EDK which check if DCR can be used or not.
As for the other part of the problem - you try to mix two different cases: one where you refer to an index, and one where you refer to an address.
In technical sense it is still address not index. It is different addressing mode.
I have done it because it is much better than a lot of ifdefs. It is more than that because ppc has dcr up to 4 DMAs but memory controller supports up to 8 DMAs that's why I think that it is better to support both modes and decide by configuration.
This obviously doesn't mix well. If there is no better way
of doing this, I'd still prefer deriving the index from the offset in a struct than deriving the address from an offset - the intention is to enable the compiler to perform type checking, which is impossible with a typeless base+offset address.
I understand the reasons for that but I can't see any elegant way how to do so.
I don't want to have this in mainline.
If this is your decision, I won't send any updated version.
Attempted extortion?
My god why do you think that it is extortion? If you don't want to add it to mainline because you think that this driver is bad/broken/anything, I can do nothing with it and make no sense for me to invest my time to it. And I am not going to disturb others with it.
Regards, Michal
Dear Michal Simek,
In message 4E60DA47.4070301@monstr.eu you wrote:
That driver is not definitely for all ppc systems. That IP is available just for Xilinx FPGA where can be possible to use it with Microblaze and xilinx ppc440 (maybe ppc405). That DCR handling, which is implemented in this driver, fits to xilinx ppc440 implementation. Which means that none can add this IP to any other PPC system out of Xilinx FPGA.
So why not use something like CONFIG_440 in this test, and add an #error for anything else?
You did not answer this - why not using CONFIG_440 instead of CONFIG_PPC ?
Do we actually need this m{f,t}dcr_local() then?
DCR handling is specific for Xilinx ppc440 which means that not all PPC440 can use it. As you see m{f,t}dcr_local setup handling for it that's why it is neeeded.
Then maybe you should chose a better name for it, say m?dcr_xilinx() or so.
My issue is that this code silently breaks or crashes when certain (undocumented) conditions are not met. Preventing this seems not to bee too difficult: add a comment, make it depend on the right CONFIG_ settings, and bail out with a clear error message when conditions are not met.
Driver is protected by CONFIG_XILINX_LL_TEMAC option which means that any platform is not silently breaks. You can use it with Microblaze and PPC and configuration is done (xparameters.h and config.mk files) by u-boot BSP in connection to Xilinx EDK which check if DCR can be used or not.
I can only offer you a solution that seems acceptable to me.
As for the other part of the problem - you try to mix two different cases: one where you refer to an index, and one where you refer to an address.
In technical sense it is still address not index. It is different addressing mode.
The Processor Core User's Manual says for example:
The DCR number (DCRN) is specified by the Device Control Register Immediate Prefix Register (DCRIPR) and the DCRF field of the mfdcr instruction.
To me, "number" translates much better into index than into address.
Also, the DCR number are incremented by 1 - if these were addresses in the common sense, they could only point to 8 bit entities - but the registers are actually 32 bit wide.
But again, I can only show you what I think could be an acceptable approach. If you don't like it, please feel free to develop a better one.
In any case, I will not accept the current (V3) code.
This obviously doesn't mix well. If there is no better way
of doing this, I'd still prefer deriving the index from the offset in a struct than deriving the address from an offset - the intention is to enable the compiler to perform type checking, which is impossible with a typeless base+offset address.
I understand the reasons for that but I can't see any elegant way how to do so.
Well, I just proposed one way - not elegant indeed, but I'd be willing to swallow that.
If you don't want to add it to mainline because you think that this driver is bad/broken/anything, I can do nothing with it and make no sense for me to invest my time to it.
I only complain about a few details of the implementation, and I even lend you a helping hand by offering specific solutions. Feel free to take or to refuse it. I got better things to do myself, either.
Wolfgang Denk
Dear Wolfgang Denk,
In message 4E60DA47.4070301@monstr.eu you wrote:
That driver is not definitely for all ppc systems. That IP is available just for Xilinx FPGA where can be possible to use it with Microblaze and xilinx ppc440 (maybe ppc405). That DCR handling, which is implemented in this driver, fits to xilinx ppc440 implementation. Which means that none can add this IP to any other PPC system out of Xilinx FPGA.
So why not use something like CONFIG_440 in this test, and add an #error for anything else?
You did not answer this - why not using CONFIG_440 instead of CONFIG_PPC ?
Probably the best is CONFIG_XILINX_440.
Do we actually need this m{f,t}dcr_local() then?
DCR handling is specific for Xilinx ppc440 which means that not all PPC440 can use it. As you see m{f,t}dcr_local setup handling for it that's why it is neeeded.
Then maybe you should chose a better name for it, say m?dcr_xilinx() or so.
np.
My issue is that this code silently breaks or crashes when certain (undocumented) conditions are not met. Preventing this seems not to bee too difficult: add a comment, make it depend on the right CONFIG_ settings, and bail out with a clear error message when conditions are not met.
Driver is protected by CONFIG_XILINX_LL_TEMAC option which means that any platform is not silently breaks. You can use it with Microblaze and PPC and configuration is done (xparameters.h and config.mk files) by u-boot BSP in connection to Xilinx EDK which check if DCR can be used or not.
I can only offer you a solution that seems acceptable to me.
seems?
As for the other part of the problem - you try to mix two different cases: one where you refer to an index, and one where you refer to an address.
In technical sense it is still address not index. It is different addressing mode.
The Processor Core User's Manual says for example:
The DCR number (DCRN) is specified by the Device Control Register Immediate Prefix Register (DCRIPR) and the DCRF field of the mfdcr instruction.
To me, "number" translates much better into index than into address.
DCRN is number but for my case I use 0 for register "address" and 1 for value. To be accurate - 0 is indirect dcr address reg, and 1 is indirect dcr access reg. (Have changed that magic value to macros according technical documentation).
Also, the DCR number are incremented by 1 - if these were addresses in the common sense, they could only point to 8 bit entities - but the registers are actually 32 bit wide.
And agree that DCR number is incremented by 1.
On www.xilinx.com/support/documentation/user_guides/ug200.pdf page 261 is written that it is address that's why I use address.
But again, I can only show you what I think could be an acceptable approach. If you don't like it, please feel free to develop a better one.
In any case, I will not accept the current (V3) code.
sure.
This obviously doesn't mix well. If there is no better way
of doing this, I'd still prefer deriving the index from the offset in a struct than deriving the address from an offset - the intention is to enable the compiler to perform type checking, which is impossible with a typeless base+offset address.
I understand the reasons for that but I can't see any elegant way how to do so.
Well, I just proposed one way - not elegant indeed, but I'd be willing to swallow that.
You mean that array of structs, right?
If you don't want to add it to mainline because you think that this driver is bad/broken/anything, I can do nothing with it and make no sense for me to invest my time to it.
I only complain about a few details of the implementation, and I even lend you a helping hand by offering specific solutions. Feel free to take or to refuse it. I got better things to do myself, either.
Sure.
Regards, Michal
Dear Michal Simek,
In message 4E647722.9080602@monstr.eu you wrote:
I can only offer you a solution that seems acceptable to me.
seems?
Well, yes. As mentioned, I'm not happy with the propsal myself, but as long as I cannot come up with any better one it would be unjust to demand it from you.
This obviously doesn't mix well. If there is no better way
of doing this, I'd still prefer deriving the index from the offset in a struct than deriving the address from an offset - the intention is to enable the compiler to perform type checking, which is impossible with a typeless base+offset address.
I understand the reasons for that but I can't see any elegant way how to do so.
Well, I just proposed one way - not elegant indeed, but I'd be willing to swallow that.
You mean that array of structs, right?
No. I mean the text above: I'd rather derive the index from the offset in a struct instead of deriving the address from an offset (but I have to admit that I don't see an elegant way to implement this yet).
Best regards,
Wolfgang Denk
participants (2)
-
Michal Simek -
Wolfgang Denk