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é <phi...@redhat.com>
>> ---
>> 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
>
