From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yijing Wang Subject: Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment Date: Fri, 26 Sep 2014 09:55:07 +0800 Message-ID: <5424C77B.2060807@huawei.com> References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Return-path: Received: from szxga03-in.huawei.com ([119.145.14.66]:18447 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752986AbaIZBzu (ORCPT ); Thu, 25 Sep 2014 21:55:50 -0400 In-Reply-To: <20140925070601.GF12423@ulmo> Sender: linux-arch-owner@vger.kernel.org List-ID: To: Thierry Reding Cc: 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.ke On 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from szxga03-in.huawei.com ([119.145.14.66]:18447 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752986AbaIZBzu (ORCPT ); Thu, 25 Sep 2014 21:55:50 -0400 Message-ID: <5424C77B.2060807@huawei.com> Date: Fri, 26 Sep 2014 09:55:07 +0800 From: Yijing Wang MIME-Version: 1.0 Subject: Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> In-Reply-To: <20140925070601.GF12423@ulmo> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit Sender: linux-arch-owner@vger.kernel.org List-ID: To: Thierry Reding Cc: 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 , Liviu Dudau Message-ID: <20140926015507.HEnOKHNIcuhrJK07Qg-D6bq4KZPNsJ8c1N2DJHP9_U8@z> On 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yijing Wang Date: Fri, 26 Sep 2014 01:55:07 +0000 Subject: Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment Message-Id: <5424C77B.2060807@huawei.com> List-Id: References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> In-Reply-To: <20140925070601.GF12423@ulmo> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Thierry Reding Cc: 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 , Liviu Dudau On 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 Received: with ECARTIS (v1.0.0; list linux-mips); Fri, 26 Sep 2014 03:56:34 +0200 (CEST) Received: from szxga03-in.huawei.com ([119.145.14.66]:20125 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by eddie.linux-mips.org with ESMTP id S27009897AbaIZB43jwalZ (ORCPT ); Fri, 26 Sep 2014 03:56:29 +0200 Received: from 172.24.2.119 (EHLO szxeml405-hub.china.huawei.com) ([172.24.2.119]) by szxrg03-dlp.huawei.com (MOS 4.4.3-GA FastPath queued) with ESMTP id AUW09288; Fri, 26 Sep 2014 09:55:26 +0800 (CST) Received: from [127.0.0.1] (10.177.27.212) by szxeml405-hub.china.huawei.com (10.82.67.60) with Microsoft SMTP Server id 14.3.158.1; Fri, 26 Sep 2014 09:55:15 +0800 Message-ID: <5424C77B.2060807@huawei.com> Date: Fri, 26 Sep 2014 09:55:07 +0800 From: Yijing Wang User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: Thierry Reding CC: Bjorn Helgaas , , , Xinwei Hu , Wuyun , , Russell King , , , , , Arnd Bergmann , Thomas Gleixner , "Konrad Rzeszutek Wilk" , , Joerg Roedel , , , Benjamin Herrenschmidt , , , Sebastian Ott , "Tony Luck" , , "David S. Miller" , , Chris Metcalf , Ralf Baechle , Lucas Stach , David Vrabel , "Sergei Shtylyov" , Michael Ellerman , Thomas Petazzoni , Liviu Dudau Subject: Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> In-Reply-To: <20140925070601.GF12423@ulmo> Content-Type: text/plain; charset="ISO-8859-1" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.177.27.212] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020205.5424C792.00B0,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 92fc8dcb982db260fa22475eeee18a7a Return-Path: X-Envelope-To: <"|/home/ecartis/ecartis -s linux-mips"> (uid 0) X-Orcpt: rfc822;linux-mips@linux-mips.org Original-Recipient: rfc822;linux-mips@linux-mips.org X-archive-position: 42819 X-ecartis-version: Ecartis v1.0.0 Sender: linux-mips-bounce@linux-mips.org Errors-to: linux-mips-bounce@linux-mips.org X-original-sender: wangyijing@huawei.com Precedence: bulk List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: linux-mips X-List-ID: linux-mips List-subscribe: List-owner: List-post: List-archive: X-list: linux-mips On 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [119.145.14.66]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 06B321A009F for ; Fri, 26 Sep 2014 11:55:58 +1000 (EST) Message-ID: <5424C77B.2060807@huawei.com> Date: Fri, 26 Sep 2014 09:55:07 +0800 From: Yijing Wang MIME-Version: 1.0 To: Thierry Reding Subject: Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> In-Reply-To: <20140925070601.GF12423@ulmo> Content-Type: text/plain; charset="ISO-8859-1" Cc: linux-mips@linux-mips.org, linux-ia64@vger.kernel.org, linux-pci@vger.kernel.org, Liviu Dudau , Bharat.Bhushan@freescale.com, 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 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yijing Wang Date: Fri, 26 Sep 2014 01:55:07 +0000 Subject: Re: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment Message-Id: <5424C77B.2060807@huawei.com> List-Id: References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> In-Reply-To: <20140925070601.GF12423@ulmo> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Thierry Reding Cc: 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.ke On 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 From: wangyijing@huawei.com (Yijing Wang) Date: Fri, 26 Sep 2014 09:55:07 +0800 Subject: [PATCH v2 02/22] PCI/MSI: Remove useless bus->msi assignment In-Reply-To: <20140925070601.GF12423@ulmo> References: <1411614872-4009-1-git-send-email-wangyijing@huawei.com> <1411614872-4009-3-git-send-email-wangyijing@huawei.com> <20140925070601.GF12423@ulmo> Message-ID: <5424C77B.2060807@huawei.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2014/9/25 15:06, Thierry Reding wrote: > On Thu, Sep 25, 2014 at 11:14:12AM +0800, Yijing Wang wrote: >> Currently, PCI drivers will initialize bus->msi in >> pcibios_add_bus(). pcibios_add_bus() will be called >> in every pci bus initialization. So the bus->msi >> assignment in pci_alloc_child_bus() is useless. > > I think this should be the other way around. The default should be to > inherit bus->msi from the parent. That way drivers don't typically have > to do it, yet they can still opt to override it if they need to. > > For Tegra for example I think it would work if we assigned the MSI chip > to the root bus (in tegra_pcie_scan_bus()) and then have it propagated > to child busses in pci_alloc_child_bus() so that tegra_pcie_add_bus() > can be removed altogether. Hi Thierry, thanks very much for your review and comments! Because pcibios_add_bus() and "child->msi = parent->msi;" in pci_alloc_child_bus() do the same thing. So I think we should use one and remove another. I like the latter. In addition, I consider several solutions to associate msi chip and PCI device. 1. Add a msi_chip member in PCI arch sysdata to store the msi_chip pointer. Because all PCI devices under a PCI hostbridge always share the same msi chip, so every PCI device can find the msi chip by pci_bus->sysdata, but in this solution, we also need to add ARCH specific functions to extract msi chip from PCI arch sysdata, like pci_domain_nr(). Then we can remove .add_bus() like tegra_pcie_add_bus() and the msi assignment in pci_alloc_child_bus(). 2. Remove .add_bus() functions, associate PCI root bus and msi chip after PCI root bus created, as you mentioned above. In this solution we need to associate PCI root bus and msi chip in all PCI hostbridge drivers, like in tegra_pcie_scan_bus(). 3. Introduce a global msi chip list, currently, only in arm, there maybe more than one type MSI chip, but in arm, we can find msi chip by PCI hostbridge platform device's of_node, Eg. use "msi-parent" property to find it. Or msi chip is integrated into PCI hostbridge, we can find msi chip by compare msi_chip->dev and PCI hostbridge's platform device's struct device *dev pointer. And because PCI hostbridge platform device pass to pci_create_root_bus() as the parent device, so every PCI devices can first find the platform device, then to find the msi chip, this solution looks a bit ugly, but we only associate pci hostbridge and msi chip, PCI child buses and devices do not have to know the msi chip details. 4. Last, in this series, introduced weak arch function arch_find_msi_chip(), it's the simplest one, because all platforms except arm, only one msi chip will exist in system. What do you think about this ? Thanks! Yijing. > > Thierry > -- Thanks! Yijing