Re: [U-Boot] [PATCH 02/20] mpc832x: add support for the mpc8321 based suvd3 board
Dear Heiko, In message <4d7dc1c1.4060...@denx.de> you wrote: > > >> + im->sysconf.ddrlaw[0].bar = CONFIG_SYS_DDR_BASE & LAWBAR_BAR; > >> + msize = fixed_sdram (); > > > > Can we not use get_ram_size() ? > > fixed_sdram calls this. I see, thanks. > > I see this: > > > > [PATCH 01/20] keymile: rework headerfiles for keymile boards > > total: 0 errors, 16 warnings, 659 lines checked > > Hmm... I see other statistics: I'm using -> checkpatch.pl --version Usage: checkpatch.pl [OPTION]... [FILE]... Version: 0.31 > [hs@pollux u-boot]$ ./../linux-2.6-denx/scripts/checkpatch.pl > 20110308/0001-keymile-rework-headerfiles-for-keymile-boards.patch This should be the same version, assuming your repository is up to date. > WARNING: line over 80 characters > #636: FILE: include/configs/kmeter1.h:179: > +#define CONFIG_SYS_MONITOR_LEN (768 * 1024) /* Reserve 768 kB for > Mon */ > > WARNING: line over 80 characters > #750: FILE: include/configs/mgcoge.h:133: > +#define CONFIG_SYS_MONITOR_LEN (768 << 10) /* Reserve 768KB for > Monitor */ > > total: 0 errors, 2 warnings, 659 lines checked > > 20110313/0001-keymile-rework-headerfiles-for-keymile-boards.patch has style > problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. > [hs@pollux u-boot]$ > > Only 2 warnings ... which checkpatch.pl do you use? You must be doing something wrong. On pollux, as you: -> /home/git/linux-2.6-denx/scripts/checkpatch.pl --no-tree /tmp/patch WARNING: please, no space before tabs #221: FILE: include/configs/keymile-common.h:117: +#define CONFIG_SYS_EEPROM_PAGE_WRITE_BITS ^I3$ ... WARNING: please, no space before tabs #791: FILE: include/configs/mgcoge.h:107: +^I^I"era 0xFE0C +0x4\0" ^I^I^I^I\$ WARNING: line over 80 characters #801: FILE: include/configs/mgcoge.h:133: +#define CONFIG_SYS_MONITOR_LEN (768 << 10) /* Reserve 768KB for Monitor */ total: 0 errors, 16 warnings, 659 lines checked 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 I'm a soldier, not a diplomat. I can only tell the truth. -- Kirk, "Errand of Mercy", stardate 3198.9 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 02/20] mpc832x: add support for the mpc8321 based suvd3 board
Hello Wolfgang, Wolfgang Denk wrote: > Dear Heiko Schocher, > > In message <1299591018-8944-3-git-send-email...@denx.de> you wrote: >> - serial console on UART1 >> - Ethernet RMII over UCC4 >> - PHY SMSC LAN8700 >> - 64MB Flash >> - 128 MB DDR2 RAM >> - I2C >> - bootcount >> >> This board is similiar to the kmeter1 (8360) board, >> so common config options are extracted into the >> include/configs/km83xx-common.h file. > > ... >> -#if defined(CONFIG_BOOTCOUNT_LIMIT) && defined(CONFIG_MPC8360) >> +#if defined(CONFIG_BOOTCOUNT_LIMIT) && \ >> +(defined(CONFIG_MPC8360) || defined(CONFIG_MPC832x)) >> #include > > Please keep lists sorted: 832x < 8360. Please fix globally. Ok. >> +phys_size_t initdram (int board_type) >> +{ >> +#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER) >> +extern void ddr_enable_ecc (unsigned int dram_size); >> +#endif >> +volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR; >> +u32 msize = 0; >> + >> +if ((im->sysconf.immrbar & IMMRBAR_BASE_ADDR) != (u32)im) >> +return -1; >> + >> +/* DDR SDRAM - Main SODIMM */ > > Is this comment correct? No, remove it. >> +im->sysconf.ddrlaw[0].bar = CONFIG_SYS_DDR_BASE & LAWBAR_BAR; >> +msize = fixed_sdram (); > > Can we not use get_ram_size() ? fixed_sdram calls this. >> +extern int ivm_read_eeprom (void); > > Protoypes belong to header files. Yep, this extern is not needed, as the header file is included, remove it. > Um... seems you did not run checkpatch? :-( Argh, you are right ... > I see this: > > [PATCH 01/20] keymile: rework headerfiles for keymile boards > total: 0 errors, 16 warnings, 659 lines checked Hmm... I see other statistics: [hs@pollux u-boot]$ ./../linux-2.6-denx/scripts/checkpatch.pl 20110308/0001-keymile-rework-headerfiles-for-keymile-boards.patch WARNING: line over 80 characters #636: FILE: include/configs/kmeter1.h:179: +#define CONFIG_SYS_MONITOR_LEN (768 * 1024) /* Reserve 768 kB for Mon */ WARNING: line over 80 characters #750: FILE: include/configs/mgcoge.h:133: +#define CONFIG_SYS_MONITOR_LEN (768 << 10) /* Reserve 768KB for Monitor */ total: 0 errors, 2 warnings, 659 lines checked 20110313/0001-keymile-rework-headerfiles-for-keymile-boards.patch has style problems, please review. If any of these errors are false positives report them to the maintainer, see CHECKPATCH in MAINTAINERS. [hs@pollux u-boot]$ Only 2 warnings ... which checkpatch.pl do you use? > [PATCH 02/20] mpc832x: add support for the mpc8321 based suvd3 board > total: 17 errors, 63 warnings, 1326 lines checked > [PATCH 03/20] mpc832x: add support for mpc8321 based tuxa1 board > total: 16 errors, 2 warnings, 250 lines checked > [PATCH 04/20] mpc832x: add support for mpc8321 based tuda1 board > total: 0 errors, 4 warnings, 265 lines checked > ... > [PATCH 06/20] arm: add support of Kirkwood based board SUEN8 > total: 0 errors, 1 warnings, 73 lines checked > [PATCH 07/20] ppc: add support for ppc based board mgcoge2ne > total: 11 errors, 14 warnings, 733 lines checked > ... > [PATCH 09/20] powerpc, 83xx: add kmsupx5 board support > total: 3 errors, 3 warnings, 103 lines checked > [PATCH 10/20] km-arm: i2c support for suenx based boards > total: 1 errors, 0 warnings, 70 lines checked > ... > [PATCH 12/20] ppc, 8321: cleanup tuxa1, tuda1 and suvd3 support > total: 1 errors, 5 warnings, 570 lines checked > [PATCH 13/20] keymile, common; fix i2c deblocking support > total: 0 errors, 21 warnings, 161 lines checked > [PATCH 14/20] arm, keymile: updates for the arm based boards from keymile > total: 0 errors, 2 warnings, 142 lines checked > [PATCH 15/20] keymile boards: add CONFIG_PIGGY_MAC_ADRESS_OFFSET > total: 1 errors, 1 warnings, 38 lines checked > [PATCH 16/20] keymile, common: add setting of some environment variables > total: 0 errors, 7 warnings, 145 lines checked > [PATCH 17/20] ppc, arm: rework and enhance keymile-common.h > total: 0 errors, 3 warnings, 253 lines checked > ... > [PATCH 19/20] keymile-common.h: remove IO mux stuff > total: 3 errors, 5 warnings, 27 lines checked > > > Please check and fix where needed. I stop reviewing here. Ok, I recheck this, thanks! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 02/20] mpc832x: add support for the mpc8321 based suvd3 board
Dear Heiko Schocher, In message <1299591018-8944-3-git-send-email...@denx.de> you wrote: > - serial console on UART1 > - Ethernet RMII over UCC4 > - PHY SMSC LAN8700 > - 64MB Flash > - 128 MB DDR2 RAM > - I2C > - bootcount > > This board is similiar to the kmeter1 (8360) board, > so common config options are extracted into the > include/configs/km83xx-common.h file. ... > -#if defined(CONFIG_BOOTCOUNT_LIMIT) && defined(CONFIG_MPC8360) > +#if defined(CONFIG_BOOTCOUNT_LIMIT) && \ > +(defined(CONFIG_MPC8360) || defined(CONFIG_MPC832x)) > #include Please keep lists sorted: 832x < 8360. Please fix globally. > +phys_size_t initdram (int board_type) > +{ > +#if defined(CONFIG_DDR_ECC) && !defined(CONFIG_ECC_INIT_VIA_DDRCONTROLLER) > + extern void ddr_enable_ecc (unsigned int dram_size); > +#endif > + volatile immap_t *im = (immap_t *)CONFIG_SYS_IMMR; > + u32 msize = 0; > + > + if ((im->sysconf.immrbar & IMMRBAR_BASE_ADDR) != (u32)im) > + return -1; > + > + /* DDR SDRAM - Main SODIMM */ Is this comment correct? > + im->sysconf.ddrlaw[0].bar = CONFIG_SYS_DDR_BASE & LAWBAR_BAR; > + msize = fixed_sdram (); Can we not use get_ram_size() ? > +extern int ivm_read_eeprom (void); Protoypes belong to header files. Um... seems you did not run checkpatch? I see this: [PATCH 01/20] keymile: rework headerfiles for keymile boards total: 0 errors, 16 warnings, 659 lines checked [PATCH 02/20] mpc832x: add support for the mpc8321 based suvd3 board total: 17 errors, 63 warnings, 1326 lines checked [PATCH 03/20] mpc832x: add support for mpc8321 based tuxa1 board total: 16 errors, 2 warnings, 250 lines checked [PATCH 04/20] mpc832x: add support for mpc8321 based tuda1 board total: 0 errors, 4 warnings, 265 lines checked ... [PATCH 06/20] arm: add support of Kirkwood based board SUEN8 total: 0 errors, 1 warnings, 73 lines checked [PATCH 07/20] ppc: add support for ppc based board mgcoge2ne total: 11 errors, 14 warnings, 733 lines checked ... [PATCH 09/20] powerpc, 83xx: add kmsupx5 board support total: 3 errors, 3 warnings, 103 lines checked [PATCH 10/20] km-arm: i2c support for suenx based boards total: 1 errors, 0 warnings, 70 lines checked ... [PATCH 12/20] ppc, 8321: cleanup tuxa1, tuda1 and suvd3 support total: 1 errors, 5 warnings, 570 lines checked [PATCH 13/20] keymile, common; fix i2c deblocking support total: 0 errors, 21 warnings, 161 lines checked [PATCH 14/20] arm, keymile: updates for the arm based boards from keymile total: 0 errors, 2 warnings, 142 lines checked [PATCH 15/20] keymile boards: add CONFIG_PIGGY_MAC_ADRESS_OFFSET total: 1 errors, 1 warnings, 38 lines checked [PATCH 16/20] keymile, common: add setting of some environment variables total: 0 errors, 7 warnings, 145 lines checked [PATCH 17/20] ppc, arm: rework and enhance keymile-common.h total: 0 errors, 3 warnings, 253 lines checked ... [PATCH 19/20] keymile-common.h: remove IO mux stuff total: 3 errors, 5 warnings, 27 lines checked Please check and fix where needed. I stop reviewing here. 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 I'd rather be led to hell than managed to heaven. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot