On 3/23/2010 11:09 PM, Ben Warren wrote:
Hi Vipin,
Hello Ben,
On 3/23/2010 1:30 AM, Vipin KUMAR wrote:
SPEAr SoCs support a synopsys network peripheral. This patch adds the support for the same
Signed-off-by: Vipin Kumarvipin.kumar@st.com
board/spear/spear300/spear300.c | 6 + board/spear/spear310/spear310.c | 6 + board/spear/spear320/spear320.c | 6 + board/spear/spear600/spear600.c | 6 + drivers/net/Makefile | 1 + drivers/net/spr_eth_syn.c | 471 ++++++++++++++++++++++++++++++ include/asm-arm/arch-spear/hardware.h | 2 + include/asm-arm/arch-spear/spr_eth_syn.h | 262 +++++++++++++++++ include/configs/spear-common.h | 24 ++- include/configs/spear3xx.h | 3 + include/netdev.h | 1 + 11 files changed, 785 insertions(+), 3 deletions(-) create mode 100755 drivers/net/spr_eth_syn.c create mode 100644 include/asm-arm/arch-spear/spr_eth_syn.h
I'd prefer to see the driver and its associated files submitted as a single patch, an board support added later, but it's not a big deal.
No problem, I would submit 2 patches in v2 1 for network driver support and other for board specific changes
diff --git a/drivers/net/Makefile b/drivers/net/Makefile index 1ec0ba1..b230874 100644 --- a/drivers/net/Makefile +++ b/drivers/net/Makefile @@ -68,6 +68,7 @@ COBJS-$(CONFIG_DRIVER_S3C4510_ETH) += s3c4510b_eth.o COBJS-$(CONFIG_SH_ETHER) += sh_eth.o COBJS-$(CONFIG_SMC91111) += smc91111.o COBJS-$(CONFIG_SMC911X) += smc911x.o +COBJS-$(CONFIG_SPEAR_ETHERNET) += spr_eth_syn.o
Please ensure that the CONFIG and driver name convey the same information (these ones don't)
OK
- */
+#include<common.h> +#include<miiphy.h> +#include<malloc.h> +#include<asm/errno.h> +#include<asm/io.h> +#include<asm/arch/hardware.h> +#include<asm/arch/spr_eth_syn.h>
+static struct eth_dma_regs *dma_p =
- (struct eth_dma_regs *)CONFIG_SPEAR_ETHDMABASE;
+static struct eth_mac_regs *mac_p =
- (struct eth_mac_regs *)CONFIG_SPEAR_ETHMACBASE;
No globals please. It makes it impossible to have more than one instance of the driver. Put data in dev->priv instead.
OK. I would use allocate memory dynamically for patch version 2
+static struct eth_device *netdev;
+static struct dmamacdescr
- tx_mac_descrtable[CONFIG_TX_DESCR_NUM] __attribute__
((aligned(256))); +static struct dmamacdescr
- rx_mac_descrtable[CONFIG_RX_DESCR_NUM] __attribute__
((aligned(256)));
+static u32 tx_currdescnum; +static u32 rx_currdescnum;
+static char txbuffs[TX_TOTAL_BUFSIZE] __attribute__ ((aligned(2048))); +static char rxbuffs[RX_TOTAL_BUFSIZE] __attribute__ ((aligned(2048)));
+static void xfer_enable(void)
All functions should take eth_device * as argument so you can access private data
OK
+}
+static void descs_init(void) +{
- tx_descs_init(CONFIG_TX_DESCR_NUM);
- rx_descs_init(CONFIG_RX_DESCR_NUM);
+}
+static int set_hwaddr(void) +{
- u8 mac_id[6];
- u32 macid_lo, macid_hi;
- if (eth_getenv_enetaddr("ethaddr", mac_id)) {
Please read the documentation and look at other drivers to see how they handle the environment. Hint: you get it from the eth_device *.
OK
+static void spear_reset_phy(struct eth_device *dev) +{
- struct spear_eth_dev *priv = dev->priv;
- u16 ctrl;
- u32 timeout = CONFIG_PHYRESET_TIMEOUT;
- u32 phy_addr = priv->address;
- eth_mdio_write(dev->name, phy_addr, PHY_BMCR, PHY_BMCR_RESET);
- do {
eth_mdio_read(dev->name, phy_addr, PHY_BMCR,&ctrl);if (!(ctrl& PHY_BMCR_RESET))break;udelay(1000);- } while (timeout--);
- udelay(CONFIG_PHY_RESET_DELAY);
+}
+void reset_phy(void)
Something with such a generic name should not be public
This is a standard global u-boot function which should be defined if CONFIG_RESET_PHY_R is #defined
+{
- spear_reset_phy(netdev);
+}
+static void configure_phy(struct eth_device *dev) +{
- struct spear_eth_dev *priv = dev->priv;
- char *an, *sp, *du;
- u8 phy_addr;
- u16 bmcr, bmsr, ctrl;
- u32 timeout;
- u16 anlpar, btsr;
- priv->address = find_phy(dev);
- phy_addr = priv->address;
- spear_reset_phy(netdev);
- an = getenv("ethautoneg");
Is this a new environment variable? If so, it needs to be interface #-specific, which is tricky. Does this really need to come from the environment? Same goes for all of these... Since you're using the U-boot MII framework, I'd much rather see the common code changed to support this than a specific driver.
Another way is to provide compile time switches. I wanted to keep this changeable on the board. So that the code does not need a recompile to change this behavior I would provide compile switches which can be defined along with board configuration
- if ((an != NULL)&& strcmp(an, "n")) {
bmcr = PHY_BMCR_AUTON | PHY_BMCR_RST_NEG | PHY_BMCR_100MB | \PHY_BMCR_DPLX | PHY_BMCR_1000_MBPS;- } else {
bmcr = PHY_BMCR_100MB | PHY_BMCR_DPLX;sp = getenv("ethspeed");if ((sp != NULL)&& (!strcmp(sp, "10M")))bmcr&= ~PHY_BMCR_100MB;du = getenv("ethduplex");if ((du != NULL)&& (!strcmp(du, "half")))bmcr&= ~PHY_BMCR_DPLX;- }
- eth_mdio_write(dev->name, phy_addr, PHY_BMCR, bmcr);
- /* Read the phy status register and populate priv structure */
- if ((an != NULL)&& strcmp(an, "n")) {
timeout = CONFIG_AUTONEG_TIMEOUT;do {eth_mdio_read(dev->name, phy_addr, PHY_BMSR,&bmsr);if (bmsr& PHY_BMSR_AUTN_COMP)break;udelay(1000);} while (timeout--);eth_mdio_read(dev->name, phy_addr, PHY_ANLPAR,&anlpar);eth_mdio_read(dev->name, phy_addr, PHY_1000BTSR,&btsr);if (btsr& (PHY_1000BTSR_1000FD | PHY_1000BTSR_1000HD)) {priv->speed = SPEED_1000M;if (btsr& PHY_1000BTSR_1000FD)priv->duplex = FULL_DUPLEX;elsepriv->duplex = HALF_DUPLEX;} else {if (anlpar& PHY_ANLPAR_100)priv->speed = SPEED_100M;elsepriv->speed = SPEED_10M;if (anlpar& (PHY_ANLPAR_10FD | PHY_ANLPAR_TXFD))priv->duplex = FULL_DUPLEX;elsepriv->duplex = HALF_DUPLEX;}- } else {
eth_mdio_read(dev->name, phy_addr, PHY_BMCR,&ctrl);if (ctrl& PHY_BMCR_DPLX)priv->duplex = FULL_DUPLEX;elsepriv->duplex = HALF_DUPLEX;if (ctrl& PHY_BMCR_1000_MBPS)priv->speed = SPEED_1000M;else if (ctrl& PHY_BMCR_100_MBPS)priv->speed = SPEED_100M;elsepriv->speed = SPEED_10M;- }
+}
+int spear_mii_initialize(u32 id)
Please pass in the base address rather than hard coding it. Look how other drivers do this (CS8900 is a good example)
OK
+{
- struct spear_eth_dev *priv;
- netdev = (struct eth_device *) malloc(sizeof(struct eth_device));
- if (!netdev)
return -ENOMEM;- priv = (struct spear_eth_dev *) malloc(sizeof(struct
spear_eth_dev));
- if (!priv) {
free(netdev);return -ENOMEM;- }
- memset(netdev, 0, sizeof(struct eth_device));
- memset(priv, 0, sizeof(struct spear_eth_dev));
- sprintf(netdev->name, "mii%d", id);
- priv->dev = netdev;
- netdev->iobase = 0;
- netdev->priv = priv;
- mac_reset(CONFIG_MACRESET_TIMEOUT);
- configure_phy(netdev);
- netdev->init = spear_eth_init;
- netdev->send = spear_eth_send;
- netdev->recv = spear_eth_recv;
- netdev->halt = spear_eth_halt;
- eth_register(netdev);
+#if defined(CONFIG_MII) || defined(CONFIG_CMD_MII)
- miiphy_register(netdev->name, eth_mdio_read, eth_mdio_write);
+#endif
- return 0;
+} diff --git a/include/asm-arm/arch-spear/hardware.h b/include/asm-arm/arch-spear/hardware.h index 818f36c..7133b3c 100644 --- a/include/asm-arm/arch-spear/hardware.h +++ b/include/asm-arm/arch-spear/hardware.h @@ -31,6 +31,8 @@ #define CONFIG_SPEAR_SYSCNTLBASE (0xFCA00000) #define CONFIG_SPEAR_TIMERBASE (0xFC800000) #define CONFIG_SPEAR_MISCBASE (0xFCA80000) +#define CONFIG_SPEAR_ETHMACBASE (0xE0800000) +#define CONFIG_SPEAR_ETHDMABASE (0xE0801000)
#define CONFIG_SYS_NAND_CLE (1<< 16) #define CONFIG_SYS_NAND_ALE (1<< 17) diff --git a/include/asm-arm/arch-spear/spr_eth_syn.h b/include/asm-arm/arch-spear/spr_eth_syn.h
Please put this header in with the driver. You're artificially making this ARM-specific.
OK
new file mode 100644 index 0000000..3a745ef --- /dev/null +++ b/include/asm-arm/arch-spear/spr_eth_syn.h @@ -0,0 +1,262 @@ +/*
- (C) Copyright 2009
- Vipin Kumar, ST Micoelectronics, vipin.kumar@st.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 _SPR_ETH_SYN_H +#define _SPR_ETH_SYN_H
+#define CONFIG_TX_DESCR_NUM 16 +#define CONFIG_RX_DESCR_NUM 16 +#define CONFIG_ETH_BUFSIZE 2048 +#define TX_TOTAL_BUFSIZE (CONFIG_ETH_BUFSIZE * CONFIG_TX_DESCR_NUM) +#define RX_TOTAL_BUFSIZE (CONFIG_ETH_BUFSIZE * CONFIG_RX_DESCR_NUM)
+#define CONFIG_MACRESET_TIMEOUT (3 * CONFIG_SYS_HZ) +#define CONFIG_MDIO_TIMEOUT (3 * CONFIG_SYS_HZ) +#define CONFIG_PHYRESET_TIMEOUT (3 * CONFIG_SYS_HZ) +#define CONFIG_AUTONEG_TIMEOUT (5 * CONFIG_SYS_HZ)
+struct eth_mac_regs {
- u32 conf; /* 0x00 */
- u32 framefilt; /* 0x04 */
- u32 hashtablehigh; /* 0x08 */
- u32 hashtablelow; /* 0x0c */
- u32 miiaddr; /* 0x10 */
- u32 miidata; /* 0x14 */
- u32 flowcontrol; /* 0x18 */
- u32 vlantag; /* 0x1c */
- u32 version; /* 0x20 */
- u8 reserved_1[4];
- u32 intreg; /* 0x28 */
- u32 intmask; /* 0x2c */
- u8 reserved_2[0x40 - 0x30];
- u32 macaddr0hi; /* 0x30 */
- u32 macaddr0lo; /* 0x34 */
+};
+/* MAC configuration register definitions */ +#define FRAMEBURSTENABLE (1<< 21) +#define MII_PORTSELECT (1<< 15) +#define DISABLERXOWN (1<< 13) +#define FULLDPLXMODE (1<< 11) +#define RXENABLE (1<< 2) +#define TXENABLE (1<< 3)
+/* MII address register definitions */ +#define MII_BUSY (1<< 0) +#define MII_WRITE (1<< 1) +#define MII_CLKRANGE_60_100M (0) +#define MII_CLKRANGE_100_150M (0x4) +#define MII_CLKRANGE_20_35M (0x8) +#define MII_CLKRANGE_35_60M (0xC) +#define MII_CLKRANGE_150_250M (0x10) +#define MII_CLKRANGE_250_300M (0x14)
+#define MIIADDRSHIFT (11) +#define MIIREGSHIFT (6) +#define MII_REGMSK (0x1F<< 6) +#define MII_ADDRMSK (0x1F<< 11)
+struct eth_dma_regs {
- u32 busmode; /* 0x00 */
- u32 txpolldemand; /* 0x04 */
- u32 rxpolldemand; /* 0x08 */
- u32 rxdesclistaddr; /* 0x0c */
- u32 txdesclistaddr; /* 0x10 */
- u32 status; /* 0x14 */
- u32 opmode; /* 0x18 */
- u32 intenable; /* 0x1c */
- u8 reserved[0x48 - 0x20];
- u32 currhosttxdesc; /* 0x48 */
- u32 currhostrxdesc; /* 0x4c */
- u32 currhosttxbuffaddr; /* 0x50 */
- u32 currhostrxbuffaddr; /* 0x54 */
+};
+/* Bus mode register definitions */ +#define FIXEDBURST (1<< 16) +#define BURST_1 (1<< 8) +#define BURST_2 (2<< 8) +#define BURST_4 (4<< 8) +#define BURST_8 (8<< 8) +#define BURST_16 (16<< 8) +#define BURST_32 (32<< 8) +#define RXHIGHPRIO (1<< 1) +#define DMAMAC_SRST (1<< 0)
+/* Poll demand definitions */ +#define POLL_DATA (0xFFFFFFFF)
+/* Operation mode definitions */ +#define STOREFORWARD (1<< 21) +#define FLUSHTXFIFO (1<< 20) +#define TXSTART (1<< 13) +#define TXSECONDFRAME (1<< 2) +#define RXSTART (1<< 1)
+/* Descriptior related definitions */ +#define MAC_MAX_FRAME_SZ (1528)
+struct dmamacdescr {
- u32 txrx_status;
- u32 dmamac_cntl;
- void *dmamac_addr;
- struct dmamacdescr *dmamac_next;
+};
+/*
- txrx_status definitions
- */
+/* tx status bits definitions */ +#if defined(CONFIG_SPEAR_ALTDESC)
+#define DESC_TXSTS_OWNBYDMA (1<< 31) +#define DESC_TXSTS_TXINT (1<< 30) +#define DESC_TXSTS_TXLAST (1<< 29) +#define DESC_TXSTS_TXFIRST (1<< 28) +#define DESC_TXSTS_TXCRCDIS (1<< 27)
+#define DESC_TXSTS_TXPADDIS (1<< 26) +#define DESC_TXSTS_TXCHECKINSCTRL (3<< 22) +#define DESC_TXSTS_TXRINGEND (1<< 21) +#define DESC_TXSTS_TXCHAIN (1<< 20) +#define DESC_TXSTS_MSK (0x1FFFF<< 0)
+#else
+#define DESC_TXSTS_OWNBYDMA (1<< 31) +#define DESC_TXSTS_MSK (0x1FFFF<< 0)
+#endif
+/* rx status bits definitions */ +#define DESC_RXSTS_OWNBYDMA (1<< 31) +#define DESC_RXSTS_DAFILTERFAIL (1<< 30) +#define DESC_RXSTS_FRMLENMSK (0x3FFF<< 16) +#define DESC_RXSTS_FRMLENSHFT (16)
+#define DESC_RXSTS_ERROR (1<< 15) +#define DESC_RXSTS_RXTRUNCATED (1<< 14) +#define DESC_RXSTS_SAFILTERFAIL (1<< 13) +#define DESC_RXSTS_RXIPC_GIANTFRAME (1<< 12) +#define DESC_RXSTS_RXDAMAGED (1<< 11) +#define DESC_RXSTS_RXVLANTAG (1<< 10) +#define DESC_RXSTS_RXFIRST (1<< 9) +#define DESC_RXSTS_RXLAST (1<< 8) +#define DESC_RXSTS_RXIPC_GIANT (1<< 7) +#define DESC_RXSTS_RXCOLLISION (1<< 6) +#define DESC_RXSTS_RXFRAMEETHER (1<< 5) +#define DESC_RXSTS_RXWATCHDOG (1<< 4) +#define DESC_RXSTS_RXMIIERROR (1<< 3) +#define DESC_RXSTS_RXDRIBBLING (1<< 2) +#define DESC_RXSTS_RXCRC (1<< 1)
+/*
- dmamac_cntl definitions
- */
+/* tx control bits definitions */ +#if defined(CONFIG_SPEAR_ALTDESC)
+#define DESC_TXCTRL_SIZE1MASK (0x1FFF<< 0) +#define DESC_TXCTRL_SIZE1SHFT (0) +#define DESC_TXCTRL_SIZE2MASK (0x1FFF<< 16) +#define DESC_TXCTRL_SIZE2SHFT (16)
+#else
+#define DESC_TXCTRL_TXINT (1<< 31) +#define DESC_TXCTRL_TXLAST (1<< 30) +#define DESC_TXCTRL_TXFIRST (1<< 29) +#define DESC_TXCTRL_TXCHECKINSCTRL (3<< 27) +#define DESC_TXCTRL_TXCRCDIS (1<< 26) +#define DESC_TXCTRL_TXRINGEND (1<< 25) +#define DESC_TXCTRL_TXCHAIN (1<< 24)
+#define DESC_TXCTRL_SIZE1MASK (0x7FF<< 0) +#define DESC_TXCTRL_SIZE1SHFT (0) +#define DESC_TXCTRL_SIZE2MASK (0x7FF<< 11) +#define DESC_TXCTRL_SIZE2SHFT (11)
+#endif
+/* rx control bits definitions */ +#if defined(CONFIG_SPEAR_ALTDESC)
+#define DESC_RXCTRL_RXINTDIS (1<< 31) +#define DESC_RXCTRL_RXRINGEND (1<< 15) +#define DESC_RXCTRL_RXCHAIN (1<< 14)
+#define DESC_RXCTRL_SIZE1MASK (0x1FFF<< 0) +#define DESC_RXCTRL_SIZE1SHFT (0) +#define DESC_RXCTRL_SIZE2MASK (0x1FFF<< 16) +#define DESC_RXCTRL_SIZE2SHFT (16)
+#else
+#define DESC_RXCTRL_RXINTDIS (1<< 31) +#define DESC_RXCTRL_RXRINGEND (1<< 25) +#define DESC_RXCTRL_RXCHAIN (1<< 24)
+#define DESC_RXCTRL_SIZE1MASK (0x7FF<< 0) +#define DESC_RXCTRL_SIZE1SHFT (0) +#define DESC_RXCTRL_SIZE2MASK (0x7FF<< 11) +#define DESC_RXCTRL_SIZE2SHFT (11)
+#endif
+/*
- dmamac_addr definitions
- */
+/* tx addr register bits definitions */
+/* rx addr register bits definitions */
+/*
- dmamac_next definitions
- */
+/* tx next register bits definitions */
+/* rx next register bits definitions */
+struct spear_eth_dev {
- u32 address;
- u32 speed;
- u32 duplex;
- struct eth_device *dev;
+};
+/* Speed specific definitions */ +#define SPEED_10M 1 +#define SPEED_100M 2 +#define SPEED_1000M 3
+/* Duplex mode specific definitions */ +#define HALF_DUPLEX 1 +#define FULL_DUPLEX 2
+#endif diff --git a/include/configs/spear-common.h b/include/configs/spear-common.h index cc52e39..fe97e94 100644 --- a/include/configs/spear-common.h +++ b/include/configs/spear-common.h @@ -27,6 +27,19 @@
- Common configurations used for both spear3xx as well as spear6xx
*/
+/* Ethernet configuration */ +#define CONFIG_SPEAR_ETHERNET +#define CONFIG_MII +#define CONFIG_NET_MULTI +#define CONFIG_PHY_RESET_DELAY (10000) /* in us */ +#define CONFIG_EXTRA_ENV_AUTONEG "ethautoneg=n\0" +#define CONFIG_EXTRA_ENV_ETHSPEED "ethspeed=100M\0" +#define CONFIG_EXTRA_ENV_ETHDUPLEX "ethduplex=full\0" +#define CONFIG_ETHADDR 00:80:E1:11:22:33 +#define CONFIG_NETMASK 255.255.255.0 +#define CONFIG_IPADDR 192.168.1.10 +#define CONFIG_SERVERIP 192.168.1.1
Please remove all default addresses/masks
Could not understand this point. These definitions are needed and are done in the same way in board configuration files. Am I missing something ?
- /* USBD driver configuration */ #define CONFIG_SPEARUDC #define CONFIG_USB_DEVICE
@@ -99,11 +112,13 @@ #define CONFIG_CMD_MEMORY #define CONFIG_CMD_RUN #define CONFIG_CMD_SAVES +#define CONFIG_CMD_MII +#define CONFIG_CMD_NET +#define CONFIG_CMD_PING +#define CONFIG_CMD_DHCP
/* This must be included AFTER the definition of CONFIG_COMMANDS (if any) */ #include<config_cmd_default.h> -#undef CONFIG_CMD_NET -#undef CONFIG_CMD_NFS
/*
- Default Environment Varible definitions
@@ -195,7 +210,10 @@ #define CONFIG_SYS_CONSOLE_INFO_QUIET 1 #define CONFIG_SYS_64BIT_VSPRINTF 1
-#define CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_USBTTY +#define CONFIG_EXTRA_ENV_SETTINGS CONFIG_EXTRA_ENV_USBTTY \
CONFIG_EXTRA_ENV_AUTONEG \CONFIG_EXTRA_ENV_ETHSPEED \CONFIG_EXTRA_ENV_ETHDUPLEX/* Stack sizes */ #define CONFIG_STACKSIZE (128*1024)
diff --git a/include/configs/spear3xx.h b/include/configs/spear3xx.h index 0248aba..b69e734 100755 --- a/include/configs/spear3xx.h +++ b/include/configs/spear3xx.h @@ -41,6 +41,9 @@
#include<configs/spear-common.h>
+/* Ethernet configuration */ +#define CONFIG_SPEAR_ALTDESC
- /* Serial Configuration (PL011) */ #define CONFIG_SYS_SERIAL0 0xD0000000
diff --git a/include/netdev.h b/include/netdev.h index 1dd80f0..bd29697 100644 --- a/include/netdev.h +++ b/include/netdev.h @@ -79,6 +79,7 @@ int scc_initialize(bd_t *bis); int skge_initialize(bd_t *bis); int smc911x_initialize(u8 dev_num, int base_addr); int smc91111_initialize(u8 dev_num, int base_addr); +int spear_mii_initialize(u32);
Please add a variable name
OK
int tsi108_eth_initialize(bd_t *bis); int uec_initialize(int index); int uec_standard_init(bd_t *bis);