[U-Boot] [PATCH 2] board support patch for phyCORE-MPC5200B-tiny

Wolfgang Denk wd at denx.de
Tue Mar 24 00:27:07 CET 2009


Dear Jon,

In message <20090323210934.660.61291.stgit at localhost> you wrote:
> Add support for the Phytec phyCORE-MPC5200B-tiny. This code is from Pengutronix.de but they
> didn't sign the patch. I have updated it from u-boot 1.2 to work on u-boot master. Edited to
> reflect feedback received.

Lines too long.

And things like "Edited to reflect feedback received." don't belong
into the commit message (put them under the "---" line below.)

> Signed-off-by: Jon Smirl <jonsmirl at gmail.com>
> 
> ---
^^^^^
Here is where comments can go.


>  Makefile                            |   10 +
>  board/phytec/pcm030/Makefile        |   50 ++++
>  board/phytec/pcm030/config.mk       |   42 +++
>  board/phytec/pcm030/mt46v32m16-75.h |   54 ++++
>  board/phytec/pcm030/pcm030.c        |  289 +++++++++++++++++++++++
>  cpu/mpc5xxx/ide.c                   |    3 
>  include/configs/pcm030.h            |  438 +++++++++++++++++++++++++++++++++++
>  7 files changed, 886 insertions(+), 0 deletions(-)
>  create mode 100644 board/phytec/pcm030/Makefile
>  create mode 100644 board/phytec/pcm030/config.mk
>  create mode 100644 board/phytec/pcm030/mt46v32m16-75.h
>  create mode 100644 board/phytec/pcm030/pcm030.c
>  create mode 100644 include/configs/pcm030.h
> 
> diff --git a/Makefile b/Makefile
> index ba6a602..fab0347 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -668,6 +668,16 @@ o2dnt_config:	unconfig
>  pf5200_config:	unconfig
>  	@$(MKCONFIG) pf5200  ppc mpc5xxx pf5200 esd
>  
> +pcm030_config \
> +pcm030_LOWBOOT_config:	unconfig
> +	@ >include/config.h
> +	@[ -z "$(findstring LOWBOOT_,$@)" ] || \
> +		{ echo "TEXT_BASE = 0xFF000000"	>board/phytec/pcm030/config.tmp ; \
> +		  echo "... with LOWBOOT configuration" ; \
> +		}
> +	@$(MKCONFIG) -a pcm030 ppc mpc5xxx pcm030 phytec
> +	@ echo "remember to set pcm030_REV to 0 for rev 1245.0 rev or to 1 for rev 1245.1"
> +

Please keep list sorted. "pc" goes before "pf".


> diff --git a/board/phytec/pcm030/config.mk b/board/phytec/pcm030/config.mk
> new file mode 100644
> index 0000000..bb01639
> --- /dev/null
> +++ b/board/phytec/pcm030/config.mk
...
> +PLATFORM_CPPFLAGS += -DTEXT_BASE=$(TEXT_BASE) -I$(TOPDIR)/board
> +LDSCRIPT := $(SRCTREE)/cpu/mpc5xxx/u-boot.lds

Please remove this line as it is redundant.

...
> +#define SDRAM_TAPDELAY	0x10000000 /* reserved Bit in MPC5200 B3-Step */
> diff --git a/board/phytec/pcm030/pcm030.c b/board/phytec/pcm030/pcm030.c
> new file mode 100644
> index 0000000..39bd4c4
> --- /dev/null
> +++ b/board/phytec/pcm030/pcm030.c
...
> +	/* set CDM clock enable register, set MPC5200B SDRAM bus to reduced driver strength */
> +	out_be32 ((unsigned __iomem *)MPC5XXX_CDM_CLK_ENA, (0x00CFFFFF));
> +}

Line too long.

> +#endif
> +
> +/*
> + * ATTENTION: Although partially referenced initdram does NOT make real use
> + *            use of CONFIG_SYS_SDRAM_BASE. The code does not work if CONFIG_SYS_SDRAM_BASE
> + *            is something else than 0x00000000.
> + */

Line too long.

> +phys_size_t initdram(int board_type)
> +{
> +	ulong dramsize = 0;
> +	ulong dramsize2 = 0;
> +#ifndef CONFIG_SYS_RAMBOOT
> +	ulong test1, test2;
> +
> +	/* setup SDRAM chip selects */
> +	out_be32 ((unsigned __iomem *)MPC5XXX_SDRAM_CS0CFG, 0x0000001b);	/* 256MB at 0x0 */
> +	out_be32 ((unsigned __iomem *)MPC5XXX_SDRAM_CS1CFG, 0x10000000);	/* disabled */

Line too long.

...
> +	if (dramsize > 0)
> +		out_be32 ((unsigned __iomem *)MPC5XXX_SDRAM_CS0CFG,
> +		    (0x13 + __builtin_ffs(dramsize >> 20) - 1));
> +	else
> +		out_be32 ((unsigned __iomem *)MPC5XXX_SDRAM_CS0CFG, 0);	/* disabled */

Line too long.

...
> +void ide_set_reset(int idereset)
> +{
> +	debug("ide_reset(%d)\n", idereset);
> +
> +	if (idereset) {
> +		clrbits_be32((unsigned __iomem *)MPC5XXX_WU_GPIO_DATA_O, GPIO_PSC2_4);

Line too long.

> +		/* Make a delay. MPC5200 spec says 25 usec min */
> +		udelay(500000);
> +	} else
> +		setbits_be32((unsigned __iomem *)MPC5XXX_WU_GPIO_DATA_O, GPIO_PSC2_4);

Line too long.

...
> +void video_get_info_str(int line_number, char *info)
> +{
> +	if (line_number == 1)
> +		strcpy(info,
> +		       " Board: phyCORE-MPC5200B tiny (Phytec Messtechnik GmbH)");

Line too long.

> +	else if (line_number == 2)
> +		strcpy(info, "    on a PCM-980 baseboard");
> +	else
> +		info[0] = '\0';
> +}
> +#endif
> +
> +/*
> + * Returns OPENIP register base address. First thing called in the driver.
> + */
> +unsigned int board_video_init(void)
> +{
> +	ulong dummy;
> +	dummy = in_be32((unsigned __iomem *)OPENIP_MMIO_BASE);	/*dummy read */
> +	dummy = in_be32((unsigned __iomem *)OPENIP_MMIO_BASE);	/*dummy read */
> +	return OPENIP_MMIO_BASE;
> +}

I asked befor what these dummy reads are supposed to be good for.

This looks like a sleeping problem to me.


...
> +/* To build RAMBOOT, add this to the main Makefile
> +@[ -z "$(findstring RAMBOOT_,$@)" ] || \
> +       { echo "TEXT_BASE = 0x00100000" >board/phycore_mpc5200b_tiny/config.tmp ; \
> +         echo "... with RAMBOOT configuration" ; \
> +         echo "... remember to make sure that MBAR is already switched to 0xF0000000 !!!" ; \
> +       }
> +*/

Lines way too long.

> +#define CONFIG_BOARDINFO	 "Phytec Phycore mpc5200b tiny"
> +
> +/*-----------------------------------------------------------------------------
> +High Level Configuration Options
> +(easy to change)
> +-----------------------------------------------------------------------------*/
> +#define CONFIG_MPC5xxx		1	/* This is an MPC5xxx CPU */
> +#define CONFIG_MPC5200		1	/* (more precisely an MPC5200 CPU) */
> +#define CONFIG_MPC5200_DDR	1	/* (with DDR-SDRAM) */
> +#define CONFIG_PHYCORE_MPC5200B_TINY 1	/* phyCORE-MPC5200B -> FEC configuration and IDE */
> +#define CONFIG_SYS_MPC5XXX_CLKIN 33333333	/* ... running at 33.333333MHz */
> +#define BOOTFLAG_COLD		0x01	/* Normal Power-On: Boot from FLASH  */
> +#define BOOTFLAG_WARM		0x02	/* Software reboot           */
> +
> +/*-----------------------------------------------------------------------------
> +Serial console configuration
> +-----------------------------------------------------------------------------*/
> +#define CONFIG_PSC_CONSOLE	3	/* console is on PSC3 -> define gps port conf. */
> +					/* register later on to enable UART function! */
> +#define CONFIG_BAUDRATE		115200	/* ... at 115200 bps */
> +#define CONFIG_SYS_BAUDRATE_TABLE { 9600, 19200, 38400, 57600, 115200, 230400 }

Lines way too long.

> +#define MTDIDS_DEFAULT 		"nor0=physmap-flash.0"
> +#define MTDPARTS_DEFAULT   	"mtdparts=physmap-flash.0:256k(ubootl),1792k(kernel),13312k(jffs2),256k(uboot)ro,256k(oftree),-(space)"

Lines way too long. Many more long lines following. Please fix them
all.


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
"Beware of programmers carrying screwdrivers."      - Chip Salzenberg


More information about the U-Boot mailing list