[U-Boot] [PATCH v2 0/10] Update gdsys ppc4xx-based boards

[U-Boot] [PATCH v2 0/10] Update gdsys ppc4xx-based boards

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <dirk.eibach@gdsys.cc>


This series depends on the "Bring in new I2C framework" series by Heiko Schocher.

Changes in v2:
- fpga_state has been moved to arch_global_data
- include cmd_fpgad in iocon
- move cmd_fpgad to common and fix whitespace issues
- update email account
- use multibus soft-i2c in iocon

Dirk Eibach (10):
  powerpc/ppc4xx: Add generic accessor functions for gdsys FPGA
  powerpc/ppc4xx: Add gdsys mclink interface
  powerpc/ppc4xx: Add fpgad command for dumping gdsys fpga registers
  powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code
  powerpc/ppc4xx: Support gdsys multichannel iocon hardware
  powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex
    boards
  powerpc/ppc4xx: Fixup phy erratum on gdsys iocon hardware
  powerpc/ppc4xx: Increase timeout for gdsys mclink bus startup
  powerpc/ppc4xx: Consider gdsys FPGA OSD size
  powerpc/ppc4xx: Remove CONFIG_SYS_FLASH_PROTECTION from gdsys boards

 board/gdsys/405ep/405ep.c        |   12 +-
 board/gdsys/405ep/dlvision-10g.c |   20 ++-
 board/gdsys/405ep/io.c           |   20 ++-
 board/gdsys/405ep/iocon.c        |  467 ++++++++++++++++++++++++++++++++++----
 board/gdsys/405ep/neo.c          |   17 +-
 board/gdsys/405ex/405ex.c        |   14 +-
 board/gdsys/405ex/io64.c         |   38 ++--
 board/gdsys/common/Makefile      |    2 +-
 board/gdsys/common/mclink.c      |  142 ++++++++++++
 board/gdsys/common/mclink.h      |   31 +++
 board/gdsys/common/osd.c         |  101 +++++----
 common/Makefile                  |    1 +
 common/cmd_fpgad.c               |   99 ++++++++
 include/configs/dlvision-10g.h   |    3 +-
 include/configs/dlvision.h       |    3 +-
 include/configs/io.h             |    3 +-
 include/configs/iocon.h          |   50 ++++-
 include/configs/neo.h            |    3 +-
 include/gdsys_fpga.h             |   20 ++-
 19 files changed, 892 insertions(+), 154 deletions(-)
 create mode 100644 board/gdsys/common/mclink.c
 create mode 100644 board/gdsys/common/mclink.h
 create mode 100644 common/cmd_fpgad.c

-- 
1.7.2.5

[U-Boot] [PATCH v2 01/10] powerpc/ppc4xx: Add generic accessor functions for gdsys FPGA

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

A set of accessor functions was added to be able to access not only
memory mapped FPGA in a generic way.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---
Changes in v2: None

 include/gdsys_fpga.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/gdsys_fpga.h b/include/gdsys_fpga.h
index 1758d74..c12a31d 100644
--- a/include/gdsys_fpga.h
+++ b/include/gdsys_fpga.h
@@ -32,9 +32,14 @@ enum {
 	FPGA_STATE_PLATFORM = 1 << 2,
 };
 
+#define REG(reg) offsetof(struct ihs_fpga, reg)
+
 int get_fpga_state(unsigned dev);
 void print_fpga_state(unsigned dev);
 
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data);
+u16 fpga_get_reg(unsigned int fpga, u16 reg);
+
 struct ihs_gpio {
 	u16 read;
 	u16 clear;
-- 
1.7.2.5

[U-Boot] [PATCH v2 01/10] powerpc/ppc4xx: Add generic accessor functions for gdsys FPGA

[Wolfgang Denk]

Dear dirk.eibach at gdsys.cc,

In message <1367847325-21463-2-git-send-email-dirk.eibach@gdsys.cc> you wrote:
> From: Dirk Eibach <eibach@gdsys.de>
> 
> A set of accessor functions was added to be able to access not only
> memory mapped FPGA in a generic way.
> 
> Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
> ---
> Changes in v2: None
> 
>  include/gdsys_fpga.h |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

This patch makes no sense.  Please squash it with the patch that
actually uses the new definition.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
If the odds are a million to one against something occuring,  chances
are 50-50 it will.

[U-Boot] [PATCH v2 01/10] powerpc/ppc4xx: Add generic accessor functions for gdsys FPGA

[Wolfgang Denk]

Dear dirk.eibach at gdsys.cc,

In message <1367847325-21463-2-git-send-email-dirk.eibach@gdsys.cc> you wrote:
> From: Dirk Eibach <eibach@gdsys.de>
> 
> A set of accessor functions was added to be able to access not only
> memory mapped FPGA in a generic way.

Please squash with patch that actually uses this define.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Love all, trust a few.                          - William Shakespeare

[U-Boot] [PATCH v2 02/10] powerpc/ppc4xx: Add gdsys mclink interface

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

mclink is a serial interface for communication between gdsys FPGA.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>

---
Changes in v2:
- update email account

 board/gdsys/common/Makefile |    2 +-
 board/gdsys/common/mclink.c |  140 +++++++++++++++++++++++++++++++++++++++++++
 board/gdsys/common/mclink.h |   31 ++++++++++
 3 files changed, 172 insertions(+), 1 deletions(-)
 create mode 100644 board/gdsys/common/mclink.c
 create mode 100644 board/gdsys/common/mclink.h

diff --git a/board/gdsys/common/Makefile b/board/gdsys/common/Makefile
index 05dd65d..6f09550 100644
--- a/board/gdsys/common/Makefile
+++ b/board/gdsys/common/Makefile
@@ -31,7 +31,7 @@ LIB	= $(obj)lib$(VENDOR).o
 
 COBJS-$(CONFIG_IO) += miiphybb.o
 COBJS-$(CONFIG_IO64) += miiphybb.o
-COBJS-$(CONFIG_IOCON) += osd.o
+COBJS-$(CONFIG_IOCON) += osd.o mclink.o
 COBJS-$(CONFIG_DLVISION_10G) += osd.o
 
 COBJS   := $(COBJS-y)
diff --git a/board/gdsys/common/mclink.c b/board/gdsys/common/mclink.c
new file mode 100644
index 0000000..9a4f3e9
--- /dev/null
+++ b/board/gdsys/common/mclink.c
@@ -0,0 +1,140 @@
+/*
+ * (C) Copyright 2012
+ * Dirk Eibach,  Guntermann & Drunck GmbH, dirk.eibach at gdsys.cc
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <errno.h>
+
+#include <gdsys_fpga.h>
+
+enum {
+	MCINT_SLAVE_LINK_CHANGED_EV = 1 << 7,
+	MCINT_TX_ERROR_EV = 1 << 9,
+	MCINT_TX_BUFFER_FREE = 1 << 10,
+	MCINT_TX_PACKET_TRANSMITTED_EV = 1 << 11,
+	MCINT_RX_ERROR_EV = 1 << 13,
+	MCINT_RX_CONTENT_AVAILABLE = 1 << 14,
+	MCINT_RX_PACKET_RECEIVED_EV = 1 << 15,
+};
+
+int mclink_probe(void)
+{
+	unsigned int k;
+	unsigned int ctr = 0;
+	int slaves = 0;
+
+	for (k = 0; k < CONFIG_SYS_MCLINK_MAX; ++k) {
+		int timeout = 0;
+
+		if (!(fpga_get_reg(k, REG(mc_status)) & (1 << 15)))
+			break;
+
+		fpga_set_reg(k, REG(mc_control), 0x8000);
+
+		while (!(fpga_get_reg(k, REG(mc_status)) & (1 << 14))) {
+			udelay(100);
+			if (ctr++ > 3) {
+				timeout = 1;
+				break;
+			}
+		}
+		if (timeout)
+			break;
+
+		slaves++;
+	}
+
+	return slaves;
+}
+
+int mclink_send(u8 slave, u16 addr, u16 data)
+{
+	unsigned int ctr = 0;
+	u16 int_status;
+	u16 rx_cmd_status;
+	u16 rx_cmd;
+
+	/* reset interrupt status */
+	int_status = fpga_get_reg(0, REG(mc_int));
+	fpga_set_reg(0, REG(mc_int), int_status);
+
+	/* send */
+	fpga_set_reg(0, REG(mc_tx_address), addr);
+	fpga_set_reg(0, REG(mc_tx_data), data);
+	fpga_set_reg(0, REG(mc_tx_cmd), (slave & 0x03) << 14);
+	fpga_set_reg(0, REG(mc_control), 0x8001);
+
+	/* wait for reply */
+	while (!(fpga_get_reg(0, REG(mc_int)) & MCINT_RX_PACKET_RECEIVED_EV)) {
+		udelay(100);
+		if (ctr++ > 3)
+			return -ETIMEDOUT;
+	}
+
+	rx_cmd_status = fpga_get_reg(0, REG(mc_rx_cmd_status));
+	rx_cmd = (rx_cmd_status >> 12) & 0x03;
+	if (rx_cmd != 0)
+		printf("mclink_send: received cmd %d, expected %d\n", rx_cmd,
+		       0);
+
+	return 0;
+}
+
+int mclink_receive(u8 slave, u16 addr, u16 *data)
+{
+	u16 rx_cmd_status;
+	u16 rx_cmd;
+	unsigned int ctr = 0;
+
+	/* send read request */
+	fpga_set_reg(0, REG(mc_tx_address), addr);
+	fpga_set_reg(0, REG(mc_tx_cmd),
+		     ((slave & 0x03) << 14) | (1 << 12) | (1 << 0));
+	fpga_set_reg(0, REG(mc_control), 0x8001);
+
+	/* wait for reply */
+	while (!(fpga_get_reg(0, REG(mc_int)) & MCINT_RX_CONTENT_AVAILABLE)) {
+		udelay(100);
+		if (ctr++ > 3)
+			return -ETIMEDOUT;
+	}
+
+	/* check reply */
+	rx_cmd_status = fpga_get_reg(0, REG(mc_rx_cmd_status));
+	if ((rx_cmd_status >> 14) != slave) {
+		printf("mclink_receive: reply from slave %d, expected %d\n",
+		       rx_cmd_status >> 14, slave);
+		return -EINVAL;
+	}
+
+	rx_cmd = (rx_cmd_status >> 12) & 0x03;
+	if (rx_cmd != 1) {
+		printf("mclink_send: received cmd %d, expected %d\n",
+		       rx_cmd, 1);
+		return -EIO;
+	}
+
+	*data = fpga_get_reg(0, REG(mc_rx_data));
+
+	return 0;
+}
diff --git a/board/gdsys/common/mclink.h b/board/gdsys/common/mclink.h
new file mode 100644
index 0000000..47a62b4
--- /dev/null
+++ b/board/gdsys/common/mclink.h
@@ -0,0 +1,31 @@
+/*
+ * (C) Copyright 2012
+ * Dirk Eibach,  Guntermann & Drunck GmbH, dirk.eibach at gdsys.cc
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _MCLINK_H_
+#define _MCLINK_H_
+
+int mclink_probe(void);
+int mclink_send(u8 slave, u16 addr, u16 data);
+int mclink_receive(u8 slave, u16 addr, u16 *data);
+
+#endif
-- 
1.7.2.5

[U-Boot] [PATCH v2 03/10] powerpc/ppc4xx: Add fpgad command for dumping gdsys fpga registers

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>

---
Changes in v2:
- move cmd_fpgad to common and fix whitespace issues
- update email account

 common/Makefile    |    1 +
 common/cmd_fpgad.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 100 insertions(+), 0 deletions(-)
 create mode 100644 common/cmd_fpgad.c

diff --git a/common/Makefile b/common/Makefile
index 0e0fff1..edabbad 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -110,6 +110,7 @@ COBJS-$(CONFIG_CMD_FLASH) += cmd_flash.o
 ifdef CONFIG_FPGA
 COBJS-$(CONFIG_CMD_FPGA) += cmd_fpga.o
 endif
+COBJS-$(CONFIG_CMD_FPGAD) += cmd_fpgad.o
 COBJS-$(CONFIG_CMD_FS_GENERIC) += cmd_fs.o
 COBJS-$(CONFIG_CMD_GETTIME) += cmd_gettime.o
 COBJS-$(CONFIG_CMD_GPIO) += cmd_gpio.o
diff --git a/common/cmd_fpgad.c b/common/cmd_fpgad.c
new file mode 100644
index 0000000..0100f83
--- /dev/null
+++ b/common/cmd_fpgad.c
@@ -0,0 +1,99 @@
+/*
+ * (C) Copyright 2013
+ * Dirk Eibach,  Guntermann & Drunck GmbH, dirk.eibach at gdsys.cc
+ *
+ * based on cmd_mem.c
+ * (C) Copyright 2000
+ * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <command.h>
+
+#include <gdsys_fpga.h>
+
+static uint	dp_last_fpga;
+static uint	dp_last_addr;
+static uint	dp_last_length = 0x40;
+
+/*
+ * FPGA Memory Display
+ *
+ * Syntax:
+ *	fpgad {fpga} {addr} {len}
+ */
+#define DISP_LINE_LEN	16
+int do_fpga_md(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+	unsigned int k;
+	unsigned int fpga;
+	ulong	addr, length;
+	int rc = 0;
+	u16	linebuf[DISP_LINE_LEN/sizeof(u16)];
+
+	/*
+	 * We use the last specified parameters, unless new ones are
+	 * entered.
+	 */
+	fpga = dp_last_fpga;
+	addr = dp_last_addr;
+	length = dp_last_length;
+
+	if (argc < 3)
+		return CMD_RET_USAGE;
+
+	if ((flag & CMD_FLAG_REPEAT) == 0) {
+		/*
+		 * FPGA is specified since argc > 2
+		 */
+		fpga = simple_strtoul(argv[1], NULL, 16);
+
+		/*
+		 * Address is specified since argc > 2
+		 */
+		addr = simple_strtoul(argv[2], NULL, 16);
+
+		/*
+		 * If another parameter, it is the length to display.
+		 * Length is the number of objects, not number of bytes.
+		 */
+		if (argc > 3)
+			length = simple_strtoul(argv[3], NULL, 16);
+	}
+
+	/* Print the lines. */
+	for (k = 0; k < DISP_LINE_LEN / sizeof(u16); ++k)
+		linebuf[k] = fpga_get_reg(fpga, addr + k * sizeof(u16));
+	print_buffer(addr, (void *)linebuf, sizeof(u16),
+		     length, DISP_LINE_LEN / sizeof(u16));
+	addr += sizeof(u16)*length;
+
+	dp_last_fpga = fpga;
+	dp_last_addr = addr;
+	dp_last_length = length;
+	return rc;
+}
+
+U_BOOT_CMD(
+	fpgad,	4,	1,	do_fpga_md,
+	"fpga register display",
+	"fpga address [# of objects]"
+);
-- 
1.7.2.5

[U-Boot] [PATCH v2 04/10] powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---
Changes in v2: None

 board/gdsys/405ep/405ep.c |   12 +++----
 board/gdsys/common/osd.c  |   70 +++++++++++++++++++++-----------------------
 2 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/board/gdsys/405ep/405ep.c b/board/gdsys/405ep/405ep.c
index 6221171..e5fbd2e 100644
--- a/board/gdsys/405ep/405ep.c
+++ b/board/gdsys/405ep/405ep.c
@@ -106,22 +106,20 @@ int board_early_init_r(void)
 	gd405ep_set_fpga_reset(0);
 
 	for (k = 0; k < CONFIG_SYS_FPGA_COUNT; ++k) {
-		struct ihs_fpga *fpga =
-			(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(k);
 #ifdef CONFIG_SYS_FPGA_NO_RFL_HI
-		u16 *reflection_target = &fpga->reflection_low;
+		u16 reflection_target = REG(reflection_low);
 #else
-		u16 *reflection_target = &fpga->reflection_high;
+		u16 reflection_target = REG(reflection_high);
 #endif
 		/*
 		 * wait for fpga out of reset
 		 */
 		ctr = 0;
 		while (1) {
-			out_le16(&fpga->reflection_low,
-				REFLECTION_TESTPATTERN);
+			fpga_set_reg(k, REG(reflection_low),
+				     REFLECTION_TESTPATTERN);
 
-			if (in_le16(reflection_target) ==
+			if (fpga_get_reg(k, reflection_target) ==
 				REFLECTION_TESTPATTERN_INV)
 				break;
 
diff --git a/board/gdsys/common/osd.c b/board/gdsys/common/osd.c
index a192c98..81c8136 100644
--- a/board/gdsys/common/osd.c
+++ b/board/gdsys/common/osd.c
@@ -22,7 +22,7 @@
  */
 
 #include <common.h>
-#include <i2c.h>
+#include "bb_i2c.h"
 #include <asm/io.h>
 
 #include <gdsys_fpga.h>
@@ -67,37 +67,34 @@ enum {
 	CH7301_DSP = 0x56,		/* DVI Sync polarity Register */
 };
 
+unsigned int max_osd_screen = CONFIG_SYS_OSD_SCREENS - 1;
+
 #if defined(CONFIG_SYS_ICS8N3QV01) || defined(CONFIG_SYS_SIL1178)
 static void fpga_iic_write(unsigned screen, u8 slave, u8 reg, u8 data)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(screen);
-	struct ihs_i2c *i2c = &fpga->i2c;
-
-	while (in_le16(&fpga->extended_interrupt) & (1 << 12))
+	while (fpga_get_reg(screen, REG(extended_interrupt)) & (1 << 12))
 		;
-	out_le16(&i2c->write_mailbox_ext, reg | (data << 8));
-	out_le16(&i2c->write_mailbox, 0xc400 | (slave << 1));
+	fpga_set_reg(screen, REG(i2c.write_mailbox_ext), reg | (data << 8));
+	fpga_set_reg(screen, REG(i2c.write_mailbox), 0xc400 | (slave << 1));
 }
 
 static u8 fpga_iic_read(unsigned screen, u8 slave, u8 reg)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(screen);
-	struct ihs_i2c *i2c = &fpga->i2c;
 	unsigned int ctr = 0;
 
-	while (in_le16(&fpga->extended_interrupt) & (1 << 12))
+	while (fpga_get_reg(screen, REG(extended_interrupt)) & (1 << 12))
 		;
-	out_le16(&fpga->extended_interrupt, 1 << 14);
-	out_le16(&i2c->write_mailbox_ext, reg);
-	out_le16(&i2c->write_mailbox, 0xc000 | (slave << 1));
-	while (!(in_le16(&fpga->extended_interrupt) & (1 << 14))) {
+	fpga_set_reg(screen, REG(extended_interrupt), 1 << 14);
+	fpga_set_reg(screen, REG(i2c.write_mailbox_ext), reg);
+	fpga_set_reg(screen, REG(i2c.write_mailbox), 0xc000 | (slave << 1));
+	while (!(fpga_get_reg(screen, REG(extended_interrupt)) & (1 << 14))) {
 		udelay(100000);
 		if (ctr++ > 5) {
 			printf("iic receive timeout\n");
 			break;
 		}
 	}
-	return in_le16(&i2c->read_mailbox_ext) >> 8;
+	return fpga_get_reg(screen, REG(i2c.read_mailbox_ext)) >> 8;
 }
 #endif
 
@@ -129,7 +126,6 @@ static void mpc92469ac_calc_parameters(unsigned int fout,
 
 static void mpc92469ac_set(unsigned screen, unsigned int fout)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(screen);
 	unsigned int n;
 	unsigned int m;
 	unsigned int bitval = 0;
@@ -150,7 +146,7 @@ static void mpc92469ac_set(unsigned screen, unsigned int fout)
 		break;
 	}
 
-	out_le16(&fpga->mpc3w_control, (bitval << 9) | m);
+	fpga_set_reg(screen, REG(mpc3w_control), (bitval << 9) | m);
 }
 #endif
 
@@ -265,14 +261,13 @@ static void ics8n3qv01_set(unsigned screen, unsigned int fout)
 static int osd_write_videomem(unsigned screen, unsigned offset,
 	u16 *data, size_t charcount)
 {
-	struct ihs_fpga *fpga =
-		(struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(screen);
 	unsigned int k;
 
 	for (k = 0; k < charcount; ++k) {
 		if (offset + k >= BUFSIZE)
 			return -1;
-		out_le16(&fpga->videomem + offset + k, data[k]);
+		fpga_set_reg(screen, REG(videomem) + sizeof(u16) * (offset + k),
+			     data[k]);
 	}
 
 	return charcount;
@@ -282,7 +277,7 @@ static int osd_print(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	unsigned screen;
 
-	for (screen = 0; screen < CONFIG_SYS_OSD_SCREENS; ++screen) {
+	for (screen = 0; screen <= max_osd_screen; ++screen) {
 		unsigned x;
 		unsigned y;
 		unsigned charcount;
@@ -318,10 +313,8 @@ static int osd_print(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 int osd_probe(unsigned screen)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(screen);
-	struct ihs_osd *osd = &fpga->osd;
-	u16 version = in_le16(&osd->version);
-	u16 features = in_le16(&osd->features);
+	u16 version = fpga_get_reg(screen, REG(osd.version));
+	u16 features = fpga_get_reg(screen, REG(osd.features));
 	unsigned width;
 	unsigned height;
 	u8 value;
@@ -333,16 +326,16 @@ int osd_probe(unsigned screen)
 		screen, version/100, version%100, width, height);
 
 #ifdef CONFIG_SYS_CH7301
-	value = i2c_reg_read(CH7301_I2C_ADDR, CH7301_DID);
+	value = bb_i2c_reg_read(screen, CH7301_I2C_ADDR, CH7301_DID);
 	if (value != 0x17) {
 		printf("       Probing CH7301 failed, DID %02x\n", value);
 		return -1;
 	}
-	i2c_reg_write(CH7301_I2C_ADDR, CH7301_TPCP, 0x08);
-	i2c_reg_write(CH7301_I2C_ADDR, CH7301_TPD, 0x16);
-	i2c_reg_write(CH7301_I2C_ADDR, CH7301_TPF, 0x60);
-	i2c_reg_write(CH7301_I2C_ADDR, CH7301_DC, 0x09);
-	i2c_reg_write(CH7301_I2C_ADDR, CH7301_PM, 0xc0);
+	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_TPCP, 0x08);
+	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_TPD, 0x16);
+	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_TPF, 0x60);
+	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_DC, 0x09);
+	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_PM, 0xc0);
 #endif
 
 #ifdef CONFIG_SYS_MPC92469AC
@@ -372,12 +365,15 @@ int osd_probe(unsigned screen)
 	fpga_iic_write(screen, SIL1178_MASTER_I2C_ADDRESS, 0x08, 0x37);
 #endif
 
-	out_le16(&fpga->videocontrol, 0x0002);
-	out_le16(&osd->control, 0x0049);
+	fpga_set_reg(screen, REG(videocontrol), 0x0002);
+	fpga_set_reg(screen, REG(osd.control), 0x0049);
+
+	fpga_set_reg(screen, REG(osd.xy_size), ((32 - 1) << 8) | (16 - 1));
+	fpga_set_reg(screen, REG(osd.x_pos), 0x007f);
+	fpga_set_reg(screen, REG(osd.y_pos), 0x005f);
 
-	out_le16(&osd->xy_size, ((32 - 1) << 8) | (16 - 1));
-	out_le16(&osd->x_pos, 0x007f);
-	out_le16(&osd->y_pos, 0x005f);
+	if (screen > max_osd_screen)
+		max_osd_screen = screen;
 
 	return 0;
 }
@@ -386,7 +382,7 @@ int osd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 {
 	unsigned screen;
 
-	for (screen = 0; screen < CONFIG_SYS_OSD_SCREENS; ++screen) {
+	for (screen = 0; screen <= max_osd_screen; ++screen) {
 		unsigned x;
 		unsigned y;
 		unsigned k;
-- 
1.7.2.5

[U-Boot] [PATCH v2 04/10] powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code

[Wolfgang Denk]

Dear dirk.eibach at gdsys.cc,

In message <1367847325-21463-5-git-send-email-dirk.eibach@gdsys.cc> you wrote:
>
...
> +			fpga_set_reg(k, REG(reflection_low),
> +				     REFLECTION_TESTPATTERN);

This breaks bisecting.  You reference functions which you only add in
a later patch.

Please fix this.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
My play was a complete success.  The audience was a failure.

[U-Boot] [PATCH v2 04/10] powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code

[Tom Rini]

On Mon, May 06, 2013 at 04:07:15PM +0200, Wolfgang Denk wrote:
> Dear dirk.eibach at gdsys.cc,
> 
> In message <1367847325-21463-5-git-send-email-dirk.eibach@gdsys.cc> you wrote:
> >
> ...
> > +			fpga_set_reg(k, REG(reflection_low),
> > +				     REFLECTION_TESTPATTERN);
> 
> This breaks bisecting.  You reference functions which you only add in
> a later patch.

Note that 'git rebase -i' and the exec keyword can help you test
bisectability.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130506/1b99c788/attachment.pgp>

[U-Boot] [PATCH v2 04/10] powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code

[Dirk Eibach]

Hi Tom,

> On Mon, May 06, 2013 at 04:07:15PM +0200, Wolfgang Denk wrote:
>> Dear dirk.eibach at gdsys.cc,
>> 
>> In message <1367847325-21463-5-git-send-email-dirk.eibach@gdsys.cc> 
>> you wrote:
>>> 
>> ...
>>> +			fpga_set_reg(k, REG(reflection_low),
>>> +				     REFLECTION_TESTPATTERN);
>> 
>> This breaks bisecting.  You reference functions which you only add in
>> a later patch.
> 
> Note that 'git rebase -i' and the exec keyword can help you test
> bisectability.

I know 'git rebase -i', but what is the exec keyword? It seems there is 
some room for improvement on patman :)

Cheers
Dirk

[U-Boot] [PATCH v2 04/10] powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code

[Tom Rini]

On Mon, May 06, 2013 at 05:02:12PM +0200, Dirk Eibach wrote:
> Hi Tom,
> 
> >On Mon, May 06, 2013 at 04:07:15PM +0200, Wolfgang Denk wrote:
> >>Dear dirk.eibach at gdsys.cc,
> >>
> >>In message
> >><1367847325-21463-5-git-send-email-dirk.eibach@gdsys.cc> you
> >>wrote:
> >>>
> >>...
> >>>+			fpga_set_reg(k, REG(reflection_low),
> >>>+				     REFLECTION_TESTPATTERN);
> >>
> >>This breaks bisecting.  You reference functions which you only add in
> >>a later patch.
> >
> >Note that 'git rebase -i' and the exec keyword can help you test
> >bisectability.
> 
> I know 'git rebase -i', but what is the exec keyword? It seems there
> is some room for improvement on patman :)

Well, it's a rebase command.  In addition to
pick/edit/reword/squash/fixup you can do 'exec make sandbox', or more
likely some script that builds a bunch of things and returns non-zero on
failure.  Git will then run that command at that point in the rebase
(say, pick the first commit, exec) and if the exec fails, you get
dropped to a command prompt and can edit the previous commit, etc.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20130506/49a88421/attachment.pgp>

[U-Boot] [PATCH v2 04/10] powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code

[Dirk Eibach]

Hi Tom,

> On Mon, May 06, 2013 at 05:02:12PM +0200, Dirk Eibach wrote:
>> Hi Tom,
>> 
>>> On Mon, May 06, 2013 at 04:07:15PM +0200, Wolfgang Denk wrote:
>>>> Dear dirk.eibach at gdsys.cc,
>>>> 
>>>> In message
>>>> <1367847325-21463-5-git-send-email-dirk.eibach@gdsys.cc> you
>>>> wrote:
>>>>> 
>>>> ...
>>>>> +			fpga_set_reg(k, REG(reflection_low),
>>>>> +				     REFLECTION_TESTPATTERN);
>>>> 
>>>> This breaks bisecting.  You reference functions which you only add 
>>>> in
>>>> a later patch.
>>> 
>>> Note that 'git rebase -i' and the exec keyword can help you test
>>> bisectability.
>> 
>> I know 'git rebase -i', but what is the exec keyword? It seems there
>> is some room for improvement on patman :)
> 
> Well, it's a rebase command.  In addition to
> pick/edit/reword/squash/fixup you can do 'exec make sandbox', or more
> likely some script that builds a bunch of things and returns non-zero 
> on
> failure.  Git will then run that command at that point in the rebase
> (say, pick the first commit, exec) and if the exec fails, you get
> dropped to a command prompt and can edit the previous commit, etc.

Wow, that's a very nice tool. I will pick it into my workflow.

Cheers
Dirk

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>

---
Changes in v2:
- fpga_state has been moved to arch_global_data
- include cmd_fpgad in iocon
- use multibus soft-i2c in iocon

 board/gdsys/405ep/iocon.c |  253 ++++++++++++++++++++++++++++++++++++++-------
 board/gdsys/common/osd.c  |   25 +++--
 include/configs/iocon.h   |   44 ++++++--
 include/gdsys_fpga.h      |   15 +++-
 4 files changed, 280 insertions(+), 57 deletions(-)

diff --git a/board/gdsys/405ep/iocon.c b/board/gdsys/405ep/iocon.c
index 0fc7db2..0ab8772 100644
--- a/board/gdsys/405ep/iocon.c
+++ b/board/gdsys/405ep/iocon.c
@@ -23,6 +23,7 @@
 
 #include <common.h>
 #include <command.h>
+#include <errno.h>
 #include <asm/processor.h>
 #include <asm/io.h>
 #include <asm/ppc4xx-gpio.h>
@@ -31,6 +32,13 @@
 #include <gdsys_fpga.h>
 
 #include "../common/osd.h"
+#include "../common/mclink.h"
+
+#include <i2c.h>
+#include <pca953x.h>
+#include <pca9698.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 #define LATCH0_BASE (CONFIG_SYS_LATCH_BASE)
 #define LATCH1_BASE (CONFIG_SYS_LATCH_BASE + 0x100)
@@ -47,11 +55,20 @@ enum {
 	HWVER_100 = 0,
 	HWVER_104 = 1,
 	HWVER_110 = 2,
+	HWVER_120 = 3,
+	HWVER_200 = 4,
+	HWVER_210 = 5,
+};
+
+enum {
+	FPGA_HWVER_200 = 0,
+	FPGA_HWVER_210 = 1,
 };
 
 enum {
 	COMPRESSION_NONE = 0,
-	COMPRESSION_TYPE1_DELTA,
+	COMPRESSION_TYPE1_DELTA = 1,
+	COMPRESSION_TYPE1_TYPE2_DELTA = 3,
 };
 
 enum {
@@ -67,8 +84,56 @@ enum {
 
 enum {
 	RAM_DDR2_32 = 0,
+	RAM_DDR3_32 = 1,
+};
+
+enum {
+	MCFPGA_DONE = 1 << 0,
+	MCFPGA_INIT_N = 1 << 1,
+	MCFPGA_PROGRAM_N = 1 << 2,
+	MCFPGA_UPDATE_ENABLE_N = 1 << 3,
+	MCFPGA_RESET_N = 1 << 4,
 };
 
+unsigned int mclink_fpgacount;
+
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+	int res;
+	struct ihs_fpga *fpga_0 = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0);
+
+	switch (fpga) {
+	case 0:
+		out_le16((u16 *)fpga_0 + reg / sizeof(u16), data);
+		break;
+	default:
+		res = mclink_send(fpga - 1, reg, data);
+		if (res < 0)
+			printf("mclink_send reg %02x data %04x returned %d\n",
+			       reg, data, res);
+		break;
+	}
+}
+
+u16 fpga_get_reg(unsigned int fpga, u16 reg)
+{
+	int res;
+	u16 data;
+	struct ihs_fpga *fpga_0 = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0);
+
+	switch (fpga) {
+	case 0:
+		return in_le16((u16 *)fpga_0 + reg / sizeof(u16));
+	default:
+		if (fpga > mclink_fpgacount)
+			return -EINVAL;
+		res = mclink_receive(fpga - 1, reg, &data);
+		if (res < 0)
+			printf("mclink_receive returned %d\n", res);
+		return data;
+	}
+}
+
 /*
  * Check Board Identity:
  */
@@ -90,12 +155,11 @@ int checkboard(void)
 	return 0;
 }
 
-static void print_fpga_info(void)
+static void print_fpga_info(unsigned int fpga)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(0);
-	u16 versions = in_le16(&fpga->versions);
-	u16 fpga_version = in_le16(&fpga->fpga_version);
-	u16 fpga_features = in_le16(&fpga->fpga_features);
+	u16 versions = fpga_get_reg(fpga, REG(versions));
+	u16 fpga_version = fpga_get_reg(fpga, REG(fpga_version));
+	u16 fpga_features = fpga_get_reg(fpga, REG(fpga_features));
 	unsigned unit_type;
 	unsigned hardware_version;
 	unsigned feature_compression;
@@ -105,9 +169,9 @@ static void print_fpga_info(void)
 	unsigned feature_ramconfig;
 	unsigned feature_carriers;
 	unsigned feature_video_channels;
+	int legacy = get_fpga_state(0) & FPGA_STATE_PLATFORM;
 
 	unit_type = (versions & 0xf000) >> 12;
-	hardware_version = versions & 0x000f;
 	feature_compression = (fpga_features & 0xe000) >> 13;
 	feature_osd = fpga_features & (1<<11);
 	feature_audio = (fpga_features & 0x0600) >> 9;
@@ -116,6 +180,9 @@ static void print_fpga_info(void)
 	feature_carriers = (fpga_features & 0x000c) >> 2;
 	feature_video_channels = fpga_features & 0x0003;
 
+	if (legacy)
+		printf("legacy ");
+
 	switch (unit_type) {
 	case UNITTYPE_MAIN_USER:
 		printf("Mainchannel");
@@ -130,27 +197,68 @@ static void print_fpga_info(void)
 		break;
 	}
 
-	switch (hardware_version) {
-	case HWVER_100:
-		printf(" HW-Ver 1.00\n");
-		break;
-
-	case HWVER_104:
-		printf(" HW-Ver 1.04\n");
-		break;
-
-	case HWVER_110:
-		printf(" HW-Ver 1.10\n");
-		break;
+	if (unit_type == UNITTYPE_MAIN_USER) {
+		if (legacy)
+			hardware_version =
+				(in_le16((void *)LATCH2_BASE)>>8) & 0x0f;
+		else
+			hardware_version =
+				  (!!pca9698_get_value(0x20, 24) << 0)
+				| (!!pca9698_get_value(0x20, 25) << 1)
+				| (!!pca9698_get_value(0x20, 26) << 2)
+				| (!!pca9698_get_value(0x20, 27) << 3);
+		switch (hardware_version) {
+		case HWVER_100:
+			printf(" HW-Ver 1.00,");
+			break;
+
+		case HWVER_104:
+			printf(" HW-Ver 1.04,");
+			break;
+
+		case HWVER_110:
+			printf(" HW-Ver 1.10,");
+			break;
+
+		case HWVER_120:
+			printf(" HW-Ver 1.20-1.21,");
+			break;
+
+		case HWVER_200:
+			printf(" HW-Ver 2.00,");
+			break;
+
+		case HWVER_210:
+			printf(" HW-Ver 2.10,");
+			break;
+
+		default:
+			printf(" HW-Ver %d(not supported),",
+			       hardware_version);
+			break;
+		}
+	}
 
-	default:
-		printf(" HW-Ver %d(not supported)\n",
-		       hardware_version);
-		break;
+	if (unit_type == UNITTYPE_VIDEO_USER) {
+		hardware_version = versions & 0x000f;
+		switch (hardware_version) {
+		case FPGA_HWVER_200:
+			printf(" HW-Ver 2.00,");
+			break;
+
+		case FPGA_HWVER_210:
+			printf(" HW-Ver 2.10,");
+			break;
+
+		default:
+			printf(" HW-Ver %d(not supported),",
+			       hardware_version);
+			break;
+		}
 	}
 
-	printf("       FPGA V %d.%02d, features:",
-		fpga_version / 100, fpga_version % 100);
+	printf(" FPGA V %d.%02d\n       features:",
+	       fpga_version / 100, fpga_version % 100);
 
 
 	switch (feature_compression) {
@@ -162,6 +270,10 @@ static void print_fpga_info(void)
 		printf(" type1-deltacompression");
 		break;
 
+	case COMPRESSION_TYPE1_TYPE2_DELTA:
+		printf(" type1-deltacompression, type2-inlinecompression");
+		break;
+
 	default:
 		printf(" compression %d(not supported)", feature_compression);
 		break;
@@ -208,6 +320,10 @@ static void print_fpga_info(void)
 		printf(", RAM 32 bit DDR2");
 		break;
 
+	case RAM_DDR3_32:
+		printf(", RAM 32 bit DDR3");
+		break;
+
 	default:
 		printf(", RAM %d(not supported)", feature_ramconfig);
 		break;
@@ -220,41 +336,94 @@ static void print_fpga_info(void)
 
 int last_stage_init(void)
 {
-	print_fpga_info();
+	int slaves;
+	unsigned int k;
+	unsigned char mclink_controllers[] = { 0x24, 0x25, 0x26 };
+
+	print_fpga_info(0);
+	osd_probe(0);
+
+	/* wait for FPGA done */
+	for (k = 0; k < ARRAY_SIZE(mclink_controllers); ++k) {
+		unsigned int ctr = 0;
+
+		if (i2c_probe(mclink_controllers[k]))
+			continue;
+
+		while (!(pca953x_get_val(mclink_controllers[k])
+		       & MCFPGA_DONE)) {
+			udelay(100000);
+			if (ctr++ > 5) {
+				printf("no done for mclink_controller %d\n", k);
+				break;
+			}
+		}
+	}
+
+	/* wait for slave-PLLs to be up and running */
+	udelay(500000);
+
+	mclink_fpgacount = CONFIG_SYS_MCLINK_MAX;
+	slaves = mclink_probe();
+	mclink_fpgacount = 0;
+
+	if (slaves <= 0)
+		return 0;
+
+	mclink_fpgacount = slaves;
+
+	for (k = 1; k <= slaves; ++k) {
+		print_fpga_info(k);
+		osd_probe(k);
+	}
 
-	return osd_probe(0);
+	return 0;
 }
 
 /*
  * provide access to fpga gpios (for I2C bitbang)
  */
-void fpga_gpio_set(int pin)
+void fpga_gpio_set(unsigned int bus, int pin)
 {
-	out_le16((void *)(CONFIG_SYS_FPGA0_BASE + 0x18), pin);
+	fpga_set_reg(bus, REG(gpio.set), pin);
 }
 
-void fpga_gpio_clear(int pin)
+void fpga_gpio_clear(unsigned int bus, int pin)
 {
-	out_le16((void *)(CONFIG_SYS_FPGA0_BASE + 0x16), pin);
+	fpga_set_reg(bus, REG(gpio.clear), pin);
 }
 
-int fpga_gpio_get(int pin)
+int fpga_gpio_get(unsigned int bus, int pin)
 {
-	return in_le16((void *)(CONFIG_SYS_FPGA0_BASE + 0x14)) & pin;
+	return fpga_get_reg(bus, REG(gpio.read)) & pin;
 }
 
 void gd405ep_init(void)
 {
+	unsigned int k;
+
+	if (i2c_probe(0x20)) { /* i2c_probe returns 0 on success */
+		for (k = 0; k < CONFIG_SYS_FPGA_COUNT; ++k)
+			gd->arch.fpga_state[k] |= FPGA_STATE_PLATFORM;
+	} else {
+		pca9698_direction_output(0x20, 4, 1);
+	}
 }
 
 void gd405ep_set_fpga_reset(unsigned state)
 {
-	if (state) {
-		out_le16((void *)LATCH0_BASE, CONFIG_SYS_LATCH0_RESET);
-		out_le16((void *)LATCH1_BASE, CONFIG_SYS_LATCH1_RESET);
+	int legacy = get_fpga_state(0) & FPGA_STATE_PLATFORM;
+
+	if (legacy) {
+		if (state) {
+			out_le16((void *)LATCH0_BASE, CONFIG_SYS_LATCH0_RESET);
+			out_le16((void *)LATCH1_BASE, CONFIG_SYS_LATCH1_RESET);
+		} else {
+			out_le16((void *)LATCH0_BASE, CONFIG_SYS_LATCH0_BOOT);
+			out_le16((void *)LATCH1_BASE, CONFIG_SYS_LATCH1_BOOT);
+		}
 	} else {
-		out_le16((void *)LATCH0_BASE, CONFIG_SYS_LATCH0_BOOT);
-		out_le16((void *)LATCH1_BASE, CONFIG_SYS_LATCH1_BOOT);
+		pca9698_set_value(0x20, 4, state ? 0 : 1);
 	}
 }
 
@@ -269,5 +438,11 @@ void gd405ep_setup_hw(void)
 
 int gd405ep_get_fpga_done(unsigned fpga)
 {
-	return in_le16((void *)LATCH2_BASE) & CONFIG_SYS_FPGA_DONE(fpga);
+	int legacy = get_fpga_state(0) & FPGA_STATE_PLATFORM;
+
+	if (legacy)
+		return in_le16((void *)LATCH2_BASE)
+		       & CONFIG_SYS_FPGA_DONE(fpga);
+	else
+		return pca9698_get_value(0x20, 20);
 }
diff --git a/board/gdsys/common/osd.c b/board/gdsys/common/osd.c
index 81c8136..2cfbe58 100644
--- a/board/gdsys/common/osd.c
+++ b/board/gdsys/common/osd.c
@@ -22,7 +22,8 @@
  */
 
 #include <common.h>
-#include "bb_i2c.h"
+#include <i2c.h>
+#include <malloc.h>
 #include <asm/io.h>
 
 #include <gdsys_fpga.h>
@@ -69,6 +70,10 @@ enum {
 
 unsigned int max_osd_screen = CONFIG_SYS_OSD_SCREENS - 1;
 
+#ifdef CONFIG_SYS_CH7301
+int ch7301_i2c[] = CONFIG_SYS_CH7301_I2C;
+#endif
+
 #if defined(CONFIG_SYS_ICS8N3QV01) || defined(CONFIG_SYS_SIL1178)
 static void fpga_iic_write(unsigned screen, u8 slave, u8 reg, u8 data)
 {
@@ -318,6 +323,9 @@ int osd_probe(unsigned screen)
 	unsigned width;
 	unsigned height;
 	u8 value;
+#ifdef CONFIG_SYS_CH7301
+	int old_bus = i2c_get_bus_num();
+#endif
 
 	width = ((features & 0x3f00) >> 8) + 1;
 	height = (features & 0x001f) + 1;
@@ -326,16 +334,19 @@ int osd_probe(unsigned screen)
 		screen, version/100, version%100, width, height);
 
 #ifdef CONFIG_SYS_CH7301
-	value = bb_i2c_reg_read(screen, CH7301_I2C_ADDR, CH7301_DID);
+	i2c_set_bus_num(ch7301_i2c[screen]);
+	value = i2c_reg_read(CH7301_I2C_ADDR, CH7301_DID);
 	if (value != 0x17) {
 		printf("       Probing CH7301 failed, DID %02x\n", value);
+		i2c_set_bus_num(old_bus);
 		return -1;
 	}
-	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_TPCP, 0x08);
-	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_TPD, 0x16);
-	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_TPF, 0x60);
-	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_DC, 0x09);
-	bb_i2c_reg_write(screen, CH7301_I2C_ADDR, CH7301_PM, 0xc0);
+	i2c_reg_write(CH7301_I2C_ADDR, CH7301_TPCP, 0x08);
+	i2c_reg_write(CH7301_I2C_ADDR, CH7301_TPD, 0x16);
+	i2c_reg_write(CH7301_I2C_ADDR, CH7301_TPF, 0x60);
+	i2c_reg_write(CH7301_I2C_ADDR, CH7301_DC, 0x09);
+	i2c_reg_write(CH7301_I2C_ADDR, CH7301_PM, 0xc0);
+	i2c_set_bus_num(old_bus);
 #endif
 
 #ifdef CONFIG_SYS_MPC92469AC
diff --git a/include/configs/iocon.h b/include/configs/iocon.h
index 5972711..921ac05 100644
--- a/include/configs/iocon.h
+++ b/include/configs/iocon.h
@@ -79,6 +79,7 @@
  * Commands additional to the ones defined in amcc-common.h
  */
 #define CONFIG_CMD_CACHE
+#define CONFIG_CMD_FPGAD
 #undef CONFIG_CMD_EEPROM
 
 /*
@@ -116,23 +117,46 @@
 #define CONFIG_SYS_I2C_PPC4XX_SPEED_0		400000
 #define CONFIG_SYS_I2C_PPC4XX_SLAVE_0		0x7F
 
+#define CONFIG_SYS_I2C_SPEED		400000
+
+#define CONFIG_PCA953X			/* NXP PCA9554 */
+#define CONFIG_PCA9698			/* NXP PCA9698 */
+
 /*
  * Software (bit-bang) I2C driver configuration
  */
+#define CONFIG_SYS_I2C_SOFT
+#define CONFIG_SYS_I2C_SOFT_SPEED		50000
+#define CONFIG_SYS_I2C_SOFT_SLAVE		0x7F
+#define I2C_SOFT_DECLARATIONS2
+#define CONFIG_SYS_I2C_SOFT_SPEED_2		50000
+#define CONFIG_SYS_I2C_SOFT_SLAVE_2		0x7F
+#define I2C_SOFT_DECLARATIONS3
+#define CONFIG_SYS_I2C_SOFT_SPEED_3		50000
+#define CONFIG_SYS_I2C_SOFT_SLAVE_3		0x7F
+#define I2C_SOFT_DECLARATIONS4
+#define CONFIG_SYS_I2C_SOFT_SPEED_4		50000
+#define CONFIG_SYS_I2C_SOFT_SLAVE_4		0x7F
+
+#define CONFIG_SYS_CH7301_I2C			{1, 2, 3, 4}
 
 #ifndef __ASSEMBLY__
-void fpga_gpio_set(int pin);
-void fpga_gpio_clear(int pin);
-int fpga_gpio_get(int pin);
+void fpga_gpio_set(unsigned int bus, int pin);
+void fpga_gpio_clear(unsigned int bus, int pin);
+int fpga_gpio_get(unsigned int bus, int pin);
 #endif
 
 #define I2C_ACTIVE	{ }
 #define I2C_TRISTATE	{ }
-#define I2C_READ	fpga_gpio_get(0x0040) ? 1 : 0
-#define I2C_SDA(bit)	if (bit) fpga_gpio_set(0x0040); \
-			else fpga_gpio_clear(0x0040)
-#define I2C_SCL(bit)	if (bit) fpga_gpio_set(0x0020); \
-			else fpga_gpio_clear(0x0020)
+#define I2C_READ	fpga_gpio_get(I2C_ADAP_HWNR, 0x0040) ? 1 : 0
+#define I2C_SDA(bit)	if (bit) \
+					fpga_gpio_set(I2C_ADAP_HWNR, 0x0040); \
+				else \
+					fpga_gpio_clear(I2C_ADAP_HWNR, 0x0040)
+#define I2C_SCL(bit)	if (bit) \
+					 fpga_gpio_set(I2C_ADAP_HWNR, 0x0020); \
+				else \
+					fpga_gpio_clear(I2C_ADAP_HWNR, 0x0020)
 #define I2C_DELAY	udelay(25)	/* 1/4 I2C clock duration */
 
 /*
@@ -252,6 +276,8 @@ int fpga_gpio_get(int pin);
 
 #define CONFIG_SYS_FPGA_COUNT		1
 
+#define CONFIG_SYS_MCLINK_MAX		3
+
 /* Memory Bank 3 (Latches) initialization */
 #define CONFIG_SYS_LATCH_BASE		0x7f200000
 #define CONFIG_SYS_EBC_PB3AP		0x02025080
@@ -267,6 +293,6 @@ int fpga_gpio_get(int pin);
  */
 #define CONFIG_SYS_MPC92469AC
 #define CONFIG_SYS_CH7301
-#define CONFIG_SYS_OSD_SCREENS		CONFIG_SYS_FPGA_COUNT
+#define CONFIG_SYS_OSD_SCREENS		1
 
 #endif	/* __CONFIG_H */
diff --git a/include/gdsys_fpga.h b/include/gdsys_fpga.h
index c12a31d..b246f2f 100644
--- a/include/gdsys_fpga.h
+++ b/include/gdsys_fpga.h
@@ -117,10 +117,21 @@ struct ihs_fpga {
 	u16 mpc3w_control;	/* 0x001a */
 	u16 reserved_1[19];	/* 0x001c */
 	u16 videocontrol;	/* 0x0042 */
-	u16 reserved_2[93];	/* 0x0044 */
+	u16 reserved_2[14];	/* 0x0044 */
+	u16 mc_int;		/* 0x0060 */
+	u16 mc_int_en;		/* 0x0062 */
+	u16 mc_status;		/* 0x0064 */
+	u16 mc_control;		/* 0x0066 */
+	u16 mc_tx_data;		/* 0x0068 */
+	u16 mc_tx_address;	/* 0x006a */
+	u16 mc_tx_cmd;		/* 0x006c */
+	u16 mc_res;		/* 0x006e */
+	u16 mc_rx_cmd_status;	/* 0x0070 */
+	u16 mc_rx_data;		/* 0x0072 */
+	u16 reserved_3[69];	/* 0x0074 */
 	u16 reflection_high;	/* 0x00fe */
 	struct ihs_osd osd;	/* 0x0100 */
-	u16 reserved_3[889];	/* 0x010e */
+	u16 reserved_4[889];	/* 0x010e */
 	u16 videomem;		/* 0x0800 */
 };
 #endif
-- 
1.7.2.5

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[Wolfgang Denk]

Dear dirk.eibach at gdsys.cc,

In message <1367847325-21463-6-git-send-email-dirk.eibach@gdsys.cc> you wrote:
> From: Dirk Eibach <eibach@gdsys.de>
> 
> Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
> 
> ---
> Changes in v2:
> - fpga_state has been moved to arch_global_data
> - include cmd_fpgad in iocon
> - use multibus soft-i2c in iocon

Please fix the checkpatch errors.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Ada is PL/I trying to be Smalltalk.                 - Codoso diBlini

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[Dirk Eibach]

Dear Wolfgang,

> Dear dirk.eibach at gdsys.cc,
> 
> In message <1367847325-21463-6-git-send-email-dirk.eibach@gdsys.cc> 
> you wrote:
>> From: Dirk Eibach <eibach@gdsys.de>
>> 
>> Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
>> 
>> ---
>> Changes in v2:
>> - fpga_state has been moved to arch_global_data
>> - include cmd_fpgad in iocon
>> - use multibus soft-i2c in iocon
> 
> Please fix the checkpatch errors.

if we are talking about:

CHECK: Blank lines aren't necessary before a close brace '}'
#491: FILE: include/configs/iocon.h:141:
+
+#define CONFIG_SYS_CH7301_I2C			{1, 2, 3, 4}


This seems to be a false positive.

Cheers
Dirk

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[Wolfgang Denk]

Dear Dirk Eibach,

In message <49d89b84e14cf375d9e113fcc15ce289@gdsys.cc> you wrote:
> 
> > Please fix the checkpatch errors.
> 
> if we are talking about:
> 
> CHECK: Blank lines aren't necessary before a close brace '}'
> #491: FILE: include/configs/iocon.h:141:
> +
> +#define CONFIG_SYS_CH7301_I2C			{1, 2, 3, 4}
> 
> 
> This seems to be a false positive.

No, I'm referring to these:

ERROR: Macros with complex values should be enclosed in parenthesis
#595: FILE: include/configs/iocon.h:151:
+#define I2C_READ	fpga_gpio_get(I2C_ADAP_HWNR, 0x0040) ? 1 : 0

ERROR: Macros with multiple statements should be enclosed in a do - while loop
#596: FILE: include/configs/iocon.h:152:
+#define I2C_SDA(bit)	if (bit) \
+					fpga_gpio_set(I2C_ADAP_HWNR, 0x0040); \
+				else \
+					fpga_gpio_clear(I2C_ADAP_HWNR, 0x0040)

ERROR: Macros with multiple statements should be enclosed in a do - while loop
#600: FILE: include/configs/iocon.h:156:
+#define I2C_SCL(bit)	if (bit) \
+					 fpga_gpio_set(I2C_ADAP_HWNR, 0x0020); \
+				else \
+					fpga_gpio_clear(I2C_ADAP_HWNR, 0x0020)

total: 3 errors, 0 warnings, 509 lines checked


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Conceptual integrity in turn dictates that the  design  must  proceed
from  one  mind,  or  from  a  very small number of agreeing resonant
minds.               - Frederick Brooks Jr., "The Mythical Man Month" 

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[Dirk Eibach]

Hi Wolfgang,

> Dear Dirk Eibach,
> 
> In message <49d89b84e14cf375d9e113fcc15ce289@gdsys.cc> you wrote:
>> 
>>> Please fix the checkpatch errors.
>> 
>> if we are talking about:
>> 
>> CHECK: Blank lines aren't necessary before a close brace '}'
>> #491: FILE: include/configs/iocon.h:141:
>> +
>> +#define CONFIG_SYS_CH7301_I2C			{1, 2, 3, 4}
>> 
>> 
>> This seems to be a false positive.
> 
> No, I'm referring to these:
> 
> ERROR: Macros with complex values should be enclosed in parenthesis
> #595: FILE: include/configs/iocon.h:151:
> +#define I2C_READ	fpga_gpio_get(I2C_ADAP_HWNR, 0x0040) ? 1 : 0
> 
> ERROR: Macros with multiple statements should be enclosed in a do - 
> while loop
> #596: FILE: include/configs/iocon.h:152:
> +#define I2C_SDA(bit)	if (bit) \
> +					fpga_gpio_set(I2C_ADAP_HWNR, 0x0040); \
> +				else \
> +					fpga_gpio_clear(I2C_ADAP_HWNR, 0x0040)
> 
> ERROR: Macros with multiple statements should be enclosed in a do - 
> while loop
> #600: FILE: include/configs/iocon.h:156:
> +#define I2C_SCL(bit)	if (bit) \
> +					 fpga_gpio_set(I2C_ADAP_HWNR, 0x0020); \
> +				else \
> +					fpga_gpio_clear(I2C_ADAP_HWNR, 0x0020)
> 
> total: 3 errors, 0 warnings, 509 lines checked

OK, thanks, that makes sense. Which version of checkpatch are you 
using? Latest u-boot master checkpatch/patman does not show these (at 
least not for me).

Cheers
Dirk

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[Wolfgang Denk]

Dear Dirk,

In message <3628fded6de7e6b4286d3444a7e5e3ec@gdsys.cc> you wrote:
> 
> > total: 3 errors, 0 warnings, 509 lines checked
> 
> OK, thanks, that makes sense. Which version of checkpatch are you 
> using? Latest u-boot master checkpatch/patman does not show these (at 
> least not for me).

It does for me, interestingly ONLY if the current directory is NOT in
the U-Boot tree.

Apparently both the COMPLEX_MACRO and
MULTISTATEMENT_MACRO_USE_DO_WHILE option should better be enabled, or
we will miss such issues.



Added Joe to Cc: - Joe, what do you think?  I'd rather remove the
silencing for these from .checkpatch.conf ?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
An optimist believes we live in the best world possible; a  pessimist
fears this is true.

[U-Boot] [PATCH v2 05/10] powerpc/ppc4xx: Support gdsys multichannel iocon hardware

[Joe Hershberger]

Hi Wolfgang,

On Mon, May 6, 2013 at 2:15 PM, Wolfgang Denk <wd@denx.de> wrote:
> Dear Dirk,
>
> In message <3628fded6de7e6b4286d3444a7e5e3ec@gdsys.cc> you wrote:
>>
>> > total: 3 errors, 0 warnings, 509 lines checked
>>
>> OK, thanks, that makes sense. Which version of checkpatch are you
>> using? Latest u-boot master checkpatch/patman does not show these (at
>> least not for me).
>
> It does for me, interestingly ONLY if the current directory is NOT in
> the U-Boot tree.
>
> Apparently both the COMPLEX_MACRO and
> MULTISTATEMENT_MACRO_USE_DO_WHILE option should better be enabled, or
> we will miss such issues.
>
>
>
> Added Joe to Cc: - Joe, what do you think?  I'd rather remove the
> silencing for these from .checkpatch.conf ?

Back in October 2011 checkpatch was generating false positives on
COMPLEX_MACRO.  If this has been fixed and copied from Linux, then we
can certainly stop ignoring it.  I reported the issue at that time.

As the .checkpatch.conf comments say, CONFIG_SYS_I2C_NOPROBES
definitions were causing false positives with
MULTISTATEMENT_MACRO_USE_DO_WHILE.  I agree that we can remove
ignoring MULTISTATEMENT_MACRO_USE_DO_WHILE as long as the false
positives are allowed when identified.

Cheers,
-Joe

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---
Changes in v2: None

 board/gdsys/405ep/dlvision-10g.c |   20 ++++++++++++++------
 board/gdsys/405ep/io.c           |   20 ++++++++++++++------
 board/gdsys/405ep/neo.c          |   17 +++++++++++++----
 board/gdsys/405ex/405ex.c        |   14 ++++++--------
 board/gdsys/405ex/io64.c         |   38 ++++++++++++++++++++++----------------
 5 files changed, 69 insertions(+), 40 deletions(-)

diff --git a/board/gdsys/405ep/dlvision-10g.c b/board/gdsys/405ep/dlvision-10g.c
index 644493b..4d1a02e 100644
--- a/board/gdsys/405ep/dlvision-10g.c
+++ b/board/gdsys/405ep/dlvision-10g.c
@@ -71,6 +71,16 @@ enum {
 	RAM_DDR2_64 = 2,
 };
 
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+	out_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16), data);
+}
+
+u16 fpga_get_reg(unsigned int fpga, u16 reg)
+{
+	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16));
+}
+
 int misc_init_r(void)
 {
 	/* startup fans */
@@ -95,10 +105,9 @@ static unsigned int get_mc2_present(void)
 
 static void print_fpga_info(unsigned dev)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(dev);
-	u16 versions = in_le16(&fpga->versions);
-	u16 fpga_version = in_le16(&fpga->fpga_version);
-	u16 fpga_features = in_le16(&fpga->fpga_features);
+	u16 versions = fpga_get_reg(dev, REG(versions));
+	u16 fpga_version = fpga_get_reg(dev, REG(fpga_version));
+	u16 fpga_features = fpga_get_reg(dev, REG(fpga_features));
 	unsigned unit_type;
 	unsigned hardware_version;
 	unsigned feature_rs232;
@@ -263,8 +272,7 @@ int checkboard(void)
 
 int last_stage_init(void)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(0);
-	u16 versions = in_le16(&fpga->versions);
+	u16 versions = fpga_get_reg(0, REG(versions));
 
 	print_fpga_info(0);
 	if (get_mc2_present())
diff --git a/board/gdsys/405ep/io.c b/board/gdsys/405ep/io.c
index 070dcbb..dbaf9d6 100644
--- a/board/gdsys/405ep/io.c
+++ b/board/gdsys/405ep/io.c
@@ -53,6 +53,16 @@ enum {
 	HWVER_122 = 3,
 };
 
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+	out_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16), data);
+}
+
+u16 fpga_get_reg(unsigned int fpga, u16 reg)
+{
+	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16));
+}
+
 int misc_init_r(void)
 {
 	/* startup fans */
@@ -117,10 +127,9 @@ int checkboard(void)
 
 static void print_fpga_info(void)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(0);
-	u16 versions = in_le16(&fpga->versions);
-	u16 fpga_version = in_le16(&fpga->fpga_version);
-	u16 fpga_features = in_le16(&fpga->fpga_features);
+	u16 versions = fpga_get_reg(0, REG(versions));
+	u16 fpga_version = fpga_get_reg(0, REG(fpga_version));
+	u16 fpga_features = fpga_get_reg(0, REG(fpga_features));
 	unsigned unit_type;
 	unsigned hardware_version;
 	unsigned feature_channels;
@@ -179,7 +188,6 @@ static void print_fpga_info(void)
  */
 int last_stage_init(void)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(0);
 	unsigned int k;
 
 	print_fpga_info();
@@ -191,7 +199,7 @@ int last_stage_init(void)
 		configure_gbit_phy(k);
 
 	/* take fpga serdes blocks out of reset */
-	out_le16(&fpga->quad_serdes_reset, 0);
+	fpga_set_reg(0, REG(quad_serdes_reset), 0);
 
 	return 0;
 }
diff --git a/board/gdsys/405ep/neo.c b/board/gdsys/405ep/neo.c
index d336e55..f06a280 100644
--- a/board/gdsys/405ep/neo.c
+++ b/board/gdsys/405ep/neo.c
@@ -44,6 +44,16 @@ enum {
 	HWVER_300 = 3,
 };
 
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+	out_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16), data);
+}
+
+u16 fpga_get_reg(unsigned int fpga, u16 reg)
+{
+	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16));
+}
+
 int misc_init_r(void)
 {
 	/* startup fans */
@@ -70,10 +80,9 @@ int checkboard(void)
 
 static void print_fpga_info(void)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(0);
-	u16 versions = in_le16(&fpga->versions);
-	u16 fpga_version = in_le16(&fpga->fpga_version);
-	u16 fpga_features = in_le16(&fpga->fpga_features);
+	u16 versions = fpga_get_reg(0, REG(versions));
+	u16 fpga_version = fpga_get_reg(0, REG(fpga_version));
+	u16 fpga_features = fpga_get_reg(0, REG(fpga_features));
 	int fpga_state = get_fpga_state(0);
 	unsigned unit_type;
 	unsigned hardware_version;
diff --git a/board/gdsys/405ex/405ex.c b/board/gdsys/405ex/405ex.c
index 32e24c0..b7e9802 100644
--- a/board/gdsys/405ex/405ex.c
+++ b/board/gdsys/405ex/405ex.c
@@ -220,23 +220,21 @@ int board_early_init_r(void)
 	gd405ex_set_fpga_reset(0);
 
 	for (k = 0; k < CONFIG_SYS_FPGA_COUNT; ++k) {
-		struct ihs_fpga *fpga =
-			(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(k);
 #ifdef CONFIG_SYS_FPGA_NO_RFL_HI
-		u16 *reflection_target = &fpga->reflection_low;
+		u16 reflection_target = REG(reflection_low);
 #else
-		u16 *reflection_target = &fpga->reflection_high;
+		u16 reflection_target = REG(reflection_high);
 #endif
 		/*
 		 * wait for fpga out of reset
 		 */
 		ctr = 0;
 		while (1) {
-			out_le16(&fpga->reflection_low,
-				REFLECTION_TESTPATTERN);
+			fpga_set_reg(k, REG(reflection_low),
+				     REFLECTION_TESTPATTERN);
 
-			if (in_le16(reflection_target) ==
-				REFLECTION_TESTPATTERN_INV)
+			if (fpga_get_reg(k, reflection_target) ==
+			    REFLECTION_TESTPATTERN_INV)
 				break;
 
 			udelay(100000);
diff --git a/board/gdsys/405ex/io64.c b/board/gdsys/405ex/io64.c
index 7d2899d..f0f4241 100644
--- a/board/gdsys/405ex/io64.c
+++ b/board/gdsys/405ex/io64.c
@@ -67,6 +67,16 @@ enum {
 	HWVER_110 = 1,
 };
 
+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+	out_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16), data);
+}
+
+u16 fpga_get_reg(unsigned int fpga, u16 reg)
+{
+	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16));
+}
+
 static inline void blank_string(int size)
 {
 	int i;
@@ -100,10 +110,9 @@ int misc_init_r(void)
 
 static void print_fpga_info(unsigned dev)
 {
-	struct ihs_fpga *fpga = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(dev);
-	u16 versions = in_le16(&fpga->versions);
-	u16 fpga_version = in_le16(&fpga->fpga_version);
-	u16 fpga_features = in_le16(&fpga->fpga_features);
+	u16 versions = fpga_get_reg(dev, REG(versions));
+	u16 fpga_version = fpga_get_reg(dev, REG(fpga_version));
+	u16 fpga_features = fpga_get_reg(dev, REG(fpga_features));
 	int fpga_state = get_fpga_state(dev);
 
 	unsigned unit_type;
@@ -242,8 +251,6 @@ int last_stage_init(void)
 {
 	unsigned int k;
 	unsigned int fpga;
-	struct ihs_fpga *fpga0 = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(0);
-	struct ihs_fpga *fpga1 = (struct ihs_fpga *) CONFIG_SYS_FPGA_BASE(1);
 	int failed = 0;
 	char str_phys[] = "Setup PHYs -";
 	char str_serdes[] = "Start SERDES blocks";
@@ -281,17 +288,16 @@ int last_stage_init(void)
 	/* take fpga serdes blocks out of reset */
 	puts(str_serdes);
 	udelay(500000);
-	out_le16(&fpga0->quad_serdes_reset, 0);
-	out_le16(&fpga1->quad_serdes_reset, 0);
+	fpga_set_reg(0, REG(quad_serdes_reset), 0);
+	fpga_set_reg(1, REG(quad_serdes_reset), 0);
 	blank_string(strlen(str_serdes));
 
 	/* take channels out of reset */
 	puts(str_channels);
 	udelay(500000);
 	for (fpga = 0; fpga < 2; ++fpga) {
-		u16 *ch0_config_int = &(fpga ? fpga1 : fpga0)->ch0_config_int;
 		for (k = 0; k < 32; ++k)
-			out_le16(ch0_config_int + 4 * k, 0);
+			fpga_set_reg(fpga, REG(ch0_config_int) + 8 * k, 0);
 	}
 	blank_string(strlen(str_channels));
 
@@ -299,16 +305,16 @@ int last_stage_init(void)
 	puts(str_locks);
 	udelay(500000);
 	for (fpga = 0; fpga < 2; ++fpga) {
-		u16 *ch0_status_int = &(fpga ? fpga1 : fpga0)->ch0_status_int;
 		for (k = 0; k < 32; ++k) {
-			u16 status = in_le16(ch0_status_int + 4*k);
+			u16 status =
+				fpga_get_reg(fpga, REG(ch0_status_int) + 8 * k);
 			if (!(status & (1 << 4))) {
 				failed = 1;
 				printf("fpga %d channel %d: no serdes lock\n",
 					fpga, k);
 			}
 			/* reset events */
-			out_le16(ch0_status_int + 4*k, status);
+			fpga_set_reg(fpga, REG(ch0_status_int) + 8 * k, 0);
 		}
 	}
 	blank_string(strlen(str_locks));
@@ -316,14 +322,14 @@ int last_stage_init(void)
 	/* verify hicb_status */
 	puts(str_hicb);
 	for (fpga = 0; fpga < 2; ++fpga) {
-		u16 *ch0_hicb_status_int = &(fpga ? fpga1 : fpga0)->ch0_hicb_status_int;
 		for (k = 0; k < 32; ++k) {
-			u16 status = in_le16(ch0_hicb_status_int + 4*k);
+			u16 status =
+				fpga_get_reg(fpga, REG(ch0_status_int) + 8 * k);
 			if (status)
 				printf("fpga %d hicb %d: hicb status %04x\n",
 					fpga, k, status);
 			/* reset events */
-			out_le16(ch0_hicb_status_int + 4*k, status);
+			fpga_set_reg(fpga, REG(ch0_status_int) + 8 * k, 0);
 		}
 	}
 	blank_string(strlen(str_hicb));
-- 
1.7.2.5

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear dirk.eibach at gdsys.cc,

In message <1367847325-21463-7-git-send-email-dirk.eibach@gdsys.cc> you wrote:
> From: Dirk Eibach <eibach@gdsys.de>
> 
> Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
> ---
> Changes in v2: None
> 
>  board/gdsys/405ep/dlvision-10g.c |   20 ++++++++++++++------
>  board/gdsys/405ep/io.c           |   20 ++++++++++++++------
>  board/gdsys/405ep/neo.c          |   17 +++++++++++++----
>  board/gdsys/405ex/405ex.c        |   14 ++++++--------
>  board/gdsys/405ex/io64.c         |   38 ++++++++++++++++++++++----------------
>  5 files changed, 69 insertions(+), 40 deletions(-)
> 
> diff --git a/board/gdsys/405ep/dlvision-10g.c b/board/gdsys/405ep/dlvision-10g.c
> index 644493b..4d1a02e 100644
> --- a/board/gdsys/405ep/dlvision-10g.c
> +++ b/board/gdsys/405ep/dlvision-10g.c
> @@ -71,6 +71,16 @@ enum {
>  	RAM_DDR2_64 = 2,
>  };
>  
> +void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
> +{
> +	out_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16), data);
> +}
> +
> +u16 fpga_get_reg(unsigned int fpga, u16 reg)
> +{
> +	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16));
> +}
> +

We strictly discourage using a notation of base address plus offset,
and require to use proper C structs instead, especially because this
allows type checking by the compiler.

You had the this, and now attempt to throw it away.  This makes no
sense.

NAK.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A fail-safe circuit will destroy others.                 -- Klipstein

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hi Wolfgang,

> ...
> 
> We strictly discourage using a notation of base address plus offset,
> and require to use proper C structs instead, especially because this
> allows type checking by the compiler.

I am very well aware of that.

> You had the this, and now attempt to throw it away.  This makes no
> sense.

In fact it does.
We have FPGAs that are memory mapped and others that are not. They must 
be accessed by the same drivers. So the alternative would be to create 
FPGA instances at address NULL and getting the register offesets by 
casting pointers to u16. Not very nice either.

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk Eibach,

In message <e8fccb4d422d619744521268691e5a9b@gdsys.cc> you wrote:
> 
> > You had the this, and now attempt to throw it away.  This makes no
> > sense.
> 
> In fact it does.
> We have FPGAs that are memory mapped and others that are not. They must 
> be accessed by the same drivers. So the alternative would be to create 
> FPGA instances at address NULL and getting the register offesets by 
> casting pointers to u16. Not very nice either.

Your new code still boils down to using the same standard I/O
accessors.

So your FPGA registers must be mapped somehow to I/O memory.

When you can do something like

+u16 fpga_get_reg(unsigned int fpga, u16 reg)
+{
+	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / sizeof(u16));
+}

why would you not be able to continue using in_le16() directly?

Sorry, I don't get it.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Ernest asks Frank how long he has been working for the company.
        "Ever since they threatened to fire me."

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hi Wolfgang,

Am 06.05.2013 17:22, schrieb Wolfgang Denk:
> Dear Dirk Eibach,
> 
> In message <e8fccb4d422d619744521268691e5a9b@gdsys.cc> you wrote:
>> 
>>> You had the this, and now attempt to throw it away.  This makes no
>>> sense.
>> 
>> In fact it does.
>> We have FPGAs that are memory mapped and others that are not. They 
>> must
>> be accessed by the same drivers. So the alternative would be to 
>> create
>> FPGA instances at address NULL and getting the register offesets by
>> casting pointers to u16. Not very nice either.
> 
> Your new code still boils down to using the same standard I/O
> accessors.
> 
> So your FPGA registers must be mapped somehow to I/O memory.
> 
> When you can do something like
> 
> +u16 fpga_get_reg(unsigned int fpga, u16 reg)
> +{
> +	return in_le16((u16 *)CONFIG_SYS_FPGA_BASE(fpga) + reg / 
> sizeof(u16));
> +}
> 
> why would you not be able to continue using in_le16() directly?
> 
> Sorry, I don't get it.

Read the source, Luke :)

On our iocon platform you find:

+void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
+{
+	int res;
+	struct ihs_fpga *fpga_0 = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0);
+
+	switch (fpga) {
+	case 0:
+		out_le16((u16 *)fpga_0 + reg / sizeof(u16), data);
+		break;
+	default:
+		res = mclink_send(fpga - 1, reg, data);
+		if (res < 0)
+			printf("mclink_send reg %02x data %04x returned %d\n",
+			       reg, data, res);
+		break;
+	}
+}

So no memory mapping here. That's the reason for all this fuzz.

And sorry for the messed up series. Sometimes rebase can make things 
worse :)

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk Eibach,

In message <449c45a6a9978c55e84d3fe7efe6f0ac@gdsys.cc> you wrote:
> 
> Read the source, Luke :)

It was a bit difficult to spot in this unordered mix of patches ;-)

> +void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
> +{
> +	int res;
> +	struct ihs_fpga *fpga_0 = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0);
> +
> +	switch (fpga) {
> +	case 0:
> +		out_le16((u16 *)fpga_0 + reg / sizeof(u16), data);
> +		break;
> +	default:
> +		res = mclink_send(fpga - 1, reg, data);
> +		if (res < 0)
> +			printf("mclink_send reg %02x data %04x returned %d\n",
> +			       reg, data, res);
> +		break;
> +	}
> +}
> 
> So no memory mapping here. That's the reason for all this fuzz.

OK - but I don't see why mclink_send() could not be used in the same
way, i. e. taking a struct member as argument?

Apparently all your FPGA accesses are for  u16  data only?  Then you
could rewrite fpga_set_reg() similar to this:

	int fpga_set_reg(u32 fpga, u16 *addr, u16 data)

And voila, no need to move away from the C structs.

Note 1: You print an error message if mclink_send() returns an error;
	I think this error should propagate, i. e. this function
	should not return  void.

Note 2: I changed the type of the first arg into "u32" so it is
        consistent with the other args. Mixing native types (unsigned
        int) here and defined types (as u16 instead of unsigned
        short) looks weird to me.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Eeeek!
'eval' on strings should have been named 'evil'.    -- Tom Phoenix in
        <Pine.GSO.3.96.980526121813.27437N-100000@user2.teleport.com>

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hi Wolfgang,

>> +void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
>> +{
>> +	int res;
>> +	struct ihs_fpga *fpga_0 = (struct ihs_fpga 
>> *)CONFIG_SYS_FPGA_BASE(0);
>> +
>> +	switch (fpga) {
>> +	case 0:
>> +		out_le16((u16 *)fpga_0 + reg / sizeof(u16), data);
>> +		break;
>> +	default:
>> +		res = mclink_send(fpga - 1, reg, data);
>> +		if (res < 0)
>> +			printf("mclink_send reg %02x data %04x returned %d\n",
>> +			       reg, data, res);
>> +		break;
>> +	}
>> +}
>> 
>> So no memory mapping here. That's the reason for all this fuzz.
> 
> OK - but I don't see why mclink_send() could not be used in the same
> way, i. e. taking a struct member as argument?

Certainly it *can* be used using *pointers* to struct members as 
argument.

What I stated yesterday was:
"We have FPGAs that are memory mapped and others that are not. They 
must be accessed by the same drivers. So the alternative would be to 
create FPGA instances at address NULL and getting the register offesets 
by casting pointers to u16. Not very nice either."


> Apparently all your FPGA accesses are for  u16  data only?  Then you
> could rewrite fpga_set_reg() similar to this:
> 
> 	int fpga_set_reg(u32 fpga, u16 *addr, u16 data)

With
   struct ihs_fpga {
	u16 reflection_low;	/* 0x0000 */
	u16 versions;		/* 0x0002 */
	...
   };
and
   void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
the call looks like
   fpga_set_reg(k, REG(reflection_low), REFLECTION_TESTPATTERN);


To use
   int fpga_set_reg(u32 fpga, u16 *addr, u16 data)
we need something like
   struct ihs_fpga system_fpgas[] = {
	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0),
	(struct ihs_fpga *)NULL,
	(struct ihs_fpga *)NULL,
	(struct ihs_fpga *)NULL,
   };
and
   fpga_set_reg(k, system_fpgas[k].reflection_low, 
REFLECTION_TESTPATTERN);

In fpga_set_reg() it would look like
   mclink_send(fpga - 1, (u16)addr, data);

I personally dislike all this NULL-pointer passing and casting. But 
YMMV. If that's the u-boot-way I will be happy to change it.

> And voila, no need to move away from the C structs.
> 
> Note 1: You print an error message if mclink_send() returns an error;
> 	I think this error should propagate, i. e. this function
> 	should not return  void.

Yep. Happy were the days when accessing FPGA registers could not fail 
:(

> Note 2: I changed the type of the first arg into "u32" so it is
>         consistent with the other args. Mixing native types (unsigned
>         int) here and defined types (as u16 instead of unsigned
>         short) looks weird to me.

I wanted to express the 16 bit io-implementation of our address/data 
busses, while the fpga index is simply a number. Maybe that is 
information overkill :)

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk Eibach,

In message <cf2913653766edc89705f49e08758df7@gdsys.cc> you wrote:
> 
> > OK - but I don't see why mclink_send() could not be used in the same
> > way, i. e. taking a struct member as argument?
> 
> Certainly it *can* be used using *pointers* to struct members as 
> argument.
> 
> What I stated yesterday was:
> "We have FPGAs that are memory mapped and others that are not. They 
> must be accessed by the same drivers. So the alternative would be to 
> create FPGA instances at address NULL and getting the register offesets 
> by casting pointers to u16. Not very nice either."

Yes, you wrote that.  I did not understand it then, nor do I
understand it now.  What do you mean by "create FPGA instances at
address NULL"?  in any case, "getting the register offsets by casting
pointers to u16" makes no sense, as this is exactly what we try to
avoid.  By referencing a C struct you do NOT use any offsets.  You use
references to struct elements; where needed, you let the compiler to
the address calculations (and the type checking).

No offsets. No casts.

> > could rewrite fpga_set_reg() similar to this:
> > 
> > 	int fpga_set_reg(u32 fpga, u16 *addr, u16 data)
> 
> With
>    struct ihs_fpga {
> 	u16 reflection_low;	/* 0x0000 */
> 	u16 versions;		/* 0x0002 */
> 	...
>    };
> and
>    void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
> the call looks like
>    fpga_set_reg(k, REG(reflection_low), REFLECTION_TESTPATTERN);

Mo. It should look like that:

	int fpga_set_reg(u32 fpga, u16 *addr, u16 data)

	rc = fpga_set_reg(k, &ptr->reflection_low, REFLECTION_TESTPATTERN);
	if (rc)
		...
> To use
>    int fpga_set_reg(u32 fpga, u16 *addr, u16 data)
> we need something like
>    struct ihs_fpga system_fpgas[] = {
> 	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0),
> 	(struct ihs_fpga *)NULL,
> 	(struct ihs_fpga *)NULL,
> 	(struct ihs_fpga *)NULL,
>    };

Is this not redundant?  If you have an array of pointers (addresses),
then you can identify the FPGA by using the pointer, and don't need
to pass both the pointer _and_ the index to the fpga_{g,s}et_reg()
functions?

> I personally dislike all this NULL-pointer passing and casting. But 
> YMMV. If that's the u-boot-way I will be happy to change it.

Why would you need casts?  The whole purpose of this is to _avoid_
casts, so that the compiler has a chance to make sure all accesses are
performed using the correct alignment and size of the respective data
types.

> > Note 2: I changed the type of the first arg into "u32" so it is
> >         consistent with the other args. Mixing native types (unsigned
> >         int) here and defined types (as u16 instead of unsigned
> >         short) looks weird to me.
> 
> I wanted to express the 16 bit io-implementation of our address/data 
> busses, while the fpga index is simply a number. Maybe that is 
> information overkill :)

Eventually you can drop the index alltogether when you just pass a
pointer?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Time is a drug. Too much of it kills you.
                                      - Terry Pratchett, _Small Gods_

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Am 07.05.2013 13:36, schrieb Wolfgang Denk:
> Dear Dirk Eibach,
> 
> In message <cf2913653766edc89705f49e08758df7@gdsys.cc> you wrote:
>> 
>>> OK - but I don't see why mclink_send() could not be used in the same
>>> way, i. e. taking a struct member as argument?
>> 
>> Certainly it *can* be used using *pointers* to struct members as
>> argument.
>> 
>> What I stated yesterday was:
>> "We have FPGAs that are memory mapped and others that are not. They
>> must be accessed by the same drivers. So the alternative would be to
>> create FPGA instances at address NULL and getting the register 
>> offesets
>> by casting pointers to u16. Not very nice either."
> 
> Yes, you wrote that.  I did not understand it then, nor do I
> understand it now.  What do you mean by "create FPGA instances at
> address NULL"?  in any case, "getting the register offsets by casting
> pointers to u16" makes no sense, as this is exactly what we try to
> avoid.  By referencing a C struct you do NOT use any offsets.  You use
> references to struct elements; where needed, you let the compiler to
> the address calculations (and the type checking).
> 
> No offsets. No casts.

OK. Once more. 3 of our 4 FPGAs are *not* memory mapped. There is no 
base address. This is what I want to show by:
>>    struct ihs_fpga system_fpgas[] = {
>> 	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0),
>> 	(struct ihs_fpga *)NULL,
>> 	(struct ihs_fpga *)NULL,
>> 	(struct ihs_fpga *)NULL,
>>    };

For accessing registers of those (not memory mapped) FPGAs I need an 
u16 register-index. That is what I want to show by:
>> mclink_send(fpga - 1, (u16)addr, data);

This is the cast I am talking about.

This is the reason why I still need the fpga index parameter.

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk Eibach,

In message <ba0b32bf657f5f3ffb6056cc6bd0452e@gdsys.cc> you wrote:
> 
> OK. Once more. 3 of our 4 FPGAs are *not* memory mapped. There is no 
> base address. This is what I want to show by:
> >>    struct ihs_fpga system_fpgas[] = {
> >> 	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0),
> >> 	(struct ihs_fpga *)NULL,
> >> 	(struct ihs_fpga *)NULL,
> >> 	(struct ihs_fpga *)NULL,
> >>    };
> 
> For accessing registers of those (not memory mapped) FPGAs I need an 
> u16 register-index. That is what I want to show by:
> >> mclink_send(fpga - 1, (u16)addr, data);

Can you please provide a bit more context?  What is "addr", and why
does it need a cast?

> This is the cast I am talking about.
> 
> This is the reason why I still need the fpga index parameter.

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hi Wolfgang,

in my example we have four FPGAs. One is memory mapped while the other 
three are not.
They all have the same register interface which is mapped by
   struct ihs_fpga {
	u16 reflection_low;
	u16 versions;
	...
	u16 mc_tx_address;
	u16 mc_tx_data;
	u16 mc_tx_cmd;
	...
   };

To have instances of those FPGA structs, we might create an array like 
this:
   struct ihs_fpga system_fpgas[] = {
	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE,
	(struct ihs_fpga *)NULL,
	(struct ihs_fpga *)NULL,
	(struct ihs_fpga *)NULL,
   };
The first element ist our memory mapped FPGA with base address 
CONFIG_SYS_FPGA_BASE. For the other three I use NULL as base address 
because I don't have any better idea what else to choose.

To probe those FPGAs we might have to do something like:
   for (k=0; k < 4; ++k)
	fpga_set_reg(k, &system_fpgas[k].reflection_low, 
REFLECTION_TESTPATTERN);

The implementation of fpga_set_reg() might look like:

void fpga_set_reg(unsigned int fpga, u16 *reg, u16 data)
{
	switch (fpga) {
	case 0:
		out_le16(reg, data);
		break;
	default:
		mclink_send(fpga - 1, (u16)reg, data);
		break;
	}
}

where mclink_send() is the routine to access the non memory mapped 
FPGAs through the memory mapped FPGA:
   int mclink_send(u8 slave, u16 addr, u16 data)
   {
	...
	fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
	fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
	fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) << 14);
   }

The cast to u16 I am talking about happens, when mclink_send() is 
called.

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk,

In message <58ff5023adcbf08481bc2707b0a70f50@gdsys.cc> you wrote:
> 
> in my example we have four FPGAs. One is memory mapped while the other 
> three are not.
> They all have the same register interface which is mapped by
>    struct ihs_fpga {
> 	u16 reflection_low;
> 	u16 versions;
> 	...
> 	u16 mc_tx_address;
> 	u16 mc_tx_data;
> 	u16 mc_tx_cmd;
> 	...
>    };
> 
> To have instances of those FPGA structs, we might create an array like 
> this:
>    struct ihs_fpga system_fpgas[] = {
> 	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE,
> 	(struct ihs_fpga *)NULL,
> 	(struct ihs_fpga *)NULL,
> 	(struct ihs_fpga *)NULL,
>    };
> The first element ist our memory mapped FPGA with base address 
> CONFIG_SYS_FPGA_BASE. For the other three I use NULL as base address 
> because I don't have any better idea what else to choose.
> 
> To probe those FPGAs we might have to do something like:
>    for (k=0; k < 4; ++k)
> 	fpga_set_reg(k, &system_fpgas[k].reflection_low, 

I think using index notation everywhere makes the code hard to read.
Use a pointer, for example like that:

	struct ihs_fpga *fpgap = system_fpgas[k];

	fpga_set_reg(k, &fpgap->reflection_low, ...

> The implementation of fpga_set_reg() might look like:
> 
> void fpga_set_reg(unsigned int fpga, u16 *reg, u16 data)
> {
> 	switch (fpga) {
> 	case 0:
> 		out_le16(reg, data);
> 		break;
> 	default:
> 		mclink_send(fpga - 1, (u16)reg, data);
> 		break;
> 	}
> }

The problem here comes from your decision to use the same interface
for two different, incompatible things.  There are two ugly parts in
this code:  first is the need for a cast, second (and even worse) is
the use of a pointer value of 0 to get to the offset of a field.
Actually this is not even standard conformng, as you initialize the
pointer as NULL, and there is no real guarantee that

	NULL == (struct ihs_fpga *)0

It appears you need both the element address and the offset in your
fpga_set_reg() code, so you should pass this information, like that
[note that we no longer need an array of addresses]:

struct ihs_fpga fpga_ptr = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE;
...

void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
{
	if (fpga)
		return mclink_send(fpga - 1, regoff, data);

	return out_le16(reg, data);
}

> where mclink_send() is the routine to access the non memory mapped 
> FPGAs through the memory mapped FPGA:
>    int mclink_send(u8 slave, u16 addr, u16 data)
>    {
> 	...
> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) << 14);
>    }

And this becomes:

	fpga_set_reg(0,
		     &fpga_ptr->mc_tx_address,
		     offsetof(struct ihs_fpga, mc_tx_address),
		     addr);

etc. Yes, this is long and ugly to write, so you may want to define a
helper macro like:

#define FPGA_SET_REG(ix,fld,val)	\
	fpga_set_reg((ix),		\
	&fpga_ptr->fld,			\
	offsetof(struct ihs_fpga, fld),	\
	val)

so this turns into a plain and simple

	FPGA_SET_REG(0, mc_tx_address, addr);
	FPGA_SET_REG(0, mc_tx_data, data);
	FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);


What do you think?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
It is much easier to suggest solutions when you know nothing

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hi Wolfgang,

thanks for investing so much time into this. It is really appreciated.

Am 08.05.2013 12:13, schrieb Wolfgang Denk:
> Dear Dirk,
> 
> In message <58ff5023adcbf08481bc2707b0a70f50@gdsys.cc> you wrote:
>> 
>> in my example we have four FPGAs. One is memory mapped while the 
>> other
>> three are not.
>> They all have the same register interface which is mapped by
>>    struct ihs_fpga {
>> 	u16 reflection_low;
>> 	u16 versions;
>> 	...
>> 	u16 mc_tx_address;
>> 	u16 mc_tx_data;
>> 	u16 mc_tx_cmd;
>> 	...
>>    };
>> 
>> To have instances of those FPGA structs, we might create an array 
>> like
>> this:
>>    struct ihs_fpga system_fpgas[] = {
>> 	(struct ihs_fpga *)CONFIG_SYS_FPGA_BASE,
>> 	(struct ihs_fpga *)NULL,
>> 	(struct ihs_fpga *)NULL,
>> 	(struct ihs_fpga *)NULL,
>>    };
>> The first element ist our memory mapped FPGA with base address
>> CONFIG_SYS_FPGA_BASE. For the other three I use NULL as base address
>> because I don't have any better idea what else to choose.
>> 
>> To probe those FPGAs we might have to do something like:
>>    for (k=0; k < 4; ++k)
>> 	fpga_set_reg(k, &system_fpgas[k].reflection_low,
> 
> I think using index notation everywhere makes the code hard to read.
> Use a pointer, for example like that:
> 
> 	struct ihs_fpga *fpgap = system_fpgas[k];
> 
> 	fpga_set_reg(k, &fpgap->reflection_low, ...
> 
>> The implementation of fpga_set_reg() might look like:
>> 
>> void fpga_set_reg(unsigned int fpga, u16 *reg, u16 data)
>> {
>> 	switch (fpga) {
>> 	case 0:
>> 		out_le16(reg, data);
>> 		break;
>> 	default:
>> 		mclink_send(fpga - 1, (u16)reg, data);
>> 		break;
>> 	}
>> }
> 
> The problem here comes from your decision to use the same interface
> for two different, incompatible things.  There are two ugly parts in
> this code:  first is the need for a cast, second (and even worse) is
> the use of a pointer value of 0 to get to the offset of a field.
> Actually this is not even standard conformng, as you initialize the
> pointer as NULL, and there is no real guarantee that
> 
> 	NULL == (struct ihs_fpga *)0

You are right. This is *exactly* what I meant when I wrote: "We have 
FPGAs that are memory mapped and others that are not. They must be 
accessed by the same drivers. So the alternative would be to create FPGA 
instances at address NULL and getting the register offesets by casting 
pointers to u16. Not very nice either." on monday.


> It appears you need both the element address and the offset in your
> fpga_set_reg() code, so you should pass this information, like that
> [note that we no longer need an array of addresses]:
> 
> struct ihs_fpga fpga_ptr = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE;
> ...
> 
> void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
> {
> 	if (fpga)
> 		return mclink_send(fpga - 1, regoff, data);
> 
> 	return out_le16(reg, data);
> }
> 
>> where mclink_send() is the routine to access the non memory mapped
>> FPGAs through the memory mapped FPGA:
>>    int mclink_send(u8 slave, u16 addr, u16 data)
>>    {
>> 	...
>> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
>> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
>> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) << 14);
>>    }
> 
> And this becomes:
> 
> 	fpga_set_reg(0,
> 		     &fpga_ptr->mc_tx_address,
> 		     offsetof(struct ihs_fpga, mc_tx_address),
> 		     addr);
> 
> etc. Yes, this is long and ugly to write, so you may want to define a
> helper macro like:
> 
> #define FPGA_SET_REG(ix,fld,val)	\
> 	fpga_set_reg((ix),		\
> 	&fpga_ptr->fld,			\
> 	offsetof(struct ihs_fpga, fld),	\
> 	val)
> 
> so this turns into a plain and simple
> 
> 	FPGA_SET_REG(0, mc_tx_address, addr);
> 	FPGA_SET_REG(0, mc_tx_data, data);
> 	FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);
> 
> 
> What do you think?

Very nice.
I think this looks supringsingly similar to my original implementation 
in the patch series. There we have:
   #define REG(reg) offsetof(struct ihs_fpga, reg)
and
   fpga_set_reg(k, REG(reflection_low), REFLECTION_TESTPATTERN);
and
   void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
   {
	int res;
	struct ihs_fpga *fpga_0 = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE(0);

	switch (fpga) {
	case 0:
		out_le16((u16 *)fpga_0 + reg / sizeof(u16), data);
		break;
	default:
		res = mclink_send(fpga - 1, reg, data);
		if (res < 0)
			printf("mclink_send reg %02x data %04x returned %d\n",
			       reg, data, res);
		break;
	}
   }

Sorry, I'm a little confused here. Is your approach an improvement 
anyway?

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk,

In message <913a7e8badaaed06605fdf68faacedc8@gdsys.cc> you wrote:
> 
> thanks for investing so much time into this. It is really appreciated.

Well, just rejecting a patch without being able to give
recommendations _how_ to improve it is not exactly nice.
Sometimes it happens (like due to lack of time and/or better
knowledge), but I try to avoid that.

> > 	NULL == (struct ihs_fpga *)0
> 
> You are right. This is *exactly* what I meant when I wrote: "We have 
> FPGAs that are memory mapped and others that are not. They must be 
> accessed by the same drivers. So the alternative would be to create FPGA 
> instances at address NULL and getting the register offesets by casting 
> pointers to u16. Not very nice either." on monday.

You don't get what I mean.  There is no guarantee by the C standard
that the "value" of NULL actually resolves to 0; it could be something
else.  So assuming it does is not only ugly, but non-conforming to the
C standard, and thus error prone.

> > What do you think?
> 
> Very nice.
> I think this looks supringsingly similar to my original implementation 
> in the patch series. There we have:

Yes, except that now we still have 1) the full type checking when we
access struct elements in the standard I/O accessors and 2) no need
for casts when using offsets, and 3) no need to assume that NULL == 0.

> Sorry, I'm a little confused here. Is your approach an improvement 
> anyway?

Yes, definitely.  At least I'm convinced of that.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The main thing is the play itself. I swear that greed for  money  has
nothing  to  do with it, although heaven knows I am sorely in need of
money.                                           - Feodor Dostoyevsky

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hello Wolfgang,

sorry for the delay, I had to clean some other things on my ToDoList.

I cleaned up my patchseries and wanted to start implementing your 
proposal. And ran into a problem. Remember, the basic reason for the the 
generic FPGA accessors was, that we have a certain amount of basically 
identical FPGAs which are only connected to the CPU in different ways. 
The drivers accessing those FPGAs should be agnostic of that and simply 
be able to access those FPGAs by index.

> It appears you need both the element address and the offset in your
> fpga_set_reg() code, so you should pass this information, like that
> [note that we no longer need an array of addresses]:
> 
> struct ihs_fpga fpga_ptr = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE;
> ...
> 
> void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
> {
> 	if (fpga)
> 		return mclink_send(fpga - 1, regoff, data);
> 
> 	return out_le16(reg, data);
> }
> 
>> where mclink_send() is the routine to access the non memory mapped
>> FPGAs through the memory mapped FPGA:
>>    int mclink_send(u8 slave, u16 addr, u16 data)
>>    {
>> 	...
>> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
>> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
>> 	fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) << 14);
>>    }
> 
> And this becomes:
> 
> 	fpga_set_reg(0,
> 		     &fpga_ptr->mc_tx_address,
> 		     offsetof(struct ihs_fpga, mc_tx_address),
> 		     addr);
> 
> etc. Yes, this is long and ugly to write, so you may want to define a
> helper macro like:
> 
> #define FPGA_SET_REG(ix,fld,val)	\
> 	fpga_set_reg((ix),		\
> 	&fpga_ptr->fld,			\
> 	offsetof(struct ihs_fpga, fld),	\
> 	val)
> 
> so this turns into a plain and simple
> 
> 	FPGA_SET_REG(0, mc_tx_address, addr);
> 	FPGA_SET_REG(0, mc_tx_data, data);
> 	FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);

This works nicely for our memory mapped FPGA. But how about the other 
FPGAs? I don't have a fpga_ptr for them. Setting it NULL would make 
FPGA_SET_REG dereference a NULL pointer.
Certainly I might call
	fpga_set_reg(1,
		     NULL,
		     offsetof(struct ihs_fpga, mc_tx_address),
		     addr);
but that would not be generic any more.

Do you have any idea how to solve this?

Cheers
Dirk

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hello Wolfgang,

would you please have a look at this? I need some feedback to get this
finally sorted.

Cheers
Dirk

2013/5/15 Dirk Eibach <dirk.eibach@gdsys.cc>:
> Hello Wolfgang,
>
> sorry for the delay, I had to clean some other things on my ToDoList.
>
> I cleaned up my patchseries and wanted to start implementing your proposal.
> And ran into a problem. Remember, the basic reason for the the generic FPGA
> accessors was, that we have a certain amount of basically identical FPGAs
> which are only connected to the CPU in different ways. The drivers accessing
> those FPGAs should be agnostic of that and simply be able to access those
> FPGAs by index.
>
>
>> It appears you need both the element address and the offset in your
>> fpga_set_reg() code, so you should pass this information, like that
>> [note that we no longer need an array of addresses]:
>>
>> struct ihs_fpga fpga_ptr = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE;
>> ...
>>
>> void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
>> {
>>         if (fpga)
>>                 return mclink_send(fpga - 1, regoff, data);
>>
>>         return out_le16(reg, data);
>> }
>>
>>> where mclink_send() is the routine to access the non memory mapped
>>> FPGAs through the memory mapped FPGA:
>>>    int mclink_send(u8 slave, u16 addr, u16 data)
>>>    {
>>>         ...
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) <<
>>> 14);
>>>    }
>>
>>
>> And this becomes:
>>
>>         fpga_set_reg(0,
>>                      &fpga_ptr->mc_tx_address,
>>                      offsetof(struct ihs_fpga, mc_tx_address),
>>                      addr);
>>
>> etc. Yes, this is long and ugly to write, so you may want to define a
>> helper macro like:
>>
>> #define FPGA_SET_REG(ix,fld,val)        \
>>         fpga_set_reg((ix),              \
>>         &fpga_ptr->fld,                 \
>>         offsetof(struct ihs_fpga, fld), \
>>         val)
>>
>> so this turns into a plain and simple
>>
>>         FPGA_SET_REG(0, mc_tx_address, addr);
>>         FPGA_SET_REG(0, mc_tx_data, data);
>>         FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);
>
>
> This works nicely for our memory mapped FPGA. But how about the other FPGAs?
> I don't have a fpga_ptr for them. Setting it NULL would make FPGA_SET_REG
> dereference a NULL pointer.
> Certainly I might call
>         fpga_set_reg(1,
>                      NULL,
>                      offsetof(struct ihs_fpga, mc_tx_address),
>                      addr);
> but that would not be generic any more.
>
> Do you have any idea how to solve this?
>
>
> Cheers
> Dirk
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Dirk Eibach]

Hello Wolfgang,

would you please have a look at this? I need some feedback to get this
finally sorted.

Cheers
Dirk

2013/5/15 Dirk Eibach <dirk.eibach@gdsys.cc>:
> Hello Wolfgang,
>
> sorry for the delay, I had to clean some other things on my ToDoList.
>
> I cleaned up my patchseries and wanted to start implementing your proposal.
> And ran into a problem. Remember, the basic reason for the the generic FPGA
> accessors was, that we have a certain amount of basically identical FPGAs
> which are only connected to the CPU in different ways. The drivers accessing
> those FPGAs should be agnostic of that and simply be able to access those
> FPGAs by index.
>
>
>> It appears you need both the element address and the offset in your
>> fpga_set_reg() code, so you should pass this information, like that
>> [note that we no longer need an array of addresses]:
>>
>> struct ihs_fpga fpga_ptr = (struct ihs_fpga *)CONFIG_SYS_FPGA_BASE;
>> ...
>>
>> void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
>> {
>>         if (fpga)
>>                 return mclink_send(fpga - 1, regoff, data);
>>
>>         return out_le16(reg, data);
>> }
>>
>>> where mclink_send() is the routine to access the non memory mapped
>>> FPGAs through the memory mapped FPGA:
>>>    int mclink_send(u8 slave, u16 addr, u16 data)
>>>    {
>>>         ...
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_address, addr);
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_data, data);
>>>         fpga_set_reg(0, &system_fpgas[0].mc_tx_cmd, (slave & 0x03) <<
>>> 14);
>>>    }
>>
>>
>> And this becomes:
>>
>>         fpga_set_reg(0,
>>                      &fpga_ptr->mc_tx_address,
>>                      offsetof(struct ihs_fpga, mc_tx_address),
>>                      addr);
>>
>> etc. Yes, this is long and ugly to write, so you may want to define a
>> helper macro like:
>>
>> #define FPGA_SET_REG(ix,fld,val)        \
>>         fpga_set_reg((ix),              \
>>         &fpga_ptr->fld,                 \
>>         offsetof(struct ihs_fpga, fld), \
>>         val)
>>
>> so this turns into a plain and simple
>>
>>         FPGA_SET_REG(0, mc_tx_address, addr);
>>         FPGA_SET_REG(0, mc_tx_data, data);
>>         FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);
>
>
> This works nicely for our memory mapped FPGA. But how about the other FPGAs?
> I don't have a fpga_ptr for them. Setting it NULL would make FPGA_SET_REG
> dereference a NULL pointer.
> Certainly I might call
>         fpga_set_reg(1,
>                      NULL,
>                      offsetof(struct ihs_fpga, mc_tx_address),
>                      addr);
> but that would not be generic any more.
>
> Do you have any idea how to solve this?
>
>
> Cheers
> Dirk
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [PATCH v2 06/10] powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex boards

[Wolfgang Denk]

Dear Dirk,

In message <0e4604dcfff89a1bd2b7f144a5b3ceae@gdsys.cc> you wrote:
> 
> sorry for the delay, I had to clean some other things on my ToDoList.

Ditto :-(

> > void fpga_set_reg(u32 fpga, u16 *reg, off_t regoff, u16 data)
> > {
> > 	if (fpga)
> > 		return mclink_send(fpga - 1, regoff, data);
> > 
> > 	return out_le16(reg, data);
> > }
...

> > #define FPGA_SET_REG(ix,fld,val)	\
> > 	fpga_set_reg((ix),		\
> > 	&fpga_ptr->fld,			\
> > 	offsetof(struct ihs_fpga, fld),	\
> > 	val)
...
> > 	FPGA_SET_REG(0, mc_tx_address, addr);
> > 	FPGA_SET_REG(0, mc_tx_data, data);
> > 	FPGA_SET_REG(0, mc_tx_cmd, (slave & 0x03) << 14);
> 
> This works nicely for our memory mapped FPGA. But how about the other 
> FPGAs? I don't have a fpga_ptr for them. Setting it NULL would make 
> FPGA_SET_REG dereference a NULL pointer.

No, it does not.

In your setup you have a single memory mapped FPGA, addressed by the
globally visible pointer  fpga_ptr.

When accessing this memory mapped FPGA (index is zero), then this
pointer gets used, which is OK.

For the other devices (index is not zero), then the pointer is
ignored, and only the offset is used.

In case you had more memory mapped devices (not only index 0; say,
something like index < N) you can still use this, and pass any
(properly aligned) pointer for the non-memory mapped case.  Note that
the pointer will NOT be dereferenced - only the address of field "fld"
will be calculated (and even this only in theory, because this code
will not be executed for the non-memory mapped case).

> Certainly I might call
> 	fpga_set_reg(1,
> 		     NULL,
> 		     offsetof(struct ihs_fpga, mc_tx_address),
> 		     addr);
> but that would not be generic any more.

You don't call fpga_set_reg() any more, you just use the
FPGA_SET_REG() macro described above.

> Do you have any idea how to solve this?

I think this needs no new solution, because it shoul just work :-)

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Alan Turing thought about criteria to settle the question of  whether
machines  can think, a question of which we now know that it is about
as relevant as the question of whether submarines can swim.
                                                   -- Edsger Dijkstra

[U-Boot] [PATCH v2 07/10] powerpc/ppc4xx: Fixup phy erratum on gdsys iocon hardware

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

Marvell 88E1518 has an erratum that requires fixing up.
This patch checks for presence of this phy and adds code
for fixup.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---
Changes in v2: None

 board/gdsys/405ep/iocon.c |  214 +++++++++++++++++++++++++++++++++++++++++++++
 include/configs/iocon.h   |    3 +
 2 files changed, 217 insertions(+), 0 deletions(-)

diff --git a/board/gdsys/405ep/iocon.c b/board/gdsys/405ep/iocon.c
index 0ab8772..bf84ae8 100644
--- a/board/gdsys/405ep/iocon.c
+++ b/board/gdsys/405ep/iocon.c
@@ -38,6 +38,8 @@
 #include <pca953x.h>
 #include <pca9698.h>
 
+#include <miiphy.h>
+
 DECLARE_GLOBAL_DATA_PTR;
 
 #define LATCH0_BASE (CONFIG_SYS_LATCH_BASE)
@@ -95,8 +97,16 @@ enum {
 	MCFPGA_RESET_N = 1 << 4,
 };
 
+enum {
+	GPIO_MDC = 1 << 14,
+	GPIO_MDIO = 1 << 15,
+};
+
 unsigned int mclink_fpgacount;
 
+static int setup_88e1518(const char *bus, unsigned char addr);
+static int verify_88e1518(const char *bus, unsigned char addr);
+
 void fpga_set_reg(unsigned int fpga, u16 reg, u16 data)
 {
 	int res;
@@ -339,6 +349,7 @@ int last_stage_init(void)
 	int slaves;
 	unsigned int k;
 	unsigned char mclink_controllers[] = { 0x24, 0x25, 0x26 };
+	int legacy = get_fpga_state(0) & FPGA_STATE_PLATFORM;
 
 	print_fpga_info(0);
 	osd_probe(0);
@@ -360,6 +371,16 @@ int last_stage_init(void)
 		}
 	}
 
+	if (!legacy) {
+		miiphy_register(bb_miiphy_buses[0].name, bb_miiphy_read,
+				bb_miiphy_write);
+		if (!verify_88e1518(bb_miiphy_buses[0].name, 0)) {
+			printf("Fixup 88e1518 erratum on %s\n",
+			       bb_miiphy_buses[0].name);
+			setup_88e1518(bb_miiphy_buses[0].name, 0);
+		}
+	}
+
 	/* wait for slave-PLLs to be up and running */
 	udelay(500000);
 
@@ -375,6 +396,13 @@ int last_stage_init(void)
 	for (k = 1; k <= slaves; ++k) {
 		print_fpga_info(k);
 		osd_probe(k);
+		miiphy_register(bb_miiphy_buses[k].name,
+				bb_miiphy_read, bb_miiphy_write);
+		if (!verify_88e1518(bb_miiphy_buses[k].name, 0)) {
+			printf("Fixup 88e1518 erratum on %s\n",
+			       bb_miiphy_buses[k].name);
+			setup_88e1518(bb_miiphy_buses[k].name, 0);
+		}
 	}
 
 	return 0;
@@ -446,3 +474,189 @@ int gd405ep_get_fpga_done(unsigned fpga)
 	else
 		return pca9698_get_value(0x20, 20);
 }
+
+/*
+ * FPGA MII bitbang implementation
+ */
+
+struct fpga_mii {
+	unsigned fpga;
+	int mdio;
+} fpga_mii[] = {
+	{ 0, 1},
+	{ 1, 1},
+	{ 2, 1},
+	{ 3, 1},
+};
+
+static int mii_dummy_init(struct bb_miiphy_bus *bus)
+{
+	return 0;
+}
+
+static int mii_mdio_active(struct bb_miiphy_bus *bus)
+{
+	struct fpga_mii *fpga_mii = bus->priv;
+
+	if (fpga_mii->mdio)
+		fpga_set_reg(fpga_mii->fpga, REG(gpio.set), GPIO_MDIO);
+	else
+		fpga_set_reg(fpga_mii->fpga, REG(gpio.clear), GPIO_MDIO);
+
+	return 0;
+}
+
+static int mii_mdio_tristate(struct bb_miiphy_bus *bus)
+{
+	struct fpga_mii *fpga_mii = bus->priv;
+
+	fpga_set_reg(fpga_mii->fpga, REG(gpio.set), GPIO_MDIO);
+
+	return 0;
+}
+
+static int mii_set_mdio(struct bb_miiphy_bus *bus, int v)
+{
+	struct fpga_mii *fpga_mii = bus->priv;
+
+	if (v)
+		fpga_set_reg(fpga_mii->fpga, REG(gpio.set), GPIO_MDIO);
+	else
+		fpga_set_reg(fpga_mii->fpga, REG(gpio.clear), GPIO_MDIO);
+
+	fpga_mii->mdio = v;
+
+	return 0;
+}
+
+static int mii_get_mdio(struct bb_miiphy_bus *bus, int *v)
+{
+	struct fpga_mii *fpga_mii = bus->priv;
+
+	*v = ((fpga_get_reg(fpga_mii->fpga, REG(gpio.read)) & GPIO_MDIO) != 0);
+
+	return 0;
+}
+
+static int mii_set_mdc(struct bb_miiphy_bus *bus, int v)
+{
+	struct fpga_mii *fpga_mii = bus->priv;
+
+	if (v)
+		fpga_set_reg(fpga_mii->fpga, REG(gpio.set), GPIO_MDC);
+	else
+		fpga_set_reg(fpga_mii->fpga, REG(gpio.clear), GPIO_MDC);
+
+	return 0;
+}
+
+static int mii_delay(struct bb_miiphy_bus *bus)
+{
+	udelay(1);
+
+	return 0;
+}
+
+struct bb_miiphy_bus bb_miiphy_buses[] = {
+	{
+		.name = "trans1",
+		.init = mii_dummy_init,
+		.mdio_active = mii_mdio_active,
+		.mdio_tristate = mii_mdio_tristate,
+		.set_mdio = mii_set_mdio,
+		.get_mdio = mii_get_mdio,
+		.set_mdc = mii_set_mdc,
+		.delay = mii_delay,
+		.priv = &fpga_mii[0],
+	},
+	{
+		.name = "trans2",
+		.init = mii_dummy_init,
+		.mdio_active = mii_mdio_active,
+		.mdio_tristate = mii_mdio_tristate,
+		.set_mdio = mii_set_mdio,
+		.get_mdio = mii_get_mdio,
+		.set_mdc = mii_set_mdc,
+		.delay = mii_delay,
+		.priv = &fpga_mii[1],
+	},
+	{
+		.name = "trans3",
+		.init = mii_dummy_init,
+		.mdio_active = mii_mdio_active,
+		.mdio_tristate = mii_mdio_tristate,
+		.set_mdio = mii_set_mdio,
+		.get_mdio = mii_get_mdio,
+		.set_mdc = mii_set_mdc,
+		.delay = mii_delay,
+		.priv = &fpga_mii[2],
+	},
+	{
+		.name = "trans4",
+		.init = mii_dummy_init,
+		.mdio_active = mii_mdio_active,
+		.mdio_tristate = mii_mdio_tristate,
+		.set_mdio = mii_set_mdio,
+		.get_mdio = mii_get_mdio,
+		.set_mdc = mii_set_mdc,
+		.delay = mii_delay,
+		.priv = &fpga_mii[3],
+	},
+};
+
+int bb_miiphy_buses_num = sizeof(bb_miiphy_buses) /
+			  sizeof(bb_miiphy_buses[0]);
+
+/*
+ * Workaround for erratum mentioned in 88E1518 release notes
+ */
+
+static int verify_88e1518(const char *bus, unsigned char addr)
+{
+	u16 phy_id1, phy_id2;
+
+	if (miiphy_read(bus, addr, 2, &phy_id1) ||
+	    miiphy_read(bus, addr, 3, &phy_id2)) {
+		printf("Error reading from the PHY addr=%02x\n", addr);
+		return -EIO;
+	}
+
+	if ((phy_id1 != 0x0141) || ((phy_id2 & 0xfff0) != 0x0dd0))
+		return -EINVAL;
+
+	return 0;
+}
+
+struct regfix_88e1518 {
+	u8 reg;
+	u16 data;
+} regfix_88e1518[] = {
+	{ 22, 0x00ff },
+	{ 17, 0x214b },
+	{ 16, 0x2144 },
+	{ 17, 0x0c28 },
+	{ 16, 0x2146 },
+	{ 17, 0xb233 },
+	{ 16, 0x214d },
+	{ 17, 0xcc0c },
+	{ 16, 0x2159 },
+	{ 22, 0x00fb },
+	{  7, 0xc00d },
+	{ 22, 0x0000 },
+};
+
+static int setup_88e1518(const char *bus, unsigned char addr)
+{
+	unsigned int k;
+
+	for (k = 0; k < ARRAY_SIZE(regfix_88e1518); ++k) {
+		if (miiphy_write(bus, addr,
+				 regfix_88e1518[k].reg,
+				 regfix_88e1518[k].data)) {
+			printf("Error writing to the PHY addr=%02x\n", addr);
+			return -1;
+		}
+	}
+
+	return 0;
+}
diff --git a/include/configs/iocon.h b/include/configs/iocon.h
index 921ac05..d5f48d3 100644
--- a/include/configs/iocon.h
+++ b/include/configs/iocon.h
@@ -295,4 +295,7 @@ int fpga_gpio_get(unsigned int bus, int pin);
 #define CONFIG_SYS_CH7301
 #define CONFIG_SYS_OSD_SCREENS		1
 
+#define CONFIG_BITBANGMII		/* bit-bang MII PHY management */
+#define CONFIG_BITBANGMII_MULTI
+
 #endif	/* __CONFIG_H */
-- 
1.7.2.5

[U-Boot] [PATCH v2 08/10] powerpc/ppc4xx: Increase timeout for gdsys mclink bus startup

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---
Changes in v2: None

 board/gdsys/common/mclink.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/board/gdsys/common/mclink.c b/board/gdsys/common/mclink.c
index 9a4f3e9..d2983bb 100644
--- a/board/gdsys/common/mclink.c
+++ b/board/gdsys/common/mclink.c
@@ -40,11 +40,11 @@ enum {
 int mclink_probe(void)
 {
 	unsigned int k;
-	unsigned int ctr = 0;
 	int slaves = 0;
 
 	for (k = 0; k < CONFIG_SYS_MCLINK_MAX; ++k) {
 		int timeout = 0;
+		unsigned int ctr = 0;
 
 		if (!(fpga_get_reg(k, REG(mc_status)) & (1 << 15)))
 			break;
@@ -53,7 +53,7 @@ int mclink_probe(void)
 
 		while (!(fpga_get_reg(k, REG(mc_status)) & (1 << 14))) {
 			udelay(100);
-			if (ctr++ > 3) {
+			if (ctr++ > 500) {
 				timeout = 1;
 				break;
 			}
@@ -61,6 +61,8 @@ int mclink_probe(void)
 		if (timeout)
 			break;
 
+		printf("waited %d us for mclink %d to come up\n", ctr * 100, k);
+
 		slaves++;
 	}
 
-- 
1.7.2.5

[U-Boot] [PATCH v2 09/10] powerpc/ppc4xx: Consider gdsys FPGA OSD size

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

OSD size was constant 32x16 characters.
Now the size is set as announced by the FPGA.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>
---
Changes in v2: None

 board/gdsys/common/osd.c |   34 ++++++++++++++++++----------------
 1 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/board/gdsys/common/osd.c b/board/gdsys/common/osd.c
index 2cfbe58..9ce51cf 100644
--- a/board/gdsys/common/osd.c
+++ b/board/gdsys/common/osd.c
@@ -43,10 +43,6 @@
 
 #define PIXCLK_640_480_60 25180000
 
-#define BASE_WIDTH 32
-#define BASE_HEIGHT 16
-#define BUFSIZE (BASE_WIDTH * BASE_HEIGHT)
-
 enum {
 	CH7301_CM = 0x1c,		/* Clock Mode Register */
 	CH7301_IC = 0x1d,		/* Input Clock Register */
@@ -68,6 +64,11 @@ enum {
 	CH7301_DSP = 0x56,		/* DVI Sync polarity Register */
 };
 
+unsigned int base_width;
+unsigned int base_height;
+size_t bufsize;
+u16 *buf;
+
 unsigned int max_osd_screen = CONFIG_SYS_OSD_SCREENS - 1;
 
 #ifdef CONFIG_SYS_CH7301
@@ -269,7 +270,7 @@ static int osd_write_videomem(unsigned screen, unsigned offset,
 	unsigned int k;
 
 	for (k = 0; k < charcount; ++k) {
-		if (offset + k >= BUFSIZE)
+		if (offset + k >= bufsize)
 			return -1;
 		fpga_set_reg(screen, REG(videomem) + sizeof(u16) * (offset + k),
 			     data[k]);
@@ -289,7 +290,6 @@ static int osd_print(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		unsigned len;
 		u8 color;
 		unsigned int k;
-		u16 buf[BUFSIZE];
 		char *text;
 		int res;
 
@@ -303,12 +303,12 @@ static int osd_print(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		color = simple_strtoul(argv[3], NULL, 16);
 		text = argv[4];
 		charcount = strlen(text);
-		len = (charcount > BUFSIZE) ? BUFSIZE : charcount;
+		len = (charcount > bufsize) ? bufsize : charcount;
 
 		for (k = 0; k < len; ++k)
 			buf[k] = (text[k] << 8) | color;
 
-		res = osd_write_videomem(screen, y * BASE_WIDTH + x, buf, len);
+		res = osd_write_videomem(screen, y * base_width + x, buf, len);
 		if (res < 0)
 			return res;
 	}
@@ -320,18 +320,20 @@ int osd_probe(unsigned screen)
 {
 	u16 version = fpga_get_reg(screen, REG(osd.version));
 	u16 features = fpga_get_reg(screen, REG(osd.features));
-	unsigned width;
-	unsigned height;
 	u8 value;
 #ifdef CONFIG_SYS_CH7301
 	int old_bus = i2c_get_bus_num();
 #endif
 
-	width = ((features & 0x3f00) >> 8) + 1;
-	height = (features & 0x001f) + 1;
+	base_width = ((features & 0x3f00) >> 8) + 1;
+	base_height = (features & 0x001f) + 1;
+	bufsize = base_width * base_height;
+	buf = malloc(sizeof(u16) * bufsize);
+	if (!buf)
+		return -1;
 
 	printf("OSD%d:  Digital-OSD version %01d.%02d, %d" "x%d characters\n",
-		screen, version/100, version%100, width, height);
+		screen, version/100, version%100, base_width, base_height);
 
 #ifdef CONFIG_SYS_CH7301
 	i2c_set_bus_num(ch7301_i2c[screen]);
@@ -397,7 +399,7 @@ int osd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		unsigned x;
 		unsigned y;
 		unsigned k;
-		u16 buffer[BASE_WIDTH];
+		u16 buffer[base_width];
 		char *rp;
 		u16 *wp = buffer;
 		unsigned count = (argc > 4) ?
@@ -422,13 +424,13 @@ int osd_write(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 
 			rp += 4;
 			wp++;
-			if (wp - buffer > BASE_WIDTH)
+			if (wp - buffer > base_width)
 				break;
 		}
 
 		for (k = 0; k < count; ++k) {
 			unsigned offset =
-				y * BASE_WIDTH + x + k * (wp - buffer);
+				y * base_width + x + k * (wp - buffer);
 			osd_write_videomem(screen, offset, buffer,
 				wp - buffer);
 		}
-- 
1.7.2.5

[U-Boot] [PATCH v2 10/10] powerpc/ppc4xx: Remove CONFIG_SYS_FLASH_PROTECTION from gdsys boards

[dirk.eibach at gdsys.cc]

From: Dirk Eibach <eibach@gdsys.de>

CONFIG_SYS_FLASH_PROTECTION was active on most gdsys boards by default,
while hardware flash protection was not implemented.
Hardware support was added recently and we get into trouble because backward
compatibility is broken (u-boot can't unprotect the protected flash after a
downgrade). So we decided to disable hardware flash protection for all our boards.

Signed-off-by: Dirk Eibach <dirk.eibach@gdsys.cc>

---
Changes in v2: None

 include/configs/dlvision-10g.h |    3 +--
 include/configs/dlvision.h     |    3 +--
 include/configs/io.h           |    3 +--
 include/configs/iocon.h        |    3 +--
 include/configs/neo.h          |    3 +--
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/include/configs/dlvision-10g.h b/include/configs/dlvision-10g.h
index 5d6308f..6528055 100644
--- a/include/configs/dlvision-10g.h
+++ b/include/configs/dlvision-10g.h
@@ -34,7 +34,7 @@
  * Include common defines/options for all AMCC eval boards
  */
 #define CONFIG_HOSTNAME		dlvsion-10g
-#define CONFIG_IDENT_STRING	" dlvision-10g 0.03"
+#define CONFIG_IDENT_STRING	" dlvision-10g 0.04"
 #include "amcc-common.h"
 
 #define CONFIG_BOARD_EARLY_INIT_F
@@ -161,7 +161,6 @@
 #define CONFIG_SYS_FLASH_WRITE_TOUT	500	/* Timeout for Flash Write/ms */
 
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE 1	/* use buff'd writes */
-#define CONFIG_SYS_FLASH_PROTECTION	1	/* use hardware flash protect */
 
 #define CONFIG_SYS_FLASH_EMPTY_INFO	/* 'E' for empty sector on flinfo */
 #define CONFIG_SYS_FLASH_QUIET_TEST	1	/* no warn upon unknown flash */
diff --git a/include/configs/dlvision.h b/include/configs/dlvision.h
index 24c9fa4..948930e 100644
--- a/include/configs/dlvision.h
+++ b/include/configs/dlvision.h
@@ -34,7 +34,7 @@
  * Include common defines/options for all AMCC eval boards
  */
 #define CONFIG_HOSTNAME		dlvision
-#define CONFIG_IDENT_STRING	" dlvision 0.01"
+#define CONFIG_IDENT_STRING	" dlvision 0.02"
 #include "amcc-common.h"
 
 #define CONFIG_BOARD_EARLY_INIT_F	/* call board_early_init_f */
@@ -125,7 +125,6 @@
 #define CONFIG_SYS_FLASH_WRITE_TOUT	500	/* Timeout for Flash Write/ms */
 
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE 1	/* use buff'd writes */
-#define CONFIG_SYS_FLASH_PROTECTION	1	/* use hardware flash protect */
 
 #define CONFIG_SYS_FLASH_EMPTY_INFO	/* 'E' for empty sector on flinfo */
 #define CONFIG_SYS_FLASH_QUIET_TEST	1	/* no warn upon unknown flash */
diff --git a/include/configs/io.h b/include/configs/io.h
index 4950ad7..0985cf0 100644
--- a/include/configs/io.h
+++ b/include/configs/io.h
@@ -34,7 +34,7 @@
  * Include common defines/options for all AMCC eval boards
  */
 #define CONFIG_HOSTNAME		io
-#define CONFIG_IDENT_STRING	" io 0.05"
+#define CONFIG_IDENT_STRING	" io 0.06"
 #include "amcc-common.h"
 
 #define CONFIG_BOARD_EARLY_INIT_F
@@ -139,7 +139,6 @@
 #define CONFIG_SYS_FLASH_WRITE_TOUT	500	/* Timeout for Flash Write/ms */
 
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE 1	/* use buff'd writes */
-#define CONFIG_SYS_FLASH_PROTECTION	1	/* use hardware flash protect */
 
 #define CONFIG_SYS_FLASH_EMPTY_INFO	/* 'E' for empty sector on flinfo */
 #define CONFIG_SYS_FLASH_QUIET_TEST	1	/* no warn upon unknown flash */
diff --git a/include/configs/iocon.h b/include/configs/iocon.h
index d5f48d3..4ac785a 100644
--- a/include/configs/iocon.h
+++ b/include/configs/iocon.h
@@ -34,7 +34,7 @@
  * Include common defines/options for all AMCC eval boards
  */
 #define CONFIG_HOSTNAME		iocon
-#define CONFIG_IDENT_STRING	" iocon 0.04"
+#define CONFIG_IDENT_STRING	" iocon 0.05"
 #include "amcc-common.h"
 
 #define CONFIG_BOARD_EARLY_INIT_F
@@ -181,7 +181,6 @@ int fpga_gpio_get(unsigned int bus, int pin);
 #define CONFIG_SYS_FLASH_WRITE_TOUT	500	/* Timeout for Flash Write/ms */
 
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE 1	/* use buff'd writes */
-#define CONFIG_SYS_FLASH_PROTECTION	1	/* use hardware flash protect */
 
 #define CONFIG_SYS_FLASH_EMPTY_INFO	/* 'E' for empty sector on flinfo */
 #define CONFIG_SYS_FLASH_QUIET_TEST	1	/* no warn upon unknown flash */
diff --git a/include/configs/neo.h b/include/configs/neo.h
index be1c9ff..e1b832a 100644
--- a/include/configs/neo.h
+++ b/include/configs/neo.h
@@ -35,7 +35,7 @@
  * Include common defines/options for all AMCC eval boards
  */
 #define CONFIG_HOSTNAME		neo
-#define CONFIG_IDENT_STRING	" neo 0.01"
+#define CONFIG_IDENT_STRING	" neo 0.02"
 #include "amcc-common.h"
 
 #define CONFIG_BOARD_EARLY_INIT_F
@@ -146,7 +146,6 @@
 #define CONFIG_SYS_FLASH_WRITE_TOUT	500	/* Timeout for Flash Write (in ms)	*/
 
 #define CONFIG_SYS_FLASH_USE_BUFFER_WRITE 1	/* use buffered writes (20x faster)	*/
-#define CONFIG_SYS_FLASH_PROTECTION	1	/* use hardware flash protection	*/
 
 #define CONFIG_SYS_FLASH_EMPTY_INFO		/* print 'E' for empty sector on flinfo */
 #define CONFIG_SYS_FLASH_QUIET_TEST	1	/* don't warn upon unknown flash	*/
-- 
1.7.2.5

[U-Boot] [PATCH v2 0/10] Update gdsys ppc4xx-based boards

[Wolfgang Denk]

Dear dirk.eibach at gdsys.cc,

In message <1367847325-21463-1-git-send-email-dirk.eibach@gdsys.cc> you wrote:
> 
>   powerpc/ppc4xx: Add generic accessor functions for gdsys FPGA
>   powerpc/ppc4xx: Add gdsys mclink interface
>   powerpc/ppc4xx: Add fpgad command for dumping gdsys fpga registers
>   powerpc/ppc4xx: Use generic FPGA accessors in gdsys common code
>   powerpc/ppc4xx: Support gdsys multichannel iocon hardware
>   powerpc/ppc4xx: Use generic FPGA accessors on all gdsys 405ep/ex
>     boards
>   powerpc/ppc4xx: Fixup phy erratum on gdsys iocon hardware
>   powerpc/ppc4xx: Increase timeout for gdsys mclink bus startup
>   powerpc/ppc4xx: Consider gdsys FPGA OSD size
>   powerpc/ppc4xx: Remove CONFIG_SYS_FLASH_PROTECTION from gdsys boards

This sequence of patches is not logically ordered, and split into
patches at random. Please group stuff the belongs together, and squash
patches theat define things with those that actually use it.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A man is not complete until he is married -- then he is finished.