From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stephen Warren <swarren@wwwdotorg.org>
Date: Tue, 08 Oct 2013 16:06:02 -0600
Subject: [U-Boot] [PATCH 8/8] Tegra124: Add Venice2 (T124) build
In-Reply-To: <1381185778-25722-8-git-send-email-twarren@nvidia.com>
References: <1381185778-25722-1-git-send-email-twarren@nvidia.com>
	<1381185778-25722-8-git-send-email-twarren@nvidia.com>
Message-ID: <525481CA.30309@wwwdotorg.org>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On 10/07/2013 04:42 PM, Tom Warren wrote:
> These are the board files for Venice2 (t124), plus the AS3722
> PMIC files. PMIC init will be moved to pmic_common_init later.
> 
> This builds/boots on Venice2, SPI/MMC/USB/I2C all work. Audio
> and display and WB/LP0 are not supported yet.

> diff --git a/board/nvidia/venice2/venice2.c b/board/nvidia/venice2/venice2.c

> +void board_sdmmc_voltage_init(void)
> +{
> +#if defined(VENICE2_LATE_PMIC_INIT)
> +	/*
> +	 * TODO(twarren at nvidia.com):
> +	 *  Find out why writing the SDMMC LDO this late hangs the CPU
> +	 *  Maybe it's the re-write of the enable bit? (already set by OTP)
> +	 */

Wouldn't it be better to only include the active code? We can always
just make a patch to move to different code later if we want.


> +Active  arm         armv7:arm720t  tegra124    nvidia          venice2             venice2                              -                                                                                                                                 Tom Warren <twarren@nvidia.com>
>  Active  arm         armv7:arm720t  tegra114    nvidia          dalmore             dalmore                              -                                                                                                                                 Tom Warren <twarren@nvidia.com>

That doesn't seem sorted. I would have expected tegra114 before
tegra124? Same if it's sorted by board name.

> diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h

> +#ifndef _TEGRA124_COMMON_H_
> +#define _TEGRA124_COMMON_H_
> +#include "tegra-common.h"

A blank line is typical after the include guard.


> +#define CONFIG_TEGRA124			/* in a NVidia Tegra124 core */

NVIDIA not NVidia.

> +#define CONFIG_SPL_LDSCRIPT		"$(CPUDIR)/tegra124/u-boot-spl.lds"

I don't think that's used upstream. Perhaps diff these files against the
Tegra114 versions and check for unexpected differences?

> diff --git a/include/configs/venice2.h b/include/configs/venice2.h

> +/* Use memory controller SDRAM size instead of ODMDATA */
> +#define CONFIG_TEGRA_USE_EMC_DRAM_SIZE

We should either always do that or never; boards shouldn't make the
decision. Rather, the core Tegra code should make the decision based on
the SoC ID.

Also, CONFIG_TEGRA_USE_EMC_DRAM_SIZE doesn't exist upstream.

> +/* Environment in eMMC, at the end of 2nd "boot sector" */
> +#define CONFIG_ENV_IS_IN_MMC
> +#define CONFIG_SYS_MMC_ENV_DEV		0
> +#define CONFIG_SYS_MMC_ENV_PART		2
> +#define CONFIG_ENV_OFFSET		((4096 * 1024) - CONFIG_ENV_SIZE)

Dalmore says:

#define CONFIG_ENV_OFFSET               (-CONFIG_ENV_SIZE)

... which seems a bit more self-managing.