From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=40290 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q8sH2-0002wW-DM for qemu-devel@nongnu.org; Sun, 10 Apr 2011 06:53:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q8sH1-0005HA-BL for qemu-devel@nongnu.org; Sun, 10 Apr 2011 06:53:44 -0400 Received: from fmmailgate01.web.de ([217.72.192.221]:35224) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q8sH0-0005Ge-UT for qemu-devel@nongnu.org; Sun, 10 Apr 2011 06:53:43 -0400 Message-ID: <4DA18C33.6050002@web.de> Date: Sun, 10 Apr 2011 12:53:39 +0200 From: Jan Kiszka MIME-Version: 1.0 References: <1301861786-6637-1-git-send-email-jordan.l.justen@intel.com> <4DA16C6A.7090303@web.de> In-Reply-To: <4DA16C6A.7090303@web.de> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Sender: jan.kiszka@web.de Subject: [Qemu-devel] [PATCH] pflash: Restore & fix lazy ROMD switching List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jordan Justen Cc: qemu-devel@nongnu.org, Aurelien Jarno On 2011-04-10 10:38, Jan Kiszka wrote: > On 2011-04-03 22:16, Jordan Justen wrote: >> When checking pfl->rom_mode for when to lazily reenter ROMD mode, >> the value was check was the opposite of what it should have been. >> This prevent the part from returning to ROMD mode after a write >> was made to the CFI rom region. >> >> Signed-off-by: Jordan Justen >> --- >> hw/pflash_cfi02.c | 2 +- >> 1 files changed, 1 insertions(+), 1 deletions(-) >> >> diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c >> index 30c8aa4..370c5ee 100644 >> --- a/hw/pflash_cfi02.c >> +++ b/hw/pflash_cfi02.c >> @@ -112,7 +112,7 @@ static uint32_t pflash_read (pflash_t *pfl, target_phys_addr_t offset, >> >> DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset); >> ret = -1; >> - if (pfl->rom_mode) { >> + if (!pfl->rom_mode) { >> /* Lazy reset of to ROMD mode */ >> if (pfl->wcycle == 0) >> pflash_register_memory(pfl, 1); > > Indeed, that block looks weird to its author today as well. But > inverting the logic completely defeats the purpose of lazy mode > switching (will likely file a patch to remove the block). We should > instead switch back using the timer. That was wishful thinking. We were actually lacking a switch-back on read, something that the old twisted logic was papering over. Patch below should resolve that more gracefully, at least it does so here. Jan ------8<------ Commit 5145b3d1cc revealed a bug in the lazy ROMD switch-back logic, but resolved it by breaking that feature. This approach addresses the issue by switching back to ROMD after a certain amount of read accesses without further unlock sequences. Signed-off-by: Jan Kiszka --- hw/pflash_cfi02.c | 12 ++++++++---- 1 files changed, 8 insertions(+), 4 deletions(-) diff --git a/hw/pflash_cfi02.c b/hw/pflash_cfi02.c index 370c5ee..14bbc34 100644 --- a/hw/pflash_cfi02.c +++ b/hw/pflash_cfi02.c @@ -50,6 +50,8 @@ do { \ #define DPRINTF(fmt, ...) do { } while (0) #endif +#define PFLASH_LAZY_ROMD_THRESHOLD 42 + struct pflash_t { BlockDriverState *bs; target_phys_addr_t base; @@ -70,6 +72,7 @@ struct pflash_t { ram_addr_t off; int fl_mem; int rom_mode; + int read_counter; /* used for lazy switch-back to rom mode */ void *storage; }; @@ -112,10 +115,10 @@ static uint32_t pflash_read (pflash_t *pfl, target_phys_addr_t offset, DPRINTF("%s: offset " TARGET_FMT_plx "\n", __func__, offset); ret = -1; - if (!pfl->rom_mode) { - /* Lazy reset of to ROMD mode */ - if (pfl->wcycle == 0) - pflash_register_memory(pfl, 1); + /* Lazy reset to ROMD mode after a certain amount of read accesses */ + if (!pfl->rom_mode && pfl->wcycle == 0 && + ++pfl->read_counter > PFLASH_LAZY_ROMD_THRESHOLD) { + pflash_register_memory(pfl, 1); } offset &= pfl->chip_len - 1; boff = offset & 0xFF; @@ -254,6 +257,7 @@ static void pflash_write (pflash_t *pfl, target_phys_addr_t offset, /* Set the device in I/O access mode if required */ if (pfl->rom_mode) pflash_register_memory(pfl, 0); + pfl->read_counter = 0; /* We're in read mode */ check_unlock0: if (boff == 0x55 && cmd == 0x98) { -- 1.7.1