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

[Zhang Wei]

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