Re: [Qemu-devel] [RFC] QOM design - add instance data to Object (-> add constructors)

[Peter Crosthwaite]

On Sat, Jun 13, 2015 at 12:52 PM, Liviu Ionescu <address@hidden> wrote:
>
>> On 13 Jun 2015, at 21:57, Liviu Ionescu <address@hidden> wrote:
>>
>> ... so, no, the class structure is not suitable for multi-instances objects, 
>> and even for singletons I think that using it for passing such configuration 
>> data is generally abusive.
>
> not to mention that in real life situations, constructors are used to pass 
> non-static data, for example the 'machine' structure (for various command 
> line params) when constructing MCUs, or the 'mcu' when constructing LEDs.
>

I think these are both questionable examples, as they are contained
objects taking and relying on pointers to their container which
reduces their re-usability as a modular component.

> passing these pointers orderly to the entire hierarchy is very simple when 
> using standard constructors.
>
> how would you pass them with the existing .instance_init or 
> .instance_post_init support? I don't think it is possible, which means the 
> entire object construction needs to be delegated to realize(), which is way 
> too late.
>
>
> for a better understanding of the problem, I'm copying again the same (fully 
> functional) code to create boards:
>
> /* ----- Olimex STM32-H103 ----- */
>
> GenericGPIOLEDInfo h103_machine_green_led = {
>     .desc = "STM32-H103 Green LED, GPIOC[12], active low",
>     .port_index = STM32_GPIO_PORT_C,
>     .port_bit = 12,
>     .active_low = true,
>     .on_message = "[Green LED On]\n",
>     .off_message = "[Green LED Off]\n",
>     .use_stderr = true };

Is any of this information needed for construction of can these fields
just be qdev properties?

>
> static void stm32_h103_board_init_callback(MachineState *machine)
> {
>     cortexm_board_greeting(machine);
>     DeviceState *mcu = qdev_alloc(NULL, TYPE_STM32F103RB);
>     {
>         STM32F103RB_GET_CLASS(mcu)->construct(OBJECT(mcu), machine);
>
>         /* Set the board specific oscillator frequencies. */
>         DeviceState *rcc = stm32_mcu_get_rcc_dev(mcu);

You should try and avoid fishing sub-devices out of the container like
this if you can.

>         qdev_prop_set_uint32(rcc, "hse-freq-hz", 8000000); /* 8.0 MHz */
>         qdev_prop_set_uint32(rcc, "lse-freq-hz", 32768); /* 32 KHz */

If these are crystal frequencies, then they should be properties of
the MCU object itself. The MCU property setter then delegates its
property to the sub-device. This abstracts SoC internals away from the
board layer. Alias properties can make shorter work of this.

Regards,
Peter

>     }
>     qdev_realize(mcu);
>
>     /* Board peripheral objects */
>     DeviceState *led = qdev_alloc(NULL, TYPE_STM32_GPIO_LED);
>     {
>         STM32_GPIO_LED_GET_CLASS(led)->construct(OBJECT(led),
>                 &h103_machine_green_led, mcu);
>     }
>     qdev_realize(led);
> }
>
> static QEMUMachine stm32_h103_machine = {
>     .name = "STM32-H103",
>     .desc = "Olimex Header Board for STM32F103RBT6 (Experimental)",
>     .init = stm32_h103_board_init_callback };
>
> -----------------------------------------------
>
> regards,
>
> Liviu
>
>