From: BALATON Zoltan <balaton@eik.bme.hu>
To: Jiaxun Yang <jiaxun.yang@flygoat.com>
Cc: aleksandar.rikalo@syrmia.com, qemu-devel@nongnu.org,
paulburton@kernel.org, f4bug@amsat.org
Subject: Re: [PATCH v2 1/3] hw/mips/boston: Massage memory map information
Date: Wed, 29 Sep 2021 17:32:49 +0200 (CEST) [thread overview]
Message-ID: <b1c25fe-c4af-264c-7a8e-fedeb272e845@eik.bme.hu> (raw)
In-Reply-To: <20210929151211.108-2-jiaxun.yang@flygoat.com>
[-- Attachment #1: Type: text/plain, Size: 8266 bytes --]
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?
Regards,
BALATON Zoltan
> + [BOSTON_PCIE0] = { 0x10000000, 0x2000000 },
> + [BOSTON_PCIE1] = { 0x12000000, 0x2000000 },
> + [BOSTON_PCIE2] = { 0x14000000, 0x2000000 },
> + [BOSTON_PCIE2_MMIO] = { 0x16000000, 0x100000 },
> + [BOSTON_CM] = { 0x16100000, 0x20000 },
> + [BOSTON_GIC] = { 0x16120000, 0x20000 },
> + [BOSTON_CDMM] = { 0x16140000, 0x8000 },
> + [BOSTON_CPC] = { 0x16200000, 0x8000 },
> + [BOSTON_PLATREG] = { 0x17ffd000, 0x1000 },
> + [BOSTON_UART] = { 0x17ffe000, 0x20 },
> + [BOSTON_LCD] = { 0x17fff000, 0x8 },
> + [BOSTON_FLASH] = { 0x18000000, 0x8000000 },
> + [BOSTON_PCIE1_MMIO] = { 0x20000000, 0x20000000 },
> + [BOSTON_PCIE0_MMIO] = { 0x40000000, 0x40000000 },
> + [BOSTON_HIGHDDR] = { 0x80000000, 0x0 },
> +};
> +
> enum boston_plat_reg {
> PLAT_FPGA_BUILD = 0x00,
> PLAT_CORE_CL = 0x04,
> @@ -275,24 +313,22 @@ type_init(boston_register_types)
>
> static void gen_firmware(uint32_t *p, hwaddr kernel_entry, hwaddr fdt_addr)
> {
> - const uint32_t cm_base = 0x16100000;
> - const uint32_t gic_base = 0x16120000;
> - const uint32_t cpc_base = 0x16200000;
> -
> /* Move CM GCRs */
> bl_gen_write_ulong(&p,
> cpu_mips_phys_to_kseg1(NULL, GCR_BASE_ADDR + GCR_BASE_OFS),
> - cm_base);
> + boston_memmap[BOSTON_CM].base);
>
> /* Move & enable GIC GCRs */
> bl_gen_write_ulong(&p,
> - cpu_mips_phys_to_kseg1(NULL, cm_base + GCR_GIC_BASE_OFS),
> - gic_base | GCR_GIC_BASE_GICEN_MSK);
> + cpu_mips_phys_to_kseg1(NULL,
> + boston_memmap[BOSTON_CM].base + GCR_GIC_BASE_OFS),
> + boston_memmap[BOSTON_GIC].base | GCR_GIC_BASE_GICEN_MSK);
>
> /* Move & enable CPC GCRs */
> bl_gen_write_ulong(&p,
> - cpu_mips_phys_to_kseg1(NULL, cm_base + GCR_CPC_BASE_OFS),
> - cpc_base | GCR_CPC_BASE_CPCEN_MSK);
> + cpu_mips_phys_to_kseg1(NULL,
> + boston_memmap[BOSTON_CM].base + GCR_CPC_BASE_OFS),
> + boston_memmap[BOSTON_CPC].base | GCR_CPC_BASE_CPCEN_MSK);
>
> /*
> * Setup argument registers to follow the UHI boot protocol:
> @@ -333,8 +369,9 @@ static const void *boston_fdt_filter(void *opaque, const void *fdt_orig,
> ram_low_sz = MIN(256 * MiB, machine->ram_size);
> ram_high_sz = machine->ram_size - ram_low_sz;
> qemu_fdt_setprop_sized_cells(fdt, "/memory@0", "reg",
> - 1, 0x00000000, 1, ram_low_sz,
> - 1, 0x90000000, 1, ram_high_sz);
> + 1, boston_memmap[BOSTON_LOWDDR].base, 1, ram_low_sz,
> + 1, boston_memmap[BOSTON_HIGHDDR].base + ram_low_sz,
> + 1, ram_high_sz);
>
> fdt = g_realloc(fdt, fdt_totalsize(fdt));
> qemu_fdt_dumpdtb(fdt, fdt_sz);
> @@ -438,11 +475,13 @@ static void boston_mach_init(MachineState *machine)
> sysbus_mmio_map_overlap(SYS_BUS_DEVICE(&s->cps), 0, 0, 1);
>
> flash = g_new(MemoryRegion, 1);
> - memory_region_init_rom(flash, NULL, "boston.flash", 128 * MiB,
> + memory_region_init_rom(flash, NULL, "boston.flash", boston_memmap[BOSTON_FLASH].size,
> &error_fatal);
> - memory_region_add_subregion_overlap(sys_mem, 0x18000000, flash, 0);
> + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_FLASH].base,
> + flash, 0);
>
> - memory_region_add_subregion_overlap(sys_mem, 0x80000000, machine->ram, 0);
> + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_HIGHDDR].base,
> + machine->ram, 0);
>
> ddr_low_alias = g_new(MemoryRegion, 1);
> memory_region_init_alias(ddr_low_alias, NULL, "boston_low.ddr",
> @@ -451,32 +490,40 @@ static void boston_mach_init(MachineState *machine)
> memory_region_add_subregion_overlap(sys_mem, 0, ddr_low_alias, 0);
>
> xilinx_pcie_init(sys_mem, 0,
> - 0x10000000, 32 * MiB,
> - 0x40000000, 1 * GiB,
> + boston_memmap[BOSTON_PCIE0].base,
> + boston_memmap[BOSTON_PCIE0].size,
> + boston_memmap[BOSTON_PCIE0_MMIO].base,
> + boston_memmap[BOSTON_PCIE0_MMIO].size,
> get_cps_irq(&s->cps, 2), false);
>
> xilinx_pcie_init(sys_mem, 1,
> - 0x12000000, 32 * MiB,
> - 0x20000000, 512 * MiB,
> + boston_memmap[BOSTON_PCIE1].base,
> + boston_memmap[BOSTON_PCIE1].size,
> + boston_memmap[BOSTON_PCIE1_MMIO].base,
> + boston_memmap[BOSTON_PCIE1_MMIO].size,
> get_cps_irq(&s->cps, 1), false);
>
> pcie2 = xilinx_pcie_init(sys_mem, 2,
> - 0x14000000, 32 * MiB,
> - 0x16000000, 1 * MiB,
> + boston_memmap[BOSTON_PCIE2].base,
> + boston_memmap[BOSTON_PCIE2].size,
> + boston_memmap[BOSTON_PCIE2_MMIO].base,
> + boston_memmap[BOSTON_PCIE2_MMIO].size,
> get_cps_irq(&s->cps, 0), true);
>
> platreg = g_new(MemoryRegion, 1);
> memory_region_init_io(platreg, NULL, &boston_platreg_ops, s,
> - "boston-platregs", 0x1000);
> - memory_region_add_subregion_overlap(sys_mem, 0x17ffd000, platreg, 0);
> + "boston-platregs",
> + boston_memmap[BOSTON_PLATREG].size);
> + memory_region_add_subregion_overlap(sys_mem,
> + boston_memmap[BOSTON_PLATREG].base, platreg, 0);
>
> - s->uart = serial_mm_init(sys_mem, 0x17ffe000, 2,
> + s->uart = serial_mm_init(sys_mem, boston_memmap[BOSTON_UART].base, 2,
> get_cps_irq(&s->cps, 3), 10000000,
> serial_hd(0), DEVICE_NATIVE_ENDIAN);
>
> lcd = g_new(MemoryRegion, 1);
> memory_region_init_io(lcd, NULL, &boston_lcd_ops, s, "boston-lcd", 0x8);
> - memory_region_add_subregion_overlap(sys_mem, 0x17fff000, lcd, 0);
> + memory_region_add_subregion_overlap(sys_mem, boston_memmap[BOSTON_LCD].base, lcd, 0);
>
> chr = qemu_chr_new("lcd", "vc:320x240", NULL);
> qemu_chr_fe_init(&s->lcd_display, chr, NULL);
>
next prev parent reply other threads:[~2021-09-29 15:35 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-29 15:12 [PATCH v2 0/3] hw/mips/boston: ELF kernel support Jiaxun Yang
2021-09-29 15:12 ` [PATCH v2 1/3] hw/mips/boston: Massage memory map information Jiaxun Yang
2021-09-29 15:32 ` BALATON Zoltan [this message]
2021-09-29 16:41 ` Jiaxun Yang
2021-09-29 15:12 ` [PATCH v2 2/3] hw/mips/boston: Allow loading elf kernel and dtb Jiaxun Yang
2021-09-29 15:36 ` BALATON Zoltan
2021-09-29 16:44 ` Jiaxun Yang
2021-09-29 15:12 ` [PATCH v2 3/3] hw/mips/boston: Add FDT generator Jiaxun Yang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b1c25fe-c4af-264c-7a8e-fedeb272e845@eik.bme.hu \
--to=balaton@eik.bme.hu \
--cc=aleksandar.rikalo@syrmia.com \
--cc=f4bug@amsat.org \
--cc=jiaxun.yang@flygoat.com \
--cc=paulburton@kernel.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).