Re: [PATCH v2 1/3] hw/mips/boston: Massage memory map information

[Jiaxun Yang]

在 2021/9/29 16:32, BALATON Zoltan 写道:
> On Wed, 29 Sep 2021, Jiaxun Yang wrote:
> > Use memmap array to uinfy address of memory map.
> > That would allow us reuse address information for FDT generation.
> >
> > Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > --
> > v2: Fix minor style issue, fix uart map size
> > ---
> > hw/mips/boston.c | 95 ++++++++++++++++++++++++++++++++++++------------
> > 1 file changed, 71 insertions(+), 24 deletions(-)
> >
> > diff --git a/hw/mips/boston.c b/hw/mips/boston.c
> > index 20b06865b2..5c720440fb 100644
> > --- a/hw/mips/boston.c
> > +++ b/hw/mips/boston.c
> > @@ -64,6 +64,44 @@ struct BostonState {
> >     hwaddr fdt_base;
> > };
> >
> > +enum {
> > +    BOSTON_LOWDDR,
> > +    BOSTON_PCIE0,
> > +    BOSTON_PCIE1,
> > +    BOSTON_PCIE2,
> > +    BOSTON_PCIE2_MMIO,
> > +    BOSTON_CM,
> > +    BOSTON_GIC,
> > +    BOSTON_CDMM,
> > +    BOSTON_CPC,
> > +    BOSTON_PLATREG,
> > +    BOSTON_UART,
> > +    BOSTON_LCD,
> > +    BOSTON_FLASH,
> > +    BOSTON_PCIE1_MMIO,
> > +    BOSTON_PCIE0_MMIO,
> > +    BOSTON_HIGHDDR,
> > +};
> > +
> > +static const MemMapEntry boston_memmap[] = {
> > +    [BOSTON_LOWDDR] =     {        0x0,    0x10000000 },
>
> What's the advantage of having it in an array as opposed to just have 
> simple defines for these? I did not see a case where you go through 
> these as array elements so it seems it's just an unnecessarily complex 
> way to write boston_memmap[BOSTON_LOWADDR] insted of BOSTON_LOWADDR. 
> Did I miss something where having an array helps?
I do consider it as sort of common practice which will make address 
allocation much more clear and easier for subsequent FDT generation 
patch to take both address and size. Many new boards such as arm-virt, 
arm-sbsa and riscv-virt is using MemMapEntry to manage their address 
allocation.

Thanks.
- Jiaxun
> Regards,
> BALATON Zoltan