>>> +
>>> + object_initialize_child(obj, "homer", &chip9->homer,
>>> sizeof(chip9->homer),
>>> + TYPE_PNV9_HOMER, &error_abort, NULL);
>>> + object_property_add_const_link(OBJECT(&chip9->homer), "chip", obj,
>>> + &error_abort);
>>
>> Does HOMER need the chip ? It is not used but we might want to in the
>> core_max_array() ?
>
> sorry, no it is not required, I will remove it.
It seems you will need the chip in core_max_array(). I would keep it.
See below,
[ ... ]
>>> +static bool core_max_array(void *opaque, hwaddr addr)
Please change the 'void *opaque' function parameter in 'PnvHOMER *homer'
>>> +{
>>> + PnvHOMER *homer = PNV_HOMER(opaque);
>>> + PnvHOMERClass *homer_class = PNV_HOMER_GET_CLASS(homer);
>>> +
>>> + MachineState *ms = MACHINE(qdev_get_machine());
>>
>> Do you need the whole machine or only the chip ?
>
> yes, I see it is for active cores in the chip, I can use `nr_cores`
> defined in PnvChip.
If you keep the QOM link above, you can grab it in the realize handler of
the HOMER model with :
Object *obj;
Error *local_err = NULL;
obj = object_property_get_link(OBJECT(dev), "chip", &local_err);
if (!obj) {
error_propagate(errp, local_err);
error_prepend(errp, "required link 'chip' not found: ");
return;
}
homer->chip = PNV_CHIP(obj);
[ ... ]
>>> +
>>> +/* P9 Pstate table */
>>> +
>>
>> no version ?
>
> PNV9_OCC_PSTATE_MAJOR_VERSION is the P9 pstate version.
why isn't it in the switch statement below ?
[ ... ]
>>> +typedef struct PnvHOMER {
>>> + DeviceState parent;
>>> +
>>> + MemoryRegion homer_regs;
>>
>> the homer_ prefix is not useful.
>
> okay, I will change it to `hregs`.
I would just remove the prefix
Thanks,
C.
