From mboxrd@z Thu Jan  1 00:00:00 1970
From: Alban Bedel <alban.bedel@avionic-design.de>
Date: Wed, 30 Oct 2013 12:19:32 +0100
Subject: [U-Boot]
 =?utf-8?q?=5BPATCH_v3=5D_ARM=3A_tegra=3A_Add_the_Tamonte?=
 =?utf-8?q?n=E2=84=A2_NG_Evaluation_Carrier_board?=
In-Reply-To: <526EDE66.2060402@wwwdotorg.org>
References: <5260393A.4000609@wwwdotorg.org>
	<1382365726-29140-1-git-send-email-alban.bedel@avionic-design.de>
	<526EDE66.2060402@wwwdotorg.org>
Message-ID: <20131030121932.645d0190@avionic-0020>
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 Mon, 28 Oct 2013 16:00:06 -0600
Stephen Warren <swarren@wwwdotorg.org> wrote:

> On 10/21/2013 08:28 AM, Alban Bedel wrote:
> > Add support for the new Tamonten? NG platform from Avionic Design.
> > Currently only I2C, MMC, USB and ethernet have been tested.
> 
> What changed in v3? There's no changelog here, so I don't know where to
> concentrate my re-review of this patch...

I'll add one. It was unclear to me how these should be done, and it
took me quiet some times to find out about this 3 dashes hidden
feature.

> > diff --git a/board/avionic-design/common/tamonten-ng.c b/board/avionic-design/common/tamonten-ng.c
> 
> > +#define MAX_I2C_RETRY		3
> 
> I don't think that's used any more.

I'll remove it.

> Most of the PMU #defines aren't used either.

I'll remove them too, althought I hate having hardcoded values in the
code instead of a human readable definition of the registers.

> > +#ifdef CONFIG_BOARD_EARLY_INIT_F
> 
> I don't think any of these ifdefs are required, since the config file
> hard-codes all those values. Same for #if defined(CONFIG_TEGRA_MMC)
> below, and perhaps more?

Will do.

> > +void gpio_early_init(void)
> > +{
> > +	/* Turn on the alive signal */
> > +	gpio_request(GPIO_PV2, "ALIVE");
> > +	gpio_direction_output(GPIO_PV2, 1);
> > +
> > +	/* Remove the reset on the reset on the external periph */
> 
> "reset on the reset on the" ?
> 
> > diff --git a/board/avionic-design/dts/tegra30-tamonten.dtsi b/board/avionic-design/dts/tegra30-tamonten.dtsi
> 
> > +	i2c at 7000c000 {
> > +		status = "okay";
> > +		clock-frequency = <100000>;
> > +	};
> > +
> > +	i2c at 7000c400 {
> > +		status = "okay";
> > +		clock-frequency = <100000>;
> > +	};
> ...
> 
> Do all the carrier boards guarantee that all those I2C and SPI, and even
> SDHCI busses are routed somewhere? It may be best to defer adding
> status="okay" to the leaf board files, so you know there's actually
> something hooked up there.

Most of these are on the COM module, but you are right, a few of them
shouldn't have status="okay" in this dtsi.

Alban