Re: [PATCH 07/15] ppc/ppc405: Add some address space definitions

[BALATON Zoltan]

On Mon, 6 Dec 2021, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> hw/ppc/ppc405.h        |  7 +++++++
> hw/ppc/ppc405_boards.c | 16 +++++++---------
> 2 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/ppc405.h b/hw/ppc/ppc405.h
> index ad5f4026b5db..ea48c3626908 100644
> --- a/hw/ppc/ppc405.h
> +++ b/hw/ppc/ppc405.h
> @@ -27,6 +27,13 @@
>
> #include "hw/ppc/ppc4xx.h"
>
> +#define PPC405EP_SDRAM_BASE 0x00000000
> +#define PPC405EP_NVRAM_BASE 0xF0000000
> +#define PPC405EP_FPGA_BASE  0xF0300000
> +#define PPC405EP_SRAM_BASE  0xFFF00000
> +#define PPC405EP_SRAM_SIZE  (512 * KiB)
> +#define PPC405EP_FLASH_BASE 0xFFF80000

Are these specific to the 405EP SoC itself or the board? Maybe it's better 
to put these in the board .c file and get rid of the ppc405.h later if 
there are no 405 specific parts needed. It's currently included also by 
440 machines because it has some functions re-used by them but those may 
be gone when QOM-ifying these eventually or could be moved to a ppc4xx.h 
instead. I've tried to make a distinction between 405, 440 and 4xx parts 
(the latter common to both) when adding sam460ex but did not finish this 
clean up completely.

Regards,
BALATON Zoltan

> +
> /* Bootinfo as set-up by u-boot */
> typedef struct ppc4xx_bd_info_t ppc4xx_bd_info_t;
> struct ppc4xx_bd_info_t {
> diff --git a/hw/ppc/ppc405_boards.c b/hw/ppc/ppc405_boards.c
> index fcdb6d4cf8a0..60dc81fa4880 100644
> --- a/hw/ppc/ppc405_boards.c
> +++ b/hw/ppc/ppc405_boards.c
> @@ -154,7 +154,6 @@ static void ref405ep_init(MachineState *machine)
>     ram_addr_t bdloc;
>     MemoryRegion *ram_memories = g_new(MemoryRegion, 2);
>     hwaddr ram_bases[2], ram_sizes[2];
> -    target_ulong sram_size;
>     long bios_size;
>     //int phy_addr = 0;
>     //static int phy_addr = 1;
> @@ -187,10 +186,9 @@ static void ref405ep_init(MachineState *machine)
>     env = &cpu->env;
>
>     /* allocate SRAM */
> -    sram_size = 512 * KiB;
> -    memory_region_init_ram(sram, NULL, "ef405ep.sram", sram_size,
> +    memory_region_init_ram(sram, NULL, "ef405ep.sram", PPC405EP_SRAM_SIZE,
>                            &error_fatal);
> -    memory_region_add_subregion(sysmem, 0xFFF00000, sram);
> +    memory_region_add_subregion(sysmem, PPC405EP_SRAM_BASE, sram);
>     /* allocate and load BIOS */
> #ifdef USE_FLASH_BIOS
>     dinfo = drive_get(IF_PFLASH, 0, 0);
> @@ -230,24 +228,24 @@ static void ref405ep_init(MachineState *machine)
>         }
>     }
>     /* Register FPGA */
> -    ref405ep_fpga_init(sysmem, 0xF0300000);
> +    ref405ep_fpga_init(sysmem, PPC405EP_FPGA_BASE);
>     /* Register NVRAM */
>     dev = qdev_new("sysbus-m48t08");
>     qdev_prop_set_int32(dev, "base-year", 1968);
>     s = SYS_BUS_DEVICE(dev);
>     sysbus_realize_and_unref(s, &error_fatal);
> -    sysbus_mmio_map(s, 0, 0xF0000000);
> +    sysbus_mmio_map(s, 0, PPC405EP_NVRAM_BASE);
>     /* Load kernel */
>     linux_boot = (kernel_filename != NULL);
>     if (linux_boot) {
>         memset(&bd, 0, sizeof(bd));
> -        bd.bi_memstart = 0x00000000;
> +        bd.bi_memstart = PPC405EP_SDRAM_BASE;
>         bd.bi_memsize = machine->ram_size;
>         bd.bi_flashstart = -bios_size;
>         bd.bi_flashsize = -bios_size;
>         bd.bi_flashoffset = 0;
> -        bd.bi_sramstart = 0xFFF00000;
> -        bd.bi_sramsize = sram_size;
> +        bd.bi_sramstart = PPC405EP_SRAM_BASE;
> +        bd.bi_sramsize = PPC405EP_SRAM_SIZE;
>         bd.bi_bootflags = 0;
>         bd.bi_intfreq = 133333333;
>         bd.bi_busfreq = 33333333;