From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:39588) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEKfH-0000Ow-UG for qemu-devel@nongnu.org; Wed, 19 Sep 2012 09:50:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1TEKf8-0004y6-Bq for qemu-devel@nongnu.org; Wed, 19 Sep 2012 09:50:07 -0400 Received: from mail-bk0-f45.google.com ([209.85.214.45]:48713) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1TEKf8-0004xp-2x for qemu-devel@nongnu.org; Wed, 19 Sep 2012 09:49:58 -0400 Received: by bkcjg9 with SMTP id jg9so362308bkc.4 for ; Wed, 19 Sep 2012 06:49:57 -0700 (PDT) Message-ID: <5059CDCC.8060704@gmail.com> Date: Wed, 19 Sep 2012 15:51:08 +0200 From: Francesco Lavra MIME-Version: 1.0 References: <505782AA.7090106@gmail.com> <50578341.8010308@gmail.com> <5058E0C7.4000908@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 2/2] Versatile Express: add modelling of NOR flash List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org, paul@codesourcery.com On 09/19/2012 12:26 PM, Peter Maydell wrote: > On 18 September 2012 21:59, Francesco Lavra wrote: >> On 09/18/2012 03:46 PM, Peter Maydell wrote: >>> On 17 September 2012 21:08, Francesco Lavra wrote: >>>> qemu_irq pic[64]; >>>> uint32_t proc_id; >>>> uint32_t sys_id; >>>> + DriveInfo *dinfo; >>>> ram_addr_t vram_size, sram_size; >>>> MemoryRegion *sysmem = get_system_memory(); >>>> MemoryRegion *vram = g_new(MemoryRegion, 1); >>>> @@ -410,8 +415,23 @@ static void vexpress_common_init(const VEDBoardInfo >>>> *daughterboard, >>>> >>>> sysbus_create_simple("pl111", map[VE_CLCD], pic[14]); >>>> >>>> - /* VE_NORFLASH0: not modelled */ >>>> - /* VE_NORFLASH1: not modelled */ >>>> + dinfo = drive_get_next(IF_PFLASH); >>>> + if (!pflash_cfi01_register(map[VE_NORFLASH0], NULL, "vexpress.flash0", >>>> + VEXPRESS_FLASH_SIZE, dinfo ? dinfo->bdrv : NULL, >>>> + VEXPRESS_FLASH_SECT_SIZE, >>>> + VEXPRESS_FLASH_SIZE / VEXPRESS_FLASH_SECT_SIZE, 4, >>>> + 0x00, 0x89, 0x00, 0x18, 0)) { >>>> + fprintf(stderr, "vexpress: error registering flash 0.\n"); >>> >>> Shouldn't these errors be fatal? >> >> I checked the existing uses of pflash_cfi_0[1,2]_register() in the code, >> and if I'm not mistaken only in 5 out of 19 devices these errors are >> fatal, in the other 14 cases initialization continues even after flash >> registration failure, with or without an error message. Let me know if >> you still prefer these errors to be fatal. > > So the only reason this can fail is if the user specified a file > to back the flash but trying to read it failed (ie, bad filename > or file not the same size as the flash). I think that merits > actually stopping on error. > > Ideally in the long term the flash devices should be converted > to proper QOM devices and we could push the error handling back > into the device itself (which is better positioned to distinguish > "bad filename" from "wrong length" I suspect). Ok, I will make these errors fatal in v3. -- Francesco