make egiga common to kirkwood and orion5x

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 4/4] egiga: add support for orion5x

Now that egiga is detached from kirkwood, we can add support for orion5x. This requires making the structures representing egiga registers and descriptors volatile, otherwise writes to them happen in the wrong order -- this did not affect kirkwood but does affect orion5x.

Signed-off-by Albert Aribaud <[email protected]>

---
 arch/arm/cpu/arm926ejs/orion5x/cpu.c        |   18 +++
 arch/arm/include/asm/arch-orion5x/orion5x.h |    5 +
 board/LaCie/edminiv2/edminiv2.c             |   36 +++++
 board/LaCie/edminiv2/edminiv2.h             |   41 ++++++
 drivers/net/egiga.c                         |    4 +-
 drivers/net/egiga.h                         |  198 
+++++++++++++-------------
 include/configs/edminiv2.h                  |   30 +++-
 7 files changed, 225 insertions(+), 107 deletions(-)
 create mode 100644 board/LaCie/edminiv2/edminiv2.h

arch/arm/cpu/arm926ejs/orion5x/cpu.c +0 -0

index 03c6d06..a468c4d 100644
@@ -268,3 +268,21 @@ int arch_misc_init(void)
 	return 0;
 }
 #endif /* CONFIG_ARCH_MISC_INIT */

arch/arm/cpu/arm926ejs/orion5x/cpu.c +8 -0

+ * if a MAC address has to be generated
+ */
+unsigned char get_random_hex(void)
+{
+	static unsigned char seed = 0;
+	return seed++;
+}
 
+#endif

arch/arm/include/asm/arch-orion5x/orion5x.h +5 -0

index 4008c84..ffe26bd 100644
@@ -56,6 +56,11 @@
 #define ORION5X_USB20_PORT1_BASE		
 (ORION5X_REGISTER(0xA0000))
 #define ORION5X_EGIGA_BASE			
 (ORION5X_REGISTER(0x72000))
 
+/* EGIGA expects EGIGA0 #define'd */
+#if defined (CONFIG_EGIGA)
+#define EGIGA0_BASE				ORION5X_EGIGA_BASE
+#endif
+
 #define CONFIG_MAX_RAM_BANK_SIZE		(64*1024*1024)

/* include here SoC variants. 5181, 5281, 6183 should go here when

board/LaCie/edminiv2/edminiv2.c +1 -0

index 54c0ffe..d46ee4a 100644
@@ -27,6 +27,7 @@
 #include <common.h>
 #include <miiphy.h>
 #include <asm/arch/orion5x.h>
+#include "edminiv2.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 

board/LaCie/edminiv2/edminiv2.c +35 -0

+
+#if defined (CONFIG_CMD_NET) && defined (CONFIG_RESET_PHY_R)
+/* Configure and enable MV88E1116 PHY */
+void reset_phy(void)
+{
+	u16 reg;
+	u16 devadr;
+	char *name = "egiga0";
+
+	if (miiphy_set_current_dev(name))
+		return;
+
+	/* command to read PHY dev address */
+	if (miiphy_read(name, 0xEE, 0xEE, (u16 *) &devadr)) {
+		printf("Err..%s could not read PHY dev address\n",
+			__FUNCTION__);
+		return;
+	}
+
+	/*
+	 * Enable RGMII delay on Tx and Rx for CPU port
+	 * Ref: sec 4.7.2 of chip datasheet
+	 */
+	miiphy_write(name, devadr, MV88E1116_PGADR_REG, 2);
+	miiphy_read(name, devadr, MV88E1116_MAC_CTRL_REG, &reg);
+	reg |= (MV88E1116_RGMII_RXTM_CTRL | MV88E1116_RGMII_TXTM_CTRL);
+	miiphy_write(name, devadr, MV88E1116_MAC_CTRL_REG, reg);
+	miiphy_write(name, devadr, MV88E1116_PGADR_REG, 0);
+
+	/* reset the phy */
+	miiphy_reset(name, devadr);
+
+	printf("88E1116 Initialized on %s\n", name);
+}
+#endif /* CONFIG_RESET_PHY_R */

board/LaCie/edminiv2/edminiv2.h new file +41 -0

new file mode 100644
index 0000000..88e62b2
@@ -0,0 +1,41 @@
+/*
+ * (C) Copyright 2009
+ * Net Insight <www.netinsight.net>
+ * Written-by: Simon Kagstrom <[email protected]>
+ *
+ * Based on sheevaplug.h:
+ * (C) Copyright 2009
+ * Marvell Semiconductor <www.marvell.com>
+ * Written-by: Prafulla Wadaskar <[email protected]>
+ *
+ * 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., 51 Franklin Street, Fifth Floor, Boston,
+ * MA 02110-1301 USA
+ */
+
+#ifndef __EDMINIV2_BASE_H
+#define __EDMINIV2_BASE_H
+
+/* PHY related */
+#define MV88E1116_LED_FCTRL_REG		10
+#define MV88E1116_CPRSP_CR3_REG		21
+#define MV88E1116_MAC_CTRL_REG		21
+#define MV88E1116_PGADR_REG		22
+#define MV88E1116_RGMII_TXTM_CTRL	(1 << 4)
+#define MV88E1116_RGMII_RXTM_CTRL	(1 << 5)
+
+#endif /* __EDMINIV2_BASE_H */

drivers/net/egiga.c +2 -0

index bb0de68..320e2ca 100644
@@ -37,6 +37,8 @@
 #include <asm/byteorder.h>
 #if defined (CONFIG_KIRKWOOD)
 #include <asm/arch/kirkwood.h>
+#elif defined CONFIG_ORION5X
+#include <asm/arch/orion5x.h>
 #endif
 #include "egiga.h"
 

drivers/net/egiga.c +0 -0

 			win_param.enable = 0;
 			win_param.attrib = 0;

drivers/net/egiga.c +0 -0

 			break;

drivers/net/egiga.h +0 -0

index f758d3c..89ac519 100644

drivers/net/egiga.h +0 -0

 /* port device data struct */

include/configs/edminiv2.h +9 -1

index c3d95a0..1f75920 100644
@@ -60,7 +60,7 @@
 
 #define ORION5X_MPP0_7		0x00000003
 #define ORION5X_MPP8_15		0x55550000
-#define ORION5X_MPP16_23	0x00000000
+#define ORION5X_MPP16_23	0x00005555
 
 /*
  * Board-specific values for Orion5x GPIO low level init:

@@ -110,6 +110,14 @@
 #define CONFIG_SYS_FLASH_SECTSZ \
 	{16384, 8192, 8192, 32768, \
 	 65536, 65536, 65536, 65536, 65536, 65536, 65536}
+/*
+ * Ethernet
+  */
+
+#define CONFIG_EGIGA		/* Enable Egiga Gbe Controller Driver */
+#define CONFIG_EGIGA_PORTS	{1,0}	/* enable port 0 only */
+#define CONFIG_PHY_BASE_ADR	0x8
+#define CONFIG_RESET_PHY_R	/* use reset_phy() to init 

mv8831116 PHY */

/* auto boot */
#define CONFIG_BOOTDELAY 3 /* default enable autoboot */

include/configs/edminiv2.h +0 -1

@@ -131,12 +139,20 @@
  * Commands configuration - using default command set for now
  */
 #include <config_cmd_default.h>
-/*

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 3/4] egiga: remove references to kirkwood SoC

Macros, types, variables, functions in egiga
refer to kirkwood even though they are not
kirkwood-specific. Rename them, across the
whole source tree when necessary.

Quoted 7 lines

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 3/4] egiga: remove references to kirkwood SoC

Quoted 4 lines

Macros, types, variables, functions in egiga
refer to kirkwood even though they are not
kirkwood-specific. Rename them, across the
whole source tree when necessary.

NAK, as commented for Patch 1/4
you can address it more like splitting a driver into generic and specific interfaces and then adding new support

Regards..
Prafulla ..

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.aribaud at free.fr]
Sent: Monday, July 05, 2010 5:12 PM
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH 3/4] egiga: remove references to kirkwood SoC

Le 05/07/2010 13:02, Prafulla Wadaskar a ?crit :

Quoted 6 lines

Quoted 6 lines

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 3/4] egiga: remove references to

kirkwood SoC

Quoted 8 lines

Quoted 5 lines

Macros, types, variables, functions in egiga
refer to kirkwood even though they are not
kirkwood-specific. Rename them, across the
whole source tree when necessary.

NAK, as commented for Patch 1/4
you can address it more like splitting a driver into

generic and specific interfaces

Quoted 1 line

and then adding new support

I'm not sure I get your meaning here. Do you suggest I submit not four
but only two patches, one with all egiga-out-of-kirkwood changes and one
with orion5x support? Or is is something else entirely?

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 2/4] egiga: Remove functional dependency on kirkwood

Set DRAM windows by using gd as other drivers do,
instead of calling kirkwood-specific functions.

Signed-off-by Albert Aribaud <[email protected]>

---
 drivers/net/egiga.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

drivers/net/egiga.c +2 -2

index 50e8ff3..cbe4748 100644
@@ -38,6 +38,8 @@
 #include <asm/arch/kirkwood.h>
 #include "egiga.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #define KIRKWOOD_PHY_ADR_REQUEST 0xee
 #define KWGBE_SMI_REG (((struct kwgbe_registers 
 *)KW_EGIGA0_BASE)->smi)

@@ -246,8 +248,8 @@ static void set_dram_access(struct 
 kwgbe_registers *regs)
 		win_param.access_ctrl = EWIN_ACCESS_FULL;
 		win_param.high_addr = 0;
 		/* Get bank base */
-		win_param.base_addr = kw_sdram_bar(i);
-		win_param.size = kw_sdram_bs(i);	/* Get 

bank size */

Quoted 39 lines

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 2/4] egiga: Remove functional dependency on kirkwood

Set DRAM windows by using gd as other drivers do,
instead of calling kirkwood-specific functions.

Signed-off-by Albert Aribaud<[email protected]>

---
  drivers/net/egiga.c |    6 ++++--
  1 files changed, 4 insertions(+), 2 deletions(-)

drivers/net/egiga.c +2 -2

index 50e8ff3..cbe4748 100644
@@ -38,6 +38,8 @@
  #include<asm/arch/kirkwood.h>
  #include "egiga.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
  #define KIRKWOOD_PHY_ADR_REQUEST 0xee
  #define KWGBE_SMI_REG (((struct kwgbe_registers
 *)KW_EGIGA0_BASE)->smi)

@@ -246,8 +248,8 @@ static void set_dram_access(struct
 kwgbe_registers *regs)
  		win_param.access_ctrl = EWIN_ACCESS_FULL;
  		win_param.high_addr = 0;
  		/* Get bank base */
-		win_param.base_addr = kw_sdram_bar(i);
-		win_param.size = kw_sdram_bs(i);	/* Get

bank size */

drivers/net/egiga.c static void set_dram_access(struct kwgbe_registers *regs) · line 255

Quoted 5 lines

drivers/net/egiga.c +2 -0

+		win_param.base_addr = gd->bd->bi_dram[i].start;
+		win_param.size = gd->bd->bi_dram[i].size;
  		if (win_param.size == 0)
  			win_param.enable = 0;
  		else

this is good patch indeed to make the driver more generic, which is independent of this activity please post this patch for current version i.e. kirkwood_egiga.c

Regards..
Prafulla . .

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.aribaud at free.fr]
Sent: Monday, July 05, 2010 11:30 PM
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de
Subject: Re: [PATCH 2/4] egiga: Remove functional dependency on kirkwood

Le 05/07/2010 13:03, Prafulla Wadaskar a ?crit :

Quoted 31 lines

Quoted 31 lines

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 2/4] egiga: Remove functional dependency on kirkwood

Set DRAM windows by using gd as other drivers do,
instead of calling kirkwood-specific functions.

Signed-off-by Albert Aribaud<[email protected]>

---
  drivers/net/egiga.c |    6 ++++--
  1 files changed, 4 insertions(+), 2 deletions(-)

drivers/net/egiga.c +2 -0

index 50e8ff3..cbe4748 100644
@@ -38,6 +38,8 @@
  #include<asm/arch/kirkwood.h>
  #include "egiga.h"
 
+DECLARE_GLOBAL_DATA_PTR;
+
  #define KIRKWOOD_PHY_ADR_REQUEST 0xee
  #define KWGBE_SMI_REG (((struct kwgbe_registers
 *)KW_EGIGA0_BASE)->smi)

Quoted 15 lines

drivers/net/egiga.c static void set_dram_access(struct kwgbe_registers *regs) · line 255

Quoted 13 lines

drivers/net/egiga.c +0 -2

@@ -246,8 +248,8 @@ static void set_dram_access(struct
 kwgbe_registers *regs)
  		win_param.access_ctrl = EWIN_ACCESS_FULL;
  		win_param.high_addr = 0;
  		/* Get bank base */
-		win_param.base_addr = kw_sdram_bar(i);
-		win_param.size = kw_sdram_bs(i);	/* Get

bank size */

drivers/net/egiga.c +2 -0

+		win_param.base_addr = gd->bd->bi_dram[i].start;
+		win_param.size = gd->bd->bi_dram[i].size;
  		if (win_param.size == 0)
  			win_param.enable = 0;
  		else

this is good patch indeed to make the driver more generic,

which is independent of this activity

Quoted 2 lines

please post this patch for current version i.e. kirkwood_egiga.c

Quoted 2 lines

Regards..
Prafulla . .

There is also a similar fix (move from kw_sdram_{bar,bs} to gd) to be done on drivers/usb/host/ehci-kirkwood.c, apparently. As I wasn't planning supporting USB with this patchset, I'd ignored it, but if you
want I can submit a separate patch for it too.

The names of the egiga files mention kirkwood even though they are not kirkwood-specific - change them.

Hi Albert,

Quoted 3 lines · Albert Aribaud

The names of the egiga files mention kirkwood even though they are not kirkwood-specific - change them.

I'm not a git expert so there may be a better way than this, but we want this to show up as a rename, rather than an add/delete. I would accomplish this using 'git mv', followed by 'git format-patch -C'

regards,
Ben

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Albert Aribaud
Sent: Friday, July 02, 2010 10:23 PM
To: u-boot at lists.denx.de
Subject: [U-Boot] [PATCH 1/4] ARM: Rename kirkwood_egiga driver to egiga

The names of the egiga files mention kirkwood even though they are not kirkwood-specific - change them.

Signed-off-by Albert Aribaud <[email protected]>

---
 drivers/net/Makefile         |    2 +-
 drivers/net/egiga.c          |  719 
++++++++++++++++++++++++++++++++++++++++++
 drivers/net/egiga.h          |  505 +++++++++++++++++++++++++++++
 drivers/net/kirkwood_egiga.c |  719 
------------------------------------------
 drivers/net/kirkwood_egiga.h |  505 -----------------------------
 5 files changed, 1225 insertions(+), 1225 deletions(-)
 create mode 100644 drivers/net/egiga.c
 create mode 100644 drivers/net/egiga.h

if it is generic gigabit Ethernet driver abstraction, could be applicable to any kind of SOC then the same egiga is relevant. which is not the case here if it is Marvell specific then the name should be like- mv_egiga

I think we should sync on the arch first, that would same lot of coding/review efforts I have below mentioned architecture for this driver -
1. Abstract generic SoC independent driver skeleton into mv"_egiga.c/h
2. Abstract and put Kirkwood specific functions is arch/arm/cpu/kirkwood/egiga.c
3. Create and put Orion specific functions is arch/arm/cpu/orion5x/egiga.c
4. Use common APIs/Macros to call SoC specific functions in generic driver
5. same strategy should be applied for header files abstraction.

there should be generic CONFIG to address this build, and CONFIG_KIRKWOOD_EGIGA should address its SOC specific dependency, you have done this in patch 3/4

As suggested by Ben rest should be rename activity than delete/add for 1/x patch

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.aribaud at free.fr]
Sent: Monday, July 05, 2010 5:06 PM
To: Prafulla Wadaskar
Cc: u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH 1/4] ARM: Rename kirkwood_egiga driver to egiga

Le 05/07/2010 13:02, Prafulla Wadaskar a ?crit :

Quoted 1 line

if it is generic gigabit Ethernet driver abstraction, could

be applicable to any kind of SOC then the same egiga is relevant. which is not the case here

Quoted 1 line

if it is Marvell specific then the name should be like- mv_egiga

As it is Marvell-IP specific, I'll rename it to mv_egiga.

Quoted 1 line

I think we should sync on the arch first, that would same

lot of coding/review efforts

Quoted 2 lines

I have below mentioned architecture for this driver -
1. Abstract generic SoC independent driver skeleton into

mv"_egiga.c/h

Quoted 1 line

2. Abstract and put Kirkwood specific functions is

arch/arm/cpu/kirkwood/egiga.c

Quoted 1 line

3. Create and put Orion specific functions is

arch/arm/cpu/orion5x/egiga.c

Quoted 1 line

4. Use common APIs/Macros to call SoC specific functions in

generic driver

Quoted 1 line

5. same strategy should be applied for header files abstraction.

Agree.

Quoted 1 line

there should be generic CONFIG to address this build, and

CONFIG_KIRKWOOD_EGIGA

Quoted 1 line

should address its SOC specific dependency, you have done

this in patch 3/4

Yes, I have. Do you mean you would prefer everything--file renaming, symbol renaming--in a single commit?

I feel you should create a patches as below:

standalone patches
1. kirkwood_egiga: Updates: using global data ptr for DRAM configuration and cosmetic fix
2. edbiniv2: board configuration cleanup and enhancements

patch series:
1. [1/5] net: rename: kirkwood_egiga as mv_egiga
2. [2/5] net: mv_egiga: Split SoC specific (Kirkwood) code to support other SoCs
3. [3/5] net: mv_egiga: add support for orion5x gigabit Ethernet controller
5. [4/5] net" mv_egiga: bugfix: DMA issued fixed using volatile
5. [5/5] edminiv2: add Ethernet support