From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:54419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RsH05-000115-MS for qemu-devel@nongnu.org; Tue, 31 Jan 2012 11:56:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RsH03-0000ZQ-7F for qemu-devel@nongnu.org; Tue, 31 Jan 2012 11:56:09 -0500 Received: from goliath.siemens.de ([192.35.17.28]:22210) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RsH02-0000ZK-SV for qemu-devel@nongnu.org; Tue, 31 Jan 2012 11:56:07 -0500 Message-ID: <4F281D23.2080900@siemens.com> Date: Tue, 31 Jan 2012 17:56:03 +0100 From: Jan Kiszka MIME-Version: 1.0 References: <1327604460-31142-1-git-send-email-aliguori@us.ibm.com> <1327604460-31142-7-git-send-email-aliguori@us.ibm.com> <4F27FBE6.7040800@siemens.com> <4F27FEFD.5070507@codemonkey.ws> <4F28000B.5060406@siemens.com> <4F280120.2070402@codemonkey.ws> <4F281A12.5000502@siemens.com> <4F281BB3.3030100@codemonkey.ws> In-Reply-To: <4F281BB3.3030100@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 06/15] piix: create i8254 through composition List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Peter Maydell , Anthony Liguori , "qemu-devel@nongnu.org" , Markus Armbruster , Avi Kivity , Paolo Bonzini On 2012-01-31 17:49, Anthony Liguori wrote: > On 01/31/2012 10:42 AM, Jan Kiszka wrote: >> On 2012-01-31 15:56, Anthony Liguori wrote: >>> On 01/31/2012 08:51 AM, Jan Kiszka wrote: >>>> On 2012-01-31 15:47, Anthony Liguori wrote: >>>>> On 01/31/2012 08:34 AM, Jan Kiszka wrote: >>>>>> On 2012-01-26 20:00, Anthony Liguori wrote: >>>>>>> @@ -548,6 +550,13 @@ static int piix3_realize(PCIDevice *dev) >>>>>>> /* Setup the RTC IRQ */ >>>>>>> s->rtc.irq = rtc_irq; >>>>>>> >>>>>>> + /* Realize the PIT */ >>>>>>> + qdev_set_parent_bus(DEVICE(&s->pit), BUS(s->bus)); >>>>>>> + qdev_init_nofail(DEVICE(&s->pit)); >>>>>>> + >>>>>>> + /* FIXME this should be refactored */ >>>>>>> + pcspk_init(ISA_DEVICE(&s->pit)); >>>>>> >>>>>> Fixing ATM, ie. converting to qdev/QOM. >>>>>> >>>>>> Q: How do I use qdev_property_add_link& Co. to establish the relation >>>>>> from the speaker port device to the PIT? >>>>> >>>>> In the state structure, have: >>>>> >>>>> struct PCSpkState { >>>>> ... >>>>> PITState *pit; >>>>> }; >>>>> >>>>> In the pcspk instance_init, do: >>>>> >>>>> object_property_add_link(obj, "pit", TYPE_PIT, (Object **)&s->pit, NULL); >>>>> >>>>> In the pcspk realize function (DeviceClass::init), do: >>>>> >>>>> assert(s->pit != NULL); // make sure the pit link is set >>>>> >>>>> And that's it. >>>>> >>>>> You can set the s->pit field directly. You are not required to use any special >>>>> QOM function to interact with link properties. >>>>> >>>>> BTW, this is yet another benefit of making structures public. You can take the >>>>> address of a child and set link fields directly without accessors. >>>> >>>> Well, that has two sides. We introduced properties to avoid this direct >>>> messing. >>>> >>>> Does linking also work without exposing internals? >>> >>> Yes, you can set links through properties (although I haven't added those >>> accessors yet). >>> >>> But... you lose type safety because now you're dealing with strings. >> >> I don't get yet why we have to give up on type safety here. Isn't all >> information stored in the property entry? Can't some >> object_set_property() service take the object pointer and validate its >> type before writing at the target location? > > Already does that. You'll get a run time warning. Fine. > >> I'm not worried about >> lacking compile-time checks if we keep them for runtime. > > I'm worried about: > > object_property_set_link(obj, "pci", OBJECT(&s->pic)); > > vs: > > spk->pic = s->pic; > > Or: > > pcspk_set_pic(spk, &s->pic); > > I tend to make a lot of typos, I like the compiler to catch them for me at build > time. Well, but this enforces clean interfaces - just postpones the check. Nothing can be set in the foreign device state outside its core. Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux