From: Yijing Wang <wangyijing@huawei.com>
To: Liviu Dudau <Liviu.Dudau@arm.com>
Cc: Liviu Dudau <liviu@dudau.co.uk>, Tony Luck <tony.luck@intel.com>,
Russell King <linux@arm.linux.org.uk>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"huxinwei@huawei.com" <huxinwei@huawei.com>,
Thierry Reding <thierry.reding@gmail.com>,
Yijing Wang <wangyijing0307@gmail.com>,
"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
Bjorn Helgaas <bhelgaas@google.com>,
"linux-ia64@vger.kernel.org" <linux-ia64@vger.kernel.org>,
Thomas Gleixner <tglx@linutronix.de>, Wuyun <wuyun.wu@huawei.com>,
"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [RFC PATCH 08/16] PCI: Introduce pci_scan_host_bridge() and pci_host_info
Date: Wed, 19 Nov 2014 10:09:12 +0800 [thread overview]
Message-ID: <546BFBC8.4030104@huawei.com> (raw)
In-Reply-To: <20141118154243.GM12037@e106497-lin.cambridge.arm.com>
>> struct pci_host_bridge *pci_create_host_bridge(
>> - struct device *parent, u32 db,
>> - struct pci_ops *ops, void *sysdata,
>> - struct list_head *resources)
>> + struct device *parent, u32 db, struct pci_ops *ops,
>> + struct pci_host_info *info)
>> {
>> int error;
>> struct pci_bus *b;
>> struct pci_host_bridge *host, *h;
>> - struct pci_host_bridge_window *window, *n;
>>
>> down_read(&pci_host_bridge_sem);
>> list_for_each_entry(h, &pci_host_bridge_list, list) {
>> @@ -53,7 +51,7 @@ struct pci_host_bridge *pci_create_host_bridge(
>> if (!host)
>> return NULL;
>>
>> - host->sysdata = sysdata;
>> + host->sysdata = info->arg;
>> host->busnum = PCI_BUSNUM(db);
>> host->domain = PCI_DOMAIN(db);
>> host->ops = ops;
>> @@ -63,18 +61,23 @@ struct pci_host_bridge *pci_create_host_bridge(
>>
>> /* this is hack, just for build, will be removed later*/
>
> Why do you need this hack? Just for calling pci_domain_nr() ?
Yes, it's temporary code, we need domain number here for pci host bridge register.
>
>> b = kzalloc(sizeof(*b), GFP_KERNEL);
>> - b->sysdata = sysdata;
>> + b->sysdata = host->sysdata;
>> pci_bus_assign_domain_nr(b, parent);
>> host->domain = pci_domain_nr(b);
>> + kfree(b);
>>
...
>> +static int pci_default_init_res(struct pci_host_bridge *host,
>> + struct pci_host_info *info)
>> +{
>> + struct pci_host_bridge_window *window, *n;
>> +
>> + if (info->res_type != PCI_HOST_RES_DEFAULT)
>> + list_for_each_entry_safe(window, n, info->resources,
>> + list)
>> + list_move_tail(&window->list, &host->windows);
>> + else
>> + info->res_type = PCI_HOST_RES_DEFAULT;
>
> I'm confused about this assignment. Isn't this a nop as the else part
> means info->res_type *is* PCI_HOST_RES_DEFAULT?
No, in this patch, host drivers pass a pci host bridge resources init hook
in pci_host_info *info, and we call this info->init_res() in pci_create_host_bridge().
+struct pci_host_info {
+ u8 res_type;
+ void *arg;
+ struct list_head *resources; /*just for build, will clean up later */
+ int (*init_res)(struct pci_host_bridge *host,
+ struct pci_host_info *info);
+};
+
>
>> +
>> + return 0;
>> +}
...
>> @@ -2038,8 +2057,13 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db,
>> bool found = false;
>> struct pci_host_bridge *host;
>> int max;
>> + struct pci_host_info info;
>> +
>> + info.arg = sysdata;
>> + info.resources = resources;
>> + info.init_res = pci_default_init_res;
>
> I have mixed feelings about this patch. While it is heading in the right direction
> of moving pci_host_bridge relevant information towards the right user, I don't think
> you picked up the right set to move. The resource list is going to be copied into
> internal pci_host_bridge list anyway, keeping another copy is not helpful *and*
> you have increased the code size.
>
> I think for now we should aim to get the *missing* data into pci_host_bridge: MSI
> controllers and PCI domain/segment. Then we can do more cleanup.
Hi Liviu, I agree with you here, the changes to resources stuff seems not a perfect
solution. In my patch 6, we could pass pci domain nr by u32 PCI_DOMBUS(domain, bus) argument,
and store it in pci_host_bridge. For msi controller, we couldn't save the msi_controller
in pci_host_bridge. Before we assume one pci host bridge only had one msi_controller,
but now something changes, Jiang introduce hierarchy irq domain in x86, and now
one pci host bridge may has more than one msi_controller. So I prefer to add a
function to pci_host_bridge something like
struct msi_controller *pci_get_msi_controller(struct pci_dev *dev)
>
>>
>> - host = pci_create_host_bridge(parent, db, ops, sysdata, resources);
>> + host = pci_create_host_bridge(parent, db, ops, &info);
>> if (!host)
>> return NULL;
>>
>> @@ -2070,6 +2094,47 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db,
>> }
>> EXPORT_SYMBOL(pci_scan_root_bus);
>>
>> +struct pci_host_bridge *pci_scan_host_bridge(
>> + struct device *parent, u32 db, struct pci_ops *ops,
>> + struct pci_host_info *info)
>> +{
>> + struct pci_host_bridge_window *window;
>> + bool found = false;
>> + struct pci_host_bridge *host;
>> + int max;
>> +
>> + host = pci_create_host_bridge(parent, db, ops, info);
>> + if (!host)
>> + return NULL;
>> +
>> + list_for_each_entry(window, &host->windows, list)
>> + if (window->res->flags & IORESOURCE_BUS) {
>> + found = true;
>> + break;
>> + }
>> +
>> + host->bus = __pci_create_root_bus(host);
>> + if (!host->bus) {
>> + pci_free_host_bridge(host);
>> + return NULL;
>> + }
>> +
>> + if (!found) {
>> + dev_info(&host->bus->dev,
>> + "No busn resource found for root bus, will use [bus %02x-ff]\n",
>> + host->busnum);
>> + pci_bus_insert_busn_res(host->bus, host->busnum, 255);
>> + }
>> +
>> + max = pci_scan_child_bus(host->bus);
>> + if (!found)
>> + pci_bus_update_busn_res_end(host->bus, max);
>> +
>> + return host;
>> +
>> +}
>> +EXPORT_SYMBOL(pci_scan_host_bridge);
>> +
>> /**
>> * pci_rescan_bus_bridge_resize - scan a PCI bus for devices.
>> * @bridge: PCI bridge for the bus to scan
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index daa7f40..a51f5f5 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -412,6 +412,21 @@ struct pci_host_bridge {
>> void *release_data;
>> };
>>
>> +struct pci_host_info {
>> + u8 res_type;
>> + void *arg;
>> + struct list_head *resources; /*just for build, will clean up later */
>> + int (*init_res)(struct pci_host_bridge *host,
>> + struct pci_host_info *info);
>> +};
>> +
>> +static inline void init_pci_host_info(struct pci_host_info *info)
>> +{
>> + memset(info, 0 , sizeof(*info));
>> +}
>
> Where is this used?
Host driver uses it to init pci_host_info.
>
>> +
>> +#define PCI_HOST_RES_DEFAULT 0x2
>> +
>
> Magic number?
Hmmm, I will rework pci host bridge resources stuff.
>
> Best regards,
> Liviu
>
>> #define to_pci_host_bridge(n) container_of(n, struct pci_host_bridge, dev)
>> void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>> void (*release_fn)(struct pci_host_bridge *),
>> @@ -420,7 +435,7 @@ void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
>> int pcibios_root_bridge_prepare(struct pci_host_bridge *bridge);
>> struct pci_host_bridge *pci_create_host_bridge(
>> struct device *parent, u32 db, struct pci_ops *ops,
>> - void *sys, struct list_head *resources);
>> + struct pci_host_info *info);
>> /*
>> * The first PCI_BRIDGE_RESOURCE_NUM PCI bus resources (those that correspond
>> * to P2P or CardBus bridge windows) go in a table. Additional ones (for
>> @@ -785,6 +800,9 @@ void pci_bus_release_busn_res(struct pci_bus *b);
>> struct pci_bus *pci_scan_root_bus(struct device *parent, u32 bus,
>> struct pci_ops *ops, void *sysdata,
>> struct list_head *resources);
>> +struct pci_host_bridge *pci_scan_host_bridge(struct device *parent,
>> + u32 db, struct pci_ops *ops,
>> + struct pci_host_info *info);
>> struct pci_bus *pci_add_new_bus(struct pci_bus *parent, struct pci_dev *dev,
>> int busnr);
>> void pcie_update_link_speed(struct pci_bus *bus, u16 link_status);
>> --
>> 1.7.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
--
Thanks!
Yijing
next prev parent reply other threads:[~2014-11-19 2:10 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-17 10:21 [RFC PATCH 00/16] Refine PCI host bridge scan interfaces Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 01/16] PCI: Enhance pci_scan_root_bus() to support default IO/MEM resources Yijing Wang
2014-11-17 10:08 ` Arnd Bergmann
2014-11-18 7:44 ` Yijing Wang
2014-11-18 9:36 ` Arnd Bergmann
2014-11-18 11:46 ` Yijing Wang
2014-11-18 14:23 ` Liviu Dudau
2014-11-19 1:15 ` Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 02/16] PCI: Use pci_scan_root_bus() instead of pci_scan_bus() Yijing Wang
2014-11-18 14:28 ` Liviu Dudau
2014-11-19 1:19 ` Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 03/16] PCI: Clean up pci_scan_bus() Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 04/16] PCI: Rip out pci_bus_add_devices() from pci_scan_root_bus() Yijing Wang
2014-11-18 14:34 ` Liviu Dudau
2014-11-19 1:21 ` Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 05/16] PCI: Use pci_scan_root_bus() instead of pci_scan_bus_parented() Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 06/16] PCI: Use u32 type to combine PCI domain and bus number Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 07/16] PCI: Separate pci_host_bridge creation out of pci_create_root_bus() Yijing Wang
2014-11-17 10:56 ` Arnd Bergmann
2014-11-18 8:32 ` Yijing Wang
2014-11-18 9:30 ` Arnd Bergmann
2014-11-18 11:44 ` Yijing Wang
2014-11-18 12:25 ` Arnd Bergmann
2014-11-18 12:41 ` Yijing Wang
2014-11-18 14:48 ` Liviu Dudau
2014-11-19 2:24 ` Yijing Wang
2014-11-19 16:29 ` Liviu Dudau
2014-11-20 2:00 ` Yijing Wang
2014-11-18 15:30 ` Liviu Dudau
2014-11-19 1:42 ` Yijing Wang
2014-11-19 16:37 ` Liviu Dudau
2014-11-20 2:47 ` Yijing Wang
2014-11-20 9:47 ` Liviu Dudau
2014-11-21 2:53 ` Yijing Wang
2014-11-21 9:53 ` Liviu Dudau
2014-11-17 10:21 ` [RFC PATCH 08/16] PCI: Introduce pci_scan_host_bridge() and pci_host_info Yijing Wang
2014-11-18 15:42 ` Liviu Dudau
2014-11-19 2:09 ` Yijing Wang [this message]
2014-11-19 16:41 ` Liviu Dudau
2014-11-20 2:54 ` Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 09/16] PCI: Associate .get_msi_ctrl() with pci_host_bridge Yijing Wang
2014-11-17 15:03 ` Lorenzo Pieralisi
2014-11-17 10:21 ` [RFC PATCH 10/16] PCI: Add of_scan_bus() to pci_host_info Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 11/16] x86/PCI: Use pci_scan_host_bridge() instead of pci_create_root_bus() Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 12/16] ia64/PCI: Remove the redundant bus variable Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 13/16] ia64/PCI: Use pci_scan_host_bridge() to refactor pci_acpi_scan_root() Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 14/16] arm/PCI: Introduce pci_get_domain_nr() Yijing Wang
2014-11-17 12:08 ` Lorenzo Pieralisi
2014-11-18 0:55 ` Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 15/16] arm/PCI: Use pci_scan_host_bridge() instead of pci_scan_root_bus() Yijing Wang
2014-11-17 10:21 ` [RFC PATCH 16/16] powerpc/PCI: Use pci_scan_host_bridge() to scan PCI bus Yijing Wang
2014-11-17 14:13 ` [RFC PATCH 00/16] Refine PCI host bridge scan interfaces Arnd Bergmann
2014-11-18 11:17 ` Yijing Wang
2014-11-18 11:30 ` Arnd Bergmann
2014-11-18 11:45 ` Lorenzo Pieralisi
2014-11-18 12:14 ` Yijing Wang
2014-11-18 12:17 ` Yijing Wang
2014-11-18 12:27 ` Arnd Bergmann
2014-11-20 12:01 ` Tomasz Nowicki
2014-11-20 13:15 ` Arnd Bergmann
2014-11-20 11:54 ` Tomasz Nowicki
2014-11-20 12:08 ` Liviu Dudau
2014-11-20 12:53 ` Tomasz Nowicki
2014-11-20 16:39 ` Liviu Dudau
2014-11-21 2:58 ` Yijing Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=546BFBC8.4030104@huawei.com \
--to=wangyijing@huawei.com \
--cc=Liviu.Dudau@arm.com \
--cc=bhelgaas@google.com \
--cc=huxinwei@huawei.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux@arm.linux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=liviu@dudau.co.uk \
--cc=suravee.suthikulpanit@amd.com \
--cc=tglx@linutronix.de \
--cc=thierry.reding@gmail.com \
--cc=tony.luck@intel.com \
--cc=wangyijing0307@gmail.com \
--cc=wuyun.wu@huawei.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).