Alex Bennée <alex.ben...@linaro.org> writes: > Markus Armbruster <arm...@redhat.com> writes: > >> Machine "ref405ep" maps its flash memory at address 2^32 - image size. >> Image size is rounded up to the next multiple of 64KiB. Useless, >> because pflash_cfi02_realize() fails with "failed to read the initial >> flash content" unless the rounding is a no-op. >> >> If the image size exceeds 0x80000 Bytes, we overlap first SRAM, then >> other stuff. No idea how that would play out, but a useful outcomes >> seem unlikely. >> >> Map the flash memory at fixed address 0xFFF80000 with size 512KiB, >> regardless of image size, to match the physical hardware. >> >> Machine "taihu" maps its boot flash memory similarly. The code even >> has a comment /* XXX: should check that size is 2MB */, followed by >> disabled code to adjust the size to 2MiB regardless of image size. >> >> Its code to map its application flash memory looks the same, except >> there the XXX comment asks for 32MiB, and the code to adjust the size >> isn't disabled. Note that pflash_cfi02_realize() fails with "failed >> to read the initial flash content" for images smaller than 32MiB. >> >> Map the boot flash memory at fixed address 0xFFE00000 with size 2MiB, >> to match the physical hardware. Delete dead code from application >> flash mapping, and simplify some. >> >> Cc: David Gibson <da...@gibson.dropbear.id.au> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> hw/ppc/ppc405_boards.c | 53 +++++++++++++----------------------------- >> 1 file changed, 16 insertions(+), 37 deletions(-) >> >> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c >> index f47b15f10e..728154aebb 100644 >> --- a/hw/ppc/ppc405_boards.c >> +++ b/hw/ppc/ppc405_boards.c >> @@ -158,7 +158,7 @@ static void ref405ep_init(MachineState *machine) >> target_ulong kernel_base, initrd_base; >> long kernel_size, initrd_size; >> int linux_boot; >> - int fl_idx, fl_sectors, len; >> + int len; >> DriveInfo *dinfo; >> MemoryRegion *sysmem = get_system_memory(); >> >> @@ -185,26 +185,19 @@ static void ref405ep_init(MachineState *machine) >> #ifdef DEBUG_BOARD_INIT >> printf("%s: register BIOS\n", __func__); >> #endif >> - fl_idx = 0; >> #ifdef USE_FLASH_BIOS >> - dinfo = drive_get(IF_PFLASH, 0, fl_idx); >> + dinfo = drive_get(IF_PFLASH, 0, 0); >> if (dinfo) { >> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo); >> - >> - bios_size = blk_getlength(blk); >> - fl_sectors = (bios_size + 65535) >> 16; >> #ifdef DEBUG_BOARD_INIT >> - printf("Register parallel flash %d size %lx" >> - " at addr %lx '%s' %d\n", >> - fl_idx, bios_size, -bios_size, >> - blk_name(blk), fl_sectors); >> + printf("Register parallel flash\n"); >> #endif >> - pflash_cfi02_register((uint32_t)(-bios_size), >> + bios_size = 0x80000; > > bios_size = 8 * MiB?
The next line has base address 0xFFF80000. I picked 0x80000 to make 0xFFF80000 + 0x80000 == 0 mod 2^32 more obvious. If I change 0x80000 to 8 * MiB, the size is more obvious, but "at end of 32 bit address space" less so. If I additionally change the base address back to ((uint32_t)-bios_size, "at end of 32 bit address space" is obvious again, but the actual base address less so. I don't really care myself. David, you're the maintainer, do you have a preference? >> + pflash_cfi02_register(0xFFF80000, >> NULL, "ef405ep.bios", bios_size, >> - blk, 65536, fl_sectors, 1, >> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, >> + 65536, bios_size / 65536, 1, > > 64 * KiB? David, same question (two additional instances below). >> 2, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, >> 0x2AA, >> 1); >> - fl_idx++; >> } else >> #endif >> { >> @@ -455,7 +448,7 @@ static void taihu_405ep_init(MachineState *machine) >> target_ulong kernel_base, initrd_base; >> long kernel_size, initrd_size; >> int linux_boot; >> - int fl_idx, fl_sectors; >> + int fl_idx; >> DriveInfo *dinfo; >> >> /* RAM is soldered to the board so the size cannot be changed */ >> @@ -486,21 +479,14 @@ static void taihu_405ep_init(MachineState *machine) >> #if defined(USE_FLASH_BIOS) >> dinfo = drive_get(IF_PFLASH, 0, fl_idx); >> if (dinfo) { >> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo); >> - >> - bios_size = blk_getlength(blk); >> - /* XXX: should check that size is 2MB */ >> - // bios_size = 2 * 1024 * 1024; >> - fl_sectors = (bios_size + 65535) >> 16; >> #ifdef DEBUG_BOARD_INIT >> - printf("Register parallel flash %d size %lx" >> - " at addr %lx '%s' %d\n", >> - fl_idx, bios_size, -bios_size, >> - blk_name(blk), fl_sectors); >> + printf("Register boot flash\n"); >> #endif >> - pflash_cfi02_register((uint32_t)(-bios_size), >> + bios_size = 2 * MiB; >> + pflash_cfi02_register(0xFFE00000, >> NULL, "taihu_405ep.bios", bios_size, >> - blk, 65536, fl_sectors, 1, >> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, >> + 65536, bios_size / 65536, 1, > > 64 * KiB > >> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, >> 0x2AA, >> 1); >> fl_idx++; >> @@ -536,20 +522,13 @@ static void taihu_405ep_init(MachineState *machine) >> /* Register Linux flash */ >> dinfo = drive_get(IF_PFLASH, 0, fl_idx); >> if (dinfo) { >> - BlockBackend *blk = blk_by_legacy_dinfo(dinfo); >> - >> - bios_size = blk_getlength(blk); >> - /* XXX: should check that size is 32MB */ >> bios_size = 32 * MiB; >> - fl_sectors = (bios_size + 65535) >> 16; >> #ifdef DEBUG_BOARD_INIT >> - printf("Register parallel flash %d size %lx" >> - " at addr " TARGET_FMT_lx " '%s'\n", >> - fl_idx, bios_size, (target_ulong)0xfc000000, >> - blk_name(blk)); >> + printf("Register application flash\n" >> #endif >> pflash_cfi02_register(0xfc000000, NULL, "taihu_405ep.flash", >> bios_size, >> - blk, 65536, fl_sectors, 1, >> + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, >> + 65536, bios_size / 65536, 1, > > 64 * KiB > >> 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, >> 0x2AA, >> 1); >> fl_idx++; > > > -- > Alex Bennée