[U-Boot-Users] [PATCH] Add MVBC_P board

[Kim Phillips]

On Fri, 04 Jul 2008 09:42:24 +0200
Andre Schwarz <andre.schwarz at matrix-vision.de> wrote:

Hello Andre,

>  board/mvbc_p/fpga.c            |  177 ++++++++++++++++++++
>  board/mvbc_p/fpga.h            |   34 ++++

couldn't help but notice this file is equal to board/mvblm7/fpga.c.
Perhaps it's time to add your board/$(VENDOR)/common directory and put
both this file and its header there.  This way you can make your code
more maintainable by avoiding duplicating it all over the place.

> +	@$(MKCONFIG) -n $@ -a MVBC_P ppc mpc5xxx mvbc_p

assuming $VENDOR == matrix-vision or something (your choice), you'd
have to modify the above line like so:

-	@$(MKCONFIG) -n $@ -a MVBC_P ppc mpc5xxx mvbc_p
+	@$(MKCONFIG) -n $@ -a MVBC_P ppc mpc5xxx mvbc_p matrix-vision

to enable building your (now a single copy) fpga.c.

> +#ifdef CONFIG_OF_LIBFDT
> +#include <fdt_support.h>
> +#endif

it'd be nice to get rid of ifdeffing CONFIG_OF_LIBFDT all over the
place, assuming, of course, you won't be supporting booting a
non-fdt-aware OS.

> +	gpio->simple_ddr = SIMPLE_DDR;
> +	gpio->simple_dvo = SIMPLE_DVO;
> +	gpio->simple_ode = SIMPLE_ODE;
> +	gpio->simple_gpioe = SIMPLE_GPIOEN;
> +
> +	gpio->sint_ode = SINT_ODE;
> +	gpio->sint_ddr = SINT_DDR;
> +	gpio->sint_dvo = SINT_DVO;
> +	gpio->sint_inten = SINT_INTEN;
> +	gpio->sint_itype = SINT_ITYPE;
> +	gpio->sint_gpioe = SINT_GPIOEN;
> +
> +	*(vu_char *)MPC5XXX_WU_GPIO_ODE	= WKUP_ODE;
> +	*(vu_char *)MPC5XXX_WU_GPIO_DIR	= WKUP_DIR;
> +	*(vu_char *)MPC5XXX_WU_GPIO_DATA_O = WKUP_DO | ARB_X_EN;
> +	*(vu_char *)MPC5XXX_WU_GPIO_ENABLE = WKUP_EN;
> +
> +	printf("simple_gpioe: 0x%08x\n", gpio->simple_gpioe);
> +	printf("sint_gpioe  : 0x%08x\n", gpio->sint_gpioe);
> +	__asm__ volatile ("sync");
> +}

same comment Wolfgang made; use in_* out_* accessor fns.

> +void hw_watchdog_reset(void)
> +{
> +	*(u8*) (0xff000005) = 0;

is this a magic number/needs a #define somewhere? 

> +#define MV_FPGA_DATA		"0xff860000"
> +#define MV_FPGA_SIZE		"0x3c886"
> +#define MV_KERNEL_ADDR		"0xffc00000"
> +#define MV_INITRD_ADDR		"0xff900000"
> +#define MV_INITRD_LENGTH	"0x00300000"
> +#define MV_SCRATCH_ADDR		"0x00000000"
> +#define MV_SCRATCH_LENGTH	MV_INITRD_LENGTH
> +#define MV_AUTOSCR_ADDR		"0xff840000"
> +#define MV_AUTOSCR_ADDR2	"0xff850000"
> +#define MV_DTB_ADDR		"0xfffc0000"

please use MK_STR (see other config files, e.g. MPC8313).

> +
> +#define CONFIG_SHOW_BOOT_PROGRESS 1
> +
> +#define MV_KERNEL_ADDR_RAM	"0x00100000"
> +#define MV_DTB_ADDR_RAM		"0x00600000"
> +#define MV_INITRD_ADDR_RAM	"0x01000000"
> +
> +/* pass open firmware flat tree */
> +#define CONFIG_OF_LIBFDT	1
> +#define CONFIG_OF_BOARD_SETUP	1
> +
> +#define OF_CPU			"PowerPC,5200 at 0"
> +#define OF_SOC			"soc5200 at f0000000"
> +#define OF_TBCLK		(bd->bi_busfreq / 4)

I thought we had done away with the above three (FLAT_TREE now
obsolete).  Oh, I see now: 5xxx still uses it in LIBFDT code.  Bad 5xxx!

Kim