net: ll_temac: Add LL TEMAC driver to u-boot

PATCH Superseded Michal Simek 4 participants 19 messages Aug 30, 2011 – Sep 1, 2011

Subsystems (2)

Network drivers Arm/Zynq architecture

lore

Share & tools

lore

Michal Simek

author

2011-08-30 12:05 UTC #0 lore raw

PATCH v2 1/1 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 <[email protected]>

---
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

drivers/net/Makefile +1 -0

index 819b197..4541eaf 100644
@@ -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)

drivers/net/xilinx_ll_temac.c new file +665 -0

665 lines not on this page · raw message

include/netdev.h +2 -0

index 26ce13d..cebae3b 100644
@@ -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);

Signature · 1 line

1.5.5.6

Michal Simek

author

2011-08-30 12:05 UTC #1 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Add the first axi_ethernet driver for little-endian Microblaze.

Signed-off-by Michal Simek <[email protected]>

---
 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

drivers/net/Makefile +1 -0

index 4541eaf..ae9d4cb 100644
@@ -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

drivers/net/xilinx_axi_emac.c new file +622 -0

622 lines not on this page · raw message

include/netdev.h +1 -0

index cebae3b..a791ece 100644
@@ -90,6 +90,7 @@ 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, int 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,

Signature · 1 line

1.5.5.6

Michal Simek

author

2011-08-31 14:46 UTC #3 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 66 lines · Marek Vasut

Quoted 65 lines · Michal Simek

Add the first axi_ethernet driver for little-endian Microblaze.

Signed-off-by Michal Simek <[email protected]>

---
 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

drivers/net/Makefile +1 -0

index 4541eaf..ae9d4cb 100644
@@ -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

drivers/net/xilinx_axi_emac.c new file +36 -0

new file mode 100644
index 0000000..ce79b80
@@ -0,0 +1,622 @@
+/*
+ * Copyright (C) 2011 Michal Simek <[email protected]>
+ * 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 */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 42

Quoted 6 lines

drivers/net/xilinx_axi_emac.c new file +6 -0

+#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.

Quoted 10 lines

Quoted 8 lines

drivers/net/xilinx_axi_emac.c new file +6 -0

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

Quoted 7 lines

Quoted 5 lines

drivers/net/xilinx_axi_emac.c new file +4 -0

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

Quoted 6 lines

drivers/net/xilinx_axi_emac.c line 102
Quoted 4 lines
drivers/net/xilinx_axi_emac.c new file +3 -0
+#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

Quoted 1 line

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.

Quoted 41 lines

drivers/net/xilinx_axi_emac.c new file +41 -0

+
+/* 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 */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 149
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+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.

Quoted 22 lines

drivers/net/xilinx_axi_emac.c new file +20 -0

+}
+
+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) +{

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 176
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	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.

Quoted 12 lines

drivers/net/xilinx_axi_emac.c new file +11 -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;
+
+	aximac_out32(dev->iobase, XAE_MDIO_MCR_OFFSET, mdioctrlreg);
+
+	/* Wait till MDIO transaction is completed. */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 193
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	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.

Quoted 14 lines

drivers/net/xilinx_axi_emac.c line 176
Quoted 12 lines
drivers/net/xilinx_axi_emac.c new file +3 -0
+}
+
+static void phywrite(struct eth_device *dev, u32 phyaddress, u32
registernum, + u32 data)
drivers/net/xilinx_axi_emac.c new file +7 -0
+{
+	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.

Quoted 32 lines

drivers/net/xilinx_axi_emac.c new file +31 -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_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;
+

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 241
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	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

Quoted 41 lines

drivers/net/xilinx_axi_emac.c new file +40 -0

+
+	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 */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 287
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	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.

Quoted 14 lines

drivers/net/xilinx_axi_emac.c line 298
Quoted 12 lines
drivers/net/xilinx_axi_emac.c new file +11 -0
+
+	/*
+	* 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.

Quoted 27 lines

drivers/net/xilinx_axi_emac.c new file +26 -0

+	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;

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 330
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	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.

Quoted 17 lines

drivers/net/xilinx_axi_emac.c new file +16 -0

+
+	/* 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);
+

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 352
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	/* 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.

Quoted 16 lines

Quoted 4 lines

drivers/net/xilinx_axi_emac.c new file +3 -0
+}
+
+static void setup_mac(struct eth_device *dev)

drivers/net/xilinx_axi_emac.c line 365

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

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

Quoted 15 lines

drivers/net/xilinx_axi_emac.c new file +14 -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--) {

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 385
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+		/* 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.

Quoted 25 lines

drivers/net/xilinx_axi_emac.c new file +24 -0

+	}
+
+	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 */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 415
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	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.

Quoted 9 lines

drivers/net/xilinx_axi_emac.c line 421
Quoted 7 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+
+	/* 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

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +9 -0

+	/* 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;

Quoted 8 lines

Quoted 6 lines

drivers/net/xilinx_axi_emac.c new file +6 -0

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

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +9 -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;
+

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 523
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -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.

Quoted 14 lines

Quoted 13 lines

drivers/net/xilinx_axi_emac.c new file +12 -0

+#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

Signature · 4 lines

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

Marek Vasut

2011-08-31 15:19 UTC #4 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 84 lines · Michal Simek

Quoted 74 lines · Marek Vasut

Quoted 73 lines · Michal Simek

Add the first axi_ethernet driver for little-endian Microblaze.

Signed-off-by Michal Simek <[email protected]>

---

 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

drivers/net/Makefile +1 -0

index 4541eaf..ae9d4cb 100644
@@ -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

(quoted diff) +0 -0

b/drivers/net/xilinx_axi_emac.c new file mode 100644 index 0000000..ce79b80

drivers/net/xilinx_axi_emac.c new file +36 -0

@@ -0,0 +1,622 @@
+/*
+ * Copyright (C) 2011 Michal Simek <[email protected]>
+ * 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 */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 42

Quoted 6 lines

drivers/net/xilinx_axi_emac.c new file +6 -0

+#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.

Quoted 2 lines

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

I see like 10 registers here, you can use "uint32_t reserved[n];"

Quoted 11 lines

Quoted 10 lines
Quoted 8 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+
+/* 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) ?

Quoted 2 lines

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

It's hard to read, really.

Quoted 22 lines

Quoted 7 lines
Quoted 5 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+/* 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..
Quoted 5 lines
drivers/net/xilinx_axi_emac.c line 102
Quoted 3 lines
drivers/net/xilinx_axi_emac.c new file +3 -0
+#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
Quoted 1 line
memalign and hide it in axidma_priv or something ?

Quoted 8 lines

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.

Ok, I don't think I understand pt. 1. -- you need to keep the state for each of the ethernet devices on the FPGA separate, don't you. As for pt. 2. -- "currently", so there's possibility, in future this won't hold?

Quoted 54 lines

BTW: Looking for that memalign function - thanks.

Quoted 42 lines

drivers/net/xilinx_axi_emac.c new file +42 -0

+
+/* 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)));

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 149
Quoted 5 lines
drivers/net/xilinx_axi_emac.c new file +5 -0
+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.

Quoted 2 lines

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.

See above -- use the structure and then just use &axiregs->reg.

Quoted 11 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

+}
+
+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

[...]

Quoted 7 lines

Quoted 4 lines
drivers/net/xilinx_axi_emac.c line 287
Quoted 2 lines
drivers/net/xilinx_axi_emac.c new file +2 -0
+	/* 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.

Please fix then. You're the microblaze maintainer, right?

[...]

Cheers!

Michal Simek

author

2011-09-01 07:17 UTC #5 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 83 lines · Marek Vasut

Quoted 82 lines · Michal Simek

Quoted 81 lines · Marek Vasut

Quoted 69 lines · Michal Simek

Add the first axi_ethernet driver for little-endian Microblaze.

Signed-off-by Michal Simek <[email protected]>

---

 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

drivers/net/Makefile +1 -0

index 4541eaf..ae9d4cb 100644
@@ -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

(quoted diff) +0 -0

b/drivers/net/xilinx_axi_emac.c new file mode 100644 index 0000000..ce79b80

drivers/net/xilinx_axi_emac.c new file +32 -0

@@ -0,0 +1,622 @@
+/*
+ * Copyright (C) 2011 Michal Simek <[email protected]>
+ * 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>
+

drivers/net/xilinx_axi_emac.c line 42

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

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

Quoted 4 lines

Quoted 2 lines
struct is not useful here because it will be big with a lot of reserved values.
I see like 10 registers here, you can use "uint32_t reserved[n];"

+ UAW0 and UAW1 with 0x700 offset.

struct axi_ethernet {
	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 */
};

Are you really sure that this is nice?

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
Quoted 8 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+
+/* 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) ?

Quoted 4 lines

Quoted 2 lines
just another solution - I prefer to use 32bit value - easier when you decode it by md.
It's hard to read, really.

At offset 40b40410. It is really hard to decode it if values are (1 << n). With defined macro you directly see that this is on 100Mbit/s LAN.

U-Boot-mONStR> md 40b40400

40b40400: ddccbbaa 6800ffee 4a000000 60000000    .......h...J...`
40b40410: 40000000 00000000 000ce000 000ce000    ... at ............
40b40420: 000ce000 000ce000 000ce000 000ce000    ................
40b40430: 000ce000 000ce000 000ce000 000ce000    ................

I am fine to use 1 << n solution but definitely in our repo I will use in way I like.

Quoted 26 lines

Quoted 17 lines
Quoted 6 lines
Quoted 5 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+/* 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..
Quoted 4 lines
drivers/net/xilinx_axi_emac.c line 102
Quoted 3 lines
drivers/net/xilinx_axi_emac.c new file +3 -0
+#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
Quoted 1 line
memalign and hide it in axidma_priv or something ?
Quoted 8 lines
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.

Quoted 2 lines

Ok, I don't think I understand pt. 1. -- you need to keep the state for each of the ethernet devices on the FPGA separate, don't you.

Just the whole private structure with addresses, + phy.

As for pt. 2. --

Quoted 1 line

"currently", so there's possibility, in future this won't hold?

BTW: I am also sharing rx/tx buffer descriptors for dma.

When do you expect that u-boot will be able to use several MACs in one time?

Quoted 70 lines

Quoted 52 lines

BTW: Looking for that memalign function - thanks.

Quoted 41 lines

drivers/net/xilinx_axi_emac.c new file +41 -0

+
+/* 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 */

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 149
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+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.

Quoted 2 lines

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.

See above -- use the structure and then just use &axiregs->reg.

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

+}
+
+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

[...]

Quoted 7 lines

Quoted 5 lines
Quoted 3 lines
drivers/net/xilinx_axi_emac.c line 287
Quoted 2 lines
drivers/net/xilinx_axi_emac.c new file +2 -0
+	/* 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.
Please fix then. You're the microblaze maintainer, right?

Custodian. But I won't do that.
If you think that all archs should have it then move it to generic location which clean code duplication and I will include it.

Thanks,
Michal

Signature · 4 lines

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

Marek Vasut

2011-09-01 08:18 UTC #6 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 9 lines · Michal Simek

Quoted 8 lines · Marek Vasut
Quoted 7 lines · Michal Simek
Quoted 6 lines · Marek Vasut
Quoted 5 lines · Michal Simek
Add the first axi_ethernet driver for little-endian Microblaze.
Signed-off-by Michal Simek <[email protected]>
---

[...]

Quoted 42 lines

Quoted 15 lines

Quoted 14 lines

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

+
+/* 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

Quoted 2 lines

*/ +#define XAE_MDIO_MRD_OFFSET 0x0000050C /* MII Management Read
Data */

Please use struct xae_regs {...} as the rest of the u-boot.

Quoted 4 lines

Quoted 2 lines
struct is not useful here because it will be big with a lot of reserved values.
I see like 10 registers here, you can use "uint32_t reserved[n];"

+ UAW0 and UAW1 with 0x700 offset.

struct axi_ethernet {
	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 */
};

Quoted 2 lines

Are you really sure that this is nice?

Yep, definitelly great.

Quoted 4 lines

Quoted 4 lines
Quoted 4 lines
Quoted 4 lines
drivers/net/xilinx_axi_emac.c
Quoted 4 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+
+/* Link setup */
+#define XAE_EMMC_LINKSPEED_MASK	0xC0000000 /* Link speed */
+#define XAE_EMMC_LINKSPD_10	0x00000000 /* Link Speed mask for 10 

Mbit

Quoted 14 lines

Quoted 10 lines
Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
*/ +#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.
It's hard to read, really.
At offset 40b40410. It is really hard to decode it if values are (1 << n). With defined macro you directly see that this is on 100Mbit/s LAN.

Quoted 8 lines

U-Boot-mONStR> md 40b40400

40b40400: ddccbbaa 6800ffee 4a000000 60000000    .......h...J...`
40b40410: 40000000 00000000 000ce000 000ce000    ... at ............
40b40420: 000ce000 000ce000 000ce000 000ce000    ................
40b40430: 000ce000 000ce000 000ce000 000ce000    ................

I am fine to use 1 << n solution but definitely in our repo I will use in way I like.

Well I see it both ways ... 0x40000000 == 1 << 30 ... it's the same thing. On the other note, it's hard to count the zeroes in there AND you can mistake 0 and 8 in a huge series of those.

Also, you can have whatever you want in your repo if you seriously care to invest the energy into maintaining it just because you need to be stubborn. But it'd really be great if you invested that energy in a more productive manner ;-)

Quoted 9 lines

Quoted 9 lines
Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+/* 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..

Yep, 1 << 3 and 1 << 7.

Quoted 22 lines

Quoted 22 lines
Quoted 21 lines
Quoted 6 lines
drivers/net/xilinx_axi_emac.c line 102
Quoted 4 lines
drivers/net/xilinx_axi_emac.c new file +3 -0
+#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
Quoted 2 lines
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.

Quoted 5 lines

Quoted 2 lines
Ok, I don't think I understand pt. 1. -- you need to keep the state for each of the ethernet devices on the FPGA separate, don't you.
Just the whole private structure with addresses, + phy.

Ok, now after reading your explanation for pt.2 below, I'm starting to follow.

Quoted 9 lines

As for pt. 2. --
Quoted 1 line
"currently", so there's possibility, in future this won't hold?
BTW: I am also sharing rx/tx buffer descriptors for dma.
When do you expect that u-boot will be able to use several MACs in one time?

It's not a matter of when, but -- write a correct code, it's much less burden to fix it later.

[...]

Quoted 6 lines

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
drivers/net/xilinx_axi_emac.c line 287
Quoted 3 lines
drivers/net/xilinx_axi_emac.c new file +2 -0
+	/* Write new speed setting out to Axi Ethernet */
+	aximac_out32(dev->iobase, XAE_EMMC_OFFSET, emmc_reg);
Use clrsetbits() here.

Quoted 6 lines

Quoted 4 lines
Quoted 2 lines
Not defined for Microblaze - just for ARM/PPC.
Not going to use it.
Please fix then. You're the microblaze maintainer, right?
Custodian.

Oh come on ...

Quoted 1 line

But I won't do that.

I think you should.

Quoted 2 lines

If you think that all archs should have it then move it to generic location which clean code duplication and I will include it.

That's not the point, it's platform specific.

Quoted 3 lines

Thanks,
Michal

Cheers

Michal Simek

author

2011-09-01 08:55 UTC #7 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 38 lines

Quoted 13 lines

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

+
+/* 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

Quoted 2 lines

*/ +#define XAE_MDIO_MRD_OFFSET 0x0000050C /* MII Management Read
Data */

Please use struct xae_regs {...} as the rest of the u-boot.

Quoted 3 lines

Quoted 2 lines
struct is not useful here because it will be big with a lot of reserved values.
I see like 10 registers here, you can use "uint32_t reserved[n];"

+ UAW0 and UAW1 with 0x700 offset.

struct axi_ethernet {
	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 */
};

Quoted 4 lines

Quoted 2 lines
Are you really sure that this is nice?
Yep, definitelly great.

ok.

Quoted 25 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
drivers/net/xilinx_axi_emac.c
Quoted 5 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+
+/* Link setup */
+#define XAE_EMMC_LINKSPEED_MASK	0xC0000000 /* Link speed */
+#define XAE_EMMC_LINKSPD_10	0x00000000 /* Link Speed mask for 10 
Mbit
Quoted 10 lines
Quoted 7 lines
Quoted 6 lines
Quoted 4 lines
Quoted 3 lines
*/ +#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.
It's hard to read, really.
At offset 40b40410. It is really hard to decode it if values are (1 << n). With defined macro you directly see that this is on 100Mbit/s LAN.
Quoted 8 lines
U-Boot-mONStR> md 40b40400
40b40400: ddccbbaa 6800ffee 4a000000 60000000    .......h...J...`
40b40410: 40000000 00000000 000ce000 000ce000    ... at ............
40b40420: 000ce000 000ce000 000ce000 000ce000    ................
40b40430: 000ce000 000ce000 000ce000 000ce000    ................
I am fine to use 1 << n solution but definitely in our repo I will use in way I like.

Quoted 7 lines

Well I see it both ways ... 0x40000000 == 1 << 30 ... it's the same thing. On the other note, it's hard to count the zeroes in there AND you can mistake 0 and 8 in a huge series of those.
Also, you can have whatever you want in your repo if you seriously care to invest the energy into maintaining it just because you need to be stubborn. But it'd really be great if you invested that energy in a more productive manner ;-)

There are two points of view. And both have con & pro.
I don't want to argue. Net custodian should decided if is OK or not.

Look at tsec.h and probably others.

Quoted 35 lines

Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
drivers/net/xilinx_axi_emac.c new file +4 -0
+/* 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..
Yep, 1 << 3 and 1 << 7.
Quoted 16 lines
Quoted 16 lines
Quoted 16 lines
Quoted 4 lines
drivers/net/xilinx_axi_emac.c line 102
Quoted 3 lines
drivers/net/xilinx_axi_emac.c new file +3 -0
+#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
Quoted 1 line
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.
Quoted 4 lines
Quoted 2 lines
Ok, I don't think I understand pt. 1. -- you need to keep the state for each of the ethernet devices on the FPGA separate, don't you.
Just the whole private structure with addresses, + phy.
Ok, now after reading your explanation for pt.2 below, I'm starting to follow.
Quoted 2 lines
As for pt. 2. --

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
"currently", so there's possibility, in future this won't hold?
BTW: I am also sharing rx/tx buffer descriptors for dma.
When do you expect that u-boot will be able to use several MACs in one time?
It's not a matter of when, but -- write a correct code, it's much less burden to fix it later.

Agree in general.
It is always question of when. You can always do it in better way. The question is if someone will pay you for doing it in better way. If this feature is not important for us, make no sense to invest our time/money to it.

Quoted 10 lines

Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
drivers/net/xilinx_axi_emac.c line 287
Quoted 2 lines
drivers/net/xilinx_axi_emac.c new file +2 -0
+	/* 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.
Please fix then. You're the microblaze maintainer, right?
Custodian.
Oh come on ...

Quoted 8 lines

Quoted 1 line
But I won't do that.
I think you should.
Quoted 2 lines
If you think that all archs should have it then move it to generic location which clean code duplication and I will include it.
That's not the point, it's platform specific.

Just compare ppc/arm implementation and they are the same. It is even pure generic code. Adding it to microblaze is just code duplication which is also not good way to go. Then in future someone will move it to generic location.
There were a lot of examples in linux kernel and includes.

+ network driver should be platform independent which is exactly how it is written. Writing in the pure C should be fine.

Thanks,
Michal

Signature · 4 lines

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

Marek Vasut

2011-09-01 10:07 UTC #8 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

On Thursday, September 01, 2011 10:55:31 AM Michal Simek wrote:

[...]

Quoted 12 lines

Quoted 11 lines
Quoted 2 lines
I am fine to use 1 << n solution but definitely in our repo I will use in way I like.
Well I see it both ways ... 0x40000000 == 1 << 30 ... it's the same thing. On the other note, it's hard to count the zeroes in there AND you can mistake 0 and 8 in a huge series of those.
Also, you can have whatever you want in your repo if you seriously care to invest the energy into maintaining it just because you need to be stubborn. But it'd really be great if you invested that energy in a more productive manner ;-)

Quoted 4 lines

There are two points of view. And both have con & pro.
I don't want to argue. Net custodian should decided if is OK or not.
Look at tsec.h and probably others.

That something's in mainline doesn't mean it's obviously correct !

[...]

Quoted 8 lines

Quoted 8 lines
Quoted 7 lines
Quoted 2 lines
"currently", so there's possibility, in future this won't hold?
BTW: I am also sharing rx/tx buffer descriptors for dma.
When do you expect that u-boot will be able to use several MACs in one time?

Quoted 6 lines

Quoted 2 lines
It's not a matter of when, but -- write a correct code, it's much less burden to fix it later.
Agree in general.
It is always question of when. You can always do it in better way. The question is if someone will pay you for doing it in better way.

And if the code isn't accepted, they won't pay you ;-)

Quoted 3 lines

If this
feature is not important for us, make no sense to invest our time/money to it.

I guess having the code mainline is important ?

Quoted 22 lines

Quoted 22 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 5 lines
Review
Quoted 3 lines
(quoted diff) +2 -0
+	/* Write new speed setting out to Axi Ethernet */
+	aximac_out32(dev->iobase, XAE_EMMC_OFFSET, emmc_reg);
Use clrsetbits() here.
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
Not defined for Microblaze - just for ARM/PPC.
Not going to use it.
Please fix then. You're the microblaze maintainer, right?
Custodian.
Oh come on ...
Quoted 1 line
But I won't do that.
I think you should.
Quoted 2 lines
If you think that all archs should have it then move it to generic location which clean code duplication and I will include it.

Quoted 7 lines

Quoted 1 line
That's not the point, it's platform specific.
Just compare ppc/arm implementation and they are the same. It is even pure generic code. Adding it to microblaze is just code duplication which is also not good way to go. Then in future someone will move it to generic location.
There were a lot of examples in linux kernel and includes.

Ok, if it's a generic code, please submit a patch putting it into a generic place and the fix this driver to use it.

Quoted 3 lines

+ network driver should be platform independent which is exactly how it is written. Writing in the pure C should be fine.

Well there's the issue with register access and endianity. Even Linux has trouble with that.

Cheers

Quoted 3 lines

Thanks,
Michal

Michal Simek

author

2011-09-01 11:04 UTC #9 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 14 lines · Marek Vasut

On Thursday, September 01, 2011 10:55:31 AM Michal Simek wrote:
[...]
Quoted 10 lines
Quoted 10 lines
Quoted 2 lines
I am fine to use 1 << n solution but definitely in our repo I will use in way I like.
Well I see it both ways ... 0x40000000 == 1 << 30 ... it's the same thing. On the other note, it's hard to count the zeroes in there AND you can mistake 0 and 8 in a huge series of those.
Also, you can have whatever you want in your repo if you seriously care to invest the energy into maintaining it just because you need to be stubborn. But it'd really be great if you invested that energy in a more productive manner ;-)

Quoted 7 lines

Quoted 4 lines
There are two points of view. And both have con & pro.
I don't want to argue. Net custodian should decided if is OK or not.
Look at tsec.h and probably others.
That something's in mainline doesn't mean it's obviously correct !

Make no sense to argue with you. Two equal solutions.

Quoted 5 lines

Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
Quoted 1 line
"currently", so there's possibility, in future this won't hold?
BTW: I am also sharing rx/tx buffer descriptors for dma.
When do you expect that u-boot will be able to use several MACs in one time?

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
It's not a matter of when, but -- write a correct code, it's much less burden to fix it later.
Agree in general.
It is always question of when. You can always do it in better way. The question is if someone will pay you for doing it in better way.
And if the code isn't accepted, they won't pay you ;-)

I don't need to add these drivers to mainline. For me is much easier to keep it just for me and don't care about others.

Quoted 6 lines

Quoted 4 lines
If this
feature is not important for us, make no sense to invest our time/money to it.
I guess having the code mainline is important ?

Partially yes. It depends how often is API changed. If API change is so often it is better to be in mainline because others fixed your driver too. If not, you can easily fix your driver yourself.
If you are out of mainline, you don't need to spend your time to keep it alive on the latest version. Most of our customers don't need u-boot for final products too.

Quoted 15 lines

Quoted 15 lines
Quoted 15 lines
Quoted 7 lines
Quoted 6 lines
Quoted 5 lines
Quoted 3 lines
Review
Quoted 2 lines
(quoted diff) +2 -0
+	/* 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.
Please fix then. You're the microblaze maintainer, right?
Custodian.
Oh come on ...
Quoted 1 line
But I won't do that.
I think you should.
Quoted 2 lines
If you think that all archs should have it then move it to generic location which clean code duplication and I will include it.
That's not the point, it's platform specific.

Quoted 8 lines

Quoted 5 lines
Just compare ppc/arm implementation and they are the same. It is even pure generic code. Adding it to microblaze is just code duplication which is also not good way to go. Then in future someone will move it to generic location.
There were a lot of examples in linux kernel and includes.
Ok, if it's a generic code, please submit a patch putting it into a generic place and the fix this driver to use it.

It it the same point to you.

Michal

Signature · 4 lines

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

Wolfgang Denk

2011-09-01 12:47 UTC #10 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

In reply to Michal Simek

Dear Michal Simek,

Quoted 11 lines · In message <[email protected]> you wrote:

Quoted 8 lines
Well I see it both ways ... 0x40000000 == 1 << 30 ... it's the same thing. On the other note, it's hard to count the zeroes in there AND you can mistake 0 and 8 in a huge series of those.
Also, you can have whatever you want in your repo if you seriously care to invest the energy into maintaining it just because you need to be stubborn. But it'd really be great if you invested that energy in a more productive manner ;-)
There are two points of view. And both have con & pro.
I don't want to argue. Net custodian should decided if is OK or not.

I can parse 0x00200000 much faster than '1 << 21' so my personal preference for such masks is the raw hex numbers.

But this is just that: a personal preference. I see no problem with using either one or the other form, and I have no really strong preferences. In any case this should not be a reason for serious fights - it's a matter of taste issue, not more.

Best regards,

Wolfgang Denk

Signature · 5 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: [email protected]
A witty saying proves nothing, but saying  something  pointless  gets
people's attention.

Wolfgang Denk

2011-08-31 20:13 UTC #11 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

In reply to Michal Simek

Dear Michal Simek,

Quoted 14 lines · In message <[email protected]> you wrote:

Quoted 13 lines

drivers/net/xilinx_axi_emac.c line 42

Quoted 11 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

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

Quoted 1 line

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

Code based on base address + offset notation will not be accepted. Please do use a struct as requested.

Best regards,

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If programming was easy, they wouldn't need something as  complicated
as a human being to do it, now would they?
                       - L. Wall & R. L. Schwartz, _Programming Perl_

Mike Frysinger

2011-08-31 19:24 UTC #12 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

In reply to Michal Simek

Quoted 6 lines · On Tuesday, August 30, 2011 08:05:19 Michal Simek wrote:

drivers/net/xilinx_axi_emac.c new file +6 -0

+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);

drivers/net/xilinx_axi_emac.c line 407

Quoted 10 lines

drivers/net/xilinx_axi_emac.c new file +10 -0

+
+	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);
+}
+
+static int axiemac_init(struct eth_device *dev, bd_t * bis)
+{
+	setup_mac(dev);

pretty sure this should be dev->write_hwaddr

Quoted 2 lines

+int xilinx_axiemac_initialize(bd_t *bis, unsigned long base_addr, int dma_addr)

you got base_addr right, but forgot to change dma_addr to unsigned long too ;)

otherwise it seems that Marek covered much of what i would have suggested -mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110831/9c258c03/attachment.pgp

Michal Simek

author

2011-09-01 06:52 UTC #13 lore raw

PATCH 1/1 net: axi_ethernet: Add driver to u-boot

Quoted 11 lines · Mike Frysinger

Quoted 10 lines · On Tuesday, August 30, 2011 08:05:19 Michal Simek wrote:

drivers/net/xilinx_axi_emac.c new file +10 -0

+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;

Quoted 8 lines

drivers/net/xilinx_axi_emac.c line 407
Quoted 6 lines
drivers/net/xilinx_axi_emac.c new file +6 -0
+	aximac_out32(dev->iobase, XAE_UAW1_OFFSET, val);
+}
+
+static int axiemac_init(struct eth_device *dev, bd_t * bis)
+{
+	setup_mac(dev);
pretty sure this should be dev->write_hwaddr

I expected it. The point is where dev->write_hwaddr is called.

I mean sequence is:
dev->write_hwaddr
u-boot network command
dev->init

If init reset device include setup mac then this sequence can't work. But not case for my MAC - I can use it.

Quoted 5 lines

Quoted 3 lines
+int xilinx_axiemac_initialize(bd_t *bis, unsigned long base_addr, int dma_addr)
you got base_addr right, but forgot to change dma_addr to unsigned long too ;)

Fixed.

Michal

Signature · 4 lines

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

Mike Frysinger

2011-08-31 19:26 UTC #14 lore raw

PATCH v2 1/1 net: ll_temac: Add LL TEMAC driver to u-boot

In reply to Michal Simek

drivers/net/xilinx_ll_temac.c line 204

Quoted 4 lines · On Tuesday, August 30, 2011 08:05:18 Michal Simek wrote:

drivers/net/xilinx_ll_temac.c new file +4 -0

+	volatile struct temac_reg *regs = priv->regs;
+
+	regs->lsw = (phy_addr << 5) | reg_addr;
+	regs->ctl = MIIMAI | (emac << 10);

you should still be going through read/write i/o helper funcs rather than relying on the volatile markings

the rest looks sane
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110831/8aa274b5/attachment.pgp

Marek Vasut

2011-08-31 20:01 UTC #15 lore raw

PATCH v2 1/1 net: ll_temac: Add LL TEMAC driver to u-boot

Quoted 8 lines · Mike Frysinger

drivers/net/xilinx_ll_temac.c line 204
Quoted 4 lines · On Tuesday, August 30, 2011 08:05:18 Michal Simek wrote:
drivers/net/xilinx_ll_temac.c new file +4 -0
+	volatile struct temac_reg *regs = priv->regs;
+
+	regs->lsw = (phy_addr << 5) | reg_addr;
+	regs->ctl = MIIMAI | (emac << 10);
you should still be going through read/write i/o helper funcs rather than relying on the volatile markings

Oh, I didn't comment on the TEMAC. I'll have to go through it.

Cheers

Quoted 3 lines

the rest looks sane
-mike

Michal Simek

author

2011-09-01 09:21 UTC #16 lore raw

PATCH v2 1/1 net: ll_temac: Add LL TEMAC driver to u-boot

In reply to Mike Frysinger

Quoted 8 lines · Mike Frysinger

drivers/net/xilinx_ll_temac.c line 204
Quoted 4 lines · On Tuesday, August 30, 2011 08:05:18 Michal Simek wrote:
drivers/net/xilinx_ll_temac.c new file +4 -0
+	volatile struct temac_reg *regs = priv->regs;
+
+	regs->lsw = (phy_addr << 5) | reg_addr;
+	regs->ctl = MIIMAI | (emac << 10);
you should still be going through read/write i/o helper funcs rather than relying on the volatile markings

Fixed.

Thanks,
Michal

Signature · 4 lines

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

Marek Vasut

2011-08-31 21:52 UTC #17 lore raw

PATCH v2 1/1 net: ll_temac: Add LL TEMAC driver to u-boot

In reply to Michal Simek

Quoted 60 lines · Michal Simek

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 <[email protected]>

---
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

drivers/net/Makefile +1 -0

index 819b197..4541eaf 100644
@@ -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)

drivers/net/xilinx_ll_temac.c new file +23 -0

new file mode 100644
index 0000000..878cdf2
@@ -0,0 +1,665 @@
+/*
+ * Xilinx xps_ll_temac ethernet driver for u-boot
+ *
+ * Copyright (C) 2008 - 2011 Michal Simek <[email protected]>
+ * 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>

drivers/net/xilinx_ll_temac.c line 33

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+#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 {...};

Quoted 62 lines

drivers/net/xilinx_ll_temac.c new file +62 -0

+#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 {

drivers/net/xilinx_ll_temac.c line 105

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+	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] ?

Quoted 35 lines

drivers/net/xilinx_ll_temac.c new file +20 -0

+};
+
+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))); +

drivers/net/xilinx_ll_temac.c new file +13 -0

+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;
+

drivers/net/xilinx_ll_temac.c line 150

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+	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 ?

drivers/net/xilinx_ll_temac.c line 159

Quoted 9 lines

drivers/net/xilinx_ll_temac.c new file +9 -0

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

dtto

drivers/net/xilinx_ll_temac.c line 168

Quoted 9 lines

drivers/net/xilinx_ll_temac.c new file +9 -0

+	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.

drivers/net/xilinx_ll_temac.c line 176

Quoted 8 lines

drivers/net/xilinx_ll_temac.c new file +8 -0

+}
+
+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.

Quoted 4 lines

drivers/net/xilinx_ll_temac.c new file +4 -0

+}
+
+#if defined(DEBUG) || defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
+/* undirect hostif write to ll_temac */

drivers/net/xilinx_ll_temac.c line 190

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+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()

drivers/net/xilinx_ll_temac.c

Quoted 2 lines

drivers/net/xilinx_ll_temac.c new file +2 -0
+	while (!(regs->rdy & XTE_RSE_MIIM_WR_MASK))
+		;

No endless loops

drivers/net/xilinx_ll_temac.c line 204

Quoted 12 lines

drivers/net/xilinx_ll_temac.c new file +12 -0

+}
+#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.

drivers/net/xilinx_ll_temac.c

Quoted 2 lines

drivers/net/xilinx_ll_temac.c new file +2 -0
+	while (!(regs->rdy & XTE_RSE_MIIM_RR_MASK))
+		;

no endless loops, please fix globally

drivers/net/xilinx_ll_temac.c

Quoted 1 line

drivers/net/xilinx_ll_temac.c new file +1 -0
+	return regs->lsw;

return readl() I guess ...

drivers/net/xilinx_ll_temac.c line 215

Quoted 8 lines

drivers/net/xilinx_ll_temac.c new file +8 -0

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

Quoted 75 lines

drivers/net/xilinx_ll_temac.c new file +75 -0

+
+	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:

drivers/net/xilinx_ll_temac.c line 300

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+		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.

drivers/net/xilinx_ll_temac.c line 308

Quoted 8 lines

drivers/net/xilinx_ll_temac.c new file +8 -0

+	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.

Quoted 12 lines

drivers/net/xilinx_ll_temac.c new file +12 -0

+}
+
+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;
+}

drivers/net/xilinx_ll_temac.c line 330

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+
+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.

Quoted 17 lines

drivers/net/xilinx_ll_temac.c new file +17 -0

+
+	/* 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 |

drivers/net/xilinx_ll_temac.c line 357

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+			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?

drivers/net/xilinx_ll_temac.c line 359

Quoted 2 lines

drivers/net/xilinx_ll_temac.c new file +2 -0
+
+	rx_bd.phys_buf_p = &rx_buffer[0];

rx_buffer would be sufficient here.

Quoted 31 lines

drivers/net/xilinx_ll_temac.c new file +31 -0

+
+	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 |

drivers/net/xilinx_ll_temac.c line 400

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+			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.

Quoted 14 lines

drivers/net/xilinx_ll_temac.c new file +14 -0

+
+	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);
+	}
+

drivers/net/xilinx_ll_temac.c line 424

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+	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.

Quoted 27 lines

drivers/net/xilinx_ll_temac.c new file +27 -0

+	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
+

drivers/net/xilinx_ll_temac.c line 461

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+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->tdfd);

Quoted 9 lines

drivers/net/xilinx_ll_temac.c new file +9 -0

+	for (i = 0; i < len; i++) {
+		val = *buf++;
+		ll_fifo->tdfd = val;
+	}
+
+	ll_fifo->tlf = length;
+
+	return 0;
+}

drivers/net/xilinx_ll_temac.c line 480

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -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.

Quoted 36 lines

drivers/net/xilinx_ll_temac.c new file +36 -0

+		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;

drivers/net/xilinx_ll_temac.c line 526

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+	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?

drivers/net/xilinx_ll_temac.c line 537

Quoted 11 lines

drivers/net/xilinx_ll_temac.c new file +11 -0

+	}
+
+	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.

drivers/net/xilinx_ll_temac.c line 548

Quoted 11 lines

drivers/net/xilinx_ll_temac.c new file +11 -0

+	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.

drivers/net/xilinx_ll_temac.c line 556

Quoted 8 lines

drivers/net/xilinx_ll_temac.c new file +8 -0

+	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?

Quoted 69 lines

drivers/net/xilinx_ll_temac.c new file +56 -0

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

drivers/net/xilinx_ll_temac.c new file +12 -0

+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) {

drivers/net/xilinx_ll_temac.c line 635

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+		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 :)

Quoted 35 lines

drivers/net/xilinx_ll_temac.c new file +24 -0

+	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);

drivers/net/xilinx_ll_temac.c new file +4 -0

+	priv->bus->reset = ll_temac_bus_reset;
+#endif
+	return 1;
+}

include/netdev.h +0 -0

index 26ce13d..cebae3b 100644

include/netdev.h int uec_standard_init(bd_t *bis); · line 99

Quoted 8 lines

include/netdev.h +2 -0

@@ -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.

Cheers!

Michal Simek

author

2011-09-01 11:11 UTC #18 lore raw

PATCH v2 1/1 net: ll_temac: Add LL TEMAC driver to u-boot

Quoted 74 lines · Marek Vasut

Quoted 60 lines · Michal Simek

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 <[email protected]>

---
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

drivers/net/Makefile +1 -0

index 819b197..4541eaf 100644
@@ -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)

drivers/net/xilinx_ll_temac.c new file +23 -0

new file mode 100644
index 0000000..878cdf2
@@ -0,0 +1,665 @@
+/*
+ * Xilinx xps_ll_temac ethernet driver for u-boot
+ *
+ * Copyright (C) 2008 - 2011 Michal Simek <[email protected]>
+ * 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>

drivers/net/xilinx_ll_temac.c line 33

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+#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,

Quoted 1 line

ok, this is the usual stuff -- use (1 << n) and struct temac_regs {...};

keep it till this is solved for axi enet.

Quoted 67 lines

drivers/net/xilinx_ll_temac.c new file +66 -0

+#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;

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 105
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+	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.

Quoted 40 lines

drivers/net/xilinx_ll_temac.c new file +20 -0

+};
+
+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))); +

drivers/net/xilinx_ll_temac.c new file +17 -0

+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;
+};
+

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 150
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+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.

Quoted 12 lines

drivers/net/xilinx_ll_temac.c line 159
Quoted 10 lines
drivers/net/xilinx_ll_temac.c new file +9 -0
+}
+
+static u32 mfdcr_local(u32 reg)
+{
+	u32 val = 0;
+#if defined(CONFIG_PPC)
+	mtdcr(0x00, reg);
+	val = mfdcr(0x01);
+#endif
dtto

too.

Quoted 14 lines

drivers/net/xilinx_ll_temac.c line 168
Quoted 10 lines
drivers/net/xilinx_ll_temac.c new file +9 -0
+	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.

Quoted 12 lines

drivers/net/xilinx_ll_temac.c line 176
Quoted 9 lines
drivers/net/xilinx_ll_temac.c new file +8 -0
+}
+
+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.

Quoted 17 lines

Quoted 5 lines

drivers/net/xilinx_ll_temac.c new file +4 -0

+}
+
+#if defined(DEBUG) || defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
+/* undirect hostif write to ll_temac */

drivers/net/xilinx_ll_temac.c line 190

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+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.

Quoted 5 lines

drivers/net/xilinx_ll_temac.c
Quoted 3 lines
drivers/net/xilinx_ll_temac.c new file +2 -0
+	while (!(regs->rdy & XTE_RSE_MIIM_WR_MASK))
+		;
No endless loops

Fixed.

Quoted 15 lines

drivers/net/xilinx_ll_temac.c line 204

Quoted 13 lines

drivers/net/xilinx_ll_temac.c new file +12 -0

+}
+#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

Quoted 5 lines

drivers/net/xilinx_ll_temac.c
Quoted 3 lines
drivers/net/xilinx_ll_temac.c new file +2 -0
+	while (!(regs->rdy & XTE_RSE_MIIM_RR_MASK))
+		;
no endless loops, please fix globally

as above

Quoted 4 lines

drivers/net/xilinx_ll_temac.c
Quoted 2 lines
drivers/net/xilinx_ll_temac.c new file +1 -0
+	return regs->lsw;
return readl() I guess ...

as above

Quoted 11 lines

drivers/net/xilinx_ll_temac.c line 215

Quoted 9 lines

drivers/net/xilinx_ll_temac.c new file +8 -0

+}
+
+/* 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

Quoted 89 lines

Quoted 76 lines

drivers/net/xilinx_ll_temac.c new file +75 -0

+
+	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:

drivers/net/xilinx_ll_temac.c line 300

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+		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 ...

Quoted 6 lines

temp = xps_ll...();
temp &= ~XTE...;
temp |= speed;
xps_ll...();
Might be a bit more readable.

fixed.

Quoted 12 lines

drivers/net/xilinx_ll_temac.c line 308
Quoted 9 lines
drivers/net/xilinx_ll_temac.c new file +8 -0
+	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.

Quoted 17 lines

drivers/net/xilinx_ll_temac.c new file +16 -0

+}
+
+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;

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 330
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+	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.

Quoted 22 lines

drivers/net/xilinx_ll_temac.c new file +21 -0

+
+	/* 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);
+}
+

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 357
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+/* 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.

Quoted 5 lines

drivers/net/xilinx_ll_temac.c line 359
Quoted 3 lines
drivers/net/xilinx_ll_temac.c new file +2 -0
+
+	rx_bd.phys_buf_p = &rx_buffer[0];
rx_buffer would be sufficient here.

fix for tx_buffer too.

Quoted 62 lines

Quoted 32 lines

drivers/net/xilinx_ll_temac.c new file +31 -0

+
+	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 |

drivers/net/xilinx_ll_temac.c line 400

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+			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.

Quoted 18 lines

drivers/net/xilinx_ll_temac.c new file +18 -0

+
+	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;

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 424
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -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.

Quoted 41 lines

Quoted 28 lines

drivers/net/xilinx_ll_temac.c new file +27 -0

+	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
+

drivers/net/xilinx_ll_temac.c line 461

Quoted 10 lines

drivers/net/xilinx_ll_temac.c new file +10 -0

+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 ?

Quoted 2 lines

for (i = 0; i < length; i +=4 )
	writel(*buf++, ll_fifo->tdf

d);

fixed.

Quoted 14 lines

drivers/net/xilinx_ll_temac.c new file +13 -0

+	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;

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 480
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+	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.

Quoted 41 lines

drivers/net/xilinx_ll_temac.c new file +40 -0

+		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);

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 526
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+		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.

Quoted 14 lines

drivers/net/xilinx_ll_temac.c line 537

Quoted 12 lines

drivers/net/xilinx_ll_temac.c new file +11 -0

+	}
+
+	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.

Quoted 15 lines

drivers/net/xilinx_ll_temac.c line 548

Quoted 12 lines

drivers/net/xilinx_ll_temac.c new file +11 -0

+	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.

Quoted 10 lines

drivers/net/xilinx_ll_temac.c line 556

Quoted 8 lines

drivers/net/xilinx_ll_temac.c new file +8 -0

+	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.

Quoted 74 lines

drivers/net/xilinx_ll_temac.c new file +56 -0

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

drivers/net/xilinx_ll_temac.c new file +16 -0

+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;
+	}
+

Quoted 8 lines

drivers/net/xilinx_ll_temac.c line 635
Quoted 6 lines
drivers/net/xilinx_ll_temac.c new file +6 -0
+	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.

Quoted 39 lines

drivers/net/xilinx_ll_temac.c new file +24 -0

+	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);

drivers/net/xilinx_ll_temac.c new file +4 -0

+	priv->bus->reset = ll_temac_bus_reset;
+#endif
+	return 1;
+}

include/netdev.h +0 -0

index 26ce13d..cebae3b 100644
@@ -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);

Quoted 8 lines

include/netdev.h int uec_standard_init(bd_t *bis); · line 99
Quoted 5 lines
include/netdev.h +2 -0
+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

Signature · 4 lines

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