Dear Luka Perkov, In message <20120315235958.GA16846@w500.iskon.local> you wrote: > > Add support for new boards RaidSonic ICY BOX NAS6210 and NAS6220 boards.
Checkpatch reports a number of issues: WARNING: please, no space before tabs ERROR: trailing whitespace WARNING: please, no spaces at the start of a line Please fix these. > Only difference between boards is number of SATA ports. By default we > use only one SATA port. In order to use both SATA ports on NAS6220 > define CONFIG_NAS6220 in board config file. You should provide a second entry to boards.cfg then which sets this define, so that editing of the board config file is not needed. > If you want to test the boot loader without touching nand you can boot > u-boot using UART. In order to create UART image you need to change > image configuration from nand to uart: > > sed -i 's/nand/uart/' board/Marvell/ib62x0/kwbimage.cfg > > To boot your kirkwood board from UART you will need this tool: > > http://www.solinno.co.uk/public/kwuartboot/ > http://www.solinno.co.uk/public/kwuartboot/kwuartboot-0.1.tar.gz > > Compile and run it like this: > > ./kwuartboot /dev/ttyUSB0 /path/to/u-boot.kwb > > And when it's finished open the console. This text should probably be added to a README and checked in. As is, this information gets lost for the users. > +#ifdef CONFIG_RESET_PHY_R > +/* > + * This must be here because of the compile issue if missing > + */ > +void reset_phy(void) {} > +#endif /* CONFIG_RESET_PHY_R */ This makes no sense. If you don't need reset_phy(), then don't define CONFIG_RESET_PHY > +/* > + * High Level Configuration Options (easy to change) > + */ > +#define CONFIG_FEROCEON_88FR131 1 /* CPU Core subversion */ > +#define CONFIG_KIRKWOOD 1 /* SOC Family Name */ > +#define CONFIG_KW88F6281 1 /* SOC Name */ Please don't define values for macros that enable features only. Please fix globally. ... > +/* > + * Memory Info > + */ > +#define CONFIG_NR_DRAM_BANKS 1 /* we have 1 bank of DRAM */ > +#define PHYS_SDRAM_1_SIZE (256 << 20) /* SDRAM size 256MB */ please use get_ram_size() instead and auto-size the memory. > +#define CONFIG_MAX_RAM_BANK_SIZE (256 << 20) ... > +/* > + * max 4k env size is enough, but in case of nand > + * it has to be rounded to sector size > + */ I think this is not correct. > +#define CONFIG_ENV_ADDR 0x80000 > +#define CONFIG_ENV_OFFSET 0x80000 /* env starts here */ Using both ADDR and OFFSET is redundant at best. Drop one. > +/* Data, registers and alternate blocks are at the same offset */ > +#define CONFIG_SYS_ATA_DATA_OFFSET (0x0100) > +#define CONFIG_SYS_ATA_REG_OFFSET (0x0100) > +#define CONFIG_SYS_ATA_ALT_OFFSET (0x0100) Please do not use parens for simple parameters. Please fix globally. 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 Speed of a tortoise breaking the sound barrier = 1 Machturtle _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
