On 2/18/19 1:56 PM, Markus Armbruster wrote: > pflash_cfi01_register() takes a size in bytes, a block size in bytes > and a number of blocks. mips_malta_init() passes BIOS_SIZE, 65536, > FLASH_SIZE >> 16. Actually consistent only because BIOS_SIZE (defined > in include/hw/mips/bios.h as (4 * MiB)) matches FLASH_SIZE (defined > locally as 0x400000). Confusing all the same. > > Pass FLASH_SIZE instead of BIOS_SIZE.
Your cleanup is correct. > > There are more uses of BIOS_SIZE, but I don't sufficiently understand > them to attempt cleanup. BIOS_SIZE is a MIPS architecture definition, incorrectly used around. This simply means "Top of 32bit address space - Boot Vector address". There is nothing wrong in plugging bigger/smaller flashes around the boot vector. (I have a series cleaning this definition, but I'm throttling my MIPS apports). > > Cc: Aurelien Jarno <aurel...@aurel32.net> > Cc: Aleksandar Rikalo <arik...@wavecomp.com> > Signed-off-by: Markus Armbruster <arm...@redhat.com> > --- > hw/mips/mips_malta.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c > index fff5ed19bd..65cdda4881 100644 > --- a/hw/mips/mips_malta.c > +++ b/hw/mips/mips_malta.c > @@ -1269,12 +1269,12 @@ void mips_malta_init(MachineState *machine) > if (dinfo) { > printf("Register parallel flash %d size " TARGET_FMT_lx " at " > "addr %08llx '%s' %x\n", > - fl_idx, bios_size, FLASH_ADDRESS, > + fl_idx, FLASH_SIZE, FLASH_ADDRESS, > blk_name(dinfo->bdrv), fl_sectors); > } > #endif > fl = pflash_cfi01_register(FLASH_ADDRESS, NULL, "mips_malta.bios", > - BIOS_SIZE, > + FLASH_SIZE, > dinfo ? blk_by_legacy_dinfo(dinfo) : NULL, > 65536, fl_sectors, > 4, 0x0000, 0x0000, 0x0000, 0x0000, be); > Does that mean you can remove the "hw/mips/bios.h" include now? Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com>