[U-Boot] [PATCH 2/2] sunxi: Universal Allwinner A10/A13/A20 u-boot binary support

[Ian Campbell]

On Sun, 2014-08-03 at 06:26 +0300, Siarhei Siamashka wrote:

> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index e385eda..95887f6 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -650,6 +650,9 @@ config TARGET_SUN5I
>  config TARGET_SUN7I
>  	bool "Support sun7i"
>  
> +config TARGET_SUN4I_SUN5I_SUN7I
> +	bool "Support sun4i/sun5i/sun7i"

Is the intention to eventually support sun6i/sun8i etc here too? I think
we should try and avoid enumerating them in the names of everything
(Kconfig, filenames etc) and instead us TARGET_SUNXI_GENERIC or
TARGET_SUNXI_MULTI or something along those lines, or perhaps Kconfig
could present a bool for each of the subarches and enabling more than
one would enable multiple mode.

Not directly related to this series but probably arch/arm/Kconfig should
have TARGET_SUNXI and the 4i/5i/7i stuff ought to move down into
arch/arm/sunxi/Kconfig.

> @@ -48,17 +48,62 @@ u32 spl_boot_mode(void)
>  }
>  #endif
>  
> +static void sunxi_soc_detect_init(void)
> +{
> +	/* Enable VER_REG (set the VER_R_EN bit) */
> +	setbits_le32((u32 *)(SUNXI_SRAMC_BASE + 0x24), 1 << 15);

Please can you #define 0x24 and the various masks/shifts.
> +}
> +
> +int soc_is_sun4i(void)

All of these should use a common helper which takes the ID as a
parameter or the SOC_IS_xxx macros could just use that helper directly
if the wrappers turn out not that useful.

A bit of cpp trickery could also lead to:

#define SUNXI_SOC_ID_SUN4I 0x1623
#define SUNXO_SOC_ID...
#define SUNXI_SOC_IS(X) soc_is(SUNXI_SOC_ID_#X)

Used as SUNXI_SOC_IS(SUN4I) etc. What do you think?

> +
> +	if (cons_index == 1 && SOC_IS_SUN5I()) {
> +		u32 val = readl(SUNXI_SID_BASE + 0x08);
> +		if (((val >> 12) & 0xf) == 3) {

Can we use some #defines for the masks and shifts please.

> diff --git a/board/sunxi/dram_sunxi_ddr3_failsafe.c b/board/sunxi/dram_sunxi_ddr3_failsafe.c
> new file mode 100644
> index 0000000..348e0b9
> --- /dev/null
> +++ b/board/sunxi/dram_sunxi_ddr3_failsafe.c

How about putting this stuff in dram.c (or a new dram_default.c if you
prefer) marked as __weak? IOW make it the default for everything which
doesn't add a more specific dram_foo.c.

> @@ -0,0 +1,28 @@
> +/* this file is generated, don't edit it yourself */

I've had my doubts about this comment in the past, but here in
particular I was under the impression that you had manually selected the
safest values.

> +#define CONFIG_SYS_PROMPT		"sunxi# "

TBH I think we should just move this to -common.h and nuke the sun[457]i
ones, they don't serve much purpose, the specific SOC is identified in
the boot banner already.

FYI I'm now AFK until the 18th.

Ian.