From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753283AbbDGWZv (ORCPT ); Tue, 7 Apr 2015 18:25:51 -0400 Received: from mail-ig0-f180.google.com ([209.85.213.180]:35651 "EHLO mail-ig0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753089AbbDGWZq (ORCPT ); Tue, 7 Apr 2015 18:25:46 -0400 Date: Tue, 7 Apr 2015 17:25:41 -0500 From: Bjorn Helgaas To: Yijing Wang Cc: Jiang Liu , linux-pci@vger.kernel.org, Yinghai Lu , linux-kernel@vger.kernel.org, Marc Zyngier , linux-arm-kernel@lists.infradead.org, Russell King , dja@axtens.net, 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 Subject: Re: [PATCH v9 07/30] PCI: Add default bus resource in pci_host_bridge Message-ID: <20150407222541.GK10892@google.com> References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-9-git-send-email-wangyijing@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1428053164-28277-9-git-send-email-wangyijing@huawei.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 03, 2015 at 05:25:41PM +0800, Yijing Wang wrote: > If there is no busn resource provided for pci_scan_root_bus(), > we would insert a default bus resource (root_bus_number, 255) > in root bus, and update the max bus number we found after > pci_scan_child_bus(). We also need to hold the default bus > resource in pci_host_bridge, then we could identify whether > the new pci host bridge conflict with existing one. > > Signed-off-by: Yijing Wang > --- > drivers/pci/host-bridge.c | 20 ++++++++++++++++++++ > drivers/pci/probe.c | 26 ++++++++++---------------- > include/linux/pci.h | 2 ++ > 3 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c > index 7d52a0a..ecc1a7c 100644 > --- a/drivers/pci/host-bridge.c > +++ b/drivers/pci/host-bridge.c > @@ -19,6 +19,25 @@ static void pci_release_host_bridge_dev(struct device *dev) > kfree(bridge); > } > > +static void pci_host_update_busn_res( > + struct pci_host_bridge *host, int bus, > + struct list_head *resources) > +{ > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, resources) > + if (window->res->flags & IORESOURCE_BUS) > + return; > + > + pr_info( > + "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n", > + host->domain, bus, bus); > + host->busn_res.flags = IORESOURCE_BUS; > + host->busn_res.start = bus; > + host->busn_res.end = 255; > + pci_add_resource(resources, &host->busn_res); > +} > + > struct pci_host_bridge *pci_create_host_bridge( > struct device *parent, int domain, int bus, > struct list_head *resources) > @@ -33,6 +52,7 @@ struct pci_host_bridge *pci_create_host_bridge( > > host->dev.parent = parent; > INIT_LIST_HEAD(&host->windows); > + pci_host_update_busn_res(host, bus, resources); > resource_list_for_each_entry_safe(window, n, resources) > list_move_tail(&window->node, &host->windows); > /* > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 9bc4784..d5a12d9 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2022,6 +2022,12 @@ int pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max) > return ret; > } > > +static void pci_host_update_busn_res_end( > + struct pci_host_bridge *host, int max) > +{ > + host->busn_res.end = max; > +} > + > void pci_bus_release_busn_res(struct pci_bus *b) > { > struct resource *res = &b->busn_res; > @@ -2040,32 +2046,20 @@ static struct pci_bus *__pci_scan_root_bus(int bus, > struct pci_host_bridge *host, struct pci_ops *ops, > void *sysdata) > { > - struct resource_entry *window; > - bool found = false; > struct pci_bus *b; > int max; > > - resource_list_for_each_entry(window, &host->windows) > - if (window->res->flags & IORESOURCE_BUS) { > - found = true; > - break; > - } > - > b = __pci_create_root_bus(bus, host, ops, sysdata); > if (!b) > return NULL; > > - if (!found) { > - dev_info(&b->dev, > - "No busn resource found for root bus, will use [bus %02x-ff]\n", > - bus); > - pci_bus_insert_busn_res(b, bus, 255); > - } > - > max = pci_scan_child_bus(b); > > - if (!found) > + /* If default busn resource used, update the max bus number */ > + if (host->busn_res.flags & IORESOURCE_BUS) { > + pci_host_update_busn_res_end(host, max); > pci_bus_update_busn_res_end(b, max); > + } > > return b; > } > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1542df8..f189dfb 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -404,6 +404,8 @@ struct pci_host_bridge { > int domain; > struct device dev; > struct pci_bus *bus; /* root bus */ > + /* we use default bus resource if no bus resource provided */ > + struct resource busn_res; I don't understand the need for another busn_res here. The host bridge bus range should be identical to the root bus range. Having two copies will confuse things. And apparently this host->busn_res is only filled in if the arch doesn't provide a busn resource? To check for conflicts between host bridges, can you iterate through the existing ones and check the range of their root buses? > struct list_head windows; /* resource_entry */ > void (*release_fn)(struct pci_host_bridge *); > void *release_data; > -- > 1.7.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ie0-f178.google.com ([209.85.223.178]:34132 "EHLO mail-ie0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753087AbbDGWZq (ORCPT ); Tue, 7 Apr 2015 18:25:46 -0400 Received: by iedfl3 with SMTP id fl3so67548659ied.1 for ; Tue, 07 Apr 2015 15:25:45 -0700 (PDT) Date: Tue, 7 Apr 2015 17:25:41 -0500 From: Bjorn Helgaas To: Yijing Wang Cc: Jiang Liu , linux-pci@vger.kernel.org, Yinghai Lu , linux-kernel@vger.kernel.org, Marc Zyngier , linux-arm-kernel@lists.infradead.org, Russell King , dja@axtens.net, 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 Subject: Re: [PATCH v9 07/30] PCI: Add default bus resource in pci_host_bridge Message-ID: <20150407222541.GK10892@google.com> References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-9-git-send-email-wangyijing@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <1428053164-28277-9-git-send-email-wangyijing@huawei.com> Sender: linux-pci-owner@vger.kernel.org List-ID: On Fri, Apr 03, 2015 at 05:25:41PM +0800, Yijing Wang wrote: > If there is no busn resource provided for pci_scan_root_bus(), > we would insert a default bus resource (root_bus_number, 255) > in root bus, and update the max bus number we found after > pci_scan_child_bus(). We also need to hold the default bus > resource in pci_host_bridge, then we could identify whether > the new pci host bridge conflict with existing one. > > Signed-off-by: Yijing Wang > --- > drivers/pci/host-bridge.c | 20 ++++++++++++++++++++ > drivers/pci/probe.c | 26 ++++++++++---------------- > include/linux/pci.h | 2 ++ > 3 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c > index 7d52a0a..ecc1a7c 100644 > --- a/drivers/pci/host-bridge.c > +++ b/drivers/pci/host-bridge.c > @@ -19,6 +19,25 @@ static void pci_release_host_bridge_dev(struct device *dev) > kfree(bridge); > } > > +static void pci_host_update_busn_res( > + struct pci_host_bridge *host, int bus, > + struct list_head *resources) > +{ > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, resources) > + if (window->res->flags & IORESOURCE_BUS) > + return; > + > + pr_info( > + "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n", > + host->domain, bus, bus); > + host->busn_res.flags = IORESOURCE_BUS; > + host->busn_res.start = bus; > + host->busn_res.end = 255; > + pci_add_resource(resources, &host->busn_res); > +} > + > struct pci_host_bridge *pci_create_host_bridge( > struct device *parent, int domain, int bus, > struct list_head *resources) > @@ -33,6 +52,7 @@ struct pci_host_bridge *pci_create_host_bridge( > > host->dev.parent = parent; > INIT_LIST_HEAD(&host->windows); > + pci_host_update_busn_res(host, bus, resources); > resource_list_for_each_entry_safe(window, n, resources) > list_move_tail(&window->node, &host->windows); > /* > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 9bc4784..d5a12d9 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2022,6 +2022,12 @@ int pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max) > return ret; > } > > +static void pci_host_update_busn_res_end( > + struct pci_host_bridge *host, int max) > +{ > + host->busn_res.end = max; > +} > + > void pci_bus_release_busn_res(struct pci_bus *b) > { > struct resource *res = &b->busn_res; > @@ -2040,32 +2046,20 @@ static struct pci_bus *__pci_scan_root_bus(int bus, > struct pci_host_bridge *host, struct pci_ops *ops, > void *sysdata) > { > - struct resource_entry *window; > - bool found = false; > struct pci_bus *b; > int max; > > - resource_list_for_each_entry(window, &host->windows) > - if (window->res->flags & IORESOURCE_BUS) { > - found = true; > - break; > - } > - > b = __pci_create_root_bus(bus, host, ops, sysdata); > if (!b) > return NULL; > > - if (!found) { > - dev_info(&b->dev, > - "No busn resource found for root bus, will use [bus %02x-ff]\n", > - bus); > - pci_bus_insert_busn_res(b, bus, 255); > - } > - > max = pci_scan_child_bus(b); > > - if (!found) > + /* If default busn resource used, update the max bus number */ > + if (host->busn_res.flags & IORESOURCE_BUS) { > + pci_host_update_busn_res_end(host, max); > pci_bus_update_busn_res_end(b, max); > + } > > return b; > } > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1542df8..f189dfb 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -404,6 +404,8 @@ struct pci_host_bridge { > int domain; > struct device dev; > struct pci_bus *bus; /* root bus */ > + /* we use default bus resource if no bus resource provided */ > + struct resource busn_res; I don't understand the need for another busn_res here. The host bridge bus range should be identical to the root bus range. Having two copies will confuse things. And apparently this host->busn_res is only filled in if the arch doesn't provide a busn resource? To check for conflicts between host bridges, can you iterate through the existing ones and check the range of their root buses? > struct list_head windows; /* resource_entry */ > void (*release_fn)(struct pci_host_bridge *); > void *release_data; > -- > 1.7.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: bhelgaas@google.com (Bjorn Helgaas) Date: Tue, 7 Apr 2015 17:25:41 -0500 Subject: [PATCH v9 07/30] PCI: Add default bus resource in pci_host_bridge In-Reply-To: <1428053164-28277-9-git-send-email-wangyijing@huawei.com> References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-9-git-send-email-wangyijing@huawei.com> Message-ID: <20150407222541.GK10892@google.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Fri, Apr 03, 2015 at 05:25:41PM +0800, Yijing Wang wrote: > If there is no busn resource provided for pci_scan_root_bus(), > we would insert a default bus resource (root_bus_number, 255) > in root bus, and update the max bus number we found after > pci_scan_child_bus(). We also need to hold the default bus > resource in pci_host_bridge, then we could identify whether > the new pci host bridge conflict with existing one. > > Signed-off-by: Yijing Wang > --- > drivers/pci/host-bridge.c | 20 ++++++++++++++++++++ > drivers/pci/probe.c | 26 ++++++++++---------------- > include/linux/pci.h | 2 ++ > 3 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c > index 7d52a0a..ecc1a7c 100644 > --- a/drivers/pci/host-bridge.c > +++ b/drivers/pci/host-bridge.c > @@ -19,6 +19,25 @@ static void pci_release_host_bridge_dev(struct device *dev) > kfree(bridge); > } > > +static void pci_host_update_busn_res( > + struct pci_host_bridge *host, int bus, > + struct list_head *resources) > +{ > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, resources) > + if (window->res->flags & IORESOURCE_BUS) > + return; > + > + pr_info( > + "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n", > + host->domain, bus, bus); > + host->busn_res.flags = IORESOURCE_BUS; > + host->busn_res.start = bus; > + host->busn_res.end = 255; > + pci_add_resource(resources, &host->busn_res); > +} > + > struct pci_host_bridge *pci_create_host_bridge( > struct device *parent, int domain, int bus, > struct list_head *resources) > @@ -33,6 +52,7 @@ struct pci_host_bridge *pci_create_host_bridge( > > host->dev.parent = parent; > INIT_LIST_HEAD(&host->windows); > + pci_host_update_busn_res(host, bus, resources); > resource_list_for_each_entry_safe(window, n, resources) > list_move_tail(&window->node, &host->windows); > /* > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 9bc4784..d5a12d9 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2022,6 +2022,12 @@ int pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max) > return ret; > } > > +static void pci_host_update_busn_res_end( > + struct pci_host_bridge *host, int max) > +{ > + host->busn_res.end = max; > +} > + > void pci_bus_release_busn_res(struct pci_bus *b) > { > struct resource *res = &b->busn_res; > @@ -2040,32 +2046,20 @@ static struct pci_bus *__pci_scan_root_bus(int bus, > struct pci_host_bridge *host, struct pci_ops *ops, > void *sysdata) > { > - struct resource_entry *window; > - bool found = false; > struct pci_bus *b; > int max; > > - resource_list_for_each_entry(window, &host->windows) > - if (window->res->flags & IORESOURCE_BUS) { > - found = true; > - break; > - } > - > b = __pci_create_root_bus(bus, host, ops, sysdata); > if (!b) > return NULL; > > - if (!found) { > - dev_info(&b->dev, > - "No busn resource found for root bus, will use [bus %02x-ff]\n", > - bus); > - pci_bus_insert_busn_res(b, bus, 255); > - } > - > max = pci_scan_child_bus(b); > > - if (!found) > + /* If default busn resource used, update the max bus number */ > + if (host->busn_res.flags & IORESOURCE_BUS) { > + pci_host_update_busn_res_end(host, max); > pci_bus_update_busn_res_end(b, max); > + } > > return b; > } > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1542df8..f189dfb 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -404,6 +404,8 @@ struct pci_host_bridge { > int domain; > struct device dev; > struct pci_bus *bus; /* root bus */ > + /* we use default bus resource if no bus resource provided */ > + struct resource busn_res; I don't understand the need for another busn_res here. The host bridge bus range should be identical to the root bus range. Having two copies will confuse things. And apparently this host->busn_res is only filled in if the arch doesn't provide a busn resource? To check for conflicts between host bridges, can you iterate through the existing ones and check the range of their root buses? > struct list_head windows; /* resource_entry */ > void (*release_fn)(struct pci_host_bridge *); > void *release_data; > -- > 1.7.1 > From mboxrd@z Thu Jan 1 00:00:00 1970 From: Bjorn Helgaas Date: Tue, 07 Apr 2015 22:25:41 +0000 Subject: Re: [PATCH v9 07/30] PCI: Add default bus resource in pci_host_bridge Message-Id: <20150407222541.GK10892@google.com> List-Id: References: <1428053164-28277-1-git-send-email-wangyijing@huawei.com> <1428053164-28277-9-git-send-email-wangyijing@huawei.com> In-Reply-To: <1428053164-28277-9-git-send-email-wangyijing@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Yijing Wang Cc: Jiang Liu , linux-pci@vger.kernel.org, Yinghai Lu , linux-kernel@vger.kernel.org, Marc Zyngier , linux-arm-kernel@lists.infradead.org, Russell King , dja@axtens.net, 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 On Fri, Apr 03, 2015 at 05:25:41PM +0800, Yijing Wang wrote: > If there is no busn resource provided for pci_scan_root_bus(), > we would insert a default bus resource (root_bus_number, 255) > in root bus, and update the max bus number we found after > pci_scan_child_bus(). We also need to hold the default bus > resource in pci_host_bridge, then we could identify whether > the new pci host bridge conflict with existing one. > > Signed-off-by: Yijing Wang > --- > drivers/pci/host-bridge.c | 20 ++++++++++++++++++++ > drivers/pci/probe.c | 26 ++++++++++---------------- > include/linux/pci.h | 2 ++ > 3 files changed, 32 insertions(+), 16 deletions(-) > > diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c > index 7d52a0a..ecc1a7c 100644 > --- a/drivers/pci/host-bridge.c > +++ b/drivers/pci/host-bridge.c > @@ -19,6 +19,25 @@ static void pci_release_host_bridge_dev(struct device *dev) > kfree(bridge); > } > > +static void pci_host_update_busn_res( > + struct pci_host_bridge *host, int bus, > + struct list_head *resources) > +{ > + struct resource_entry *window; > + > + resource_list_for_each_entry(window, resources) > + if (window->res->flags & IORESOURCE_BUS) > + return; > + > + pr_info( > + "No busn resource found for pci%04x:%02x, will use [bus %02x-ff]\n", > + host->domain, bus, bus); > + host->busn_res.flags = IORESOURCE_BUS; > + host->busn_res.start = bus; > + host->busn_res.end = 255; > + pci_add_resource(resources, &host->busn_res); > +} > + > struct pci_host_bridge *pci_create_host_bridge( > struct device *parent, int domain, int bus, > struct list_head *resources) > @@ -33,6 +52,7 @@ struct pci_host_bridge *pci_create_host_bridge( > > host->dev.parent = parent; > INIT_LIST_HEAD(&host->windows); > + pci_host_update_busn_res(host, bus, resources); > resource_list_for_each_entry_safe(window, n, resources) > list_move_tail(&window->node, &host->windows); > /* > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 9bc4784..d5a12d9 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -2022,6 +2022,12 @@ int pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max) > return ret; > } > > +static void pci_host_update_busn_res_end( > + struct pci_host_bridge *host, int max) > +{ > + host->busn_res.end = max; > +} > + > void pci_bus_release_busn_res(struct pci_bus *b) > { > struct resource *res = &b->busn_res; > @@ -2040,32 +2046,20 @@ static struct pci_bus *__pci_scan_root_bus(int bus, > struct pci_host_bridge *host, struct pci_ops *ops, > void *sysdata) > { > - struct resource_entry *window; > - bool found = false; > struct pci_bus *b; > int max; > > - resource_list_for_each_entry(window, &host->windows) > - if (window->res->flags & IORESOURCE_BUS) { > - found = true; > - break; > - } > - > b = __pci_create_root_bus(bus, host, ops, sysdata); > if (!b) > return NULL; > > - if (!found) { > - dev_info(&b->dev, > - "No busn resource found for root bus, will use [bus %02x-ff]\n", > - bus); > - pci_bus_insert_busn_res(b, bus, 255); > - } > - > max = pci_scan_child_bus(b); > > - if (!found) > + /* If default busn resource used, update the max bus number */ > + if (host->busn_res.flags & IORESOURCE_BUS) { > + pci_host_update_busn_res_end(host, max); > pci_bus_update_busn_res_end(b, max); > + } > > return b; > } > diff --git a/include/linux/pci.h b/include/linux/pci.h > index 1542df8..f189dfb 100644 > --- a/include/linux/pci.h > +++ b/include/linux/pci.h > @@ -404,6 +404,8 @@ struct pci_host_bridge { > int domain; > struct device dev; > struct pci_bus *bus; /* root bus */ > + /* we use default bus resource if no bus resource provided */ > + struct resource busn_res; I don't understand the need for another busn_res here. The host bridge bus range should be identical to the root bus range. Having two copies will confuse things. And apparently this host->busn_res is only filled in if the arch doesn't provide a busn resource? To check for conflicts between host bridges, can you iterate through the existing ones and check the range of their root buses? > struct list_head windows; /* resource_entry */ > void (*release_fn)(struct pci_host_bridge *); > void *release_data; > -- > 1.7.1 >