On Mon, 15 Mar 2021 at 11:34, Paolo Bonzini <pbonz...@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
