From patchwork Thu Mar 7 03:32:33 2013
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: [v6,12/24] hw/nand.c: bug fix to erase operation
From: Peter Crosthwaite
X-Patchwork-Id: 225721
Message-Id:
To: Peter Maydell
Cc: i.mitsyanko@samsung.com, qemu-devel@nongnu.org,
Blue Swirl , Kuo-Jung Su ,
Wendy Liang ,
Paul Brook , Andreas ,
fred.konrad@greensocs.com
Date: Thu, 7 Mar 2013 13:32:33 +1000
Hi Peter, Kuo-Jung,
On Thu, Mar 7, 2013 at 12:28 PM, Peter Maydell wrote:
> On 7 March 2013 10:18, Peter Crosthwaite 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
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