On Tue, Apr 26, 2011 at 09:42:04AM +0200, Jan Kiszka wrote:
> On 2011-04-10 12:53, Jan Kiszka wrote:
> > 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 <jordan.l.jus...@intel.com>
> >>> ---
> >>> 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 <jan.kis...@web.de>
> > ---
> > 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) {
>
> Please apply unless concerns remain.
>
Done.
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
