[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM

From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v5 17/79] arm/integratorcp: use memdev for RAM
Date: Tue, 18 Feb 2020 18:09:56 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.4.1

On 2/18/20 5:41 PM, Igor Mammedov wrote:
On Tue, 18 Feb 2020 07:55:14 +0100
Philippe Mathieu-Daudé <address@hidden> wrote:

On 2/17/20 6:33 PM, Igor Mammedov wrote:
memory_region_allocate_system_memory() API is going away, so
replace it with memdev allocated MemoryRegion. The later is
initialized by generic code, so board only needs to opt in
to memdev scheme by providing
and using MachineState::ram instead of manually initializing
RAM memory region.

Signed-off-by: Igor Mammedov <address@hidden>
Reviewed-by: Andrew Jones <address@hidden>
CC: address@hidden
   hw/arm/integratorcp.c | 9 ++++-----
   1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 0cd94d9f09..cc845b8534 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -585,7 +585,6 @@ static void integratorcp_init(MachineState *machine)
       Object *cpuobj;
       ARMCPU *cpu;
       MemoryRegion *address_space_mem = get_system_memory();
-    MemoryRegion *ram = g_new(MemoryRegion, 1);
       MemoryRegion *ram_alias = g_new(MemoryRegion, 1);
       qemu_irq pic[32];
       DeviceState *dev, *sic, *icp;
@@ -605,14 +604,13 @@ static void integratorcp_init(MachineState *machine)
cpu = ARM_CPU(cpuobj); - memory_region_allocate_system_memory(ram, NULL, "integrator.ram",
-                                         ram_size);
       /* ??? On a real system the first 1Mb is mapped as SSRAM or boot flash.  
       /* ??? RAM should repeat to fill physical memory space.  */
       /* SDRAM at address zero*/
-    memory_region_add_subregion(address_space_mem, 0, ram);
+    memory_region_add_subregion(address_space_mem, 0, machine->ram);
       /* And again at address 0x80000000 */
-    memory_region_init_alias(ram_alias, NULL, "ram.alias", ram, 0, ram_size);
+    memory_region_init_alias(ram_alias, NULL, "ram.alias", machine->ram,
+                             0, ram_size);
       memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
@@ -660,6 +658,7 @@ static void integratorcp_machine_init(MachineClass *mc)
       mc->init = integratorcp_init;
       mc->ignore_memory_transaction_failures = true;
       mc->default_cpu_type = ARM_CPU_TYPE_NAME("arm926");
+    mc->default_ram_id = "integrator.ram";
DEFINE_MACHINE("integratorcp", integratorcp_machine_init)

Looking at integratorcm_realize() this machine seems to handle at most

According to

it supports "optionally, 16 to 256MB of SDRAM plugged into the DIMM socket"
so I'm not sure that realize is valid reference here.
(well I don't know anything about arm boards, but it probably should be
double checked by maintainer).

It should not hold this series (as check wasn't there to begin with),
I'll post a patch on top to add check once we decide to what limit it
should be set.

This is certainly not a blocking comment (neither am I waiting for you to fix this, I was just making a comment while reviewing the whole). Sorry if this was not clear.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]