Dear Heiko Schocher, In message <4a0691fe.6050...@denx.de> you wrote: > > > + /* > > + * Configure Flash Speed > > + */ > > + out_be32(&im->lpc.cs_cfg[0], CONFIG_SYS_CS0_CFG); > > + > > + spridr = in_be32(&im->sysconf.spridr); > > + > > no blank line necessary.
Not necessary, but I find it easier to read. > > + u32 msize = 0; > > + > > + msize = fixed_sdram (); > > + > > no blank line necessary. > > > + return msize; Same here. ... > > + out_be32(&im->mddrc.lut_table1_alternate_lower, > > CONFIG_SYS_MDDRCGRP_LUT1_AL); > > + out_be32(&im->mddrc.lut_table2_alternate_upper, > > CONFIG_SYS_MDDRCGRP_LUT2_AU); > > + out_be32(&im->mddrc.lut_table2_alternate_lower, > > CONFIG_SYS_MDDRCGRP_LUT2_AL); > > + out_be32(&im->mddrc.lut_table3_alternate_upper, > > CONFIG_SYS_MDDRCGRP_LUT3_AU); > > + out_be32(&im->mddrc.lut_table3_alternate_lower, > > CONFIG_SYS_MDDRCGRP_LUT3_AL); > > + out_be32(&im->mddrc.lut_table4_alternate_upper, > > CONFIG_SYS_MDDRCGRP_LUT4_AU); > > + out_be32(&im->mddrc.lut_table4_alternate_lower, > > CONFIG_SYS_MDDRCGRP_LUT4_AL); > > too long lines ... but 2 lines would also look ugly ... I tried splitting it in too lines, but it looks *really* ugly. As it's just 85 columns, I decided to leave it that way. > > +#ifdef CONFIG_FSL_DIU_FB > > +#if !(defined(CONFIG_VIDEO) || defined(CONFIG_CFB_CONSOLE)) > > + mpc5121_diu_init(); > > +#endif > > +#endif > > + > > no blank line necessary. Same as above. > > + iopin_initialize(ioregs_init, sizeof(ioregs_init) / > > sizeof(ioregs_init[0])); > > line too long Changed. > > + > > +#define CALC_TIMING(t) (t + period - 1) / period > > + > > +int ide_preinit (void) > > +{ > > + volatile immap_t *im = (immap_t *) CONFIG_SYS_IMMR; > > + long t; > > + const struct { > > + short t0; > > + short t1; > > + short t2_8; > > + short t2_16; > > + short t2i; > > + short t4; > > + short t9; > > + short tA; > > + } pio_specs = { > > Is this a processor specific register? If so, shouldn;t this go in > include/asm-ppc/immap_512x.h? Well spotted, thanks - especially, since the whole code is duplicated in board/freescale/mpc5121ads/mpc5121ads.c; split into new file, cpu/mpc512x/ide.c > > +#define CONFIG_ARIA 1 > > +/* > > + * Memory map for the ARIA board: > > + * > > + * 0x0000_0000 - 0x0FFF_FFFF DDR RAM (256 MB) > > + * 0x3000_0000 - 0x3001_FFFF On Chip SRAM (128 KB) > > + * 0x3010_0000 - 0x3003_FFFF On Board SRAM (128 KB - max 512KB - 1MB > > reserved) - CS6 > > line too long. Indeed. All long lines fixed. Thanks for pointing out. > > +#define CONFIG_ENV_IS_IN_FLASH 1 > > +/* This has to be a multiple of the Flash sector size */ > > +#define CONFIG_ENV_ADDR (CONFIG_SYS_MONITOR_BASE + > > CONFIG_SYS_MONITOR_LEN) > > line too long ... Hmm.. shouldn;t we accept such long lines? It seems > better to me to have such long lines, because it would look uglier if > we split this in two or more lines? No, in this case it's easy to write n two lines, I think. Thanks a lot! 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: w...@denx.de How many QA engineers does it take to screw in a lightbulb? 3: 1 to screw it in and 2 to say "I told you so" when it doesn't work. _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot