| Message ID | 20190307130323.9353-8-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | pflash: Fixes and cleanups | expand |
On Thu, Mar 07, 2019 at 02:03:16PM +0100, Markus Armbruster wrote: > 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 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 <david@gibson.dropbear.id.au> > Signed-off-by: Markus Armbruster <armbru@redhat.com> > Acked-by: David Gibson <david@gibson.dropbear.id.au> > Reviewed-by: Alex Bennée <alex.bennee@linaro.org> I'm assuming because this is in a series I'm not otherwise CCed on that this is going in through someone else's tree. Let me know if you want me take it through mine. > --- > hw/ppc/ppc405_boards.c | 51 +++++++++++++----------------------------- > 1 file changed, 15 insertions(+), 36 deletions(-) > > diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c > index f47b15f10e..672717ef1b 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 > + bios_size = 8 * MiB; > pflash_cfi02_register((uint32_t)(-bios_size), > NULL, "ef405ep.bios", bios_size, > - blk, 65536, fl_sectors, 1, > + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, > + 64 * KiB, bios_size / (64 * KiB), 1, > 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, > + 64 * KiB, bios_size / (64 * KiB), 1, > 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, > + 64 * KiB, bios_size / (64 * KiB), 1, > 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA, > 1); > fl_idx++;
David Gibson <david@gibson.dropbear.id.au> writes: > On Thu, Mar 07, 2019 at 02:03:16PM +0100, Markus Armbruster wrote: >> 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 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 <david@gibson.dropbear.id.au> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> Acked-by: David Gibson <david@gibson.dropbear.id.au> >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > I'm assuming because this is in a series I'm not otherwise CCed on > that this is going in through someone else's tree. Let me know if you > want me take it through mine. I intend to take the complete series through my tree unless a maintainer objects.
On Fri, Mar 08, 2019 at 08:24:04AM +0100, Markus Armbruster wrote: > David Gibson <david@gibson.dropbear.id.au> writes: > > > On Thu, Mar 07, 2019 at 02:03:16PM +0100, Markus Armbruster wrote: > >> 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 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 <david@gibson.dropbear.id.au> > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > >> Acked-by: David Gibson <david@gibson.dropbear.id.au> > >> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> > > > > I'm assuming because this is in a series I'm not otherwise CCed on > > that this is going in through someone else's tree. Let me know if you > > want me take it through mine. > > I intend to take the complete series through my tree unless a maintainer > objects. No objection here.
diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c index f47b15f10e..672717ef1b 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 + bios_size = 8 * MiB; pflash_cfi02_register((uint32_t)(-bios_size), NULL, "ef405ep.bios", bios_size, - blk, 65536, fl_sectors, 1, + dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, + 64 * KiB, bios_size / (64 * KiB), 1, 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, + 64 * KiB, bios_size / (64 * KiB), 1, 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, + 64 * KiB, bios_size / (64 * KiB), 1, 4, 0x0001, 0x22DA, 0x0000, 0x0000, 0x555, 0x2AA, 1); fl_idx++;