public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Sebastian Siewior <bigeasy@linutronix.de>
To: u-boot@lists.denx.de
Subject: [U-Boot-Users] [PATCH] cfi_flash: fix flash on Big Endian machines.
Date: Tue, 15 Jul 2008 11:36:26 +0200	[thread overview]
Message-ID: <20080715093626.GD30428@www.tglx.de> (raw)
In-Reply-To: <200807151046.33294.sr@denx.de>

* Stefan Roese | 2008-07-15 10:46:33 [+0200]:

>Hi Sebastian,
Hi Stefan,

>On Monday 14 July 2008, Sebastian Siewior wrote:
>> This got broken by commits 93c56f212c
>>  [cfi_flash: support of long cmd in U-boot.]
>>
>> 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.

>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?

>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 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.

Sebastian
-------------- next part --------------
#include <stdio.h>
#include <string.h>

int portwidth = 1;

int chipwidth = 1;

static void flash_make_cmd(unsigned int cmd, void *cmdbuf)
{
	int i;
	int cword_offset;
	int cp_offset;
	unsigned char val;
	unsigned char *cp = (unsigned char *) cmdbuf;

	for (i = portwidth; i > 0; i--){
		cword_offset = (portwidth-i)%chipwidth;
#if defined(__LITTLE_ENDIAN) || defined(CFG_WRITE_SWAPPED_DATA)
		cp_offset = portwidth - i;
		val = *((unsigned char*)&cmd + cword_offset);
#else
		cp_offset = i - 1;
		val = *((unsigned char*)&cmd + sizeof(unsigned long) - cword_offset - 1);
#endif
		cp[cp_offset] = (cword_offset >= sizeof(unsigned long)) ? 0x00 : val;
	}
}

int main(void) {

	int i;
	unsigned char buf[8];

	memset(buf, 0, 8);
	flash_make_cmd(0xf0, &buf);
	for (i = 0; i < 8; i++)
		printf("%d -> %02x\n",
				i, buf[i]);
	return 0;
}

  reply	other threads:[~2008-07-15  9:36 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20080714101829.GA11938@www.tglx.de>
2008-07-15  8:46 ` [U-Boot-Users] [PATCH] cfi_flash: fix flash on Big Endian machines Stefan Roese
2008-07-15  9:36   ` Sebastian Siewior [this message]
2008-07-15  9:48     ` Stefan Roese
2008-07-15  9:57       ` Sebastian Siewior
2008-07-15 12:57         ` Stefan Roese
2008-07-15 21:12           ` [U-Boot-Users] [PATCH] cfi_flash: fix flash on BE machines with CFG_WRITE_SWAPPED_DATA Sebastian Siewior
2008-07-15 22:05             ` Wolfgang Denk
2008-07-16  8:04               ` Sebastian Siewior
2008-07-16 18:04               ` [U-Boot-Users] [PATCH v2] " Sebastian Siewior
2008-07-17  9:46                 ` Stefan Roese
2008-07-17 10:41                   ` [U-Boot-Users] [PATCH v2] cfi_flash: fix flash on BE machineswith CFG_WRITE_SWAPPED_DATA Vasiliy Leoenenko
2008-07-10 12:35 [U-Boot-Users] [PATCH] cfi_flash: fix flash on Big Endian machines Sebastian Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080715093626.GD30428@www.tglx.de \
    --to=bigeasy@linutronix.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox