Re: [Qemu-devel] [PATCH v6 12/24] hw/nand.c: bug fix to erase operation

[Kuo-Jung Su]

2013/3/7 Peter Maydell <address@hidden>:
> On 7 March 2013 10:18, Peter Crosthwaite <address@hidden> wrote:
>> This fixes a no-boot bug in u-boot for us as well. RE PMMs comments in
>> v5, I realise the desire to fix this properly by rewriting that
>> if-else mess, but can we get a merge on this one more immediately to
>> get QEMU working again? Rewriting this is probably not at the top of
>> either mine or Kuo Jungs priority list but at the same time we would
>> like a working boot.
>
> The trouble is that the control flow is so complicated that I don't
> feel comfortable saying "yes, this change doesn't break operation
> of any of the other commands". That's why I didn't give it a
> reviewed-by tag. If you can provide a reasonably coherent explanation
> of why it doesn't break anything else with reference to a decent
> datasheet, you could convince me it's OK.
>
> -- PMM

I've checked the history of hw/nand.c, it might be bug-free before the patch
'hw/nand: Support large NAND devices' being applied.
Because it modifies the data type of addr from uint32_t into uint64_t,
and combined with
the problematic address clear (clear only address length), which then
leads to a bug of this.

BTW, if you're looking for the datasheet on which the nand.c based,
please refer to the link bellow:

http://www.datasheetcatalog.org/datasheet/SamsungElectronic/mXvvrxx.pdf

P.S:
The u-boot / linux suffers this bug because of the BBT, which stands for
Bad Block Table.
In other words, the u-boot / linux usually starts with a bunch of bad block scan
on block boundary (i.e. 64KB, 128KB).
While the addr is a uint32_t, right-shift 24 bits would always clear
address long enough for the incoming new address, but things would go
mad, if it's a uint64_t.

--
Best wishes,
Kuo-Jung Su