On 2011-08-24 15:40, Avi Kivity wrote: > cfi02 is annoying in that is ignores some address bits; we probably > want explicit support in the memory API for that.
... > diff --git a/hw/musicpal.c b/hw/musicpal.c > index 63dd391..5e74ee7 100644 > --- a/hw/musicpal.c > +++ b/hw/musicpal.c > @@ -1502,6 +1502,7 @@ static void musicpal_init(ram_addr_t ram_size, > unsigned long flash_size; > DriveInfo *dinfo; > ram_addr_t sram_off; > + MemoryRegion *flash = g_new(MemoryRegion, 1); > > if (!cpu_model) { > cpu_model = "arm926"; > @@ -1565,21 +1566,23 @@ static void musicpal_init(ram_addr_t ram_size, > * image is smaller than 32 MB. > */ > #ifdef TARGET_WORDS_BIGENDIAN > - pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, qemu_ram_alloc(NULL, > - "musicpal.flash", flash_size), > + memory_region_init_rom_device(flash, &pflash_cfi02_ops_be, > + NULL, "musicpal.flash", flash_size); > + pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, flash, > dinfo->bdrv, 0x10000, > (flash_size + 0xffff) >> 16, > MP_FLASH_SIZE_MAX / flash_size, > 2, 0x00BF, 0x236D, 0x0000, 0x0000, > - 0x5555, 0x2AAA, 1); > + 0x5555, 0x2AAA); > #else > - pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, qemu_ram_alloc(NULL, > - "musicpal.flash", flash_size), > + memory_region_init_rom_device(flash, &pflash_cfi02_ops_le, > + NULL, "musicpal.flash", flash_size); > + pflash_cfi02_register(0-MP_FLASH_SIZE_MAX, flash, > dinfo->bdrv, 0x10000, > (flash_size + 0xffff) >> 16, > MP_FLASH_SIZE_MAX / flash_size, > 2, 0x00BF, 0x236D, 0x0000, 0x0000, > - 0x5555, 0x2AAA, 0); > + 0x5555, 0x2AAA); > #endif This would deserve some reordering to reduce the code under #ifdef. Anyway, it also breaks the musicpal (and presumably all other flash users) because memory_region_init_rom_device is broken. It assumes mr->ops is set, but I guess it rather should set that field itself, no? Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux