From mboxrd@z Thu Jan  1 00:00:00 1970
From: Stefan Roese <sr@denx.de>
Date: Tue, 15 Jul 2008 11:48:11 +0200
Subject: [U-Boot-Users] [PATCH] cfi_flash: fix flash on Big Endian
	machines.
In-Reply-To: <20080715093626.GD30428@www.tglx.de>
References: <20080714101829.GA11938@www.tglx.de>
	<200807151046.33294.sr@denx.de>
	<20080715093626.GD30428@www.tglx.de>
Message-ID: <200807151148.12166.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

On Tuesday 15 July 2008, Sebastian Siewior wrote:
> >> That command seems to be access in a little endian way so
> >> wrappers are required.
> >
> >Please explain why this is the case? Little endian wrappers on big endian
> >machines for FLASH access? This sounds wrong to me.
>
> Look at this snippet from flash_make_cmd (my case: big endian &
>
> CFG_WRITE_SWAPPED_DATA):
> |         for (i = info->portwidth; i > 0; i--){
> |                 cword_offset = (info->portwidth-i)%info->chipwidth;
> |                 cp_offset = info->portwidth - i;
> |                 val = *((uchar*)&cmd + cword_offset);
> |                 cp[cp_offset] = (cword_offset >= sizeof(uint)) ? 0x00 :
> | val; }
>
> Now, 8bit access, 8bit bus: portwidth = 1, chipwidth = 1. I am only once
> in this for loop and I get the MSB byte of cmd which is 0x00 instead of
> the command which is never touched.
>
> >> Long is the wrong type because it will behave differently on
> >> 64bit machnines in a way that is probably not expected.
> >> int should be enough.
> >
> >Yes. I suggest that you move this long/int issue to a separate patch. It
> > isn't related to this endian issue.
>
> Right.

OK, I'll await a separate patch for this.

> >And to you main patch:
> >
> >NAK. After applying this, CFI support on kilauea (PPC405EX with 1*
> > Spansion S29GL512N, 16bit wide) breaks:
>
> Is this big endian + CFG_WRITE_SWAPPED_DATA? Did it work before the
> patch?

This is big endian and CFG_WRITE_SWAPPED_DATA *not* defined. You can check 
this yourself, the port is officially available. And yes, of course it worked 
before the patch.

> >FLASH: CFI: Unknown command set 0x0
>
> The command has not been sent but MSB which was 0x00.
>
> >## Unknown FLASH on Bank 1 - Size = 0x00000000 = 0 MB
>
> That's the message I got before my patch. There is probably still
> something wrong.
>
> >Please explain what exactly you need to fix, perhaps with an example. The
> >current version breaks other boards.
>
> I attached a file which can be used for testing. On your little endian
> machine you should have __LITTLE_ENDIAN

Little endian? Why should I have this defined on my PPC system. It's not.

> defined and your result should 
> be 0xf0 in the case i = 0. Right now, I get 0x00 on my BE machine while
> LE case is working fine.

I have to admit that I don't have the time to test anything right now. It 
seems that you have a problem with the #else path in flash_make_cmd(). Do you 
have access to a big endian platform which doesn't define 
CFG_WRITE_SWAPPED_DATA? If yes, you could test both cases.

Best regards,
Stefan

=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de
=====================================================================