From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48576) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z3uBt-0004OR-Na for Qemu-devel@nongnu.org; Sat, 13 Jun 2015 18:46:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z3uBq-0007eE-Ew for Qemu-devel@nongnu.org; Sat, 13 Jun 2015 18:46:17 -0400 Received: from mail-yk0-f175.google.com ([209.85.160.175]:36545) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z3uBq-0007e7-Aw for Qemu-devel@nongnu.org; Sat, 13 Jun 2015 18:46:14 -0400 Received: by ykaz81 with SMTP id z81so33738969yka.3 for ; Sat, 13 Jun 2015 15:46:13 -0700 (PDT) MIME-Version: 1.0 Sender: peter.crosthwaite@petalogix.com In-Reply-To: References: <0B3F8291-A1F7-437C-8111-CC4800F200D4@livius.net> <42AEE112-EC4A-4814-B3C7-56EF1EF41363@livius.net> Date: Sat, 13 Jun 2015 15:46:13 -0700 Message-ID: From: Peter Crosthwaite Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [RFC] QOM design - add instance data to Object (-> add constructors) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Liviu Ionescu Cc: QEMU Developers On Sat, Jun 13, 2015 at 12:52 PM, Liviu Ionescu wrote: > >> On 13 Jun 2015, at 21:57, Liviu Ionescu 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 > >