[U-Boot] [PATCH 02/20] mpc832x: add support for the mpc8321 based suvd3 board

[Heiko Schocher]

Hello Wolfgang,

Wolfgang Denk wrote:
> Dear Heiko Schocher,
> 
> In message <1299591018-8944-3-git-send-email-hs at 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 <asm/immap_qe.h>
> 
> 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 at 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 at 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