Hi Ben,
thanks for your review!
El Tue, Jan 19, 2010 at 01:30:21PM -0800 Ben Warren ha dit:
> Matthias Kaehlcke wrote:
>> Added ethernet driver for EP93xx SoCs
>>
>> Signed-off-by: Matthias Kaehlcke <matth...@kaehlcke.net>
>> ---
>> drivers/net/Makefile | 1 +
>> drivers/net/ep93xx.c | 658
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> drivers/net/ep93xx.h | 145 +++++++++++
>> include/common.h | 8 +-
>> include/netdev.h | 1 +
>> 5 files changed, 812 insertions(+), 1 deletions(-)
>> create mode 100644 drivers/net/ep93xx.c
>> create mode 100644 drivers/net/ep93xx.h
>>
>> diff --git a/drivers/net/Makefile b/drivers/net/Makefile
>> index 904727e..0694133 100644
>> --- a/drivers/net/Makefile
>> +++ b/drivers/net/Makefile
>> @@ -37,6 +37,7 @@ COBJS-$(CONFIG_DNET) += dnet.o
>> COBJS-$(CONFIG_E1000) += e1000.o
>> COBJS-$(CONFIG_EEPRO100) += eepro100.o
>> COBJS-$(CONFIG_ENC28J60) += enc28j60.o
>> +COBJS-$(CONFIG_EP93XX) += ep93xx.o
>> COBJS-$(CONFIG_FEC_MXC) += fec_mxc.o
>> COBJS-$(CONFIG_FSLDMAFEC) += fsl_mcdmafec.o mcfmii.o
>> COBJS-$(CONFIG_FTMAC100) += ftmac100.o
>> diff --git a/drivers/net/ep93xx.c b/drivers/net/ep93xx.c
>> new file mode 100644
>> index 0000000..150f7f8
>> --- /dev/null
>> +++ b/drivers/net/ep93xx.c
>> @@ -0,0 +1,658 @@
>> +/*
>> + * Cirrus Logic EP93xx ethernet MAC / MII driver.
>> + *
>> + * Copyright (C) 2009 Matthias Kaehlcke <matth...@kaehlcke.net>
>> + *
>> + * Copyright (C) 2004, 2005
>> + * Cory T. Tusar, Videon Central, Inc., <ctu...@videon-central.com>
>> + *
>> + * Based on the original eth.[ch] Cirrus Logic EP93xx Rev D. Ethernet
>> Driver,
>> + * which is
>> + *
>> + * (C) Copyright 2002 2003
>> + * Adam Bezanson, Network Audio Technologies, Inc.
>> + * <bezan...@netaudiotech.com>
>> + *
>> + * 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.,
>> + * 675 Mass Ave, Cambridge, MA 02139, USA.
>> + */
>> +#include <command.h>
>> +#include <common.h>
>> +#include <asm/arch/ep93xx.h>
>> +#include <asm/io.h>
>> +#include <malloc.h>
>> +#include <miiphy.h>
>> +#include <linux/types.h>
>> +#include "ep93xx.h"
>>
> If the EP93XX is an SOC, you should name this driver something with
> extra namespace, e.g. (ep93xx_mac) or whatever the documentation calls
> it. The fact that you're including two files with the same name is a
> bad sign.
ok
>> +
>> +static int ep93xx_eth_send_packet(struct eth_device *dev,
>> + volatile void * const packet, int const length);
>> +static int ep93xx_eth_rcv_packet(struct eth_device *dev);
>> +
>> +/*
>> ----------------------------------------------------------------------------
>> + * EP93xx ethernet MAC functionality
>> + */
>> +#if defined(CONFIG_DRIVER_EP93XX_MAC)
>>
> What's this for?
you're right, this should have been removed
>> +
>> +/* ep93xx_miiphy ops forward declarations */
>> +static int ep93xx_miiphy_read(char * const dev, unsigned char const addr,
>> + unsigned char const reg, unsigned short * const value);
>> +static int ep93xx_miiphy_write(char * const dev, unsigned char const addr,
>> + unsigned char const reg, unsigned short const value);
>> +
>> +/* Reserve memory for the MAC's private data */
>> +static struct ep93xx_mac ep93xx_mac = { 0 };
>>
> Each instance of the driver should have its own private structure. In
> other words, malloc() at runtime.
ok
>> +
>> +#if defined(EP93XX_MAC_DEBUG)
>> +/**
>> + * Dump ep93xx_mac values to the terminal.
>> + */
>> +inline void dump_dev(void)
>> +{
>> + int i;
>> +
>> + printf("\ndump_dev()\n");
>> + printf(" is_initialized %02X\n", ep93xx_mac.is_initialized);
>> + printf(" rx_dq.base %08X\n", ep93xx_mac.rx_dq.base);
>> + printf(" rx_dq.current %08X\n", ep93xx_mac.rx_dq.current);
>> + printf(" rx_dq.end %08X\n", ep93xx_mac.rx_dq.end);
>> + printf(" rx_sq.base %08X\n", ep93xx_mac.rx_sq.base);
>> + printf(" rx_sq.current %08X\n", ep93xx_mac.rx_sq.current);
>> + printf(" rx_sq.end %08X\n", ep93xx_mac.rx_sq.end);
>> +
>> + for (i = 0; i < NUMRXDESC; i++)
>> + printf(" rx_buffer[%2.d] %08X\n", i, NetRxPackets[i]);
>> +
>> + printf(" tx_dq.base %08X\n", ep93xx_mac.tx_dq.base);
>> + printf(" tx_dq.current %08X\n", ep93xx_mac.tx_dq.current);
>> + printf(" tx_dq.end %08X\n", ep93xx_mac.tx_dq.end);
>> + printf(" tx_sq.base %08X\n", ep93xx_mac.tx_sq.base);
>> + printf(" tx_sq.current %08X\n", ep93xx_mac.tx_sq.current);
>> + printf(" tx_sq.end %08X\n", ep93xx_mac.tx_sq.end);
>> +}
>>
> Pass in eth_device and dereference private data instead of using global.
> You get the idea...
ok
>> +
>> +/**
>> + * Dump all RX status queue entries to the terminal.
>> + */
>> +inline void dump_rx_status_queue(void)
>> +{
>> + int i;
>> +
>> + printf("\ndump_rx_status_queue()\n");
>> + printf(" descriptor address word1 word2\n");
>> + for (i = 0; i < NUMRXDESC; i++) {
>> + printf(" [ %08X ] %08X %08X\n",
>> + (ep93xx_mac.rx_sq.base + i),
>> + (ep93xx_mac.rx_sq.base + i)->word1,
>> + (ep93xx_mac.rx_sq.base + i)->word2);
>> + }
>> +}
>> +
>> +/**
>> + * Dump all RX descriptor queue entries to the terminal.
>> + */
>> +inline void dump_rx_descriptor_queue(void)
>> +{
>> + int i;
>> +
>> + printf("\ndump_rx_descriptor_queue()\n");
>> + printf(" descriptor address word1 word2\n");
>> + for (i = 0; i < NUMRXDESC; i++) {
>> + printf(" [ %08X ] %08X %08X\n",
>> + (ep93xx_mac.rx_dq.base + i),
>> + (ep93xx_mac.rx_dq.base + i)->word1,
>> + (ep93xx_mac.rx_dq.base + i)->word2);
>> + }
>> +}
>> +
>> +/**
>> + * Dump all TX descriptor queue entries to the terminal.
>> + */
>> +inline void dump_tx_descriptor_queue(void)
>> +{
>> + int i;
>> +
>> + printf("\ndump_tx_descriptor_queue()\n");
>> + printf(" descriptor address word1 word2\n");
>> + for (i = 0; i < NUMTXDESC; i++) {
>> + printf(" [ %08X ] %08X %08X\n",
>> + (ep93xx_mac.tx_dq.base + i),
>> + (ep93xx_mac.tx_dq.base + i)->word1,
>> + (ep93xx_mac.tx_dq.base + i)->word2);
>> + }
>> +}
>> +
>> +/**
>> + * Dump all TX status queue entries to the terminal.
>> + */
>> +inline void dump_tx_status_queue(void)
>> +{
>> + int i;
>> +
>> + printf("\ndump_tx_status_queue()\n");
>> + printf(" descriptor address word1\n");
>> + for (i = 0; i < NUMTXDESC; i++) {
>> + printf(" [ %08X ] %08X\n",
>> + (ep93xx_mac.rx_sq.base + i),
>> + (ep93xx_mac.rx_sq.base + i)->word1);
>> + }
>> +}
>> +#else
>> +#define dump_dev(x)
>> +#define dump_rx_descriptor_queue()
>> +#define dump_rx_status_queue()
>> +#define dump_tx_descriptor_queue()
>> +#define dump_tx_status_queue()
>> +#endif /* defined(EP93XX_MAC_DEBUG) */
>> +
>> +/**
>> + * Reset the EP93xx MAC by twiddling the soft reset bit and spinning until
>> + * it's cleared.
>> + */
>> +static void ep93xx_mac_reset(void)
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>> + uint32_t value;
>> +
>> + debug("+ep93xx_mac_reset");
>> +
>> + value = readl(&mac->selfctl);
>> + value |= SELFCTL_RESET;
>> + writel(value, &mac->selfctl);
>> +
>> + while (readl(&mac->selfctl) & SELFCTL_RESET)
>> + ; /* noop */
>>
> Most people write this as 'nop', but I won't be too picky...
in the first revision of this patch set it was 'nop' and i was asked
to change it to 'noop', and so i did ;)
>> +
>> + debug("-ep93xx_mac_reset");
>> +}
>> +
>> +/* Eth device open */
>> +static int ep93xx_eth_open(struct eth_device *dev, bd_t *bd)
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>>
> Get this from dev->priv
ok
>> + uchar enetaddr[6];
>> + int i;
>> +
>> + debug("+ep93xx_eth_open");
>> +
>> + /* Reset the MAC */
>> + ep93xx_mac_reset();
>> +
>> + /* Reset the descriptor queues' current and end address values */
>> + ep93xx_mac.tx_dq.current = ep93xx_mac.tx_dq.base;
>> + ep93xx_mac.tx_dq.end = (ep93xx_mac.tx_dq.base + NUMTXDESC);
>> +
>> + ep93xx_mac.tx_sq.current = ep93xx_mac.tx_sq.base;
>> + ep93xx_mac.tx_sq.end = (ep93xx_mac.tx_sq.base + NUMTXDESC);
>> +
>> + ep93xx_mac.rx_dq.current = ep93xx_mac.rx_dq.base;
>> + ep93xx_mac.rx_dq.end = (ep93xx_mac.rx_dq.base + NUMRXDESC);
>> +
>> + ep93xx_mac.rx_sq.current = ep93xx_mac.rx_sq.base;
>> + ep93xx_mac.rx_sq.end = (ep93xx_mac.rx_sq.base + NUMRXDESC);
>> +
>> + /*
>> + * Set the transmit descriptor and status queues' base address,
>> + * current address, and length registers. Set the maximum frame
>> + * length and threshold. Enable the transmit descriptor processor.
>> + */
>> + writel((uint32_t)ep93xx_mac.tx_dq.base, &mac->txdq.badd);
>> + writel((uint32_t)ep93xx_mac.tx_dq.base, &mac->txdq.curadd);
>> + writel(sizeof(struct tx_descriptor) * NUMTXDESC, &mac->txdq.blen);
>> +
>> + writel((uint32_t)ep93xx_mac.tx_sq.base, &mac->txstsq.badd);
>> + writel((uint32_t)ep93xx_mac.tx_sq.base, &mac->txstsq.curadd);
>> + writel(sizeof(struct tx_status) * NUMTXDESC, &mac->txstsq.blen);
>> +
>> + writel(0x00040000, &mac->txdthrshld);
>> + writel(0x00040000, &mac->txststhrshld);
>> +
>> + writel((TXSTARTMAX << 0) | (PKTSIZE_ALIGN << 16), &mac->maxfrmlen);
>> + writel(BMCTL_TXEN, &mac->bmctl);
>> +
>> + /*
>> + * Set the receive descriptor and status queues' base address,
>> + * current address, and length registers. Enable the receive
>> + * descriptor processor.
>> + */
>> + writel((uint32_t)ep93xx_mac.rx_dq.base, &mac->rxdq.badd);
>> + writel((uint32_t)ep93xx_mac.rx_dq.base, &mac->rxdq.curadd);
>> + writel(sizeof(struct rx_descriptor) * NUMRXDESC, &mac->rxdq.blen);
>> +
>> + writel((uint32_t)ep93xx_mac.rx_sq.base, &mac->rxstsq.badd);
>> + writel((uint32_t)ep93xx_mac.rx_sq.base, &mac->rxstsq.curadd);
>> + writel(sizeof(struct rx_status) * NUMRXDESC, &mac->rxstsq.blen);
>> +
>> + writel(0x00040000, &mac->rxdthrshld);
>> +
>> + writel(BMCTL_RXEN, &mac->bmctl);
>> +
>> + writel(0x00040000, &mac->rxststhrshld);
>> +
>> + /* Wait until the receive descriptor processor is active */
>> + while (!(readl(&mac->bmsts) & BMSTS_RXACT))
>> + ; /* noop */
>> +
>> + /*
>> + * Initialize the RX descriptor queue. Clear the TX descriptor queue.
>> + * Clear the RX and TX status queues. Enqueue the RX descriptor and
>> + * status entries to the MAC.
>> + */
>> + for (i = 0; i < NUMRXDESC; i++) {
>> + /* set buffer address */
>> + (ep93xx_mac.rx_dq.base + i)->word1 = (uint32_t)NetRxPackets[i];
>> +
>> + /* set buffer length, clear buffer index and NSOF */
>> + (ep93xx_mac.rx_dq.base + i)->word2 = PKTSIZE_ALIGN;
>> + }
>> +
>> + memset(ep93xx_mac.tx_dq.base, 0,
>> + (sizeof(struct tx_descriptor) * NUMTXDESC));
>> + memset(ep93xx_mac.rx_sq.base, 0,
>> + (sizeof(struct rx_status) * NUMRXDESC));
>> + memset(ep93xx_mac.tx_sq.base, 0,
>> + (sizeof(struct tx_status) * NUMTXDESC));
>> +
>> + writel(NUMRXDESC, &mac->rxdqenq);
>> + writel(NUMRXDESC, &mac->rxstsqenq);
>> +
>> + /* Set the primary MAC address */
>> + writel(AFP_IAPRIMARY, &mac->afp);
>> + eth_getenv_enetaddr("ethaddr", enetaddr);
>>
> Get this from dev->enetaddr.
ok
>> + writel(enetaddr[0] | (enetaddr[1] << 8) |
>> + (enetaddr[2] << 16) | (enetaddr[3] << 24),
>> + &mac->indad);
>> + writel(enetaddr[4] | (enetaddr[5] << 8), &mac->indad_upper);
>> +
>> + /* Turn on RX and TX */
>> + writel(RXCTL_IA0 | RXCTL_BA | RXCTL_SRXON |
>> + RXCTL_RCRCA | RXCTL_MA, &mac->rxctl);
>> + writel(TXCTL_STXON, &mac->txctl);
>> +
>> + /* Dump data structures if we're debugging */
>> + dump_dev();
>> + dump_rx_descriptor_queue();
>> + dump_rx_status_queue();
>> + dump_tx_descriptor_queue();
>> + dump_tx_status_queue();
>> +
>> + debug("-ep93xx_eth_open");
>> +
>> + return 1;
>> +}
>> +
>> +/**
>> + * Halt EP93xx MAC transmit and receive by clearing the TxCTL and RxCTL
>> + * registers.
>> + */
>> +static void ep93xx_eth_close(struct eth_device *dev)
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>> +
>> + debug("+ep93xx_eth_close");
>> +
>> + writel(0x00000000, &mac->rxctl);
>> + writel(0x00000000, &mac->txctl);
>> +
>> + debug("-ep93xx_eth_close");
>> +}
>> +
>> +#if defined(CONFIG_MII)
>> +int ep93xx_miiphy_initialize(bd_t * const bd)
>> +{
>> + miiphy_register("ep93xx_eth0", ep93xx_miiphy_read, ep93xx_miiphy_write);
>> + return 0;
>> +}
>> +#endif
>> +
>> +/**
>> + * Initialize the EP93xx MAC. The MAC hardware is reset. Buffers are
>> + * allocated, if necessary, for the TX and RX descriptor and status queues,
>> + * as well as for received packets. The EP93XX MAC hardware is initialized.
>> + * Transmit and receive operations are enabled.
>> + */
>> +int ep93xx_eth_init(bd_t * const bd)
>>
> Please call this ep93xx_eth_initialize(), as all other drivers do. It
> doesn't look like you use bd anywhere, so choose parameters that are
> more relevant. Drivers for memory-mapped devices typically pass in a
> base address and index (see cs8900.c as an example).
ok
>> +{
>> + int ret = -1;
>> + struct eth_device *dev;
>> +
>> + debug("+ep93xx_eth_init");
>> +
>> + /* Parameter check */
>> + BUG_ON(bd == NULL);
>> +
>> + /*
>> + * Allocate space for the queues and RX packet buffers if we're not
>> + * already initialized
>> + */
>> + if (!ep93xx_mac.is_initialized) {\
>>
> As I mentioned earlier, each instance of the driver needs its own
> private struct. Please malloc() and attach to dev->priv
ok
>> + ep93xx_mac.tx_dq.base = calloc(NUMTXDESC,
>> + sizeof(struct tx_descriptor));
>> + if (ep93xx_mac.tx_dq.base == NULL) {
>> + error("calloc() failed");
>> + goto eth_init_failed_0;
>> + }
>> +
>> + ep93xx_mac.tx_sq.base = calloc(NUMTXDESC,
>> + sizeof(struct tx_status));
>> + if (ep93xx_mac.tx_sq.base == NULL) {
>> + error("calloc() failed");
>> + goto eth_init_failed_1;
>> + }
>> +
>> + ep93xx_mac.rx_dq.base = calloc(NUMRXDESC,
>> + sizeof(struct rx_descriptor));
>> + if (ep93xx_mac.rx_dq.base == NULL) {
>> + error("calloc() failed");
>> + goto eth_init_failed_2;
>> + }
>> +
>> + ep93xx_mac.rx_sq.base = calloc(NUMRXDESC,
>> + sizeof(struct rx_status));
>> + if (ep93xx_mac.rx_sq.base == NULL) {
>> + error("calloc() failed");
>> + goto eth_init_failed_3;
>> + }
>> +
>> + dev = malloc(sizeof *dev);
>> + if (dev == NULL) {
>> + error("malloc() failed");
>> + goto eth_init_failed_4;
>> + }
>> +
>> + memset(dev, 0, sizeof *dev);
>> + sprintf(dev->name, "ep93xx MAC");
>> + dev->iobase = 0;
>>
> dev->iobase is a convenient place to put your base address.
ok
>> + dev->init = ep93xx_eth_open;
>> + dev->halt = ep93xx_eth_close;
>> + dev->send = ep93xx_eth_send_packet;
>> + dev->recv = ep93xx_eth_rcv_packet;
>> +
>> + eth_register(dev);
>> +
>> + /*
>> + * Set is_initialized flag so we don't go through allocation
>> + * portion of init again.
>> + */
>> + ep93xx_mac.is_initialized = 1;
>>
> You probably don't need this.
ok
>> + }
>> +
>> + /* Done! */
>> + ret = 0;
>> + goto eth_init_done;
>> +
>> +eth_init_failed_4:
>> + free(ep93xx_mac.rx_sq.base);
>> + /* Fall through */
>> +
>> +eth_init_failed_3:
>> + free(ep93xx_mac.rx_dq.base);
>> + /* Fall through */
>> +
>> +eth_init_failed_2:
>> + free(ep93xx_mac.tx_sq.base);
>> + /* Fall through */
>> +
>> +eth_init_failed_1:
>> + free(ep93xx_mac.tx_dq.base);
>> + /* Fall through */
>> +
>> +eth_init_failed_0:
>> + /* Fall through */
>> +
>> +eth_init_done:
>> + debug("-ep93xx_eth_init %d", ret);
>> + return ret;
>> +}
>> +
>> +/**
>> + * Copy a frame of data from the MAC into the protocol layer for further
>> + * processing.
>> + */
>> +static int ep93xx_eth_rcv_packet(struct eth_device *dev)
>>
> Why not move this up higher to avoid the forward declarations?
ok
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>> + int len = -1;
>> +
>> + debug("+ep93xx_eth_rcv_packet");
>> +
>> + if (RX_STATUS_RFP(ep93xx_mac.rx_sq.current)) {
>> + if (RX_STATUS_RWE(ep93xx_mac.rx_sq.current)) {
>> + /*
>> + * We have a good frame. Extract the frame's length
>> + * from the current rx_status_queue entry, and copy
>> + * the frame's data into NetRxPackets[] of the
>> + * protocol stack. We track the total number of
>> + * bytes in the frame (nbytes_frame) which will be
>> + * used when we pass the data off to the protocol
>> + * layer via NetReceive().
>> + */
>> + len = RX_STATUS_FRAME_LEN(ep93xx_mac.rx_sq.current);
>> +
>> + NetReceive((uchar *)ep93xx_mac.rx_dq.current->word1,
>> + len);
>> +
>> + debug("reporting %d bytes...\n", len);
>> + } else {
>> + /* Do we have an erroneous packet? */
>> + error("packet rx error, status %08X %08X",
>> + ep93xx_mac.rx_sq.current->word1,
>> + ep93xx_mac.rx_sq.current->word2);
>> + dump_rx_descriptor_queue();
>> + dump_rx_status_queue();
>> + }
>> +
>> + /*
>> + * Clear the associated status queue entry, and
>> + * increment our current pointers to the next RX
>> + * descriptor and status queue entries (making sure
>> + * we wrap properly).
>> + */
>> + memset((void *)ep93xx_mac.rx_sq.current, 0,
>> + sizeof(struct rx_status));
>> +
>> + ep93xx_mac.rx_sq.current++;
>> + if (ep93xx_mac.rx_sq.current >= ep93xx_mac.rx_sq.end)
>> + ep93xx_mac.rx_sq.current = ep93xx_mac.rx_sq.base;
>> +
>> + ep93xx_mac.rx_dq.current++;
>> + if (ep93xx_mac.rx_dq.current >= ep93xx_mac.rx_dq.end)
>> + ep93xx_mac.rx_dq.current = ep93xx_mac.rx_dq.base;
>> +
>> + /*
>> + * Finally, return the RX descriptor and status entries
>> + * back to the MAC engine, and loop again, checking for
>> + * more descriptors to process.
>> + */
>> + writel(1, &mac->rxdqenq);
>> + writel(1, &mac->rxstsqenq);
>> + } else {
>> + len = 0;
>> + }
>> +
>> + debug("-ep93xx_eth_rcv_packet %d", len);
>> + return len;
>> +}
>> +
>> +/**
>> + * Send a block of data via ethernet.
>> + */
>> +static int ep93xx_eth_send_packet(struct eth_device *dev,
>> + volatile void * const packet, int const length)
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>> + int ret = -1;
>> +
>> + debug("+ep93xx_eth_send_packet");
>> +
>> + /* Parameter check */
>> + BUG_ON(packet == NULL);
>> +
>> + /*
>> + * Initialize the TX descriptor queue with the new packet's info.
>> + * Clear the associated status queue entry. Enqueue the packet
>> + * to the MAC for transmission.
>> + */
>> +
>> + /* set buffer address */
>> + ep93xx_mac.tx_dq.current->word1 = (uint32_t)packet;
>> +
>> + /* set buffer length and EOF bit */
>> + ep93xx_mac.tx_dq.current->word2 = length | TX_DESC_EOF;
>> +
>> + /* clear tx status */
>> + ep93xx_mac.tx_sq.current->word1 = 0;
>> +
>> + /* enqueue the TX descriptor */
>> + writel(1, &mac->txdqenq);
>> +
>> + /* wait for the frame to become processed */
>> + while (!TX_STATUS_TXFP(ep93xx_mac.tx_sq.current))
>> + ; /* noop */
>> +
>> + if (!TX_STATUS_TXWE(ep93xx_mac.tx_sq.current)) {
>> + error("packet tx error, status %08X",
>> + ep93xx_mac.tx_sq.current->word1);
>> + dump_tx_descriptor_queue();
>> + dump_tx_status_queue();
>> +
>> + /* TODO: Add better error handling? */
>> + goto eth_send_failed_0;
>> + }
>> +
>> + ret = 0;
>> + /* Fall through */
>> +
>> +eth_send_failed_0:
>> + debug("-ep93xx_eth_send_packet %d", ret);
>> + return ret;
>> +}
>> +#endif /* defined(CONFIG_DRIVER_EP93XX_MAC) */
>> +
>> +/*
>> -----------------------------------------------------------------------------
>> + * EP93xx ethernet MII functionality.
>> + */
>> +#if defined(CONFIG_MII)
>> +
>> +/**
>> + * Maximum MII address we support
>> + */
>> +#define MII_ADDRESS_MAX (31)
>>
> Parens not needed
ok
>> +
>> +/**
>> + * Maximum MII register address we support
>> + */
>> +#define MII_REGISTER_MAX (31)
>> +
>> +/**
>> + * Read a 16-bit value from an MII register.
>> + */
>> +static int ep93xx_miiphy_read(char * const dev, unsigned char const addr,
>> + unsigned char const reg, unsigned short * const value)
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>> + int ret = -1;
>> + uint32_t self_ctl;
>> +
>> + debug("+ep93xx_miiphy_read");
>> +
>> + /* Parameter checks */
>> + BUG_ON(dev == NULL);
>> + BUG_ON(addr > MII_ADDRESS_MAX);
>> + BUG_ON(reg > MII_REGISTER_MAX);
>> + BUG_ON(value == NULL);
>> +
>> + /*
>> + * Save the current SelfCTL register value. Set MAC to suppress
>> + * preamble bits. Wait for any previous MII command to complete
>> + * before issuing the new command.
>> + */
>> + self_ctl = readl(&mac->selfctl);
>> +#if defined(CONFIG_MII_SUPPRESS_PREAMBLE)
>> + writel(self_ctl & ~(1 << 8), &mac->selfctl);
>> +#endif /* defined(CONFIG_MII_SUPPRESS_PREAMBLE) */
>> +
>> + while (readl(&mac->miists) & MIISTS_BUSY)
>> + ; /* noop */
>> +
>> + /*
>> + * Issue the MII 'read' command. Wait for the command to complete.
>> + * Read the MII data value.
>> + */
>> + writel(MIICMD_OPCODE_READ | ((uint32_t)addr << 5) | (uint32_t)reg,
>> + &mac->miicmd);
>> + while (readl(&mac->miists) & MIISTS_BUSY)
>> + ; /* noop */
>> +
>> + *value = (unsigned short)readl(&mac->miidata);
>> +
>> + /* Restore the saved SelfCTL value and return. */
>> + writel(self_ctl, &mac->selfctl);
>> +
>> + ret = 0;
>> + /* Fall through */
>> +
>> + debug("-ep93xx_miiphy_read");
>> + return ret;
>> +}
>> +
>> +/**
>> + * Write a 16-bit value to an MII register.
>> + */
>> +static int ep93xx_miiphy_write(char * const dev, unsigned char const addr,
>> + unsigned char const reg, unsigned short const value)
>> +{
>> + struct mac_regs *mac = (struct mac_regs *)MAC_BASE;
>> + int ret = -1;
>> + uint32_t self_ctl;
>> +
>> + debug("+ep93xx_miiphy_write");
>> +
>> + /* Parameter checks */
>> + BUG_ON(dev == NULL);
>> + BUG_ON(addr > MII_ADDRESS_MAX);
>> + BUG_ON(reg > MII_REGISTER_MAX);
>> +
>> + /*
>> + * Save the current SelfCTL register value. Set MAC to suppress
>> + * preamble bits. Wait for any previous MII command to complete
>> + * before issuing the new command.
>> + */
>> + self_ctl = readl(&mac->selfctl);
>> +#if defined(CONFIG_MII_SUPPRESS_PREAMBLE)
>> + writel(self_ctl & ~(1 << 8), &mac->selfctl);
>> +#endif /* defined(CONFIG_MII_SUPPRESS_PREAMBLE) */
>> +
>> + while (readl(&mac->miists) & MIISTS_BUSY)
>> + ; /* noop */
>> +
>> + /* Issue the MII 'write' command. Wait for the command to complete. */
>> + writel((uint32_t)value, &mac->miidata);
>> + writel(MIICMD_OPCODE_WRITE | ((uint32_t)addr << 5) | (uint32_t)reg,
>> + &mac->miicmd);
>> + while (readl(&mac->miists) & MIISTS_BUSY)
>> + ; /* noop */
>> +
>> + /* Restore the saved SelfCTL value and return. */
>> + writel(self_ctl, &mac->selfctl);
>> +
>> + ret = 0;
>> + /* Fall through */
>> +
>> + debug("-ep93xx_miiphy_write");
>> + return ret;
>> +}
>> +#endif /* defined(CONFIG_MII) */
>> diff --git a/drivers/net/ep93xx.h b/drivers/net/ep93xx.h
>> new file mode 100644
>> index 0000000..ca450ed
>> --- /dev/null
>> +++ b/drivers/net/ep93xx.h
>> @@ -0,0 +1,145 @@
>> +/*
>> + * Copyright (C) 2009 Matthias Kaehlcke <matth...@kaehlcke.net>
>> + *
>> + * Copyright (C) 2004, 2005
>> + * Cory T. Tusar, Videon Central, Inc., <ctu...@videon-central.com>
>> + *
>> + * 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
>> + *
>> + */
>> +
>> +#ifndef _ETH_H
>> +#define _ETH_H
>> +
>> +#include <net.h>
>> +
>> +/**
>> + * #define this to dump device status and queue info during initialization
>> and
>> + * following errors.
>> + */
>> +#undef EP93XX_MAC_DEBUG
>> +
>> +/**
>> + * Number of descriptor and status entries in our RX queues.
>> + * It must be power of 2 !
>> + */
>> +#define NUMRXDESC PKTBUFSRX
>> +
>> +/**
>> + * Number of descriptor and status entries in our TX queues.
>> + */
>> +#define NUMTXDESC 1
>> +
>> +/**
>> + * 944 = (1024 - 64) - 16, Fifo size - Minframesize - 16 (Chip FACT)
>> + */
>> +#define TXSTARTMAX 944
>> +
>> +/**
>> + * Receive descriptor queue entry
>> + */
>> +struct rx_descriptor {
>> + uint32_t word1;
>> + uint32_t word2;
>> +} __attribute__((packed));
>>
> Why do you need to pack 32-bit integers?
you're right, that's superfluous
>> +
>> +/**
>> + * Receive status queue entry
>> + */
>> +struct rx_status {
>> + uint32_t word1;
>> + uint32_t word2;
>> +} __attribute__((packed));
>> +
>> +#define RX_STATUS_RWE(rx_status) ((rx_status->word1 >> 30) & 0x01)
>> +#define RX_STATUS_RFP(rx_status) ((rx_status->word1 >> 31) & 0x01)
>> +#define RX_STATUS_FRAME_LEN(rx_status) (rx_status->word2 & 0xFFFF)
>> +
>> +
>> +/**
>> + * Transmit descriptor queue entry
>> + */
>> +struct tx_descriptor {
>> + uint32_t word1;
>> + uint32_t word2;
>> +} __attribute__((packed));
>> +
>> +#define TX_DESC_EOF (1 << 31)
>> +
>> +/**
>> + * Transmit status queue entry
>> + */
>> +struct tx_status {
>> + uint32_t word1;
>> +} __attribute__((packed));
>> +
>> +#define TX_STATUS_TXWE(tx_status) (((tx_status)->word1 >> 30) & 0x01)
>> +#define TX_STATUS_TXFP(tx_status) (((tx_status)->word1 >> 31) & 0x01)
>> +
>> +/**
>> + * Transmit descriptor queue
>> + */
>> +struct tx_descriptor_queue {
>> + struct tx_descriptor *base;
>> + struct tx_descriptor *current;
>> + struct tx_descriptor *end;
>> +};
>> +
>> +/**
>> + * Transmit status queue
>> + */
>> +struct tx_status_queue {
>> + struct tx_status *base;
>> + volatile struct tx_status *current;
>> + struct tx_status *end;
>> +};
>> +
>> +/**
>> + * Receive descriptor queue
>> + */
>> +struct rx_descriptor_queue {
>> + struct rx_descriptor *base;
>> + struct rx_descriptor *current;
>> + struct rx_descriptor *end;
>> +};
>> +
>> +/**
>> + * Receive status queue
>> + */
>> +struct rx_status_queue {
>> + struct rx_status *base;
>> + volatile struct rx_status *current;
>> + struct rx_status *end;
>> +};
>> +
>> +/**
>> + * EP93xx MAC private data structure
>> + */
>> +struct ep93xx_mac {
>> + int is_initialized;
>> +
>> + struct rx_descriptor_queue rx_dq;
>> + struct rx_status_queue rx_sq;
>> + void *rx_buffer[NUMRXDESC];
>> +
>> + struct tx_descriptor_queue tx_dq;
>> + struct tx_status_queue tx_sq;
>> +};
>> +
>> +#endif
>> diff --git a/include/common.h b/include/common.h
>> index 391790a..c0dfc45 100644
>> --- a/include/common.h
>> +++ b/include/common.h
>> @@ -123,6 +123,11 @@ typedef volatile unsigned char vu_char;
>> #define debugX(level,fmt,args...)
>> #endif /* DEBUG */
>> +#define error(fmt, args...) do { \
>> + printf("ERROR: " fmt "\nat %s:%d/%s()\n", \
>> + ##args, __FILE__, __LINE__, __func__); \
>> +} while (0)
>> +
>>
> Is this really needed?
before i had a private macro defined in the driver, but i was told to
use the macros in common.h instead. for the error case no such macro
existed, so i created one.
i wondered if it would be interesting to add a set of macros like
pr_debug, pr_info, ... in the linux kernel
>> #ifndef BUG
>> #define BUG() do { \
>> printf("BUG: failure at %s:%d/%s()!\n", __FILE__, __LINE__,
>> __FUNCTION__); \
>> @@ -502,7 +507,8 @@ ulong get_PCI_freq (void);
>> #endif
>> #if defined(CONFIG_S3C24X0) || \
>> defined(CONFIG_LH7A40X) || \
>> - defined(CONFIG_S3C6400)
>> + defined(CONFIG_S3C6400) || \
>> + defined(CONFIG_EP93XX)
>> ulong get_FCLK (void);
>> ulong get_HCLK (void);
>> ulong get_PCLK (void);
>>
> I don't see these referenced anywhere. How do they apply to this
> driver?
oops, amended to the wrong patch
thanks for pointing it out
>> diff --git a/include/netdev.h b/include/netdev.h
>> index a9d5ec9..7800ff1 100644
>> --- a/include/netdev.h
>> +++ b/include/netdev.h
>> @@ -49,6 +49,7 @@ int davinci_emac_initialize(void);
>> int dnet_eth_initialize(int id, void *regs, unsigned int phy_addr);
>> int e1000_initialize(bd_t *bis);
>> int eepro100_initialize(bd_t *bis);
>> +int ep93xx_eth_init(bd_t * const bd);
>> int eth_3com_initialize (bd_t * bis);
>> int fec_initialize (bd_t *bis);
>> int fecmxc_initialize (bd_t *bis);
thanks again for your helpful comments, i'll address them in the next
revision of the patch
best regards
--
Matthias Kaehlcke
Embedded Linux Developer
Barcelona
The assumption that what currently exists must necessarily
exist is the acid that corrodes all visionary thinking
.''`.
using free software / Debian GNU/Linux | http://debian.org : :' :
`. `'`
gpg --keyserver pgp.mit.edu --recv-keys 47D8E5D4 `-
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
