From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753101AbbDGCw6 (ORCPT ); Mon, 6 Apr 2015 22:52:58 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:8145 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752685AbbDGCwx (ORCPT ); Mon, 6 Apr 2015 22:52:53 -0400 Message-ID: <5523466A.3090500@huawei.com> Date: Tue, 7 Apr 2015 10:52:26 +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: Daniel Axtens CC: Bjorn Helgaas , Jiang Liu , , Yinghai Lu , , Marc Zyngier , , Russell King , , Thomas Gleixner , Benjamin Herrenschmidt , Rusty Russell , Tony Luck , , "David S. Miller" , "Guan Xuetao" , , , Liviu Dudau , "Arnd Bergmann" , Geert Uytterhoeven , Subject: Re: [PATCH v9 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-21-git-send-email-wangyijing@huawei.com> <1428363334.22268.9.camel@axtens.net> In-Reply-To: <1428363334.22268.9.camel@axtens.net> Content-Type: text/plain; charset="UTF-8" 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.0A020206.5523467B.00B4,ss=1,re=0.001,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: df84ca9003089a126ec7ca9a73d061da Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015/4/7 7:35, Daniel Axtens wrote: > I've been looking at this patch series for a while now, and I now > believe it's ready on the PowerPC side. > > I was originally concerned that it would break odd corner cases, > particularly where similar code appears (namely kernel/pci_hotplug.c and > kernel/pci_of_scan.c). However, upon further examination, talking with > Yijing, and some testing, I'm now convinced that it is indeed restricted > to the generic code, and doesn't change behaviour. > > This is both a plus and a minus: because it's currently restricted to > generic code, I'm confident it works, but the down side is that it > doesn't yet simplify our arch-specific complexity. We'll need to do some > more work on our side to reap the full benefits. > > I tested this entire series on a PowerNV machine, including doing EEH > injection to trigger PCI hotplug: > Tested-by: Daniel Axtens > For completeness, it would be good to test it on Cell, as they are the > only remaining user of pci_of_scan.c > > In conclusion, this patch is > Reviewed-by: Daniel Axtens > Thanks very much for your test and review. Thanks! Yijing. > > > On Fri, 2015-04-03 at 17:25 +0800, Yijing Wang wrote: >> Now we could use pci_scan_host_bridge() to scan >> pci buses, provide powerpc specific pci_host_bridge_ops. >> >> Signed-off-by: Yijing Wang >> CC: Benjamin Herrenschmidt >> CC: linuxppc-dev@lists.ozlabs.org >> --- >> arch/powerpc/kernel/pci-common.c | 62 +++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 24 deletions(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 2c58200..50b32f6 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -773,6 +773,29 @@ void pcibios_set_root_bus_speed(struct pci_host_bridge *bridge) >> return ppc_md.pcibios_set_root_bus_speed(bridge); >> } >> >> +static int pci_host_scan_bus(struct pci_host_bridge *host) >> +{ >> + int mode = PCI_PROBE_NORMAL; >> + struct pci_bus *bus = host->bus; >> + struct pci_controller *hose = dev_get_drvdata(&host->dev); >> + >> + /* Get probe mode and perform scan */ >> + if (hose->dn && ppc_md.pci_probe_mode) >> + mode = ppc_md.pci_probe_mode(bus); >> + >> + pr_debug(" probe mode: %d\n", mode); >> + if (mode == PCI_PROBE_DEVTREE) >> + of_scan_bus(hose->dn, bus); >> + >> + if (mode == PCI_PROBE_NORMAL) { >> + pci_bus_update_busn_res_end(bus, 255); >> + hose->last_busno = pci_scan_child_bus(bus); >> + pci_bus_update_busn_res_end(bus, hose->last_busno); >> + } >> + >> + return pci_bus_child_max_busnr(bus); >> +} >> + >> /* This header fixup will do the resource fixup for all devices as they are >> * probed, but not for bridge ranges >> */ >> @@ -1585,6 +1608,11 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> return of_node_get(hose->dn); >> } >> >> +static struct pci_host_bridge_ops pci_host_ops = { >> + .set_root_bus_speed = pcibios_set_root_bus_speed, >> + .scan_bus = pci_host_scan_bus, >> +}; >> + >> /** >> * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus >> * @hose: Pointer to the PCI host controller instance structure >> @@ -1592,9 +1620,8 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> void pcibios_scan_phb(struct pci_controller *hose) >> { >> LIST_HEAD(resources); >> - struct pci_bus *bus; >> + struct pci_host_bridge *host; >> struct device_node *node = hose->dn; >> - int mode; >> >> pr_debug("PCI: Scanning PHB %s\n", of_node_full_name(node)); >> >> @@ -1609,30 +1636,17 @@ void pcibios_scan_phb(struct pci_controller *hose) >> hose->busn.flags = IORESOURCE_BUS; >> pci_add_resource(&resources, &hose->busn); >> >> + pci_host_ops.pci_ops = hose->ops; >> /* Create an empty bus for the toplevel */ >> - bus = pci_create_root_bus(hose->parent, hose->global_number, >> - hose->first_busno, hose->ops, hose, &resources); >> - if (bus == NULL) { >> - pr_err("Failed to create bus for PCI domain %04x\n", >> - hose->global_number); >> + host = pci_scan_host_bridge(hose->parent, hose->global_number, >> + hose->first_busno, hose, &resources, &pci_host_ops); >> + if (host == NULL) { >> + pr_err("Failed to create host bridge for pci%04x:%02x\n", >> + hose->global_number, hose->first_busno); >> pci_free_resource_list(&resources); >> return; >> } >> - hose->bus = bus; >> - >> - /* Get probe mode and perform scan */ >> - mode = PCI_PROBE_NORMAL; >> - if (node && ppc_md.pci_probe_mode) >> - mode = ppc_md.pci_probe_mode(bus); >> - pr_debug(" probe mode: %d\n", mode); >> - if (mode == PCI_PROBE_DEVTREE) >> - of_scan_bus(node, bus); >> - >> - if (mode == PCI_PROBE_NORMAL) { >> - pci_bus_update_busn_res_end(bus, 255); >> - hose->last_busno = pci_scan_child_bus(bus); >> - pci_bus_update_busn_res_end(bus, hose->last_busno); >> - } >> + hose->bus = host->bus; >> >> /* Platform gets a chance to do some global fixups before >> * we proceed to resource allocation >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) >> ppc_md.pcibios_fixup_phb(hose); >> >> /* Configure PCI Express settings */ >> - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> struct pci_bus *child; >> - list_for_each_entry(child, &bus->children, node) >> + list_for_each_entry(child, &host->bus->children, node) >> pcie_bus_configure_settings(child); >> } >> } > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from szxga03-in.huawei.com ([119.145.14.66]:8145 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752685AbbDGCwx (ORCPT ); Mon, 6 Apr 2015 22:52:53 -0400 Message-ID: <5523466A.3090500@huawei.com> Date: Tue, 7 Apr 2015 10:52:26 +0800 From: Yijing Wang MIME-Version: 1.0 To: Daniel Axtens CC: Bjorn Helgaas , Jiang Liu , , Yinghai Lu , , Marc Zyngier , , Russell King , , Thomas Gleixner , Benjamin Herrenschmidt , Rusty Russell , Tony Luck , , "David S. Miller" , "Guan Xuetao" , , , Liviu Dudau , "Arnd Bergmann" , Geert Uytterhoeven , Subject: Re: [PATCH v9 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-21-git-send-email-wangyijing@huawei.com> <1428363334.22268.9.camel@axtens.net> In-Reply-To: <1428363334.22268.9.camel@axtens.net> Content-Type: text/plain; charset="UTF-8" Sender: linux-pci-owner@vger.kernel.org List-ID: On 2015/4/7 7:35, Daniel Axtens wrote: > I've been looking at this patch series for a while now, and I now > believe it's ready on the PowerPC side. > > I was originally concerned that it would break odd corner cases, > particularly where similar code appears (namely kernel/pci_hotplug.c and > kernel/pci_of_scan.c). However, upon further examination, talking with > Yijing, and some testing, I'm now convinced that it is indeed restricted > to the generic code, and doesn't change behaviour. > > This is both a plus and a minus: because it's currently restricted to > generic code, I'm confident it works, but the down side is that it > doesn't yet simplify our arch-specific complexity. We'll need to do some > more work on our side to reap the full benefits. > > I tested this entire series on a PowerNV machine, including doing EEH > injection to trigger PCI hotplug: > Tested-by: Daniel Axtens > For completeness, it would be good to test it on Cell, as they are the > only remaining user of pci_of_scan.c > > In conclusion, this patch is > Reviewed-by: Daniel Axtens > Thanks very much for your test and review. Thanks! Yijing. > > > On Fri, 2015-04-03 at 17:25 +0800, Yijing Wang wrote: >> Now we could use pci_scan_host_bridge() to scan >> pci buses, provide powerpc specific pci_host_bridge_ops. >> >> Signed-off-by: Yijing Wang >> CC: Benjamin Herrenschmidt >> CC: linuxppc-dev@lists.ozlabs.org >> --- >> arch/powerpc/kernel/pci-common.c | 62 +++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 24 deletions(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 2c58200..50b32f6 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -773,6 +773,29 @@ void pcibios_set_root_bus_speed(struct pci_host_bridge *bridge) >> return ppc_md.pcibios_set_root_bus_speed(bridge); >> } >> >> +static int pci_host_scan_bus(struct pci_host_bridge *host) >> +{ >> + int mode = PCI_PROBE_NORMAL; >> + struct pci_bus *bus = host->bus; >> + struct pci_controller *hose = dev_get_drvdata(&host->dev); >> + >> + /* Get probe mode and perform scan */ >> + if (hose->dn && ppc_md.pci_probe_mode) >> + mode = ppc_md.pci_probe_mode(bus); >> + >> + pr_debug(" probe mode: %d\n", mode); >> + if (mode == PCI_PROBE_DEVTREE) >> + of_scan_bus(hose->dn, bus); >> + >> + if (mode == PCI_PROBE_NORMAL) { >> + pci_bus_update_busn_res_end(bus, 255); >> + hose->last_busno = pci_scan_child_bus(bus); >> + pci_bus_update_busn_res_end(bus, hose->last_busno); >> + } >> + >> + return pci_bus_child_max_busnr(bus); >> +} >> + >> /* This header fixup will do the resource fixup for all devices as they are >> * probed, but not for bridge ranges >> */ >> @@ -1585,6 +1608,11 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> return of_node_get(hose->dn); >> } >> >> +static struct pci_host_bridge_ops pci_host_ops = { >> + .set_root_bus_speed = pcibios_set_root_bus_speed, >> + .scan_bus = pci_host_scan_bus, >> +}; >> + >> /** >> * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus >> * @hose: Pointer to the PCI host controller instance structure >> @@ -1592,9 +1620,8 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> void pcibios_scan_phb(struct pci_controller *hose) >> { >> LIST_HEAD(resources); >> - struct pci_bus *bus; >> + struct pci_host_bridge *host; >> struct device_node *node = hose->dn; >> - int mode; >> >> pr_debug("PCI: Scanning PHB %s\n", of_node_full_name(node)); >> >> @@ -1609,30 +1636,17 @@ void pcibios_scan_phb(struct pci_controller *hose) >> hose->busn.flags = IORESOURCE_BUS; >> pci_add_resource(&resources, &hose->busn); >> >> + pci_host_ops.pci_ops = hose->ops; >> /* Create an empty bus for the toplevel */ >> - bus = pci_create_root_bus(hose->parent, hose->global_number, >> - hose->first_busno, hose->ops, hose, &resources); >> - if (bus == NULL) { >> - pr_err("Failed to create bus for PCI domain %04x\n", >> - hose->global_number); >> + host = pci_scan_host_bridge(hose->parent, hose->global_number, >> + hose->first_busno, hose, &resources, &pci_host_ops); >> + if (host == NULL) { >> + pr_err("Failed to create host bridge for pci%04x:%02x\n", >> + hose->global_number, hose->first_busno); >> pci_free_resource_list(&resources); >> return; >> } >> - hose->bus = bus; >> - >> - /* Get probe mode and perform scan */ >> - mode = PCI_PROBE_NORMAL; >> - if (node && ppc_md.pci_probe_mode) >> - mode = ppc_md.pci_probe_mode(bus); >> - pr_debug(" probe mode: %d\n", mode); >> - if (mode == PCI_PROBE_DEVTREE) >> - of_scan_bus(node, bus); >> - >> - if (mode == PCI_PROBE_NORMAL) { >> - pci_bus_update_busn_res_end(bus, 255); >> - hose->last_busno = pci_scan_child_bus(bus); >> - pci_bus_update_busn_res_end(bus, hose->last_busno); >> - } >> + hose->bus = host->bus; >> >> /* Platform gets a chance to do some global fixups before >> * we proceed to resource allocation >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) >> ppc_md.pcibios_fixup_phb(hose); >> >> /* Configure PCI Express settings */ >> - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> struct pci_bus *child; >> - list_for_each_entry(child, &bus->children, node) >> + list_for_each_entry(child, &host->bus->children, node) >> pcie_bus_configure_settings(child); >> } >> } > -- 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 E5A0F1A074A for ; Tue, 7 Apr 2015 12:53:11 +1000 (AEST) Message-ID: <5523466A.3090500@huawei.com> Date: Tue, 7 Apr 2015 10:52:26 +0800 From: Yijing Wang MIME-Version: 1.0 To: Daniel Axtens Subject: Re: [PATCH v9 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-21-git-send-email-wangyijing@huawei.com> <1428363334.22268.9.camel@axtens.net> In-Reply-To: <1428363334.22268.9.camel@axtens.net> Content-Type: text/plain; charset="UTF-8" Cc: linux-ia64@vger.kernel.org, linux-pci@vger.kernel.org, Guan Xuetao , Russell King , x86@kernel.org, Geert Uytterhoeven , Arnd Bergmann , Marc Zyngier , Rusty Russell , linux-m68k@lists.linux-m68k.org, Bjorn Helgaas , Thomas Gleixner , Yinghai Lu , linux-arm-kernel@lists.infradead.org, Liviu Dudau , Tony Luck , linux-kernel@vger.kernel.org, Jiang Liu , linux-alpha@vger.kernel.org, linuxppc-dev@lists.ozlabs.org, "David S. Miller" List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On 2015/4/7 7:35, Daniel Axtens wrote: > I've been looking at this patch series for a while now, and I now > believe it's ready on the PowerPC side. > > I was originally concerned that it would break odd corner cases, > particularly where similar code appears (namely kernel/pci_hotplug.c and > kernel/pci_of_scan.c). However, upon further examination, talking with > Yijing, and some testing, I'm now convinced that it is indeed restricted > to the generic code, and doesn't change behaviour. > > This is both a plus and a minus: because it's currently restricted to > generic code, I'm confident it works, but the down side is that it > doesn't yet simplify our arch-specific complexity. We'll need to do some > more work on our side to reap the full benefits. > > I tested this entire series on a PowerNV machine, including doing EEH > injection to trigger PCI hotplug: > Tested-by: Daniel Axtens > For completeness, it would be good to test it on Cell, as they are the > only remaining user of pci_of_scan.c > > In conclusion, this patch is > Reviewed-by: Daniel Axtens > Thanks very much for your test and review. Thanks! Yijing. > > > On Fri, 2015-04-03 at 17:25 +0800, Yijing Wang wrote: >> Now we could use pci_scan_host_bridge() to scan >> pci buses, provide powerpc specific pci_host_bridge_ops. >> >> Signed-off-by: Yijing Wang >> CC: Benjamin Herrenschmidt >> CC: linuxppc-dev@lists.ozlabs.org >> --- >> arch/powerpc/kernel/pci-common.c | 62 +++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 24 deletions(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 2c58200..50b32f6 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -773,6 +773,29 @@ void pcibios_set_root_bus_speed(struct pci_host_bridge *bridge) >> return ppc_md.pcibios_set_root_bus_speed(bridge); >> } >> >> +static int pci_host_scan_bus(struct pci_host_bridge *host) >> +{ >> + int mode = PCI_PROBE_NORMAL; >> + struct pci_bus *bus = host->bus; >> + struct pci_controller *hose = dev_get_drvdata(&host->dev); >> + >> + /* Get probe mode and perform scan */ >> + if (hose->dn && ppc_md.pci_probe_mode) >> + mode = ppc_md.pci_probe_mode(bus); >> + >> + pr_debug(" probe mode: %d\n", mode); >> + if (mode == PCI_PROBE_DEVTREE) >> + of_scan_bus(hose->dn, bus); >> + >> + if (mode == PCI_PROBE_NORMAL) { >> + pci_bus_update_busn_res_end(bus, 255); >> + hose->last_busno = pci_scan_child_bus(bus); >> + pci_bus_update_busn_res_end(bus, hose->last_busno); >> + } >> + >> + return pci_bus_child_max_busnr(bus); >> +} >> + >> /* This header fixup will do the resource fixup for all devices as they are >> * probed, but not for bridge ranges >> */ >> @@ -1585,6 +1608,11 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> return of_node_get(hose->dn); >> } >> >> +static struct pci_host_bridge_ops pci_host_ops = { >> + .set_root_bus_speed = pcibios_set_root_bus_speed, >> + .scan_bus = pci_host_scan_bus, >> +}; >> + >> /** >> * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus >> * @hose: Pointer to the PCI host controller instance structure >> @@ -1592,9 +1620,8 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> void pcibios_scan_phb(struct pci_controller *hose) >> { >> LIST_HEAD(resources); >> - struct pci_bus *bus; >> + struct pci_host_bridge *host; >> struct device_node *node = hose->dn; >> - int mode; >> >> pr_debug("PCI: Scanning PHB %s\n", of_node_full_name(node)); >> >> @@ -1609,30 +1636,17 @@ void pcibios_scan_phb(struct pci_controller *hose) >> hose->busn.flags = IORESOURCE_BUS; >> pci_add_resource(&resources, &hose->busn); >> >> + pci_host_ops.pci_ops = hose->ops; >> /* Create an empty bus for the toplevel */ >> - bus = pci_create_root_bus(hose->parent, hose->global_number, >> - hose->first_busno, hose->ops, hose, &resources); >> - if (bus == NULL) { >> - pr_err("Failed to create bus for PCI domain %04x\n", >> - hose->global_number); >> + host = pci_scan_host_bridge(hose->parent, hose->global_number, >> + hose->first_busno, hose, &resources, &pci_host_ops); >> + if (host == NULL) { >> + pr_err("Failed to create host bridge for pci%04x:%02x\n", >> + hose->global_number, hose->first_busno); >> pci_free_resource_list(&resources); >> return; >> } >> - hose->bus = bus; >> - >> - /* Get probe mode and perform scan */ >> - mode = PCI_PROBE_NORMAL; >> - if (node && ppc_md.pci_probe_mode) >> - mode = ppc_md.pci_probe_mode(bus); >> - pr_debug(" probe mode: %d\n", mode); >> - if (mode == PCI_PROBE_DEVTREE) >> - of_scan_bus(node, bus); >> - >> - if (mode == PCI_PROBE_NORMAL) { >> - pci_bus_update_busn_res_end(bus, 255); >> - hose->last_busno = pci_scan_child_bus(bus); >> - pci_bus_update_busn_res_end(bus, hose->last_busno); >> - } >> + hose->bus = host->bus; >> >> /* Platform gets a chance to do some global fixups before >> * we proceed to resource allocation >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) >> ppc_md.pcibios_fixup_phb(hose); >> >> /* Configure PCI Express settings */ >> - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> struct pci_bus *child; >> - list_for_each_entry(child, &bus->children, node) >> + list_for_each_entry(child, &host->bus->children, node) >> pcie_bus_configure_settings(child); >> } >> } > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 From: wangyijing@huawei.com (Yijing Wang) Date: Tue, 7 Apr 2015 10:52:26 +0800 Subject: [PATCH v9 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity In-Reply-To: <1428363334.22268.9.camel@axtens.net> References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-21-git-send-email-wangyijing@huawei.com> <1428363334.22268.9.camel@axtens.net> Message-ID: <5523466A.3090500@huawei.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2015/4/7 7:35, Daniel Axtens wrote: > I've been looking at this patch series for a while now, and I now > believe it's ready on the PowerPC side. > > I was originally concerned that it would break odd corner cases, > particularly where similar code appears (namely kernel/pci_hotplug.c and > kernel/pci_of_scan.c). However, upon further examination, talking with > Yijing, and some testing, I'm now convinced that it is indeed restricted > to the generic code, and doesn't change behaviour. > > This is both a plus and a minus: because it's currently restricted to > generic code, I'm confident it works, but the down side is that it > doesn't yet simplify our arch-specific complexity. We'll need to do some > more work on our side to reap the full benefits. > > I tested this entire series on a PowerNV machine, including doing EEH > injection to trigger PCI hotplug: > Tested-by: Daniel Axtens > For completeness, it would be good to test it on Cell, as they are the > only remaining user of pci_of_scan.c > > In conclusion, this patch is > Reviewed-by: Daniel Axtens > Thanks very much for your test and review. Thanks! Yijing. > > > On Fri, 2015-04-03 at 17:25 +0800, Yijing Wang wrote: >> Now we could use pci_scan_host_bridge() to scan >> pci buses, provide powerpc specific pci_host_bridge_ops. >> >> Signed-off-by: Yijing Wang >> CC: Benjamin Herrenschmidt >> CC: linuxppc-dev at lists.ozlabs.org >> --- >> arch/powerpc/kernel/pci-common.c | 62 +++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 24 deletions(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 2c58200..50b32f6 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -773,6 +773,29 @@ void pcibios_set_root_bus_speed(struct pci_host_bridge *bridge) >> return ppc_md.pcibios_set_root_bus_speed(bridge); >> } >> >> +static int pci_host_scan_bus(struct pci_host_bridge *host) >> +{ >> + int mode = PCI_PROBE_NORMAL; >> + struct pci_bus *bus = host->bus; >> + struct pci_controller *hose = dev_get_drvdata(&host->dev); >> + >> + /* Get probe mode and perform scan */ >> + if (hose->dn && ppc_md.pci_probe_mode) >> + mode = ppc_md.pci_probe_mode(bus); >> + >> + pr_debug(" probe mode: %d\n", mode); >> + if (mode == PCI_PROBE_DEVTREE) >> + of_scan_bus(hose->dn, bus); >> + >> + if (mode == PCI_PROBE_NORMAL) { >> + pci_bus_update_busn_res_end(bus, 255); >> + hose->last_busno = pci_scan_child_bus(bus); >> + pci_bus_update_busn_res_end(bus, hose->last_busno); >> + } >> + >> + return pci_bus_child_max_busnr(bus); >> +} >> + >> /* This header fixup will do the resource fixup for all devices as they are >> * probed, but not for bridge ranges >> */ >> @@ -1585,6 +1608,11 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> return of_node_get(hose->dn); >> } >> >> +static struct pci_host_bridge_ops pci_host_ops = { >> + .set_root_bus_speed = pcibios_set_root_bus_speed, >> + .scan_bus = pci_host_scan_bus, >> +}; >> + >> /** >> * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus >> * @hose: Pointer to the PCI host controller instance structure >> @@ -1592,9 +1620,8 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> void pcibios_scan_phb(struct pci_controller *hose) >> { >> LIST_HEAD(resources); >> - struct pci_bus *bus; >> + struct pci_host_bridge *host; >> struct device_node *node = hose->dn; >> - int mode; >> >> pr_debug("PCI: Scanning PHB %s\n", of_node_full_name(node)); >> >> @@ -1609,30 +1636,17 @@ void pcibios_scan_phb(struct pci_controller *hose) >> hose->busn.flags = IORESOURCE_BUS; >> pci_add_resource(&resources, &hose->busn); >> >> + pci_host_ops.pci_ops = hose->ops; >> /* Create an empty bus for the toplevel */ >> - bus = pci_create_root_bus(hose->parent, hose->global_number, >> - hose->first_busno, hose->ops, hose, &resources); >> - if (bus == NULL) { >> - pr_err("Failed to create bus for PCI domain %04x\n", >> - hose->global_number); >> + host = pci_scan_host_bridge(hose->parent, hose->global_number, >> + hose->first_busno, hose, &resources, &pci_host_ops); >> + if (host == NULL) { >> + pr_err("Failed to create host bridge for pci%04x:%02x\n", >> + hose->global_number, hose->first_busno); >> pci_free_resource_list(&resources); >> return; >> } >> - hose->bus = bus; >> - >> - /* Get probe mode and perform scan */ >> - mode = PCI_PROBE_NORMAL; >> - if (node && ppc_md.pci_probe_mode) >> - mode = ppc_md.pci_probe_mode(bus); >> - pr_debug(" probe mode: %d\n", mode); >> - if (mode == PCI_PROBE_DEVTREE) >> - of_scan_bus(node, bus); >> - >> - if (mode == PCI_PROBE_NORMAL) { >> - pci_bus_update_busn_res_end(bus, 255); >> - hose->last_busno = pci_scan_child_bus(bus); >> - pci_bus_update_busn_res_end(bus, hose->last_busno); >> - } >> + hose->bus = host->bus; >> >> /* Platform gets a chance to do some global fixups before >> * we proceed to resource allocation >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) >> ppc_md.pcibios_fixup_phb(hose); >> >> /* Configure PCI Express settings */ >> - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> struct pci_bus *child; >> - list_for_each_entry(child, &bus->children, node) >> + list_for_each_entry(child, &host->bus->children, node) >> pcie_bus_configure_settings(child); >> } >> } > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yijing Wang Date: Tue, 07 Apr 2015 02:52:26 +0000 Subject: Re: [PATCH v9 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity Message-Id: <5523466A.3090500@huawei.com> List-Id: References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-21-git-send-email-wangyijing@huawei.com> <1428363334.22268.9.camel@axtens.net> In-Reply-To: <1428363334.22268.9.camel@axtens.net> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Daniel Axtens Cc: Bjorn Helgaas , Jiang Liu , linux-pci@vger.kernel.org, Yinghai Lu , linux-kernel@vger.kernel.org, Marc Zyngier , linux-arm-kernel@lists.infradead.org, Russell King , x86@kernel.org, Thomas Gleixner , Benjamin Herrenschmidt , Rusty Russell , Tony Luck , linux-ia64@vger.kernel.org, "David S. Miller" , Guan Xuetao , linux-alpha@vger.kernel.org, linux-m68k@vger.kernel.org, Liviu Dudau , Arnd Bergmann , Geert Uytterhoeven , linuxppc-dev@lists.ozlabs.org On 2015/4/7 7:35, Daniel Axtens wrote: > I've been looking at this patch series for a while now, and I now > believe it's ready on the PowerPC side. > > I was originally concerned that it would break odd corner cases, > particularly where similar code appears (namely kernel/pci_hotplug.c and > kernel/pci_of_scan.c). However, upon further examination, talking with > Yijing, and some testing, I'm now convinced that it is indeed restricted > to the generic code, and doesn't change behaviour. > > This is both a plus and a minus: because it's currently restricted to > generic code, I'm confident it works, but the down side is that it > doesn't yet simplify our arch-specific complexity. We'll need to do some > more work on our side to reap the full benefits. > > I tested this entire series on a PowerNV machine, including doing EEH > injection to trigger PCI hotplug: > Tested-by: Daniel Axtens > For completeness, it would be good to test it on Cell, as they are the > only remaining user of pci_of_scan.c > > In conclusion, this patch is > Reviewed-by: Daniel Axtens > Thanks very much for your test and review. Thanks! Yijing. > > > On Fri, 2015-04-03 at 17:25 +0800, Yijing Wang wrote: >> Now we could use pci_scan_host_bridge() to scan >> pci buses, provide powerpc specific pci_host_bridge_ops. >> >> Signed-off-by: Yijing Wang >> CC: Benjamin Herrenschmidt >> CC: linuxppc-dev@lists.ozlabs.org >> --- >> arch/powerpc/kernel/pci-common.c | 62 +++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 24 deletions(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 2c58200..50b32f6 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -773,6 +773,29 @@ void pcibios_set_root_bus_speed(struct pci_host_bridge *bridge) >> return ppc_md.pcibios_set_root_bus_speed(bridge); >> } >> >> +static int pci_host_scan_bus(struct pci_host_bridge *host) >> +{ >> + int mode = PCI_PROBE_NORMAL; >> + struct pci_bus *bus = host->bus; >> + struct pci_controller *hose = dev_get_drvdata(&host->dev); >> + >> + /* Get probe mode and perform scan */ >> + if (hose->dn && ppc_md.pci_probe_mode) >> + mode = ppc_md.pci_probe_mode(bus); >> + >> + pr_debug(" probe mode: %d\n", mode); >> + if (mode = PCI_PROBE_DEVTREE) >> + of_scan_bus(hose->dn, bus); >> + >> + if (mode = PCI_PROBE_NORMAL) { >> + pci_bus_update_busn_res_end(bus, 255); >> + hose->last_busno = pci_scan_child_bus(bus); >> + pci_bus_update_busn_res_end(bus, hose->last_busno); >> + } >> + >> + return pci_bus_child_max_busnr(bus); >> +} >> + >> /* This header fixup will do the resource fixup for all devices as they are >> * probed, but not for bridge ranges >> */ >> @@ -1585,6 +1608,11 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> return of_node_get(hose->dn); >> } >> >> +static struct pci_host_bridge_ops pci_host_ops = { >> + .set_root_bus_speed = pcibios_set_root_bus_speed, >> + .scan_bus = pci_host_scan_bus, >> +}; >> + >> /** >> * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus >> * @hose: Pointer to the PCI host controller instance structure >> @@ -1592,9 +1620,8 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> void pcibios_scan_phb(struct pci_controller *hose) >> { >> LIST_HEAD(resources); >> - struct pci_bus *bus; >> + struct pci_host_bridge *host; >> struct device_node *node = hose->dn; >> - int mode; >> >> pr_debug("PCI: Scanning PHB %s\n", of_node_full_name(node)); >> >> @@ -1609,30 +1636,17 @@ void pcibios_scan_phb(struct pci_controller *hose) >> hose->busn.flags = IORESOURCE_BUS; >> pci_add_resource(&resources, &hose->busn); >> >> + pci_host_ops.pci_ops = hose->ops; >> /* Create an empty bus for the toplevel */ >> - bus = pci_create_root_bus(hose->parent, hose->global_number, >> - hose->first_busno, hose->ops, hose, &resources); >> - if (bus = NULL) { >> - pr_err("Failed to create bus for PCI domain %04x\n", >> - hose->global_number); >> + host = pci_scan_host_bridge(hose->parent, hose->global_number, >> + hose->first_busno, hose, &resources, &pci_host_ops); >> + if (host = NULL) { >> + pr_err("Failed to create host bridge for pci%04x:%02x\n", >> + hose->global_number, hose->first_busno); >> pci_free_resource_list(&resources); >> return; >> } >> - hose->bus = bus; >> - >> - /* Get probe mode and perform scan */ >> - mode = PCI_PROBE_NORMAL; >> - if (node && ppc_md.pci_probe_mode) >> - mode = ppc_md.pci_probe_mode(bus); >> - pr_debug(" probe mode: %d\n", mode); >> - if (mode = PCI_PROBE_DEVTREE) >> - of_scan_bus(node, bus); >> - >> - if (mode = PCI_PROBE_NORMAL) { >> - pci_bus_update_busn_res_end(bus, 255); >> - hose->last_busno = pci_scan_child_bus(bus); >> - pci_bus_update_busn_res_end(bus, hose->last_busno); >> - } >> + hose->bus = host->bus; >> >> /* Platform gets a chance to do some global fixups before >> * we proceed to resource allocation >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) >> ppc_md.pcibios_fixup_phb(hose); >> >> /* Configure PCI Express settings */ >> - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> struct pci_bus *child; >> - list_for_each_entry(child, &bus->children, node) >> + list_for_each_entry(child, &host->bus->children, node) >> pcie_bus_configure_settings(child); >> } >> } > -- Thanks! Yijing From mboxrd@z Thu Jan 1 00:00:00 1970 From: Yijing Wang Subject: Re: [PATCH v9 19/30] powerpc/pci: Use pci_scan_host_bridge() for simplicity Date: Tue, 7 Apr 2015 10:52:26 +0800 Message-ID: <5523466A.3090500@huawei.com> References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-21-git-send-email-wangyijing@huawei.com> <1428363334.22268.9.camel@axtens.net> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1428363334.22268.9.camel@axtens.net> Sender: linux-ia64-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii" To: Daniel Axtens Cc: Bjorn Helgaas , Jiang Liu , linux-pci@vger.kernel.org, Yinghai Lu , linux-kernel@vger.kernel.org, Marc Zyngier , linux-arm-kernel@lists.infradead.org, Russell King , x86@kernel.org, Thomas Gleixner , Benjamin Herrenschmidt , Rusty Russell , Tony Luck , linux-ia64@vger.kernel.org, "David S. Miller" , Guan Xuetao , linux-alpha@vger.kernel.org, linux-m68k@lists.linux-m68k.org, Liviu Dudau , Arnd Bergmann , Geert Uytterhoeven , linuxppc-dev@lists.ozlabs.org On 2015/4/7 7:35, Daniel Axtens wrote: > I've been looking at this patch series for a while now, and I now > believe it's ready on the PowerPC side. > > I was originally concerned that it would break odd corner cases, > particularly where similar code appears (namely kernel/pci_hotplug.c and > kernel/pci_of_scan.c). However, upon further examination, talking with > Yijing, and some testing, I'm now convinced that it is indeed restricted > to the generic code, and doesn't change behaviour. > > This is both a plus and a minus: because it's currently restricted to > generic code, I'm confident it works, but the down side is that it > doesn't yet simplify our arch-specific complexity. We'll need to do some > more work on our side to reap the full benefits. > > I tested this entire series on a PowerNV machine, including doing EEH > injection to trigger PCI hotplug: > Tested-by: Daniel Axtens > For completeness, it would be good to test it on Cell, as they are the > only remaining user of pci_of_scan.c > > In conclusion, this patch is > Reviewed-by: Daniel Axtens > Thanks very much for your test and review. Thanks! Yijing. > > > On Fri, 2015-04-03 at 17:25 +0800, Yijing Wang wrote: >> Now we could use pci_scan_host_bridge() to scan >> pci buses, provide powerpc specific pci_host_bridge_ops. >> >> Signed-off-by: Yijing Wang >> CC: Benjamin Herrenschmidt >> CC: linuxppc-dev@lists.ozlabs.org >> --- >> arch/powerpc/kernel/pci-common.c | 62 +++++++++++++++++++++++-------------- >> 1 files changed, 38 insertions(+), 24 deletions(-) >> >> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c >> index 2c58200..50b32f6 100644 >> --- a/arch/powerpc/kernel/pci-common.c >> +++ b/arch/powerpc/kernel/pci-common.c >> @@ -773,6 +773,29 @@ void pcibios_set_root_bus_speed(struct pci_host_bridge *bridge) >> return ppc_md.pcibios_set_root_bus_speed(bridge); >> } >> >> +static int pci_host_scan_bus(struct pci_host_bridge *host) >> +{ >> + int mode = PCI_PROBE_NORMAL; >> + struct pci_bus *bus = host->bus; >> + struct pci_controller *hose = dev_get_drvdata(&host->dev); >> + >> + /* Get probe mode and perform scan */ >> + if (hose->dn && ppc_md.pci_probe_mode) >> + mode = ppc_md.pci_probe_mode(bus); >> + >> + pr_debug(" probe mode: %d\n", mode); >> + if (mode == PCI_PROBE_DEVTREE) >> + of_scan_bus(hose->dn, bus); >> + >> + if (mode == PCI_PROBE_NORMAL) { >> + pci_bus_update_busn_res_end(bus, 255); >> + hose->last_busno = pci_scan_child_bus(bus); >> + pci_bus_update_busn_res_end(bus, hose->last_busno); >> + } >> + >> + return pci_bus_child_max_busnr(bus); >> +} >> + >> /* This header fixup will do the resource fixup for all devices as they are >> * probed, but not for bridge ranges >> */ >> @@ -1585,6 +1608,11 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> return of_node_get(hose->dn); >> } >> >> +static struct pci_host_bridge_ops pci_host_ops = { >> + .set_root_bus_speed = pcibios_set_root_bus_speed, >> + .scan_bus = pci_host_scan_bus, >> +}; >> + >> /** >> * pci_scan_phb - Given a pci_controller, setup and scan the PCI bus >> * @hose: Pointer to the PCI host controller instance structure >> @@ -1592,9 +1620,8 @@ struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) >> void pcibios_scan_phb(struct pci_controller *hose) >> { >> LIST_HEAD(resources); >> - struct pci_bus *bus; >> + struct pci_host_bridge *host; >> struct device_node *node = hose->dn; >> - int mode; >> >> pr_debug("PCI: Scanning PHB %s\n", of_node_full_name(node)); >> >> @@ -1609,30 +1636,17 @@ void pcibios_scan_phb(struct pci_controller *hose) >> hose->busn.flags = IORESOURCE_BUS; >> pci_add_resource(&resources, &hose->busn); >> >> + pci_host_ops.pci_ops = hose->ops; >> /* Create an empty bus for the toplevel */ >> - bus = pci_create_root_bus(hose->parent, hose->global_number, >> - hose->first_busno, hose->ops, hose, &resources); >> - if (bus == NULL) { >> - pr_err("Failed to create bus for PCI domain %04x\n", >> - hose->global_number); >> + host = pci_scan_host_bridge(hose->parent, hose->global_number, >> + hose->first_busno, hose, &resources, &pci_host_ops); >> + if (host == NULL) { >> + pr_err("Failed to create host bridge for pci%04x:%02x\n", >> + hose->global_number, hose->first_busno); >> pci_free_resource_list(&resources); >> return; >> } >> - hose->bus = bus; >> - >> - /* Get probe mode and perform scan */ >> - mode = PCI_PROBE_NORMAL; >> - if (node && ppc_md.pci_probe_mode) >> - mode = ppc_md.pci_probe_mode(bus); >> - pr_debug(" probe mode: %d\n", mode); >> - if (mode == PCI_PROBE_DEVTREE) >> - of_scan_bus(node, bus); >> - >> - if (mode == PCI_PROBE_NORMAL) { >> - pci_bus_update_busn_res_end(bus, 255); >> - hose->last_busno = pci_scan_child_bus(bus); >> - pci_bus_update_busn_res_end(bus, hose->last_busno); >> - } >> + hose->bus = host->bus; >> >> /* Platform gets a chance to do some global fixups before >> * we proceed to resource allocation >> @@ -1641,9 +1655,9 @@ void pcibios_scan_phb(struct pci_controller *hose) >> ppc_md.pcibios_fixup_phb(hose); >> >> /* Configure PCI Express settings */ >> - if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> + if (host->bus && !pci_has_flag(PCI_PROBE_ONLY)) { >> struct pci_bus *child; >> - list_for_each_entry(child, &bus->children, node) >> + list_for_each_entry(child, &host->bus->children, node) >> pcie_bus_configure_settings(child); >> } >> } > -- Thanks! Yijing