Versatile Express: add modelling of NOR flash

This patch adds modelling of the two NOR flash banks found in the Versatile Express motherboard. Tested with U-Boot running on an emulated Versatile Express A9. The alias of the first NOR flash in the Cortex-A Series memory map is not modelled.

Signed-off-by Francesco Lavra <[email protected]>

---
  hw/vexpress.c |   25 +++++++++++++++++++++++--
  1 files changed, 23 insertions(+), 2 deletions(-)

hw/vexpress.c +5 -0

index b615844..695892b 100644
@@ -29,8 +29,12 @@
  #include "sysemu.h"
  #include "boards.h"
  #include "exec-memory.h"
+#include "blockdev.h"
+#include "flash.h"
 
  #define VEXPRESS_BOARD_ID 0x8e0
+#define VEXPRESS_FLASH_SIZE (64 * 1024 * 1024)
+#define VEXPRESS_FLASH_SECT_SIZE (256 * 1024)
 
  static struct arm_boot_info vexpress_binfo;
 

@@ -357,6 +361,7 @@ static void vexpress_common_init(const VEDBoardInfo
 *daughterboard,
      qemu_irq pic[64];
      uint32_t proc_id;
      uint32_t sys_id;
+    DriveInfo *dinfo;
      ram_addr_t vram_size, sram_size;
      MemoryRegion *sysmem = get_system_memory();

      MemoryRegion *vram = g_new(MemoryRegion, 1);

hw/vexpress.c +0 -1

@@ -412,9 +417,25 @@ static void vexpress_common_init(const VEDBoardInfo
 *daughterboard,
 
      sysbus_create_simple("pl111", map[VE_CLCD], pic[14]);
 
-    /* VE_NORFLASH0: not modelled */

hw/vexpress.c +9 -1

-    /* VE_NORFLASH1: not modelled */
+
+    dinfo = drive_get(IF_PFLASH, 0, 1);
+    if (!pflash_cfi01_register(map[VE_NORFLASH1], NULL, "vexpress.flash1",
+            VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL,
+            VEXPRESS_FLASH_SECT_SIZE,
+            VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4,
+            0x00, 0x89, 0x00, 0x18, 0)) {
+        fprintf(stderr, "vexpress: error registering flash 1.\n");
+    }
 
      sram_size = 0x2000000;

      memory_region_init_ram(sram, "vexpress.sram", sram_size);

Am 04.09.2012 19:08, schrieb Francesco Lavra:

Quoted 1 line

      /* VE_NORFLASH0ALIAS: not modelled */

What about that alias? It's not difficult to add it, too. Just look for memory_region_init_alias in the code to see how it is done (hw/mips_malta.c has an alias region for flash).

Quoted 8 lines · Stefan Weil

Am 04.09.2012 19:08, schrieb Francesco Lavra:

Quoted 1 line

      /* VE_NORFLASH0ALIAS: not modelled */

What about that alias? It's not difficult to add it, too. Just look for memory_region_init_alias in the code to see how it is done (hw/mips_malta.c has an alias region for flash).

It's painful because you might also have to add the logic for letting the guest map and unmap the alias (which implies implementing a whole section of the A15 board we don't currently bother with, the SCC registers). I'd need to check the board documentation more carefully to see if we can get away with always mapping that area as the flash alias.

(Also we'd need to fix the current problem with the motherboard address map arrays that there's no way to distinguish "peripheral not present on this board" from "peripheral at address 0", since the A9 board doesn't have the flash alias.)

More to the point, this is the third attempt at doing this. Previously Liming Wang sent a patch:
http://patchwork.ozlabs.org/patch/147905/
and Jagan sent a two-patch set:
http://patchwork.ozlabs.org/patch/171812/
http://patchwork.ozlabs.org/patch/171814/

both of which failed in the code review stage. Francesco, can you check that you haven't fallen into any of the same problems they did, please?

Hi,

Quoted 10 lines · Peter Maydell

Quoted 8 lines · Stefan Weil

Am 04.09.2012 19:08, schrieb Francesco Lavra:

Quoted 1 line

      /* VE_NORFLASH0ALIAS: not modelled */

What about that alias? It's not difficult to add it, too. Just look for memory_region_init_alias in the code to see how it is done (hw/mips_malta.c has an alias region for flash).

Quoted 6 lines

It's painful because you might also have to add the logic for letting the guest map and unmap the alias (which implies implementing a whole section of the A15 board we don't currently bother with, the SCC registers). I'd need to check the board documentation more carefully to see if we can get away with always mapping that area as the flash alias.

Documentation at
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0503c/CHDEFDJF.html
says that the entire first 512 MB can be mapped to either SMC (which is the default) or AXI, so if AXI is selected neither of the 2 flash banks is visible. Also, the same doc says that it's possible to map either NOR0 (default) or NOR1 to the address 0x00000000. This implies that in the A Series memory map VE_NORFLASH0 should be at 0x08000000 and VE_NORFLASH0ALIAS at 0x00000000, not the other way around (by the way, this is also how U-Boot defines the memory for the A5 CoreTile). Maybe worth a patch?

If we can get way with always aliasing to flash 0, the actual implementation of the alias is made difficult by the fact that memory_region_init_alias() needs the MemoryRegion of the aliased memory, and the daughterboard-specific initialization is done in a function which doesn't have access to that MemoryRegion. So we can either:
1. move initialization of common flash modelling before daughterboard-specific initialization and pass the relevant MemoryRegion to the daughterboard-specific init function
2. add another field to VEDBoardInfo which tells if the alias capability is implemented, and use this info in vexpress_common_init() to define the alias if appropriate
Or we can simply deem this alias not worth the trouble, which is what I thought before sending the patch... Let me know your thoughts.

Quoted 14 lines

(Also we'd need to fix the current problem with the motherboard address map arrays that there's no way to distinguish "peripheral not present on this board" from "peripheral at address 0", since the A9 board doesn't have the flash alias.)

More to the point, this is the third attempt at doing this. Previously Liming Wang sent a patch:
http://patchwork.ozlabs.org/patch/147905/
and Jagan sent a two-patch set:
http://patchwork.ozlabs.org/patch/171812/
http://patchwork.ozlabs.org/patch/171814/

Quoted 3 lines

both of which failed in the code review stage. Francesco, can you check that you haven't fallen into any of the same problems they did, please?

I read the reviews of previous attempts, and in fact there is a fix which can be easily done, i.e. replacing the calls to drive_get() with drive_get_next(). Will do that in v2, but first the above points need to be addressed.

Thanks,
Francesco

Documentation at
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0503c/CHDEFDJF.html
says that the entire first 512 MB can be mapped to either SMC (which is the default) or AXI, so if AXI is selected neither of the 2 flash banks is visible. Also, the same doc says that it's possible to map either NOR0 (default) or NOR1 to the address 0x00000000. This implies that in the A Series memory map VE_NORFLASH0 should be at 0x08000000 and VE_NORFLASH0ALIAS at 0x00000000, not the other way around (by the way, this is also how U-Boot defines the memory for the A5 CoreTile). Maybe worth a patch?

Quoted 10 lines · Francesco Lavra

Documentation at
http://infocenter.arm.com/help/topic/com.arm.doc.ddi0503c/CHDEFDJF.html
says that the entire first 512 MB can be mapped to either SMC (which is the default) or AXI, so if AXI is selected neither of the 2 flash banks is visible. Also, the same doc says that it's possible to map either NOR0 (default) or NOR1 to the address 0x00000000. This implies that in the A Series memory map VE_NORFLASH0 should be at 0x08000000 and VE_NORFLASH0ALIAS at 0x00000000, not the other way around (by the way, this is also how U-Boot defines the memory for the A5 CoreTile). Maybe worth a patch?

Agreed, NORFLASH0 should be at 0x08000000 in the new memory map. Unless anybody actually needs the aliasing of the flash at address zero, I suggest just deleting all references to VE_NORFLASH0ALIAS.