Dear Michal Simek,
[...]
> +static inline int mdio_wait(struct eth_device *dev)
> +{
> + struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> + u32 timeout = 200;
> +
> + /* Wait till MDIO interface is ready to accept a new transaction. */
> + while (timeout && !(readl(®s->nwsr) & ZYNQ_GEM_NWSR_MDIOIDLE_MASK)) {
I'd say, rework this to
while (--timeout) {
if (readl() & ... )
break;
WATCHDOG_RESET();
}
The WATCHDOG_RESET will give you the udelay and restart the WDT if you use any.
Also, I think it's more readable when you omit the complex condition for the
while cycle and split it a bit.
> + timeout--;
> + udelay(1);
> + }
> +
> + if (!timeout) {
> + printf("%s: Timeout\n", __func__);
> + return 1;
> + }
> +
> + return 0;
> +}
[...]
> +static int zynq_gem_init(struct eth_device *dev, bd_t * bis)
> +{
> + int tmp;
> + int i;
> + struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> + struct zynq_gem_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->initialized)
> + return 0;
> +
> + /* Disable all interrupts */
> + writel(0xFFFFFFFF, ®s->idr);
> +
> + /* Disable the receiver & transmitter */
> + writel(0, ®s->nwctrl);
> + writel(0, ®s->txsr);
> + writel(0, ®s->rxsr);
> + writel(0, ®s->phymntnc);
> +
> + /* Clear the Hash registers for the mac address pointed by AddressPtr */
> + writel(0x0, ®s->hashl);
> + /* Write bits [63:32] in TOP */
> + writel(0x0, ®s->hashh);
> +
> + /* Clear all counters */
> + for (i = 0; i <= (sizeof(struct zynq_gem_regs) -
> + offsetof(struct zynq_gem_regs, stat)) / 4; i++)
Add a const int variable and use it here so you don't have to break the for () .
> + readl(®s->stat[i]);
> +
> + /* Setup RxBD space */
> + memset(&(priv->rx_bd), 0, sizeof(priv->rx_bd));
> + /* Create the RxBD ring */
> + memset(&(priv->rxbuffers), 0, sizeof(priv->rxbuffers));
> +
> + for (i = 0; i < RX_BUF; i++) {
> + priv->rx_bd[i].status = 0xF0000000;
> + priv->rx_bd[i].addr = (u32)((char *) &(priv->rxbuffers) +
> + (i * PKTSIZE_ALIGN));
> + }
> + /* WRAP bit to last BD */
> + priv->rx_bd[--i].addr |= ZYNQ_GEM_RXBUF_WRAP_MASK;
> + /* Write RxBDs to IP */
> + writel((u32) &(priv->rx_bd), ®s->rxqbase);
> +
> +
> + /* MAC Setup */
> + /*
> + * Following is the setup for Network Configuration register.
> + * Bit 0: Set for 100 Mbps operation.
> + * Bit 1: Set for Full Duplex mode.
> + * Bit 4: Unset to allow Copy all frames - MAC checking
> + * Bit 17: Set for FCS removal.
> + * Bits 20-18: Set with value binary 010 to divide pclk by 32
> + * (pclk up to 80 MHz)
> + */
> + writel(0x000A0003, ®s->nwcfg);
Well you know ... magic numbers and defined bits ;-)
> + /*
> + * Following is the setup for DMA Configuration register.
> + * Bits 4-0: To set AHB fixed burst length for DMA data operations ->
> + * Set with binary 00100 to use INCR4 AHB bursts.
> + * Bits 9-8: Receiver packet buffer memory size ->
> + * Set with binary 11 to Use full configured addressable space (8 Kb)
> + * Bit 10 : Transmitter packet buffer memory size ->
> + * Set with binary 1 to Use full configured addressable space (4 Kb)
> + * Bits 23-16 : DMA receive buffer size in AHB system memory ->
> + * Set with binary 00011000 to use 1536 byte(1*max length frame/buffer)
> + */
> + writel(0x00180704, ®s->dmacr);
> +
> + /*
> + * Following is the setup for Network Control register.
> + * Bit 2: Set to enable Receive operation.
> + * Bit 3: Set to enable Transmitt operation.
> + * Bit 4: Set to enable MDIO operation.
> + */
> + tmp = readl(®s->nwctrl);
> + /* MDIO, Rx and Tx enable */
> + tmp |= ZYNQ_GEM_NWCTRL_MDEN_MASK | ZYNQ_GEM_NWCTRL_RXEN_MASK |
> + ZYNQ_GEM_NWCTRL_TXEN_MASK;
> + writel(tmp, ®s->nwctrl);
setbits_le32()
> + /* 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);
> +
> + priv->initialized = 1;
> + return 0;
> +}
> +
> +static int zynq_gem_send(struct eth_device *dev, void *ptr, int len)
> +{
> + int status;
> + u32 val;
> + struct zynq_gem_priv *priv = dev->priv;
> + struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase;
> +
> + if (!priv->initialized) {
> + puts("Error GMAC not initialized");
> + return -1;
> + }
> +
> + /* setup BD */
> + writel((u32)&(priv->tx_bd), ®s->txqbase);
> +
> + /* Setup Tx BD */
> + memset((void *) &(priv->tx_bd), 0, sizeof(struct emac_bd));
> +
> + priv->tx_bd.addr = (u32)ptr;
> + priv->tx_bd.status = len | ZYNQ_GEM_TXBUF_LAST_MASK |
> + ZYNQ_GEM_TXBUF_WRAP_MASK;
> +
> + /* Start transmit */
> + val = readl(®s->nwctrl) | ZYNQ_GEM_NWCTRL_STARTTX_MASK;
> + writel(val, ®s->nwctrl);
setbits_le32()
> + /* Read the stat register to know if the packet has been transmitted */
> + status = readl(®s->txsr);
> + if (status & (ZYNQ_GEM_TXSR_HRESPNOK_MASK | ZYNQ_GEM_TXSR_URUN_MASK |
> + ZYNQ_GEM_TXSR_BUFEXH_MASK)) {
Add const int mask for the above.
> + printf("Something has gone wrong here!? Status is 0x%x.\n",
> + status);
> + }
> +
> + /* Clear Tx status register before leaving . */
> + writel(status, ®s->txsr);
> + return 0;
> +}
> +
> +/* Do not check frame_recd flag in rx_status register 0x20 - just poll BD
> */ +static int zynq_gem_recv(struct eth_device *dev)
> +{
> + int frame_len;
> + struct zynq_gem_priv *priv = dev->priv;
> +
> + if (!(priv->rx_bd[priv->rxbd_current].addr & ZYNQ_GEM_RXBUF_NEW_MASK))
> + return 0;
> +
> + if (!(priv->rx_bd[priv->rxbd_current].status &
> + (ZYNQ_GEM_RXBUF_SOF_MASK | ZYNQ_GEM_RXBUF_EOF_MASK))) {
> + printf("GEM: SOF or EOF not set for last buffer received!\n");
> + return 0;
> + }
> +
> + frame_len = priv->rx_bd[priv->rxbd_current].status &
> + ZYNQ_GEM_RXBUF_LEN_MASK;
Just introduce some variable for priv->rx_bd[priv->rxbd_current] so you don't
have such long lines
> + if (frame_len) {
> + NetReceive((u8 *) (priv->rx_bd[priv->rxbd_current].addr &
> + ZYNQ_GEM_RXBUF_ADD_MASK), frame_len);
> +
> + if (priv->rx_bd[priv->rxbd_current].status &
> + ZYNQ_GEM_RXBUF_SOF_MASK)
> + priv->rx_first_buf = priv->rxbd_current;
> + else {
> + priv->rx_bd[priv->rxbd_current].addr &=
> + ~ZYNQ_GEM_RXBUF_NEW_MASK;
> + priv->rx_bd[priv->rxbd_current].status = 0xF0000000;
> + }
> +
> + if (priv->rx_bd[priv->rxbd_current].status &
> + ZYNQ_GEM_RXBUF_EOF_MASK) {
> + priv->rx_bd[priv->rx_first_buf].addr &=
> + ~ZYNQ_GEM_RXBUF_NEW_MASK;
> + priv->rx_bd[priv->rx_first_buf].status = 0xF0000000;
> + }
> +
> + if ((++priv->rxbd_current) >= RX_BUF)
> + priv->rxbd_current = 0;
> +
> + return frame_len;
> + }
> +
> + return 0;
> +}
> +
> +static void zynq_gem_halt(struct eth_device *dev)
> +{
> + return;
Does this need to be implemented at all ?
> +}
> +
> +static int zynq_gem_miiphyread(const char *devname, uchar addr,
> + uchar reg, ushort *val)
> +{
> + struct eth_device *dev = eth_get_dev();
> + int ret;
> +
> + ret = phyread(dev, addr, reg, val);
> + debug("%s 0x%x, 0x%x, 0x%x\n", __func__, addr, reg, *val);
> + return ret;
> +}
> +
> +static int zynq_gem_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);
> + return phywrite(dev, addr, reg, val);
> +}
> +
> +int zynq_gem_initialize(bd_t *bis, int base_addr)
> +{
> + struct eth_device *dev;
> + struct zynq_gem_priv *priv;
> +
> + 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;
> +
> +#ifdef CONFIG_PHY_ADDR
> + priv->phyaddr = CONFIG_PHY_ADDR;
> +#else
> + priv->phyaddr = -1;
> +#endif
> +
> + sprintf(dev->name, "Gem.%x", base_addr);
> +
> + dev->iobase = base_addr;
> +
> + 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;
> +
> + eth_register(dev);
> +
> + miiphy_register(dev->name, zynq_gem_miiphyread, zynq_gem_miiphy_write);
> + priv->bus = miiphy_get_dev_by_name(dev->name);
> +
> + return 1;
> +}
> diff --git a/include/netdev.h b/include/netdev.h
> index d1aaf0c..b8d303d 100644
> --- a/include/netdev.h
> +++ b/include/netdev.h
> @@ -104,7 +104,7 @@ 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, int base_addr);
> /*
> * 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.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
