* 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
#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;
}
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
_______________________________________________
U-Boot-Users mailing list
U-Boot-Users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/u-boot-users

Reply via email to