[U-Boot] [PATCH v2] net: ll_temac: Add LL TEMAC driver to u-boot

Michal Simek monstr at monstr.eu
Thu Sep 1 13:11:27 CEST 2011


Marek Vasut wrote:
> On Tuesday, August 30, 2011 02:05:18 PM Michal Simek 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.
>>
>> Signed-off-by: Michal Simek <monstr at 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
>> ---
>>  drivers/net/Makefile          |    1 +
>>  drivers/net/xilinx_ll_temac.c |  665
>> +++++++++++++++++++++++++++++++++++++++++ include/netdev.h              | 
>>   2 +
>>  3 files changed, 668 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..878cdf2
>> --- /dev/null
>> +++ b/drivers/net/xilinx_ll_temac.c
>> @@ -0,0 +1,665 @@
>> +/*
>> + * Xilinx xps_ll_temac ethernet driver for u-boot
>> + *
>> + * Copyright (C) 2008 - 2011 Michal Simek <monstr at monstr.eu>
>> + * Copyright (C) 2008 - 2011 PetaLogix
>> + *
>> + * Based on Yoshio Kashiwagi kashiwagi at 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
>> +
>> +#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 */
>> +
> 
> Hi,
> 
> ok, this is the usual stuff -- use (1 << n) and struct temac_regs {...};

keep it till this is solved for axi enet.

> 
>> +#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
>> +
>> +/* 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 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;
> 
> maybe app[4] ?

Just follow temac documentation.

> 
>> +};
>> +
>> +static struct cdmac_bd_t tx_bd __attribute((aligned(DMAALIGN)));
>> +static struct cdmac_bd_t rx_bd __attribute((aligned(DMAALIGN)));
>> +
>> +struct ll_fifo_s {
>> +	int isr; /* Interrupt Status Register 0x0 */
>> +	int ier; /* Interrupt Enable Register 0x4 */
>> +	int tdfr; /* Transmit data FIFO reset 0x8 */
>> +	int tdfv; /* Transmit data FIFO Vacancy 0xC */
>> +	int tdfd; /* Transmit data FIFO 32bit wide data write port 0x10 */
>> +	int tlf; /* Write Transmit Length FIFO 0x14 */
>> +	int rdfr; /* Read Receive data FIFO reset 0x18 */
>> +	int rdfo; /* Receive data FIFO Occupancy 0x1C */
>> +	int rdfd; /* Read Receive data FIFO 32bit wide data read port 0x20 */
>> +	int rlf; /* Read Receive Length FIFO 0x24 */
>> +	int llr; /* Read LocalLink reset 0x28 */
>> +};
>> +
>> +static unsigned char tx_buffer[PKTSIZE_ALIGN]
>> __attribute((aligned(DMAALIGN))); +static unsigned char
>> rx_buffer[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))); +
>> +struct temac_reg {
>> +	unsigned long msw; /* Hard TEMAC MSW Data Register */
>> +	unsigned long lsw; /* Hard TEMAC LSW Data Register */
>> +	unsigned long ctl; /* Hard TEMAC Control Register */
>> +	unsigned long rdy; /* Hard TEMAC Ready Status */
>> +};
>> +
>> +struct ll_priv {
>> +	unsigned int ctrl;
>> +	struct temac_reg *regs;
>> +	unsigned int mode;
>> +	int phyaddr;
>> +
>> +	struct phy_device *phydev;
>> +	struct mii_dev *bus;
>> +};
>> +
>> +static void mtdcr_local(u32 reg, u32 val)
>> +{
>> +#if defined(CONFIG_PPC)
>> +	mtdcr(0x00, reg);
>> +	mtdcr(0x01, val);
>> +#endif
> 
> What are these magic values with no description ?

DCR number. Fix that.

> 
>> +}
>> +
>> +static u32 mfdcr_local(u32 reg)
>> +{
>> +	u32 val = 0;
>> +#if defined(CONFIG_PPC)
>> +	mtdcr(0x00, reg);
>> +	val = mfdcr(0x01);
>> +#endif
> 
> dtto

too.

> 
>> +	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);
> 
> If priv->ctrl was well defined structure, you won't need this cast, offset etc. 
> Besides I see you have it defined as unsigned int ... make it u32 if you want to 
> define registers that way. Still, struct is way to go.

Not sure if struct is way to go because offset for DCR and PLB is the same but register length
is different that's why I am using these helpers. It is different case than was for axi emac where
is no DCR access.

> 
>> +}
>> +
>> +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));
> 
> DITTO, besides, in the previous code, you only do out_be32() in else branch, 
> here you do it unconditionally. Maybe you can make them look similar.

Because if DCR_BIT is set it ends on return. Above I should use
one more line to return.

> 
>> +}
>> +
>> +#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 ll_priv *priv = dev->priv;
>> +	volatile struct temac_reg *regs = priv->regs;
>> +
>> +	regs->lsw = phy_data;
>> +	regs->ctl = CNTLREG_WRITE_ENABLE_MASK | MIIMWD;
>> +	regs->lsw = (phy_addr << 5) | reg_addr;
>> +	regs->ctl = CNTLREG_WRITE_ENABLE_MASK | MIIMAI | (emac << 10);
> 
> writel() or out_be32()

In Mike's comment.

> 
>> +	while (!(regs->rdy & XTE_RSE_MIIM_WR_MASK))
>> +		;
> 
> No endless loops

Fixed.

> 
>> +}
>> +#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 ll_priv *priv = dev->priv;
>> +	volatile struct temac_reg *regs = priv->regs;
>> +
>> +	regs->lsw = (phy_addr << 5) | reg_addr;
>> +	regs->ctl = MIIMAI | (emac << 10);
> 
> writel() or out_be32(), please fix globally.

as above

> 
>> +	while (!(regs->rdy & XTE_RSE_MIIM_RR_MASK))
>> +		;
> 
> no endless loops, please fix globally

as above

> 
>> +	return regs->lsw;
> 
> return readl() I guess ... 

as above
> 
>> +}
>> +
>> +/* 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 ll_priv *priv = dev->priv;
>> +	volatile struct temac_reg *regs = priv->regs;
> 
> No volatiles please.

as above

> 
>> +
>> +	regs->lsw = reg_data;
>> +	regs->ctl = CNTLREG_WRITE_ENABLE_MASK | (emac << 10) | reg_offset;
>> +	while (!(regs->rdy & 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 ll_priv *priv = dev->priv;
>> +	volatile struct temac_reg *regs = priv->regs;
>> +
>> +	regs->ctl = (emac << 10) | reg_offset;
>> +	while (!(regs->rdy & XTE_RSE_CFG_RR_MASK))
>> +		;
>> +	return regs->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)
>> +{
>> +#ifdef CONFIG_PHYLIB
>> +	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) &
>> +						(~XTE_EMMC_LINKSPEED_MASK);
> 
> Do you need those parenthesis ? Maybe you can divide this into ...
> 
> temp = xps_ll...();
> temp &= ~XTE...;
> temp |= speed;
> xps_ll...();
> 
> Might be a bit more readable.

fixed.

> 
>> +	temp |= speed;
>> +	xps_ll_temac_indirect_set(dev, 0, EMMC, temp);
>> +
>> +	return 1;
>> +#else
>> +	puts("Enable PHYLIB support!\n");
>> +	return 0;
>> +#endif
> 
> Please move this #ifdef CONFIG_PHYLIB at the begining of the file and emit a 
> compile-time warning.

Reported in axi thread.

> 
>> +}
>> +
>> +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, 0x00000001);
>> +	while (sdma_in_be32(priv, DMA_CONTROL_REG) & 1)
>> +		;
> 
> Ok, maybe #define SDMA_RESET 0x1 won't hurt here.

Done.

> 
>> +
>> +	/* 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((void *)&tx_bd, 0, sizeof(tx_bd));
>> +	memset((void *)&rx_bd, 0, sizeof(rx_bd));
> 
> Do you need the cast?

no, fixed.

> 
>> +
>> +	rx_bd.phys_buf_p = &rx_buffer[0];
> 
> rx_buffer would be sufficient here.

fix for tx_buffer too.

> 
>> +
>> +	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[0];
>> +	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 (!(((volatile int)tx_bd.stat) & BDSTAT_COMPLETED_MASK));
> 
> Whoa ... volatile int is really dangerous. And please add timeout.
>> +
>> +	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;
> 
> 0x3fff should be explained here.

Add comment to that line.

> 
>> +	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, ll_fifo->isr,
>> +		ll_fifo->ier, ll_fifo->rdfr, ll_fifo->rdfo, 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 len, i, val;
>> +
>> +	len = (length / 4) + 1;
>> +
> 
> Do you need the len at all ?
> 
> for (i = 0; i < length; i +=4 )
> 	writel(*buf++, ll_fifo->tdf
d);

fixed.

> 
>> +	for (i = 0; i < len; i++) {
>> +		val = *buf++;
>> +		ll_fifo->tdfd = val;
>> +	}
>> +
>> +	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 len = 0;
>> +	u32 len2, i, val;
>> +	u32 *buf = (u32 *)&rx_buffer;
>> +
>> +	if (ll_fifo->isr & 0x04000000) {
> 
> Another magic value, please fix globally.

use comments.

> 
>> +		ll_fifo->isr = 0xffffffff; /* reset isr */
>> +
>> +		/* while (ll_fifo->isr); */
>> +		len = ll_fifo->rlf & 0x7FF;
>> +		len2 = (len / 4) + 1;
>> +
>> +		for (i = 0; i < len2; i++) {
>> +			val = ll_fifo->rdfd;
>> +			*buf++ = val ;
>> +		}
>> +#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 {
>> +		ll_fifo->tdfr = 0x000000a5; /* set fifo length */
>> +		ll_fifo->rdfr = 0x000000a5;
>> +		/* ll_fifo->isr = 0x0; */
>> +		/* ll_fifo->ier = 0x0; */
> 
> Why do you have this dead code here?

Fixed.

> 
>> +	}
>> +
>> +	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, 0x00000000);
>> +	/* Enable Receiver */
>> +	xps_ll_temac_indirect_set(dev, 0, RCW1, 0x10000000);
>> +	/* Enable Transmitter */
>> +	xps_ll_temac_indirect_set(dev, 0, TC, 0x10000000);
> 
> Magic values.

Extend comment.


> 
>> +	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, 0x00000000);
>> +	/* Disable Transmitter */
> 
> Magic.
> 
>> +	xps_ll_temac_indirect_set(dev, 0, TC, 0x00000000);
>> +
>> +	if (priv->mode & SDMA_BIT) {
>> +		sdma_out_be32(priv->ctrl, DMA_CONTROL_REG, 0x00000001);
>> +		while (sdma_in_be32(priv->ctrl, DMA_CONTROL_REG) & 1)
>> +			;
>> +	}
>> +	/* reset fifos */
> 
> Dead comment?

More question if make sense to do it - probably not - I removed it.

> 
>> +#endif
>> +}
>> +
>> +static int ll_temac_init(struct eth_device *dev, bd_t *bis)
>> +{
>> +	struct ll_priv *priv = dev->priv;
>> +#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, int 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->regs = (void *) (base_addr + 0x20);
> 
> Here you'd just load the struct :)

It is changed.

> 
>> +	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) ||
>> defined(CONFIG_PHYLIB) +	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..cebae3b 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, int ctrl);
>>  int sh_eth_initialize(bd_t *bis);
>>  int dm9000_initialize(bd_t *bis);
>>  int fecmxc_initialize(bd_t *bis);
> 
> The driver has some way to go, but it's mostly coding style. One or two more 
> rounds and it's ready.

I will look at fifo mode and will send v3.

Thanks for comments,
Michal

-- 
Michal Simek, Ing. (M.Eng)
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel 2.6 Microblaze Linux - http://www.monstr.eu/fdt/
Microblaze U-BOOT custodian


More information about the U-Boot mailing list