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

[Zhang Wei]

Hi, Wolfgang,

Wolfgang Denk wrote:
> Now this is what I want to understand. What exactly is the "potential
> problem"?
>   
That's the issue in the flash 'Spinsion S29GL064M90TFIR6' with x16 
connection. After running flash_read_jedec_ids(), any follow CFI query 
command will get the data with high 8bit = 0xff, but the low 8bit is 
valid. And if we only read low 8bit, we'll get the 0xff too. In 
addition, the second follow CFI query command has no that issue. So, I 
read the full 16bit date and only take the valid low 8bit.
> Can you please point out which specific part of the Linux MTD code you
> mean? And which version of the code?
>   
The reference codes is in Linux Kernel 2.6.19.

drivers/mtd/chips/cfi_probe.c: cfi_chip_setup() calls:
    int num_erase_regions = cfi_read_query(map, base + (0x10 + 
28)*ofs_factor);
include/linux/mtd/cfi.h: cfi_read_query() calls:
    map_word val = map_read(map, addr);
include/linux/mtd/map.h defines:
    #define map_read(map, ofs) inline_map_read(map, ofs)
include/linux/mtd/map.h: function inline_map_read() body:
static inline map_word inline_map_read(struct map_info *map, unsigned 
long ofs)
{
    map_word r;

    if (map_bankwidth_is_1(map))
        r.x[0] = __raw_readb(map->virt + ofs);
    else if (map_bankwidth_is_2(map))
        r.x[0] = __raw_readw(map->virt + ofs);
    else if (map_bankwidth_is_4(map))
        r.x[0] = __raw_readl(map->virt + ofs);
#if BITS_PER_LONG >= 64
    else if (map_bankwidth_is_8(map))
        r.x[0] = __raw_readq(map->virt + ofs);
#endif
    else if (map_bankwidth_is_large(map))
        memcpy_fromio(r.x, map->virt+ofs, map->bankwidth);

    return r;
}
And the 'map_word' definition in include/linux/mtd/map.h is below:
typedef union {
    unsigned long x[MAX_MAP_LONGS];
} map_word;

> I have to admit that I don't really understand this. I would not be
> surprised that some statement like this can be found in some chip
> errata, but I would like to know this for certain first.
>   
I only find one clue from 'Spinsion S29GL064M90TFIR6' specification, 
which is the comment 'Data bits DQ15–DQ8 are don’t care. ' for 'RESET' 
command. And the comment has not found in some other AMD, Spinsion chips 
specifications.
> For me this is an indication that the problem is  actually  somewhere
> else,  and while your modification might actually fix the symptoms, I
> doubt that it is the correct fix - or the correct  problem.  If  your
> explanation was right, this should not depend on compiler versions.
>   

This is a real weird issue. The compiler is also a factor. Chris's 
different compilers get the different results. In fact, the same gcc 
with different size codes will also get different results.

Thanks!

Best Regards,
Zhang Wei