Marek Vasut wrote:
On Wednesday, August 31, 2011 04:46:22 PM Michal Simek wrote:
Marek Vasut wrote:
On Tuesday, August 30, 2011 02:05:19 PM Michal Simek wrote:
Add the first axi_ethernet driver for little-endian Microblaze.
Signed-off-by: Michal Simek monstr@monstr.eu
drivers/net/Makefile | 1 + drivers/net/xilinx_axi_emac.c | 622
+++++++++++++++++++++++++++++++++++++++++ include/netdev.h |
1 +
3 files changed, 624 insertions(+), 0 deletions(-) create mode 100644 drivers/net/xilinx_axi_emac.c
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 4541eaf..ae9d4cb 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -83,6 +83,7 @@ COBJS-$(CONFIG_TSEC_ENET) += tsec.o fsl_mdio.o
COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o COBJS-$(CONFIG_ULI526X) += uli526x.o COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o
+COBJS-$(CONFIG_XILINX_AXIEMAC) += xilinx_axi_emac.o
COBJS-$(CONFIG_XILINX_EMACLITE) += xilinx_emaclite.o COBJS-$(CONFIG_XILINX_LL_TEMAC) += xilinx_ll_temac.o
diff --git a/drivers/net/xilinx_axi_emac.c b/drivers/net/xilinx_axi_emac.c new file mode 100644 index 0000000..ce79b80 --- /dev/null +++ b/drivers/net/xilinx_axi_emac.c @@ -0,0 +1,622 @@ +/*
- Copyright (C) 2011 Michal Simek monstr@monstr.eu
- Copyright (C) 2011 PetaLogix
- Copyright (C) 2010 Xilinx, Inc. All rights reserved.
- See file CREDITS for list of people who contributed to this
- project.
- This program is free software; you can redistribute it and/or
- modify it under the terms of the GNU General Public License as
- published by the Free Software Foundation; either version 2 of
- the License, or (at your option) any later version.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
- MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- GNU General Public License for more details.
- You should have received a copy of the GNU General Public License
- along with this program; if not, write to the Free Software
- Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- MA 02111-1307 USA
- */
+#include <config.h> +#include <common.h> +#include <net.h> +#include <malloc.h> +#include <asm/io.h> +#include <phy.h> +#include <miiphy.h>
+/* Axi Ethernet registers offset */ +#define XAE_IS_OFFSET 0x0000000C /* Interrupt status */ +#define XAE_IE_OFFSET 0x00000014 /* Interrupt enable */ +#define XAE_RCW1_OFFSET 0x00000404 /* Rx Configuration Word 1 */ +#define XAE_TC_OFFSET 0x00000408 /* Tx Configuration */ +#define XAE_EMMC_OFFSET 0x00000410 /* EMAC mode configuration */ +#define XAE_MDIO_MC_OFFSET 0x00000500 /* MII Management Config */ +#define XAE_MDIO_MCR_OFFSET 0x00000504 /* MII Management Control */ +#define XAE_MDIO_MWD_OFFSET 0x00000508 /* MII Management Write Data */ +#define XAE_MDIO_MRD_OFFSET 0x0000050C /* MII Management Read Data */
Please use struct xae_regs {...} as the rest of the u-boot.
struct is not useful here because it will be big with a lot of reserved values.
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?
+/* 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.
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 ...@............ 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.
+/* Interrupt Status/Enable/Mask Registers bit definitions */ +#define XAE_INT_RXRJECT_MASK 0x00000008 /* Rx frame rejected */ +#define XAE_INT_MGTRDY_MASK 0x00000080 /* MGT clock Lock */
[...]
same here..
+#define DMAALIGN 128
+static u8 RxFrame[PKTSIZE_ALIGN] __attribute((aligned(DMAALIGN))) ;
Don't use cammelcase, all lowcase please.
Agree - will be done in v2.
Also, can't you allocate this with
memalign and hide it in axidma_priv or something ?
There are two things in one.
- 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.
Just the whole private structure with addresses, + phy.
As for pt. 2. --
"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?
BTW: Looking for that memalign function - thanks.
+/* reflect dma offsets */ +struct axidma_reg {
- u32 control; /* DMACR */
- u32 status; /* DMASR */
- u32 current; /* CURDESC */
- u32 reserved;
- u32 tail; /* TAILDESC */
+};
+/* Private driver structures */ +struct axidma_priv {
- struct axidma_reg *dmatx;
- struct axidma_reg *dmarx;
- int phyaddr;
- struct phy_device *phydev;
- struct mii_dev *bus;
+};
+/* BD descriptors */ +struct axidma_bd {
- u32 next; /* Next descriptor pointer */
- u32 reserved1;
- u32 phys; /* Buffer address */
- u32 reserved2;
- u32 reserved3;
- u32 reserved4;
- u32 cntrl; /* Control */
- u32 status; /* Status */
- u32 app0;
- u32 app1; /* TX start << 16 | insert */
- u32 app2; /* TX csum seed */
- u32 app3;
- u32 app4;
- u32 sw_id_offset;
- u32 reserved5;
- u32 reserved6;
+};
+/* Static BDs - driver uses only one BD */ +static struct axidma_bd tx_bd __attribute((aligned(DMAALIGN))); +static struct axidma_bd rx_bd __attribute((aligned(DMAALIGN)));
+static inline void aximac_out32(u32 addr, u32 offset, u32 val) +{
- out_be32((u32 *)(addr + offset), val);
Please fix these casts ... though I don't think you even need these functions.
Cast is necessary. I use that helper just because of recast. If you see solution which will be elegant, I am open to use it.
See above -- use the structure and then just use &axiregs->reg.
+}
+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
[...]
- /* 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