From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Wed, 25 Apr 2007 07:19:35 +0200
Subject: [U-Boot-Users] [PATCH] Fix AMCC bootstrap option letter label
	order
In-Reply-To: <1628E43D99629C46988BE46087A3FBB997AD7F@ep-01.EmbeddedPlanet.local>
References: <1628E43D99629C46988BE46087A3FBB997AD7F@ep-01.EmbeddedPlanet.local>
Message-ID: <200704250719.36215.sr@denx.de>
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

Hi Jeff,

On Tuesday 24 April 2007 21:49, Jeff Mann wrote:
> Several of the AMCC 440 processors have a stupid 'error' in the user
> manual resulting in the logical order of option 'G' and option 'F' being
> labeled in reverse. (The logical order of the bootstrap pins represent
> settings from 0 through 7, while the letters are ordered A, B, C, D, E,
> G, F, H.) In my case, I found this problem affecting the 440EPx and GRx
> processors.

Good catch. I never noticed this, thanks.

> See table 8-2 in the 440EPx Users' Manual.
>
> The option AMCC_PINSTP_F_G_REVERSED has been added to these processors
> where they appear in cpu.c where the array bootstrap_str[] is located.
> Code to reverse a bootstrap setting of 5 and 6 (corresponding to G and
> F) has been added to bootstrap_option().

<insert some code from the patch>

> @@ -207,7 +208,18 @@ static int bootstrap_option(void)
>  	unsigned long val;
>
>  	mfsdr(SDR_PINSTP, val);
> -	return ((val & 0xf0000000) >> SDR0_PINSTP_SHIFT);
> +	val = ((val & 0xf0000000) >> SDR0_PINSTP_SHIFT);
> +
> +/* Some AMCC 440 processors have an error with bootstrap Option F and G
> +   labeled in reverse in the user manual. It has never been fixed */
> +#if defined(AMCC_PINSTP_F_G_REVERSED)
> +	if (val == 5)
> +		val=6;
> +	else if (val==6)
> +		val=5;
> +#endif
> +
> +	return val;
>  }
>  #endif /* SDR0_PINSTP_SHIFT */

I don't think this is the right way do deal with this problem. We should 
instead encode the bootstrap character "A..." into the string too:

#if defined(CONFIG_440EPX) || defined(CONFIG_440GRX)
#define SDR0_PINSTP_SHIFT       29
static char *bootstrap_str[] = {
        "A - EBC (8 bits)",
        "B - EBC (16 bits)",
        "C - EBC (16 bits)",
        "D - NAND (8 bits)",
        "E - PCI",
        "G - I2C (Addr 0x54)",
        "F - PCI",
        "H - I2C (Addr 0x52)",
};
#endif

I think this gives us more flexibility and doesn't "pollute" the code with 
more #ifdef's, especially when other 4xx PPC's will have such a "problem" 
too.

> I know that this problem 
> affects more processors in this family, so other users should add this
> option when they are identified.

I know that 440EP/GR are affected too. Do you know more?

If you (or any other) have no objection against my suggestions mentioned 
above, I'll change the code accordingly.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH, HRB 165235 Munich, CEO: Wolfgang Denk
Office:  Kirchenstr. 5,       D-82194 Groebenzell,            Germany
=====================================================================