[U-Boot] [PATCH] lsxl: add support for lschlv2 and lsxhl

Michael Walle
2012-01-03T00:32:02Z

Hi Wolfgang,

quick answer ;)

> In message <1325545909-17678-1-git-send-email-michael@walle.cc> you wrote:
> > This patchset adds support for both the Linkstation Live (LS-CHLv2) and
> > Linkstation Pro (LS-XHL) by Buffalo.
>
> If Buffalo is the vendor, then why do you add this to the Marvell
> vendor directory?
I was just guessing. It is a Marvell SoC and marvell SoC boards of other
vendors are in the Marvell/ directory, too. But i'm also happy with Buffalo/
(or buffalo/?).

> > +Michael Walle <michael@walle.cc>
> > +
> > + lsxl ARM926EJS (Kirkwood SoC)
>
> You add two boards, so please also maintain both.
oops sorry.

> > +#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 select featues only.
> Please fix globally.
ok

> > +#ifdef CONFIG_SPI_FLASH
> > +#define CONFIG_SYS_MAX_FLASH_BANKS 1
> > +#define CONFIG_SYS_MAX_FLASH_SECT 8
> > +#define CONFIG_ENV_IS_IN_SPI_FLASH 1
> > +#define CONFIG_ENV_SECT_SIZE 0x10000 /* 64K */
> > +#else
> > +#define CONFIG_ENV_IS_NOWHERE 1 /* if env in SDRAM */
> > +#endif
> > +#define CONFIG_ENV_SIZE 0x10000 /* 64k */
> > +#define CONFIG_ENV_OFFSET 0x70000 /* env starts here */
>
> Please use TABs only for vertical alignment. Please fix globally.
ok

> > +#define CONFIG_ETHADDR 02:00:01:00:00:00
>
> ...
>
> > +#define CONFIG_IPADDR 192.168.11.150
> > +#define CONFIG_SERVERIP 192.168.11.1
> > +#define CONFIG_NETMASK 255.255.255.0
>
> NAK. We do not allow such static network configurations.
i've already guessed that ;) you may remember our lengthy discussions about
the board which don't have a dedicated NVRAM for the ethernet address. Well,
this is such a board. I would have expected more responses from other
maintainers of this type of boards, esp. after the patch which removes the mac
randomization from the ethernet driver, was merged.

So if
#define CONFIG_ETHADDR xxx
#define CONFIG_OVERWRITE_ETHADDR_ONCE
is a no-no (i really dont see the point, why it's not allowed, sorry), i guess
i have to maintain this on my own tree :(

--
Michael