Hello Igor,
Igor Grinberg wrote:
> On 10/21/2011 08:32 AM, Heiko Schocher wrote:
>> - DM368 SOC
>> - booting with spl not with UBL from TI
>> - before loading u-boot from NAND into RAM, test
>> the RAM with the post memory test. If error
>> is found, switch all LEDs on and halt system.
>> - SPI Flash
>> Dataflash Typ: M25PE80
>> - Ethernet DM9161BI
>> - MMC
>> - USB
>>
>> Signed-off-by: Heiko Schocher <h...@denx.de>
>> Cc: Sandeep Paulraj <s-paul...@ti.com>
>> Cc: Albert ARIBAUD <albert.u.b...@aribaud.net>
>> ---
[...]
>> diff --git a/board/ait/cam_enc_4xx/Makefile b/board/ait/cam_enc_4xx/Makefile
>> new file mode 100644
>> index 0000000..4804597
>> --- /dev/null
>> +++ b/board/ait/cam_enc_4xx/Makefile
[...]
> I don't think you should be adding this.
> Please, see the commit 464c79207c89f247f97b344495924eabb0c9738e
> (punt unused clean/distclean targets) by Mike.
Yep, you are right, done.
>> diff --git a/board/ait/cam_enc_4xx/cam_enc_4xx.c
>> b/board/ait/cam_enc_4xx/cam_enc_4xx.c
>> new file mode 100644
>> index 0000000..059a08a
>> --- /dev/null
>> +++ b/board/ait/cam_enc_4xx/cam_enc_4xx.c
[...]
>> +int board_init(void)
>> +{
>> + gd->bd->bi_arch_number = MACH_TYPE_DAVINCI_DM365_EVM;
>
> You should be using the new standard for specifying the machine type.
> Please, read the README file (CONFIG_MACH_TYPE option).
Changed, thanks!
[...]
>> +static void davinci_std_write_page_syndrome(struct mtd_info *mtd,
>> + struct nand_chip *chip, const uint8_t *buf)
>> +{
>> + unsigned char davinci_ecc_buf[NAND_MAX_OOBSIZE];
>> + struct nand_chip *this = mtd->priv;
>> + int i, eccsize = chip->ecc.size;
>> + int eccbytes = chip->ecc.bytes;
>> + int eccsteps = chip->ecc.steps;
>> + int chunk = chip->ecc.bytes + chip->ecc.prepad + chip->ecc.postpad;
>> + int offset = 0;
>> + const uint8_t *p = buf;
>> + uint8_t *oob = chip->oob_poi;
>> +
>> + for (i = 0; eccsteps; eccsteps--, i += eccbytes, p += eccsize) {
>> + chip->ecc.hwctl(mtd, NAND_ECC_WRITE);
>> + chip->write_buf(mtd, p, eccsize);
>> +
>> + /* Calculate ECC without prepad */
>> + chip->ecc.calculate(mtd, p, oob + chip->ecc.prepad);
>> +
>> + if (chip->ecc.prepad) {
>> + offset = ((chip->ecc.steps - eccsteps) * chunk);
>> + memcpy(&davinci_ecc_buf[offset], oob, chip->ecc.prepad);
>> + oob += chip->ecc.prepad;
>> + }
>> +
>> + offset = (((chip->ecc.steps - eccsteps) * chunk) +
>> + chip->ecc.prepad);
>
> 2 sets of parenthesis is enough.
> I don't see any good in having the whole expression wrapped.
Changed.
>> + memcpy(&davinci_ecc_buf[offset], oob, eccbytes);
>> + oob += eccbytes;
>> +
>> + if (chip->ecc.postpad) {
>> + offset = (((chip->ecc.steps - eccsteps) * chunk) +
>> + (chip->ecc.prepad + eccbytes));
>
> same here
Changed.
>> + memcpy(&davinci_ecc_buf[offset], oob,
>> + chip->ecc.postpad);
>> + oob += chip->ecc.postpad;
>> + }
>> + }
>> +
>> + /*
>> + * Write the sparebytes into the page once
>> + * all eccsteps have been covered
>> + */
>> + for (i = 0; i < mtd->oobsize; i++)
>> + writeb(davinci_ecc_buf[i], this->IO_ADDR_W);
>> +
>> + /* Calculate remaining oob bytes */
>> + i = mtd->oobsize - (oob - chip->oob_poi);
>
> This one looks good, I think the previous should be the same.
Yep.
[...]
>> +void enable_vbus(void)
>> +{
>> + /*
>> + * nothing to do, but this function is needed from
>> + * drivers/usb/musb/davinci.c
>> + */
>> +}
>
> I think the common way would be to define a "weak" implementation in
> drivers/usb/musb/davinci.c?
Yes, that would be cleaner, changed.
>> +
>> +int board_late_init(void)
>> +{
>> + struct davinci_gpio *gpio = davinci_gpio_bank45;
>> +
>> + /* 24MHz InputClock / 15 prediv -> 1.6 MHz timer running */
>> + while (get_timer_val() < 0x186a00)
>> + ;
>> +
>> + /* 1 sec reached -> stop timer, clear all LED */
>> + stop_timer();
>> + clrbits_le32(&gpio->out_data, CONFIG_CAM_ENC_LED_MASK);
>> + return 0;
>> +}
>> +
>> +void reset_phy(void)
>> +{
>> + char *name = "GENERIC @ 0x00";
>> +
>> + /* reset the phy */
>> + miiphy_reset(name, 0x0);
>> +}
>> +
>> +#else
>
> I think, adding a comment to which #if that #else belongs,
> will improve the understanding/readability.
Done.
[...]
Thanks for the review!
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
