Re: [RFC PATCH 2/3] hw/block/pflash: Move code around

[David Edmondson]

On Friday, 2021-02-26 at 00:02:37 +01, Philippe Mathieu-Daudé wrote:

> First do the block checks, so we know if it is read-only or not.
> Then create the MemoryRegion. This will allow optimization in
> the next commit.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Edmondson <david.edmondson@oracle.com>

> ---
>  hw/block/pflash_cfi01.c | 24 ++++++++++++------------
>  hw/block/pflash_cfi02.c | 18 +++++++++---------
>  2 files changed, 21 insertions(+), 21 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index 22287a1522e..a5fa8d8b74a 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -731,18 +731,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
> **errp)
>      }
>      device_len = sector_len_per_device * blocks_per_device;
>  
> -    memory_region_init_rom_device(
> -        &pfl->mem, OBJECT(dev),
> -        &pflash_cfi01_ops,
> -        pfl,
> -        pfl->name, total_len, errp);
> -    if (*errp) {
> -        return;
> -    }
> -
> -    pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
> -    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> -
>      if (pfl->blk) {
>          uint64_t perm;
>          pfl->ro = !blk_supports_write_perm(pfl->blk);
> @@ -755,6 +743,18 @@ static void pflash_cfi01_realize(DeviceState *dev, Error 
> **errp)
>          pfl->ro = 0;
>      }
>  
> +    memory_region_init_rom_device(
> +        &pfl->mem, OBJECT(dev),
> +        &pflash_cfi01_ops,
> +        pfl,
> +        pfl->name, total_len, errp);
> +    if (*errp) {
> +        return;
> +    }
> +
> +    pfl->storage = memory_region_get_ram_ptr(&pfl->mem);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(dev), &pfl->mem);
> +
>      if (pfl->blk) {
>          if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len,
>                                           errp)) {
> diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c
> index 7962cff7455..4f62ce8917d 100644
> --- a/hw/block/pflash_cfi02.c
> +++ b/hw/block/pflash_cfi02.c
> @@ -791,15 +791,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
> **errp)
>          return;
>      }
>  
> -    memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
> -                                  &pflash_cfi02_ops, pfl, pfl->name,
> -                                  pfl->chip_len, errp);
> -    if (*errp) {
> -        return;
> -    }
> -
> -    pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
> -
>      if (pfl->blk) {
>          uint64_t perm;
>          pfl->ro = !blk_supports_write_perm(pfl->blk);
> @@ -812,6 +803,15 @@ static void pflash_cfi02_realize(DeviceState *dev, Error 
> **errp)
>          pfl->ro = 0;
>      }
>  
> +    memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl),
> +                                  &pflash_cfi02_ops, pfl, pfl->name,
> +                                  pfl->chip_len, errp);
> +    if (*errp) {
> +        return;
> +    }
> +
> +    pfl->storage = memory_region_get_ram_ptr(&pfl->orig_mem);
> +
>      if (pfl->blk) {
>          if (!blk_check_size_and_read_all(pfl->blk, pfl->storage,
>                                           pfl->chip_len, errp)) {
> -- 
> 2.26.2

dme.
-- 
She's as sweet as Tupelo honey, she's an angel of the first degree.