[U-Boot] [PATCH v1 6/6] mpc83xx: Add gdsys hrcon board

[Kim Phillips]

On Wed, 29 Oct 2014 16:03:57 +0100
<dirk.eibach at gdsys.cc> wrote:

> From: Dirk Eibach <dirk.eibach at gdsys.cc>
> 
> The gdsys hrcon board is based on a Freescale MPC8308 SOC.
> It boots from NOR-Flash, kernel and rootfs are stored on
> SD-Card.
> 
> On board peripherals include:
> - 1x GbE (optional)
> - Lattice ECP3 FPGA connected via eLBC and PCIe
> 
> Signed-off-by: Dirk Eibach <dirk.eibach at gdsys.cc>
> ---

sorry for the delay, I bricked a board when going through my queue
lately, and haven't been able to fully recover since.

>  arch/powerpc/cpu/mpc83xx/Kconfig |   4 +
>  board/gdsys/405ep/iocon.c        | 190 +----------
>  board/gdsys/common/Makefile      |   3 +-
>  board/gdsys/common/ihs_mdio.c    |  88 +++++
>  board/gdsys/common/ihs_mdio.h    |  18 ++
>  board/gdsys/common/phy.c         | 280 ++++++++++++++++
>  board/gdsys/common/phy.h         |  14 +

is it me, or should PHY support go under drivers/net/phy/gdsys.c (or
something like that)?  Even if not immediately ported to the Generic
PHY Management layer, still, I think it would be a good idea to get
it in the right vicinity.

Taking a closer look, it looks like at least one of the PHYs
involved here (the Marvell 88E1518) is already implemented in
drivers/phy/marvell.c to a certain degree, so it might be helpful to
define CONFIG_PHY_MARVELL as a first step to migrating to the
generic PHY subsystem.

>  board/gdsys/mpc8308/Kconfig      |  12 +
>  board/gdsys/mpc8308/MAINTAINERS  |   6 +
>  board/gdsys/mpc8308/Makefile     |   9 +
>  board/gdsys/mpc8308/hrcon.c      | 677 +++++++++++++++++++++++++++++++++++++++
>  board/gdsys/mpc8308/mpc8308.c    | 109 +++++++
>  board/gdsys/mpc8308/mpc8308.h    |  10 +
>  board/gdsys/mpc8308/sdram.c      |  82 +++++

>  common/Makefile                  |   1 +
>  common/cmd_ioloop.c              | 295 +++++++++++++++++

IDK what this is (FPGA io-endpoint looper/reflector?), but I'm
guessing since it's in common/, it should be separated from this
board support patch, otherwise it won't get the intended audience's
attention.

> +int last_stage_init(void)
> +{
> +	int slaves;
> +	unsigned int k;
> +	unsigned int mux_ch;
> +	unsigned char mclink_controllers[] = { 0x24, 0x25, 0x26 };
> +	u16 fpga_features;
> +	bool hw_type_cat = pca9698_get_value(0x20, 20);
> +	bool ch0_rgmii2_present = false;
> +
> +	FPGA_GET_REG(0, fpga_features, &fpga_features);
> +
> +	/* Turn on Parade DP501 */
> +	pca9698_direction_output(0x20, 10, 1);
> +
> +	ch0_rgmii2_present = !pca9698_get_value(0x20, 30);
> +
> +	/* 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;
> +			}
> +		}
> +	}
> +
> +	if (hw_type_cat) {
> +		miiphy_register(bb_miiphy_buses[0].name, bb_miiphy_read,
> +				bb_miiphy_write);
> +		for (mux_ch = 0; mux_ch < MAX_MUX_CHANNELS; ++mux_ch) {
> +			if ((mux_ch == 1) && !ch0_rgmii2_present)
> +				continue;
> +
> +			setup_88e1514(bb_miiphy_buses[0].name, mux_ch);
> +		}
> +	}
> +
> +	/* give slave-PLLs and Parade DP501 some time to be up and running */
> +	udelay(500000);
> +
> +	mclink_fpgacount = CONFIG_SYS_MCLINK_MAX;
> +	slaves = mclink_probe();
> +	mclink_fpgacount = 0;
> +
> +	print_fpga_info(0, ch0_rgmii2_present);
> +	osd_probe(0);
> +	return 0;

unless this was left in from debugging (in which case it should be
removed), it implies the remaining code in the fn..:

> +
> +	if (slaves <= 0)
> +		return 0;
> +
> +	mclink_fpgacount = slaves;
> +
> +	for (k = 1; k <= slaves; ++k) {
> +		FPGA_GET_REG(k, fpga_features, &fpga_features);
> +
> +		print_fpga_info(k, false);
> +		osd_probe(k);
> +		if (hw_type_cat) {
> +			miiphy_register(bb_miiphy_buses[k].name,
> +					bb_miiphy_read, bb_miiphy_write);
> +			setup_88e1514(bb_miiphy_buses[k].name, 0);
> +		}
> +	}
> +
> +	return 0;
> +}

..is dead code, and therefore not welcome here.

> +int board_early_init_r(void)
> +{
> +	unsigned k;
> +	unsigned ctr;
> +
> +	for (k = 0; k < CONFIG_SYS_FPGA_COUNT; ++k)
> +		gd->arch.fpga_state[k] = 0;
> +
> +	/*
> +	 * reset FPGA
> +	 */
> +	mpc8308_init();
> +
> +	mpc8308_set_fpga_reset(1);
> +
> +	mpc8308_setup_hw();
> +
> +	for (k = 0; k < CONFIG_SYS_FPGA_COUNT; ++k) {
> +		ctr = 0;
> +		while (!mpc8308_get_fpga_done(k)) {
> +			udelay(100000);
> +			if (ctr++ > 5) {
> +				gd->arch.fpga_state[k] |=
> +					FPGA_STATE_DONE_FAILED;
> +				break;
> +			}
> +		}
> +	}
> +
> +	udelay(10);
> +
> +	mpc8308_set_fpga_reset(0);
> +
> +	for (k = 0; k < CONFIG_SYS_FPGA_COUNT; ++k) {
> +		/*
> +		 * wait for fpga out of reset
> +		 */
> +		ctr = 0;
> +		while (1) {
> +			u16 val;
> +
> +			FPGA_SET_REG(k, reflection_low, REFLECTION_TESTPATTERN);
> +
> +			FPGA_GET_REG(k, REFLECTION_TESTREG, &val);
> +			if (val == REFLECTION_TESTPATTERN_INV)
> +				break;
> +
> +			udelay(100000);
> +			if (ctr++ > 5) {
> +				gd->arch.fpga_state[k] |=
> +					FPGA_STATE_REFLECTION_FAILED;
> +				break;
> +			}
> +		}
> +	}

should these functions have timeouts?  not sure if there's anything
useful to do if they do timeout...

> +/* Fixed sdram init -- doesn't use serial presence detect.
> + *
> + * This is useful for faster booting in configs where the RAM is unlikely
> + * to be changed, or for things like NAND booting where space is tight.
> + */

this is ok if the board has its RAM soldered-on: does it?

> diff --git a/common/Makefile b/common/Makefile
> index b19d379..b5ed9ae 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -119,6 +119,7 @@ obj-$(CONFIG_CMD_HASH) += cmd_hash.o
>  obj-$(CONFIG_CMD_IDE) += cmd_ide.o
>  obj-$(CONFIG_CMD_IMMAP) += cmd_immap.o
>  obj-$(CONFIG_CMD_INI) += cmd_ini.o
> +COBJS-$(CONFIG_CMD_IOLOOP) += cmd_ioloop.o
>  obj-$(CONFIG_CMD_IRQ) += cmd_irq.o
>  obj-$(CONFIG_CMD_ITEST) += cmd_itest.o
>  obj-$(CONFIG_CMD_JFFS2) += cmd_jffs2.o

hmm, does obj-$... not work?

Thanks,

Kim