Hi, Iwamatsu-san,
Thank you for your review!
2011/01/17 21:50, Nobuhiro Iwamatsu wrote:
>> +LDSCRIPT = $(OBJTREE)/board/$(BOARDDIR)/u-boot.lds
>> +#sinclude $(OBJTREE)/board/$(BOARDDIR)/config.tmp
>> +
>
> Is this nesseary?
The "#sinclude" is not nessesary. I will remove it.
However the "LDSCRIPT" is necessary on this board because
the LSI needs the top of address when booting.
>> +CONFIG_SYS_TEXT_BASE = 0x8ef80000
>
> Please move this to config file of board.
I will move it.
>> +static void mac_string_to_hex(unsigned char *mac, char *mac_string)
>> +{
>> + int i;
>> + char *s, *e;
>> +
>> + s = mac_string;
>> + for (i = 0; i < 6; i++) {
>> + mac[i] = s ? simple_strtoul(s, &e, 16) : 0;
>> + if (s)
>> + s = (*e) ? e + 1 : e;
>> + }
>> +}
>
> Please use eth_parse_enetaddr.
Oh, I didn't know that function. I will replace it.
>> +static void init_ethernet_mac(void)
>> +{
>> + char mac_string[64];
>> + char env_string[64];
>> + int i;
>> + unsigned char *buf;
>> +
>> + buf = malloc(256);
>
> Please add NULL check.
OK, I will add it.
>> + /* Gigabit Ethernet */
>> + for (i = 0; i < SH7757LCR_GIGA_ETHERNET_NUM_CH; i++) {
>> + get_sh_eth_mac(i + 2, mac_string, buf);
>> + sprintf(env_string, "eth%daddr", i + 2);
>
> I think that you should use i + SH7757LCR_ETHERNET_NUM_CH.
Thank you. I will modify it.
>> +int do_write_mac(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[])
>> +{
>> + int i, ret;
>> + char mac_string[256];
>> + struct spi_flash *spi;
>> + unsigned char *buf;
>> +
>> + if (argc != 5) {
>> + buf = malloc(256);
>
> please add NULL check.
I will modify it.
>> +/*
>> + * Copyright (C) 2007
>> + * Nobuhiro Iwamatsu <iwama...@nigauri.org>
>> + *
>> + * Copyright (C) 2008-2009
>> + * Yoshihiro Shimoda <shimoda.yoshih...@renesas.com>
>
> Please update your copyright.
OK, I will update it.
Best regards,
Yoshihiro Shimoda
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
