[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory()
From: |
Peter Maydell |
Subject: |
Re: [PATCH 0/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory() |
Date: |
Mon, 15 Mar 2021 12:08:58 +0000 |
On Mon, 15 Mar 2021 at 11:34, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> 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.
I agree that sysbus_mmio_get_region()'s use of arbitrary
integers is unfortunate (we should look at improving that
to use usefully named regions I guess), but I don't see
why pflash_cfi01 should expose its MemoryRegion to users
in a different way to every other sysbus device.
thanks
-- PMM
- [PATCH 0/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory(), Philippe Mathieu-Daudé, 2021/03/07
- [PATCH 1/4] hw/i386/pc: Get pflash MemoryRegion with sysbus_mmio_get_region(), Philippe Mathieu-Daudé, 2021/03/07
- [PATCH 2/4] hw/mips/malta: Get pflash MemoryRegion with sysbus_mmio_get_region(), Philippe Mathieu-Daudé, 2021/03/07
- [PATCH 3/4] hw/xtensa/xtfpga: Get pflash MemoryRegion with sysbus_mmio_get_region(), Philippe Mathieu-Daudé, 2021/03/07
- [PATCH 4/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory(), Philippe Mathieu-Daudé, 2021/03/07
- Re: [PATCH 0/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory(), Paolo Bonzini, 2021/03/15
- Re: [PATCH 0/4] hw/block/pflash_cfi01: Remove pflash_cfi01_get_memory(),
Peter Maydell <=