[U-Boot] [PATCH 0/3 v3] New MIIPHYBB implementation with multi-bus support

[U-Boot] [PATCH 0/3 v3] New MIIPHYBB implementation with multi-bus support

[Luigi 'Comio' Mantellini]

From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>

This patch rewrites the miiphybb ( Bit-banged MII bus driver ) in order to
support an arbitrary number of buses. This feature is useful when your board
uses different mii buses for different phys and all (or a part) of these buses
are implemented via bit-banging mode.

The driver requires that the following macros should be defined into the board
configuration file:

CONFIG_BITBANGMII       - Enable the miiphybb driver
CONFIG_BITBANGMII_MULTI - Enable the multi bus support

If the CONFIG_BITBANGMII_MULTI is not defined, the board's config file needs to define
the following macros:

MII_INIT      - Generic cod to enable the MII bus (like gpios setup)
MDIO_DECLARE  - Declaration needed to access to the MDIO pin
MDIO_ACTIVE   - Activate the MDIO pin as out pin
MDIO_TRISTATE - Activate the MDIO pin as input/tristate pin
MDIO_READ     - Read the MDIO pin
MDIO(v)       - Write v on the MDIO pin
MDC_DECLARE   - Declaration needed to access to the MDC pin
MDC(v)        - Write v on the MDC pin

The previous macros make the driver compatible with the previous version
(that didn't support the multi-bus).

When the CONFIG_BITBANGMII_MULTI is defined, the board code needs to fill the
bbmiibusses[] array with a record for each required bus.
The record (struct bbmiibus) has the following fields/callbacks (see miiphy.h for
details):

char name[]            - The symbolic name that must be equal to the MII bus registered
                         name
int (*init)()          - Initialization function called at startup time (just before the
                         Ethernet initialization)
int (*mdio_active)()   - Activate the MDIO pin as output
int (*mdio_tristate)() - Activate the MDIO pin as input/tristate pin
int (*set_mdio)()      - Write the MDIO pin
int (*get_mdio)()      - Read the MDIO pin
int (*set_mdc)()       - Write the MDC pin
int (*delay)()         - Delay function
void *priv             - Private data used by board specific code

The board code will look like:

struct bbmiibus bbmiibusses[] = {
 { .name = "miibus#1", .init = b1_init, .mdio_active = b1_mdio_active, ... },
 { .name = "miibus#2", .init = b2_init, .mdio_active = b2_mdio_active, ... },
 ...
};

Patch Changelog:

v1 -- First (broken) release
v2 -- Fix some typos and disable callbacks pointers relacation (if
      CONFIG_RELOC_FIXUP_WORKS is not defined)
v3 -- Do not relocate NULL pointers

Luigi 'Comio' Mantellini (3):
  Bit-banged MII driver with multi-bus support.
  Add bb_miiphy_init call before any ethernet bring-up code.
  Update all board to support new bbmiiphy driver (with multibus
    support)

 drivers/net/phy/miiphybb.c   |  324 +++++++++++++++++++++++++++++-------------
 include/configs/ISPAN.h      |    3 +
 include/configs/MPC8260ADS.h |    2 +
 include/configs/MPC8266ADS.h |    3 +
 include/configs/MPC8560ADS.h |    3 +
 include/configs/Rattler.h    |    3 +
 include/configs/SBC8540.h    |    3 +
 include/configs/TQM8272.h    |    2 +
 include/configs/VoVPN-GW.h   |    3 +
 include/configs/ZPC1900.h    |    3 +
 include/configs/ep8248.h     |    3 +
 include/configs/ep82xxm.h    |    3 +
 include/configs/gw8260.h     |    3 +
 include/configs/hymod.h      |    9 ++
 include/configs/muas3001.h   |    3 +
 include/configs/ppmc8260.h   |    3 +
 include/configs/sacsng.h     |    3 +
 include/configs/sbc8260.h    |    3 +
 include/configs/sbc8560.h    |    3 +
 include/miiphy.h             |   22 +++
 lib_arm/board.c              |    3 +
 lib_avr32/board.c            |    3 +
 lib_blackfin/board.c         |    3 +
 lib_i386/board.c             |    3 +
 lib_m68k/board.c             |    3 +
 lib_mips/board.c             |    3 +
 lib_ppc/board.c              |    4 +-
 lib_sh/board.c               |    3 +
 lib_sparc/board.c            |    3 +
 29 files changed, 335 insertions(+), 97 deletions(-)

[U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.

[Luigi 'Comio' Mantellini]

From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>

Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
---
 drivers/net/phy/miiphybb.c |  324 +++++++++++++++++++++++++++++++-------------
 include/miiphy.h           |   22 +++
 2 files changed, 250 insertions(+), 96 deletions(-)

diff --git a/drivers/net/phy/miiphybb.c b/drivers/net/phy/miiphybb.c
index b77c917..1ed27f1 100644
--- a/drivers/net/phy/miiphybb.c
+++ b/drivers/net/phy/miiphybb.c
@@ -1,4 +1,7 @@
 /*
+ * (C) Copyright 2009 Industrie Dial Face S.p.A.
+ * Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
+ *
  * (C) Copyright 2001
  * Gerald Van Baren, Custom IDEAS, vanbaren at cideas.com.
  *
@@ -29,18 +32,137 @@
 #include <common.h>
 #include <ioports.h>
 #include <ppc_asm.tmpl>
+#include <miiphy.h>
+
+#define BBMII_RELOATE(v,off) (v += (v?off:0))
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#ifndef CONFIG_BITBANGMII_MULTI
+/*
+ * If CONFIG_BITBANGMII_MULTI is not defined we use a
+ * compatibility layer with the previous miiphybb implementation
+ * based on macros usage.
+ *
+ */
+static int bb_mii_init_wrap(struct bbmiibus *bus)
+{
+#ifdef MII_INIT
+	MII_INIT;
+#endif
+	return 0;
+}
+
+static int bb_mdio_active_wrap(struct bbmiibus *bus)
+{
+#ifdef MDIO_DECLARE
+	MDIO_DECLARE;
+#endif
+	MDIO_ACTIVE;
+	return 0;
+}
+
+static int bb_mdio_tristate_wrap(struct bbmiibus *bus)
+{
+#ifdef MDIO_DECLARE
+	MDIO_DECLARE;
+#endif
+	MDIO_TRISTATE;
+	return 0;
+}
+
+static int bb_set_mdio_wrap(struct bbmiibus *bus, int v)
+{
+#ifdef MDIO_DECLARE
+	MDIO_DECLARE;
+#endif
+	MDIO (v);
+	return 0;
+}
+
+static int bb_get_mdio_wrap(struct bbmiibus *bus, int *v)
+{
+#ifdef MDIO_DECLARE
+	MDIO_DECLARE;
+#endif
+	*v = MDIO_READ;
+	return 0;
+}
+
+static int bb_set_mdc_wrap(struct bbmiibus *bus, int v)
+{
+#ifdef MDC_DECLARE
+	MDC_DECLARE;
+#endif
+	MDC (v);
+	return 0;
+}
+
+static int bb_delay_wrap(struct bbmiibus *bus)
+{
+	MIIDELAY;
+	return 0;
+}
+
+struct bbmiibus bbmiibusses[] = {
+	{
+		.name = BB_MII_DEVNAME,
+		.init = bb_mii_init_wrap,
+		.mdio_active = bb_mdio_active_wrap,
+		.mdio_tristate = bb_mdio_tristate_wrap,
+		.set_mdio = bb_set_mdio_wrap,
+		.get_mdio = bb_get_mdio_wrap,
+		.set_mdc = bb_set_mdc_wrap,
+		.delay = bb_delay_wrap,
+	}
+};
+#endif
+
+void bb_miiphy_init(void)
+{
+	int i;
+	for (i = 0; i < sizeof(bbmiibusses)/sizeof(bbmiibusses[0]); i++) {
+#if !defined(CONFIG_RELOC_FIXUP_WORKS)
+		/* Reloate the hooks pointers*/
+		BBMII_RELOATE(bbmiibusses[i].init, gd->reloc_off);
+		BBMII_RELOATE(bbmiibusses[i].mdio_active, gd->reloc_off);
+		BBMII_RELOATE(bbmiibusses[i].mdio_tristate, gd->reloc_off);
+		BBMII_RELOATE(bbmiibusses[i].set_mdio, gd->reloc_off);
+		BBMII_RELOATE(bbmiibusses[i].get_mdio, gd->reloc_off);
+		BBMII_RELOATE(bbmiibusses[i].set_mdc, gd->reloc_off);
+		BBMII_RELOATE(bbmiibusses[i].delay, gd->reloc_off);
+#endif
+
+		if (bbmiibusses[i].init != NULL) {
+			bbmiibusses[i].init(&bbmiibusses[i]);
+		}
+	}
+}
+
+static inline struct bbmiibus *bb_miiphy_getbus(char *devname)
+{
+#ifdef CONFIG_BITBANGMII_MULTI
+	/* Search the correct bus */
+	for (j = 0; j < sizeof(bbmiibusses)/sizeof(bbmmis[0]); j++) {
+		if (!strcmp(bbmiibusses[i].name, devname)) {
+			return &bbmiibusses[i];
+		}
+	}
+	return NULL;
+#else
+	/* We have just one bitbanging bus */
+	return &bbmiibusses[0];
+#endif
+}
 
 /*****************************************************************************
  *
  * Utility to send the preamble, address, and register (common to read
  * and write).
  */
-static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
+static void miiphy_pre (struct bbmiibus *bus, char read, unsigned char addr, unsigned char reg)
 {
 	int j;			/* counter */
-#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
-	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
-#endif
 
 	/*
 	 * Send a 32 bit preamble ('1's) with an extra '1' bit for good measure.
@@ -50,67 +172,66 @@ static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
 	 * but it is safer and will be much more robust.
 	 */
 
-	MDIO_ACTIVE;
-	MDIO (1);
+	bus->mdio_active(bus);
+	bus->set_mdio (bus, 1);
 	for (j = 0; j < 32; j++) {
-		MDC (0);
-		MIIDELAY;
-		MDC (1);
-		MIIDELAY;
+		bus->set_mdc (bus, 0);
+		bus->delay(bus);
+		bus->set_mdc (bus, 1);
+		bus->delay(bus);
 	}
 
 	/* send the start bit (01) and the read opcode (10) or write (10) */
-	MDC (0);
-	MDIO (0);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
-	MDC (0);
-	MDIO (1);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
-	MDC (0);
-	MDIO (read);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
-	MDC (0);
-	MDIO (!read);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
+	bus->set_mdc (bus, 0);
+	bus->set_mdio (bus, 0);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 0);
+	bus->set_mdio (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 0);
+	bus->set_mdio (bus, read);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 0);
+	bus->set_mdio (bus, !read);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
 
 	/* send the PHY address */
 	for (j = 0; j < 5; j++) {
-		MDC (0);
+		bus->set_mdc (bus, 0);
 		if ((addr & 0x10) == 0) {
-			MDIO (0);
+			bus->set_mdio (bus, 0);
 		} else {
-			MDIO (1);
+			bus->set_mdio (bus, 1);
 		}
-		MIIDELAY;
-		MDC (1);
-		MIIDELAY;
+		bus->delay(bus);
+		bus->set_mdc (bus, 1);
+		bus->delay(bus);
 		addr <<= 1;
 	}
 
 	/* send the register address */
 	for (j = 0; j < 5; j++) {
-		MDC (0);
+		bus->set_mdc (bus, 0);
 		if ((reg & 0x10) == 0) {
-			MDIO (0);
+			bus->set_mdio (bus, 0);
 		} else {
-			MDIO (1);
+			bus->set_mdio (bus, 1);
 		}
-		MIIDELAY;
-		MDC (1);
-		MIIDELAY;
+		bus->delay(bus);
+		bus->set_mdc (bus, 1);
+		bus->delay(bus);
 		reg <<= 1;
 	}
 }
 
-
 /*****************************************************************************
  *
  * Read a MII PHY register.
@@ -122,59 +243,66 @@ int bb_miiphy_read (char *devname, unsigned char addr,
 		unsigned char reg, unsigned short *value)
 {
 	short rdreg;		/* register working value */
+	int v;
 	int j;			/* counter */
-#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
-	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
-#endif
+	struct bbmiibus *bus;
+
+	bus = bb_miiphy_getbus(devname);
+	if (bus == NULL) {
+		/* Bus not found! */
+		return -1;
+	}
 
 	if (value == NULL) {
 		puts("NULL value pointer\n");
 		return (-1);
 	}
 
-	miiphy_pre (1, addr, reg);
+	miiphy_pre (bus, 1, addr, reg);
 
 	/* tri-state our MDIO I/O pin so we can read */
-	MDC (0);
-	MDIO_TRISTATE;
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
+	bus->set_mdc (bus, 0);
+	bus->mdio_tristate(bus);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
 
 	/* check the turnaround bit: the PHY should be driving it to zero */
-	if (MDIO_READ != 0) {
+	bus->get_mdio(bus, &v);
+	if (v != 0) {
 		/* puts ("PHY didn't drive TA low\n"); */
 		for (j = 0; j < 32; j++) {
-			MDC (0);
-			MIIDELAY;
-			MDC (1);
-			MIIDELAY;
+			bus->set_mdc (bus, 0);
+			bus->delay(bus);
+			bus->set_mdc (bus, 1);
+			bus->delay(bus);
 		}
 		/* There is no PHY, set value to 0xFFFF and return */
 		*value = 0xFFFF;
 		return (-1);
 	}
 
-	MDC (0);
-	MIIDELAY;
+	bus->set_mdc (bus, 0);
+	bus->delay(bus);
 
 	/* read 16 bits of register data, MSB first */
 	rdreg = 0;
 	for (j = 0; j < 16; j++) {
-		MDC (1);
-		MIIDELAY;
+		bus->set_mdc (bus, 1);
+		bus->delay(bus);
 		rdreg <<= 1;
-		rdreg |= MDIO_READ;
-		MDC (0);
-		MIIDELAY;
+		bus->get_mdio(bus, &v);
+		rdreg |= (v & 0x1);
+		bus->set_mdc (bus, 0);
+		bus->delay(bus);
 	}
 
-	MDC (1);
-	MIIDELAY;
-	MDC (0);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 0);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
 
 	*value = rdreg;
 
@@ -196,47 +324,51 @@ int bb_miiphy_read (char *devname, unsigned char addr,
 int bb_miiphy_write (char *devname, unsigned char addr,
 		unsigned char reg, unsigned short value)
 {
+	struct bbmiibus *bus;
 	int j;			/* counter */
-#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
-	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
-#endif
 
-	miiphy_pre (0, addr, reg);
+	bus = bb_miiphy_getbus(devname);
+	if (bus == NULL) {
+		/* Bus not found! */
+		return -1;
+	}
+
+	miiphy_pre (bus, 0, addr, reg);
 
 	/* send the turnaround (10) */
-	MDC (0);
-	MDIO (1);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
-	MDC (0);
-	MDIO (0);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
+	bus->set_mdc (bus, 0);
+	bus->set_mdio (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
+	bus->set_mdc (bus, 0);
+	bus->set_mdio (bus, 0);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
 
 	/* write 16 bits of register data, MSB first */
 	for (j = 0; j < 16; j++) {
-		MDC (0);
+		bus->set_mdc (bus, 0);
 		if ((value & 0x00008000) == 0) {
-			MDIO (0);
+			bus->set_mdio (bus, 0);
 		} else {
-			MDIO (1);
+			bus->set_mdio (bus, 1);
 		}
-		MIIDELAY;
-		MDC (1);
-		MIIDELAY;
+		bus->delay(bus);
+		bus->set_mdc (bus, 1);
+		bus->delay(bus);
 		value <<= 1;
 	}
 
 	/*
 	 * Tri-state the MDIO line.
 	 */
-	MDIO_TRISTATE;
-	MDC (0);
-	MIIDELAY;
-	MDC (1);
-	MIIDELAY;
+	bus->mdio_tristate(bus);
+	bus->set_mdc (bus, 0);
+	bus->delay(bus);
+	bus->set_mdc (bus, 1);
+	bus->delay(bus);
 
 	return 0;
-}
+}
\ No newline at end of file
diff --git a/include/miiphy.h b/include/miiphy.h
index fa33ec7..478c050 100644
--- a/include/miiphy.h
+++ b/include/miiphy.h
@@ -19,6 +19,8 @@
 |
 |	COPYRIGHT   I B M   CORPORATION 1999
 |	LICENSED MATERIAL  -  PROGRAM PROPERTY OF I B M
+|
+|   Additions (C) Copyright 2009 Industrie Dial Face S.p.A.
 +----------------------------------------------------------------------------*/
 /*----------------------------------------------------------------------------+
 |
@@ -61,12 +63,32 @@ char *miiphy_get_current_dev (void);
 
 void miiphy_listdev (void);
 
+#ifdef CONFIG_BITBANGMII
+
 #define BB_MII_DEVNAME	"bbmii"
 
+struct bbmiibus {
+	char name[NAMESIZE];
+	int (*init)(struct bbmiibus *bus);
+	int (*mdio_active)(struct bbmiibus *bus);
+	int (*mdio_tristate)(struct bbmiibus *bus);
+	int (*set_mdio)(struct bbmiibus *bus, int v);
+	int (*get_mdio)(struct bbmiibus *bus, int *v);
+	int (*set_mdc)(struct bbmiibus *bus, int v);
+	int (*delay)(struct bbmiibus *bus);
+#ifdef CONFIG_BITBANGMII_MULTI
+	void *priv;
+#endif
+};
+
+extern struct bbmiibus bbmiibusses[];
+
+void bb_miiphy_init (void);
 int bb_miiphy_read (char *devname, unsigned char addr,
 		    unsigned char reg, unsigned short *value);
 int bb_miiphy_write (char *devname, unsigned char addr,
 		     unsigned char reg, unsigned short value);
+#endif
 
 /* phy seed setup */
 #define AUTO			99
-- 
1.6.3.3

[U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code.

[Luigi 'Comio' Mantellini]

From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>

Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
---
 lib_arm/board.c      |    3 +++
 lib_avr32/board.c    |    3 +++
 lib_blackfin/board.c |    3 +++
 lib_i386/board.c     |    3 +++
 lib_m68k/board.c     |    3 +++
 lib_mips/board.c     |    3 +++
 lib_ppc/board.c      |    4 +++-
 lib_sh/board.c       |    3 +++
 lib_sparc/board.c    |    3 +++
 9 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/lib_arm/board.c b/lib_arm/board.c
index a0e56d5..6e77468 100644
--- a/lib_arm/board.c
+++ b/lib_arm/board.c
@@ -417,6 +417,9 @@ extern void davinci_eth_set_mac_addr (const u_int8_t *addr);
 	mmc_initialize (gd->bd);
 #endif
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 #if defined(CONFIG_NET_MULTI)
 	puts ("Net:   ");
diff --git a/lib_avr32/board.c b/lib_avr32/board.c
index 29999d8..e715872 100644
--- a/lib_avr32/board.c
+++ b/lib_avr32/board.c
@@ -337,6 +337,9 @@ void board_init_r(gd_t *new_gd, ulong dest_addr)
 	if (s)
 		load_addr = simple_strtoul(s, NULL, 16);
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 	s = getenv("bootfile");
 	if (s)
diff --git a/lib_blackfin/board.c b/lib_blackfin/board.c
index 1053f69..f1a7479 100644
--- a/lib_blackfin/board.c
+++ b/lib_blackfin/board.c
@@ -270,6 +270,9 @@ void board_init_f(ulong bootflag)
 
 static void board_net_init_r(bd_t *bd)
 {
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #ifdef CONFIG_CMD_NET
 	uchar enetaddr[6];
 	char *s;
diff --git a/lib_i386/board.c b/lib_i386/board.c
index 0262b5e..821713a 100644
--- a/lib_i386/board.c
+++ b/lib_i386/board.c
@@ -351,6 +351,9 @@ void start_i386boot (void)
 	doc_init();
 #endif
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 #if defined(CONFIG_NET_MULTI)
 	WATCHDOG_RESET();
diff --git a/lib_m68k/board.c b/lib_m68k/board.c
index 3d88530..32ae592 100644
--- a/lib_m68k/board.c
+++ b/lib_m68k/board.c
@@ -630,6 +630,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	nand_init();		/* go init the NAND */
 #endif
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 	WATCHDOG_RESET();
 #if defined(FEC_ENET)
diff --git a/lib_mips/board.c b/lib_mips/board.c
index f62a46a..fd12e68 100644
--- a/lib_mips/board.c
+++ b/lib_mips/board.c
@@ -407,6 +407,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	misc_init_r ();
 #endif
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 #if defined(CONFIG_NET_MULTI)
 	puts ("Net:   ");
diff --git a/lib_ppc/board.c b/lib_ppc/board.c
index e8509ee..0bb159e 100644
--- a/lib_ppc/board.c
+++ b/lib_ppc/board.c
@@ -1002,6 +1002,9 @@ void board_init_r (gd_t *id, ulong dest_addr)
 	doc_init ();
 #endif
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 #if defined(CONFIG_NET_MULTI)
 	WATCHDOG_RESET ();
@@ -1009,7 +1012,6 @@ void board_init_r (gd_t *id, ulong dest_addr)
 #endif
 	eth_initialize (bd);
 #endif
-
 #if defined(CONFIG_CMD_NET) && ( \
     defined(CONFIG_CCM)		|| \
     defined(CONFIG_ELPT860)	|| \
diff --git a/lib_sh/board.c b/lib_sh/board.c
index 5d61f0d..52bbc6d 100644
--- a/lib_sh/board.c
+++ b/lib_sh/board.c
@@ -178,6 +178,9 @@ void sh_generic_init(void)
 #endif /* CONFIG_WATCHDOG*/
 
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 	{
 		char *s;
diff --git a/lib_sparc/board.c b/lib_sparc/board.c
index 6aadb56..56192a7 100644
--- a/lib_sparc/board.c
+++ b/lib_sparc/board.c
@@ -405,6 +405,9 @@ void board_init_f(ulong bootflag)
 	doc_init();
 #endif
 
+#ifdef CONFIG_BITBANGMII
+	bb_miiphy_init();
+#endif
 #if defined(CONFIG_CMD_NET)
 #if defined(CONFIG_NET_MULTI)
 	WATCHDOG_RESET();
-- 
1.6.3.3

[U-Boot] [PATCH 3/3 v3] Update all board to support new bbmiiphy driver (with multibus support)

[Luigi 'Comio' Mantellini]

From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>

Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
---
 include/configs/ISPAN.h      |    3 +++
 include/configs/MPC8260ADS.h |    2 ++
 include/configs/MPC8266ADS.h |    3 +++
 include/configs/MPC8560ADS.h |    3 +++
 include/configs/Rattler.h    |    3 +++
 include/configs/SBC8540.h    |    3 +++
 include/configs/TQM8272.h    |    2 ++
 include/configs/VoVPN-GW.h   |    3 +++
 include/configs/ZPC1900.h    |    3 +++
 include/configs/ep8248.h     |    3 +++
 include/configs/ep82xxm.h    |    3 +++
 include/configs/gw8260.h     |    3 +++
 include/configs/hymod.h      |    9 +++++++++
 include/configs/muas3001.h   |    3 +++
 include/configs/ppmc8260.h   |    3 +++
 include/configs/sacsng.h     |    3 +++
 include/configs/sbc8260.h    |    3 +++
 include/configs/sbc8560.h    |    3 +++
 18 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/include/configs/ISPAN.h b/include/configs/ISPAN.h
index 6eb466a..be41f37 100644
--- a/include/configs/ISPAN.h
+++ b/include/configs/ISPAN.h
@@ -84,6 +84,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT		3		/* Port D */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 
 #define CONFIG_SYS_MDIO_PIN		0x00040000	/* PD13 */
 #define CONFIG_SYS_MDC_PIN		0x00080000	/* PD12 */
diff --git a/include/configs/MPC8260ADS.h b/include/configs/MPC8260ADS.h
index 942a4cc..255b4aa 100644
--- a/include/configs/MPC8260ADS.h
+++ b/include/configs/MPC8260ADS.h
@@ -149,6 +149,8 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT	2		/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
 
 #if CONFIG_ADSTYPE == CONFIG_SYS_8272ADS
 #define CONFIG_SYS_MDIO_PIN	0x00002000	/* PC18 */
diff --git a/include/configs/MPC8266ADS.h b/include/configs/MPC8266ADS.h
index 4fd86d3..eb35fc4 100644
--- a/include/configs/MPC8266ADS.h
+++ b/include/configs/MPC8266ADS.h
@@ -95,6 +95,9 @@
  * Port pins used for bit-banged MII communictions (if applicable).
  */
 #define MDIO_PORT	2	/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE	(iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE	(iop->pdir &= ~0x00400000)
 #define MDIO_READ	((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/MPC8560ADS.h b/include/configs/MPC8560ADS.h
index c1a1a6d..1b9f624 100644
--- a/include/configs/MPC8560ADS.h
+++ b/include/configs/MPC8560ADS.h
@@ -362,6 +362,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT	2		/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE	(iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE	(iop->pdir &= ~0x00400000)
 #define MDIO_READ	((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/Rattler.h b/include/configs/Rattler.h
index 5b6f271..249667c 100644
--- a/include/configs/Rattler.h
+++ b/include/configs/Rattler.h
@@ -103,6 +103,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT		2	/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE		(iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE		(iop->pdir &= ~0x00400000)
 #define MDIO_READ		((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/SBC8540.h b/include/configs/SBC8540.h
index 7239f84..ede7c10 100644
--- a/include/configs/SBC8540.h
+++ b/include/configs/SBC8540.h
@@ -286,6 +286,9 @@
    * GPIO pins used for bit-banged MII communications
    */
   #define MDIO_PORT	2		/* Port C */
+  #define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+  #define MDC_DECLARE	MDIO_DECLARE
+
   #define MDIO_ACTIVE	(iop->pdir |=  0x00400000)
   #define MDIO_TRISTATE	(iop->pdir &= ~0x00400000)
   #define MDIO_READ	((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/TQM8272.h b/include/configs/TQM8272.h
index 6c462af..522e29e 100644
--- a/include/configs/TQM8272.h
+++ b/include/configs/TQM8272.h
@@ -219,6 +219,8 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT	2		/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
 
 #if STK82xx_150
 #define CONFIG_SYS_MDIO_PIN	0x00008000	/* PC16 */
diff --git a/include/configs/VoVPN-GW.h b/include/configs/VoVPN-GW.h
index b2d75e3..5c99be8 100644
--- a/include/configs/VoVPN-GW.h
+++ b/include/configs/VoVPN-GW.h
@@ -124,6 +124,9 @@
 #define CONFIG_BITBANGMII
 
 #define MDIO_PORT			1		/* Port B */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define CONFIG_SYS_MDIO_PIN			0x00002000	/* PB18 */
 #define CONFIG_SYS_MDC_PIN			0x00001000	/* PB19 */
 #define MDIO_ACTIVE			(iop->pdir |=  CONFIG_SYS_MDIO_PIN)
diff --git a/include/configs/ZPC1900.h b/include/configs/ZPC1900.h
index 9cda3f9..6c21540 100644
--- a/include/configs/ZPC1900.h
+++ b/include/configs/ZPC1900.h
@@ -86,6 +86,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT		2	/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE		(iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE		(iop->pdir &= ~0x00400000)
 #define MDIO_READ		((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/ep8248.h b/include/configs/ep8248.h
index cb4185a..113b040 100644
--- a/include/configs/ep8248.h
+++ b/include/configs/ep8248.h
@@ -92,6 +92,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT		0	/* Not used - implemented in BCSR */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE		(*(vu_char *)(CONFIG_SYS_BCSR + 8) &= 0xFB)
 #define MDIO_TRISTATE		(*(vu_char *)(CONFIG_SYS_BCSR + 8) |= 0x04)
 #define MDIO_READ		(*(vu_char *)(CONFIG_SYS_BCSR + 8) & 1)
diff --git a/include/configs/ep82xxm.h b/include/configs/ep82xxm.h
index 239ff67..29fe6d5 100644
--- a/include/configs/ep82xxm.h
+++ b/include/configs/ep82xxm.h
@@ -85,6 +85,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT		0	/* Not used - implemented in BCSR */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE		(*(vu_char *)(CONFIG_SYS_BCSR + 8) &= 0xFB)
 #define MDIO_TRISTATE		(*(vu_char *)(CONFIG_SYS_BCSR + 8) |= 0x04)
 #define MDIO_READ		(*(vu_char *)(CONFIG_SYS_BCSR + 8) & 1)
diff --git a/include/configs/gw8260.h b/include/configs/gw8260.h
index 53a001d..30a79ff 100644
--- a/include/configs/gw8260.h
+++ b/include/configs/gw8260.h
@@ -212,6 +212,9 @@
  * Port pins used for bit-banged MII communictions (if applicable).
  */
 #define MDIO_PORT   2       /* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE    (iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE  (iop->pdir &= ~0x00400000)
 #define MDIO_READ     ((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/hymod.h b/include/configs/hymod.h
index 284672b..023303a 100644
--- a/include/configs/hymod.h
+++ b/include/configs/hymod.h
@@ -93,6 +93,9 @@
 # define CONFIG_SYS_FCC_PSMR		(FCC_PSMR_FDE|FCC_PSMR_LPB)
 
 # define MDIO_PORT		0		/* Port A */
+# define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+# define MDC_DECLARE	MDIO_DECLARE
+
 # define MDIO_DATA_PINMASK	0x00040000	/* Pin 13 */
 # define MDIO_CLCK_PINMASK	0x00080000	/* Pin 12 */
 
@@ -110,6 +113,9 @@
 # define CONFIG_SYS_FCC_PSMR		(FCC_PSMR_FDE|FCC_PSMR_LPB)
 
 # define MDIO_PORT		0		/* Port A */
+# define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+# define MDC_DECLARE	MDIO_DECLARE
+
 # define MDIO_DATA_PINMASK	0x00000040	/* Pin 25 */
 # define MDIO_CLCK_PINMASK	0x00000080	/* Pin 24 */
 
@@ -127,6 +133,9 @@
 # define CONFIG_SYS_FCC_PSMR		(FCC_PSMR_FDE|FCC_PSMR_LPB)
 
 # define MDIO_PORT		0		/* Port A */
+# define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+# define MDC_DECLARE	MDIO_DECLARE
+
 # define MDIO_DATA_PINMASK	0x00000100	/* Pin 23 */
 # define MDIO_CLCK_PINMASK	0x00000200	/* Pin 22 */
 
diff --git a/include/configs/muas3001.h b/include/configs/muas3001.h
index ae033b2..3228c23 100644
--- a/include/configs/muas3001.h
+++ b/include/configs/muas3001.h
@@ -101,6 +101,9 @@
  * GPIO pins used for bit-banged MII communications
  */
 #define MDIO_PORT	0		/* Port A */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 
 #define CONFIG_SYS_MDIO_PIN	0x00200000	/* PA10 */
 #define CONFIG_SYS_MDC_PIN	0x00400000	/* PA9  */
diff --git a/include/configs/ppmc8260.h b/include/configs/ppmc8260.h
index ff7d614..3036fb7 100644
--- a/include/configs/ppmc8260.h
+++ b/include/configs/ppmc8260.h
@@ -182,6 +182,9 @@
  * Port pins used for bit-banged MII communictions (if applicable).
  */
 #define MDIO_PORT	2	/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE	(iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE	(iop->pdir &= ~0x00400000)
 #define MDIO_READ	((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/sacsng.h b/include/configs/sacsng.h
index 0ab6fc3..bfb181f 100644
--- a/include/configs/sacsng.h
+++ b/include/configs/sacsng.h
@@ -179,6 +179,9 @@
  */
 
 #define MDIO_PORT	2	        /* Port A=0, B=1, C=2, D=3 */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE	(iop->pdir |=  0x40000000)
 #define MDIO_TRISTATE	(iop->pdir &= ~0x40000000)
 #define MDIO_READ	((iop->pdat &  0x40000000) != 0)
diff --git a/include/configs/sbc8260.h b/include/configs/sbc8260.h
index 26ed557..5be1d8f 100644
--- a/include/configs/sbc8260.h
+++ b/include/configs/sbc8260.h
@@ -201,6 +201,9 @@
  * Port pins used for bit-banged MII communictions (if applicable).
  */
 #define MDIO_PORT	2	/* Port C */
+#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+#define MDC_DECLARE		MDIO_DECLARE
+
 #define MDIO_ACTIVE	(iop->pdir |=  0x00400000)
 #define MDIO_TRISTATE	(iop->pdir &= ~0x00400000)
 #define MDIO_READ	((iop->pdat &  0x00400000) != 0)
diff --git a/include/configs/sbc8560.h b/include/configs/sbc8560.h
index 4fa501d..f62d3c3 100644
--- a/include/configs/sbc8560.h
+++ b/include/configs/sbc8560.h
@@ -285,6 +285,9 @@
    * GPIO pins used for bit-banged MII communications
    */
   #define MDIO_PORT	2		/* Port C */
+  #define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
+  #define MDC_DECLARE	MDIO_DECLARE
+
   #define MDIO_ACTIVE	(iop->pdir |=  0x00400000)
   #define MDIO_TRISTATE	(iop->pdir &= ~0x00400000)
   #define MDIO_READ	((iop->pdat &  0x00400000) != 0)
-- 
1.6.3.3

[U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.

[Luigi 'Comio' Mantellini]

(autoreview)

Hi Ben,

I make some changes to the patch, but I will post after your review.
See my annotation, that will be present into the next patch release.

best regards.

Il mercoled? 23 settembre 2009 15:10:54 Luigi 'Comio' Mantellini ha scritto:
> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> 
> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> ---
>  drivers/net/phy/miiphybb.c |  324
>  +++++++++++++++++++++++++++++++------------- include/miiphy.h           | 
>   22 +++
>  2 files changed, 250 insertions(+), 96 deletions(-)
> 
> diff --git a/drivers/net/phy/miiphybb.c b/drivers/net/phy/miiphybb.c
> index b77c917..1ed27f1 100644
> --- a/drivers/net/phy/miiphybb.c
> +++ b/drivers/net/phy/miiphybb.c

...

> +
> +struct bbmiibus bbmiibusses[] = {

[COMIO] s/busses/buses - Sorry for the typo.

...

> +
> +void bb_miiphy_init(void)
> +{
> +	int i;
> +	for (i = 0; i < sizeof(bbmiibusses)/sizeof(bbmiibusses[0]); i++) {

[COMIO] Add bbmiibuses_num external variable. The board specific code need
to instance it as:

int bbmiibuses_num = sizeof(bbmiibuses)/sizeof(bbmiibuses[0])

I need this because I cannot know the array size at compile time. The other 
solution may be to use a dummy value@the array end... What do you suggest?

...
> +static inline struct bbmiibus *bb_miiphy_getbus(char *devname)
> +{
> +#ifdef CONFIG_BITBANGMII_MULTI
> +	/* Search the correct bus */
> +	for (j = 0; j < sizeof(bbmiibusses)/sizeof(bbmmis[0]); j++) {

[COMIO] Changed j to i and add i declaration.

> diff --git a/include/miiphy.h b/include/miiphy.h
> index fa33ec7..478c050 100644
> --- a/include/miiphy.h
> +++ b/include/miiphy.h
> @@ -19,6 +19,8 @@
> +
> +extern struct bbmiibus bbmiibusses[];

[COMIO] Change bbmiibusses to bbmiibuses and add the following declaration:

extern int bbmiibuses_num

Any comment is welcome

best regards,

luigi


-- 

Luigi 'Comio' Mantellini
R&D - Software

Industrie Dial Face S.p.A.
Via Canzo, 4 
20068 Peschiera Borromeo (MI) Italy
Tel.: +39 02 5167 2813
Fax: +39 02 5167 2459
E-mail: luigi.mantellini at idf-hit.com

Questo messaggio e i suoi allegati sono indirizzati esclusivamente alle 
persone indicate. La diffusione, copia o qualsiasi altra azione derivante 
dalla conoscenza di queste informazioni sono rigorosamente vietate. Qualora 
abbiate ricevuto questo documento per errore siete cortesemente pregati di 
darne immediata comunicazione al mittente e di provvedere alla sua 
distruzione, Grazie.

Rispetta l'ambiente. Non stampare questa mail se non ? realmente necessario.

This e-mail and any attachments is confidential and may contain privileged 
information intended for the addressee(s) only. Dissemination, copying, 
printing or use by anybody else is unauthorized. If you are not the intended 
recipient, please delete this message and any attachments and advise the 
sender by return e-mail, Thanks.

Help the environment. Please do not print this email unless it is absolutely 
necessary.

[U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.

[Jerry Van Baren]

Hi Luigi,

Luigi 'Comio' Mantellini wrote:
> (autoreview)
> 
> Hi Ben,
> 
> I make some changes to the patch, but I will post after your review.
> See my annotation, that will be present into the next patch release.
> 
> best regards.
> 
> Il mercoled? 23 settembre 2009 15:10:54 Luigi 'Comio' Mantellini ha scritto:
>> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>>
>> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>> ---
>>  drivers/net/phy/miiphybb.c |  324

[snip]

>> +static inline struct bbmiibus *bb_miiphy_getbus(char *devname)
>> +{
>> +#ifdef CONFIG_BITBANGMII_MULTI
>> +	/* Search the correct bus */
>> +	for (j = 0; j < sizeof(bbmiibusses)/sizeof(bbmmis[0]); j++) {
> 
> [COMIO] Changed j to i and add i declaration.

You may have inherited the "j" loop counter from me.  I stopped using 
"i" in loop counters and started with "j" because most code (variables 
and keywords) do not use words with the letter "j" or "k" in them.  On 
the other hand, A LOT of variables and keywords have "i" in their names. 
  The result is that it is a PAIN to find loop variable references for 
the loop variable "i" but it is usually trivial to find them for "j" and 
"k".

Case in point: how many "i"s does your loop have?  Just in the for() 
loop control line, I count five.

Trivia:
* I find I almost never need to nest loops more than 2 deep, so "j" and 
"k" cover almost all loop variable needs.

* I believe Fortran coding conventions started the "ijk" counter mania.

[snip]

> Any comment is welcome
> 
> best regards,
> 
> luigi

Best regards,
gvb

[U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.

[Ben Warren]

Hi Luigi,

Luigi 'Comio' Mantellini wrote:
> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>
>   
Please add some descriptive information here.  This is a big patch and 
deserves a changelong
> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> ---
>  drivers/net/phy/miiphybb.c |  324 +++++++++++++++++++++++++++++++-------------
>  include/miiphy.h           |   22 +++
>  2 files changed, 250 insertions(+), 96 deletions(-)
>
> diff --git a/drivers/net/phy/miiphybb.c b/drivers/net/phy/miiphybb.c
> index b77c917..1ed27f1 100644
> --- a/drivers/net/phy/miiphybb.c
> +++ b/drivers/net/phy/miiphybb.c
> @@ -1,4 +1,7 @@
>  /*
> + * (C) Copyright 2009 Industrie Dial Face S.p.A.
> + * Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> + *
>   * (C) Copyright 2001
>   * Gerald Van Baren, Custom IDEAS, vanbaren at cideas.com.
>   *
> @@ -29,18 +32,137 @@
>  #include <common.h>
>  #include <ioports.h>
>  #include <ppc_asm.tmpl>
> +#include <miiphy.h>
> +
> +#define BBMII_RELOATE(v,off) (v += (v?off:0))
>   
s/RELOATE/RELOCATE/

Please apply globally
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#ifndef CONFIG_BITBANGMII_MULTI
> +/*
> + * If CONFIG_BITBANGMII_MULTI is not defined we use a
> + * compatibility layer with the previous miiphybb implementation
> + * based on macros usage.
> + *
> + */
> +static int bb_mii_init_wrap(struct bbmiibus *bus)
> +{
> +#ifdef MII_INIT
> +	MII_INIT;
> +#endif
> +	return 0;
> +}
> +
> +static int bb_mdio_active_wrap(struct bbmiibus *bus)
> +{
> +#ifdef MDIO_DECLARE
> +	MDIO_DECLARE;
> +#endif
> +	MDIO_ACTIVE;
> +	return 0;
> +}
> +
> +static int bb_mdio_tristate_wrap(struct bbmiibus *bus)
> +{
> +#ifdef MDIO_DECLARE
> +	MDIO_DECLARE;
> +#endif
> +	MDIO_TRISTATE;
> +	return 0;
> +}
> +
> +static int bb_set_mdio_wrap(struct bbmiibus *bus, int v)
> +{
> +#ifdef MDIO_DECLARE
> +	MDIO_DECLARE;
> +#endif
> +	MDIO (v);
> +	return 0;
> +}
> +
> +static int bb_get_mdio_wrap(struct bbmiibus *bus, int *v)
> +{
> +#ifdef MDIO_DECLARE
> +	MDIO_DECLARE;
> +#endif
> +	*v = MDIO_READ;
> +	return 0;
> +}
> +
> +static int bb_set_mdc_wrap(struct bbmiibus *bus, int v)
> +{
> +#ifdef MDC_DECLARE
> +	MDC_DECLARE;
> +#endif
> +	MDC (v);
> +	return 0;
> +}
> +
> +static int bb_delay_wrap(struct bbmiibus *bus)
> +{
> +	MIIDELAY;
> +	return 0;
> +}
> +
> +struct bbmiibus bbmiibusses[] = {
>   
Elsewhere, you name things 'bb_mii', but here it's 'bbmii'.  I 
personally prefer adding the underscores, but please be consistent.
> +	{
> +		.name = BB_MII_DEVNAME,
> +		.init = bb_mii_init_wrap,
> +		.mdio_active = bb_mdio_active_wrap,
> +		.mdio_tristate = bb_mdio_tristate_wrap,
> +		.set_mdio = bb_set_mdio_wrap,
> +		.get_mdio = bb_get_mdio_wrap,
> +		.set_mdc = bb_set_mdc_wrap,
> +		.delay = bb_delay_wrap,
> +	}
> +};
> +#endif
> +
> +void bb_miiphy_init(void)
> +{
> +	int i;
> +	for (i = 0; i < sizeof(bbmiibusses)/sizeof(bbmiibusses[0]); i++) {
> +#if !defined(CONFIG_RELOC_FIXUP_WORKS)
> +		/* Reloate the hooks pointers*/
>   
s/Reloate/Relocate/
s/hooks/hook/
> +		BBMII_RELOATE(bbmiibusses[i].init, gd->reloc_off);
> +		BBMII_RELOATE(bbmiibusses[i].mdio_active, gd->reloc_off);
> +		BBMII_RELOATE(bbmiibusses[i].mdio_tristate, gd->reloc_off);
> +		BBMII_RELOATE(bbmiibusses[i].set_mdio, gd->reloc_off);
> +		BBMII_RELOATE(bbmiibusses[i].get_mdio, gd->reloc_off);
> +		BBMII_RELOATE(bbmiibusses[i].set_mdc, gd->reloc_off);
> +		BBMII_RELOATE(bbmiibusses[i].delay, gd->reloc_off);
> +#endif
> +
> +		if (bbmiibusses[i].init != NULL) {
> +			bbmiibusses[i].init(&bbmiibusses[i]);
> +		}
> +	}
> +}
> +
> +static inline struct bbmiibus *bb_miiphy_getbus(char *devname)
> +{
> +#ifdef CONFIG_BITBANGMII_MULTI
> +	/* Search the correct bus */
> +	for (j = 0; j < sizeof(bbmiibusses)/sizeof(bbmmis[0]); j++) {
> +		if (!strcmp(bbmiibusses[i].name, devname)) {
> +			return &bbmiibusses[i];
> +		}
> +	}
> +	return NULL;
> +#else
> +	/* We have just one bitbanging bus */
> +	return &bbmiibusses[0];
> +#endif
> +}
>  
>  /*****************************************************************************
>   *
>   * Utility to send the preamble, address, and register (common to read
>   * and write).
>   */
> -static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
> +static void miiphy_pre (struct bbmiibus *bus, char read, unsigned char addr, unsigned char reg)
>   
This line's too long (please keep < 78 characters).  Apply globally
>  {
>  	int j;			/* counter */
> -#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
> -	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
> -#endif
>  
>  	/*
>  	 * Send a 32 bit preamble ('1's) with an extra '1' bit for good measure.
> @@ -50,67 +172,66 @@ static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
>  	 * but it is safer and will be much more robust.
>  	 */
>  
> -	MDIO_ACTIVE;
> -	MDIO (1);
> +	bus->mdio_active(bus);
> +	bus->set_mdio (bus, 1);
>  	for (j = 0; j < 32; j++) {
> -		MDC (0);
> -		MIIDELAY;
> -		MDC (1);
> -		MIIDELAY;
> +		bus->set_mdc (bus, 0);
> +		bus->delay(bus);
> +		bus->set_mdc (bus, 1);
> +		bus->delay(bus);
>  	}
>  
>  	/* send the start bit (01) and the read opcode (10) or write (10) */
> -	MDC (0);
> -	MDIO (0);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> -	MDC (0);
> -	MDIO (1);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> -	MDC (0);
> -	MDIO (read);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> -	MDC (0);
> -	MDIO (!read);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> +	bus->set_mdc (bus, 0);
> +	bus->set_mdio (bus, 0);
> +	bus->delay(bus);
>   
Please be consistent with whitespace.  I prefer no space before the 
opening paren, although I think Wolfgang likes that.   He's funny that 
way.  Note that this only relates to function calls.  Control logic (if, 
for, while etc.) should always have a space before the opening paren.
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 0);
> +	bus->set_mdio (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 0);
> +	bus->set_mdio (bus, read);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 0);
> +	bus->set_mdio (bus, !read);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
>  
>  	/* send the PHY address */
>  	for (j = 0; j < 5; j++) {
> -		MDC (0);
> +		bus->set_mdc (bus, 0);
>  		if ((addr & 0x10) == 0) {
> -			MDIO (0);
> +			bus->set_mdio (bus, 0);
>  		} else {
> -			MDIO (1);
> +			bus->set_mdio (bus, 1);
>  		}
> -		MIIDELAY;
> -		MDC (1);
> -		MIIDELAY;
> +		bus->delay(bus);
> +		bus->set_mdc (bus, 1);
> +		bus->delay(bus);
>  		addr <<= 1;
>  	}
>  
>  	/* send the register address */
>  	for (j = 0; j < 5; j++) {
> -		MDC (0);
> +		bus->set_mdc (bus, 0);
>  		if ((reg & 0x10) == 0) {
> -			MDIO (0);
> +			bus->set_mdio (bus, 0);
>  		} else {
> -			MDIO (1);
> +			bus->set_mdio (bus, 1);
>  		}
> -		MIIDELAY;
> -		MDC (1);
> -		MIIDELAY;
> +		bus->delay(bus);
> +		bus->set_mdc (bus, 1);
> +		bus->delay(bus);
>  		reg <<= 1;
>  	}
>  }
>  
> -
>  /*****************************************************************************
>   *
>   * Read a MII PHY register.
> @@ -122,59 +243,66 @@ int bb_miiphy_read (char *devname, unsigned char addr,
>  		unsigned char reg, unsigned short *value)
>  {
>  	short rdreg;		/* register working value */
> +	int v;
>  	int j;			/* counter */
> -#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
> -	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
> -#endif
> +	struct bbmiibus *bus;
> +
> +	bus = bb_miiphy_getbus(devname);
> +	if (bus == NULL) {
> +		/* Bus not found! */
>   
Comment's kinda superfluous...
> +		return -1;
> +	}
>  
>  	if (value == NULL) {
>  		puts("NULL value pointer\n");
>  		return (-1);
>  	}
>  
> -	miiphy_pre (1, addr, reg);
> +	miiphy_pre (bus, 1, addr, reg);
>  
>  	/* tri-state our MDIO I/O pin so we can read */
> -	MDC (0);
> -	MDIO_TRISTATE;
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> +	bus->set_mdc (bus, 0);
> +	bus->mdio_tristate(bus);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
>  
>  	/* check the turnaround bit: the PHY should be driving it to zero */
> -	if (MDIO_READ != 0) {
> +	bus->get_mdio(bus, &v);
> +	if (v != 0) {
>  		/* puts ("PHY didn't drive TA low\n"); */
>  		for (j = 0; j < 32; j++) {
> -			MDC (0);
> -			MIIDELAY;
> -			MDC (1);
> -			MIIDELAY;
> +			bus->set_mdc (bus, 0);
> +			bus->delay(bus);
> +			bus->set_mdc (bus, 1);
> +			bus->delay(bus);
>  		}
>  		/* There is no PHY, set value to 0xFFFF and return */
>  		*value = 0xFFFF;
>  		return (-1);
>  	}
>  
> -	MDC (0);
> -	MIIDELAY;
> +	bus->set_mdc (bus, 0);
> +	bus->delay(bus);
>  
>  	/* read 16 bits of register data, MSB first */
>  	rdreg = 0;
>  	for (j = 0; j < 16; j++) {
> -		MDC (1);
> -		MIIDELAY;
> +		bus->set_mdc (bus, 1);
> +		bus->delay(bus);
>  		rdreg <<= 1;
> -		rdreg |= MDIO_READ;
> -		MDC (0);
> -		MIIDELAY;
> +		bus->get_mdio(bus, &v);
> +		rdreg |= (v & 0x1);
> +		bus->set_mdc (bus, 0);
> +		bus->delay(bus);
>  	}
>  
> -	MDC (1);
> -	MIIDELAY;
> -	MDC (0);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 0);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
>  
>  	*value = rdreg;
>  
> @@ -196,47 +324,51 @@ int bb_miiphy_read (char *devname, unsigned char addr,
>  int bb_miiphy_write (char *devname, unsigned char addr,
>  		unsigned char reg, unsigned short value)
>  {
> +	struct bbmiibus *bus;
>  	int j;			/* counter */
> -#if !(defined(CONFIG_EP8248) || defined(CONFIG_EP82XXM))
> -	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT);
> -#endif
>  
> -	miiphy_pre (0, addr, reg);
> +	bus = bb_miiphy_getbus(devname);
> +	if (bus == NULL) {
> +		/* Bus not found! */
> +		return -1;
> +	}
> +
> +	miiphy_pre (bus, 0, addr, reg);
>  
>  	/* send the turnaround (10) */
> -	MDC (0);
> -	MDIO (1);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> -	MDC (0);
> -	MDIO (0);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> +	bus->set_mdc (bus, 0);
> +	bus->set_mdio (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 0);
> +	bus->set_mdio (bus, 0);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
>  
>  	/* write 16 bits of register data, MSB first */
>  	for (j = 0; j < 16; j++) {
> -		MDC (0);
> +		bus->set_mdc (bus, 0);
>  		if ((value & 0x00008000) == 0) {
> -			MDIO (0);
> +			bus->set_mdio (bus, 0);
>  		} else {
> -			MDIO (1);
> +			bus->set_mdio (bus, 1);
>  		}
> -		MIIDELAY;
> -		MDC (1);
> -		MIIDELAY;
> +		bus->delay(bus);
> +		bus->set_mdc (bus, 1);
> +		bus->delay(bus);
>  		value <<= 1;
>  	}
>  
>  	/*
>  	 * Tri-state the MDIO line.
>  	 */
> -	MDIO_TRISTATE;
> -	MDC (0);
> -	MIIDELAY;
> -	MDC (1);
> -	MIIDELAY;
> +	bus->mdio_tristate(bus);
> +	bus->set_mdc (bus, 0);
> +	bus->delay(bus);
> +	bus->set_mdc (bus, 1);
> +	bus->delay(bus);
>  
>  	return 0;
> -}
> +}
> \ No newline at end of file
> diff --git a/include/miiphy.h b/include/miiphy.h
> index fa33ec7..478c050 100644
> --- a/include/miiphy.h
> +++ b/include/miiphy.h
> @@ -19,6 +19,8 @@
>  |
>  |	COPYRIGHT   I B M   CORPORATION 1999
>  |	LICENSED MATERIAL  -  PROGRAM PROPERTY OF I B M
> +|
> +|   Additions (C) Copyright 2009 Industrie Dial Face S.p.A.
>  +----------------------------------------------------------------------------*/
>  /*----------------------------------------------------------------------------+
>  |
> @@ -61,12 +63,32 @@ char *miiphy_get_current_dev (void);
>  
>  void miiphy_listdev (void);
>  
> +#ifdef CONFIG_BITBANGMII
> +
>  #define BB_MII_DEVNAME	"bbmii"
>  
> +struct bbmiibus {
> +	char name[NAMESIZE];
> +	int (*init)(struct bbmiibus *bus);
> +	int (*mdio_active)(struct bbmiibus *bus);
> +	int (*mdio_tristate)(struct bbmiibus *bus);
> +	int (*set_mdio)(struct bbmiibus *bus, int v);
> +	int (*get_mdio)(struct bbmiibus *bus, int *v);
> +	int (*set_mdc)(struct bbmiibus *bus, int v);
> +	int (*delay)(struct bbmiibus *bus);
> +#ifdef CONFIG_BITBANGMII_MULTI
> +	void *priv;
> +#endif
> +};
> +
> +extern struct bbmiibus bbmiibusses[];
> +
> +void bb_miiphy_init (void);
>  int bb_miiphy_read (char *devname, unsigned char addr,
>  		    unsigned char reg, unsigned short *value);
>  int bb_miiphy_write (char *devname, unsigned char addr,
>  		     unsigned char reg, unsigned short value);
> +#endif
>  
>  /* phy seed setup */
>  #define AUTO			99
>   
Thanks for this submission.  It's a really big improvement.

regards,
Ben

[U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code.

[Ben Warren]

Hi Luigi,

Luigi 'Comio' Mantellini wrote:
> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>
> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> ---
>  lib_arm/board.c      |    3 +++
>  lib_avr32/board.c    |    3 +++
>  lib_blackfin/board.c |    3 +++
>  lib_i386/board.c     |    3 +++
>  lib_m68k/board.c     |    3 +++
>  lib_mips/board.c     |    3 +++
>  lib_ppc/board.c      |    4 +++-
>  lib_sh/board.c       |    3 +++
>  lib_sparc/board.c    |    3 +++
>  9 files changed, 27 insertions(+), 1 deletions(-)
>   
I'm not sure about this.  Bit-banged MII is very board-specific, and 
IMHO should be initialized in board_eth_init()

regards,
Ben

[U-Boot] [PATCH 3/3 v3] Update all board to support new bbmiiphy driver (with multibus support)

[Ben Warren]

Hi Luigi,

Luigi 'Comio' Mantellini wrote:
> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>
> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
> ---
>  include/configs/ISPAN.h      |    3 +++
>  include/configs/MPC8260ADS.h |    2 ++
>  include/configs/MPC8266ADS.h |    3 +++
>  include/configs/MPC8560ADS.h |    3 +++
>  include/configs/Rattler.h    |    3 +++
>  include/configs/SBC8540.h    |    3 +++
>  include/configs/TQM8272.h    |    2 ++
>  include/configs/VoVPN-GW.h   |    3 +++
>  include/configs/ZPC1900.h    |    3 +++
>  include/configs/ep8248.h     |    3 +++
>  include/configs/ep82xxm.h    |    3 +++
>  include/configs/gw8260.h     |    3 +++
>  include/configs/hymod.h      |    9 +++++++++
>  include/configs/muas3001.h   |    3 +++
>  include/configs/ppmc8260.h   |    3 +++
>  include/configs/sacsng.h     |    3 +++
>  include/configs/sbc8260.h    |    3 +++
>  include/configs/sbc8560.h    |    3 +++
>  18 files changed, 58 insertions(+), 0 deletions(-)
>
> diff --git a/include/configs/ISPAN.h b/include/configs/ISPAN.h
> index 6eb466a..be41f37 100644
> --- a/include/configs/ISPAN.h
> +++ b/include/configs/ISPAN.h
> @@ -84,6 +84,9 @@
>   * GPIO pins used for bit-banged MII communications
>   */
>  #define MDIO_PORT		3		/* Port D */
> +#define MDIO_DECLARE	volatile ioport_t *iop = ioport_addr ((immap_t *) CONFIG_SYS_IMMR, MDIO_PORT)
>   
Line's too long (as are many in this patch).  Please shorten them.  The 
content seems fine...

<snip>

regards,
Ben

[U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.

[Luigi Mantellini]

Hi Ben,

thank for your review. Find my inline comments.

Thanks and best regards,

luigi

On Mon, Oct 5, 2009 at 8:27 AM, Ben Warren <biggerbadderben@gmail.com> wrote:
> Hi Luigi,
>
> Luigi 'Comio' Mantellini wrote:
>>
>> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>>
>>
>
> Please add some descriptive information here. ?This is a big patch and
> deserves a changelong
>>
>> +
>> +#define BBMII_RELOATE(v,off) (v += (v?off:0))
>>
>
> s/RELOATE/RELOCATE/
>
> Please apply globally

Ok.

>> +
>> +struct bbmiibus bbmiibusses[] = {
>>
>
> Elsewhere, you name things 'bb_mii', but here it's 'bbmii'. ?I personally
> prefer adding the underscores, but please be consistent.

What do you mean? change bbmiibus -> bb_mii_bus and bbmiibuses -> bb_mii_buses?

>>
>> + ? ? ? {
>> + ? ? ? ? ? ? ? .name = BB_MII_DEVNAME,

BB_MII_DEVNAME will be "bb_mii". is it ok?

>> + ? ? ? ? ? ? ? .init = bb_mii_init_wrap,
>> + ? ? ? ? ? ? ? .mdio_active = bb_mdio_active_wrap,
>> + ? ? ? ? ? ? ? .mdio_tristate = bb_mdio_tristate_wrap,
>> + ? ? ? ? ? ? ? .set_mdio = bb_set_mdio_wrap,
>> + ? ? ? ? ? ? ? .get_mdio = bb_get_mdio_wrap,
>> + ? ? ? ? ? ? ? .set_mdc = bb_set_mdc_wrap,
>> + ? ? ? ? ? ? ? .delay = bb_delay_wrap,
>> + ? ? ? }
>> +};

>> +#if !defined(CONFIG_RELOC_FIXUP_WORKS)
>> + ? ? ? ? ? ? ? /* Reloate the hooks pointers*/
>>
>
> s/Reloate/Relocate/
> s/hooks/hook/

ok


>> ?*/
>> -static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
>> +static void miiphy_pre (struct bbmiibus *bus, char read, unsigned char
>> addr, unsigned char reg)
>>
>
> This line's too long (please keep < 78 characters). ?Apply globally

ok

>> */
>> - ? ? ? MDC (0);
>> - ? ? ? MDIO (0);
>> - ? ? ? MIIDELAY;
...
>> + ? ? ? bus->set_mdc (bus, 0);
>> + ? ? ? bus->set_mdio (bus, 0);
>> + ? ? ? bus->delay(bus);
>>
>
> Please be consistent with whitespace. ?I prefer no space before the opening
> paren, although I think Wolfgang likes that. ? He's funny that way. ?Note
> that this only relates to function calls. ?Control logic (if, for, while
> etc.) should always have a space before the opening paren.

I just replaced MD{C,IO} with bus->set_md{c,io}.
Ok.

>> +
>> + ? ? ? bus = bb_miiphy_getbus(devname);
>> + ? ? ? if (bus == NULL) {
>> + ? ? ? ? ? ? ? /* Bus not found! */
>>
>
> Comment's kinda superfluous...

I know that the source code is always sufficiently clear regard what it does. :)
ok


>
> Thanks for this submission. ?It's a really big improvement.
>
> regards,
> Ben
>

I use GPL code on my devices... It obvious that my improvements will
be shred with community.

thanks again and best regards,

luigi


-- 
Luigi 'Comio' Mantellini
R&D - Software
Industrie Dial Face S.p.A.
Via Canzo, 4
20068 Peschiera Borromeo (MI), Italy

Tel.: +39 02 5167 2813
Fax: +39 02 5167 2459
web: www.idf-hit.com
mail: luigi.mantellini at idf-hit.com

[U-Boot] [PATCH 1/3 v3] Bit-banged MII driver with multi-bus support.

[Ben Warren]

Hi Luigi,

Luigi Mantellini wrote:
> Hi Ben,
>
> thank for your review. Find my inline comments.
>
> Thanks and best regards,
>
> luigi
>
> On Mon, Oct 5, 2009 at 8:27 AM, Ben Warren <biggerbadderben@gmail.com> wrote:
>   
>> Hi Luigi,
>>
>> Luigi 'Comio' Mantellini wrote:
>>     
>>> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>>>
>>>
>>>       
>> Please add some descriptive information here.  This is a big patch and
>> deserves a changelong
>>     
>>> +
>>> +#define BBMII_RELOATE(v,off) (v += (v?off:0))
>>>
>>>       
>> s/RELOATE/RELOCATE/
>>
>> Please apply globally
>>     
>
> Ok.
>
>   
>>> +
>>> +struct bbmiibus bbmiibusses[] = {
>>>
>>>       
>> Elsewhere, you name things 'bb_mii', but here it's 'bbmii'.  I personally
>> prefer adding the underscores, but please be consistent.
>>     
>
> What do you mean? change bbmiibus -> bb_mii_bus and bbmiibuses -> bb_mii_buses?
>
>   
Sure, that would be fine.  Consistency is the key.
>>> +       {
>>> +               .name = BB_MII_DEVNAME,
>>>       
>
> BB_MII_DEVNAME will be "bb_mii". is it ok?
>
>   
Yes.
>>> +               .init = bb_mii_init_wrap,
>>> +               .mdio_active = bb_mdio_active_wrap,
>>> +               .mdio_tristate = bb_mdio_tristate_wrap,
>>> +               .set_mdio = bb_set_mdio_wrap,
>>> +               .get_mdio = bb_get_mdio_wrap,
>>> +               .set_mdc = bb_set_mdc_wrap,
>>> +               .delay = bb_delay_wrap,
>>> +       }
>>> +};
>>>       
>
>   
>>> +#if !defined(CONFIG_RELOC_FIXUP_WORKS)
>>> +               /* Reloate the hooks pointers*/
>>>
>>>       
>> s/Reloate/Relocate/
>> s/hooks/hook/
>>     
>
> ok
>
>
>   
>>>  */
>>> -static void miiphy_pre (char read, unsigned char addr, unsigned char reg)
>>> +static void miiphy_pre (struct bbmiibus *bus, char read, unsigned char
>>> addr, unsigned char reg)
>>>
>>>       
>> This line's too long (please keep < 78 characters).  Apply globally
>>     
>
> ok
>
>   
>>> */
>>> -       MDC (0);
>>> -       MDIO (0);
>>> -       MIIDELAY;
>>>       
> ...
>   
>>> +       bus->set_mdc (bus, 0);
>>> +       bus->set_mdio (bus, 0);
>>> +       bus->delay(bus);
>>>
>>>       
>> Please be consistent with whitespace.  I prefer no space before the opening
>> paren, although I think Wolfgang likes that.   He's funny that way.  Note
>> that this only relates to function calls.  Control logic (if, for, while
>> etc.) should always have a space before the opening paren.
>>     
>
> I just replaced MD{C,IO} with bus->set_md{c,io}.
> Ok.
>
>   
I assumed that's what happened, but now's a good time to clean it up.  
Thanks.
>>> +
>>> +       bus = bb_miiphy_getbus(devname);
>>> +       if (bus == NULL) {
>>> +               /* Bus not found! */
>>>
>>>       
>> Comment's kinda superfluous...
>>     
>
> I know that the source code is always sufficiently clear regard what it does. :)
> ok
>
>
>   
>> Thanks for this submission.  It's a really big improvement.
>>
>> regards,
>> Ben
>>
>>     
>
> I use GPL code on my devices... It obvious that my improvements will
> be shred with community.
>
> thanks again and best regards,
>
> luigi
>
>   
regards,
Ben

[U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code.

[Luigi Mantellini]

The lib_[arch]/board.c code calls the bb_miiphy_init that will do just 2 things:

1) Relocate the bb_mii_buses[i].* hooks (.init, .mdio_active, .delay, ...)
2) Call the board bb_mii_buses[i].init (board specific code).

The board specifc code will be contained into the hook .init filed
into the bb_mii_buses vector.

Do you have a better solution?

thanks and best regards,

luigi

On Mon, Oct 5, 2009 at 8:30 AM, Ben Warren <biggerbadderben@gmail.com> wrote:
> Hi Luigi,
>
> Luigi 'Comio' Mantellini wrote:
>>
>> From: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>>
>> Signed-off-by: Luigi 'Comio' Mantellini <luigi.mantellini@idf-hit.com>
>> ---
>> ?lib_arm/board.c ? ? ?| ? ?3 +++
>> ?lib_avr32/board.c ? ?| ? ?3 +++
>> ?lib_blackfin/board.c | ? ?3 +++
>> ?lib_i386/board.c ? ? | ? ?3 +++
>> ?lib_m68k/board.c ? ? | ? ?3 +++
>> ?lib_mips/board.c ? ? | ? ?3 +++
>> ?lib_ppc/board.c ? ? ?| ? ?4 +++-
>> ?lib_sh/board.c ? ? ? | ? ?3 +++
>> ?lib_sparc/board.c ? ?| ? ?3 +++
>> ?9 files changed, 27 insertions(+), 1 deletions(-)
>>
>
> I'm not sure about this. ?Bit-banged MII is very board-specific, and IMHO
> should be initialized in board_eth_init()
>
> regards,
> Ben
>



-- 
Luigi 'Comio' Mantellini
R&D - Software
Industrie Dial Face S.p.A.
Via Canzo, 4
20068 Peschiera Borromeo (MI), Italy

Tel.: +39 02 5167 2813
Fax: +39 02 5167 2459
web: www.idf-hit.com
mail: luigi.mantellini at idf-hit.com

[U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code.

[Ben Warren]

Luigi Mantellini wrote:
> The lib_[arch]/board.c code calls the bb_miiphy_init that will do just 2 things:
>
> 1) Relocate the bb_mii_buses[i].* hooks (.init, .mdio_active, .delay, ...)
> 2) Call the board bb_mii_buses[i].init (board specific code).
>
> The board specifc code will be contained into the hook .init filed
> into the bb_mii_buses vector.
>
> Do you have a better solution?
>
>   
In each case, you put the bb_miiphy_init() right before 
eth_initialize().  One of the first things eth_initialize() does is call 
board_eth_init().  Moving the call to each board's specific code would 
therefore not make any difference code-wise, but IMHO is architecturally 
cleaner.  If #ifdef's are necessary (in this case they are), I believe 
it's better to have them in the more specific code (board) than generic 
(arch).

regards,
Ben

[U-Boot] [PATCH 2/3 v3] Add bb_miiphy_init call before any ethernet bring-up code.

[Luigi Mantellini]

On Mon, Oct 5, 2009 at 10:18 PM, Ben Warren <biggerbadderben@gmail.com> wrote:
...
>>
>> The board specifc code will be contained into the hook .init filed
>> into the bb_mii_buses vector.
>>
>> Do you have a better solution?
>>
>>
>
> In each case, you put the bb_miiphy_init() right before eth_initialize().
> ?One of the first things eth_initialize() does is call board_eth_init().
> ?Moving the call to each board's specific code would therefore not make any
> difference code-wise, but IMHO is architecturally cleaner. ?If #ifdef's are
> necessary (in this case they are), I believe it's better to have them in the
> more specific code (board) than generic (arch).

I agree your point of view, but there are a lot of network cards that
registers the bb_mii bus. Another fact is that the network deivces can
access to phy to complete the initialization. From these observation,
I think that the bb_mii bus should be ready before the ethernet
bring-up code.
Another solution is to modify all board specific bring-up code in
order to init the bb_mii... but this solution seems to require a lot
of trivial patches.

ciao

luigi

>
> regards,
> Ben
>



-- 
Luigi 'Comio' Mantellini
R&D - Software
Industrie Dial Face S.p.A.
Via Canzo, 4
20068 Peschiera Borromeo (MI), Italy

Tel.: +39 02 5167 2813
Fax: +39 02 5167 2459
web: www.idf-hit.com
mail: luigi.mantellini at idf-hit.com