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

[Kuo-Jung Su]

2013/3/7 Peter Crosthwaite <address@hidden>:
> Hi Peter, Kuo-Jung,
>
> On Thu, Mar 7, 2013 at 12:28 PM, Peter Maydell <address@hidden> wrote:
>> 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".
>
> There may be a complication with NAND_CMD_COPYBACKPRG1, this code may
> truncate a currently in use address there although I confess I too am
> confused by the control flow here. Kuo Jungs patch could be more
> defensive by not trampling the address on this command. But rather
> than trawl through 100 datasheets looking for all the corner cases RE
> Nand addressing, Wendy (independently of this discussion) fixed this
> issue in our tree with a lower impact change. I'll post a full patch
> to list for review - it may be what you are looking for in that it is
> a direct approach to solve the issue of the broken BLOCK_ERASE command
> (which i believe was Kuo Jungs motivation to begin with) without
> changing the shared address construction logic:
>
> --- a/hw/nand.c
> +++ b/hw/nand.c
> @@ -296,10 +296,14 @@ static void nand_command(NANDFlashState *s)
>          break;
>
>      case NAND_CMD_BLOCKERASE2:
> +       s->addr &= 0xffffff;
>
> Regards,
> Peter
>

I've verified that it works to me, too. So it's should be a
replacement of my buggy one.

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



--
Best wishes,
Kuo-Jung Su