| Message ID | 1421372666-12288-11-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Friday 16 January 2015 09:44:08 Yijing Wang wrote: > @@ -2066,11 +2064,11 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db, > { > struct pci_host_bridge *host; > > - host = pci_create_host_bridge(parent, db, resources); > + host = pci_create_host_bridge(parent, db, resources, sysdata); > if (!host) > return NULL; > > - return __pci_scan_root_bus(host, ops, sysdata); > + return __pci_scan_root_bus(host, ops); > } > EXPORT_SYMBOL(pci_scan_root_bus); > How about keeping the sysdata out of the pci_create_host_bridge interface, and refactoring it so that the call sequence becomes host = pci_create_host_bridge(parent, db, resources); host->sysdata = sysdata; __pci_scan_root_bus(host, ops); This way, we can make sysdata completely option. I assume that more of the fields we have in sysdata today can get moved into pci_host_bridge over time, so a host bridge driver can just assign those members individually between pci_create_host_bridge and __pci_scan_root_bus. Arnd -- 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
On 2015/1/16 17:18, Arnd Bergmann wrote: > On Friday 16 January 2015 09:44:08 Yijing Wang wrote: >> @@ -2066,11 +2064,11 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db, >> { >> struct pci_host_bridge *host; >> >> - host = pci_create_host_bridge(parent, db, resources); >> + host = pci_create_host_bridge(parent, db, resources, sysdata); >> if (!host) >> return NULL; >> >> - return __pci_scan_root_bus(host, ops, sysdata); >> + return __pci_scan_root_bus(host, ops); >> } >> EXPORT_SYMBOL(pci_scan_root_bus); >> > > How about keeping the sysdata out of the pci_create_host_bridge interface, and > refactoring it so that the call sequence becomes > > host = pci_create_host_bridge(parent, db, resources); > host->sysdata = sysdata; > __pci_scan_root_bus(host, ops); > > This way, we can make sysdata completely option. I assume that more of the > fields we have in sysdata today can get moved into pci_host_bridge > over time, so a host bridge driver can just assign those members individually > between pci_create_host_bridge and __pci_scan_root_bus. I put the sysdata in pci_host_bridge, because some platforms x86/ia64 need to call pcibios_root_bridge_prepare() before pci_host_bridge registration in pci_create_host_bridge(), and pcibios_root_bridge_prepare() need the companion acpi device pointer which is stored in sysdata. I like you idea, but I haven't find a better way yet. Thanks! Yijing. > > Arnd > > . >
On Monday 19 January 2015 11:12:50 Yijing Wang wrote: > On 2015/1/16 17:18, Arnd Bergmann wrote: > > On Friday 16 January 2015 09:44:08 Yijing Wang wrote: > >> @@ -2066,11 +2064,11 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db, > >> { > >> struct pci_host_bridge *host; > >> > >> - host = pci_create_host_bridge(parent, db, resources); > >> + host = pci_create_host_bridge(parent, db, resources, sysdata); > >> if (!host) > >> return NULL; > >> > >> - return __pci_scan_root_bus(host, ops, sysdata); > >> + return __pci_scan_root_bus(host, ops); > >> } > >> EXPORT_SYMBOL(pci_scan_root_bus); > >> > > > > How about keeping the sysdata out of the pci_create_host_bridge interface, and > > refactoring it so that the call sequence becomes > > > > host = pci_create_host_bridge(parent, db, resources); > > host->sysdata = sysdata; > > __pci_scan_root_bus(host, ops); > > > > This way, we can make sysdata completely option. I assume that more of the > > fields we have in sysdata today can get moved into pci_host_bridge > > over time, so a host bridge driver can just assign those members individually > > between pci_create_host_bridge and __pci_scan_root_bus. > > I put the sysdata in pci_host_bridge, because some platforms x86/ia64 > need to call pcibios_root_bridge_prepare() before pci_host_bridge registration > in pci_create_host_bridge(), and pcibios_root_bridge_prepare() need the companion > acpi device pointer which is stored in sysdata. > > I like you idea, but I haven't find a better way yet. Ok, fair enough. We can always try to find ways for getting rid of pcibios_root_bridge_prepare() later, though I suspect as soon as we have the callback, it will get used for other things as well ;-) Arnd -- 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
diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c index c9ee582..0b6ba5c 100644 --- a/drivers/pci/host-bridge.c +++ b/drivers/pci/host-bridge.c @@ -23,7 +23,8 @@ static void pci_release_host_bridge_dev(struct device *dev) } struct pci_host_bridge *pci_create_host_bridge( - struct device *parent, u32 db, struct list_head *resources) + struct device *parent, u32 db, + struct list_head *resources, void *sysdata) { int error; int bus = PCI_BUSNUM(db); @@ -58,6 +59,7 @@ struct pci_host_bridge *pci_create_host_bridge( host->dev.parent = parent; INIT_LIST_HEAD(&host->windows); host->dev.release = pci_release_host_bridge_dev; + dev_set_drvdata(&host->dev, sysdata); dev_set_name(&host->dev, "pci%04x:%02x", host->domain, host->busnum); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 2e0b952..98a8d97 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -1865,8 +1865,7 @@ void __weak pcibios_remove_bus(struct pci_bus *bus) } static struct pci_bus *__pci_create_root_bus( - struct pci_host_bridge *bridge, struct pci_ops *ops, - void *sysdata) + struct pci_host_bridge *bridge, struct pci_ops *ops) { int error; struct pci_bus *b; @@ -1882,7 +1881,7 @@ static struct pci_bus *__pci_create_root_bus( if (!b) return NULL; - b->sysdata = sysdata; + b->sysdata = dev_get_drvdata(&bridge->dev); b->ops = ops; b->number = b->busn_res.start = bridge->busnum; pci_bus_assign_domain_nr(b, parent); @@ -1954,11 +1953,11 @@ struct pci_bus *pci_create_root_bus(struct device *parent, u32 db, { struct pci_host_bridge *host; - host = pci_create_host_bridge(parent, db, resources); + host = pci_create_host_bridge(parent, db, resources, sysdata); if (!host) return NULL; - return __pci_create_root_bus(host, ops, sysdata); + return __pci_create_root_bus(host, ops); } int pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max) @@ -2025,8 +2024,7 @@ void pci_bus_release_busn_res(struct pci_bus *b) } static struct pci_bus *__pci_scan_root_bus( - struct pci_host_bridge *host, struct pci_ops *ops, - void *sysdata) + struct pci_host_bridge *host, struct pci_ops *ops) { struct pci_host_bridge_window *window; @@ -2040,7 +2038,7 @@ static struct pci_bus *__pci_scan_root_bus( break; } - b = __pci_create_root_bus(host, ops, sysdata); + b = __pci_create_root_bus(host, ops); if (!b) { pci_free_host_bridge(host); return NULL; @@ -2066,11 +2064,11 @@ struct pci_bus *pci_scan_root_bus(struct device *parent, u32 db, { struct pci_host_bridge *host; - host = pci_create_host_bridge(parent, db, resources); + host = pci_create_host_bridge(parent, db, resources, sysdata); if (!host) return NULL; - return __pci_scan_root_bus(host, ops, sysdata); + return __pci_scan_root_bus(host, ops); } EXPORT_SYMBOL(pci_scan_root_bus); diff --git a/include/linux/pci.h b/include/linux/pci.h index 5ee0033..3ee8436 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -419,7 +419,8 @@ 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 dombus, struct list_head *resources); + struct device *parent, u32 dombus, + struct list_head *resources, void *sysdata); /* * 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
Save platform specific sysdata in pci_host_bridge drvdata, host bridge specific operation need to access it before the pci bus creation. Signed-off-by: Yijing Wang <wangyijing@huawei.com> --- drivers/pci/host-bridge.c | 4 +++- drivers/pci/probe.c | 18 ++++++++---------- include/linux/pci.h | 3 ++- 3 files changed, 13 insertions(+), 12 deletions(-)