Re: [PATCH 0/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory()

[Paolo Bonzini <pbonzini@redhat.com>]

On 07/03/21 23:26, Philippe Mathieu-Daudé wrote:
> TYPE_PFLASH_CFI01 is a TYPE_SYS_BUS_DEVICE which registers its romd
> MemoryRegion with sysbus_init_mmio(), so we can use the generic
> sysbus_mmio_get_region() to get the region, no need for a specific
> pflash_cfi01_get_memory() helper.
> 
> First replace the few pflash_cfi01_get_memory() uses by
> sysbus_mmio_get_region(), then remove the now unused helper.

Why is this an improvement?  You're replacing nice and readable code 
with an implementation-dependent function whose second argument is a 
magic number.  The right patch would _add_ more of these helpers, not 
remove them.

Paolo

> Philippe Mathieu-Daudé (4):
>    hw/i386/pc: Get pflash MemoryRegion with sysbus_mmio_get_region()
>    hw/mips/malta: Get pflash MemoryRegion with sysbus_mmio_get_region()
>    hw/xtensa/xtfpga: Get pflash MemoryRegion with
>      sysbus_mmio_get_region()
>    hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory()
> 
>   include/hw/block/flash.h | 1 -
>   hw/block/pflash_cfi01.c  | 5 -----
>   hw/i386/pc_sysfw.c       | 2 +-
>   hw/mips/malta.c          | 2 +-
>   hw/xtensa/xtfpga.c       | 3 ++-
>   5 files changed, 4 insertions(+), 9 deletions(-)
>