From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:57538) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwYFB-0004jU-KB for qemu-devel@nongnu.org; Thu, 25 Aug 2011 07:37:10 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QwYFA-0006bm-FD for qemu-devel@nongnu.org; Thu, 25 Aug 2011 07:37:09 -0400 Received: from thoth.sbs.de ([192.35.17.2]:24365) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwYFA-0006QA-5y for qemu-devel@nongnu.org; Thu, 25 Aug 2011 07:37:08 -0400 Message-ID: <4E5633E0.3010400@siemens.com> Date: Thu, 25 Aug 2011 13:37:04 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1314193259-27092-1-git-send-email-avi@redhat.com> <1314193259-27092-15-git-send-email-avi@redhat.com> In-Reply-To: <1314193259-27092-15-git-send-email-avi@redhat.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 14/22] pflash_cfi01/pflash_cfi02: convert to memory API List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Avi Kivity Cc: qemu-devel@nongnu.org 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