[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

From: Reinhard Meyer <info@emk-elektronik.de>

Signed-off-by: Reinhard Meyer <u-boot@emk-elektronik.de>
---
 drivers/net/Makefile   |    1 +
 drivers/net/enc28j60.c |  650 ++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/net/enc28j60.h |  217 ++++++++++++++++
 include/netdev.h       |    1 +
 4 files changed, 869 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/enc28j60.c
 create mode 100644 drivers/net/enc28j60.h

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 479954d..79eb66b 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -39,6 +39,7 @@ COBJS-$(CONFIG_DRIVER_DM9000) += dm9000x.o
 COBJS-$(CONFIG_DNET) += dnet.o
 COBJS-$(CONFIG_E1000) += e1000.o
 COBJS-$(CONFIG_EEPRO100) += eepro100.o
+COBJS-$(CONFIG_ENC28J60) += enc28j60.o
 COBJS-$(CONFIG_ENC28J60_LPC2292) += enc28j60_lpc2292.o
 COBJS-$(CONFIG_EP93XX) += ep93xx_eth.o
 COBJS-$(CONFIG_ETHOC) += ethoc.o
diff --git a/drivers/net/enc28j60.c b/drivers/net/enc28j60.c
new file mode 100644
index 0000000..dec5d78
--- /dev/null
+++ b/drivers/net/enc28j60.c
@@ -0,0 +1,650 @@
+/*
+ * (X) reworked enc28j60.c
+ * Reinhard Meyer, EMK Elektronik, reinhard.meyer at emk-elektronik.de
+ *
+ * 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 <spi.h>
+#include <malloc.h>
+#include <asm/arch/hardware.h>
+#include "enc28j60.h"
+
+#define ENC28J60_DRIVERNAME "enc"
+
+#define FAILSAFE_VALUE 5000
+
+/*
+ * Controller memory layout:
+ *
+ * 0x0000 - 0x17ff  6k bytes receive buffer
+ * 0x1800 - 0x1fff  2k bytes transmit buffer
+ *
+ * Use the lower memory for receiver buffer. See errata pt. 5
+ */
+#define ENC_RX_BUF_START	0x0000
+#define ENC_TX_BUF_START	0x1800
+#define ENC_RX_BUF_END		0x17ff
+#define ENC_TX_BUF_END		0x1fff
+#define ENC_MAX_FRM_LEN		1518
+
+/*
+ * NEW feature:
+ * we encode a register's bank in the upper byte,
+ * use a cached value to keep track of the current bank
+ * and add automatic bank switching to register accesses
+ *
+ * registers available in all banks have bank number 0
+ */
+static u8 enc_r8 (u16 regNo);
+static u16 enc_r16 (u16 regNo);
+static void enc_w8 (u16 regNo, u8 data);
+static void enc_w16 (u16 regNo, u16 data);
+static void enc_w8Retry (u16 regNo, u8 data, int c);
+static void enc_w16Retry (u16 regNo, u16 data, int c);
+static void enc_rbuf (u16 length, u8 *pBuff);
+static void enc_wbuf (u16 length, u8 *pBuff);
+/* regNo is char here to detect someone trying to use a non common register */
+static void enc_bset (u8 regNo, u8 data);
+static void enc_bclr (u8 regNo, u8 data);
+static void enc_reset (void);
+static void enc_init (u8 *pEthAddr);
+static u16 phy_read (u8 addr);
+static void phy_write(u8, u16);
+static void enc_poll (void);
+static void enc_receive (void);
+
+static u8 bank = 0;	/* current bank in enc28j60 */
+static u16 next_pointer;
+/* NetReceive() requires the packet to be word aligned! */
+static u8 packet[ENC_MAX_FRM_LEN] __attribute__ ((aligned(4)));
+static int rx_reset_counter = 0;
+static struct spi_slave *slave;
+
+#define RX_RESET_COUNTER 1000;
+
+/*
+ * high byte of the register contains bank number:
+ * 0: no bank switch necessary
+ * 1: switch to bank 0
+ * 2: switch to bank 1
+ * 3: switch to bank 2
+ * 4: switch to bank 3
+ */
+static void encSetBank (const u16 regNo)
+{
+	u8 newbank = regNo >> 8;
+
+	if (newbank == 0 || newbank == bank)
+		return;
+	switch (newbank) {
+	case 1:
+		enc_bclr(CTL_REG_ECON1, ENC_ECON1_BSEL0 | ENC_ECON1_BSEL1);
+		break;
+	case 2:
+		enc_bset(CTL_REG_ECON1, ENC_ECON1_BSEL0);
+		enc_bclr(CTL_REG_ECON1, ENC_ECON1_BSEL1);
+		break;
+	case 3:
+		enc_bclr(CTL_REG_ECON1, ENC_ECON1_BSEL0);
+		enc_bset(CTL_REG_ECON1, ENC_ECON1_BSEL1);
+		break;
+	case 4:
+		enc_bset(CTL_REG_ECON1, ENC_ECON1_BSEL0 | ENC_ECON1_BSEL1);
+		break;
+	}
+	bank = newbank;
+}
+
+int enc28j60_init (struct eth_device *dev, bd_t * bis)
+{
+	u8 estatVal;
+	u8 enetaddr[6];
+
+	slave = spi_setup_slave(ENC_SPI_BUS, ENC_CS_NUM, ENC_SPI_CLOCK, SPI_MODE_0);
+	spi_claim_bus (slave);
+	/*
+	 * Wait for CLKRDY to become set (i.e., check that we can communicate with
+	 * the ENC)
+	 * TODO: a timeout might be wise here...
+	 */
+	do
+	{
+		estatVal = enc_r8(CTL_REG_ESTAT);
+	} while ((estatVal & 0x08) || (~estatVal & ENC_ESTAT_CLKRDY));
+
+	/* initialize controller */
+	enc_reset ();
+#ifdef CONFIG_NET_MULTI
+	memcpy(enetaddr, eth_get_dev()->enetaddr, 6);
+#else
+	eth_getenv_enetaddr("ethaddr", enetaddr);
+#endif
+	enc_init(enetaddr);
+	enc_bset(CTL_REG_ECON1, ENC_ECON1_RXEN);	/* enable receive */
+	return 0;
+}
+
+int enc28j60_send (struct eth_device *dev, volatile void *packet, int length)
+{
+	/* check frame length, etc. */
+
+	/* set EWRPT */
+	enc_w16(CTL_REG_EWRPTL, ENC_TX_BUF_START);
+	/* set ETXND */
+	enc_w16(CTL_REG_ETXNDL, length + ENC_TX_BUF_START);
+	/* set ETXST */
+	enc_w16(CTL_REG_ETXSTL, ENC_TX_BUF_START);
+	/* write packet */
+	enc_wbuf(length, (u8 *) packet);
+	/*
+	 * Verify that the internal transmit logic has not been altered by excessive
+	 *  collisions.  See Errata B4 12 and 14.
+	 */
+	if (enc_r8(CTL_REG_EIR) & ENC_EIR_TXERIF) {
+		enc_bset(CTL_REG_ECON1, ENC_ECON1_TXRST);
+		enc_bclr(CTL_REG_ECON1, ENC_ECON1_TXRST);
+	}
+	enc_bclr(CTL_REG_EIR, (ENC_EIR_TXERIF | ENC_EIR_TXIF));
+	/* set ECON1.TXRTS */
+	enc_bset(CTL_REG_ECON1, ENC_ECON1_TXRTS);
+	return 0;
+}
+
+/*
+ * This function resets the receiver only. This function may be called from
+ * interrupt-context.
+ */
+static void encReceiverReset (void)
+{
+	u8 econ1;
+
+	econ1 = enc_r8(CTL_REG_ECON1);
+	if ((econ1 & ENC_ECON1_RXRST) == 0) {
+		enc_bset(CTL_REG_ECON1, ENC_ECON1_RXRST);
+		rx_reset_counter = RX_RESET_COUNTER;
+	}
+}
+
+/*
+ * receiver reset timer
+ */
+static void encReceiverResetCallback (void)
+{
+	enc_bclr(CTL_REG_ECON1, ENC_ECON1_RXRST);
+	enc_bset(CTL_REG_ECON1, ENC_ECON1_RXEN);	/* enable receive */
+}
+
+/*
+ * Check for received packets. Call NetReceive for each packet. The return
+ * value is ignored by the caller.
+ */
+int enc28j60_recv (struct eth_device *dev)
+{
+	if (rx_reset_counter > 0 && --rx_reset_counter == 0) {
+		encReceiverResetCallback();
+	}
+	enc_poll();
+	return 0;
+}
+
+void enc28j60_halt (struct eth_device *dev)
+{
+	enc_bclr(CTL_REG_ECON1, ENC_ECON1_RXEN);	/* disable receive */
+}
+
+static void enc_poll (void)
+{
+	u8 eir_reg;
+	volatile u8 estat_reg;
+	u8 pkt_cnt;
+
+#ifdef CONFIG_USE_IRQ
+	/* clear global interrupt enable bit in enc28j60 */
+	enc_bclr(CTL_REG_EIE, ENC_EIE_INTIE);
+#endif
+	estat_reg = enc_r8(CTL_REG_ESTAT);
+	eir_reg = enc_r8(CTL_REG_EIR);
+	if (eir_reg & ENC_EIR_TXIF) {
+		/* clear TXIF bit in EIR */
+		enc_bclr(CTL_REG_EIR, ENC_EIR_TXIF);
+	}
+	/* We have to use pktcnt and not pktif bit, see errata pt. 6 */
+	/* read pktcnt */
+	pkt_cnt = enc_r8(CTL_REG_EPKTCNT);
+	if (pkt_cnt > 0) {
+		if ((eir_reg & ENC_EIR_PKTIF) == 0) {
+			debug("enc_poll: pkt cnt > 0, but pktif not set\n");
+		}
+		enc_receive();
+		/*
+		 * clear PKTIF bit in EIR, this should not need to be done but it
+		 *  seems like we get problems if we do not
+		 */
+		enc_bclr(CTL_REG_EIR, ENC_EIR_PKTIF);
+	}
+	if (eir_reg & ENC_EIR_RXERIF) {
+		printf("enc_poll: rx error\n");
+		enc_bclr(CTL_REG_EIR, ENC_EIR_RXERIF);
+	}
+	if (eir_reg & ENC_EIR_TXERIF) {
+		printf("enc_poll: tx error\n");
+		enc_bclr(CTL_REG_EIR, ENC_EIR_TXERIF);
+	}
+#ifdef CONFIG_USE_IRQ
+	/* set global interrupt enable bit in enc28j60 */
+	enc_bset(CTL_REG_EIE, ENC_EIE_INTIE);
+#endif
+}
+
+static void enc_receive (void)
+{
+	u16 pkt_len;
+	u16 copy_len;
+	u16 status;
+	u8 eir_reg;
+	u8 pkt_cnt = 0;
+	u16 rxbuf_rdpt;
+	u8 hbuf[6];
+
+	enc_w16(CTL_REG_ERDPTL, next_pointer);
+	do {
+		enc_rbuf(6, hbuf);
+		next_pointer = hbuf[0] | (hbuf[1] << 8);
+		pkt_len = hbuf[2] | (hbuf[3] << 8);
+		status = hbuf[4] | (hbuf[5] << 8);
+#ifdef DEBUG
+		printf("next_pointer=$%04x pkt_len=%u status=$%04x\n",
+			next_pointer, pkt_len, status);
+#endif
+		if (pkt_len <= ENC_MAX_FRM_LEN)
+			copy_len = pkt_len;
+		else
+			copy_len = 0;
+		if ((status & (1L << 7)) == 0) /* check Received Ok bit */
+			copy_len = 0;
+		/* check if next pointer is resonable */
+		if (next_pointer >= ENC_TX_BUF_START)
+			copy_len = 0;
+		if (copy_len > 0) {
+			enc_rbuf(copy_len, packet);
+		}
+		/* advance read pointer to next pointer */
+		enc_w16(CTL_REG_ERDPTL, next_pointer);
+		/* decrease packet counter */
+		enc_bset(CTL_REG_ECON2, ENC_ECON2_PKTDEC);
+		/*
+		 * Only odd values should be written to ERXRDPTL,
+		 * see errata B4 pt.13
+		 */
+		rxbuf_rdpt = next_pointer - 1;
+		if ((rxbuf_rdpt < enc_r16(CTL_REG_ERXSTL)) ||
+			(rxbuf_rdpt > enc_r16(CTL_REG_ERXNDL))) {
+			enc_w16(CTL_REG_ERXRDPTL, enc_r16(CTL_REG_ERXNDL));
+		} else {
+			enc_w16(CTL_REG_ERXRDPTL, rxbuf_rdpt);
+		}
+		/* read pktcnt */
+		pkt_cnt = enc_r8(CTL_REG_EPKTCNT);
+		if (copy_len == 0) {
+			eir_reg = enc_r8(CTL_REG_EIR);
+			encReceiverReset();
+			printf("eth_rx: copy_len=0\n");
+			continue;
+		}
+		NetReceive(packet, pkt_len);
+		eir_reg = enc_r8(CTL_REG_EIR);
+	} while (pkt_cnt);	/* Use EPKTCNT not EIR.PKTIF flag, see errata pt. 6 */
+}
+
+static void enc_w8 (u16 regNo, u8 data)
+{
+	u8 dout[2];
+
+	encSetBank(regNo);
+	dout[0] = (0x40 | regNo); /* write in regNo */
+	dout[1] = data;
+	spi_xfer(slave, 2 * 8, dout, NULL, SPI_XFER_BEGIN | SPI_XFER_END);
+}
+
+static void enc_w16 (u16 regNo, u16 data)
+{
+	u8 dout[2];
+
+	encSetBank(regNo);
+	dout[0] = (0x40 | regNo); /* write in regNo */
+	dout[1] = data;
+	spi_xfer(slave, 2 * 8, dout, NULL, SPI_XFER_BEGIN | SPI_XFER_END);
+	dout[0]++; /* write in regNo+1 */
+	dout[1] = data >> 8;
+	spi_xfer(slave, 2 * 8, dout, NULL, SPI_XFER_BEGIN | SPI_XFER_END);
+}
+
+static void enc_w8Retry (u16 regNo, u8 data, int c)
+{
+	u8 dout[2];
+	u8 readback;
+	int i;
+
+	encSetBank(regNo);
+	for (i = 0; i < c; i++) {
+		dout[0] = (0x40 | regNo); /* write in regNo */
+		dout[1] = data;
+		spi_xfer(slave, 2 * 8, dout, NULL,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		readback = enc_r8(regNo);
+		if (readback == data)
+			break;
+	}
+	if (i == c) {
+		printf("enc28j60: write reg %d failed\n", regNo);
+	}
+}
+
+static void enc_w16Retry (u16 regNo, u16 data, int c)
+{
+	enc_w8Retry(regNo, data, c);
+	enc_w8Retry(regNo+1, data>>8, c);
+}
+
+static u8 enc_r8 (u16 regNo)
+{
+	u8 dout[3];
+	u8 din[3];
+
+	encSetBank(regNo);
+	dout[0] = regNo;	/* read in regNo */
+	/* check if MAC or MII register */
+	if (((bank == 3) && (dout[0] <= 0x1a)) ||
+	    ((bank == 4) && (dout[0] <= 0x05 || dout[0] == 0x0a))) {
+		/* read three bytes */
+		spi_xfer(slave, 3 * 8, dout, din,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		return din[2];
+	} else {
+		spi_xfer(slave, 2 * 8, dout, din,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		return din[1];
+	}
+}
+
+static u16 enc_r16 (u16 regNo)
+{
+	u8 dout[3];
+	u8 din[3];
+	u16 rxByte;
+
+	encSetBank(regNo);
+	dout[0] = regNo;	/* read in regNo */
+	/* check if MAC or MII register */
+	if (((bank == 3) && (dout[0] <= 0x1a)) ||
+	    ((bank == 4) && (dout[0] <= 0x05 || dout[0] == 0x0a))) {
+		/* read three bytes (twice) */
+		spi_xfer(slave, 3 * 8, dout, din,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		rxByte = din[2];
+		dout[0]++;
+		spi_xfer(slave, 3 * 8, dout, din,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		rxByte |= din[2] << 8;
+	} else {
+		spi_xfer(slave, 2 * 8, dout, din,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		rxByte = din[1];
+		dout[0]++;
+		spi_xfer(slave, 2 * 8, dout, din,
+			  SPI_XFER_BEGIN | SPI_XFER_END);
+		rxByte |= din[1] << 8;
+	}
+	return rxByte;
+}
+
+static void enc_rbuf (u16 length, u8 *pBuff)
+{
+	u8 dout[1];
+
+	dout[0] = (0x20 | 0x1a);
+	spi_xfer(slave, 8, dout, NULL, SPI_XFER_BEGIN);
+	spi_xfer(slave, length * 8, NULL, pBuff, SPI_XFER_END);
+	debug("Rx:\n");
+#ifdef DEBUG
+	print_buffer(0, pBuff, 1, length, 0);
+#endif
+}
+
+static void enc_wbuf (u16 length, u8 *pBuff)
+{
+	u8 dout[2];
+	dout[0] = (0x60 | 0x1a);
+	dout[1] = (0x00);
+	spi_xfer(slave, 2 * 8, dout, NULL, SPI_XFER_BEGIN);
+	spi_xfer(slave, length * 8, pBuff, NULL, SPI_XFER_END);
+	debug("Tx:\n");
+#ifdef DEBUG
+	print_buffer(0, pBuff, 1, length, 0);
+#endif
+}
+
+static void enc_bset (u8 regNo, u8 data)
+{
+	u8 dout[2];
+
+	dout[0] = (0x80 | regNo);
+	dout[1] = data;
+	spi_xfer(slave, 2 * 8, dout, NULL, SPI_XFER_BEGIN | SPI_XFER_END);
+}
+
+static void enc_bclr (u8 regNo, u8 data)
+{
+	u8 dout[2];
+
+	dout[0] = (0xA0 | regNo);
+	dout[1] = data;
+	spi_xfer(slave, 2 * 8, dout, NULL, SPI_XFER_BEGIN | SPI_XFER_END);
+}
+
+static void enc_reset (void)
+{
+	u8 dout[1];
+
+	dout[0] = 0xff;
+	spi_xfer(slave, 8, dout, NULL, SPI_XFER_BEGIN | SPI_XFER_END);
+	/* sleep 1 ms. See errata pt. 2 */
+	udelay(1000);
+}
+
+static void enc_init (u8 *pEthAddr)
+{
+	u16 phid1 = 0;
+	u16 phid2 = 0;
+
+	/*
+	 * Setup the buffer space. The reset values are valid for the
+	 * other pointers.
+	 *
+	 * We shall not write to ERXST, see errata pt. 5. Instead we
+	 * have to make sure that ENC_RX_BUS_START is 0.
+	 */
+	enc_w16Retry(CTL_REG_ERXSTL, ENC_RX_BUF_START, 1);
+	enc_w16Retry(CTL_REG_ERXNDL, ENC_RX_BUF_END, 1);
+	enc_w16Retry(CTL_REG_ERDPTL, ENC_RX_BUF_START, 1);
+	next_pointer = ENC_RX_BUF_START;
+	/* verify identification */
+	phid1 = phy_read(PHY_REG_PHID1);
+	phid2 = phy_read(PHY_REG_PHID2);
+	if (phid1 != ENC_PHID1_VALUE
+	    || (phid2 & ENC_PHID2_MASK) != ENC_PHID2_VALUE) {
+		printf("ERROR: failed to identify controller\n");
+		printf("phid1 = %x, phid2 = %x\n",
+			phid1, (phid2 & ENC_PHID2_MASK));
+		printf("should be phid1 = %x, phid2 = %x\n",
+			ENC_PHID1_VALUE, ENC_PHID2_VALUE);
+	}
+#ifdef ENC_BROADCAST_RCV
+	/* set the filter to receive only good-CRC unicast and broadcast frames */
+	enc_w8Retry(CTL_REG_ERXFCON,
+		ENC_RFR_BCEN | ENC_RFR_UCEN | ENC_RFR_CRCEN, 10);
+#else
+	/* set the filter to receive only good-CRC unicast frames */
+	enc_w8Retry(CTL_REG_ERXFCON,
+		ENC_RFR_UCEN | ENC_RFR_CRCEN, 10);
+#endif
+	/* Pull MAC out of Reset */
+	/* enable MAC to receive frames */
+	enc_w8Retry(CTL_REG_MACON1,
+		(ENC_MACON1_MARXEN | ENC_MACON1_TXPAUS | ENC_MACON1_RXPAUS), 10);
+	/* configure pad, tx-crc and duplex */
+	enc_w8Retry(CTL_REG_MACON3,
+		(ENC_MACON3_PADCFG0 | ENC_MACON3_TXCRCEN | ENC_MACON3_FRMLNEN), 10);
+	/* Allow infinite deferals if the medium is continously busy */
+	enc_w8Retry(CTL_REG_MACON4, (1<<6) /*ENC_MACON4_DEFER*/, 10);
+	/* Late collisions occur beyond 63 bytes */
+	enc_w8Retry(CTL_REG_MACLCON2, 63, 10);
+	/* Set (low byte) Non-Back-to_Back Inter-Packet Gap. Recommended 0x12 */
+	enc_w8Retry(CTL_REG_MAIPGL, 0x12, 10);
+	/*
+	* Set (high byte) Non-Back-to_Back Inter-Packet Gap. Recommended
+	* 0x0c for half-duplex. Nothing for full-duplex
+	*/
+	enc_w8Retry(CTL_REG_MAIPGH, 0x0C, 10);
+	/* set maximum frame length */
+	enc_w16Retry(CTL_REG_MAMXFLL, ENC_MAX_FRM_LEN, 10);
+	/*
+	 * Set MAC back-to-back inter-packet gap. Recommended 0x12 for half duplex
+	 * and 0x15 for full duplex.
+	 */
+	enc_w8Retry(CTL_REG_MABBIPG, 0x12, 10);
+	/* set MAC address */
+	enc_w8Retry(CTL_REG_MAADR0, pEthAddr[5], 1);
+	enc_w8Retry(CTL_REG_MAADR1, pEthAddr[4], 1);
+	enc_w8Retry(CTL_REG_MAADR2, pEthAddr[3], 1);
+	enc_w8Retry(CTL_REG_MAADR3, pEthAddr[2], 1);
+	enc_w8Retry(CTL_REG_MAADR4, pEthAddr[1], 1);
+	enc_w8Retry(CTL_REG_MAADR5, pEthAddr[0], 1);
+	/*
+	 * Prevent automatic loopback of data beeing transmitted by setting
+	 * ENC_PHCON2_HDLDIS
+	 */
+	phy_write(PHY_REG_PHCON2, (1<<8));
+	/*
+	 * LEDs configuration
+	 * LEDA: LACFG = 0100 -> display link status
+	 * LEDB: LBCFG = 0111 -> display TX & RX activity
+	 * STRCH = 1 -> LED pulses
+	 */
+	phy_write(PHY_REG_PHLCON, 0x0472);
+	/* Reset PDPXMD-bit => half duplex */
+	phy_write(PHY_REG_PHCON1, 0);
+	/*
+	 * Receive settings
+	 */
+#ifdef CONFIG_USE_IRQ
+	/* enable interrupts */
+	enc_bset(CTL_REG_EIE, ENC_EIE_PKTIE);
+	enc_bset(CTL_REG_EIE, ENC_EIE_TXIE);
+	enc_bset(CTL_REG_EIE, ENC_EIE_RXERIE);
+	enc_bset(CTL_REG_EIE, ENC_EIE_TXERIE);
+	enc_bset(CTL_REG_EIE, ENC_EIE_INTIE);
+#endif
+}
+
+/*
+ *
+ * Description:
+ *    Read PHY registers.
+ *
+ *    NOTE! This function will change to Bank 2.
+ *
+ * Params:
+ *    [in] addr address of the register to read
+ *
+ * Returns:
+ *    The value in the register
+ */
+static u16 phy_read (u8 addr)
+{
+	int cnt;
+
+	/* write address to MIREGADR */
+	enc_w8(CTL_REG_MIREGADR, addr);
+	/* set MICMD.MIIRD */
+	enc_w8(CTL_REG_MICMD, ENC_MICMD_MIIRD);
+	/* poll MISTAT.BUSY bit until operation is complete */
+	cnt = 0;
+	while ((enc_r8(CTL_REG_MISTAT) & ENC_MISTAT_BUSY) != 0) {
+		if (cnt++ >= 1000) {
+			/* GJ - this seems extremely dangerous! */
+			printf("#1");
+			return 0;
+		}
+	}
+	/* clear MICMD.MIIRD */
+	enc_w8(CTL_REG_MICMD, 0);
+	return enc_r16(CTL_REG_MIRDL);
+}
+
+/*
+ *
+ * Taken from the Linux driver.
+ * Description:
+ * Write PHY registers.
+ *
+ * NOTE! This function will change to Bank 3.
+ *
+ * Params:
+ * [in] addr address of the register to write to
+ * [in] data to be written
+ *
+ * Returns:
+ *    None
+ */
+static void phy_write(u8 addr, u16 data)
+{
+	int cnt;
+	/* write address to MIREGADR */
+	enc_w8(CTL_REG_MIREGADR, addr);
+	enc_w16(CTL_REG_MIWRL, data);
+	/* poll MISTAT.BUSY bit until operation is complete */
+	cnt = 0;
+	while((enc_r8(CTL_REG_MISTAT) & ENC_MISTAT_BUSY) != 0) {
+		if (cnt++ >= 1000) {
+			/* GJ - this seems extremely dangerous! */
+			printf("#2");
+			return;
+		}
+	}
+}
+
+int enc28j60_initialize(u8 dev_num)
+{
+	struct eth_device *dev;
+
+	dev = malloc(sizeof(*dev));
+	if (!dev) {
+		return 0;
+	}
+	memset(dev, 0, sizeof(*dev));
+	dev->init = enc28j60_init;
+	dev->halt = enc28j60_halt;
+	dev->send = enc28j60_send;
+	dev->recv = enc28j60_recv;
+	sprintf(dev->name, "%s%hu", ENC28J60_DRIVERNAME, dev_num);
+	eth_register(dev);
+	return 0;
+}
diff --git a/drivers/net/enc28j60.h b/drivers/net/enc28j60.h
new file mode 100644
index 0000000..8bbee68
--- /dev/null
+++ b/drivers/net/enc28j60.h
@@ -0,0 +1,217 @@
+/*
+ * (X) extracted from enc28j60.c
+ * Reinhard Meyer, EMK Elektronik, reinhard.meyer at emk-elektronik.de
+ *
+ * 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
+ */
+
+/* NEW: encode (bank number+1) in upper byte */
+
+/* these are common in all banks */
+#define CTL_REG_EIE		0x01B
+#define CTL_REG_EIR		0x01C
+#define CTL_REG_ESTAT		0x01D
+#define CTL_REG_ECON2		0x01E
+#define CTL_REG_ECON1		0x01F
+
+/* Control Registers in Bank 0 */
+#define CTL_REG_ERDPTL		0x100
+#define CTL_REG_ERDPTH		0x101
+#define CTL_REG_EWRPTL		0x102
+#define CTL_REG_EWRPTH		0x103
+#define CTL_REG_ETXSTL		0x104
+#define CTL_REG_ETXSTH		0x105
+#define CTL_REG_ETXNDL		0x106
+#define CTL_REG_ETXNDH		0x107
+#define CTL_REG_ERXSTL		0x108
+#define CTL_REG_ERXSTH		0x109
+#define CTL_REG_ERXNDL		0x10A
+#define CTL_REG_ERXNDH		0x10B
+#define CTL_REG_ERXRDPTL	0x10C
+#define CTL_REG_ERXRDPTH	0x10D
+#define CTL_REG_ERXWRPTL	0x10E
+#define CTL_REG_ERXWRPTH	0x10F
+#define CTL_REG_EDMASTL		0x110
+#define CTL_REG_EDMASTH		0x111
+#define CTL_REG_EDMANDL		0x112
+#define CTL_REG_EDMANDH		0x113
+#define CTL_REG_EDMADSTL	0x114
+#define CTL_REG_EDMADSTH	0x115
+#define CTL_REG_EDMACSL		0x116
+#define CTL_REG_EDMACSH		0x117
+
+/* Control Registers in Bank 1 */
+#define CTL_REG_EHT0		0x200
+#define CTL_REG_EHT1		0x201
+#define CTL_REG_EHT2		0x202
+#define CTL_REG_EHT3		0x203
+#define CTL_REG_EHT4		0x204
+#define CTL_REG_EHT5		0x205
+#define CTL_REG_EHT6		0x206
+#define CTL_REG_EHT7		0x207
+#define CTL_REG_EPMM0		0x208
+#define CTL_REG_EPMM1		0x209
+#define CTL_REG_EPMM2		0x20A
+#define CTL_REG_EPMM3		0x20B
+#define CTL_REG_EPMM4		0x20C
+#define CTL_REG_EPMM5		0x20D
+#define CTL_REG_EPMM6		0x20E
+#define CTL_REG_EPMM7		0x20F
+#define CTL_REG_EPMCSL		0x210
+#define CTL_REG_EPMCSH		0x211
+#define CTL_REG_EPMOL		0x214
+#define CTL_REG_EPMOH		0x215
+#define CTL_REG_EWOLIE		0x216
+#define CTL_REG_EWOLIR		0x217
+#define CTL_REG_ERXFCON		0x218
+#define CTL_REG_EPKTCNT		0x219
+
+/* Control Registers in Bank 2 */
+#define CTL_REG_MACON1		0x300
+#define CTL_REG_MACON2		0x301
+#define CTL_REG_MACON3		0x302
+#define CTL_REG_MACON4		0x303
+#define CTL_REG_MABBIPG		0x304
+#define CTL_REG_MAIPGL		0x306
+#define CTL_REG_MAIPGH		0x307
+#define CTL_REG_MACLCON1	0x308
+#define CTL_REG_MACLCON2	0x309
+#define CTL_REG_MAMXFLL		0x30A
+#define CTL_REG_MAMXFLH		0x30B
+#define CTL_REG_MAPHSUP		0x30D
+#define CTL_REG_MICON		0x311
+#define CTL_REG_MICMD		0x312
+#define CTL_REG_MIREGADR	0x314
+#define CTL_REG_MIWRL		0x316
+#define CTL_REG_MIWRH		0x317
+#define CTL_REG_MIRDL		0x318
+#define CTL_REG_MIRDH		0x319
+
+/* Control Registers in Bank 3 */
+#define CTL_REG_MAADR1		0x400
+#define CTL_REG_MAADR0		0x401
+#define CTL_REG_MAADR3		0x402
+#define CTL_REG_MAADR2		0x403
+#define CTL_REG_MAADR5		0x404
+#define CTL_REG_MAADR4		0x405
+#define CTL_REG_EBSTSD		0x406
+#define CTL_REG_EBSTCON		0x407
+#define CTL_REG_EBSTCSL		0x408
+#define CTL_REG_EBSTCSH		0x409
+#define CTL_REG_MISTAT		0x40A
+#define CTL_REG_EREVID		0x412
+#define CTL_REG_ECOCON		0x415
+#define CTL_REG_EFLOCON		0x417
+#define CTL_REG_EPAUSL		0x418
+#define CTL_REG_EPAUSH		0x419
+
+/* PHY Register */
+#define PHY_REG_PHID1		0x02
+#define PHY_REG_PHID2		0x03
+#define PHY_REG_PHCON1		0x00
+#define PHY_REG_PHCON2		0x10
+#define PHY_REG_PHLCON		0x14
+
+/* Receive Filter Register (ERXFCON) bits */
+#define ENC_RFR_UCEN		0x80
+#define ENC_RFR_ANDOR		0x40
+#define ENC_RFR_CRCEN		0x20
+#define ENC_RFR_PMEN		0x10
+#define ENC_RFR_MPEN		0x08
+#define ENC_RFR_HTEN		0x04
+#define ENC_RFR_MCEN		0x02
+#define ENC_RFR_BCEN		0x01
+
+/* ECON1 Register Bits */
+#define ENC_ECON1_TXRST		0x80
+#define ENC_ECON1_RXRST		0x40
+#define ENC_ECON1_DMAST		0x20
+#define ENC_ECON1_CSUMEN	0x10
+#define ENC_ECON1_TXRTS		0x08
+#define ENC_ECON1_RXEN		0x04
+#define ENC_ECON1_BSEL1		0x02
+#define ENC_ECON1_BSEL0		0x01
+
+/* ECON2 Register Bits */
+#define ENC_ECON2_AUTOINC	0x80
+#define ENC_ECON2_PKTDEC	0x40
+#define ENC_ECON2_PWRSV		0x20
+#define ENC_ECON2_VRPS		0x08
+
+/* EIR Register Bits */
+#define ENC_EIR_PKTIF		0x40
+#define ENC_EIR_DMAIF		0x20
+#define ENC_EIR_LINKIF		0x10
+#define ENC_EIR_TXIF		0x08
+#define ENC_EIR_WOLIF		0x04
+#define ENC_EIR_TXERIF		0x02
+#define ENC_EIR_RXERIF		0x01
+
+/* ESTAT Register Bits */
+#define ENC_ESTAT_INT		0x80
+#define ENC_ESTAT_LATECOL	0x10
+#define ENC_ESTAT_RXBUSY	0x04
+#define ENC_ESTAT_TXABRT	0x02
+#define ENC_ESTAT_CLKRDY	0x01
+
+/* EIE Register Bits */
+#define ENC_EIE_INTIE		0x80
+#define ENC_EIE_PKTIE		0x40
+#define ENC_EIE_DMAIE		0x20
+#define ENC_EIE_LINKIE		0x10
+#define ENC_EIE_TXIE		0x08
+#define ENC_EIE_WOLIE		0x04
+#define ENC_EIE_TXERIE		0x02
+#define ENC_EIE_RXERIE		0x01
+
+/* MACON1 Register Bits */
+#define ENC_MACON1_LOOPBK	0x10
+#define ENC_MACON1_TXPAUS	0x08
+#define ENC_MACON1_RXPAUS	0x04
+#define ENC_MACON1_PASSALL	0x02
+#define ENC_MACON1_MARXEN	0x01
+
+/* MACON2 Register Bits */
+#define ENC_MACON2_MARST	0x80
+#define ENC_MACON2_RNDRST	0x40
+#define ENC_MACON2_MARXRST	0x08
+#define ENC_MACON2_RFUNRST	0x04
+#define ENC_MACON2_MATXRST	0x02
+#define ENC_MACON2_TFUNRST	0x01
+
+/* MACON3 Register Bits */
+#define ENC_MACON3_PADCFG2	0x80
+#define ENC_MACON3_PADCFG1	0x40
+#define ENC_MACON3_PADCFG0	0x20
+#define ENC_MACON3_TXCRCEN	0x10
+#define ENC_MACON3_PHDRLEN	0x08
+#define ENC_MACON3_HFRMEN	0x04
+#define ENC_MACON3_FRMLNEN	0x02
+#define ENC_MACON3_FULDPX	0x01
+
+/* MICMD Register Bits */
+#define ENC_MICMD_MIISCAN	0x02
+#define ENC_MICMD_MIIRD		0x01
+
+/* MISTAT Register Bits */
+#define ENC_MISTAT_NVALID	0x04
+#define ENC_MISTAT_SCAN		0x02
+#define ENC_MISTAT_BUSY		0x01
+
+/* PHID1 and PHID2 values */
+#define ENC_PHID1_VALUE		0x0083
+#define ENC_PHID2_VALUE		0x1400
+#define ENC_PHID2_MASK		0xFC00
diff --git a/include/netdev.h b/include/netdev.h
index 94eedfe..4210e69 100644
--- a/include/netdev.h
+++ b/include/netdev.h
@@ -54,6 +54,7 @@ int designware_initialize(u32 id, ulong base_addr, u32 phy_addr);
 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 enc28j60_initialize(int id);
 int ep93xx_eth_initialize(u8 dev_num, int base_addr);
 int ethoc_initialize(u8 dev_num, int base_addr);
 int eth_3com_initialize (bd_t * bis);
-- 
1.5.6.5

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Mike Frysinger]

On Wednesday, August 25, 2010 08:47:39 Reinhard Meyer wrote:
> From: Reinhard Meyer <info@emk-elektronik.de>
> 
> Signed-off-by: Reinhard Meyer <u-boot@emk-elektronik.de>

these two fields should match (e-mail)

also, this patch seems to depend on some other change not in mainline or Ben's 
net tree ...

> +#include <config.h>
> +#include <common.h>

the common.h header takes care of config.h already

> +#include <asm/arch/hardware.h>

err, what ?

> +#define ENC28J60_DRIVERNAME "enc"

this is used in only one place, so no point in a define

> +#define FAILSAFE_VALUE 5000

this isnt used anywhere

> +static u8 enc_r8 (u16 regNo);
> +static void enc_rbuf (u16 length, u8 *pBuff);

this whole file needs to be checked for function style.  no space after the 
function name.

also, drop the camel case and hungarian notation.  use "reg" and "buff".

> +/* NetReceive() requires the packet to be word aligned! */
> +static u8 packet[ENC_MAX_FRM_LEN] __attribute__ ((aligned(4)));

why do you need to declare your own buffer ?  the common code already sets up 
one for you and it uses the standard 1518 size (PKTSIZE).

> +int enc28j60_init (struct eth_device *dev, bd_t * bis)

all of the enc28j60 funcs that go into eth_device dont need to be public.  so 
add static to the init/halt/recv/send funcs.

also, it's "bd_t *bis"

> +	slave = spi_setup_slave(ENC_SPI_BUS, ENC_CS_NUM, ENC_SPI_CLOCK,
> SPI_MODE_0);

pass the bus/cs/speed/mode in to the initialize function and store it in the 
per-device state.  then you can work fine with multiple enc28j60 devices in 
one board.

along those lines, all of the local enc funcs should be changed to take the 
private state and operate off that instead of random global variables.

> +	spi_claim_bus (slave);

missing check on the return value

> +	do
> +	{
> +		estatVal = enc_r8(CTL_REG_ESTAT);
> +	} while ((estatVal & 0x08) || (~estatVal & ENC_ESTAT_CLKRDY));

the bit inverting ("~") should be on the mask so it's done at compile time 
instead of forcing the board to do it at runtime

> +#ifdef CONFIG_NET_MULTI
> +	memcpy(enetaddr, eth_get_dev()->enetaddr, 6);
> +#else
> +	eth_getenv_enetaddr("ethaddr", enetaddr);
> +#endif
> +	enc_init(enetaddr);

this should be one line:
	enc_init(dev->enetaddr);

also, nowhere in this init func can i see error checking that the enc28j60 
device is actually out there.  the enc_init() should return an error if the 
expected phy's/etc... dont match, and this init func should key off that.

> +static void enc_init (u8 *pEthAddr)
> +{
> +	u16 phid1 = 0;
> +	u16 phid2 = 0;

these are always set up below, so no point in setting them to 0

> +static void phy_write(u8 addr, u16 data)
>
> +	while((enc_r8(CTL_REG_MISTAT) & ENC_MISTAT_BUSY) != 0) {

missing a space after that "while"

> +	sprintf(dev->name, "%s%hu", ENC28J60_DRIVERNAME, dev_num);

once this func is fixed to take the spi bus/cs, the name can be based on that 
instead of an arbitrary integer.
	"enc%i.%i", bus, cs

> --- /dev/null
> +++ b/drivers/net/enc28j60.h

this header is missing ifdef protection against multiple inclusion

> --- a/include/netdev.h
> +++ b/include/netdev.h
>
> +int enc28j60_initialize(int id);

this doesnt match the prototype in the source.  you should have the .c file 
include netdev.h so that desyncs are caught & prevented in the future.
-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/20100825/87dd332f/attachment.pgp 

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

Dear Mike Frysinger,
>> From: Reinhard Meyer<info@emk-elektronik.de>

Give me a tip how I can change that info. The first patch to this was
committed while I used <info>. Squashing other patches into it does not
change that. rebase -i, edit, --amend does not present that line for
edit. The only idea I have now is making an empty commit and squashing
that before the original patch...

> also, this patch seems to depend on some other change not in mainline or Ben's
> net tree ...

Should have mentioned that the "move enc28j60 to sidetrack" patch is to be
applied first.

>> +static u8 enc_r8 (u16 regNo);
>> +static void enc_rbuf (u16 length, u8 *pBuff);
>
> this whole file needs to be checked for function style.  no space after the
> function name.

That's what happens if one reworks an existing driver. I'll recheck all this.

> why do you need to declare your own buffer ?  the common code already sets up
> one for you and it uses the standard 1518 size (PKTSIZE).

Ok, better. Just hint me on its name, browsing the common code none did catch
my eye:)

> pass the bus/cs/speed/mode in to the initialize function and store it in the
> per-device state.  then you can work fine with multiple enc28j60 devices in
> one board.

<joke>
Ok, I was already planning to build a 16 port hub using 16 such devices ;)
</joke>
But generalization is good. I might end up using two, actually.

> also, nowhere in this init func can i see error checking that the enc28j60
> device is actually out there.  the enc_init() should return an error if the
> expected phy's/etc... dont match, and this init func should key off that.

I'd like to even have the phy start auto-negotiation at that moment, I'm
just not sure that's ok. I won't wait for it to complete, of course, just
trigger it.

> once this func is fixed to take the spi bus/cs, the name can be based on that
> instead of an arbitrary integer.
> 	"enc%i.%i", bus, cs

setenv ethact enc1.13 - one can get used to that ;)

Reinhard

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Mike Frysinger]

On Thursday, August 26, 2010 02:19:07 Reinhard Meyer wrote:
> >> From: Reinhard Meyer<info@emk-elektronik.de>
> 
> Give me a tip how I can change that info. The first patch to this was
> committed while I used <info>. Squashing other patches into it does not
> change that. rebase -i, edit, --amend does not present that line for
> edit. The only idea I have now is making an empty commit and squashing
> that before the original patch...

use the --author option to the commit sub command

> > why do you need to declare your own buffer ?  the common code already
> > sets up one for you and it uses the standard 1518 size (PKTSIZE).
> 
> Ok, better. Just hint me on its name, browsing the common code none did
> catch my eye:)

NetRxPackets and friends
-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/20100826/933d07b3/attachment.pgp 

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

Dear Mike Frysinger,
>>>> From: Reinhard Meyer<info@emk-elektronik.de>
>>
>> Give me a tip how I can change that info. The first patch to this was
>> committed while I used<info>. Squashing other patches into it does not
>> change that. rebase -i, edit, --amend does not present that line for
>> edit. The only idea I have now is making an empty commit and squashing
>> that before the original patch...
>
> use the --author option to the commit sub command

Hmm, as I said above, the first commit was made while I had the <info>
address in use. All new commits use the <u-boot> address anyway.
But when I squash new commits onto the first one, the old address
prevails...

I'll try "rebase -i", "edit", reset HEAD^, and a new commit then.
That might work.

Reinhard

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Mike Frysinger]

On Thursday, August 26, 2010 02:41:56 Reinhard Meyer wrote:
> >>>> From: Reinhard Meyer<info@emk-elektronik.de>
> >> 
> >> Give me a tip how I can change that info. The first patch to this was
> >> committed while I used<info>. Squashing other patches into it does not
> >> change that. rebase -i, edit, --amend does not present that line for
> >> edit. The only idea I have now is making an empty commit and squashing
> >> that before the original patch...
> > 
> > use the --author option to the commit sub command
> 
> Hmm, as I said above, the first commit was made while I had the <info>
> address in use. All new commits use the <u-boot> address anyway.
> But when I squash new commits onto the first one, the old address
> prevails...

there is no "field" to edit in the commit message.  the "From:" is generated 
by the author information in the commit.  the only way to change it is to 
amend the commit in question with --author.
-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/20100826/172ca580/attachment.pgp 

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

Dear Mike Frysinger,

> pass the bus/cs/speed/mode in to the initialize function and store it in the
> per-device state.  then you can work fine with multiple enc28j60 devices in
> one board.
>
> along those lines, all of the local enc funcs should be changed to take the
> private state and operate off that instead of random global variables.

Something like this?

typedef struct enc_device {
	struct eth_device	netdev;
	struct spi_slave	*slave;
	u16			next_pointer;
	int			rx_reset_counter;
	u8			bank;	/* current bank in enc28j60 */
} enc_dev_t;
#define to_enc(_nd) container_of(_nd, struct enc_device, netdev)

...

static u8 enc_r8(enc_dev_t *, u16 reg);
static u16 enc_r16(enc_dev_t *, u16 reg);
static void enc_w8(enc_dev_t *, u16 reg, u8 data);
static void enc_w16(enc_dev_t *, u16 reg, u16 data);
static void enc_w8_retry(enc_dev_t *, u16 reg, u8 data, int c);

I agree that's more perfect but kind of overkill, too;
assuming that only one enc can be active at any given time;)

Best Regards,
Reinhard

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Mike Frysinger]

On Friday, September 03, 2010 13:50:28 Reinhard Meyer wrote:
> Something like this?

yes

> I agree that's more perfect but kind of overkill, too;
> assuming that only one enc can be active at any given time;)

yes and no.  the spi bus/cs/mode/speed is established at initialize() time, 
not init(), so you'd need to store that per-instance information somewhere.  
and how the discussion about netconsole handling falls out (not calling 
init/halt after every transaction), this base assumption may not be valid.
-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/20100903/3300012f/attachment.pgp 

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

On 03.09.2010 21:51, Mike Frysinger wrote:
> yes and no.  the spi bus/cs/mode/speed is established at initialize() time,
> not init(), so you'd need to store that per-instance information somewhere.
> and how the discussion about netconsole handling falls out (not calling
> init/halt after every transaction), this base assumption may not be valid.
> -mike

/*
  * This is the only exported function.
  *
  * It may be called several times with different bus:cs combinations.
  */
int enc_initialize(int bus, int cs, int speed)
{
	enc_dev_t *enc;

	enc = malloc(sizeof(*enc));
	if (!enc) {
		return -1;
	}
	memset(enc, 0, sizeof(*enc));
	enc->slave = spi_setup_slave(bus, cs, speed, SPI_MODE_0);
	if (!enc->slave) {
		free(enc);
		return -1;
	}
	enc->netdev.init = enc_init;
	enc->netdev.halt = enc_halt;
	enc->netdev.send = enc_send;
	enc->netdev.recv = enc_recv;
	sprintf(enc->netdev.name, "enc%i.%i", bus, cs);
	eth_register(&(enc->netdev));
	return 0;
}
(not compiled yet)
not sure, however, if calling spi_setup_slave() is ok at this point.

Reinhard

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Mike Frysinger]

On Friday, September 03, 2010 16:31:47 Reinhard Meyer wrote:
> int enc_initialize(int bus, int cs, int speed)

should take the same parameters (including type) as spi_setup_slave

> 	eth_register(&(enc->netdev));

no need for the () before the &

> not sure, however, if calling spi_setup_slave() is ok at this point.

i believe the func should only doing validation on the arguments.  it shouldnt 
need to talk to any actual hardware.  maybe this needs codifying in the API 
documentation.
-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/20100903/3bb55261/attachment.pgp 

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

On 03.09.2010 23:03, Mike Frysinger wrote:
>> not sure, however, if calling spi_setup_slave() is ok at this point.
>
> i believe the func should only doing validation on the arguments.  it shouldnt
> need to talk to any actual hardware.  maybe this needs codifying in the API
> documentation.
In atmel_spi.c it sets up the csr[indexed by cs=0..3] register with clock,
phase and polarity.
That's harmless, but might need some rethinking there.

Reinhard

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Mike Frysinger]

On Friday, September 03, 2010 17:23:43 Reinhard Meyer wrote:
> On 03.09.2010 23:03, Mike Frysinger wrote:
> >> not sure, however, if calling spi_setup_slave() is ok at this point.
> > 
> > i believe the func should only doing validation on the arguments.  it
> > shouldnt need to talk to any actual hardware.  maybe this needs
> > codifying in the API documentation.
> 
> In atmel_spi.c it sets up the csr[indexed by cs=0..3] register with clock,
> phase and polarity.
> That's harmless, but might need some rethinking there.

i'm not familiar with the Atmel SPI controller, but my gut reaction is that 
register values should be calculated in spi_setup_slave(), stored in the 
internal xxx_spi_slave struct, and then written at spi_claim_bus() time.
-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/20100903/dd926dab/attachment.pgp 

[U-Boot] [PATCH] NET: add ENC28J60 driver using SPI framework

[Reinhard Meyer]

On 04.09.2010 00:14, Mike Frysinger wrote:
> On Friday, September 03, 2010 17:23:43 Reinhard Meyer wrote:
>> On 03.09.2010 23:03, Mike Frysinger wrote:
>>>> not sure, however, if calling spi_setup_slave() is ok at this point.
>>>
>>> i believe the func should only doing validation on the arguments.  it
>>> shouldnt need to talk to any actual hardware.  maybe this needs
>>> codifying in the API documentation.
>>
>> In atmel_spi.c it sets up the csr[indexed by cs=0..3] register with clock,
>> phase and polarity.
>> That's harmless, but might need some rethinking there.
>
> i'm not familiar with the Atmel SPI controller, but my gut reaction is that
> register values should be calculated in spi_setup_slave(), stored in the
> internal xxx_spi_slave struct, and then written at spi_claim_bus() time.
> -mike
Well, the SPI hardware can store the settings for 4 chip selects, and quite
automagically switch between them, including programmable switch times. When
using DMA it would be possible to setup transfers to/from different chip
selects without software doing the switches, ex:
15 words of 13 bits using 199kHz and mode 3 via CS2,
2 words of 8 bits using 10MHz and mode 0 via CS3, etc...
All this, of course, is not needed in u-boot and currently only half used
anyway. Therefore I tend to rework the atmel_spi.c not to make use of that
feature at all.

For codig the ENC driver I will just postulate its save to setup the slave
at initialize time.

Reinhard