Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754751AbaKSQh0 (ORCPT ); Wed, 19 Nov 2014 11:37:26 -0500 Received: from dliviu.plus.com ([80.229.23.120]:60535 "EHLO smtp.dudau.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755825AbaKSQhX (ORCPT ); Wed, 19 Nov 2014 11:37:23 -0500 Date: Wed, 19 Nov 2014 16:29:14 +0000 From: Liviu Dudau To: Yijing Wang Cc: Liviu Dudau , Arnd Bergmann , "linuxppc-dev@lists.ozlabs.org" , Bjorn Helgaas , Tony Luck , Russell King , "linux-pci@vger.kernel.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "huxinwei@huawei.com" , Thierry Reding , "suravee.suthikulpanit@amd.com" , "linux-ia64@vger.kernel.org" , Thomas Gleixner , Wuyun , "linux-arm-kernel@lists.infradead.org" Subject: Re: [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus() Message-ID: <20141119162914.GB9162@bart.dudau.co.uk> References: <1416219710-26088-1-git-send-email-wangyijing@huawei.com> <1416219710-26088-8-git-send-email-wangyijing@huawei.com> <2507218.mHliopJb05@wuerfel> <546B041A.4060403@huawei.com> <20141118144819.GK12037@e106497-lin.cambridge.arm.com> <546BFF74.4030101@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <546BFF74.4030101@huawei.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Nov 19, 2014 at 10:24:52AM +0800, Yijing Wang wrote: > >> We need, some platforms pass NULL pointer as host bridge parent. > > > > Yijing, > > > > May I suggest a different approach here? Rather than having to pass an opaque > > pointer that gets converted by the host bridge driver back to the private > > structure, what about promoting a new style of usage, that is similar to the > > way device drivers work? Lets try to promote the embedding of the generic > > pci_host_bridge structure in the host bridge specific structure! Then we can > > access the private data doing container_of(). > > > > Something like this: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > /* private host bridge data here */ > > ..... > > }; > > > > #define PCI_CONTROLLER(bus) ({ > > struct pci_host_bridge *hb = to_pci_host_bridge(bus->bridge); \ > > container_of(hb, struct pci_controller, bridge); }) > > > > > > Then we can retrieve the host bridge structure from everywhere we have a device. > > Hi Liviu, it looks good to me, because this change will involve lots platforms, > I would think more about it. Thanks for your suggestion very much! :) Given that I also look at this area maybe we should join forces and divide the problem? Best regards, Liviu > > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > >> > >>> > >>>> + host = kzalloc(sizeof(*host), GFP_KERNEL); > >>>> + if (!host) > >>>> + return NULL; > >>> > >>> devm_kzalloc maybe? > >> > >> I don't know much detail about devm_kzalloc(), but we have no pci host driver > >> here, and I found no devm_kzalloc() uses in core PCI code before. > >> > >>> > >>>> + if (!resources) { > >>>> + /* Use default IO/MEM/BUS resources*/ > >>>> + pci_add_resource(&host->windows, &ioport_resource); > >>>> + pci_add_resource(&host->windows, &iomem_resource); > >>>> + pci_add_resource(&host->windows, &busn_resource); > >>>> + } else { > >>>> + list_for_each_entry_safe(window, n, resources, list) > >>>> + list_move_tail(&window->list, &host->windows); > >>>> + } > >>> > >>> I think we should assume that the correct resources are passed. You > >>> could add a wrapper around this function to convert old platforms > >>> though. > >> > >> OK, I will move these code out of pci_create_host_bridge, and add a wrapper > >> to setup the default resources. > >> > >>> > >>>> +EXPORT_SYMBOL(pci_create_host_bridge); > >>> > >>> EXPORT_SYMBOL_GPL() maybe? > >> > >> OK, will update it. > >> > >>> > >>>> diff --git a/include/linux/pci.h b/include/linux/pci.h > >>>> index 8b11b38..daa7f40 100644 > >>>> --- a/include/linux/pci.h > >>>> +++ b/include/linux/pci.h > >>>> @@ -402,7 +402,12 @@ struct pci_host_bridge_window { > >>>> struct pci_host_bridge { > >>>> struct device dev; > >>>> struct pci_bus *bus; /* root bus */ > >>>> + struct list_head list; > >>>> struct list_head windows; /* pci_host_bridge_windows */ > >>>> + int busnum; > >>> > >>> The busnum should already be implied through the bus resource. > >> > >> Yes, I will consider remove it and introduce a helper function to get the root bus number, thanks! > >> > >> Thanks! > >> Yijing. > >> > >>> > >>> Arnd > >>> > >>> . > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-pci" in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> > > > > > -- > Thanks! > Yijing > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/