From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45185) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aAxO7-0003gk-6e for qemu-devel@nongnu.org; Mon, 21 Dec 2015 05:08:20 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aAxO2-0000gT-2i for qemu-devel@nongnu.org; Mon, 21 Dec 2015 05:08:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:38179) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aAxO1-0000gL-RJ for qemu-devel@nongnu.org; Mon, 21 Dec 2015 05:08:13 -0500 References: <1450436632-23980-1-git-send-email-caoj.fnst@cn.fujitsu.com> <1450436632-23980-4-git-send-email-caoj.fnst@cn.fujitsu.com> <56768170.2080602@gmail.com> <56768766.1040300@cn.fujitsu.com> <56768F2B.5030104@redhat.com> <56776B07.7000102@cn.fujitsu.com> From: Marcel Apfelbaum Message-ID: <5677CF88.7010001@redhat.com> Date: Mon, 21 Dec 2015 12:08:08 +0200 MIME-Version: 1.0 In-Reply-To: <56776B07.7000102@cn.fujitsu.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 3/5] PXB: convert to realize() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cao jin , qemu-devel@nongnu.org Cc: ehabkost@redhat.com, mst@redhat.com, Markus Armbruster , pbonzini@redhat.com, leon.alrae@imgtec.com, =?UTF-8?Q?Andreas_F=c3=a4rber?= , aurelien@aurel32.net, rth@twiddle.net On 12/21/2015 04:59 AM, Cao jin wrote: > > > On 12/20/2015 07:21 PM, Marcel Apfelbaum wrote: >> On 12/20/2015 12:48 PM, Cao jin wrote: >>> Hi, >>> >>> On 12/20/2015 06:22 PM, Marcel Apfelbaum wrote: > [...] >>>>> + >>>>> +err_register_bus: >>>>> + object_unref(OBJECT(ds)); >>>>> + object_unref(OBJECT(bds)); >>>>> + object_unref(OBJECT(bus)); >>>> >>>> >>>> The order should be in the reverse order of creation: >>>> bds, bus, ds >>>> >>> >>> Ok, I can do that. But it seems the order here doesn`t matter? Is >>> there dependency among these three? >> >> Yes, there is a dependency: >> At first the pxb host (ds) is created, then the bus (bus) is created as >> host's child (see pci_bus_new) >> and in the end a pci bridge (bds) is attached to the bus (see qdev_create). >> > > Yup...thanks for reminding, I did read the code trying to find the parent relationship...but seem I didn`t read it thoroughly:-[ > >> By the way, indeed you should call object_unparent at least for the >> pxb_host(ds), but you may want to >> check the others parent relationship as well. >> > > yes, but I think you are saying: object_unparent(bus), right? the relationship seems is: > pxb host-->(child property)bus-->(link property)bds > > Another question: because some of this series is CCed to qemu-trivial(which means: reviewed-by?) by other maintainer, so next time, do I need to send the whole series with "v2", or the rest? Hi, Since the patches are not related, the ones cc-ed to qemu-trivial will be taken by the maintainer of trivial patches, for the rest you should prepare a V2 to be reviewed by the corresponding sub-tree maintainer. CC to qemu-trivial does not mean "reviewed-by", it just implies the patch is simple enough to go through the trivial tree and does not need to go through the sub-tree maintainer. Thanks, Marcel > >>> >>>> >>>>> } >>>>> >>>>> static void pxb_dev_exitfn(PCIDevice *pci_dev) >>>>> @@ -259,7 +263,7 @@ static void pxb_dev_class_init(ObjectClass *klass, >>>>> void *data) >>>>> DeviceClass *dc = DEVICE_CLASS(klass); >>>>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass); >>>>> >>>>> - k->init = pxb_dev_initfn; >>>>> + k->realize = pxb_dev_realize; >>>>> k->exit = pxb_dev_exitfn; >>>> >>>> If init is converted to realize, maybe the exit should be converted to >>>> unrealize? >>>> >>> >>> Yup, I agree with you from the point that the names should be antonym. >>> But it seems there is no PCIDeviceClass.unrealize:( >> >> You are right. The pci_qdev_unrealize ultimately calls exit. But the >> same goes for init, pci_qdev_realize calls for pc->realize. >> This is the reason I chose to use init/exit instead of the strange >> realize/exit. >> >> But since the intention is to get rid of init, I am not against it. >> >>> >>> And I am also not aware why there is no comment for .exit while there >>> is for .init. It is appreciated if somebody could tell me the history >>> O:-) >> >> I'll add Markus, Andreas and Michael (the PCI maintainer), maybe they >> have a better insight to this. >> >> On the other hand you should continue with the patch and leave the >> "unrealize" until we'll know more :) > > Got it;) > >> >> Thanks, >> Marcel >> >>> >>>> >>>> Thanks, >>>> Marcel >>>> >>>>> k->vendor_id = PCI_VENDOR_ID_REDHAT; >>>>> k->device_id = PCI_DEVICE_ID_REDHAT_PXB; >>>>> >>>> >>>> >>>> >>>> . >>>> >>> >> >> >> >> . >> >