Re: [PATCH v2 05/12] hw/pci-host/bonito: Do not use SysBus API to map local MMIO region

[Philippe Mathieu-Daudé]

On 19/10/23 09:26, Thomas Huth wrote:
> On 19/10/2023 09.16, Philippe Mathieu-Daudé wrote:
> > There is no point in exposing an internal MMIO region via
> > SysBus and directly mapping it in the very same device.
> >
> > Just map it without using the SysBus API.
> >
> > Transformation done using the following coccinelle script:
> >
> >    @@
> >    expression sbdev;
> >    expression index;
> >    expression addr;
> >    expression subregion;
> >    @@
> >    -    sysbus_init_mmio(sbdev, subregion);
> >         ... when != sbdev
> >    -    sysbus_mmio_map(sbdev, index, addr);
> >    +    memory_region_add_subregion(get_system_memory(), addr, 
> > subregion);
> >
> > and manually adding the local 'host_mem' variable to
> > avoid multiple calls to get_system_memory().
>
> Thanks for updating it!
>
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > ---
> >   hw/pci-host/bonito.c | 32 ++++++++++++++++----------------
> >   1 file changed, 16 insertions(+), 16 deletions(-)


> >       /* PCI copier */
> >       memory_region_init_io(&s->iomem_cop, OBJECT(s), &bonito_cop_ops, s,
> >                             "cop", 0x100);
> > -    sysbus_init_mmio(sysbus, &s->iomem_cop);
> > -    sysbus_mmio_map(sysbus, 4, 0x1fe00300);
> > +    memory_region_add_subregion(host_mem, 0x1fe00300,
> > +                                &s->iomem_cop);
>
> At least the above two hunks look like they could now fit into one line?

This file will be heavily reworked soon, but meanwhile I did what you
suggested.

> Anyway:
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!