From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752814AbaI2Ja2 (ORCPT ); Mon, 29 Sep 2014 05:30:28 -0400 Received: from dliviu.plus.com ([80.229.23.120]:50513 "EHLO smtp.dudau.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752209AbaI2JaZ (ORCPT ); Mon, 29 Sep 2014 05:30:25 -0400 Date: Mon, 29 Sep 2014 10:26:00 +0100 From: Liviu Dudau To: Yijing Wang Cc: Thierry Reding , Bjorn Helgaas , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Xinwei Hu , Wuyun , linux-arm-kernel@lists.infradead.org, Russell King , linux-arch@vger.kernel.org, arnab.basu@freescale.com, Bharat.Bhushan@freescale.com, x86@kernel.org, Arnd Bergmann , Thomas Gleixner , Konrad Rzeszutek Wilk , xen-devel@lists.xenproject.org, Joerg Roedel , iommu@lists.linux-foundation.org, linux-mips@linux-mips.org, Benjamin Herrenschmidt , linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, Sebastian Ott , Tony Luck , linux-ia64@vger.kernel.org, "David S. Miller" , sparclinux@vger.kernel.org, Chris Metcalf , Ralf Baechle , Lucas Stach , David Vrabel , Sergei Shtylyov , Michael Ellerman , Thomas Petazzoni Subject: Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms Message-ID: <20140929092600.GA15854@bart.dudau.co.uk> References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <20140925074235.GN12423@ulmo> <20140925144855.GB31157@bart.dudau.co.uk> <20140925164937.GB30382@ulmo> <5424E09F.50701@huawei.com> <20140926085030.GE31157@bart.dudau.co.uk> <54276F6C.5010705@huawei.com> <20140928112144.GA4671@bart.dudau.co.uk> <5428B971.50101@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <5428B971.50101@huawei.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-DSPAM-Result: Whitelisted X-DSPAM-Processed: Mon Sep 29 10:30:23 2014 X-DSPAM-Confidence: 0.9899 X-DSPAM-Probability: 0.0000 X-DSPAM-Signature: 13,542926ac48392053617530 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Sep 29, 2014 at 09:44:17AM +0800, Yijing Wang wrote: > On 2014/9/28 19:21, Liviu Dudau wrote: > > On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote: > >>>>>> What I would like to see is a way of creating the pci_host_bridge structure outside > >>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of platform > >>>>>> details like associated msi_chip into the host bridge and the child busses will > >>>>>> have an easy way of finding the information needed by finding the root bus and then > >>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly) > >>>>>> everyone and the drivers can remove their kludges that try to work around the > >>>>>> current limitations. > >>>> > >>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate. > >>> > >>> Except that arch sysdata at the moment is an opaque pointer. I am all in favour in > >>> changing the type of sysdata from void* into pci_host_bridge* and arches can wrap their old > >>> sysdata around the pci_host_bridge*. > >> > >> I inspected every arch and found there are almost no common stuff, > > > > I will disagree here. Most (all?) of the structures that are passed as sysdata argument to > > Most. > > > pci_create_root_bus() or pci_scan_root_bus() have a set of resources for storing the MEM and > > IO ranges, which struct pci_host_bridge already has. So that can be factored out of the > > arch code. Same for pci_domain_nr. Then there are some variables that are used for communication > > with the platform code due to convoluted way(s) in which PCI code gets instantiated. > > Yes, currently some archs store MEM and IO resource in pci sysdata, and others not, move the MEM and IO > resource to pci_host_bride could make code become simple, we can clean up the resource list argument in > pci scan functions. > > > > > What I am arguing here is not that the arch equivalent of pci_host_bridge structure is already > > common, but that by moving the members that are common out of arch sysdata into pci_host_bridge > > we will have more commonality and it will be easier to re-factor the code. > > Now, I got it, thanks! > > > > >> and generic data struct should > >> be created in generic PCI code. > > > > Not necessarily. What I have in mind is something like this: > > This is a good idea, what I'm worried is this series is already large, so I think we need to post > another series to do it. I wasn't asking to do it here, I was just offering a suggestion (and sharing some experience) when it comes to handling msi chip in an arch independent way. > > > > > > - drivers/pci/ exports pci_init_host_bridge() that does the initialisation of bridge->windows > > and anything else that is needed (like find_pci_host_bridge() function). > > - arch code does: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > ..... > > }; > > > > #define to_pci_controller(bridge) container_of(bridge, struct pci_controller, bridge) > > > > static inline struct pci_controller *get_host_controller(const struct pci_bus *bus) > > { > > struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > > if (bridge) > > return to_pci_controller(bridge); > > > > return NULL; > > } > > > > int arch_pci_init(....) > > { > > struct pci_controller *hose; > > .... > > hose = kzalloc(sizeof(*hose), GFP_KERNEL); > > pci_init_host_bridge(&hose->bridge); > > .... > > pci_scan_root_bus(...., &hose->bridge, &resources); > > .... > > return 0; > > } > > > > Then finding the right structure will be easy. > > > >> Another, I don't like associate msi chip and every PCI device, further more, > >> almost all platforms except arm have only one MSI controller, and currently, PCI enumerating code doesn't need > >> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need to know which IRQ controller they > >> should deliver IRQ to. I would think more about it, and hope other PCI guys can give some comments, especially from Bjorn. > >> > > > > I wasn't suggesing to associate an msi chip with every PCI device, but with the pci_host_bridge. > > I don't expect a host bridge to have more than one msi chip, so that should be OK. Also, I'm > > thinking that getting the associated msi chip should be some sort of pci_host_bridge ops function, > > and for arches that don't care about MSI it doesn't get implemented. > > Currently, a property "msi-parent" was introduced in arm, and all msi chip integrated in irq chip controller will > be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by its of_node. OK. But as you might have seen that still implies open coding a separate version of pci_scan_root_bus(). Best regards, Liviu > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > > > >> Thanks! > >> Yijing. > >> > >>> > >>> Best regards, > >>> Liviu > >>> > >>>> > >>>>> > >>>>> I think both issues are orthogonal. Last time I checked a lot of work > >>>>> was still necessary to unify host bridges enough so that it could be > >>>>> shared across architectures. But perhaps some of that work has > >>>>> happened in the meantime. > >>>>> > >>>>> But like I said, when you create the root bus, you can easily attach the > >>>>> MSI chip to it. > >>>>> > >>>>> Thierry > >>>>> > >>>> > >>>> > >>>> -- > >>>> Thanks! > >>>> Yijing > >>>> > >>>> > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> > > > > > -- > Thanks! > Yijing > > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Subject: Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms Date: Mon, 29 Sep 2014 10:26:00 +0100 Message-ID: <20140929092600.GA15854@bart.dudau.co.uk> References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <20140925074235.GN12423@ulmo> <20140925144855.GB31157@bart.dudau.co.uk> <20140925164937.GB30382@ulmo> <5424E09F.50701@huawei.com> <20140926085030.GE31157@bart.dudau.co.uk> <54276F6C.5010705@huawei.com> <20140928112144.GA4671@bart.dudau.co.uk> <5428B971.50101@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <5428B971.50101-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Yijing Wang Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org, linux-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Thierry Reding , sparclinux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Russell King , Michael Ellerman , x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Sebastian Ott , Benjamin Herrenschmidt , xen-devel-GuqFBffKawtpuQazS67q72D2FQJk+8+b@public.gmane.org, arnab.basu-KZfg59tc24xl57MIdRCFDg@public.gmane.org, Arnd Bergmann , Chris Metcalf , Bjorn Helgaas , Thomas Gleixner , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Thomas Petazzoni , Xinwei Hu , Tony Luck , Sergei Shtylyov , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ralf Baechle , iom List-Id: linux-arch.vger.kernel.org On Mon, Sep 29, 2014 at 09:44:17AM +0800, Yijing Wang wrote: > On 2014/9/28 19:21, Liviu Dudau wrote: > > On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote: > >>>>>> What I would like to see is a way of creating the pci_host_bridge structure outside > >>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of platform > >>>>>> details like associated msi_chip into the host bridge and the child busses will > >>>>>> have an easy way of finding the information needed by finding the root bus and then > >>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly) > >>>>>> everyone and the drivers can remove their kludges that try to work around the > >>>>>> current limitations. > >>>> > >>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate. > >>> > >>> Except that arch sysdata at the moment is an opaque pointer. I am all in favour in > >>> changing the type of sysdata from void* into pci_host_bridge* and arches can wrap their old > >>> sysdata around the pci_host_bridge*. > >> > >> I inspected every arch and found there are almost no common stuff, > > > > I will disagree here. Most (all?) of the structures that are passed as sysdata argument to > > Most. > > > pci_create_root_bus() or pci_scan_root_bus() have a set of resources for storing the MEM and > > IO ranges, which struct pci_host_bridge already has. So that can be factored out of the > > arch code. Same for pci_domain_nr. Then there are some variables that are used for communication > > with the platform code due to convoluted way(s) in which PCI code gets instantiated. > > Yes, currently some archs store MEM and IO resource in pci sysdata, and others not, move the MEM and IO > resource to pci_host_bride could make code become simple, we can clean up the resource list argument in > pci scan functions. > > > > > What I am arguing here is not that the arch equivalent of pci_host_bridge structure is already > > common, but that by moving the members that are common out of arch sysdata into pci_host_bridge > > we will have more commonality and it will be easier to re-factor the code. > > Now, I got it, thanks! > > > > >> and generic data struct should > >> be created in generic PCI code. > > > > Not necessarily. What I have in mind is something like this: > > This is a good idea, what I'm worried is this series is already large, so I think we need to post > another series to do it. I wasn't asking to do it here, I was just offering a suggestion (and sharing some experience) when it comes to handling msi chip in an arch independent way. > > > > > > - drivers/pci/ exports pci_init_host_bridge() that does the initialisation of bridge->windows > > and anything else that is needed (like find_pci_host_bridge() function). > > - arch code does: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > ..... > > }; > > > > #define to_pci_controller(bridge) container_of(bridge, struct pci_controller, bridge) > > > > static inline struct pci_controller *get_host_controller(const struct pci_bus *bus) > > { > > struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > > if (bridge) > > return to_pci_controller(bridge); > > > > return NULL; > > } > > > > int arch_pci_init(....) > > { > > struct pci_controller *hose; > > .... > > hose = kzalloc(sizeof(*hose), GFP_KERNEL); > > pci_init_host_bridge(&hose->bridge); > > .... > > pci_scan_root_bus(...., &hose->bridge, &resources); > > .... > > return 0; > > } > > > > Then finding the right structure will be easy. > > > >> Another, I don't like associate msi chip and every PCI device, further more, > >> almost all platforms except arm have only one MSI controller, and currently, PCI enumerating code doesn't need > >> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need to know which IRQ controller they > >> should deliver IRQ to. I would think more about it, and hope other PCI guys can give some comments, especially from Bjorn. > >> > > > > I wasn't suggesing to associate an msi chip with every PCI device, but with the pci_host_bridge. > > I don't expect a host bridge to have more than one msi chip, so that should be OK. Also, I'm > > thinking that getting the associated msi chip should be some sort of pci_host_bridge ops function, > > and for arches that don't care about MSI it doesn't get implemented. > > Currently, a property "msi-parent" was introduced in arm, and all msi chip integrated in irq chip controller will > be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by its of_node. OK. But as you might have seen that still implies open coding a separate version of pci_scan_root_bus(). Best regards, Liviu > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > > > >> Thanks! > >> Yijing. > >> > >>> > >>> Best regards, > >>> Liviu > >>> > >>>> > >>>>> > >>>>> I think both issues are orthogonal. Last time I checked a lot of work > >>>>> was still necessary to unify host bridges enough so that it could be > >>>>> shared across architectures. But perhaps some of that work has > >>>>> happened in the meantime. > >>>>> > >>>>> But like I said, when you create the root bus, you can easily attach the > >>>>> MSI chip to it. > >>>>> > >>>>> Thierry > >>>>> > >>>> > >>>> > >>>> -- > >>>> Thanks! > >>>> Yijing > >>>> > >>>> > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> > > > > > -- > Thanks! > Yijing > > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Date: Mon, 29 Sep 2014 09:26:00 +0000 Subject: Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms Message-Id: <20140929092600.GA15854@bart.dudau.co.uk> List-Id: References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <20140925074235.GN12423@ulmo> <20140925144855.GB31157@bart.dudau.co.uk> <20140925164937.GB30382@ulmo> <5424E09F.50701@huawei.com> <20140926085030.GE31157@bart.dudau.co.uk> <54276F6C.5010705@huawei.com> <20140928112144.GA4671@bart.dudau.co.uk> <5428B971.50101@huawei.com> In-Reply-To: <5428B971.50101-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Yijing Wang Cc: linux-mips-6z/3iImG2C8G8FEW9MqTrA@public.gmane.org, linux-ia64-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-pci-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Thierry Reding , sparclinux-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-arch-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-s390-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Russell King , Michael Ellerman , x86-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org, Sebastian Ott , Benjamin Herrenschmidt , xen-devel-GuqFBffKawtpuQazS67q72D2FQJk+8+b@public.gmane.org, arnab.basu-KZfg59tc24xl57MIdRCFDg@public.gmane.org, Arnd Bergmann , Chris Metcalf , Bjorn Helgaas , Thomas Gleixner , linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, Thomas Petazzoni , Xinwei Hu , Tony Luck , Sergei Shtylyov , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Ralf Baechle , iom On Mon, Sep 29, 2014 at 09:44:17AM +0800, Yijing Wang wrote: > On 2014/9/28 19:21, Liviu Dudau wrote: > > On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote: > >>>>>> What I would like to see is a way of creating the pci_host_bridge structure outside > >>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of platform > >>>>>> details like associated msi_chip into the host bridge and the child busses will > >>>>>> have an easy way of finding the information needed by finding the root bus and then > >>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly) > >>>>>> everyone and the drivers can remove their kludges that try to work around the > >>>>>> current limitations. > >>>> > >>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate. > >>> > >>> Except that arch sysdata at the moment is an opaque pointer. I am all in favour in > >>> changing the type of sysdata from void* into pci_host_bridge* and arches can wrap their old > >>> sysdata around the pci_host_bridge*. > >> > >> I inspected every arch and found there are almost no common stuff, > > > > I will disagree here. Most (all?) of the structures that are passed as sysdata argument to > > Most. > > > pci_create_root_bus() or pci_scan_root_bus() have a set of resources for storing the MEM and > > IO ranges, which struct pci_host_bridge already has. So that can be factored out of the > > arch code. Same for pci_domain_nr. Then there are some variables that are used for communication > > with the platform code due to convoluted way(s) in which PCI code gets instantiated. > > Yes, currently some archs store MEM and IO resource in pci sysdata, and others not, move the MEM and IO > resource to pci_host_bride could make code become simple, we can clean up the resource list argument in > pci scan functions. > > > > > What I am arguing here is not that the arch equivalent of pci_host_bridge structure is already > > common, but that by moving the members that are common out of arch sysdata into pci_host_bridge > > we will have more commonality and it will be easier to re-factor the code. > > Now, I got it, thanks! > > > > >> and generic data struct should > >> be created in generic PCI code. > > > > Not necessarily. What I have in mind is something like this: > > This is a good idea, what I'm worried is this series is already large, so I think we need to post > another series to do it. I wasn't asking to do it here, I was just offering a suggestion (and sharing some experience) when it comes to handling msi chip in an arch independent way. > > > > > > - drivers/pci/ exports pci_init_host_bridge() that does the initialisation of bridge->windows > > and anything else that is needed (like find_pci_host_bridge() function). > > - arch code does: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > ..... > > }; > > > > #define to_pci_controller(bridge) container_of(bridge, struct pci_controller, bridge) > > > > static inline struct pci_controller *get_host_controller(const struct pci_bus *bus) > > { > > struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > > if (bridge) > > return to_pci_controller(bridge); > > > > return NULL; > > } > > > > int arch_pci_init(....) > > { > > struct pci_controller *hose; > > .... > > hose = kzalloc(sizeof(*hose), GFP_KERNEL); > > pci_init_host_bridge(&hose->bridge); > > .... > > pci_scan_root_bus(...., &hose->bridge, &resources); > > .... > > return 0; > > } > > > > Then finding the right structure will be easy. > > > >> Another, I don't like associate msi chip and every PCI device, further more, > >> almost all platforms except arm have only one MSI controller, and currently, PCI enumerating code doesn't need > >> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need to know which IRQ controller they > >> should deliver IRQ to. I would think more about it, and hope other PCI guys can give some comments, especially from Bjorn. > >> > > > > I wasn't suggesing to associate an msi chip with every PCI device, but with the pci_host_bridge. > > I don't expect a host bridge to have more than one msi chip, so that should be OK. Also, I'm > > thinking that getting the associated msi chip should be some sort of pci_host_bridge ops function, > > and for arches that don't care about MSI it doesn't get implemented. > > Currently, a property "msi-parent" was introduced in arm, and all msi chip integrated in irq chip controller will > be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by its of_node. OK. But as you might have seen that still implies open coding a separate version of pci_scan_root_bus(). Best regards, Liviu > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > > > >> Thanks! > >> Yijing. > >> > >>> > >>> Best regards, > >>> Liviu > >>> > >>>> > >>>>> > >>>>> I think both issues are orthogonal. Last time I checked a lot of work > >>>>> was still necessary to unify host bridges enough so that it could be > >>>>> shared across architectures. But perhaps some of that work has > >>>>> happened in the meantime. > >>>>> > >>>>> But like I said, when you create the root bus, you can easily attach the > >>>>> MSI chip to it. > >>>>> > >>>>> Thierry > >>>>> > >>>> > >>>> > >>>> -- > >>>> Thanks! > >>>> Yijing > >>>> > >>>> > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> > > > > > -- > Thanks! > Yijing > > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.dudau.co.uk (dliviu.plus.com [80.229.23.120]) by lists.ozlabs.org (Postfix) with ESMTP id 32CCE1A0315 for ; Mon, 29 Sep 2014 19:28:58 +1000 (EST) Received: from localhost (localhost.localdomain [127.0.0.1]) by smtp.dudau.co.uk (Postfix) with SMTP id C45E71002ED for ; Mon, 29 Sep 2014 10:28:55 +0100 (BST) Date: Mon, 29 Sep 2014 10:26:00 +0100 From: Liviu Dudau To: Yijing Wang Subject: Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms Message-ID: <20140929092600.GA15854@bart.dudau.co.uk> References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <20140925074235.GN12423@ulmo> <20140925144855.GB31157@bart.dudau.co.uk> <20140925164937.GB30382@ulmo> <5424E09F.50701@huawei.com> <20140926085030.GE31157@bart.dudau.co.uk> <54276F6C.5010705@huawei.com> <20140928112144.GA4671@bart.dudau.co.uk> <5428B971.50101@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 In-Reply-To: <5428B971.50101@huawei.com> Cc: linux-mips@linux-mips.org, linux-ia64@vger.kernel.org, linux-pci@vger.kernel.org, Bharat.Bhushan@freescale.com, Thierry Reding , sparclinux@vger.kernel.org, linux-arch@vger.kernel.org, linux-s390@vger.kernel.org, Russell King , Joerg Roedel , x86@kernel.org, Sebastian Ott , xen-devel@lists.xenproject.org, arnab.basu@freescale.com, Arnd Bergmann , Konrad Rzeszutek Wilk , Chris Metcalf , Bjorn Helgaas , Thomas Gleixner , linux-arm-kernel@lists.infradead.org, Thomas Petazzoni , Xinwei Hu , Tony Luck , Sergei Shtylyov , linux-kernel@vger.kernel.org, Ralf Baechle , iommu@lists.linux-foundation.org, David Vrabel , Wuyun , linuxppc-dev@lists.ozlabs.org, "David S. Miller" , Lucas Stach List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Mon, Sep 29, 2014 at 09:44:17AM +0800, Yijing Wang wrote: > On 2014/9/28 19:21, Liviu Dudau wrote: > > On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote: > >>>>>> What I would like to see is a way of creating the pci_host_bridge structure outside > >>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of platform > >>>>>> details like associated msi_chip into the host bridge and the child busses will > >>>>>> have an easy way of finding the information needed by finding the root bus and then > >>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly) > >>>>>> everyone and the drivers can remove their kludges that try to work around the > >>>>>> current limitations. > >>>> > >>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate. > >>> > >>> Except that arch sysdata at the moment is an opaque pointer. I am all in favour in > >>> changing the type of sysdata from void* into pci_host_bridge* and arches can wrap their old > >>> sysdata around the pci_host_bridge*. > >> > >> I inspected every arch and found there are almost no common stuff, > > > > I will disagree here. Most (all?) of the structures that are passed as sysdata argument to > > Most. > > > pci_create_root_bus() or pci_scan_root_bus() have a set of resources for storing the MEM and > > IO ranges, which struct pci_host_bridge already has. So that can be factored out of the > > arch code. Same for pci_domain_nr. Then there are some variables that are used for communication > > with the platform code due to convoluted way(s) in which PCI code gets instantiated. > > Yes, currently some archs store MEM and IO resource in pci sysdata, and others not, move the MEM and IO > resource to pci_host_bride could make code become simple, we can clean up the resource list argument in > pci scan functions. > > > > > What I am arguing here is not that the arch equivalent of pci_host_bridge structure is already > > common, but that by moving the members that are common out of arch sysdata into pci_host_bridge > > we will have more commonality and it will be easier to re-factor the code. > > Now, I got it, thanks! > > > > >> and generic data struct should > >> be created in generic PCI code. > > > > Not necessarily. What I have in mind is something like this: > > This is a good idea, what I'm worried is this series is already large, so I think we need to post > another series to do it. I wasn't asking to do it here, I was just offering a suggestion (and sharing some experience) when it comes to handling msi chip in an arch independent way. > > > > > > - drivers/pci/ exports pci_init_host_bridge() that does the initialisation of bridge->windows > > and anything else that is needed (like find_pci_host_bridge() function). > > - arch code does: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > ..... > > }; > > > > #define to_pci_controller(bridge) container_of(bridge, struct pci_controller, bridge) > > > > static inline struct pci_controller *get_host_controller(const struct pci_bus *bus) > > { > > struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > > if (bridge) > > return to_pci_controller(bridge); > > > > return NULL; > > } > > > > int arch_pci_init(....) > > { > > struct pci_controller *hose; > > .... > > hose = kzalloc(sizeof(*hose), GFP_KERNEL); > > pci_init_host_bridge(&hose->bridge); > > .... > > pci_scan_root_bus(...., &hose->bridge, &resources); > > .... > > return 0; > > } > > > > Then finding the right structure will be easy. > > > >> Another, I don't like associate msi chip and every PCI device, further more, > >> almost all platforms except arm have only one MSI controller, and currently, PCI enumerating code doesn't need > >> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need to know which IRQ controller they > >> should deliver IRQ to. I would think more about it, and hope other PCI guys can give some comments, especially from Bjorn. > >> > > > > I wasn't suggesing to associate an msi chip with every PCI device, but with the pci_host_bridge. > > I don't expect a host bridge to have more than one msi chip, so that should be OK. Also, I'm > > thinking that getting the associated msi chip should be some sort of pci_host_bridge ops function, > > and for arches that don't care about MSI it doesn't get implemented. > > Currently, a property "msi-parent" was introduced in arm, and all msi chip integrated in irq chip controller will > be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by its of_node. OK. But as you might have seen that still implies open coding a separate version of pci_scan_root_bus(). Best regards, Liviu > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > > > >> Thanks! > >> Yijing. > >> > >>> > >>> Best regards, > >>> Liviu > >>> > >>>> > >>>>> > >>>>> I think both issues are orthogonal. Last time I checked a lot of work > >>>>> was still necessary to unify host bridges enough so that it could be > >>>>> shared across architectures. But perhaps some of that work has > >>>>> happened in the meantime. > >>>>> > >>>>> But like I said, when you create the root bus, you can easily attach the > >>>>> MSI chip to it. > >>>>> > >>>>> Thierry > >>>>> > >>>> > >>>> > >>>> -- > >>>> Thanks! > >>>> Yijing > >>>> > >>>> > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> > > > > > -- > Thanks! > Yijing > > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... From mboxrd@z Thu Jan 1 00:00:00 1970 From: liviu@dudau.co.uk (Liviu Dudau) Date: Mon, 29 Sep 2014 10:26:00 +0100 Subject: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms In-Reply-To: <5428B971.50101@huawei.com> References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <20140925074235.GN12423@ulmo> <20140925144855.GB31157@bart.dudau.co.uk> <20140925164937.GB30382@ulmo> <5424E09F.50701@huawei.com> <20140926085030.GE31157@bart.dudau.co.uk> <54276F6C.5010705@huawei.com> <20140928112144.GA4671@bart.dudau.co.uk> <5428B971.50101@huawei.com> Message-ID: <20140929092600.GA15854@bart.dudau.co.uk> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Sep 29, 2014 at 09:44:17AM +0800, Yijing Wang wrote: > On 2014/9/28 19:21, Liviu Dudau wrote: > > On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote: > >>>>>> What I would like to see is a way of creating the pci_host_bridge structure outside > >>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of platform > >>>>>> details like associated msi_chip into the host bridge and the child busses will > >>>>>> have an easy way of finding the information needed by finding the root bus and then > >>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly) > >>>>>> everyone and the drivers can remove their kludges that try to work around the > >>>>>> current limitations. > >>>> > >>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate. > >>> > >>> Except that arch sysdata at the moment is an opaque pointer. I am all in favour in > >>> changing the type of sysdata from void* into pci_host_bridge* and arches can wrap their old > >>> sysdata around the pci_host_bridge*. > >> > >> I inspected every arch and found there are almost no common stuff, > > > > I will disagree here. Most (all?) of the structures that are passed as sysdata argument to > > Most. > > > pci_create_root_bus() or pci_scan_root_bus() have a set of resources for storing the MEM and > > IO ranges, which struct pci_host_bridge already has. So that can be factored out of the > > arch code. Same for pci_domain_nr. Then there are some variables that are used for communication > > with the platform code due to convoluted way(s) in which PCI code gets instantiated. > > Yes, currently some archs store MEM and IO resource in pci sysdata, and others not, move the MEM and IO > resource to pci_host_bride could make code become simple, we can clean up the resource list argument in > pci scan functions. > > > > > What I am arguing here is not that the arch equivalent of pci_host_bridge structure is already > > common, but that by moving the members that are common out of arch sysdata into pci_host_bridge > > we will have more commonality and it will be easier to re-factor the code. > > Now, I got it, thanks! > > > > >> and generic data struct should > >> be created in generic PCI code. > > > > Not necessarily. What I have in mind is something like this: > > This is a good idea, what I'm worried is this series is already large, so I think we need to post > another series to do it. I wasn't asking to do it here, I was just offering a suggestion (and sharing some experience) when it comes to handling msi chip in an arch independent way. > > > > > > - drivers/pci/ exports pci_init_host_bridge() that does the initialisation of bridge->windows > > and anything else that is needed (like find_pci_host_bridge() function). > > - arch code does: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > ..... > > }; > > > > #define to_pci_controller(bridge) container_of(bridge, struct pci_controller, bridge) > > > > static inline struct pci_controller *get_host_controller(const struct pci_bus *bus) > > { > > struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > > if (bridge) > > return to_pci_controller(bridge); > > > > return NULL; > > } > > > > int arch_pci_init(....) > > { > > struct pci_controller *hose; > > .... > > hose = kzalloc(sizeof(*hose), GFP_KERNEL); > > pci_init_host_bridge(&hose->bridge); > > .... > > pci_scan_root_bus(...., &hose->bridge, &resources); > > .... > > return 0; > > } > > > > Then finding the right structure will be easy. > > > >> Another, I don't like associate msi chip and every PCI device, further more, > >> almost all platforms except arm have only one MSI controller, and currently, PCI enumerating code doesn't need > >> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need to know which IRQ controller they > >> should deliver IRQ to. I would think more about it, and hope other PCI guys can give some comments, especially from Bjorn. > >> > > > > I wasn't suggesing to associate an msi chip with every PCI device, but with the pci_host_bridge. > > I don't expect a host bridge to have more than one msi chip, so that should be OK. Also, I'm > > thinking that getting the associated msi chip should be some sort of pci_host_bridge ops function, > > and for arches that don't care about MSI it doesn't get implemented. > > Currently, a property "msi-parent" was introduced in arm, and all msi chip integrated in irq chip controller will > be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by its of_node. OK. But as you might have seen that still implies open coding a separate version of pci_scan_root_bus(). Best regards, Liviu > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > > > >> Thanks! > >> Yijing. > >> > >>> > >>> Best regards, > >>> Liviu > >>> > >>>> > >>>>> > >>>>> I think both issues are orthogonal. Last time I checked a lot of work > >>>>> was still necessary to unify host bridges enough so that it could be > >>>>> shared across architectures. But perhaps some of that work has > >>>>> happened in the meantime. > >>>>> > >>>>> But like I said, when you create the root bus, you can easily attach the > >>>>> MSI chip to it. > >>>>> > >>>>> Thierry > >>>>> > >>>> > >>>> > >>>> -- > >>>> Thanks! > >>>> Yijing > >>>> > >>>> > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> > > > > > -- > Thanks! > Yijing > > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ... From mboxrd@z Thu Jan 1 00:00:00 1970 From: Liviu Dudau Date: Mon, 29 Sep 2014 09:26:00 +0000 Subject: Re: [PATCH v2 00/22] Use MSI chip framework to configure MSI/MSI-X in all platforms Message-Id: <20140929092600.GA15854@bart.dudau.co.uk> List-Id: References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <20140925074235.GN12423@ulmo> <20140925144855.GB31157@bart.dudau.co.uk> <20140925164937.GB30382@ulmo> <5424E09F.50701@huawei.com> <20140926085030.GE31157@bart.dudau.co.uk> <54276F6C.5010705@huawei.com> <20140928112144.GA4671@bart.dudau.co.uk> <5428B971.50101@huawei.com> In-Reply-To: <5428B971.50101@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Yijing Wang Cc: Thierry Reding , Bjorn Helgaas , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, Xinwei Hu , Wuyun , linux-arm-kernel@lists.infradead.org, Russell King , linux-arch@vger.kernel.org, arnab.basu@freescale.com, Bharat.Bhushan@freescale.com, x86@kernel.org, Arnd Bergmann , Thomas Gleixner , Konrad Rzeszutek Wilk , xen-devel@lists.xenproject.org, Joerg Roedel , iommu@lists.linux-foundation.org, linux-mips@linux-mips.org, Benjamin Herrenschmidt , linuxppc-dev@lists.ozlabs.org, linux-s390@vger.kernel.org, Sebastian Ott , Tony Luck , linux-ia64@vger.kernel.org, "David S. Miller" , sparclinux@vger.kernel.org, Chris Metcalf , Ralf Baechle , Lucas Stach , David Vrabel , Sergei Shtylyov , Michael Ellerman , Thomas Petazzoni On Mon, Sep 29, 2014 at 09:44:17AM +0800, Yijing Wang wrote: > On 2014/9/28 19:21, Liviu Dudau wrote: > > On Sun, Sep 28, 2014 at 10:16:12AM +0800, Yijing Wang wrote: > >>>>>> What I would like to see is a way of creating the pci_host_bridge structure outside > >>>>>> the pci_create_root_bus(). That would then allow us to pass this sort of platform > >>>>>> details like associated msi_chip into the host bridge and the child busses will > >>>>>> have an easy way of finding the information needed by finding the root bus and then > >>>>>> the host bridge structure. Then the generic pci_scan_root_bus() can be used by (mostly) > >>>>>> everyone and the drivers can remove their kludges that try to work around the > >>>>>> current limitations. > >>>> > >>>> So I think maybe save msi chip in PCI arch sysdata is a good candidate. > >>> > >>> Except that arch sysdata at the moment is an opaque pointer. I am all in favour in > >>> changing the type of sysdata from void* into pci_host_bridge* and arches can wrap their old > >>> sysdata around the pci_host_bridge*. > >> > >> I inspected every arch and found there are almost no common stuff, > > > > I will disagree here. Most (all?) of the structures that are passed as sysdata argument to > > Most. > > > pci_create_root_bus() or pci_scan_root_bus() have a set of resources for storing the MEM and > > IO ranges, which struct pci_host_bridge already has. So that can be factored out of the > > arch code. Same for pci_domain_nr. Then there are some variables that are used for communication > > with the platform code due to convoluted way(s) in which PCI code gets instantiated. > > Yes, currently some archs store MEM and IO resource in pci sysdata, and others not, move the MEM and IO > resource to pci_host_bride could make code become simple, we can clean up the resource list argument in > pci scan functions. > > > > > What I am arguing here is not that the arch equivalent of pci_host_bridge structure is already > > common, but that by moving the members that are common out of arch sysdata into pci_host_bridge > > we will have more commonality and it will be easier to re-factor the code. > > Now, I got it, thanks! > > > > >> and generic data struct should > >> be created in generic PCI code. > > > > Not necessarily. What I have in mind is something like this: > > This is a good idea, what I'm worried is this series is already large, so I think we need to post > another series to do it. I wasn't asking to do it here, I was just offering a suggestion (and sharing some experience) when it comes to handling msi chip in an arch independent way. > > > > > > - drivers/pci/ exports pci_init_host_bridge() that does the initialisation of bridge->windows > > and anything else that is needed (like find_pci_host_bridge() function). > > - arch code does: > > > > struct pci_controller { > > struct pci_host_bridge bridge; > > ..... > > }; > > > > #define to_pci_controller(bridge) container_of(bridge, struct pci_controller, bridge) > > > > static inline struct pci_controller *get_host_controller(const struct pci_bus *bus) > > { > > struct pci_host_bridge *bridge = find_pci_host_bridge(bus); > > if (bridge) > > return to_pci_controller(bridge); > > > > return NULL; > > } > > > > int arch_pci_init(....) > > { > > struct pci_controller *hose; > > .... > > hose = kzalloc(sizeof(*hose), GFP_KERNEL); > > pci_init_host_bridge(&hose->bridge); > > .... > > pci_scan_root_bus(...., &hose->bridge, &resources); > > .... > > return 0; > > } > > > > Then finding the right structure will be easy. > > > >> Another, I don't like associate msi chip and every PCI device, further more, > >> almost all platforms except arm have only one MSI controller, and currently, PCI enumerating code doesn't need > >> to know the MSI chip details, like for legacy IRQ, PCI device doesn't need to know which IRQ controller they > >> should deliver IRQ to. I would think more about it, and hope other PCI guys can give some comments, especially from Bjorn. > >> > > > > I wasn't suggesing to associate an msi chip with every PCI device, but with the pci_host_bridge. > > I don't expect a host bridge to have more than one msi chip, so that should be OK. Also, I'm > > thinking that getting the associated msi chip should be some sort of pci_host_bridge ops function, > > and for arches that don't care about MSI it doesn't get implemented. > > Currently, a property "msi-parent" was introduced in arm, and all msi chip integrated in irq chip controller will > be added to of_pci_msi_chip_list. PCI host driver find the match msi chip by its of_node. OK. But as you might have seen that still implies open coding a separate version of pci_scan_root_bus(). Best regards, Liviu > > Thanks! > Yijing. > > > > > Best regards, > > Liviu > > > > > >> Thanks! > >> Yijing. > >> > >>> > >>> Best regards, > >>> Liviu > >>> > >>>> > >>>>> > >>>>> I think both issues are orthogonal. Last time I checked a lot of work > >>>>> was still necessary to unify host bridges enough so that it could be > >>>>> shared across architectures. But perhaps some of that work has > >>>>> happened in the meantime. > >>>>> > >>>>> But like I said, when you create the root bus, you can easily attach the > >>>>> MSI chip to it. > >>>>> > >>>>> Thierry > >>>>> > >>>> > >>>> > >>>> -- > >>>> Thanks! > >>>> Yijing > >>>> > >>>> > >>> > >> > >> > >> -- > >> Thanks! > >> Yijing > >> > >> > > > > > -- > Thanks! > Yijing > > -- ------------------- .oooO ( ) \ ( Oooo. \_) ( ) ) / (_/ One small step for me ...