[U-Boot] [PATCH 6/6] arm: mx5: Add support for DENX M53EVK

Marek Vasut
2013-04-19T11:58:24Z

Dear Wolfgang Denk,

> Dear Marek Vasut,
>
> In message <1366344655-8535-6-git-send-email-marex@denx.de> you wrote:
> ...
>
> > +#define CONFIG_CMD_DHCP
> > +#define CONFIG_CMD_EXT2
> > +#define CONFIG_CMD_FAT
> > +#define CONFIG_CMD_FAT
>
> One CONFIG_CMD_FAT should be enough.

Indeed, it might be too fat now.

[...]
>
> > +#define CONFIG_CMDLINE_TAG
> > +#define CONFIG_INITRD_TAG
> > +#define CONFIG_SETUP_MEMORY_TAGS
>
> I think we support only DT enabled kernels, so do we really need
> these?

I do need those to boot ancient FSL kernel (for that Android 4.1.2 for MX53 with
working graphics acceleration I'm cooking here).

> > +#define CONFIG_BOOTFILE "uImage"
>
> Please make this "m53evk/uImage" as usual.
>
> Don't we need a DT file as well?

OK

> > +#define CONFIG_BOOTARGS "console=ttymxc1,115200"
> > +#define CONFIG_BOOTCOMMAND "run bootcmd_net"
>
> Where is "bootcmd_net" defined?
>
> I doubt that this is actually a working environment.

Right.

> > +#define CONFIG_LOADADDR 0x70800000
> > +#define CONFIG_SYS_LOAD_ADDR CONFIG_LOADADDR
>
> What do we need CONFIG_LOADADDR for?

For this stuff, the "loadaddr" env variable. I have kinda idea to kill this and
unify it to CONFIG_SYS_LOAD_ADDR all around, what do you think?

include/env_default.h:#ifdef CONFIG_LOADADDR
include/env_default.h: "loadaddr=" __stringify(CONFIG_LOADADDR) "\0"

> > +/*
> > + * The 0x1000 offset must be present, otherwise board won't boot.
> > + * The value (IRAM_BASE_ADDR) must be spelled out, otherwise linker
> > won't link. + */
> > +#define CONFIG_SPL_TEXT_BASE 0x70008000
> > +#define CONFIG_SPL_PAD_TO 0x8000
> > +#define CONFIG_SPL_STACK 0x70004000
> > +#define CONFIG_SPL_LIBCOMMON_SUPPORT
> > +#define CONFIG_SPL_LIBGENERIC_SUPPORT
> > +#define CONFIG_SPL_SERIAL_SUPPORT
> > +#define CONFIG_SPL_GPIO_SUPPORT
>
> The comment above does not relate to the definitions here. Is it
> misplaced, or not correct?

Damn, remnant of my experimentation.