[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v6 12/24] hw/nand.c: bug fix to erase operation
From: |
Peter Crosthwaite |
Subject: |
Re: [Qemu-devel] [PATCH v6 12/24] hw/nand.c: bug fix to erase operation |
Date: |
Thu, 7 Mar 2013 13:32:33 +1000 |
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
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
>
- [Qemu-devel] [PATCH v6 08/24] hw/arm: add Faraday FTRTC011 RTC timer support, (continued)
- [Qemu-devel] [PATCH v6 08/24] hw/arm: add Faraday FTRTC011 RTC timer support, Kuo-Jung Su, 2013/03/06
- [Qemu-devel] [PATCH v6 09/24] hw/arm: add Faraday FTDMAC020 AHB DMA support, Kuo-Jung Su, 2013/03/06
- [Qemu-devel] [PATCH v6 10/24] hw/arm: add Faraday FTAPBBRG020 APB DMA support, Kuo-Jung Su, 2013/03/06
- [Qemu-devel] [PATCH v6 11/24] hw/nand.c: correct the sense of the BUSY/READY status bit, Kuo-Jung Su, 2013/03/06
- [Qemu-devel] [PATCH v6 12/24] hw/nand.c: bug fix to erase operation, Kuo-Jung Su, 2013/03/06
[Qemu-devel] [PATCH v6 13/24] hw/arm: add Faraday FTNANDC021 nand flash controller support, Kuo-Jung Su, 2013/03/06
[Qemu-devel] [PATCH v6 14/24] hw/arm: add Faraday FTI2C010 I2C controller support, Kuo-Jung Su, 2013/03/06
[Qemu-devel] [PATCH v6 15/24] hw: add WM8731 codec support, Kuo-Jung Su, 2013/03/06
[Qemu-devel] [PATCH v6 16/24] hw/arm: add Faraday FTSSP010 multi-function controller support, Kuo-Jung Su, 2013/03/06
[Qemu-devel] [PATCH v6 17/24] qemu/bitops.h: add the bit ordering reversal functions stolen from linux, Kuo-Jung Su, 2013/03/06
[Qemu-devel] [PATCH v6 18/24] hw/arm: add Faraday FTGMAC100 1Gbps ethernet support, Kuo-Jung Su, 2013/03/06