[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