Dear Jon, In message <20090323210934.660.61291.st...@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 <[email protected]> > > --- ^^^^^ 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: [email protected] "Beware of programmers carrying screwdrivers." - Chip Salzenberg _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
