[U-Boot-Users] [PATCH] Fixed cfi flash read uchar bug.

Zhang Wei
2006-12-25T07:03:44Z

Hi, Wolfgang,

Merry X'mas!

I want to explain two key points. I also take the x16 connection flash
example.

1. Reading 16bit data in flash_read_uchar() will not cause an error.
We can get an example from the CFI specification. We run the CFI
Query-unique ACSII string "QRY" command on x16 device with x16 mode
connection. From the word address 0x10, 0x11, 0x12, we could get "Q",
"R", "Y". If using byte address, we should get them like below: byte
address 0x20 got "Q", 0x21 got 0x0, 0x22 got "R", 0x23 got 0x0, 0x24 got
"Y", 0x25 got 0x0. In the u-boot, The word address offset '0x10' in
flash_read_uchar() will be translated to byte address '0x20' by
flash_make_addr(). I get the high 8-bit in flash_read_uchar(), which is
just a NULL and discarded.

2. Reading 16bit data in flash_read_uchar() is a safety operation, which
will avoid the potential problem.
In the Linux kernel, the cfi driver using the similar
implementation. I think reading the full width bit data from the flash
port may clear the flash data buffer. I've tested to add 'volatile'
keyword to the flash_get_size() and flash_read_jedec_ids() functions'
declaration. The flash can not work also. From Chris' work, the
compiler's optimizing causes some flash to get the error.

Thanks!

Best Regards,
Zhang Wei

Wolfgang Denk wrote:
> In message <2176B872C0407E44887F07CCAA869293832458@zch01exm21.fsl.freescale.net> you wrote:
>
>> Yes, the memcpy() is just a byte copy. But a x16 read can be emulated by
>> two x8 read. And in fact, the flash_read_ushort(), flash_read_long() in
>> cfi_flash.c are using the same implementation.
>>
>
> But these are supposed to read more than one byte.
>
>
>> In addition, the original code only reads 8bit, not the full 16bit. My
>> patch ensures the full 16bit data are read completely.
>>
>
> I still don't understand why flash_read_uchar() should read more than
> one byte? If you need a 16 bit read operation I would expect you to
> use flash_read_ushort() instead.
>
> Best regards,
>
> Wolfgang Denk
>
>