[U-Boot] [PATCH] net: axi_ethernet: Add driver to u-boot

Michal Simek monstr at monstr.eu
Wed Aug 31 16:46:22 CEST 2011


Marek Vasut wrote:
> On Tuesday, August 30, 2011 02:05:19 PM Michal Simek wrote:
>> Add the first axi_ethernet driver for little-endian Microblaze.
>>
>> Signed-off-by: Michal Simek <monstr at monstr.eu>
>> ---
>>  drivers/net/Makefile          |    1 +
>>  drivers/net/xilinx_axi_emac.c |  622
>> +++++++++++++++++++++++++++++++++++++++++ include/netdev.h              | 
>>   1 +
>>  3 files changed, 624 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..ce79b80
>> --- /dev/null
>> +++ b/drivers/net/xilinx_axi_emac.c
>> @@ -0,0 +1,622 @@
>> +/*
>> + * Copyright (C) 2011 Michal Simek <monstr at 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>
>> +
>> +/* Axi Ethernet registers offset */
>> +#define XAE_IS_OFFSET		0x0000000C /* Interrupt status */
>> +#define XAE_IE_OFFSET		0x00000014 /* Interrupt enable */
>> +#define XAE_RCW1_OFFSET		0x00000404 /* Rx Configuration Word 1 */
>> +#define XAE_TC_OFFSET		0x00000408 /* Tx Configuration */
>> +#define XAE_EMMC_OFFSET		0x00000410 /* EMAC mode configuration */
>> +#define XAE_MDIO_MC_OFFSET	0x00000500 /* MII Management Config */
>> +#define XAE_MDIO_MCR_OFFSET	0x00000504 /* MII Management Control */
>> +#define XAE_MDIO_MWD_OFFSET	0x00000508 /* MII Management Write Data */
>> +#define XAE_MDIO_MRD_OFFSET	0x0000050C /* MII Management Read Data */
> 
> Please use struct xae_regs {...} as the rest of the u-boot.

struct is not useful here because it will be big with a lot of reserved values.

> 
>> +
>> +/* 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
>> */ +
> 
> Use (1 << n) ?

just another solution - I prefer to use 32bit value - easier when you decode it
by md.

> 
>> +/* 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 */
>> +
> 
> [...]

same here..

> 
>> +#define DMAALIGN	128
>> +
>> +static u8 RxFrame[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))) ;
> 
> Don't use cammelcase, all lowcase please.

Agree - will be done in v2.

  Also, can't you allocate this with
> memalign and hide it in axidma_priv or something ?

There are two things in one.
1. Adding it to axidma_priv means that I will need to dynamically allocate big private structure
which is bad thing to do for several eth devices. This is FPGA you can create a lot of MACs that's
why I think it is better not to do so.
2. Current style is sharing this rxframe buffer among all controllers because only one MAC works.
Others are stopped which means that no packet come to them.

BTW: Looking for that memalign function - thanks.



>> +
>> +/* 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)));
>> +
>> +static inline void aximac_out32(u32 addr, u32 offset, u32 val)
>> +{
>> +	out_be32((u32 *)(addr + offset), val);
> 
> Please fix these casts ... though I don't think you even need these functions.

Cast is necessary. I use that helper just because of recast.
If you see solution which will be elegant, I am open to use it.

> 
>> +}
>> +
>> +static inline u32 aximac_in32(u32 addr, u32 offset)
>> +{
>> +	return in_be32((u32 *)(addr + offset));
>> +}
>> +
>> +
>> +/* 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 u16 phyread(struct eth_device *dev, u32 phyaddress, u32
>> registernum) +{
>> +	u32 mdioctrlreg = 0;
>> +
>> +	/* Wait till MDIO interface is ready to accept a new transaction. */
>> +	while (!(aximac_in32(dev->iobase, XAE_MDIO_MCR_OFFSET)
>> +						& XAE_MDIO_MCR_READY_MASK))
>> +		;
> 
> No endless loops please.

Agree will fix it.

> 
>> +
>> +	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;
>> +
>> +	aximac_out32(dev->iobase, XAE_MDIO_MCR_OFFSET, mdioctrlreg);
>> +
>> +	/* Wait till MDIO transaction is completed. */
>> +	while (!(aximac_in32(dev->iobase, XAE_MDIO_MCR_OFFSET)
>> +						& XAE_MDIO_MCR_READY_MASK))
>> +		;
>> +
>> +	/* Read data */
>> +	return (u16) aximac_in32(dev->iobase, XAE_MDIO_MRD_OFFSET);
> 
> Is the cast needed ?

not - thanks.

> 
>> +}
>> +
>> +static void phywrite(struct eth_device *dev, u32 phyaddress, u32
>> registernum, +								u32 data)
>> +{
>> +	u32 mdioctrlreg = 0;
>> +
>> +	/* Wait till MDIO interface is ready to accept a new transaction. */
>> +	while (!(aximac_in32(dev->iobase, XAE_MDIO_MCR_OFFSET)
>> +						& XAE_MDIO_MCR_READY_MASK))
>> +		;
> 
> No endless loops.

will be fixed.

> 
>> +
>> +	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 */
>> +	aximac_out32(dev->iobase, XAE_MDIO_MWD_OFFSET, data);
>> +
>> +	aximac_out32(dev->iobase, XAE_MDIO_MCR_OFFSET, mdioctrlreg);
>> +
>> +	/* Wait till MDIO transaction is completed. */
>> +	while (!(aximac_in32(dev->iobase, XAE_MDIO_MCR_OFFSET)
>> +						& XAE_MDIO_MCR_READY_MASK))
>> +		;
>> +}
>> +
>> +
>> +/* setting axi emac and phy to proper setting */
>> +static int setup_phy(struct eth_device *dev)
>> +{
>> +#ifdef CONFIG_PHYLIB
>> +	int i;
>> +	unsigned int speed;
>> +	u16 phyreg;
>> +	u32 emmc_reg;
>> +	struct axidma_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;
> 
> cammelcase ?

Just using standard values.
include/phy.h
include/linux/ethtool.h

> 
>> +
>> +	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("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 = aximac_in32(dev->iobase, XAE_EMMC_OFFSET);
>> +	emmc_reg &= ~XAE_EMMC_LINKSPEED_MASK;
>> +	emmc_reg |= speed;
>> +
>> +	/* Write new speed setting out to Axi Ethernet */
>> +	aximac_out32(dev->iobase, XAE_EMMC_OFFSET, emmc_reg);
> 
> Use clrsetbits() here.

Not defined for Microblaze - just for ARM/PPC.
Not going to use it.

> 
>> +
>> +	/*
>> +	* 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;
>> +#else
>> +	puts("Enable PHYLIB support!\n");
> 
> Compile time warning at the top of the file please.

Fixed - the same for ll_temac.

> 
>> +	return 0;
>> +#endif
>> +}
>> +
>> +/* 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 void axi_ethernet_init(struct eth_device *dev)
>> +{
>> +	/*
>> +	 * 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 && (!(aximac_in32(dev->iobase, XAE_IS_OFFSET) &
>> +							XAE_INT_MGTRDY_MASK)))
>> +		timeout--;
>> +
>> +	if (!timeout)
>> +		printf("%s: Timeout\n", __func__);
> 
> I think you don't want to continue in case of timeout ?

fixed.

> 
>> +
>> +	/* Stop the device and reset HW */
>> +	/* Disable interrupts */
>> +	aximac_out32(dev->iobase, XAE_IE_OFFSET, 0);
>> +
>> +	/* Disable the receiver */
>> +	aximac_out32(dev->iobase, XAE_RCW1_OFFSET,
>> +		aximac_in32(dev->iobase, XAE_RCW1_OFFSET) & ~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 */
>> +	aximac_out32(dev->iobase, XAE_IS_OFFSET, XAE_INT_RXRJECT_MASK);
>> +
>> +	/* Setup HW */
>> +	/* Set default MDIO divisor */
>> +	aximac_out32(dev->iobase, XAE_MDIO_MC_OFFSET,
>> +			(u32) XAE_MDIO_DIV_DFT | XAE_MDIO_MC_MDIOEN_MASK);
>> +
>> +	debug("XAxiEthernet InitHw: done\n");
> 
> Unify the debuging message please.

good point.

> 
>> +}
>> +
>> +static void setup_mac(struct eth_device *dev)
>> +{
>> +	/* Set the MAC address */
>> +	int val = ((dev->enetaddr[3] << 24) | (dev->enetaddr[2] << 16) |
>> +		(dev->enetaddr[1] << 8) | (dev->enetaddr[0]));
>> +	aximac_out32(dev->iobase, XAE_UAW0_OFFSET, val);
>> +
>> +	val = (dev->enetaddr[5] << 8) | dev->enetaddr[4] ;
>> +	val |= aximac_in32(dev->iobase, XAE_UAW1_OFFSET) &
>> +						~XAE_UAW1_UNICASTADDR_MASK;
>> +	aximac_out32(dev->iobase, XAE_UAW1_OFFSET, val);
> 
> clrsetbits()

same as above.

> 
>> +}
>> +
>> +/* 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;
>> +		timeout -= 1;
> 
> You're decrementing it twice in here.

thanks.

> 
>> +	}
>> +
>> +	if (!timeout)
>> +		printf("%s: Timeout\n", __func__);
>> +}
>> +
>> +static int axiemac_init(struct eth_device *dev, bd_t * bis)
>> +{
>> +	struct axidma_priv *priv = dev->priv;
>> +	debug("axi emac 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. */
>> +	axi_ethernet_init(dev);
>> +	setup_mac(dev);
>> +
>> +	/* 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 & XAXIDMA_IRQ_ALL_MASK);
> 
> I don't think I understand the above code.

There is one more XAXIDMA_IRQ_ALL_MASK. There is longer history above it.
Fixed.

> 
>> +
>> +	/* Setup the BD. */
>> +	memset((void *) &rx_bd, 0, sizeof(rx_bd));
>> +	rx_bd.next = (u32)&rx_bd;
>> +	rx_bd.phys = (u32)&RxFrame;
>> +	rx_bd.cntrl = sizeof(RxFrame);
> 
> please get rid of the cammelcase.

done

> 
>> +	/* 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 */
>> +	aximac_out32(dev->iobase, XAE_TC_OFFSET, XAE_TC_TX_MASK);
>> +	/* enable RX */
>> +	aximac_out32(dev->iobase, XAE_RCW1_OFFSET, XAE_RCW1_RX_MASK);
>> +
> 
> [...]

above.

> 
>> +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("axi emac, rx data ready\n");
>> +
>> +	/* Disable IRQ for a moment till packet is handled */
>> +	priv->dmarx->control &= ~(XAXIDMA_IRQ_ALL_MASK & XAXIDMA_IRQ_ALL_MASK);
>> +
>> +	length = rx_bd.app4 & 0x0000FFFF;
> 
> Get rid of the strange constants please.

done.

> 
>> +#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 */
> [...]

Thanks for your 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