Re: [PATCH 7/9] hw/block/pflash_cfi02: Factor out DeviceReset method

[Bin Meng <bmeng.cn@gmail.com>]

On Wed, Mar 10, 2021 at 7:55 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> There is multiple places doing a device reset. Factor that
> out in a common method which matches the DeviceReset prototype,
> so we can also remove the reset code from the DeviceRealize()
> handler. Explicit the device is set in "read array" mode on
> reset.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  hw/block/pflash_cfi02.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 2ba77a0171b..484b056b898 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -193,6 +193,14 @@ static void pflash_mode_read_array(PFlashCFI02 *pfl)
>      memory_region_rom_device_set_romd(&pfl->orig_mem, true);
>  }
>
> +static void pflash_cfi02_reset(DeviceState *dev)
> +{
> +    PFlashCFI02 *pfl = PFLASH_CFI02(dev);
> +
> +    trace_pflash_reset();
> +    pflash_mode_read_array(pfl);
> +}
> +
>  static size_t pflash_regions_count(PFlashCFI02 *pfl)
>  {
>      return pfl->cfi_table[0x2c];
> @@ -330,8 +338,7 @@ static uint64_t pflash_read(void *opaque, hwaddr offset, unsigned int width)
>      default:
>          /* This should never happen : reset state & treat it as a read*/
>          DPRINTF("%s: unknown command state: %x\n", __func__, pfl->cmd);
> -        pfl->wcycle = 0;
> -        pfl->cmd = 0;
> +        pflash_cfi02_reset(DEVICE(pfl));
>          /* fall through to the read code */
>      case 0x80: /* Erase (unlock) */
>          /* We accept reads during second unlock sequence... */
> @@ -710,10 +717,8 @@ static void pflash_write(void *opaque, hwaddr offset, uint64_t value,
>
>      /* Reset flash */
>   reset_flash:
> -    trace_pflash_reset();
>      pfl->bypass = 0;
> -    pfl->wcycle = 0;
> -    pfl->cmd = 0;
> +    pflash_cfi02_reset(DEVICE(pfl));

The old codes did not set pfl->rom_mode to true, but the new codes
pflash_cfi02_reset() do. Is this correct?

>      return;
>
>   do_bypass:
> @@ -977,6 +982,7 @@ static void pflash_cfi02_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>
>      dc->realize = pflash_cfi02_realize;
> +    dc->reset = pflash_cfi02_reset;
>      dc->unrealize = pflash_cfi02_unrealize;
>      device_class_set_props(dc, pflash_cfi02_properties);
>      set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
> --

Regards,
Bin