| Message ID | 1345098630-27902-2-git-send-email-monstr@monstr.eu |
|---|---|
| State | Superseded |
| Delegated to: | Albert ARIBAUD |
| Headers | show |
On 08/16/2012 08:30 AM, Michal Simek wrote: > Device driver for Zynq Gem IP. > > Signed-off-by: Michal Simek <monstr@monstr.eu> > CC: Joe Hershberger <joe.hershberger@gmail.com> > > --- > v2: Remove phylib protection > Rename driver file name xilinx_gem to zynq_gem > Rename XEMACPSS to ZYNQ_GEM > Rename gemac_priv to zynq_gem_priv > Rename gem_regs to zynq_gem_regs > Add zynq_ prefix to several functions > Remove phy detection > Rename driver name XGem to Gem > Change setup_mac function to reflect u-boot style > --- > drivers/net/Makefile | 1 + > drivers/net/zynq_gem.c | 453 ++++++++++++++++++++++++++++++++++++++++++++++++ > include/netdev.h | 2 +- > 3 files changed, 455 insertions(+), 1 deletions(-) > create mode 100644 drivers/net/zynq_gem.c Joe: I am not sure if you have seen this patch. Can you please review it and comment it. If is ok, I would like to ask you to add this patch to your net custodian tree or sending me your ACK. Thanks, Michal
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.
On 09/13/2012 11:28 AM, Marek Vasut wrote: > 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. make sense > >> + 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 () . if you like. > >> + 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 ;-) :-) Or. let me create macros. > >> + /* >> + * 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); the same here. >> + >> + /* >> + * 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() ok. > >> + /* 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() ok > >> + /* 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. or macro should be fine. > >> + 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 no problem. > >> + 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 ? probably not. Let me do that changes and I will send updated version. Thanks, Michal
Dear Michal Simek, [...] > > > > Add const int mask for the above. > > or macro should be fine. const int is better due to type checking . [...] Thanks! Best regards, Marek Vasut
Hi Marek, On Thu, Sep 13, 2012 at 4:28 AM, Marek Vasut <marex@denx.de> wrote: > Dear Michal Simek, >> + /* >> + * 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() This is not equivalent. Using setbits_le32() will not provide a dmb() on the operations the way that readl(), writel() does. I believe this will cause problems when the dcache is enabled, right? -Joe
Dear Joe Hershberger, > Hi Marek, > > On Thu, Sep 13, 2012 at 4:28 AM, Marek Vasut <marex@denx.de> wrote: > > Dear Michal Simek, > > > >> + /* > >> + * 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() > > This is not equivalent. Using setbits_le32() will not provide a dmb() > on the operations the way that readl(), writel() does. I believe this > will cause problems when the dcache is enabled, right? Not when dcache is enabled, the register space isn't cached. But the compiler can run some wild optimizations across that. So where's the problem, do we add dmb() to clrsetbits() calls ? > -Joe Best regards, Marek Vasut
Dear Joe Hershberger, > On Thu, Sep 13, 2012 at 11:45 PM, Marek Vasut <marex@denx.de> wrote: > > Dear Joe Hershberger, > > > >> Hi Marek, > >> > >> On Thu, Sep 13, 2012 at 4:28 AM, Marek Vasut <marex@denx.de> wrote: > >> > Dear Michal Simek, > >> > > >> >> + /* > >> >> + * 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() > >> > >> This is not equivalent. Using setbits_le32() will not provide a dmb() > >> on the operations the way that readl(), writel() does. I believe this > >> will cause problems when the dcache is enabled, right? > > > > Not when dcache is enabled, the register space isn't cached. But the > > compiler can run some wild optimizations across that. So where's the > > problem, do we add dmb() to clrsetbits() calls ? > > I'm not grokking this paragraph. The only thing that is ever cached is DRAM. It's the only cacheable area configured in the MMU tables. The rest is uncached. And the dmb() doesn't have anything to do with caches, it has to do with compiler reordering the register accesses, which might screw things up. But wait, I shouldn't reply to emails when I've been awake for 20+ hours. Look at the macro again. Let's see an example: clrbits_le32 expands to clrbits(le32, ...) clrbits(le32, ...) expands to out_le32(..., in_le32(...) & ~...) out_le32() expands to out_arch() which expands to __raw_writel() in_le32() expands to in_arch() which expands to __raw_readl() __raw_writel() expands to __arch_putl() __raw_readl() expands to __arch_getl() arch_putl() is *(volatile unsigned long *), therefore compiler reordering doesn't happen accross this (because of the volatile). So is arch_getl() . Bottom line, no need for explicit dmb() in clrsetbits_le32() and friends. But whew, you got me scared there ;-) > You are saying that when the dcache is enabled, that the register > space is not cached? Why would it not be? It doesn't know those > addresses from any other memory, right? > > clrsetbits() does not call dmb(). Perhaps it should. It's never been > clear to me why writeX/readX used dmb, but out_XY/in_XY/clrsetbits_XY > do not. Are they not both for accessing registers? > > Perhaps test all of these drivers with write-back cache enabled and > make sure they work. > > -Joe Best regards, Marek Vasut
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 430f90c..b298588 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -79,6 +79,7 @@ COBJS-$(CONFIG_XILINX_AXIEMAC) += xilinx_axi_emac.o COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o COBJS-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o xilinx_ll_temac_mdio.o \ xilinx_ll_temac_fifo.o xilinx_ll_temac_sdma.o +COBJS-$(CONFIG_ZYNQ_GEM) += zynq_gem.o COBJS := $(sort $(COBJS-y)) SRCS := $(COBJS:.o=.c) diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c new file mode 100644 index 0000000..d62a493 --- /dev/null +++ b/drivers/net/zynq_gem.c @@ -0,0 +1,453 @@ +/* + * (C) Copyright 2011 Michal Simek + * + * Michal SIMEK <monstr@monstr.eu> + * + * Based on Xilinx gmac driver: + * (C) Copyright 2011 Xilinx + * + * 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 <common.h> +#include <net.h> +#include <config.h> +#include <malloc.h> +#include <asm/io.h> +#include <phy.h> +#include <miiphy.h> + +#if !defined(CONFIG_PHYLIB) +# error XILINX_GEM_ETHERNET requires PHYLIB +#endif + +/* Bit/mask specification */ +#define ZYNQ_GEM_PHYMNTNC_OP_MASK 0x40020000 /* operation mask bits */ +#define ZYNQ_GEM_PHYMNTNC_OP_R_MASK 0x20000000 /* read operation */ +#define ZYNQ_GEM_PHYMNTNC_OP_W_MASK 0x10000000 /* write operation */ +#define ZYNQ_GEM_PHYMNTNC_PHYAD_SHIFT_MASK 23 /* Shift bits for PHYAD */ +#define ZYNQ_GEM_PHYMNTNC_PHREG_SHIFT_MASK 18 /* Shift bits for PHREG */ + +#define ZYNQ_GEM_RXBUF_EOF_MASK 0x00008000 /* End of frame. */ +#define ZYNQ_GEM_RXBUF_SOF_MASK 0x00004000 /* Start of frame. */ +#define ZYNQ_GEM_RXBUF_LEN_MASK 0x00003FFF /* Mask for length field */ + +#define ZYNQ_GEM_RXBUF_WRAP_MASK 0x00000002 /* Wrap bit, last BD */ +#define ZYNQ_GEM_RXBUF_NEW_MASK 0x00000001 /* Used bit.. */ +#define ZYNQ_GEM_RXBUF_ADD_MASK 0xFFFFFFFC /* Mask for address */ + +/* Wrap bit, last descriptor */ +#define ZYNQ_GEM_TXBUF_WRAP_MASK 0x40000000 +#define ZYNQ_GEM_TXBUF_LAST_MASK 0x00008000 /* Last buffer */ + +#define ZYNQ_GEM_TXSR_HRESPNOK_MASK 0x00000100 /* Transmit hresp not OK */ +#define ZYNQ_GEM_TXSR_URUN_MASK 0x00000040 /* Transmit underrun */ +/* Transmit buffs exhausted mid frame */ +#define ZYNQ_GEM_TXSR_BUFEXH_MASK 0x00000010 + +#define ZYNQ_GEM_NWCTRL_TXEN_MASK 0x00000008 /* Enable transmit */ +#define ZYNQ_GEM_NWCTRL_RXEN_MASK 0x00000004 /* Enable receive */ +#define ZYNQ_GEM_NWCTRL_MDEN_MASK 0x00000010 /* Enable MDIO port */ +#define ZYNQ_GEM_NWCTRL_STARTTX_MASK 0x00000200 /* Start tx (tx_go) */ + +#define ZYNQ_GEM_NWSR_MDIOIDLE_MASK 0x00000004 /* PHY management idle */ + +/* Device registers */ +struct zynq_gem_regs { + u32 nwctrl; /* Network Control reg */ + u32 nwcfg; /* Network Config reg */ + u32 nwsr; /* Network Status reg */ + u32 reserved1; + u32 dmacr; /* DMA Control reg */ + u32 txsr; /* TX Status reg */ + u32 rxqbase; /* RX Q Base address reg */ + u32 txqbase; /* TX Q Base address reg */ + u32 rxsr; /* RX Status reg */ + u32 reserved2[2]; + u32 idr; /* Interrupt Disable reg */ + u32 reserved3; + u32 phymntnc; /* Phy Maintaince reg */ + u32 reserved4[18]; + u32 hashl; /* Hash Low address reg */ + u32 hashh; /* Hash High address reg */ +#define LADDR_LOW 0 +#define LADDR_HIGH 1 + u32 laddr[4][LADDR_HIGH + 1]; /* Specific1 addr low/high reg */ + u32 match[4]; /* Type ID1 Match reg */ + u32 reserved6[18]; + u32 stat[44]; /* Octects transmitted Low reg - stat start */ +}; + +/* BD descriptors */ +struct emac_bd { + u32 addr; /* Next descriptor pointer */ + u32 status; +}; + +#define RX_BUF 3 + +/* Initialized, rxbd_current, rx_first_buf must be 0 after init */ +struct zynq_gem_priv { + struct emac_bd tx_bd; + struct emac_bd rx_bd[RX_BUF]; + u32 initialized; + char rxbuffers[RX_BUF * PKTSIZE_ALIGN]; + u32 rxbd_current; + u32 rx_first_buf; + int phyaddr; + struct phy_device *phydev; + struct mii_dev *bus; +}; + +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)) { + timeout--; + udelay(1); + } + + if (!timeout) { + printf("%s: Timeout\n", __func__); + return 1; + } + + return 0; +} + +static u32 phy_setup_op(struct eth_device *dev, u32 phy_addr, u32 regnum, + u32 op, u16 *data) +{ + u32 mgtcr; + struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->iobase; + + if (mdio_wait(dev)) + return 1; + + /* Construct mgtcr mask for the operation */ + mgtcr = ZYNQ_GEM_PHYMNTNC_OP_MASK | op | + (phy_addr << ZYNQ_GEM_PHYMNTNC_PHYAD_SHIFT_MASK) | + (regnum << ZYNQ_GEM_PHYMNTNC_PHREG_SHIFT_MASK) | *data; + + /* Write mgtcr and wait for completion */ + writel(mgtcr, ®s->phymntnc); + + if (mdio_wait(dev)) + return 1; + + if (op == ZYNQ_GEM_PHYMNTNC_OP_R_MASK) + *data = readl(®s->phymntnc); + + return 0; +} + +static u32 phyread(struct eth_device *dev, u32 phy_addr, u32 regnum, u16 *val) +{ + return phy_setup_op(dev, phy_addr, regnum, + ZYNQ_GEM_PHYMNTNC_OP_R_MASK, val); +} + +static u32 phywrite(struct eth_device *dev, u32 phy_addr, u32 regnum, u16 data) +{ + return phy_setup_op(dev, phy_addr, regnum, + ZYNQ_GEM_PHYMNTNC_OP_W_MASK, &data); +} + +static int zynq_gem_setup_mac(struct eth_device *dev) +{ + u32 i, macaddrlow, macaddrhigh; + struct zynq_gem_regs *regs = (struct zynq_gem_regs *)dev->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; + + /* Set MAC bits [47:32] in TOP */ + macaddrhigh |= dev->enetaddr[4]; + macaddrhigh |= dev->enetaddr[5] << 8; + + for (i = 0; i < 4; i++) { + writel(0, ®s->laddr[i][LADDR_LOW]); + writel(0, ®s->laddr[i][LADDR_HIGH]); + /* Do not use MATCHx register */ + writel(0, ®s->match[i]); + } + + writel(macaddrlow, ®s->laddr[0][LADDR_LOW]); + writel(macaddrhigh, ®s->laddr[0][LADDR_HIGH]); + + 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++) + 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); + + /* + * 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); + + /* 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); + + /* 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)) { + 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; + 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; +} + +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.
Device driver for Zynq Gem IP. Signed-off-by: Michal Simek <monstr@monstr.eu> CC: Joe Hershberger <joe.hershberger@gmail.com> --- v2: Remove phylib protection Rename driver file name xilinx_gem to zynq_gem Rename XEMACPSS to ZYNQ_GEM Rename gemac_priv to zynq_gem_priv Rename gem_regs to zynq_gem_regs Add zynq_ prefix to several functions Remove phy detection Rename driver name XGem to Gem Change setup_mac function to reflect u-boot style --- drivers/net/Makefile | 1 + drivers/net/zynq_gem.c | 453 ++++++++++++++++++++++++++++++++++++++++++++++++ include/netdev.h | 2 +- 3 files changed, 455 insertions(+), 1 deletions(-) create mode 100644 drivers/net/zynq_gem.c