[U-Boot] [PATCH v2 0/2] Added CPSW support

[U-Boot] [PATCH v2 0/2] Added CPSW support

[Chandan Nath]

This patch series is submitted to add support for CPSW which is
an on-chip ethernet switch on TI AM335X EVM.

The patches have been compile tested and run on AM335X EVM.

The patches depends on previous patch series which was submitted
for supporting AM33xx platform.

Changes since v1:
 - Used eth_setenv_enetaddr function and code cleanup
 - Added weak alias miiphy_is_1000base_x function

Chandan Nath (1):
  ARM:AM33XX:Added cpsw support for AM335x EVM

Cyril Chemparathy (1):
  TI: netdev: add driver for cpsw ethernet device

 arch/arm/cpu/armv7/am33xx/clock.c           |    9 +-
 arch/arm/include/asm/arch-am33xx/cpu.h      |   22 +-
 arch/arm/include/asm/arch-am33xx/hardware.h |    5 +
 board/ti/am335x/common_def.h                |    2 +
 board/ti/am335x/evm.c                       |  149 +++++
 board/ti/am335x/mux.c                       |   24 +
 common/miiphyutil.c                         |    5 +-
 drivers/net/Makefile                        |    1 +
 drivers/net/cpsw.c                          |  919 +++++++++++++++++++++++++++
 include/configs/am335x_evm.h                |   16 +-
 include/netdev.h                            |   36 +
 11 files changed, 1182 insertions(+), 6 deletions(-)
 create mode 100644 drivers/net/cpsw.c

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Chandan Nath]

From: Cyril Chemparathy <cyril@ti.com>

CPSW is an on-chip ethernet switch that is found on various SoCs from Texas
Instruments.  This patch adds a simple driver (based on the Linux driver) for
this hardware module.

Signed-off-by: Cyril Chemparathy <cyril@ti.com>
---
Changes since v1:
 - Code cleanup and removal of cpsw_eth_set_mac_addr function

 drivers/net/Makefile |    1 +
 drivers/net/cpsw.c   |  862 ++++++++++++++++++++++++++++++++++++++++++++++++++
 include/netdev.h     |   29 ++
 3 files changed, 892 insertions(+), 0 deletions(-)
 create mode 100644 drivers/net/cpsw.c

diff --git a/drivers/net/Makefile b/drivers/net/Makefile
index 819b197..080f767 100644
--- a/drivers/net/Makefile
+++ b/drivers/net/Makefile
@@ -80,6 +80,7 @@ COBJS-$(CONFIG_TIGON3) += bcm570x_autoneg.o
 COBJS-$(CONFIG_TIGON3) += 5701rls.o
 COBJS-$(CONFIG_DRIVER_TI_EMAC) += davinci_emac.o
 COBJS-$(CONFIG_TSEC_ENET) += tsec.o fsl_mdio.o
+COBJS-$(CONFIG_DRIVER_TI_CPSW) += cpsw.o
 COBJS-$(CONFIG_TSI108_ETH) += tsi108_eth.o
 COBJS-$(CONFIG_ULI526X) += uli526x.o
 COBJS-$(CONFIG_VSC7385_ENET) += vsc7385.o
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
new file mode 100644
index 0000000..4cef4cf
--- /dev/null
+++ b/drivers/net/cpsw.c
@@ -0,0 +1,862 @@
+/*
+ * CPSW Ethernet Switch Driver
+ *
+ * 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 <common.h>
+#include <command.h>
+#include <net.h>
+#include <miiphy.h>
+#include <malloc.h>
+#include <net.h>
+#include <netdev.h>
+#include <asm/errno.h>
+#include <asm/io.h>
+#include <linux/mii.h>
+
+#define BIT(x)			(1 << (x))
+#define BITMASK(bits)		(BIT(bits) - 1)
+#define PHY_REG_MASK		0x1f
+#define PHY_ID_MASK		0x1f
+#define NUM_DESCS		(PKTBUFSRX * 2)
+#define PKT_MIN			60
+#define PKT_MAX			(1500 + 14 + 4 + 4)
+
+/* DMA Registers */
+#define CPDMA_TXCONTROL		0x004
+#define CPDMA_RXCONTROL		0x014
+#define CPDMA_SOFTRESET		0x01c
+#define CPDMA_RXFREE		0x0e0
+#define CPDMA_TXHDP		0x100
+#define CPDMA_RXHDP		0x120
+#define CPDMA_TXCP		0x140
+#define CPDMA_RXCP		0x160
+
+/* Descriptor mode bits */
+#define CPDMA_DESC_SOP		BIT(31)
+#define CPDMA_DESC_EOP		BIT(30)
+#define CPDMA_DESC_OWNER	BIT(29)
+#define CPDMA_DESC_EOQ		BIT(28)
+
+struct cpsw_mdio_regs {
+	u32	version;
+	u32	control;
+#define CONTROL_IDLE		(1 << 31)
+#define CONTROL_ENABLE		(1 << 30)
+
+	u32	alive;
+	u32	link;
+	u32	linkintraw;
+	u32	linkintmasked;
+	u32	__reserved_0[2];
+	u32	userintraw;
+	u32	userintmasked;
+	u32	userintmaskset;
+	u32	userintmaskclr;
+	u32	__reserved_1[20];
+
+	struct {
+		u32		access;
+		u32		physel;
+#define USERACCESS_GO		(1 << 31)
+#define USERACCESS_WRITE	(1 << 30)
+#define USERACCESS_ACK		(1 << 29)
+#define USERACCESS_READ		(0)
+#define USERACCESS_DATA		(0xffff)
+	} user[0];
+};
+
+struct cpsw_regs {
+	u32	id_ver;
+	u32	control;
+	u32	soft_reset;
+	u32	stat_port_en;
+	u32	ptype;
+};
+
+struct cpsw_slave_regs {
+	u32	max_blks;
+	u32	blk_cnt;
+	u32	flow_thresh;
+	u32	port_vlan;
+	u32	tx_pri_map;
+	u32	gap_thresh;
+	u32	sa_lo;
+	u32	sa_hi;
+};
+
+struct cpsw_host_regs {
+	u32	max_blks;
+	u32	blk_cnt;
+	u32	flow_thresh;
+	u32	port_vlan;
+	u32	tx_pri_map;
+	u32	cpdma_tx_pri_map;
+	u32	cpdma_rx_chan_map;
+};
+
+struct cpsw_sliver_regs {
+	u32	id_ver;
+	u32	mac_control;
+	u32	mac_status;
+	u32	soft_reset;
+	u32	rx_maxlen;
+	u32	__reserved_0;
+	u32	rx_pause;
+	u32	tx_pause;
+	u32	__reserved_1;
+	u32	rx_pri_map;
+};
+
+#define ALE_ENTRY_BITS		68
+#define ALE_ENTRY_WORDS		DIV_ROUND_UP(ALE_ENTRY_BITS, 32)
+
+/* ALE Registers */
+#define ALE_CONTROL		0x08
+#define ALE_UNKNOWNVLAN		0x18
+#define ALE_TABLE_CONTROL	0x20
+#define ALE_TABLE		0x34
+#define ALE_PORTCTL		0x40
+
+#define ALE_TABLE_WRITE		BIT(31)
+
+#define ALE_TYPE_FREE			0
+#define ALE_TYPE_ADDR			1
+#define ALE_TYPE_VLAN			2
+#define ALE_TYPE_VLAN_ADDR		3
+
+#define ALE_UCAST_PERSISTANT		0
+#define ALE_UCAST_UNTOUCHED		1
+#define ALE_UCAST_OUI			2
+#define ALE_UCAST_TOUCHED		3
+
+#define ALE_MCAST_FWD			0
+#define ALE_MCAST_BLOCK_LEARN_FWD	1
+#define ALE_MCAST_FWD_LEARN		2
+#define ALE_MCAST_FWD_2			3
+
+enum cpsw_ale_port_state {
+	ALE_PORT_STATE_DISABLE	= 0x00,
+	ALE_PORT_STATE_BLOCK	= 0x01,
+	ALE_PORT_STATE_LEARN	= 0x02,
+	ALE_PORT_STATE_FORWARD	= 0x03,
+};
+
+/* ALE unicast entry flags - passed into cpsw_ale_add_ucast() */
+#define ALE_SECURE	1
+#define ALE_BLOCKED	2
+
+struct cpsw_slave {
+	struct cpsw_slave_regs		*regs;
+	struct cpsw_sliver_regs		*sliver;
+	int				slave_num;
+	u32				mac_control;
+	struct cpsw_slave_data		*data;
+};
+
+struct cpdma_desc {
+	/* hardware fields */
+	u32			hw_next;
+	u32			hw_buffer;
+	u32			hw_len;
+	u32			hw_mode;
+	/* software fields */
+	u32			sw_buffer;
+	u32			sw_len;
+};
+
+struct cpdma_chan {
+	struct cpdma_desc	*head, *tail;
+	void			*hdp, *cp, *rxfree;
+};
+
+#define desc_write(desc, fld, val)	__raw_writel((u32)(val), &(desc)->fld)
+#define desc_read(desc, fld)		__raw_readl(&(desc)->fld)
+#define desc_read_ptr(desc, fld)	((void *)__raw_readl(&(desc)->fld))
+
+#define chan_write(chan, fld, val)	__raw_writel((u32)(val), (chan)->fld)
+#define chan_read(chan, fld)		__raw_readl((chan)->fld)
+#define chan_read_ptr(chan, fld)	((void *)__raw_readl((chan)->fld))
+
+struct cpsw_priv {
+	struct eth_device		*dev;
+	struct cpsw_platform_data	data;
+	int				host_port;
+
+	struct cpsw_regs		*regs;
+	void				*dma_regs;
+	struct cpsw_host_regs		*host_port_regs;
+	void				*ale_regs;
+
+	struct cpdma_desc		descs[NUM_DESCS];
+	struct cpdma_desc		*desc_free;
+	struct cpdma_chan		rx_chan, tx_chan;
+
+	struct cpsw_slave		*slaves;
+#define for_each_slave(priv, func, arg...)			\
+	do {							\
+		int idx;					\
+		for (idx = 0; idx < (priv)->data.slaves; idx++)	\
+			(func)((priv)->slaves + idx, ##arg);	\
+	} while (0)
+};
+
+static inline int cpsw_ale_get_field(u32 *ale_entry, u32 start, u32 bits)
+{
+	int idx;
+
+	idx    = start / 32;
+	start -= idx * 32;
+	idx    = 2 - idx; /* flip */
+	return (ale_entry[idx] >> start) & BITMASK(bits);
+}
+
+static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32 bits,
+				      u32 value)
+{
+	int idx;
+
+	value &= BITMASK(bits);
+	idx    = start / 32;
+	start -= idx * 32;
+	idx    = 2 - idx; /* flip */
+	ale_entry[idx] &= ~(BITMASK(bits) << start);
+	ale_entry[idx] |=  (value << start);
+}
+
+#define DEFINE_ALE_FIELD(name, start, bits)				\
+static inline int cpsw_ale_get_##name(u32 *ale_entry)			\
+{									\
+	return cpsw_ale_get_field(ale_entry, start, bits);		\
+}									\
+static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value)	\
+{									\
+	cpsw_ale_set_field(ale_entry, start, bits, value);		\
+}
+
+DEFINE_ALE_FIELD(entry_type,		60,	2)
+DEFINE_ALE_FIELD(mcast_state,		62,	2)
+DEFINE_ALE_FIELD(port_mask,		64,	3)
+DEFINE_ALE_FIELD(ucast_type,		66,	2)
+DEFINE_ALE_FIELD(port_num,		64,	2)
+DEFINE_ALE_FIELD(blocked,		63,	1)
+DEFINE_ALE_FIELD(secure,		62,	1)
+DEFINE_ALE_FIELD(mcast,			47,	1)
+
+/* The MAC address field in the ALE entry cannot be macroized as above */
+static inline void cpsw_ale_get_addr(u32 *ale_entry, u8 *addr)
+{
+	int i;
+
+	for (i = 0; i < 6; i++)
+		addr[i] = cpsw_ale_get_field(ale_entry, 40 - 8*i, 8);
+}
+
+static inline void cpsw_ale_set_addr(u32 *ale_entry, u8 *addr)
+{
+	int i;
+
+	for (i = 0; i < 6; i++)
+		cpsw_ale_set_field(ale_entry, 40 - 8*i, 8, addr[i]);
+}
+
+static int cpsw_ale_read(struct cpsw_priv *priv, int idx, u32 *ale_entry)
+{
+	int i;
+
+	__raw_writel(idx, priv->ale_regs + ALE_TABLE_CONTROL);
+
+	for (i = 0; i < ALE_ENTRY_WORDS; i++)
+		ale_entry[i] = __raw_readl(priv->ale_regs + ALE_TABLE + 4 * i);
+
+	return idx;
+}
+
+static int cpsw_ale_write(struct cpsw_priv *priv, int idx, u32 *ale_entry)
+{
+	int i;
+
+	for (i = 0; i < ALE_ENTRY_WORDS; i++)
+		__raw_writel(ale_entry[i], priv->ale_regs + ALE_TABLE + 4 * i);
+
+	__raw_writel(idx | ALE_TABLE_WRITE, priv->ale_regs + ALE_TABLE_CONTROL);
+
+	return idx;
+}
+
+static int cpsw_ale_match_addr(struct cpsw_priv *priv, u8* addr)
+{
+	u32 ale_entry[ALE_ENTRY_WORDS];
+	int type, idx;
+
+	for (idx = 0; idx < priv->data.ale_entries; idx++) {
+		u8 entry_addr[6];
+
+		cpsw_ale_read(priv, idx, ale_entry);
+		type = cpsw_ale_get_entry_type(ale_entry);
+		if (type != ALE_TYPE_ADDR && type != ALE_TYPE_VLAN_ADDR)
+			continue;
+		cpsw_ale_get_addr(ale_entry, entry_addr);
+		if (memcmp(entry_addr, addr, 6) == 0)
+			return idx;
+	}
+	return -ENOENT;
+}
+
+static int cpsw_ale_match_free(struct cpsw_priv *priv)
+{
+	u32 ale_entry[ALE_ENTRY_WORDS];
+	int type, idx;
+
+	for (idx = 0; idx < priv->data.ale_entries; idx++) {
+		cpsw_ale_read(priv, idx, ale_entry);
+		type = cpsw_ale_get_entry_type(ale_entry);
+		if (type == ALE_TYPE_FREE)
+			return idx;
+	}
+	return -ENOENT;
+}
+
+static int cpsw_ale_find_ageable(struct cpsw_priv *priv)
+{
+	u32 ale_entry[ALE_ENTRY_WORDS];
+	int type, idx;
+
+	for (idx = 0; idx < priv->data.ale_entries; idx++) {
+		cpsw_ale_read(priv, idx, ale_entry);
+		type = cpsw_ale_get_entry_type(ale_entry);
+		if (type != ALE_TYPE_ADDR && type != ALE_TYPE_VLAN_ADDR)
+			continue;
+		if (cpsw_ale_get_mcast(ale_entry))
+			continue;
+		type = cpsw_ale_get_ucast_type(ale_entry);
+		if (type != ALE_UCAST_PERSISTANT &&
+		    type != ALE_UCAST_OUI)
+			return idx;
+	}
+	return -ENOENT;
+}
+
+static int cpsw_ale_add_ucast(struct cpsw_priv *priv, u8 *addr,
+			      int port, int flags)
+{
+	u32 ale_entry[ALE_ENTRY_WORDS] = {0, 0, 0};
+	int idx;
+
+	cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_ADDR);
+	cpsw_ale_set_addr(ale_entry, addr);
+	cpsw_ale_set_ucast_type(ale_entry, ALE_UCAST_PERSISTANT);
+	cpsw_ale_set_secure(ale_entry, (flags & ALE_SECURE) ? 1 : 0);
+	cpsw_ale_set_blocked(ale_entry, (flags & ALE_BLOCKED) ? 1 : 0);
+	cpsw_ale_set_port_num(ale_entry, port);
+
+	idx = cpsw_ale_match_addr(priv, addr);
+	if (idx < 0)
+		idx = cpsw_ale_match_free(priv);
+	if (idx < 0)
+		idx = cpsw_ale_find_ageable(priv);
+	if (idx < 0)
+		return -ENOMEM;
+
+	cpsw_ale_write(priv, idx, ale_entry);
+	return 0;
+}
+
+static int cpsw_ale_add_mcast(struct cpsw_priv *priv, u8 *addr, int port_mask)
+{
+	u32 ale_entry[ALE_ENTRY_WORDS] = {0, 0, 0};
+	int idx, mask;
+
+	idx = cpsw_ale_match_addr(priv, addr);
+	if (idx >= 0)
+		cpsw_ale_read(priv, idx, ale_entry);
+
+	cpsw_ale_set_entry_type(ale_entry, ALE_TYPE_ADDR);
+	cpsw_ale_set_addr(ale_entry, addr);
+	cpsw_ale_set_mcast_state(ale_entry, ALE_MCAST_FWD_2);
+
+	mask = cpsw_ale_get_port_mask(ale_entry);
+	port_mask |= mask;
+	cpsw_ale_set_port_mask(ale_entry, port_mask);
+
+	if (idx < 0)
+		idx = cpsw_ale_match_free(priv);
+	if (idx < 0)
+		idx = cpsw_ale_find_ageable(priv);
+	if (idx < 0)
+		return -ENOMEM;
+
+	cpsw_ale_write(priv, idx, ale_entry);
+	return 0;
+}
+
+static inline void cpsw_ale_control(struct cpsw_priv *priv, int bit, int val)
+{
+	u32 tmp, mask = BIT(bit);
+
+	tmp  = __raw_readl(priv->ale_regs + ALE_CONTROL);
+	tmp &= ~mask;
+	tmp |= val ? mask : 0;
+	__raw_writel(tmp, priv->ale_regs + ALE_CONTROL);
+}
+
+#define cpsw_ale_enable(priv, val)	cpsw_ale_control(priv, 31, val)
+#define cpsw_ale_clear(priv, val)	cpsw_ale_control(priv, 30, val)
+#define cpsw_ale_vlan_aware(priv, val)	cpsw_ale_control(priv,  2, val)
+
+static inline void cpsw_ale_port_state(struct cpsw_priv *priv, int port,
+				       int val)
+{
+	int offset = ALE_PORTCTL + 4 * port;
+	u32 tmp, mask = 0x3;
+
+	tmp  = __raw_readl(priv->ale_regs + offset);
+	tmp &= ~mask;
+	tmp |= val & 0x3;
+	__raw_writel(tmp, priv->ale_regs + offset);
+}
+
+static struct cpsw_mdio_regs *mdio_regs;
+
+/* wait until hardware is ready for another user access */
+static inline u32 wait_for_user_access(void)
+{
+	u32 reg;
+
+	while ((reg = __raw_readl(&mdio_regs->user[0].access)) & USERACCESS_GO)
+		;
+
+	return reg;
+}
+
+/* wait until hardware state machine is idle */
+static inline void wait_for_idle(void)
+{
+	while ((__raw_readl(&mdio_regs->control) & CONTROL_IDLE) == 0)
+		;
+}
+
+static int cpsw_mdio_read(char *devname, unsigned char phy_id,
+			  unsigned char phy_reg, unsigned short *data)
+{
+	u32 reg;
+
+	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
+		return -EINVAL;
+
+	wait_for_user_access();
+	reg = (USERACCESS_GO | USERACCESS_READ | (phy_reg << 21) |
+	       (phy_id << 16));
+	__raw_writel(reg, &mdio_regs->user[0].access);
+	reg = wait_for_user_access();
+
+	*data = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -1;
+	return (reg & USERACCESS_ACK) ? 0 : -EIO;
+}
+
+static int cpsw_mdio_write(char *devname, unsigned char phy_id,
+			   unsigned char phy_reg, unsigned short data)
+{
+	u32 reg;
+
+	if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
+		return -EINVAL;
+
+	wait_for_user_access();
+	reg = (USERACCESS_GO | USERACCESS_WRITE | (phy_reg << 21) |
+		   (phy_id << 16) | (data & USERACCESS_DATA));
+	__raw_writel(reg, &mdio_regs->user[0].access);
+	wait_for_user_access();
+
+	return 0;
+}
+
+static void cpsw_mdio_init(char *name, u32 mdio_base, u32 div)
+{
+	mdio_regs = (struct cpsw_mdio_regs *)mdio_base;
+
+	/* set enable and clock divider */
+	__raw_writel(div | CONTROL_ENABLE, &mdio_regs->control);
+
+	/*
+	 * wait for scan logic to settle:
+	 * the scan time consists of (a) a large fixed component, and (b) a
+	 * small component that varies with the mii bus frequency.  These
+	 * were estimated using measurements at 1.1 and 2.2 MHz on tnetv107x
+	 * silicon.  Since the effect of (b) was found to be largely
+	 * negligible, we keep things simple here.
+	 */
+	udelay(1000);
+
+	miiphy_register(name, cpsw_mdio_read, cpsw_mdio_write);
+}
+
+static inline void soft_reset(void *reg)
+{
+	__raw_writel(1, reg);
+	while (__raw_readl(reg) & 1)
+		;
+}
+
+#define mac_hi(mac)	(((mac)[0] << 0) | ((mac)[1] << 8) |	\
+			 ((mac)[2] << 16) | ((mac)[3] << 24))
+#define mac_lo(mac)	(((mac)[4] << 0) | ((mac)[5] << 8))
+
+static void cpsw_set_slave_mac(struct cpsw_slave *slave,
+			       struct cpsw_priv *priv)
+{
+	__raw_writel(mac_hi(priv->dev->enetaddr), &slave->regs->sa_hi);
+	__raw_writel(mac_lo(priv->dev->enetaddr), &slave->regs->sa_lo);
+}
+
+static void cpsw_slave_update_link(struct cpsw_slave *slave,
+				   struct cpsw_priv *priv, int *link)
+{
+	char *name = priv->dev->name;
+	int phy_id = slave->data->phy_id;
+	int speed, duplex;
+	unsigned short reg;
+	u32 mac_control = 0;
+
+	if (miiphy_read(name, phy_id, MII_BMSR, &reg))
+		return; /* could not read, assume no link */
+
+	if (reg & BMSR_LSTATUS) { /* link up */
+		speed = miiphy_speed(name, phy_id);
+		duplex = miiphy_duplex(name, phy_id);
+
+		*link = 1;
+		mac_control = priv->data.mac_control;
+		if (speed == 1000)
+			mac_control |= BIT(7);	/* GIGABITEN	*/
+		if (duplex == FULL)
+			mac_control |= BIT(0);	/* FULLDUPLEXEN	*/
+	}
+
+	if (mac_control == slave->mac_control)
+		return;
+
+	if (mac_control) {
+		printf("link up on port %d, speed %d, %s duplex\n",
+				slave->slave_num, speed,
+				(duplex == FULL) ?  "full" : "half");
+	} else {
+		printf("link down on port %d\n", slave->slave_num);
+	}
+
+	__raw_writel(mac_control, &slave->sliver->mac_control);
+	slave->mac_control = mac_control;
+}
+
+static int cpsw_update_link(struct cpsw_priv *priv)
+{
+	int link = 0;
+	for_each_slave(priv, cpsw_slave_update_link, priv, &link);
+	return link;
+}
+
+static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
+{
+	soft_reset(&slave->sliver->soft_reset);
+
+	/* setup priority mapping */
+	__raw_writel(0x76543210, &slave->sliver->rx_pri_map);
+	__raw_writel(0x33221100, &slave->regs->tx_pri_map);
+
+	/* setup max packet size, and mac address */
+	__raw_writel(PKT_MAX, &slave->sliver->rx_maxlen);
+	cpsw_set_slave_mac(slave, priv);
+
+	slave->mac_control = 0;	/* no link yet */
+
+	/* enable forwarding */
+	cpsw_ale_port_state(priv, slave->slave_num, ALE_PORT_STATE_FORWARD);
+
+	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << slave->slave_num);
+
+	priv->data.phy_init(priv->dev->name, slave->data->phy_id);
+}
+
+static struct cpdma_desc *cpdma_desc_alloc(struct cpsw_priv *priv)
+{
+	struct cpdma_desc *desc = priv->desc_free;
+
+	if (desc)
+		priv->desc_free = desc_read_ptr(desc, hw_next);
+	return desc;
+}
+
+static void cpdma_desc_free(struct cpsw_priv *priv, struct cpdma_desc *desc)
+{
+	if (desc) {
+		desc_write(desc, hw_next, priv->desc_free);
+		priv->desc_free = desc;
+	}
+}
+
+static int cpdma_submit(struct cpsw_priv *priv, struct cpdma_chan *chan,
+			volatile void *buffer, int len)
+{
+	struct cpdma_desc *desc, *prev;
+	u32 mode;
+
+	desc = cpdma_desc_alloc(priv);
+	if (!desc)
+		return -ENOMEM;
+
+	if (len < PKT_MIN)
+		len = PKT_MIN;
+
+	mode = CPDMA_DESC_OWNER | CPDMA_DESC_SOP | CPDMA_DESC_EOP;
+
+	desc_write(desc, hw_next,   0);
+	desc_write(desc, hw_buffer, buffer);
+	desc_write(desc, hw_len,    len);
+	desc_write(desc, hw_mode,   mode | len);
+	desc_write(desc, sw_buffer, buffer);
+	desc_write(desc, sw_len,    len);
+
+	if (!chan->head) {
+		/* simple case - first packet enqueued */
+		chan->head = desc;
+		chan->tail = desc;
+		chan_write(chan, hdp, desc);
+		goto done;
+	}
+
+	/* not the first packet - enqueue at the tail */
+	prev = chan->tail;
+	desc_write(prev, hw_next, desc);
+	chan->tail = desc;
+
+	/* next check if EOQ has been triggered already */
+	if (desc_read(prev, hw_mode) & CPDMA_DESC_EOQ)
+		chan_write(chan, hdp, desc);
+
+done:
+	if (chan->rxfree)
+		chan_write(chan, rxfree, 1);
+	return 0;
+}
+
+static int cpdma_process(struct cpsw_priv *priv, struct cpdma_chan *chan,
+			 void **buffer, int *len)
+{
+	struct cpdma_desc *desc = chan->head;
+	u32 status;
+
+	if (!desc)
+		return -ENOENT;
+
+	status	= desc_read(desc, hw_mode);
+
+	if (len)
+		*len = status & 0x7ff;
+
+	if (buffer)
+		*buffer = desc_read_ptr(desc, sw_buffer);
+
+	if (status & CPDMA_DESC_OWNER)
+		return -EBUSY;
+
+	chan->head = desc_read_ptr(desc, hw_next);
+	chan_write(chan, cp, desc);
+
+	cpdma_desc_free(priv, desc);
+	return 0;
+}
+
+static int cpsw_init(struct eth_device *dev, bd_t *bis)
+{
+	struct cpsw_priv	*priv = dev->priv;
+	int i, ret;
+
+	priv->data.control(1);
+
+	/* soft reset the controller and initialize priv */
+	soft_reset(&priv->regs->soft_reset);
+
+	/* initialize and reset the address lookup engine */
+	cpsw_ale_enable(priv, 1);
+	cpsw_ale_clear(priv, 1);
+	cpsw_ale_vlan_aware(priv, 0); /* vlan unaware mode */
+
+	/* setup host port priority mapping */
+	__raw_writel(0x76543210, &priv->host_port_regs->cpdma_tx_pri_map);
+	__raw_writel(0, &priv->host_port_regs->cpdma_rx_chan_map);
+
+	/* disable priority elevation and enable statistics on all ports */
+	__raw_writel(0, &priv->regs->ptype);
+
+	/* enable statistics collection only on the host port */
+	__raw_writel(BIT(priv->host_port), &priv->regs->stat_port_en);
+
+	cpsw_ale_port_state(priv, priv->host_port, ALE_PORT_STATE_FORWARD);
+
+	cpsw_ale_add_ucast(priv, priv->dev->enetaddr, priv->host_port,
+			   ALE_SECURE);
+	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << priv->host_port);
+
+	for_each_slave(priv, cpsw_slave_init, priv);
+
+	cpsw_update_link(priv);
+
+	/* init descriptor pool */
+	for (i = 0; i < NUM_DESCS; i++) {
+		desc_write(&priv->descs[i], hw_next,
+			   (i == (NUM_DESCS - 1)) ? 0 : &priv->descs[i+1]);
+	}
+	priv->desc_free = &priv->descs[0];
+
+	/* initialize channels */
+	memset(&priv->rx_chan, 0, sizeof(struct cpdma_chan));
+	priv->rx_chan.hdp	= priv->dma_regs + CPDMA_RXHDP;
+	priv->rx_chan.cp	= priv->dma_regs + CPDMA_RXCP;
+	priv->rx_chan.rxfree	= priv->dma_regs + CPDMA_RXFREE;
+
+	memset(&priv->tx_chan, 0, sizeof(struct cpdma_chan));
+	priv->tx_chan.hdp	= priv->dma_regs + CPDMA_TXHDP;
+	priv->tx_chan.cp	= priv->dma_regs + CPDMA_TXCP;
+
+	/* clear dma state */
+	soft_reset(priv->dma_regs + CPDMA_SOFTRESET);
+
+	for (i = 0; i < priv->data.channels; i++) {
+		__raw_writel(0, priv->dma_regs + CPDMA_RXHDP + 4 * i);
+		__raw_writel(0, priv->dma_regs + CPDMA_RXFREE + 4 * i);
+		__raw_writel(0, priv->dma_regs + CPDMA_RXCP + 4 * i);
+		__raw_writel(0, priv->dma_regs + CPDMA_TXHDP + 4 * i);
+		__raw_writel(0, priv->dma_regs + CPDMA_TXCP + 4 * i);
+	}
+	__raw_writel(1, priv->dma_regs + CPDMA_TXCONTROL);
+	__raw_writel(1, priv->dma_regs + CPDMA_RXCONTROL);
+
+	/* submit rx descs */
+	for (i = 0; i < PKTBUFSRX; i++) {
+		ret = cpdma_submit(priv, &priv->rx_chan, NetRxPackets[i],
+				   PKTSIZE);
+		if (ret < 0) {
+			printf("error %d submitting rx desc\n", ret);
+			break;
+		}
+	}
+
+	return 0;
+}
+
+static void cpsw_halt(struct eth_device *dev)
+{
+	struct cpsw_priv	*priv = dev->priv;
+	priv->data.control(0);
+}
+
+static int cpsw_send(struct eth_device *dev, volatile void *packet, int length)
+{
+	struct cpsw_priv	*priv = dev->priv;
+	void *buffer;
+	int len;
+
+	if (!cpsw_update_link(priv))
+		return -EIO;
+
+	/* first reap completed packets */
+	while (cpdma_process(priv, &priv->tx_chan, &buffer, &len) >= 0)
+		;
+
+	return cpdma_submit(priv, &priv->tx_chan, packet, length);
+}
+
+static int cpsw_recv(struct eth_device *dev)
+{
+	struct cpsw_priv	*priv = dev->priv;
+	void *buffer;
+	int len;
+
+	cpsw_update_link(priv);
+
+	while (cpdma_process(priv, &priv->rx_chan, &buffer, &len) >= 0) {
+		NetReceive(buffer, len);
+		cpdma_submit(priv, &priv->rx_chan, buffer, PKTSIZE);
+	}
+
+	return 0;
+}
+
+static void cpsw_slave_setup(struct cpsw_slave *slave, int slave_num,
+			    struct cpsw_priv *priv)
+{
+	void			*regs = priv->regs;
+	struct cpsw_slave_data	*data = priv->data.slave_data + slave_num;
+
+	slave->slave_num = slave_num;
+	slave->data	= data;
+	slave->regs	= regs + data->slave_reg_ofs;
+	slave->sliver	= regs + data->sliver_reg_ofs;
+}
+
+int cpsw_register(struct cpsw_platform_data *data)
+{
+	struct cpsw_priv	*priv;
+	void			*regs = (void *)data->cpsw_base;
+	struct eth_device	*dev;
+
+	dev = malloc(sizeof(*dev));
+	if (!dev)
+		return -ENOMEM;
+	memset(dev, 0, sizeof(*dev));
+
+	priv = malloc(sizeof(*priv));
+	if (!priv) {
+		free(dev);
+		return -ENOMEM;
+	}
+	memset(priv, 0, sizeof(*priv));
+
+	priv->data = *data;
+	priv->dev  = dev;
+
+	priv->slaves = malloc(sizeof(struct cpsw_slave) * data->slaves);
+	if (!priv->slaves) {
+		free(dev);
+		free(priv);
+		return -ENOMEM;
+	}
+
+	priv->host_port		= data->slaves;
+	priv->regs		= regs;
+	priv->host_port_regs	= regs + data->host_port_reg_ofs;
+	priv->dma_regs		= regs + data->cpdma_reg_ofs;
+	priv->ale_regs		= regs + data->ale_reg_ofs;
+
+	for_each_slave(priv, cpsw_slave_setup, idx, priv);
+
+	strcpy(dev->name, "cpsw");
+	dev->iobase	= 0;
+	dev->init	= cpsw_init;
+	dev->halt	= cpsw_halt;
+	dev->send	= cpsw_send;
+	dev->recv	= cpsw_recv;
+	dev->priv	= priv;
+
+	eth_register(dev);
+
+	cpsw_mdio_init(dev->name, data->mdio_base, data->mdio_div);
+
+	return 1;
+}
diff --git a/include/netdev.h b/include/netdev.h
index 96c7b9b..740213e 100644
--- a/include/netdev.h
+++ b/include/netdev.h
@@ -185,4 +185,33 @@ struct mv88e61xx_config {
 int mv88e61xx_switch_initialize(struct mv88e61xx_config *swconfig);
 #endif /* CONFIG_MV88E61XX_SWITCH */
 
+#ifdef CONFIG_DRIVER_TI_CPSW
+
+struct cpsw_slave_data {
+	u32		slave_reg_ofs;
+	u32		sliver_reg_ofs;
+	int		phy_id;
+};
+
+struct cpsw_platform_data {
+	u32	mdio_base;
+	u32	cpsw_base;
+	int	mdio_div;
+	int	channels;       /* number of cpdma channels (symmetric)	*/
+	u32     cpdma_reg_ofs;  /* cpdma register offset		*/
+	int     slaves;         /* number of slave cpgmac ports		*/
+	u32     ale_reg_ofs;    /* address lookup engine reg offset	*/
+	int     ale_entries;	/* ale table size			*/
+	u32	host_port_reg_ofs;	/* cpdma host port registers	*/
+	u32	hw_stats_reg_ofs;	/* cpsw hw stats counters	*/
+	u32	mac_control;
+	struct cpsw_slave_data  *slave_data;
+	void	(*control)(int enabled);
+	void	(*phy_init)(char *name, int addr);
+};
+
+int cpsw_register(struct cpsw_platform_data *data);
+
+#endif /* CONFIG_DRIVER_TI_CPSW */
+
 #endif /* _NETDEV_H_ */
-- 
1.7.1

[U-Boot] [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for AM335x EVM

[Chandan Nath]

This patch adds cpsw support on AM335X EVM.

Signed-off-by: Chandan Nath <chandan.nath@ti.com>
---
Changes since v1:
 - Added weak alias miiphy_is_1000base_x function to fix atheros phy issue

 arch/arm/cpu/armv7/am33xx/clock.c           |    9 ++-
 arch/arm/include/asm/arch-am33xx/cpu.h      |   22 ++++-
 arch/arm/include/asm/arch-am33xx/hardware.h |    5 +
 board/ti/am335x/common_def.h                |    2 +
 board/ti/am335x/evm.c                       |  149 +++++++++++++++++++++++++++
 board/ti/am335x/mux.c                       |   24 +++++
 common/miiphyutil.c                         |    5 +-
 drivers/net/cpsw.c                          |  147 ++++++++++++++++++--------
 include/configs/am335x_evm.h                |   16 +++-
 include/netdev.h                            |   19 +++-
 10 files changed, 341 insertions(+), 57 deletions(-)

diff --git a/arch/arm/cpu/armv7/am33xx/clock.c b/arch/arm/cpu/armv7/am33xx/clock.c
index 4ca6c45..73b1408 100644
--- a/arch/arm/cpu/armv7/am33xx/clock.c
+++ b/arch/arm/cpu/armv7/am33xx/clock.c
@@ -24,6 +24,7 @@
 
 #define PRCM_MOD_EN		0x2
 #define PRCM_FORCE_WAKEUP	0x2
+#define PRCM_FUNCTL		0x0
 
 #define PRCM_EMIF_CLK_ACTIVITY	BIT(2)
 #define PRCM_L3_GCLK_ACTIVITY	BIT(4)
@@ -38,7 +39,7 @@
 #define CLK_MODE_SEL		0x7
 #define CLK_MODE_MASK		0xfffffff8
 #define CLK_DIV_SEL		0xFFFFFFE0
-
+#define CPGMAC0_IDLE		0x30000
 
 const struct cm_perpll *cmper = (struct cm_perpll *)CM_PER;
 const struct cm_wkuppll *cmwkup = (struct cm_wkuppll *)CM_WKUP;
@@ -105,6 +106,12 @@ static void enable_per_clocks(void)
 	writel(PRCM_MOD_EN, &cmwkup->wkup_uart0ctrl);
 	while (readl(&cmwkup->wkup_uart0ctrl) != PRCM_MOD_EN)
 		;
+
+	/* Ethernet */
+	writel(PRCM_MOD_EN, &cmper->cpgmac0clkctrl);
+	writel(PRCM_MOD_EN, &cmper->cpswclkstctrl);
+	while ((readl(&cmper->cpgmac0clkctrl) & CPGMAC0_IDLE) != PRCM_FUNCTL)
+		;
 }
 
 static void mpu_pll_config(void)
diff --git a/arch/arm/include/asm/arch-am33xx/cpu.h b/arch/arm/include/asm/arch-am33xx/cpu.h
index ad9156e..e255dd9 100644
--- a/arch/arm/include/asm/arch-am33xx/cpu.h
+++ b/arch/arm/include/asm/arch-am33xx/cpu.h
@@ -108,7 +108,9 @@ struct cm_perpll {
 	unsigned int l3sclkstctrl;	/* offset 0x04 */
 	unsigned int l4fwclkstctrl;	/* offset 0x08 */
 	unsigned int l3clkstctrl;	/* offset 0x0c */
-	unsigned int resv1[6];
+	unsigned int resv0[1];
+	unsigned int cpgmac0clkctrl;	/* offset 0x14 */
+	unsigned int resv1[4];
 	unsigned int emifclkctrl;	/* offset 0x28 */
 	unsigned int ocmcramclkctrl;	/* offset 0x2c */
 	unsigned int resv2[12];
@@ -124,6 +126,8 @@ struct cm_perpll {
 	unsigned int resv6[14];
 	unsigned int l4hsclkstctrl;	/* offset 0x11C */
 	unsigned int l4hsclkctrl;	/* offset 0x120 */
+	unsigned int resv7[8];
+	unsigned int cpswclkstctrl;	/* offset 0x144 */
 };
 
 /* Encapsulating Display pll registers */
@@ -173,9 +177,9 @@ struct timer_reg {
 /* Timer 32 bit registers */
 struct gptimer {
 	unsigned int tidr;		/* offset 0x00 */
-	unsigned int res1[0xc];
+	unsigned int res1[3];
 	unsigned int tiocp_cfg;		/* offset 0x10 */
-	unsigned int res2[0xc];
+	unsigned int res2[3];
 	unsigned int tier;		/* offset 0x20 */
 	unsigned int tistatr;		/* offset 0x24 */
 	unsigned int tistat;		/* offset 0x28 */
@@ -211,6 +215,18 @@ struct ctrl_stat {
 	unsigned int statusreg;		/* ofset 0x40 */
 };
 
+/* Control Device Register */
+struct ctrl_dev {
+	unsigned int deviceid;		/* offset 0x00 */
+	unsigned int resv1[11];
+	unsigned int macid0l;		/* offset 0x30 */
+	unsigned int macid0h;		/* offset 0x34 */
+	unsigned int macid1l;		/* offset 0x38 */
+	unsigned int macid1h;		/* offset 0x3c */
+	unsigned int resv2[4];
+	unsigned int miisel;		/* offset 0x50 */
+};
+
 void init_timer(void);
 #endif /* __ASSEMBLY__ */
 #endif /* __KERNEL_STRICT_NAMES */
diff --git a/arch/arm/include/asm/arch-am33xx/hardware.h b/arch/arm/include/asm/arch-am33xx/hardware.h
index 0ec22eb..4b1c725 100644
--- a/arch/arm/include/asm/arch-am33xx/hardware.h
+++ b/arch/arm/include/asm/arch-am33xx/hardware.h
@@ -46,6 +46,7 @@
 
 /* Control Module Base Address */
 #define CTRL_BASE			0x44E10000
+#define CTRL_DEVICE_BASE		0x44E10600
 
 /* PRCM Base Address */
 #define PRCM_BASE			0x44E00000
@@ -78,4 +79,8 @@
 #define DDRPHY_0_CONFIG_BASE		(CTRL_BASE + 0x1400)
 #define DDRPHY_CONFIG_BASE		DDRPHY_0_CONFIG_BASE
 
+/* CPSW Config space */
+#define AM335X_CPSW_BASE		0x4A100000
+#define AM335X_CPSW_MDIO_BASE		0x4A101000
+
 #endif /* __AM33XX_HARDWARE_H */
diff --git a/board/ti/am335x/common_def.h b/board/ti/am335x/common_def.h
index 1696d60..01cda88 100644
--- a/board/ti/am335x/common_def.h
+++ b/board/ti/am335x/common_def.h
@@ -17,8 +17,10 @@
 #define __COMMON_DEF_H__
 
 extern void enable_uart0_pin_mux(void);
+extern void enable_rgmii1_pin_mux(void);
 extern void configure_evm_pin_mux(unsigned char daughter_board_id,
 					unsigned short daughter_board_profile,
 					unsigned char daughter_board_flag);
+extern void cpsw_eth_set_mac_addr(const unsigned char *addr);
 
 #endif/*__COMMON_DEF_H__ */
diff --git a/board/ti/am335x/evm.c b/board/ti/am335x/evm.c
index b4eddd8..993d2a1 100644
--- a/board/ti/am335x/evm.c
+++ b/board/ti/am335x/evm.c
@@ -18,6 +18,8 @@
 #include <asm/arch/hardware.h>
 #include "common_def.h"
 #include <serial.h>
+#include <miiphy.h>
+#include <netdev.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -25,6 +27,12 @@ DECLARE_GLOBAL_DATA_PTR;
 #define UART_CLK_RUNNING_MASK	0x1
 #define UART_SMART_IDLE_EN	(0x1 << 0x3)
 
+/* RGMII mode define */
+#define RGMII_MODE_ENABLE	0xA
+#define ETH_ALEN		6
+
+struct ctrl_dev *cdev = (struct ctrl_dev *)CTRL_DEVICE_BASE;
+
 /*
  * Basic board specific setup
  */
@@ -42,7 +50,148 @@ int init_basic_setup(void)
 int board_init(void)
 {
 	enable_uart0_pin_mux();
+	enable_rgmii1_pin_mux();
 	init_basic_setup();
 
 	return 0;
 }
+
+#ifdef CONFIG_DRIVER_TI_CPSW
+/* Atheros PHY report 1000baseX speed incorrectly and so this function
+ * is define to overcome this issue. The original code is kept as
+ * _miiphy_is_1000base_x as an weak alias to this function.
+ */
+int miiphy_is_1000base_x(const char *devname, unsigned char addr)
+{
+	return 0;
+}
+
+/* TODO : Check for the board specific PHY */
+static void evm_phy_init(char *name, int addr)
+{
+	unsigned short val;
+	unsigned int cntr = 0;
+
+	/* Enable Autonegotiation */
+	if (miiphy_read(name, addr, MII_BMCR, &val) != 0) {
+		printf("failed to read bmcr\n");
+		return;
+	}
+
+	val |= BMCR_FULLDPLX | BMCR_ANENABLE | BMCR_SPEED100;
+	if (miiphy_write(name, addr, MII_BMCR, val) != 0) {
+		printf("failed to write bmcr\n");
+		return;
+	}
+	miiphy_read(name, addr, MII_BMCR, &val);
+
+	/* TODO: Disable GIG advertisement for the time being */
+	miiphy_read(name, addr, MII_CTRL1000, &val);
+	val |= PHY_1000BTCR_1000FD;
+	val &= ~PHY_1000BTCR_1000HD;
+	miiphy_write(name, addr, MII_CTRL1000, val);
+	miiphy_read(name, addr, MII_CTRL1000, &val);
+
+	/* Setup general advertisement */
+	if (miiphy_read(name, addr, MII_ADVERTISE, &val) != 0) {
+		printf("failed to read anar\n");
+		return;
+	}
+	val |= (LPA_10HALF | LPA_10FULL | LPA_100HALF | LPA_100FULL);
+	if (miiphy_write(name, addr, MII_ADVERTISE, val) != 0) {
+		printf("failed to write anar\n");
+		return;
+	}
+	miiphy_read(name, addr, MII_ADVERTISE, &val);
+
+	/* Restart auto negotiation*/
+	miiphy_read(name, addr, MII_BMCR, &val);
+	val |= BMCR_ANRESTART;
+	miiphy_write(name, addr, MII_BMCR, val);
+
+	/*check AutoNegotiate complete - it can take upto 3 secs*/
+	do {
+		udelay(40000);
+		cntr++;
+		if (!miiphy_read(name, addr, MII_BMSR, &val)) {
+			if (val & BMSR_ANEGCOMPLETE)
+				break;
+		}
+	} while (cntr < 1000);
+
+	if (cntr >= 1000)
+		printf("Auto negotitation failed\n");
+
+	return;
+}
+
+static void cpsw_control(int enabled)
+{
+	/* nothing for now */
+	/* TODO : VTP was here before */
+	return;
+}
+
+static struct cpsw_slave_data cpsw_slaves[] = {
+	{
+		.slave_reg_ofs	= 0x208,
+		.sliver_reg_ofs	= 0xd80,
+		.phy_id		= 0,
+	},
+	{
+		.slave_reg_ofs	= 0x308,
+		.sliver_reg_ofs	= 0xdc0,
+		.phy_id		= 1,
+	},
+};
+
+static struct cpsw_platform_data cpsw_data = {
+	.mdio_base		= AM335X_CPSW_MDIO_BASE,
+	.cpsw_base		= AM335X_CPSW_BASE,
+	.mdio_div		= 0xff,
+	.channels		= 8,
+	.cpdma_reg_ofs		= 0x800,
+	.slaves			= 1,
+	.slave_data		= cpsw_slaves,
+	.ale_reg_ofs		= 0xd00,
+	.ale_entries		= 1024,
+	.host_port_reg_ofs	= 0x108,
+	.hw_stats_reg_ofs	= 0x900,
+	.mac_control		= (1 << 5),
+	.control		= cpsw_control,
+	.phy_init		= evm_phy_init,
+	.host_port_num		= 0,
+	.version		= CPSW_CTRL_VERSION_2,
+};
+
+int board_eth_init(bd_t *bis)
+{
+	uint8_t mac_addr[6];
+	uint32_t mac_hi, mac_lo;
+
+	if (!eth_getenv_enetaddr("ethaddr", mac_addr)) {
+		debug("<ethaddr> not set. Reading from E-fuse\n");
+		/* try reading mac address from efuse */
+		mac_lo = readl(&cdev->macid0l);
+		mac_hi = readl(&cdev->macid0h);
+		mac_addr[0] = mac_hi & 0xFF;
+		mac_addr[1] = (mac_hi & 0xFF00) >> 8;
+		mac_addr[2] = (mac_hi & 0xFF0000) >> 16;
+		mac_addr[3] = (mac_hi & 0xFF000000) >> 24;
+		mac_addr[4] = mac_lo & 0xFF;
+		mac_addr[5] = (mac_lo & 0xFF00) >> 8;
+
+		if (is_valid_ether_addr(mac_addr)) {
+			eth_setenv_enetaddr("ethaddr", mac_addr);
+		} else {
+			printf("Caution: Using hardcoded mac address. "
+				"Set <ethaddr> variable to overcome this.\n");
+		}
+	}
+
+	/* set mii mode to rgmii in in device configure register */
+	writel(RGMII_MODE_ENABLE, &cdev->miisel);
+
+	return cpsw_register(&cpsw_data);
+}
+#endif
diff --git a/board/ti/am335x/mux.c b/board/ti/am335x/mux.c
index 8f27409..3cda206 100644
--- a/board/ti/am335x/mux.c
+++ b/board/ti/am335x/mux.c
@@ -258,6 +258,24 @@ static struct module_pin_mux uart0_pin_mux[] = {
 	{-1},
 };
 
+static struct module_pin_mux rgmii1_pin_mux[] = {
+	{OFFSET(mii1_txen), MODE(2)},			/* RGMII1_TCTL */
+	{OFFSET(mii1_rxdv), MODE(2) | RXACTIVE},	/* RGMII1_RCTL */
+	{OFFSET(mii1_txd3), MODE(2)},			/* RGMII1_TD3 */
+	{OFFSET(mii1_txd2), MODE(2)},			/* RGMII1_TD2 */
+	{OFFSET(mii1_txd1), MODE(2)},			/* RGMII1_TD1 */
+	{OFFSET(mii1_txd0), MODE(2)},			/* RGMII1_TD0 */
+	{OFFSET(mii1_txclk), MODE(2)},			/* RGMII1_TCLK */
+	{OFFSET(mii1_rxclk), MODE(2) | RXACTIVE},	/* RGMII1_RCLK */
+	{OFFSET(mii1_rxd3), MODE(2) | RXACTIVE},	/* RGMII1_RD3 */
+	{OFFSET(mii1_rxd2), MODE(2) | RXACTIVE},	/* RGMII1_RD2 */
+	{OFFSET(mii1_rxd1), MODE(2) | RXACTIVE},	/* RGMII1_RD1 */
+	{OFFSET(mii1_rxd0), MODE(2) | RXACTIVE},	/* RGMII1_RD0 */
+	{OFFSET(mdio_data), MODE(0) | RXACTIVE | PULLUP_EN},/* MDIO_DATA */
+	{OFFSET(mdio_clk), MODE(0) | PULLUP_EN},	/* MDIO_CLK */
+	{-1},
+};
+
 /*
  * Configure the pin mux for the module
  */
@@ -276,3 +294,9 @@ void enable_uart0_pin_mux(void)
 {
 	configure_module_pin_mux(uart0_pin_mux);
 }
+
+
+void enable_rgmii1_pin_mux(void)
+{
+	configure_module_pin_mux(rgmii1_pin_mux);
+}
diff --git a/common/miiphyutil.c b/common/miiphyutil.c
index 35ad357..dd56fda 100644
--- a/common/miiphyutil.c
+++ b/common/miiphyutil.c
@@ -548,7 +548,7 @@ miiphy_read_failed:
  * Return 1 if PHY supports 1000BASE-X, 0 if PHY supports 10BASE-T/100BASE-TX/
  * 1000BASE-T, or on error.
  */
-int miiphy_is_1000base_x(const char *devname, unsigned char addr)
+static int __miiphy_is_1000base_x(const char *devname, unsigned char addr)
 {
 #if defined(CONFIG_PHY_GIGE)
 	u16 exsr;
@@ -564,6 +564,9 @@ int miiphy_is_1000base_x(const char *devname, unsigned char addr)
 #endif
 }
 
+int miiphy_is_1000base_x(const char *devname, unsigned char addr)
+	__attribute__((weak, alias("__miiphy_is_1000base_x")));
+
 #ifdef CONFIG_SYS_FAULT_ECHO_LINK_DOWN
 /*****************************************************************************
  *
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index 4cef4cf..c41eeaf 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -1,22 +1,16 @@
 /*
  * CPSW Ethernet Switch Driver
  *
- * See file CREDITS for list of people who contributed to this
- * project.
+ * Copyright (C) 2010 Texas Instruments Incorporated - http://www.ti.com/
  *
- * 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 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 version 2.
  *
- * 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
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; 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 <common.h>
@@ -28,9 +22,7 @@
 #include <netdev.h>
 #include <asm/errno.h>
 #include <asm/io.h>
-#include <linux/mii.h>
 
-#define BIT(x)			(1 << (x))
 #define BITMASK(bits)		(BIT(bits) - 1)
 #define PHY_REG_MASK		0x1f
 #define PHY_ID_MASK		0x1f
@@ -43,10 +35,15 @@
 #define CPDMA_RXCONTROL		0x014
 #define CPDMA_SOFTRESET		0x01c
 #define CPDMA_RXFREE		0x0e0
-#define CPDMA_TXHDP		0x100
-#define CPDMA_RXHDP		0x120
-#define CPDMA_TXCP		0x140
-#define CPDMA_RXCP		0x160
+#define CPDMA_TXHDP_VER1	0x100
+#define CPDMA_TXHDP_VER2	0x200
+#define CPDMA_RXHDP_VER1	0x120
+#define CPDMA_RXHDP_VER2	0x220
+#define CPDMA_TXCP_VER1		0x140
+#define CPDMA_TXCP_VER2		0x240
+#define CPDMA_RXCP_VER1		0x160
+#define CPDMA_RXCP_VER2		0x260
+
 
 /* Descriptor mode bits */
 #define CPDMA_DESC_SOP		BIT(31)
@@ -252,12 +249,12 @@ static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value)	\
 
 DEFINE_ALE_FIELD(entry_type,		60,	2)
 DEFINE_ALE_FIELD(mcast_state,		62,	2)
-DEFINE_ALE_FIELD(port_mask,		64,	3)
-DEFINE_ALE_FIELD(ucast_type,		66,	2)
-DEFINE_ALE_FIELD(port_num,		64,	2)
-DEFINE_ALE_FIELD(blocked,		63,	1)
-DEFINE_ALE_FIELD(secure,		62,	1)
-DEFINE_ALE_FIELD(mcast,			47,	1)
+DEFINE_ALE_FIELD(port_mask,		66,	3)
+DEFINE_ALE_FIELD(ucast_type,		62,	2)
+DEFINE_ALE_FIELD(port_num,		66,	2)
+DEFINE_ALE_FIELD(blocked,		65,	1)
+DEFINE_ALE_FIELD(secure,		64,	1)
+DEFINE_ALE_FIELD(mcast,			40,	1)
 
 /* The MAC address field in the ALE entry cannot be macroized as above */
 static inline void cpsw_ale_get_addr(u32 *ale_entry, u8 *addr)
@@ -452,7 +449,7 @@ static inline void wait_for_idle(void)
 		;
 }
 
-static int cpsw_mdio_read(char *devname, unsigned char phy_id,
+static int cpsw_mdio_read(const char *devname, unsigned char phy_id,
 			  unsigned char phy_reg, unsigned short *data)
 {
 	u32 reg;
@@ -470,7 +467,7 @@ static int cpsw_mdio_read(char *devname, unsigned char phy_id,
 	return (reg & USERACCESS_ACK) ? 0 : -EIO;
 }
 
-static int cpsw_mdio_write(char *devname, unsigned char phy_id,
+static int cpsw_mdio_write(const char *devname, unsigned char phy_id,
 			   unsigned char phy_reg, unsigned short data)
 {
 	u32 reg;
@@ -502,6 +499,7 @@ static void cpsw_mdio_init(char *name, u32 mdio_base, u32 div)
 	 * silicon.  Since the effect of (b) was found to be largely
 	 * negligible, we keep things simple here.
 	 */
+
 	udelay(1000);
 
 	miiphy_register(name, cpsw_mdio_read, cpsw_mdio_write);
@@ -514,6 +512,20 @@ static inline void soft_reset(void *reg)
 		;
 }
 
+static u_int8_t cpsw_eth_mac_addr[] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x66 };
+
+/*
+ * This function must be called before cpsw_init() if you want to override
+ * the default mac address.
+ */
+void cpsw_eth_set_mac_addr(const u_int8_t *addr)
+{
+	int i;
+
+	for (i = 0; i < sizeof(cpsw_eth_mac_addr); i++)
+		cpsw_eth_mac_addr[i] = addr[i];
+}
+
 #define mac_hi(mac)	(((mac)[0] << 0) | ((mac)[1] << 8) |	\
 			 ((mac)[2] << 16) | ((mac)[3] << 24))
 #define mac_lo(mac)	(((mac)[4] << 0) | ((mac)[5] << 8))
@@ -547,6 +559,8 @@ static void cpsw_slave_update_link(struct cpsw_slave *slave,
 			mac_control |= BIT(7);	/* GIGABITEN	*/
 		if (duplex == FULL)
 			mac_control |= BIT(0);	/* FULLDUPLEXEN	*/
+		if (speed == 100)
+			mac_control |= BIT(15);
 	}
 
 	if (mac_control == slave->mac_control)
@@ -571,8 +585,17 @@ static int cpsw_update_link(struct cpsw_priv *priv)
 	return link;
 }
 
+static inline u32  cpsw_get_slave_port(struct cpsw_priv *priv, u32 slave_num)
+{
+	if (priv->host_port == 0)
+		return slave_num + 1;
+	else
+		return slave_num;
+}
+
 static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
 {
+	u32     slave_port;
 	soft_reset(&slave->sliver->soft_reset);
 
 	/* setup priority mapping */
@@ -586,9 +609,10 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
 	slave->mac_control = 0;	/* no link yet */
 
 	/* enable forwarding */
-	cpsw_ale_port_state(priv, slave->slave_num, ALE_PORT_STATE_FORWARD);
+	slave_port = cpsw_get_slave_port(priv, slave->slave_num);
+	cpsw_ale_port_state(priv, slave_port, ALE_PORT_STATE_FORWARD);
 
-	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << slave->slave_num);
+	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << slave_port);
 
 	priv->data.phy_init(priv->dev->name, slave->data->phy_id);
 }
@@ -708,7 +732,7 @@ static int cpsw_init(struct eth_device *dev, bd_t *bis)
 	__raw_writel(BIT(priv->host_port), &priv->regs->stat_port_en);
 
 	cpsw_ale_port_state(priv, priv->host_port, ALE_PORT_STATE_FORWARD);
-
+	memcpy(priv->dev->enetaddr, cpsw_eth_mac_addr, 6);
 	cpsw_ale_add_ucast(priv, priv->dev->enetaddr, priv->host_port,
 			   ALE_SECURE);
 	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << priv->host_port);
@@ -725,25 +749,58 @@ static int cpsw_init(struct eth_device *dev, bd_t *bis)
 	priv->desc_free = &priv->descs[0];
 
 	/* initialize channels */
-	memset(&priv->rx_chan, 0, sizeof(struct cpdma_chan));
-	priv->rx_chan.hdp	= priv->dma_regs + CPDMA_RXHDP;
-	priv->rx_chan.cp	= priv->dma_regs + CPDMA_RXCP;
-	priv->rx_chan.rxfree	= priv->dma_regs + CPDMA_RXFREE;
-
-	memset(&priv->tx_chan, 0, sizeof(struct cpdma_chan));
-	priv->tx_chan.hdp	= priv->dma_regs + CPDMA_TXHDP;
-	priv->tx_chan.cp	= priv->dma_regs + CPDMA_TXCP;
+	if (priv->data.version == CPSW_CTRL_VERSION_2) {
+		memset(&priv->rx_chan, 0, sizeof(struct cpdma_chan));
+		priv->rx_chan.hdp       = priv->dma_regs + CPDMA_RXHDP_VER2;
+		priv->rx_chan.cp        = priv->dma_regs + CPDMA_RXCP_VER2;
+		priv->rx_chan.rxfree    = priv->dma_regs + CPDMA_RXFREE;
+
+		memset(&priv->tx_chan, 0, sizeof(struct cpdma_chan));
+		priv->tx_chan.hdp       = priv->dma_regs + CPDMA_TXHDP_VER2;
+		priv->tx_chan.cp        = priv->dma_regs + CPDMA_TXCP_VER2;
+	} else {
+		memset(&priv->rx_chan, 0, sizeof(struct cpdma_chan));
+		priv->rx_chan.hdp       = priv->dma_regs + CPDMA_RXHDP_VER1;
+		priv->rx_chan.cp        = priv->dma_regs + CPDMA_RXCP_VER1;
+		priv->rx_chan.rxfree    = priv->dma_regs + CPDMA_RXFREE;
+
+		memset(&priv->tx_chan, 0, sizeof(struct cpdma_chan));
+		priv->tx_chan.hdp       = priv->dma_regs + CPDMA_TXHDP_VER1;
+		priv->tx_chan.cp        = priv->dma_regs + CPDMA_TXCP_VER1;
+	}
 
 	/* clear dma state */
 	soft_reset(priv->dma_regs + CPDMA_SOFTRESET);
 
-	for (i = 0; i < priv->data.channels; i++) {
-		__raw_writel(0, priv->dma_regs + CPDMA_RXHDP + 4 * i);
-		__raw_writel(0, priv->dma_regs + CPDMA_RXFREE + 4 * i);
-		__raw_writel(0, priv->dma_regs + CPDMA_RXCP + 4 * i);
-		__raw_writel(0, priv->dma_regs + CPDMA_TXHDP + 4 * i);
-		__raw_writel(0, priv->dma_regs + CPDMA_TXCP + 4 * i);
+	if (priv->data.version == CPSW_CTRL_VERSION_2) {
+		for (i = 0; i < priv->data.channels; i++) {
+			__raw_writel(0, priv->dma_regs + CPDMA_RXHDP_VER2 + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_RXFREE + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_RXCP_VER2 + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_TXHDP_VER2 + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_TXCP_VER2 + 4
+					* i);
+		}
+	} else {
+		for (i = 0; i < priv->data.channels; i++) {
+			__raw_writel(0, priv->dma_regs + CPDMA_RXHDP_VER1 + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_RXFREE + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_RXCP_VER1 + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_TXHDP_VER1 + 4
+					* i);
+			__raw_writel(0, priv->dma_regs + CPDMA_TXCP_VER1 + 4
+					* i);
+
+		}
 	}
+
 	__raw_writel(1, priv->dma_regs + CPDMA_TXCONTROL);
 	__raw_writel(1, priv->dma_regs + CPDMA_RXCONTROL);
 
@@ -838,7 +895,7 @@ int cpsw_register(struct cpsw_platform_data *data)
 		return -ENOMEM;
 	}
 
-	priv->host_port		= data->slaves;
+	priv->host_port		= data->host_port_num;
 	priv->regs		= regs;
 	priv->host_port_regs	= regs + data->host_port_reg_ofs;
 	priv->dma_regs		= regs + data->cpdma_reg_ofs;
diff --git a/include/configs/am335x_evm.h b/include/configs/am335x_evm.h
index b471c9b..18844db 100644
--- a/include/configs/am335x_evm.h
+++ b/include/configs/am335x_evm.h
@@ -21,7 +21,6 @@
 #undef CONFIG_GZIP
 #undef CONFIG_ZLIB
 #undef CONFIG_SYS_HUSH_PARSER
-#undef CONFIG_CMD_NET
 
 #include <asm/arch/cpu.h>
 #include <asm/arch/hardware.h>
@@ -116,6 +115,21 @@
 
 #define CONFIG_SYS_TEXT_BASE		0x402f0400
 
+# define CONFIG_CMD_NET
+# define CONFIG_CMD_DHCP
+# define CONFIG_CMD_PING
+# define CONFIG_DRIVER_TI_CPSW
+# define CONFIG_MII
+# define CONFIG_BOOTP_DEFAULT
+# define CONFIG_BOOTP_DNS
+# define CONFIG_BOOTP_DNS2
+# define CONFIG_BOOTP_SEND_HOSTNAME
+# define CONFIG_BOOTP_GATEWAY
+# define CONFIG_BOOTP_SUBNETMASK
+# define CONFIG_NET_RETRY_COUNT         10
+# define CONFIG_NET_MULTI
+# define CONFIG_PHY_GIGE
+
 /* Unsupported features */
 #undef CONFIG_USE_IRQ
 
diff --git a/include/netdev.h b/include/netdev.h
index 740213e..b8c12ae 100644
--- a/include/netdev.h
+++ b/include/netdev.h
@@ -193,21 +193,28 @@ struct cpsw_slave_data {
 	int		phy_id;
 };
 
+enum {
+	CPSW_CTRL_VERSION_1 = 0, /* version1 devices */
+	CPSW_CTRL_VERSION_2      /* version2 devices */
+};
+
 struct cpsw_platform_data {
 	u32	mdio_base;
 	u32	cpsw_base;
 	int	mdio_div;
-	int	channels;       /* number of cpdma channels (symmetric)	*/
-	u32     cpdma_reg_ofs;  /* cpdma register offset		*/
-	int     slaves;         /* number of slave cpgmac ports		*/
-	u32     ale_reg_ofs;    /* address lookup engine reg offset	*/
-	int     ale_entries;	/* ale table size			*/
+	int	channels;	/* number of cpdma channels (symmetric)	*/
+	u32	cpdma_reg_ofs;	/* cpdma register offset		*/
+	int	slaves;		/* number of slave cpgmac ports		*/
+	u32	ale_reg_ofs;	/* address lookup engine reg offset	*/
+	int	ale_entries;	/* ale table size			*/
 	u32	host_port_reg_ofs;	/* cpdma host port registers	*/
 	u32	hw_stats_reg_ofs;	/* cpsw hw stats counters	*/
 	u32	mac_control;
-	struct cpsw_slave_data  *slave_data;
+	struct	cpsw_slave_data  *slave_data;
 	void	(*control)(int enabled);
 	void	(*phy_init)(char *name, int addr);
+	u32	host_port_num;
+	u8	version;
 };
 
 int cpsw_register(struct cpsw_platform_data *data);
-- 
1.7.1

[U-Boot] [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for AM335x EVM

[Chandan Nath]

 #ifdef CONFIG_ARCH_EBSA110
 # ifdef machine_arch_type
@@ -42913,6 +42915,19 @@ extern unsigned int __machine_arch_type;
 # define machine_is_goflexhome()	(0)
 #endif
 
+#ifdef CONFIG_MACH_TIAM335EVM
+# ifdef machine_arch_type
+#  undef machine_arch_type
+#  define machine_arch_type     __machine_arch_type
+# else
+#  define machine_arch_type     MACH_TYPE_TIAM335EVM
+# endif
+# define machine_is_tiam335evm() (machine_arch_type == MACH_TYPE_TIAM335EVM)
+#else
+# define machine_is_tiam335evm() (0)
+#endif
+
+
 /*
  * These have not yet been registered
  */
diff --git a/board/ti/am335x/evm.c b/board/ti/am335x/evm.c
index 4530a76..993d2a1 100644
--- a/board/ti/am335x/evm.c
+++ b/board/ti/am335x/evm.c
@@ -57,6 +57,15 @@ int board_init(void)
 }
 
 #ifdef CONFIG_DRIVER_TI_CPSW
+/* Atheros PHY report 1000baseX speed incorrectly and so this function
+ * is define to overcome this issue. The original code is kept as
+ * _miiphy_is_1000base_x as an weak alias to this function.
+ */
+int miiphy_is_1000base_x(const char *devname, unsigned char addr)
+{
+	return 0;
+}
+
 /* TODO : Check for the board specific PHY */
 static void evm_phy_init(char *name, int addr)
 {
@@ -159,7 +168,6 @@ int board_eth_init(bd_t *bis)
 {
 	uint8_t mac_addr[6];
 	uint32_t mac_hi, mac_lo;
-	char mac_buf[32];
 
 	if (!eth_getenv_enetaddr("ethaddr", mac_addr)) {
 		debug("<ethaddr> not set. Reading from E-fuse\n");
@@ -174,18 +182,11 @@ int board_eth_init(bd_t *bis)
 		mac_addr[5] = (mac_lo & 0xFF00) >> 8;
 
 		if (is_valid_ether_addr(mac_addr)) {
-			sprintf(mac_buf, "%02x:%02x:%02x:%02x:%02x:%02x",
-				mac_addr[0], mac_addr[1], mac_addr[2],
-				mac_addr[3], mac_addr[4], mac_addr[5]);
-
-			printf("ENET MAC address: %s\n", mac_buf);
-			setenv("ethaddr", (char *)mac_buf);
+			eth_setenv_enetaddr("ethaddr", mac_addr);
 		} else {
 			printf("Caution: Using hardcoded mac address. "
 				"Set <ethaddr> variable to overcome this.\n");
 		}
-
-		cpsw_eth_set_mac_addr(mac_addr);
 	}
 
 	/* set mii mode to rgmii in in device configure register */
diff --git a/common/miiphyutil.c b/common/miiphyutil.c
index 4551240..dd56fda 100644
--- a/common/miiphyutil.c
+++ b/common/miiphyutil.c
@@ -548,7 +548,7 @@ miiphy_read_failed:
  * Return 1 if PHY supports 1000BASE-X, 0 if PHY supports 10BASE-T/100BASE-TX/
  * 1000BASE-T, or on error.
  */
-int miiphy_is_1000base_x(const char *devname, unsigned char addr)
+static int __miiphy_is_1000base_x(const char *devname, unsigned char addr)
 {
 #if defined(CONFIG_PHY_GIGE)
 	u16 exsr;
@@ -558,12 +558,15 @@ int miiphy_is_1000base_x(const char *devname, unsigned char addr)
 			"1000BASE-X\n");
 		return 0;
 	}
-	return 0;
+	return 0 != (exsr & (ESTATUS_1000XF | ESTATUS_1000XH));
 #else
 	return 0;
 #endif
 }
 
+int miiphy_is_1000base_x(const char *devname, unsigned char addr)
+	__attribute__((weak, alias("__miiphy_is_1000base_x")));
+
 #ifdef CONFIG_SYS_FAULT_ECHO_LINK_DOWN
 /*****************************************************************************
  *
diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
index c41eeaf..3167509 100644
--- a/drivers/net/cpsw.c
+++ b/drivers/net/cpsw.c
@@ -512,20 +512,6 @@ static inline void soft_reset(void *reg)
 		;
 }
 
-static u_int8_t cpsw_eth_mac_addr[] = { 0x00, 0x11, 0x22, 0x33, 0x44, 0x66 };
-
-/*
- * This function must be called before cpsw_init() if you want to override
- * the default mac address.
- */
-void cpsw_eth_set_mac_addr(const u_int8_t *addr)
-{
-	int i;
-
-	for (i = 0; i < sizeof(cpsw_eth_mac_addr); i++)
-		cpsw_eth_mac_addr[i] = addr[i];
-}
-
 #define mac_hi(mac)	(((mac)[0] << 0) | ((mac)[1] << 8) |	\
 			 ((mac)[2] << 16) | ((mac)[3] << 24))
 #define mac_lo(mac)	(((mac)[4] << 0) | ((mac)[5] << 8))
@@ -732,7 +718,7 @@ static int cpsw_init(struct eth_device *dev, bd_t *bis)
 	__raw_writel(BIT(priv->host_port), &priv->regs->stat_port_en);
 
 	cpsw_ale_port_state(priv, priv->host_port, ALE_PORT_STATE_FORWARD);
-	memcpy(priv->dev->enetaddr, cpsw_eth_mac_addr, 6);
+
 	cpsw_ale_add_ucast(priv, priv->dev->enetaddr, priv->host_port,
 			   ALE_SECURE);
 	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << priv->host_port);

[U-Boot] [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for AM335x EVM

[Kumar]

Please ignore this patch which went by mistake.


> -----Original Message-----
> From: Kumar Nath, Chandan
> Sent: Thursday, November 10, 2011 6:11 PM
> To: u-boot at lists.denx.de
> Cc: Kumar Nath, Chandan
> Subject: [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for AM335x EVM
> 
>  #ifdef CONFIG_ARCH_EBSA110
>  # ifdef machine_arch_type
> @@ -42913,6 +42915,19 @@ extern unsigned int __machine_arch_type;
>  # define machine_is_goflexhome()	(0)
>  #endif
> 
> +#ifdef CONFIG_MACH_TIAM335EVM
> +# ifdef machine_arch_type
> +#  undef machine_arch_type
> +#  define machine_arch_type     __machine_arch_type
> +# else
> +#  define machine_arch_type     MACH_TYPE_TIAM335EVM
> +# endif
> +# define machine_is_tiam335evm() (machine_arch_type ==
> MACH_TYPE_TIAM335EVM)
> +#else
> +# define machine_is_tiam335evm() (0)
> +#endif
> +
> +
>  /*
>   * These have not yet been registered
>   */
> diff --git a/board/ti/am335x/evm.c b/board/ti/am335x/evm.c
> index 4530a76..993d2a1 100644
> --- a/board/ti/am335x/evm.c
> +++ b/board/ti/am335x/evm.c
> @@ -57,6 +57,15 @@ int board_init(void)
>  }
> 
>  #ifdef CONFIG_DRIVER_TI_CPSW
> +/* Atheros PHY report 1000baseX speed incorrectly and so this function
> + * is define to overcome this issue. The original code is kept as
> + * _miiphy_is_1000base_x as an weak alias to this function.
> + */
> +int miiphy_is_1000base_x(const char *devname, unsigned char addr)
> +{
> +	return 0;
> +}
> +
>  /* TODO : Check for the board specific PHY */
>  static void evm_phy_init(char *name, int addr)
>  {
> @@ -159,7 +168,6 @@ int board_eth_init(bd_t *bis)
>  {
>  	uint8_t mac_addr[6];
>  	uint32_t mac_hi, mac_lo;
> -	char mac_buf[32];
> 
>  	if (!eth_getenv_enetaddr("ethaddr", mac_addr)) {
>  		debug("<ethaddr> not set. Reading from E-fuse\n");
> @@ -174,18 +182,11 @@ int board_eth_init(bd_t *bis)
>  		mac_addr[5] = (mac_lo & 0xFF00) >> 8;
> 
>  		if (is_valid_ether_addr(mac_addr)) {
> -			sprintf(mac_buf, "%02x:%02x:%02x:%02x:%02x:%02x",
> -				mac_addr[0], mac_addr[1], mac_addr[2],
> -				mac_addr[3], mac_addr[4], mac_addr[5]);
> -
> -			printf("ENET MAC address: %s\n", mac_buf);
> -			setenv("ethaddr", (char *)mac_buf);
> +			eth_setenv_enetaddr("ethaddr", mac_addr);
>  		} else {
>  			printf("Caution: Using hardcoded mac address. "
>  				"Set <ethaddr> variable to overcome this.\n");
>  		}
> -
> -		cpsw_eth_set_mac_addr(mac_addr);
>  	}
> 
>  	/* set mii mode to rgmii in in device configure register */
> diff --git a/common/miiphyutil.c b/common/miiphyutil.c
> index 4551240..dd56fda 100644
> --- a/common/miiphyutil.c
> +++ b/common/miiphyutil.c
> @@ -548,7 +548,7 @@ miiphy_read_failed:
>   * Return 1 if PHY supports 1000BASE-X, 0 if PHY supports 10BASE-
> T/100BASE-TX/
>   * 1000BASE-T, or on error.
>   */
> -int miiphy_is_1000base_x(const char *devname, unsigned char addr)
> +static int __miiphy_is_1000base_x(const char *devname, unsigned char
> addr)
>  {
>  #if defined(CONFIG_PHY_GIGE)
>  	u16 exsr;
> @@ -558,12 +558,15 @@ int miiphy_is_1000base_x(const char *devname,
> unsigned char addr)
>  			"1000BASE-X\n");
>  		return 0;
>  	}
> -	return 0;
> +	return 0 != (exsr & (ESTATUS_1000XF | ESTATUS_1000XH));
>  #else
>  	return 0;
>  #endif
>  }
> 
> +int miiphy_is_1000base_x(const char *devname, unsigned char addr)
> +	__attribute__((weak, alias("__miiphy_is_1000base_x")));
> +
>  #ifdef CONFIG_SYS_FAULT_ECHO_LINK_DOWN
> 
> /**********************************************************************
> *******
>   *
> diff --git a/drivers/net/cpsw.c b/drivers/net/cpsw.c
> index c41eeaf..3167509 100644
> --- a/drivers/net/cpsw.c
> +++ b/drivers/net/cpsw.c
> @@ -512,20 +512,6 @@ static inline void soft_reset(void *reg)
>  		;
>  }
> 
> -static u_int8_t cpsw_eth_mac_addr[] = { 0x00, 0x11, 0x22, 0x33, 0x44,
> 0x66 };
> -
> -/*
> - * This function must be called before cpsw_init() if you want to
> override
> - * the default mac address.
> - */
> -void cpsw_eth_set_mac_addr(const u_int8_t *addr)
> -{
> -	int i;
> -
> -	for (i = 0; i < sizeof(cpsw_eth_mac_addr); i++)
> -		cpsw_eth_mac_addr[i] = addr[i];
> -}
> -
>  #define mac_hi(mac)	(((mac)[0] << 0) | ((mac)[1] << 8) |	\
>  			 ((mac)[2] << 16) | ((mac)[3] << 24))
>  #define mac_lo(mac)	(((mac)[4] << 0) | ((mac)[5] << 8))
> @@ -732,7 +718,7 @@ static int cpsw_init(struct eth_device *dev, bd_t
> *bis)
>  	__raw_writel(BIT(priv->host_port), &priv->regs->stat_port_en);
> 
>  	cpsw_ale_port_state(priv, priv->host_port,
> ALE_PORT_STATE_FORWARD);
> -	memcpy(priv->dev->enetaddr, cpsw_eth_mac_addr, 6);
> +
>  	cpsw_ale_add_ucast(priv, priv->dev->enetaddr, priv->host_port,
>  			   ALE_SECURE);
>  	cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << priv->host_port);

[U-Boot] [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for AM335x EVM

[Tom Rini]

On Thu, Nov 10, 2011 at 5:40 AM, Chandan Nath <chandan.nath@ti.com> wrote:
> This patch adds cpsw support on AM335X EVM.
>
> Signed-off-by: Chandan Nath <chandan.nath@ti.com>

You aren't allowed to have hard-coded MACs anymore, iirc.  And
everyone will be able to get valid MACs from the e-fuse (or on the
other platforms, EEPROM), we need to drop that particular debug print
and just fail when ethaddr isn't set.  So you can drop the
..set_mac_addr function, and fold the rest of the changes to cpsw.c
into the original patch.  Finally, please split the miiphyutil.c
change into its own commit.  Thanks.

-- 
Tom

[U-Boot] [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for AM335x EVM

[Kumar]

> -----Original Message-----
> From: Tom Rini [mailto:tom.rini at gmail.com]
> Sent: Thursday, November 10, 2011 8:17 PM
> To: Kumar Nath, Chandan
> Cc: u-boot at lists.denx.de
> Subject: Re: [U-Boot] [PATCH v2 2/2] ARM:AM33XX:Added cpsw support for
> AM335x EVM
> 
> On Thu, Nov 10, 2011 at 5:40 AM, Chandan Nath <chandan.nath@ti.com>
> wrote:
> > This patch adds cpsw support on AM335X EVM.
> >
> > Signed-off-by: Chandan Nath <chandan.nath@ti.com>
> 
> You aren't allowed to have hard-coded MACs anymore, iirc.  And
> everyone will be able to get valid MACs from the e-fuse (or on the
> other platforms, EEPROM), we need to drop that particular debug print
> and just fail when ethaddr isn't set.  So you can drop the
> ..set_mac_addr function, and fold the rest of the changes to cpsw.c
> into the original patch.  Finally, please split the miiphyutil.c
> change into its own commit.  Thanks.
> 

I will remove the debug print and return failure when ethaddr is not set.
I have already remove cpsw_eth_set_mac_addr function from cpsw.c and pushed
to the original patch.

I will split miiphyutil.c changes in to a separate patch.

> --
> Tom

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Mike Frysinger]

On Thursday 10 November 2011 07:40:28 Chandan Nath wrote:
> +#define mac_hi(mac)    (((mac)[0] << 0) | ((mac)[1] << 8) |    \
> +                        ((mac)[2] << 16) | ((mac)[3] << 24))

looks like __get_unaligned_u32()

> +#define mac_lo(mac)    (((mac)[4] << 0) | ((mac)[5] << 8))

looks like __get_unaligned_u16()
-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/20111110/e76893bf/attachment.pgp 

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Tom Rini]

On Thu, Nov 10, 2011 at 9:26 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Thursday 10 November 2011 07:40:28 Chandan Nath wrote:
>> +#define mac_hi(mac) ? ?(((mac)[0] << 0) | ((mac)[1] << 8) | ? ?\
>> + ? ? ? ? ? ? ? ? ? ? ? ?((mac)[2] << 16) | ((mac)[3] << 24))
>
> looks like __get_unaligned_u32()
>
>> +#define mac_lo(mac) ? ?(((mac)[4] << 0) | ((mac)[5] << 8))
>
> looks like __get_unaligned_u16()

It's the regular MAC shifting around everyone does.  Are you saying
there's a general cleanup to be done here?

-- 
Tom

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Mike Frysinger]

On Thursday 10 November 2011 11:39:49 Tom Rini wrote:
> On Thu, Nov 10, 2011 at 9:26 AM, Mike Frysinger <vapier@gentoo.org> wrote:
> > On Thursday 10 November 2011 07:40:28 Chandan Nath wrote:
> >> +#define mac_hi(mac)    (((mac)[0] << 0) | ((mac)[1] << 8) |    \
> >> +                        ((mac)[2] << 16) | ((mac)[3] << 24))
> > 
> > looks like __get_unaligned_u32()
> > 
> >> +#define mac_lo(mac)    (((mac)[4] << 0) | ((mac)[5] << 8))
> > 
> > looks like __get_unaligned_u16()
> 
> It's the regular MAC shifting around everyone does.  Are you saying
> there's a general cleanup to be done here?

other drivers probably could benefit from using the unaligned helpers.  but it 
isn't something we could generally unify as it requires assumption on 
core/system endian, and the layout of the registers in the peripheral.
-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/20111110/afa2dcd4/attachment.pgp 

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Andy Fleming]

On Thu, Nov 10, 2011 at 6:40 AM, Chandan Nath <chandan.nath@ti.com> wrote:
> From: Cyril Chemparathy <cyril@ti.com>
>
> CPSW is an on-chip ethernet switch that is found on various SoCs from Texas
> Instruments. ?This patch adds a simple driver (based on the Linux driver) for
> this hardware module.


Which Linux driver is this based on?


>
> Signed-off-by: Cyril Chemparathy <cyril@ti.com>
> ---
> Changes since v1:
> ?- Code cleanup and removal of cpsw_eth_set_mac_addr function
>
> ?drivers/net/Makefile | ? ?1 +
> ?drivers/net/cpsw.c ? | ?862 ++++++++++++++++++++++++++++++++++++++++++++++++++
> ?include/netdev.h ? ? | ? 29 ++
> ?3 files changed, 892 insertions(+), 0 deletions(-)
> ?create mode 100644 drivers/net/cpsw.c
>
> +
> +#define BIT(x) ? ? ? ? ? ? ? ? (1 << (x))
> +#define BITMASK(bits) ? ? ? ? ?(BIT(bits) - 1)
> +#define PHY_REG_MASK ? ? ? ? ? 0x1f
> +#define PHY_ID_MASK ? ? ? ? ? ?0x1f

These 4 defines seem very generic


> +struct cpsw_priv {
> + ? ? ? struct eth_device ? ? ? ? ? ? ? *dev;
> + ? ? ? struct cpsw_platform_data ? ? ? data;
> + ? ? ? int ? ? ? ? ? ? ? ? ? ? ? ? ? ? host_port;
> +
> + ? ? ? struct cpsw_regs ? ? ? ? ? ? ? ?*regs;
> + ? ? ? void ? ? ? ? ? ? ? ? ? ? ? ? ? ?*dma_regs;
> + ? ? ? struct cpsw_host_regs ? ? ? ? ? *host_port_regs;
> + ? ? ? void ? ? ? ? ? ? ? ? ? ? ? ? ? ?*ale_regs;
> +
> + ? ? ? struct cpdma_desc ? ? ? ? ? ? ? descs[NUM_DESCS];
> + ? ? ? struct cpdma_desc ? ? ? ? ? ? ? *desc_free;
> + ? ? ? struct cpdma_chan ? ? ? ? ? ? ? rx_chan, tx_chan;
> +
> + ? ? ? struct cpsw_slave ? ? ? ? ? ? ? *slaves;
> +#define for_each_slave(priv, func, arg...) ? ? ? ? ? ? ? ? ? ? \
> + ? ? ? do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? int idx; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? ? ? ? ? for (idx = 0; idx < (priv)->data.slaves; idx++) \
> + ? ? ? ? ? ? ? ? ? ? ? (func)((priv)->slaves + idx, ##arg); ? ?\
> + ? ? ? } while (0)
> +};


It seems a bit awkward to me to put a complex macro like this inside
the structure definition. After further review, I think it should also
be defined differently, more like the list macros:

#define for_each_slave(slave, priv) \
        for (slave = (priv)->slaves; slave != (priv)->slaves +
(priv)->data.slaves; slave++)

It makes the code more maintainable, as the macro no longer contains
the implicit assumption that "func" has a slave pointer as its first
argument.


> +
> +static inline int cpsw_ale_get_field(u32 *ale_entry, u32 start, u32 bits)
> +{
> + ? ? ? int idx;
> +
> + ? ? ? idx ? ?= start / 32;
> + ? ? ? start -= idx * 32;
> + ? ? ? idx ? ?= 2 - idx; /* flip */
> + ? ? ? return (ale_entry[idx] >> start) & BITMASK(bits);
> +}
> +
> +static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32 bits,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u32 value)
> +{
> + ? ? ? int idx;
> +
> + ? ? ? value &= BITMASK(bits);
> + ? ? ? idx ? ?= start / 32;
> + ? ? ? start -= idx * 32;
> + ? ? ? idx ? ?= 2 - idx; /* flip */
> + ? ? ? ale_entry[idx] &= ~(BITMASK(bits) << start);
> + ? ? ? ale_entry[idx] |= ?(value << start);
> +}


These two functions seem both very generic, and also over-worked.


> +
> +#define DEFINE_ALE_FIELD(name, start, bits) ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> +static inline int cpsw_ale_get_##name(u32 *ale_entry) ? ? ? ? ? ? ? ? ?\
> +{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? return cpsw_ale_get_field(ale_entry, start, bits); ? ? ? ? ? ? ?\
> +} ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> +static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value) ? ? ?\
> +{ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> + ? ? ? cpsw_ale_set_field(ale_entry, start, bits, value); ? ? ? ? ? ? ?\
> +}
> +
> +DEFINE_ALE_FIELD(entry_type, ? ? ? ? ? 60, ? ? 2)
> +DEFINE_ALE_FIELD(mcast_state, ? ? ? ? ?62, ? ? 2)
> +DEFINE_ALE_FIELD(port_mask, ? ? ? ? ? ?64, ? ? 3)
> +DEFINE_ALE_FIELD(ucast_type, ? ? ? ? ? 66, ? ? 2)
> +DEFINE_ALE_FIELD(port_num, ? ? ? ? ? ? 64, ? ? 2)
> +DEFINE_ALE_FIELD(blocked, ? ? ? ? ? ? ?63, ? ? 1)
> +DEFINE_ALE_FIELD(secure, ? ? ? ? ? ? ? 62, ? ? 1)
> +DEFINE_ALE_FIELD(mcast, ? ? ? ? ? ? ? ? ? ? ? ?47, ? ? 1)
> +
> +/* The MAC address field in the ALE entry cannot be macroized as above */
> +static inline void cpsw_ale_get_addr(u32 *ale_entry, u8 *addr)
> +{
> + ? ? ? int i;
> +
> + ? ? ? for (i = 0; i < 6; i++)
> + ? ? ? ? ? ? ? addr[i] = cpsw_ale_get_field(ale_entry, 40 - 8*i, 8);
> +}
> +
> +static inline void cpsw_ale_set_addr(u32 *ale_entry, u8 *addr)
> +{
> + ? ? ? int i;
> +
> + ? ? ? for (i = 0; i < 6; i++)
> + ? ? ? ? ? ? ? cpsw_ale_set_field(ale_entry, 40 - 8*i, 8, addr[i]);
> +}
> +
> +static int cpsw_ale_read(struct cpsw_priv *priv, int idx, u32 *ale_entry)
> +{
> + ? ? ? int i;
> +
> + ? ? ? __raw_writel(idx, priv->ale_regs + ALE_TABLE_CONTROL);
> +
> + ? ? ? for (i = 0; i < ALE_ENTRY_WORDS; i++)
> + ? ? ? ? ? ? ? ale_entry[i] = __raw_readl(priv->ale_regs + ALE_TABLE + 4 * i);
> +
> + ? ? ? return idx;
> +}
> +
> +static int cpsw_ale_write(struct cpsw_priv *priv, int idx, u32 *ale_entry)
> +{
> + ? ? ? int i;
> +
> + ? ? ? for (i = 0; i < ALE_ENTRY_WORDS; i++)
> + ? ? ? ? ? ? ? __raw_writel(ale_entry[i], priv->ale_regs + ALE_TABLE + 4 * i);
> +
> + ? ? ? __raw_writel(idx | ALE_TABLE_WRITE, priv->ale_regs + ALE_TABLE_CONTROL);
> +
> + ? ? ? return idx;
> +}


I don't know how your device works, but it seems odd to me to have to
read out a large table, and modify it, and then write it back out.

I'm also highly suspicious of the overworked bitfield math. Are
standard C bitfields not usable for some reason? It borders on code
obfuscation, IMO. I've spent far too much time, already, just trying
to figure out what that code was doing, and why.


> +static inline void cpsw_ale_port_state(struct cpsw_priv *priv, int port,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int val)
> +{
> + ? ? ? int offset = ALE_PORTCTL + 4 * port;
> + ? ? ? u32 tmp, mask = 0x3;
> +
> + ? ? ? tmp ?= __raw_readl(priv->ale_regs + offset);
> + ? ? ? tmp &= ~mask;
> + ? ? ? tmp |= val & 0x3;

All of this can probably be done with more generic setbits functions,
but at least use "mask" consistently for both the clearing and the
masking of "val".

> + ? ? ? __raw_writel(tmp, priv->ale_regs + offset);
> +}
> +
> +static struct cpsw_mdio_regs *mdio_regs;


Global variables like this shouldn't be necessary. Especially with the
new PHY API



> +
> +static int cpsw_mdio_read(char *devname, unsigned char phy_id,
> + ? ? ? ? ? ? ? ? ? ? ? ? unsigned char phy_reg, unsigned short *data)
> +{

This is the legacy interface for PHY interactions. You should look
into using the new "PHYLIB" code.


> + ? ? ? u32 reg;
> +
> + ? ? ? if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
> + ? ? ? ? ? ? ? return -EINVAL;
> +
> + ? ? ? wait_for_user_access();
> + ? ? ? reg = (USERACCESS_GO | USERACCESS_READ | (phy_reg << 21) |
> + ? ? ? ? ? ? ?(phy_id << 16));
> + ? ? ? __raw_writel(reg, &mdio_regs->user[0].access);
> + ? ? ? reg = wait_for_user_access();
> +
> + ? ? ? *data = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -1;
> + ? ? ? return (reg & USERACCESS_ACK) ? 0 : -EIO;
> +}
> +
> +static int cpsw_mdio_write(char *devname, unsigned char phy_id,
> + ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned char phy_reg, unsigned short data)

Same comment as above.


> +static void cpsw_mdio_init(char *name, u32 mdio_base, u32 div)
> +{
> + ? ? ? mdio_regs = (struct cpsw_mdio_regs *)mdio_base;
> +
> + ? ? ? /* set enable and clock divider */
> + ? ? ? __raw_writel(div | CONTROL_ENABLE, &mdio_regs->control);
> +
> + ? ? ? /*
> + ? ? ? ?* wait for scan logic to settle:
> + ? ? ? ?* the scan time consists of (a) a large fixed component, and (b) a
> + ? ? ? ?* small component that varies with the mii bus frequency. ?These
> + ? ? ? ?* were estimated using measurements at 1.1 and 2.2 MHz on tnetv107x
> + ? ? ? ?* silicon. ?Since the effect of (b) was found to be largely
> + ? ? ? ?* negligible, we keep things simple here.
> + ? ? ? ?*/
> + ? ? ? udelay(1000);

This seems sketchy. Is there no way to *know* the bus is ready, other
than to wait?

> +
> + ? ? ? miiphy_register(name, cpsw_mdio_read, cpsw_mdio_write);


This is the legacy API.


> +}
> +
> +static inline void soft_reset(void *reg)
> +{
> + ? ? ? __raw_writel(1, reg);
> + ? ? ? while (__raw_readl(reg) & 1)
> + ? ? ? ? ? ? ? ;
> +}


???

Can reg be *any* reg? Use a better name. Also, name "1" something sensible.

Perhaps a cleaner implementation (which might be usable in other
people's code) would be something like:

/* Set a self-clearing bit in a register, and wait for it to clear */
static inline void setbit_and_wait_for_clear32(void *addr, u32 mask)
{
   __raw_writel(val, addr);
  while (__raw_readl(addr) & val)
    ;
}


> +static void cpsw_slave_update_link(struct cpsw_slave *slave,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpsw_priv *priv, int *link)
> +{
> + ? ? ? char *name = priv->dev->name;
> + ? ? ? int phy_id = slave->data->phy_id;
> + ? ? ? int speed, duplex;
> + ? ? ? unsigned short reg;
> + ? ? ? u32 mac_control = 0;
> +
> + ? ? ? if (miiphy_read(name, phy_id, MII_BMSR, &reg))
> + ? ? ? ? ? ? ? return; /* could not read, assume no link */


Please use phy_read(), with the new API.


> +
> + ? ? ? if (reg & BMSR_LSTATUS) { /* link up */
> + ? ? ? ? ? ? ? speed = miiphy_speed(name, phy_id);
> + ? ? ? ? ? ? ? duplex = miiphy_duplex(name, phy_id);
> +
> + ? ? ? ? ? ? ? *link = 1;
> + ? ? ? ? ? ? ? mac_control = priv->data.mac_control;
> + ? ? ? ? ? ? ? if (speed == 1000)
> + ? ? ? ? ? ? ? ? ? ? ? mac_control |= BIT(7); ?/* GIGABITEN ? ?*/
> + ? ? ? ? ? ? ? if (duplex == FULL)
> + ? ? ? ? ? ? ? ? ? ? ? mac_control |= BIT(0); ?/* FULLDUPLEXEN */

Why not:
    mac_control |= GIGABITEN;

etc



> +static int cpsw_update_link(struct cpsw_priv *priv)
> +{
> + ? ? ? int link = 0;
> + ? ? ? for_each_slave(priv, cpsw_slave_update_link, priv, &link);

As I mentioned, there are a lot of similarly-named macros in Linux and
U-boot. Usually they act exactly like for-loops, where you provide an
iterator, and then use it in each iteration:

struct cpsw_slave *slave;

for_each_slave(slave, priv)
  cpsw_slave_update_link(slave, priv, &link);

I think this is much easier to understand and maintain.

> + ? ? ? return link;

Also, link looks useless. It will always be the value in the last
slave. Is that what you meant to do?


> +}
> +
> +static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv)
> +{

[...]

> +
> + ? ? ? priv->data.phy_init(priv->dev->name, slave->data->phy_id);

What does this do? Probably needs to be converted to the newer API.


> +}
> +
> +static struct cpdma_desc *cpdma_desc_alloc(struct cpsw_priv *priv)
> +{
> + ? ? ? struct cpdma_desc *desc = priv->desc_free;
> +
> + ? ? ? if (desc)
> + ? ? ? ? ? ? ? priv->desc_free = desc_read_ptr(desc, hw_next);
> + ? ? ? return desc;
> +}
> +
> +static void cpdma_desc_free(struct cpsw_priv *priv, struct cpdma_desc *desc)
> +{
> + ? ? ? if (desc) {
> + ? ? ? ? ? ? ? desc_write(desc, hw_next, priv->desc_free);
> + ? ? ? ? ? ? ? priv->desc_free = desc;
> + ? ? ? }
> +}
> +
> +static int cpdma_submit(struct cpsw_priv *priv, struct cpdma_chan *chan,
> + ? ? ? ? ? ? ? ? ? ? ? volatile void *buffer, int len)
> +{
> + ? ? ? struct cpdma_desc *desc, *prev;
> + ? ? ? u32 mode;
> +
> + ? ? ? desc = cpdma_desc_alloc(priv);
> + ? ? ? if (!desc)
> + ? ? ? ? ? ? ? return -ENOMEM;
> +
> + ? ? ? if (len < PKT_MIN)
> + ? ? ? ? ? ? ? len = PKT_MIN;
> +
> + ? ? ? mode = CPDMA_DESC_OWNER | CPDMA_DESC_SOP | CPDMA_DESC_EOP;
> +
> + ? ? ? desc_write(desc, hw_next, ? 0);
> + ? ? ? desc_write(desc, hw_buffer, buffer);
> + ? ? ? desc_write(desc, hw_len, ? ?len);
> + ? ? ? desc_write(desc, hw_mode, ? mode | len);
> + ? ? ? desc_write(desc, sw_buffer, buffer);
> + ? ? ? desc_write(desc, sw_len, ? ?len);
> +
> + ? ? ? if (!chan->head) {
> + ? ? ? ? ? ? ? /* simple case - first packet enqueued */
> + ? ? ? ? ? ? ? chan->head = desc;
> + ? ? ? ? ? ? ? chan->tail = desc;
> + ? ? ? ? ? ? ? chan_write(chan, hdp, desc);
> + ? ? ? ? ? ? ? goto done;
> + ? ? ? }
> +
> + ? ? ? /* not the first packet - enqueue at the tail */
> + ? ? ? prev = chan->tail;
> + ? ? ? desc_write(prev, hw_next, desc);
> + ? ? ? chan->tail = desc;
> +
> + ? ? ? /* next check if EOQ has been triggered already */
> + ? ? ? if (desc_read(prev, hw_mode) & CPDMA_DESC_EOQ)
> + ? ? ? ? ? ? ? chan_write(chan, hdp, desc);
> +
> +done:
> + ? ? ? if (chan->rxfree)
> + ? ? ? ? ? ? ? chan_write(chan, rxfree, 1);
> + ? ? ? return 0;
> +}
> +
> +static int cpdma_process(struct cpsw_priv *priv, struct cpdma_chan *chan,
> + ? ? ? ? ? ? ? ? ? ? ? ?void **buffer, int *len)
> +{
> + ? ? ? struct cpdma_desc *desc = chan->head;
> + ? ? ? u32 status;
> +
> + ? ? ? if (!desc)
> + ? ? ? ? ? ? ? return -ENOENT;
> +
> + ? ? ? status ?= desc_read(desc, hw_mode);
> +
> + ? ? ? if (len)
> + ? ? ? ? ? ? ? *len = status & 0x7ff;
> +
> + ? ? ? if (buffer)
> + ? ? ? ? ? ? ? *buffer = desc_read_ptr(desc, sw_buffer);
> +
> + ? ? ? if (status & CPDMA_DESC_OWNER)
> + ? ? ? ? ? ? ? return -EBUSY;
> +
> + ? ? ? chan->head = desc_read_ptr(desc, hw_next);
> + ? ? ? chan_write(chan, cp, desc);
> +
> + ? ? ? cpdma_desc_free(priv, desc);
> + ? ? ? return 0;
> +}
> +
> +static int cpsw_init(struct eth_device *dev, bd_t *bis)
> +{
> + ? ? ? struct cpsw_priv ? ? ? ?*priv = dev->priv;
> + ? ? ? int i, ret;
> +
> + ? ? ? priv->data.control(1);

1 what? The "control" function probably needs a better name. And maybe
so does "1".

> +
> + ? ? ? /* soft reset the controller and initialize priv */
> + ? ? ? soft_reset(&priv->regs->soft_reset);
> +

See comment about soft_reset, above.

> +
> + ? ? ? cpsw_ale_port_state(priv, priv->host_port, ALE_PORT_STATE_FORWARD);
> +
> + ? ? ? cpsw_ale_add_ucast(priv, priv->dev->enetaddr, priv->host_port,
> + ? ? ? ? ? ? ? ? ? ? ? ? ?ALE_SECURE);
> + ? ? ? cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << priv->host_port);
> +
> + ? ? ? for_each_slave(priv, cpsw_slave_init, priv);

Same comment as before about the for-loop construct.

[...]

> + ? ? ? eth_register(dev);
> +
> + ? ? ? cpsw_mdio_init(dev->name, data->mdio_base, data->mdio_div);

I bet you only need one MDIO bus, and the new PHY API makes that
straightforward.

> +
> + ? ? ? return 1;
> +}
> diff --git a/include/netdev.h b/include/netdev.h
> index 96c7b9b..740213e 100644
> --- a/include/netdev.h
> +++ b/include/netdev.h
> @@ -185,4 +185,33 @@ struct mv88e61xx_config {
> ?int mv88e61xx_switch_initialize(struct mv88e61xx_config *swconfig);
> ?#endif /* CONFIG_MV88E61XX_SWITCH */
>
> +#ifdef CONFIG_DRIVER_TI_CPSW
> +
> +struct cpsw_slave_data {
> + ? ? ? u32 ? ? ? ? ? ? slave_reg_ofs;
> + ? ? ? u32 ? ? ? ? ? ? sliver_reg_ofs;
> + ? ? ? int ? ? ? ? ? ? phy_id;
> +};
> +
> +struct cpsw_platform_data {
> + ? ? ? u32 ? ? mdio_base;
> + ? ? ? u32 ? ? cpsw_base;
> + ? ? ? int ? ? mdio_div;
> + ? ? ? int ? ? channels; ? ? ? /* number of cpdma channels (symmetric) */
> + ? ? ? u32 ? ? cpdma_reg_ofs; ?/* cpdma register offset ? ? ? ? ? ? ? ?*/
> + ? ? ? int ? ? slaves; ? ? ? ? /* number of slave cpgmac ports ? ? ? ? */
> + ? ? ? u32 ? ? ale_reg_ofs; ? ?/* address lookup engine reg offset ? ? */
> + ? ? ? int ? ? ale_entries; ? ?/* ale table size ? ? ? ? ? ? ? ? ? ? ? */
> + ? ? ? u32 ? ? host_port_reg_ofs; ? ? ?/* cpdma host port registers ? ?*/
> + ? ? ? u32 ? ? hw_stats_reg_ofs; ? ? ? /* cpsw hw stats counters ? ? ? */
> + ? ? ? u32 ? ? mac_control;
> + ? ? ? struct cpsw_slave_data ?*slave_data;
> + ? ? ? void ? ?(*control)(int enabled);
> + ? ? ? void ? ?(*phy_init)(char *name, int addr);
> +};
> +
> +int cpsw_register(struct cpsw_platform_data *data);
> +
> +#endif /* CONFIG_DRIVER_TI_CPSW */

Hmmm... nothing here seems like it should be in a generic header like
"netdev.h". Only cpsw_register(), and probably not that, either. I see
there's some Marvell stuff in there, too, which is suspect, but let's
not propagate that issue. There's a cpu_eth_init() and a
board_eth_init(). Surely one of those can call cpsw_register(), and
then you can put the cpsw-specific structures in something like
include/cpsw.h.

Andy

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Tom Rini]

On Thu, Nov 10, 2011 at 3:43 PM, Andy Fleming <afleming@gmail.com> wrote:
> On Thu, Nov 10, 2011 at 6:40 AM, Chandan Nath <chandan.nath@ti.com> wrote:
>> From: Cyril Chemparathy <cyril@ti.com>
>>
>> CPSW is an on-chip ethernet switch that is found on various SoCs from Texas
>> Instruments. ?This patch adds a simple driver (based on the Linux driver) for
>> this hardware module.
>
> Which Linux driver is this based on?

drivers/net/cpsw.c, which isn't in mainline (and I didn't go running
about other maintainer trees).  There's a copy over in
http://arago-project.org/git/projects/?p=linux-am33x.git

-- 
Tom

[U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw ethernet device

[Kumar]

Andy,

> -----Original Message-----
> From: Andy Fleming [mailto:afleming at gmail.com]
> Sent: Friday, November 11, 2011 4:14 AM
> To: Kumar Nath, Chandan
> Cc: u-boot at lists.denx.de; Chemparathy, Cyril
> Subject: Re: [U-Boot] [PATCH v2 1/2] TI: netdev: add driver for cpsw
> ethernet device
> 
> On Thu, Nov 10, 2011 at 6:40 AM, Chandan Nath <chandan.nath@ti.com>
> wrote:
> > From: Cyril Chemparathy <cyril@ti.com>
> >
> > CPSW is an on-chip ethernet switch that is found on various SoCs from
> Texas
> > Instruments. ?This patch adds a simple driver (based on the Linux
> driver) for
> > this hardware module.
> 
> 
> Which Linux driver is this based on?

This is based on linux CPSW driver, which is yet to be up streamed.

> 
> 
> >
> > Signed-off-by: Cyril Chemparathy <cyril@ti.com>
> > ---
> > Changes since v1:
> > ?- Code cleanup and removal of cpsw_eth_set_mac_addr function
> >
> > ?drivers/net/Makefile | ? ?1 +
> > ?drivers/net/cpsw.c ? | ?862
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> > ?include/netdev.h ? ? | ? 29 ++
> > ?3 files changed, 892 insertions(+), 0 deletions(-)
> > ?create mode 100644 drivers/net/cpsw.c
> >
> > +
> > +#define BIT(x) ? ? ? ? ? ? ? ? (1 << (x))
> > +#define BITMASK(bits) ? ? ? ? ?(BIT(bits) - 1)
> > +#define PHY_REG_MASK ? ? ? ? ? 0x1f
> > +#define PHY_ID_MASK ? ? ? ? ? ?0x1f
> 
> These 4 defines seem very generic

I did not find any generic code for these definitions in arch/arm
and so keeping these definitions here.

> 
> 
> > +struct cpsw_priv {
> > + ? ? ? struct eth_device ? ? ? ? ? ? ? *dev;
> > + ? ? ? struct cpsw_platform_data ? ? ? data;
> > + ? ? ? int ? ? ? ? ? ? ? ? ? ? ? ? ? ? host_port;
> > +
> > + ? ? ? struct cpsw_regs ? ? ? ? ? ? ? ?*regs;
> > + ? ? ? void ? ? ? ? ? ? ? ? ? ? ? ? ? ?*dma_regs;
> > + ? ? ? struct cpsw_host_regs ? ? ? ? ? *host_port_regs;
> > + ? ? ? void ? ? ? ? ? ? ? ? ? ? ? ? ? ?*ale_regs;
> > +
> > + ? ? ? struct cpdma_desc ? ? ? ? ? ? ? descs[NUM_DESCS];
> > + ? ? ? struct cpdma_desc ? ? ? ? ? ? ? *desc_free;
> > + ? ? ? struct cpdma_chan ? ? ? ? ? ? ? rx_chan, tx_chan;
> > +
> > + ? ? ? struct cpsw_slave ? ? ? ? ? ? ? *slaves;
> > +#define for_each_slave(priv, func, arg...) ? ? ? ? ? ? ? ? ? ? \
> > + ? ? ? do { ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> > + ? ? ? ? ? ? ? int idx; ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?\
> > + ? ? ? ? ? ? ? for (idx = 0; idx < (priv)->data.slaves; idx++) \
> > + ? ? ? ? ? ? ? ? ? ? ? (func)((priv)->slaves + idx, ##arg); ? ?\
> > + ? ? ? } while (0)
> > +};
> 
> 
> It seems a bit awkward to me to put a complex macro like this inside
> the structure definition. After further review, I think it should also
> be defined differently, more like the list macros:
> 
> #define for_each_slave(slave, priv) \
>         for (slave = (priv)->slaves; slave != (priv)->slaves +
> (priv)->data.slaves; slave++)
> 
> It makes the code more maintainable, as the macro no longer contains
> the implicit assumption that "func" has a slave pointer as its first
> argument.
> 

This complex macro will be separated out and will be defined like list macros as you have
mentioned above.

> 
> > +
> > +static inline int cpsw_ale_get_field(u32 *ale_entry, u32 start, u32
> bits)
> > +{
> > + ? ? ? int idx;
> > +
> > + ? ? ? idx ? ?= start / 32;
> > + ? ? ? start -= idx * 32;
> > + ? ? ? idx ? ?= 2 - idx; /* flip */
> > + ? ? ? return (ale_entry[idx] >> start) & BITMASK(bits);
> > +}
> > +
> > +static inline void cpsw_ale_set_field(u32 *ale_entry, u32 start, u32
> bits,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? u32 value)
> > +{
> > + ? ? ? int idx;
> > +
> > + ? ? ? value &= BITMASK(bits);
> > + ? ? ? idx ? ?= start / 32;
> > + ? ? ? start -= idx * 32;
> > + ? ? ? idx ? ?= 2 - idx; /* flip */
> > + ? ? ? ale_entry[idx] &= ~(BITMASK(bits) << start);
> > + ? ? ? ale_entry[idx] |= ?(value << start);
> > +}
> 
> 
> These two functions seem both very generic, and also over-worked.
> 

Should this be changed to a different implementation?

> 
> > +
> > +#define DEFINE_ALE_FIELD(name, start, bits)
> ? ?\
> > +static inline int cpsw_ale_get_##name(u32 *ale_entry)
> ? ?\
> > +{
> ? ?\
> > + ? ? ? return cpsw_ale_get_field(ale_entry, start, bits);
> ? ?\
> > +}
> ? ?\
> > +static inline void cpsw_ale_set_##name(u32 *ale_entry, u32 value)
> ? ?\
> > +{
> ? ?\
> > + ? ? ? cpsw_ale_set_field(ale_entry, start, bits, value);
> ? ?\
> > +}
> > +
> > +DEFINE_ALE_FIELD(entry_type, ? ? ? ? ? 60, ? ? 2)
> > +DEFINE_ALE_FIELD(mcast_state, ? ? ? ? ?62, ? ? 2)
> > +DEFINE_ALE_FIELD(port_mask, ? ? ? ? ? ?64, ? ? 3)
> > +DEFINE_ALE_FIELD(ucast_type, ? ? ? ? ? 66, ? ? 2)
> > +DEFINE_ALE_FIELD(port_num, ? ? ? ? ? ? 64, ? ? 2)
> > +DEFINE_ALE_FIELD(blocked, ? ? ? ? ? ? ?63, ? ? 1)
> > +DEFINE_ALE_FIELD(secure, ? ? ? ? ? ? ? 62, ? ? 1)
> > +DEFINE_ALE_FIELD(mcast, ? ? ? ? ? ? ? ? ? ? ? ?47, ? ? 1)
> > +
> > +/* The MAC address field in the ALE entry cannot be macroized as
> above */
> > +static inline void cpsw_ale_get_addr(u32 *ale_entry, u8 *addr)
> > +{
> > + ? ? ? int i;
> > +
> > + ? ? ? for (i = 0; i < 6; i++)
> > + ? ? ? ? ? ? ? addr[i] = cpsw_ale_get_field(ale_entry, 40 - 8*i, 8);
> > +}
> > +
> > +static inline void cpsw_ale_set_addr(u32 *ale_entry, u8 *addr)
> > +{
> > + ? ? ? int i;
> > +
> > + ? ? ? for (i = 0; i < 6; i++)
> > + ? ? ? ? ? ? ? cpsw_ale_set_field(ale_entry, 40 - 8*i, 8, addr[i]);
> > +}
> > +
> > +static int cpsw_ale_read(struct cpsw_priv *priv, int idx, u32
> *ale_entry)
> > +{
> > + ? ? ? int i;
> > +
> > + ? ? ? __raw_writel(idx, priv->ale_regs + ALE_TABLE_CONTROL);
> > +
> > + ? ? ? for (i = 0; i < ALE_ENTRY_WORDS; i++)
> > + ? ? ? ? ? ? ? ale_entry[i] = __raw_readl(priv->ale_regs + ALE_TABLE
> + 4 * i);
> > +
> > + ? ? ? return idx;
> > +}
> > +
> > +static int cpsw_ale_write(struct cpsw_priv *priv, int idx, u32
> *ale_entry)
> > +{
> > + ? ? ? int i;
> > +
> > + ? ? ? for (i = 0; i < ALE_ENTRY_WORDS; i++)
> > + ? ? ? ? ? ? ? __raw_writel(ale_entry[i], priv->ale_regs + ALE_TABLE
> + 4 * i);
> > +
> > + ? ? ? __raw_writel(idx | ALE_TABLE_WRITE, priv->ale_regs +
> ALE_TABLE_CONTROL);
> > +
> > + ? ? ? return idx;
> > +}
> 
> 
> I don't know how your device works, but it seems odd to me to have to
> read out a large table, and modify it, and then write it back out.
> 
> I'm also highly suspicious of the overworked bitfield math. Are
> standard C bitfields not usable for some reason? It borders on code
> obfuscation, IMO. I've spent far too much time, already, just trying
> to figure out what that code was doing, and why.
> 

I will think of some standard C bitfields to make it simpler.

> > +static inline void cpsw_ale_port_state(struct cpsw_priv *priv, int
> port,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int val)
> > +{
> > + ? ? ? int offset = ALE_PORTCTL + 4 * port;
> > + ? ? ? u32 tmp, mask = 0x3;
> > +
> > + ? ? ? tmp ?= __raw_readl(priv->ale_regs + offset);
> > + ? ? ? tmp &= ~mask;
> > + ? ? ? tmp |= val & 0x3;
> 
> All of this can probably be done with more generic setbits functions,
> but at least use "mask" consistently for both the clearing and the
> masking of "val".
> 

Ok, mask will be used consistently.

> > + ? ? ? __raw_writel(tmp, priv->ale_regs + offset);
> > +}
> > +
> > +static struct cpsw_mdio_regs *mdio_regs;
> 
> 
> Global variables like this shouldn't be necessary. Especially with the
> new PHY API
>

This global variable is required to access mdio registers across different functions.
 
> 	
> 
> > +
> > +static int cpsw_mdio_read(char *devname, unsigned char phy_id,
> > + ? ? ? ? ? ? ? ? ? ? ? ? unsigned char phy_reg, unsigned short
> *data)
> > +{
> 
> This is the legacy interface for PHY interactions. You should look
> into using the new "PHYLIB" code.
> 

Sure, I will be using new "PHYLIB" code instead of this legacy code.

> 
> > + ? ? ? u32 reg;
> > +
> > + ? ? ? if (phy_reg & ~PHY_REG_MASK || phy_id & ~PHY_ID_MASK)
> > + ? ? ? ? ? ? ? return -EINVAL;
> > +
> > + ? ? ? wait_for_user_access();
> > + ? ? ? reg = (USERACCESS_GO | USERACCESS_READ | (phy_reg << 21) |
> > + ? ? ? ? ? ? ?(phy_id << 16));
> > + ? ? ? __raw_writel(reg, &mdio_regs->user[0].access);
> > + ? ? ? reg = wait_for_user_access();
> > +
> > + ? ? ? *data = (reg & USERACCESS_ACK) ? (reg & USERACCESS_DATA) : -
> 1;
> > + ? ? ? return (reg & USERACCESS_ACK) ? 0 : -EIO;
> > +}
> > +
> > +static int cpsw_mdio_write(char *devname, unsigned char phy_id,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ?unsigned char phy_reg, unsigned short
> data)
> 
> Same comment as above.
> 

Yes, I will use "PHYLIB" code instead of legacy code.

> 
> > +static void cpsw_mdio_init(char *name, u32 mdio_base, u32 div)
> > +{
> > + ? ? ? mdio_regs = (struct cpsw_mdio_regs *)mdio_base;
> > +
> > + ? ? ? /* set enable and clock divider */
> > + ? ? ? __raw_writel(div | CONTROL_ENABLE, &mdio_regs->control);
> > +
> > + ? ? ? /*
> > + ? ? ? ?* wait for scan logic to settle:
> > + ? ? ? ?* the scan time consists of (a) a large fixed component, and
> (b) a
> > + ? ? ? ?* small component that varies with the mii bus frequency.
> ?These
> > + ? ? ? ?* were estimated using measurements at 1.1 and 2.2 MHz on
> tnetv107x
> > + ? ? ? ?* silicon. ?Since the effect of (b) was found to be largely
> > + ? ? ? ?* negligible, we keep things simple here.
> > + ? ? ? ?*/
> > + ? ? ? udelay(1000);
> 
> This seems sketchy. Is there no way to *know* the bus is ready, other
> than to wait?
> 
> > +
> > + ? ? ? miiphy_register(name, cpsw_mdio_read, cpsw_mdio_write);
> 
> 
> This is the legacy API.
> 

PHYLIB code will be used instead of this.

> 
> > +}
> > +
> > +static inline void soft_reset(void *reg)
> > +{
> > + ? ? ? __raw_writel(1, reg);
> > + ? ? ? while (__raw_readl(reg) & 1)
> > + ? ? ? ? ? ? ? ;
> > +}
> 
> 
> ???
> 
> Can reg be *any* reg? Use a better name. Also, name "1" something
> sensible.
> 
> Perhaps a cleaner implementation (which might be usable in other
> people's code) would be something like:
> 
> /* Set a self-clearing bit in a register, and wait for it to clear */
> static inline void setbit_and_wait_for_clear32(void *addr, u32 mask)
> {
>    __raw_writel(val, addr);
>   while (__raw_readl(addr) & val)
>     ;
> }
> 
> 

Ok, I will implement this in a cleaner manner.

> > +static void cpsw_slave_update_link(struct cpsw_slave *slave,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?struct cpsw_priv *priv, int *link)
> > +{
> > + ? ? ? char *name = priv->dev->name;
> > + ? ? ? int phy_id = slave->data->phy_id;
> > + ? ? ? int speed, duplex;
> > + ? ? ? unsigned short reg;
> > + ? ? ? u32 mac_control = 0;
> > +
> > + ? ? ? if (miiphy_read(name, phy_id, MII_BMSR, &reg))
> > + ? ? ? ? ? ? ? return; /* could not read, assume no link */
> 
> 
> Please use phy_read(), with the new API.
>

Ok, I will use phy_read(), instead of miiphy_read.
 
> 
> > +
> > + ? ? ? if (reg & BMSR_LSTATUS) { /* link up */
> > + ? ? ? ? ? ? ? speed = miiphy_speed(name, phy_id);
> > + ? ? ? ? ? ? ? duplex = miiphy_duplex(name, phy_id);
> > +
> > + ? ? ? ? ? ? ? *link = 1;
> > + ? ? ? ? ? ? ? mac_control = priv->data.mac_control;
> > + ? ? ? ? ? ? ? if (speed == 1000)
> > + ? ? ? ? ? ? ? ? ? ? ? mac_control |= BIT(7); ?/* GIGABITEN ? ?*/
> > + ? ? ? ? ? ? ? if (duplex == FULL)
> > + ? ? ? ? ? ? ? ? ? ? ? mac_control |= BIT(0); ?/* FULLDUPLEXEN */
> 
> Why not:
>     mac_control |= GIGABITEN;
> 
> etc
> 

Sure, I will make this change.

> 
> 
> > +static int cpsw_update_link(struct cpsw_priv *priv)
> > +{
> > + ? ? ? int link = 0;
> > + ? ? ? for_each_slave(priv, cpsw_slave_update_link, priv, &link);
> 
> As I mentioned, there are a lot of similarly-named macros in Linux and
> U-boot. Usually they act exactly like for-loops, where you provide an
> iterator, and then use it in each iteration:
> 
> struct cpsw_slave *slave;
> 
> for_each_slave(slave, priv)
>   cpsw_slave_update_link(slave, priv, &link);
> 
> I think this is much easier to understand and maintain.
> 
> > + ? ? ? return link;
> 
> Also, link looks useless. It will always be the value in the last
> slave. Is that what you meant to do?
> 
> 

Ok, I will use this as per your comment.
for_each_slave(slave, priv)
	cpsw_slave_update_link(slave, priv, &link);

Also, I am returning link which returns the value of last slave.

> > +}
> > +
> > +static void cpsw_slave_init(struct cpsw_slave *slave, struct
> cpsw_priv *priv)
> > +{
> 
> [...]
> 
> > +
> > + ? ? ? priv->data.phy_init(priv->dev->name, slave->data->phy_id);
> 
> What does this do? Probably needs to be converted to the newer API.
> 

I will remove this and new PHYLIB APIs will be used instead.

> 
> > +}
> > +
> > +static struct cpdma_desc *cpdma_desc_alloc(struct cpsw_priv *priv)
> > +{
> > + ? ? ? struct cpdma_desc *desc = priv->desc_free;
> > +
> > + ? ? ? if (desc)
> > + ? ? ? ? ? ? ? priv->desc_free = desc_read_ptr(desc, hw_next);
> > + ? ? ? return desc;
> > +}
> > +
> > +static void cpdma_desc_free(struct cpsw_priv *priv, struct
> cpdma_desc *desc)
> > +{
> > + ? ? ? if (desc) {
> > + ? ? ? ? ? ? ? desc_write(desc, hw_next, priv->desc_free);
> > + ? ? ? ? ? ? ? priv->desc_free = desc;
> > + ? ? ? }
> > +}
> > +
> > +static int cpdma_submit(struct cpsw_priv *priv, struct cpdma_chan
> *chan,
> > + ? ? ? ? ? ? ? ? ? ? ? volatile void *buffer, int len)
> > +{
> > + ? ? ? struct cpdma_desc *desc, *prev;
> > + ? ? ? u32 mode;
> > +
> > + ? ? ? desc = cpdma_desc_alloc(priv);
> > + ? ? ? if (!desc)
> > + ? ? ? ? ? ? ? return -ENOMEM;
> > +
> > + ? ? ? if (len < PKT_MIN)
> > + ? ? ? ? ? ? ? len = PKT_MIN;
> > +
> > + ? ? ? mode = CPDMA_DESC_OWNER | CPDMA_DESC_SOP | CPDMA_DESC_EOP;
> > +
> > + ? ? ? desc_write(desc, hw_next, ? 0);
> > + ? ? ? desc_write(desc, hw_buffer, buffer);
> > + ? ? ? desc_write(desc, hw_len, ? ?len);
> > + ? ? ? desc_write(desc, hw_mode, ? mode | len);
> > + ? ? ? desc_write(desc, sw_buffer, buffer);
> > + ? ? ? desc_write(desc, sw_len, ? ?len);
> > +
> > + ? ? ? if (!chan->head) {
> > + ? ? ? ? ? ? ? /* simple case - first packet enqueued */
> > + ? ? ? ? ? ? ? chan->head = desc;
> > + ? ? ? ? ? ? ? chan->tail = desc;
> > + ? ? ? ? ? ? ? chan_write(chan, hdp, desc);
> > + ? ? ? ? ? ? ? goto done;
> > + ? ? ? }
> > +
> > + ? ? ? /* not the first packet - enqueue at the tail */
> > + ? ? ? prev = chan->tail;
> > + ? ? ? desc_write(prev, hw_next, desc);
> > + ? ? ? chan->tail = desc;
> > +
> > + ? ? ? /* next check if EOQ has been triggered already */
> > + ? ? ? if (desc_read(prev, hw_mode) & CPDMA_DESC_EOQ)
> > + ? ? ? ? ? ? ? chan_write(chan, hdp, desc);
> > +
> > +done:
> > + ? ? ? if (chan->rxfree)
> > + ? ? ? ? ? ? ? chan_write(chan, rxfree, 1);
> > + ? ? ? return 0;
> > +}
> > +
> > +static int cpdma_process(struct cpsw_priv *priv, struct cpdma_chan
> *chan,
> > + ? ? ? ? ? ? ? ? ? ? ? ?void **buffer, int *len)
> > +{
> > + ? ? ? struct cpdma_desc *desc = chan->head;
> > + ? ? ? u32 status;
> > +
> > + ? ? ? if (!desc)
> > + ? ? ? ? ? ? ? return -ENOENT;
> > +
> > + ? ? ? status ?= desc_read(desc, hw_mode);
> > +
> > + ? ? ? if (len)
> > + ? ? ? ? ? ? ? *len = status & 0x7ff;
> > +
> > + ? ? ? if (buffer)
> > + ? ? ? ? ? ? ? *buffer = desc_read_ptr(desc, sw_buffer);
> > +
> > + ? ? ? if (status & CPDMA_DESC_OWNER)
> > + ? ? ? ? ? ? ? return -EBUSY;
> > +
> > + ? ? ? chan->head = desc_read_ptr(desc, hw_next);
> > + ? ? ? chan_write(chan, cp, desc);
> > +
> > + ? ? ? cpdma_desc_free(priv, desc);
> > + ? ? ? return 0;
> > +}
> > +
> > +static int cpsw_init(struct eth_device *dev, bd_t *bis)
> > +{
> > + ? ? ? struct cpsw_priv ? ? ? ?*priv = dev->priv;
> > + ? ? ? int i, ret;
> > +
> > + ? ? ? priv->data.control(1);
> 
> 1 what? The "control" function probably needs a better name. And maybe
> so does "1".
> 

This is not required and will be removed from the code.

> > +
> > + ? ? ? /* soft reset the controller and initialize priv */
> > + ? ? ? soft_reset(&priv->regs->soft_reset);
> > +
> 
> See comment about soft_reset, above.
> 

Ok, I will address this as above.

> > +
> > + ? ? ? cpsw_ale_port_state(priv, priv->host_port,
> ALE_PORT_STATE_FORWARD);
> > +
> > + ? ? ? cpsw_ale_add_ucast(priv, priv->dev->enetaddr, priv-
> >host_port,
> > + ? ? ? ? ? ? ? ? ? ? ? ? ?ALE_SECURE);
> > + ? ? ? cpsw_ale_add_mcast(priv, NetBcastAddr, 1 << priv->host_port);
> > +
> > + ? ? ? for_each_slave(priv, cpsw_slave_init, priv);
> 
> Same comment as before about the for-loop construct.
> 

Ok, will address in similar way as above.

> [...]
> 
> > + ? ? ? eth_register(dev);
> > +
> > + ? ? ? cpsw_mdio_init(dev->name, data->mdio_base, data->mdio_div);
> 
> I bet you only need one MDIO bus, and the new PHY API makes that
> straightforward.
>

Yes, I will use new PHYLIB APIs.
 
> > +
> > + ? ? ? return 1;
> > +}
> > diff --git a/include/netdev.h b/include/netdev.h
> > index 96c7b9b..740213e 100644
> > --- a/include/netdev.h
> > +++ b/include/netdev.h
> > @@ -185,4 +185,33 @@ struct mv88e61xx_config {
> > ?int mv88e61xx_switch_initialize(struct mv88e61xx_config *swconfig);
> > ?#endif /* CONFIG_MV88E61XX_SWITCH */
> >
> > +#ifdef CONFIG_DRIVER_TI_CPSW
> > +
> > +struct cpsw_slave_data {
> > + ? ? ? u32 ? ? ? ? ? ? slave_reg_ofs;
> > + ? ? ? u32 ? ? ? ? ? ? sliver_reg_ofs;
> > + ? ? ? int ? ? ? ? ? ? phy_id;
> > +};
> > +
> > +struct cpsw_platform_data {
> > + ? ? ? u32 ? ? mdio_base;
> > + ? ? ? u32 ? ? cpsw_base;
> > + ? ? ? int ? ? mdio_div;
> > + ? ? ? int ? ? channels; ? ? ? /* number of cpdma channels
> (symmetric) */
> > + ? ? ? u32 ? ? cpdma_reg_ofs; ?/* cpdma register offset
> ? ?*/
> > + ? ? ? int ? ? slaves; ? ? ? ? /* number of slave cpgmac ports
> ? */
> > + ? ? ? u32 ? ? ale_reg_ofs; ? ?/* address lookup engine reg offset
> ? */
> > + ? ? ? int ? ? ale_entries; ? ?/* ale table size
> ? */
> > + ? ? ? u32 ? ? host_port_reg_ofs; ? ? ?/* cpdma host port registers
> ? ?*/
> > + ? ? ? u32 ? ? hw_stats_reg_ofs; ? ? ? /* cpsw hw stats counters
> ? */
> > + ? ? ? u32 ? ? mac_control;
> > + ? ? ? struct cpsw_slave_data ?*slave_data;
> > + ? ? ? void ? ?(*control)(int enabled);
> > + ? ? ? void ? ?(*phy_init)(char *name, int addr);
> > +};
> > +
> > +int cpsw_register(struct cpsw_platform_data *data);
> > +
> > +#endif /* CONFIG_DRIVER_TI_CPSW */
> 
> Hmmm... nothing here seems like it should be in a generic header like
> "netdev.h". Only cpsw_register(), and probably not that, either. I see
> there's some Marvell stuff in there, too, which is suspect, but let's
> not propagate that issue. There's a cpu_eth_init() and a
> board_eth_init(). Surely one of those can call cpsw_register(), and
> then you can put the cpsw-specific structures in something like
> include/cpsw.h.
> 

Sure, I will remove this code from netdev.h and will keep in separate cpsw header, i.e. cpsw.h

> Andy