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. Jan
signature.asc
Description: OpenPGP digital signature
