diff mbox

hw/arm: Fix Integrator/CP memsz initialization

Message ID 1474314852-29465-1-git-send-email-jakub@jermar.eu (mailing list archive)
State New, archived
Headers show

Commit Message

Jakub Jermář Sept. 19, 2016, 7:54 p.m. UTC
* Do not assume memsz is already initialized in integratorcm_init
* Calculate memsz directly from MachineState
* Get rid of the now unused memsz property

Signed-off-by: Jakub Jermar <jakub@jermar.eu>
---
 hw/arm/integratorcp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Peter Maydell Sept. 20, 2016, 5:34 p.m. UTC | #1
On 19 September 2016 at 20:54, Jakub Jermář <jakub@jermar.eu> wrote:
>
> * Do not assume memsz is already initialized in integratorcm_init
> * Calculate memsz directly from MachineState
> * Get rid of the now unused memsz property
>
> Signed-off-by: Jakub Jermar <jakub@jermar.eu>

Thanks for this patch; this is definitely a bug but I think
this is not the best way to fix it.

What we should do is add a realize function to the IntegratorCM
device, and move the code that depends on the value of memsz
from the init function to the realize function. That way the
device doesn't have to know anything about the machine.

thanks
-- PMM
Jakub Jermář Sept. 25, 2016, 8:13 p.m. UTC | #2
On 09/20/2016 07:34 PM, Peter Maydell wrote:
> On 19 September 2016 at 20:54, Jakub Jermář <jakub@jermar.eu> wrote:
>>
>> * Do not assume memsz is already initialized in integratorcm_init
>> * Calculate memsz directly from MachineState
>> * Get rid of the now unused memsz property
>>
>> Signed-off-by: Jakub Jermar <jakub@jermar.eu>
> 
> Thanks for this patch; this is definitely a bug but I think
> this is not the best way to fix it.
> 
> What we should do is add a realize function to the IntegratorCM
> device, and move the code that depends on the value of memsz
> from the init function to the realize function. That way the
> device doesn't have to know anything about the machine.

Thanks, I will be sending a reworked patch shortly.

Jakub
diff mbox

Patch

diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c
index 96dc150..3d88369 100644
--- a/hw/arm/integratorcp.c
+++ b/hw/arm/integratorcp.c
@@ -247,7 +247,9 @@  static void integratorcm_init(Object *obj)
 {
     IntegratorCMState *s = INTEGRATOR_CM(obj);
     SysBusDevice *dev = SYS_BUS_DEVICE(obj);
+    MachineState *machine = MACHINE(qdev_get_machine());
 
+    s->memsz = machine->ram_size >> 20;
     s->cm_osc = 0x01000048;
     /* ??? What should the high bits of this value be?  */
     s->cm_auxosc = 0x0007feff;
@@ -574,7 +576,6 @@  static void integratorcp_init(MachineState *machine)
     memory_region_add_subregion(address_space_mem, 0x80000000, ram_alias);
 
     dev = qdev_create(NULL, TYPE_INTEGRATOR_CM);
-    qdev_prop_set_uint32(dev, "memsz", ram_size >> 20);
     qdev_init_nofail(dev);
     sysbus_mmio_map((SysBusDevice *)dev, 0, 0x10000000);
 
@@ -624,7 +625,6 @@  static void integratorcp_machine_init(MachineClass *mc)
 DEFINE_MACHINE("integratorcp", integratorcp_machine_init)
 
 static Property core_properties[] = {
-    DEFINE_PROP_UINT32("memsz", IntegratorCMState, memsz, 0),
     DEFINE_PROP_END_OF_LIST(),
 };