Re: [Qemu-block] [PATCH 4/5] hw/block/pflash_cfi02: Extract the pflash_reset() code

[Philippe Mathieu-Daudé]

On 5/6/19 4:57 PM, Laszlo Ersek wrote:
> On 05/05/19 22:06, Philippe Mathieu-Daudé wrote:
>> The reset() code is used in various places, refactor it.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>> ---
>>  hw/block/pflash_cfi02.c | 25 +++++++++++++++----------
>>  1 file changed, 15 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
>> index f2c6201f813..f321b74433c 100644
>> --- a/hw/block/pflash_cfi02.c
>> +++ b/hw/block/pflash_cfi02.c
>> @@ -120,6 +120,17 @@ static void pflash_register_memory(PFlashCFI02 *pfl, 
>> int rom_mode)
>>      pfl->rom_mode = rom_mode;
>>  }
>>  
>> +static void pflash_reset(PFlashCFI02 *pfl)
>> +{
>> +    trace_pflash_reset();
>> +    timer_del(&pfl->timer);
>> +    pfl->bypass = 0;
>> +    pfl->wcycle = 0;
>> +    pfl->cmd = 0;
>> +    pfl->status = 0;
>> +    pflash_register_memory(pfl, 1);
>> +}
>> +
>>  static void pflash_timer (void *opaque)
>>  {
>>      PFlashCFI02 *pfl = opaque;
>> @@ -129,11 +140,10 @@ static void pflash_timer (void *opaque)
>>      pfl->status ^= 0x80;
>>      if (pfl->bypass) {
>>          pfl->wcycle = 2;
>> +        pfl->cmd = 0;
>>      } else {
>> -        pflash_register_memory(pfl, 1);
>> -        pfl->wcycle = 0;
>> +        pflash_reset(pfl);
>>      }
>> -    pfl->cmd = 0;
>>  }
>>  
>>  static uint32_t pflash_read(PFlashCFI02 *pfl, hwaddr offset,
>> @@ -481,10 +491,7 @@ static void pflash_write(PFlashCFI02 *pfl, hwaddr 
>> offset,
>>  
>>      /* Reset flash */
>>   reset_flash:
>> -    trace_pflash_reset();
>> -    pfl->bypass = 0;
>> -    pfl->wcycle = 0;
>> -    pfl->cmd = 0;
>> +    pflash_reset(pfl);
>>      return;
>>  
>>   do_bypass:
>> @@ -588,9 +595,7 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
>> **errp)
>>      sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
>>  
>>      timer_init_ns(&pfl->timer, QEMU_CLOCK_VIRTUAL, pflash_timer, pfl);
>> -    pfl->wcycle = 0;
>> -    pfl->cmd = 0;
>> -    pfl->status = 0;
>> +    pflash_reset(pfl);
>>      /* Hardcoded CFI table (mostly from SG29 Spansion flash) */
>>      /* Standard "QRY" string */
>>      pfl->cfi_table[0x10] = 'Q';
>>
> 
> I don't have a vested interest in the pflash_cfi02 device model, but I
> guess my earlier (cfi01) comments would apply -- unify first, extract
> second. (Or at least document why these changes are unobservable from
> the behavior POV.)

Ah sorry I forgot to add "Laszlo, please only review patches 1-3" in the
cover :( I use git-publish that send all the patches of the series to
all the list. I might start to add 'Cc:' tags in my patches.

> Thanks
> Laszlo
>