| Message ID | 2355100.WsW1DXh57P@wuerfel (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, Oct 23, 2014 at 02:33:16PM +0100, Arnd Bergmann wrote: > On Thursday 23 October 2014 10:13:09 Liviu Dudau wrote: > > > @@ -335,7 +329,9 @@ void __init cns3xxx_pcie_init_late(void) > > > cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i)); > > > cns3xxx_pcie_check_link(&cns3xxx_pcie[i]); > > > cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]); > > > - pci_common_init(&cns3xxx_pcie[i].hw_pci); > > > + hw_pci->domain = i; > > > + private_data = &cns3xxx_pcie[i]; > > > > Is this dance with pointers absolutely necessary? Does gcc though dishes at you > > for doing hw_pci->private_data = &cns3xxx_pcie[i] directly? > > hw_pci->private_data is an array of pointers to private_data for each > host controller instance within the domain. There is only one entry > here, but you still the the correct type, so that would be > > hw_pci->private_data = (void **)&&cns3xxx_pcie[i]; > > which is even more confusing and ugly than what I wrote. If you have > a better idea, I'm all for it. Maybe it's clearer to write like this > (taken from rcar driver)? I was not questioning the implementation (and I do like it) I was just wondering if it is to work around some quirk of gcc. As I did not had the time to test it I was hoping that if there is a gcc requirement to phrase the code that way you might provide me with an answer. Best regards, Liviu > > void *hw_private[1]; > hw_pci.private_data = hw_private; > > for each host { > ... > hw_private[0] = &cns3xxx_pcie[i]; > pci_common_init_dev(&hw_pci); > } > > Note that all 'modern' controllers always use nr_controllers=1, so we > only need a single private_data pointer per domain, and the entire > hw_pci interface is a bit pointless. > > The platforms that currently require it are iop13xx, dove, mv78xx0 > and orion5x. We have plans to remove the last three platforms in > the next merge window or two, once all users are able to migrate to > mach-mvebu. Once that happens, we could probably move the entire > hw_pci logic that deals with multiple hosts per domain into the > iop13xx pci driver if we want to. A less intrusive simplification > would be to convert all 'multiplatform'-aware host controllers to > use pci_common_init_dev() and then take hw_pci out of that. > > See below for a sample patch I just did. It duplicates the code from > pci_common_init_dev/pci_common_init because we know that all users > of pci_common_init_dev are modern and only pass a single host bridge. > The new pci_common_init_dev is simpler than the old one but should > do the exact same thing for all current users, with the addition > of propagating the return value. > > pci_init_single() is the new internal helper and we should be able to > convert all existing users of pci_common_init_dev() to use that directly > and no longer define hw_pci at all. > > I've converted two drivers to give an example, but the conversions > should be done in follow-up patches really, and the pci_common_init_dev > function removed after all users are moved over. > > The new pci_init_single() is also rather simple, and it should just > converge with what we do for arm64 over time. > > Arnd > > --- > arch/arm/include/asm/mach/pci.h | 20 ++++--- > arch/arm/kernel/bios32.c | 103 ++++++++++++++++++++++++++++++++++-- > drivers/pci/host/pci-host-generic.c | 53 ++++++++----------- > drivers/pci/host/pci-mvebu.c | 44 +++++++-------- > 4 files changed, 157 insertions(+), 63 deletions(-) > > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > index 7fc42784becb..fe7e13759ec0 100644 > --- a/arch/arm/include/asm/mach/pci.h > +++ b/arch/arm/include/asm/mach/pci.h > @@ -73,16 +73,22 @@ struct pci_sys_data { > /* > * Call this with your hw_pci struct to initialise the PCI system. > */ > -void pci_common_init_dev(struct device *, struct hw_pci *); > +void pci_common_init(struct hw_pci *); > > /* > - * Compatibility wrapper for older platforms that do not care about > - * passing the parent device. > + * Used by modern platforms, only one host allowed. > */ > -static inline void pci_common_init(struct hw_pci *hw) > -{ > - pci_common_init_dev(NULL, hw); > -} > +int pci_common_init_dev(struct device *, struct hw_pci *); > + > +/* > + * Replaces pci_common_init_dev for drivers that want to do the > + * initialization simpler and avoid defining hw_pci > + */ > +int pci_init_single(struct device *parent, > + struct pci_sys_data *sys, > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *), > + struct pci_ops *ops); > + > > /* > * Setup early fixed I/O mapping. > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index 17a26c17f7f5..bccc8703e575 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -456,8 +456,7 @@ static int pcibios_init_resources(int busnr, struct pci_sys_data *sys) > return 0; > } > > -static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > - struct list_head *head) > +static void pcibios_init_hw(struct hw_pci *hw, struct list_head *head) > { > struct pci_sys_data *sys = NULL; > int ret; > @@ -494,7 +493,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > if (hw->scan) > sys->bus = hw->scan(nr, sys); > else > - sys->bus = pci_scan_root_bus(parent, sys->busnr, > + sys->bus = pci_scan_root_bus(NULL, sys->busnr, > hw->ops, sys, &sys->resources); > > if (!sys->bus) > @@ -511,7 +510,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > } > } > > -void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > +void pci_common_init(struct hw_pci *hw) > { > struct pci_sys_data *sys; > LIST_HEAD(head); > @@ -519,7 +518,7 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > pci_add_flags(PCI_REASSIGN_ALL_RSRC); > if (hw->preinit) > hw->preinit(); > - pcibios_init_hw(parent, hw, &head); > + pcibios_init_hw(hw, &head); > if (hw->postinit) > hw->postinit(); > > @@ -559,6 +558,100 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > } > } > > +int pci_init_single(struct device *parent, > + struct pci_sys_data *sys, > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *), > + struct pci_ops *ops) > +{ > + int ret; > + struct pci_bus *bus; > + > + ret = pcibios_init_resources(0, sys); > + if (ret) > + return ret; > + > + if (scan) > + bus = scan(0, sys); > + else > + bus = pci_scan_root_bus(parent, 0, ops, sys, &sys->resources); > + > + if (!bus) { > + dev_err(parent, "PCI: unable to scan bus!"); > + return -ENXIO; > + } > + sys->bus = bus; > + > + pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq); > + > + if (!pci_has_flag(PCI_PROBE_ONLY)) { > + /* > + * Size the bridge windows. > + */ > + pci_bus_size_bridges(bus); > + > + /* > + * Assign resources. > + */ > + pci_bus_assign_resources(bus); > + } > + > + /* > + * Tell drivers about devices found. > + */ > + pci_bus_add_devices(bus); > + > + /* Configure PCI Express settings */ > + if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { > + struct pci_bus *child; > + > + list_for_each_entry(child, &bus->children, node) > + pcie_bus_configure_settings(child); > + } > + > + return 0; > +} > + > +int pci_common_init_dev(struct device *parent, struct hw_pci *hw) > +{ > + struct pci_sys_data *sys; > + int ret; > + > + if (hw->nr_controllers != 1 || > + hw->preinit || hw->postinit) > + return -EINVAL; > + > + sys = kzalloc(sizeof(struct pci_sys_data), GFP_KERNEL); > + if (!sys) > + return -ENOMEM; > + > +#ifdef CONFIG_PCI_DOMAINS > + sys->domain = hw->domain; > +#endif > + sys->swizzle = hw->swizzle; > + sys->map_irq = hw->map_irq; > + sys->align_resource = hw->align_resource; > + sys->add_bus = hw->add_bus; > + sys->remove_bus = hw->remove_bus; > + INIT_LIST_HEAD(&sys->resources); > + > + if (hw->private_data) > + sys->private_data = hw->private_data[0]; > + > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + ret = hw->setup(0, sys); > + if (ret == 0) > + ret = -ENXIO; > + if (ret < 0) > + return ret; > + > + ret = pcibios_init_sysdata(parent, sys, hw->scan, hw->ops); > + if (ret) > + /* FIXME: undo ->setup */ > + kfree(sys); > + > + return ret; > +} > + > #ifndef CONFIG_PCI_HOST_ITE8152 > void pcibios_set_master(struct pci_dev *dev) > { > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index 3d2076f59911..3542a7b740e5 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -40,16 +40,20 @@ struct gen_pci_cfg_windows { > > struct gen_pci { > struct pci_host_bridge host; > + struct pci_sys_data sys; > struct gen_pci_cfg_windows cfg; > - struct list_head resources; > }; > > +static inline struct gen_pci *gen_pci_from_sys(struct pci_sys_data *sys) > +{ > + return container_of(sys, struct gen_pci, sys); > +} > + > static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > return pci->cfg.win[idx] + ((devfn << 8) | where); > @@ -64,8 +68,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > return pci->cfg.win[idx] + ((devfn << 12) | where); > @@ -80,8 +83,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 *val) > { > void __iomem *addr; > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > @@ -103,8 +105,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 val) > { > void __iomem *addr; > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > @@ -181,10 +182,10 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci) > { > struct pci_host_bridge_window *win; > > - list_for_each_entry(win, &pci->resources, list) > + list_for_each_entry(win, &pci->sys.resources, list) > release_resource(win->res); > > - pci_free_resource_list(&pci->resources); > + pci_free_resource_list(&pci->sys.resources); > } > > static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > @@ -237,7 +238,7 @@ static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > if (err) > goto out_release_res; > > - pci_add_resource_offset(&pci->resources, res, offset); > + pci_add_resource_offset(&pci->sys.resources, res, offset); > } > > if (!res_valid) { > @@ -306,17 +307,10 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > } > > /* Register bus resource */ > - pci_add_resource(&pci->resources, bus_range); > + pci_add_resource(&pci->sys.resources, bus_range); > return 0; > } > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > -{ > - struct gen_pci *pci = sys->private_data; > - list_splice_init(&pci->resources, &sys->resources); > - return 1; > -} > - > static int gen_pci_probe(struct platform_device *pdev) > { > int err; > @@ -326,17 +320,12 @@ static int gen_pci_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > - struct hw_pci hw = { > - .nr_controllers = 1, > - .private_data = (void **)&pci, > - .setup = gen_pci_setup, > - .map_irq = of_irq_parse_and_map_pci, > - .ops = &gen_pci_ops, > - }; > > if (!pci) > return -ENOMEM; > > + pci->sys.map_irq = of_irq_parse_and_map_pci, > + > type = of_get_property(np, "device_type", NULL); > if (!type || strcmp(type, "pci")) { > dev_err(dev, "invalid \"device_type\" %s\n", type); > @@ -355,7 +344,7 @@ static int gen_pci_probe(struct platform_device *pdev) > pci->cfg.ops = of_id->data; > pci->host.dev.parent = dev; > INIT_LIST_HEAD(&pci->host.windows); > - INIT_LIST_HEAD(&pci->resources); > + INIT_LIST_HEAD(&pci->sys.resources); > > /* Parse our PCI ranges and request their resources */ > err = gen_pci_parse_request_of_pci_ranges(pci); > @@ -369,8 +358,12 @@ static int gen_pci_probe(struct platform_device *pdev) > return err; > } > > - pci_common_init_dev(dev, &hw); > - return 0; > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + err = pci_init_single(dev, &pci->sys, NULL, &gen_pci_ops); > + if (err) > + gen_pci_release_of_pci_ranges(pci); > + > + return err; > } > > static struct platform_driver gen_pci_driver = { > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e197ffb..e1381c0699be 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -99,6 +99,7 @@ struct mvebu_pcie_port; > struct mvebu_pcie { > struct platform_device *pdev; > struct mvebu_pcie_port *ports; > + struct pci_sys_data sysdata; > struct msi_chip *msi; > struct resource io; > char io_name[30]; > @@ -611,7 +612,7 @@ static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port, > > static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys) > { > - return sys->private_data; > + return container_of(sys, struct mvebu_pcie, sysdata); > } > > static struct mvebu_pcie_port *mvebu_pcie_find_port(struct mvebu_pcie *pcie, > @@ -718,11 +719,26 @@ static struct pci_ops mvebu_pcie_ops = { > .write = mvebu_pcie_wr_conf, > }; > > -static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > +/* FIXME: move the code around to avoid these */ > +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys); > +static void mvebu_pcie_add_bus(struct pci_bus *bus); > +static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > + const struct resource *res, > + resource_size_t start, > + resource_size_t size, > + resource_size_t align); > + > +static int mvebu_pcie_enable(struct mvebu_pcie *pcie) > { > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > int domain = 0; > + struct pci_sys_data *sys = &pcie->sysdata; > + > + pcie->sysdata = (struct pci_sys_data) { > + .map_irq = of_irq_parse_and_map_pci, > + .align_resource = mvebu_pcie_align_resource, > + .add_bus = mvebu_pcie_add_bus, > + }; > > #ifdef CONFIG_PCI_DOMAINS > domain = sys->domain; > @@ -738,11 +754,13 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > if (request_resource(&iomem_resource, &pcie->mem)) > return 0; > > + INIT_LIST_HEAD(&sys->resources); > if (resource_size(&pcie->realio) != 0) { > if (request_resource(&ioport_resource, &pcie->realio)) { > release_resource(&pcie->mem); > return 0; > } > + > pci_add_resource_offset(&sys->resources, &pcie->realio, > sys->io_offset); > } > @@ -756,7 +774,9 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > mvebu_pcie_setup_hw(port); > } > > - return 1; > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + return pci_init_single(&pcie->pdev->dev, &pcie->sysdata, > + mvebu_pcie_scan_bus, &mvebu_pcie_ops); > } > > static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > @@ -810,24 +830,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > return start; > } > > -static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > -{ > - struct hw_pci hw; > - > - memset(&hw, 0, sizeof(hw)); > - > - hw.nr_controllers = 1; > - hw.private_data = (void **)&pcie; > - hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > - hw.map_irq = of_irq_parse_and_map_pci; > - hw.ops = &mvebu_pcie_ops; > - hw.align_resource = mvebu_pcie_align_resource; > - hw.add_bus = mvebu_pcie_add_bus; > - > - pci_common_init(&hw); > -} > - > /* > * Looks up the list of register addresses encoded into the reg = > * <...> property for one that matches the given port/lane. Once > @@ -1066,9 +1068,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) > pci_ioremap_io(i, pcie->io.start + i); > > mvebu_pcie_msi_enable(pcie); > - mvebu_pcie_enable(pcie); > - > - return 0; > + return mvebu_pcie_enable(pcie); > } > > static const struct of_device_id mvebu_pcie_of_match_table[] = { > > >
On Thu, Oct 23, 2014 at 03:33:16PM +0200, Arnd Bergmann wrote: > On Thursday 23 October 2014 10:13:09 Liviu Dudau wrote: > > > @@ -335,7 +329,9 @@ void __init cns3xxx_pcie_init_late(void) > > > cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i)); > > > cns3xxx_pcie_check_link(&cns3xxx_pcie[i]); > > > cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]); > > > - pci_common_init(&cns3xxx_pcie[i].hw_pci); > > > + hw_pci->domain = i; > > > + private_data = &cns3xxx_pcie[i]; > > > > Is this dance with pointers absolutely necessary? Does gcc though dishes at you > > for doing hw_pci->private_data = &cns3xxx_pcie[i] directly? > > hw_pci->private_data is an array of pointers to private_data for each > host controller instance within the domain. There is only one entry > here, but you still the the correct type, so that would be > > hw_pci->private_data = (void **)&&cns3xxx_pcie[i]; > > which is even more confusing and ugly than what I wrote. If you have > a better idea, I'm all for it. Maybe it's clearer to write like this > (taken from rcar driver)? > > void *hw_private[1]; > hw_pci.private_data = hw_private; > > for each host { > ... > hw_private[0] = &cns3xxx_pcie[i]; > pci_common_init_dev(&hw_pci); > } > > Note that all 'modern' controllers always use nr_controllers=1, so we > only need a single private_data pointer per domain, and the entire > hw_pci interface is a bit pointless. > > The platforms that currently require it are iop13xx, dove, mv78xx0 > and orion5x. We have plans to remove the last three platforms in > the next merge window or two, once all users are able to migrate to > mach-mvebu. Once that happens, we could probably move the entire > hw_pci logic that deals with multiple hosts per domain into the > iop13xx pci driver if we want to. A less intrusive simplification > would be to convert all 'multiplatform'-aware host controllers to > use pci_common_init_dev() and then take hw_pci out of that. > > See below for a sample patch I just did. It duplicates the code from > pci_common_init_dev/pci_common_init because we know that all users > of pci_common_init_dev are modern and only pass a single host bridge. > The new pci_common_init_dev is simpler than the old one but should > do the exact same thing for all current users, with the addition > of propagating the return value. Same with this one; I'm ignoring for now on the theory that this was just a sample to show the idea, and a more formal patch might come later. Bjorn > pci_init_single() is the new internal helper and we should be able to > convert all existing users of pci_common_init_dev() to use that directly > and no longer define hw_pci at all. > > I've converted two drivers to give an example, but the conversions > should be done in follow-up patches really, and the pci_common_init_dev > function removed after all users are moved over. > > The new pci_init_single() is also rather simple, and it should just > converge with what we do for arm64 over time. > > Arnd > > --- > arch/arm/include/asm/mach/pci.h | 20 ++++--- > arch/arm/kernel/bios32.c | 103 ++++++++++++++++++++++++++++++++++-- > drivers/pci/host/pci-host-generic.c | 53 ++++++++----------- > drivers/pci/host/pci-mvebu.c | 44 +++++++-------- > 4 files changed, 157 insertions(+), 63 deletions(-) > > diff --git a/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h > index 7fc42784becb..fe7e13759ec0 100644 > --- a/arch/arm/include/asm/mach/pci.h > +++ b/arch/arm/include/asm/mach/pci.h > @@ -73,16 +73,22 @@ struct pci_sys_data { > /* > * Call this with your hw_pci struct to initialise the PCI system. > */ > -void pci_common_init_dev(struct device *, struct hw_pci *); > +void pci_common_init(struct hw_pci *); > > /* > - * Compatibility wrapper for older platforms that do not care about > - * passing the parent device. > + * Used by modern platforms, only one host allowed. > */ > -static inline void pci_common_init(struct hw_pci *hw) > -{ > - pci_common_init_dev(NULL, hw); > -} > +int pci_common_init_dev(struct device *, struct hw_pci *); > + > +/* > + * Replaces pci_common_init_dev for drivers that want to do the > + * initialization simpler and avoid defining hw_pci > + */ > +int pci_init_single(struct device *parent, > + struct pci_sys_data *sys, > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *), > + struct pci_ops *ops); > + > > /* > * Setup early fixed I/O mapping. > diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c > index 17a26c17f7f5..bccc8703e575 100644 > --- a/arch/arm/kernel/bios32.c > +++ b/arch/arm/kernel/bios32.c > @@ -456,8 +456,7 @@ static int pcibios_init_resources(int busnr, struct pci_sys_data *sys) > return 0; > } > > -static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > - struct list_head *head) > +static void pcibios_init_hw(struct hw_pci *hw, struct list_head *head) > { > struct pci_sys_data *sys = NULL; > int ret; > @@ -494,7 +493,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > if (hw->scan) > sys->bus = hw->scan(nr, sys); > else > - sys->bus = pci_scan_root_bus(parent, sys->busnr, > + sys->bus = pci_scan_root_bus(NULL, sys->busnr, > hw->ops, sys, &sys->resources); > > if (!sys->bus) > @@ -511,7 +510,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, > } > } > > -void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > +void pci_common_init(struct hw_pci *hw) > { > struct pci_sys_data *sys; > LIST_HEAD(head); > @@ -519,7 +518,7 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > pci_add_flags(PCI_REASSIGN_ALL_RSRC); > if (hw->preinit) > hw->preinit(); > - pcibios_init_hw(parent, hw, &head); > + pcibios_init_hw(hw, &head); > if (hw->postinit) > hw->postinit(); > > @@ -559,6 +558,100 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw) > } > } > > +int pci_init_single(struct device *parent, > + struct pci_sys_data *sys, > + struct pci_bus *(*scan)(int nr, struct pci_sys_data *), > + struct pci_ops *ops) > +{ > + int ret; > + struct pci_bus *bus; > + > + ret = pcibios_init_resources(0, sys); > + if (ret) > + return ret; > + > + if (scan) > + bus = scan(0, sys); > + else > + bus = pci_scan_root_bus(parent, 0, ops, sys, &sys->resources); > + > + if (!bus) { > + dev_err(parent, "PCI: unable to scan bus!"); > + return -ENXIO; > + } > + sys->bus = bus; > + > + pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq); > + > + if (!pci_has_flag(PCI_PROBE_ONLY)) { > + /* > + * Size the bridge windows. > + */ > + pci_bus_size_bridges(bus); > + > + /* > + * Assign resources. > + */ > + pci_bus_assign_resources(bus); > + } > + > + /* > + * Tell drivers about devices found. > + */ > + pci_bus_add_devices(bus); > + > + /* Configure PCI Express settings */ > + if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { > + struct pci_bus *child; > + > + list_for_each_entry(child, &bus->children, node) > + pcie_bus_configure_settings(child); > + } > + > + return 0; > +} > + > +int pci_common_init_dev(struct device *parent, struct hw_pci *hw) > +{ > + struct pci_sys_data *sys; > + int ret; > + > + if (hw->nr_controllers != 1 || > + hw->preinit || hw->postinit) > + return -EINVAL; > + > + sys = kzalloc(sizeof(struct pci_sys_data), GFP_KERNEL); > + if (!sys) > + return -ENOMEM; > + > +#ifdef CONFIG_PCI_DOMAINS > + sys->domain = hw->domain; > +#endif > + sys->swizzle = hw->swizzle; > + sys->map_irq = hw->map_irq; > + sys->align_resource = hw->align_resource; > + sys->add_bus = hw->add_bus; > + sys->remove_bus = hw->remove_bus; > + INIT_LIST_HEAD(&sys->resources); > + > + if (hw->private_data) > + sys->private_data = hw->private_data[0]; > + > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + ret = hw->setup(0, sys); > + if (ret == 0) > + ret = -ENXIO; > + if (ret < 0) > + return ret; > + > + ret = pcibios_init_sysdata(parent, sys, hw->scan, hw->ops); > + if (ret) > + /* FIXME: undo ->setup */ > + kfree(sys); > + > + return ret; > +} > + > #ifndef CONFIG_PCI_HOST_ITE8152 > void pcibios_set_master(struct pci_dev *dev) > { > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index 3d2076f59911..3542a7b740e5 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -40,16 +40,20 @@ struct gen_pci_cfg_windows { > > struct gen_pci { > struct pci_host_bridge host; > + struct pci_sys_data sys; > struct gen_pci_cfg_windows cfg; > - struct list_head resources; > }; > > +static inline struct gen_pci *gen_pci_from_sys(struct pci_sys_data *sys) > +{ > + return container_of(sys, struct gen_pci, sys); > +} > + > static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > return pci->cfg.win[idx] + ((devfn << 8) | where); > @@ -64,8 +68,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > return pci->cfg.win[idx] + ((devfn << 12) | where); > @@ -80,8 +83,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 *val) > { > void __iomem *addr; > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > @@ -103,8 +105,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 val) > { > void __iomem *addr; > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > @@ -181,10 +182,10 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci) > { > struct pci_host_bridge_window *win; > > - list_for_each_entry(win, &pci->resources, list) > + list_for_each_entry(win, &pci->sys.resources, list) > release_resource(win->res); > > - pci_free_resource_list(&pci->resources); > + pci_free_resource_list(&pci->sys.resources); > } > > static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > @@ -237,7 +238,7 @@ static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > if (err) > goto out_release_res; > > - pci_add_resource_offset(&pci->resources, res, offset); > + pci_add_resource_offset(&pci->sys.resources, res, offset); > } > > if (!res_valid) { > @@ -306,17 +307,10 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > } > > /* Register bus resource */ > - pci_add_resource(&pci->resources, bus_range); > + pci_add_resource(&pci->sys.resources, bus_range); > return 0; > } > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > -{ > - struct gen_pci *pci = sys->private_data; > - list_splice_init(&pci->resources, &sys->resources); > - return 1; > -} > - > static int gen_pci_probe(struct platform_device *pdev) > { > int err; > @@ -326,17 +320,12 @@ static int gen_pci_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > - struct hw_pci hw = { > - .nr_controllers = 1, > - .private_data = (void **)&pci, > - .setup = gen_pci_setup, > - .map_irq = of_irq_parse_and_map_pci, > - .ops = &gen_pci_ops, > - }; > > if (!pci) > return -ENOMEM; > > + pci->sys.map_irq = of_irq_parse_and_map_pci, > + > type = of_get_property(np, "device_type", NULL); > if (!type || strcmp(type, "pci")) { > dev_err(dev, "invalid \"device_type\" %s\n", type); > @@ -355,7 +344,7 @@ static int gen_pci_probe(struct platform_device *pdev) > pci->cfg.ops = of_id->data; > pci->host.dev.parent = dev; > INIT_LIST_HEAD(&pci->host.windows); > - INIT_LIST_HEAD(&pci->resources); > + INIT_LIST_HEAD(&pci->sys.resources); > > /* Parse our PCI ranges and request their resources */ > err = gen_pci_parse_request_of_pci_ranges(pci); > @@ -369,8 +358,12 @@ static int gen_pci_probe(struct platform_device *pdev) > return err; > } > > - pci_common_init_dev(dev, &hw); > - return 0; > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + err = pci_init_single(dev, &pci->sys, NULL, &gen_pci_ops); > + if (err) > + gen_pci_release_of_pci_ranges(pci); > + > + return err; > } > > static struct platform_driver gen_pci_driver = { > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e197ffb..e1381c0699be 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -99,6 +99,7 @@ struct mvebu_pcie_port; > struct mvebu_pcie { > struct platform_device *pdev; > struct mvebu_pcie_port *ports; > + struct pci_sys_data sysdata; > struct msi_chip *msi; > struct resource io; > char io_name[30]; > @@ -611,7 +612,7 @@ static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port, > > static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys) > { > - return sys->private_data; > + return container_of(sys, struct mvebu_pcie, sysdata); > } > > static struct mvebu_pcie_port *mvebu_pcie_find_port(struct mvebu_pcie *pcie, > @@ -718,11 +719,26 @@ static struct pci_ops mvebu_pcie_ops = { > .write = mvebu_pcie_wr_conf, > }; > > -static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > +/* FIXME: move the code around to avoid these */ > +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys); > +static void mvebu_pcie_add_bus(struct pci_bus *bus); > +static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > + const struct resource *res, > + resource_size_t start, > + resource_size_t size, > + resource_size_t align); > + > +static int mvebu_pcie_enable(struct mvebu_pcie *pcie) > { > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > int domain = 0; > + struct pci_sys_data *sys = &pcie->sysdata; > + > + pcie->sysdata = (struct pci_sys_data) { > + .map_irq = of_irq_parse_and_map_pci, > + .align_resource = mvebu_pcie_align_resource, > + .add_bus = mvebu_pcie_add_bus, > + }; > > #ifdef CONFIG_PCI_DOMAINS > domain = sys->domain; > @@ -738,11 +754,13 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > if (request_resource(&iomem_resource, &pcie->mem)) > return 0; > > + INIT_LIST_HEAD(&sys->resources); > if (resource_size(&pcie->realio) != 0) { > if (request_resource(&ioport_resource, &pcie->realio)) { > release_resource(&pcie->mem); > return 0; > } > + > pci_add_resource_offset(&sys->resources, &pcie->realio, > sys->io_offset); > } > @@ -756,7 +774,9 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > mvebu_pcie_setup_hw(port); > } > > - return 1; > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + return pci_init_single(&pcie->pdev->dev, &pcie->sysdata, > + mvebu_pcie_scan_bus, &mvebu_pcie_ops); > } > > static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > @@ -810,24 +830,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > return start; > } > > -static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > -{ > - struct hw_pci hw; > - > - memset(&hw, 0, sizeof(hw)); > - > - hw.nr_controllers = 1; > - hw.private_data = (void **)&pcie; > - hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > - hw.map_irq = of_irq_parse_and_map_pci; > - hw.ops = &mvebu_pcie_ops; > - hw.align_resource = mvebu_pcie_align_resource; > - hw.add_bus = mvebu_pcie_add_bus; > - > - pci_common_init(&hw); > -} > - > /* > * Looks up the list of register addresses encoded into the reg = > * <...> property for one that matches the given port/lane. Once > @@ -1066,9 +1068,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) > pci_ioremap_io(i, pcie->io.start + i); > > mvebu_pcie_msi_enable(pcie); > - mvebu_pcie_enable(pcie); > - > - return 0; > + return mvebu_pcie_enable(pcie); > } > > static const struct of_device_id mvebu_pcie_of_match_table[] = { > -- 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 Wednesday 05 November 2014 16:40:58 Bjorn Helgaas wrote: > > > > See below for a sample patch I just did. It duplicates the code from > > pci_common_init_dev/pci_common_init because we know that all users > > of pci_common_init_dev are modern and only pass a single host bridge. > > The new pci_common_init_dev is simpler than the old one but should > > do the exact same thing for all current users, with the addition > > of propagating the return value. > > Same with this one; I'm ignoring for now on the theory that this was just a > sample to show the idea, and a more formal patch might come later. > Correct. Or we might not do it at all, if some of the other patches are done before we need this. 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
Hi, I am not sure if this thread is still alive. I'm trying to see what I can do to help clean up/convert to make the PCI GHC also works for arm64 w/ zero or minimal ifdefs. Please let me know if someone is already working on this. I noticed that Lorenzo's patches has already been in 3.19-rc1, and in Bjorn's pci/domain branch. Otherwise, I'll try to continue the work based on the sample patch from Arnd here. On 10/23/14 08:33, Arnd Bergmann wrote: > [...] > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index 3d2076f59911..3542a7b740e5 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -40,16 +40,20 @@ struct gen_pci_cfg_windows { > > struct gen_pci { > struct pci_host_bridge host; > + struct pci_sys_data sys; > struct gen_pci_cfg_windows cfg; > - struct list_head resources; > }; Arnd, based on the patch here, if we are trying to use the pci-host-generic driver on arm64, this means that we are going to have to introduce struct pci_sys_data for the arm64 as well (e.g move the struct from include/asm/mach/pci.h to include/linux/pci.h). Is this also your intention? Thanks, Suravee > > +static inline struct gen_pci *gen_pci_from_sys(struct pci_sys_data *sys) > +{ > + return container_of(sys, struct gen_pci, sys); > +} > + > static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > return pci->cfg.win[idx] + ((devfn << 8) | where); > @@ -64,8 +68,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, > unsigned int devfn, > int where) > { > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > return pci->cfg.win[idx] + ((devfn << 12) | where); > @@ -80,8 +83,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 *val) > { > void __iomem *addr; > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > @@ -103,8 +105,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn, > int where, int size, u32 val) > { > void __iomem *addr; > - struct pci_sys_data *sys = bus->sysdata; > - struct gen_pci *pci = sys->private_data; > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > @@ -181,10 +182,10 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci) > { > struct pci_host_bridge_window *win; > > - list_for_each_entry(win, &pci->resources, list) > + list_for_each_entry(win, &pci->sys.resources, list) > release_resource(win->res); > > - pci_free_resource_list(&pci->resources); > + pci_free_resource_list(&pci->sys.resources); > } > > static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > @@ -237,7 +238,7 @@ static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > if (err) > goto out_release_res; > > - pci_add_resource_offset(&pci->resources, res, offset); > + pci_add_resource_offset(&pci->sys.resources, res, offset); > } > > if (!res_valid) { > @@ -306,17 +307,10 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > } > > /* Register bus resource */ > - pci_add_resource(&pci->resources, bus_range); > + pci_add_resource(&pci->sys.resources, bus_range); > return 0; > } > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > -{ > - struct gen_pci *pci = sys->private_data; > - list_splice_init(&pci->resources, &sys->resources); > - return 1; > -} > - > static int gen_pci_probe(struct platform_device *pdev) > { > int err; > @@ -326,17 +320,12 @@ static int gen_pci_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct device_node *np = dev->of_node; > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > - struct hw_pci hw = { > - .nr_controllers = 1, > - .private_data = (void **)&pci, > - .setup = gen_pci_setup, > - .map_irq = of_irq_parse_and_map_pci, > - .ops = &gen_pci_ops, > - }; > > if (!pci) > return -ENOMEM; > > + pci->sys.map_irq = of_irq_parse_and_map_pci, > + > type = of_get_property(np, "device_type", NULL); > if (!type || strcmp(type, "pci")) { > dev_err(dev, "invalid \"device_type\" %s\n", type); > @@ -355,7 +344,7 @@ static int gen_pci_probe(struct platform_device *pdev) > pci->cfg.ops = of_id->data; > pci->host.dev.parent = dev; > INIT_LIST_HEAD(&pci->host.windows); > - INIT_LIST_HEAD(&pci->resources); > + INIT_LIST_HEAD(&pci->sys.resources); > > /* Parse our PCI ranges and request their resources */ > err = gen_pci_parse_request_of_pci_ranges(pci); > @@ -369,8 +358,12 @@ static int gen_pci_probe(struct platform_device *pdev) > return err; > } > > - pci_common_init_dev(dev, &hw); > - return 0; > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + err = pci_init_single(dev, &pci->sys, NULL, &gen_pci_ops); > + if (err) > + gen_pci_release_of_pci_ranges(pci); > + > + return err; > } > > static struct platform_driver gen_pci_driver = { > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > index b1315e197ffb..e1381c0699be 100644 > --- a/drivers/pci/host/pci-mvebu.c > +++ b/drivers/pci/host/pci-mvebu.c > @@ -99,6 +99,7 @@ struct mvebu_pcie_port; > struct mvebu_pcie { > struct platform_device *pdev; > struct mvebu_pcie_port *ports; > + struct pci_sys_data sysdata; > struct msi_chip *msi; > struct resource io; > char io_name[30]; > @@ -611,7 +612,7 @@ static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port, > > static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys) > { > - return sys->private_data; > + return container_of(sys, struct mvebu_pcie, sysdata); > } > > static struct mvebu_pcie_port *mvebu_pcie_find_port(struct mvebu_pcie *pcie, > @@ -718,11 +719,26 @@ static struct pci_ops mvebu_pcie_ops = { > .write = mvebu_pcie_wr_conf, > }; > > -static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > +/* FIXME: move the code around to avoid these */ > +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys); > +static void mvebu_pcie_add_bus(struct pci_bus *bus); > +static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > + const struct resource *res, > + resource_size_t start, > + resource_size_t size, > + resource_size_t align); > + > +static int mvebu_pcie_enable(struct mvebu_pcie *pcie) > { > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > int i; > int domain = 0; > + struct pci_sys_data *sys = &pcie->sysdata; > + > + pcie->sysdata = (struct pci_sys_data) { > + .map_irq = of_irq_parse_and_map_pci, > + .align_resource = mvebu_pcie_align_resource, > + .add_bus = mvebu_pcie_add_bus, > + }; > > #ifdef CONFIG_PCI_DOMAINS > domain = sys->domain; > @@ -738,11 +754,13 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > if (request_resource(&iomem_resource, &pcie->mem)) > return 0; > > + INIT_LIST_HEAD(&sys->resources); > if (resource_size(&pcie->realio) != 0) { > if (request_resource(&ioport_resource, &pcie->realio)) { > release_resource(&pcie->mem); > return 0; > } > + > pci_add_resource_offset(&sys->resources, &pcie->realio, > sys->io_offset); > } > @@ -756,7 +774,9 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > mvebu_pcie_setup_hw(port); > } > > - return 1; > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > + return pci_init_single(&pcie->pdev->dev, &pcie->sysdata, > + mvebu_pcie_scan_bus, &mvebu_pcie_ops); > } > > static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > @@ -810,24 +830,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > return start; > } > > -static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > -{ > - struct hw_pci hw; > - > - memset(&hw, 0, sizeof(hw)); > - > - hw.nr_controllers = 1; > - hw.private_data = (void **)&pcie; > - hw.setup = mvebu_pcie_setup; > - hw.scan = mvebu_pcie_scan_bus; > - hw.map_irq = of_irq_parse_and_map_pci; > - hw.ops = &mvebu_pcie_ops; > - hw.align_resource = mvebu_pcie_align_resource; > - hw.add_bus = mvebu_pcie_add_bus; > - > - pci_common_init(&hw); > -} > - > /* > * Looks up the list of register addresses encoded into the reg = > * <...> property for one that matches the given port/lane. Once > @@ -1066,9 +1068,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) > pci_ioremap_io(i, pcie->io.start + i); > > mvebu_pcie_msi_enable(pcie); > - mvebu_pcie_enable(pcie); > - > - return 0; > + return mvebu_pcie_enable(pcie); > } > > static const struct of_device_id mvebu_pcie_of_match_table[] = { > -- 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
Hi Suravee, On Mon, Dec 29, 2014 at 07:32:44PM +0000, Suravee Suthikulpanit wrote: > Hi, > > I am not sure if this thread is still alive. I'm trying to see what I > can do to help clean up/convert to make the PCI GHC also works for arm64 > w/ zero or minimal ifdefs. > > Please let me know if someone is already working on this. I noticed that > Lorenzo's patches has already been in 3.19-rc1, and in Bjorn's > pci/domain branch. Otherwise, I'll try to continue the work based on the > sample patch from Arnd here. If I am not mistaken, the only bit missing to remove pci_sys_data (and so having a generic host controller driver that works on ARM32/64) is generic MSI management. I know for certain Marc is working on it, and the solution is WIP, I think we should prevent adding more churn to pci_sys_data, since I managed to remove most of the dependencies (domain, mem_offset). So to sum it up, to have a generic host controller driver for ARM32/64 we just need to work out how to handle the MSI data, patches will be on the lists shortly to handle that, please review. Thanks, Lorenzo > On 10/23/14 08:33, Arnd Bergmann wrote: > > [...] > > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > > index 3d2076f59911..3542a7b740e5 100644 > > --- a/drivers/pci/host/pci-host-generic.c > > +++ b/drivers/pci/host/pci-host-generic.c > > @@ -40,16 +40,20 @@ struct gen_pci_cfg_windows { > > > > struct gen_pci { > > struct pci_host_bridge host; > > + struct pci_sys_data sys; > > struct gen_pci_cfg_windows cfg; > > - struct list_head resources; > > }; > > Arnd, based on the patch here, if we are trying to use the > pci-host-generic driver on arm64, this means that we are going to have > to introduce struct pci_sys_data for the arm64 as well (e.g move the > struct from include/asm/mach/pci.h to include/linux/pci.h). Is this also > your intention? > > Thanks, > > Suravee > > > > > +static inline struct gen_pci *gen_pci_from_sys(struct pci_sys_data *sys) > > +{ > > + return container_of(sys, struct gen_pci, sys); > > +} > > + > > static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, > > unsigned int devfn, > > int where) > > { > > - struct pci_sys_data *sys = bus->sysdata; > > - struct gen_pci *pci = sys->private_data; > > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > > > return pci->cfg.win[idx] + ((devfn << 8) | where); > > @@ -64,8 +68,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, > > unsigned int devfn, > > int where) > > { > > - struct pci_sys_data *sys = bus->sysdata; > > - struct gen_pci *pci = sys->private_data; > > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > resource_size_t idx = bus->number - pci->cfg.bus_range.start; > > > > return pci->cfg.win[idx] + ((devfn << 12) | where); > > @@ -80,8 +83,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn, > > int where, int size, u32 *val) > > { > > void __iomem *addr; > > - struct pci_sys_data *sys = bus->sysdata; > > - struct gen_pci *pci = sys->private_data; > > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > > > @@ -103,8 +105,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn, > > int where, int size, u32 val) > > { > > void __iomem *addr; > > - struct pci_sys_data *sys = bus->sysdata; > > - struct gen_pci *pci = sys->private_data; > > + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); > > > > addr = pci->cfg.ops->map_bus(bus, devfn, where); > > > > @@ -181,10 +182,10 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci) > > { > > struct pci_host_bridge_window *win; > > > > - list_for_each_entry(win, &pci->resources, list) > > + list_for_each_entry(win, &pci->sys.resources, list) > > release_resource(win->res); > > > > - pci_free_resource_list(&pci->resources); > > + pci_free_resource_list(&pci->sys.resources); > > } > > > > static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > > @@ -237,7 +238,7 @@ static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) > > if (err) > > goto out_release_res; > > > > - pci_add_resource_offset(&pci->resources, res, offset); > > + pci_add_resource_offset(&pci->sys.resources, res, offset); > > } > > > > if (!res_valid) { > > @@ -306,17 +307,10 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) > > } > > > > /* Register bus resource */ > > - pci_add_resource(&pci->resources, bus_range); > > + pci_add_resource(&pci->sys.resources, bus_range); > > return 0; > > } > > > > -static int gen_pci_setup(int nr, struct pci_sys_data *sys) > > -{ > > - struct gen_pci *pci = sys->private_data; > > - list_splice_init(&pci->resources, &sys->resources); > > - return 1; > > -} > > - > > static int gen_pci_probe(struct platform_device *pdev) > > { > > int err; > > @@ -326,17 +320,12 @@ static int gen_pci_probe(struct platform_device *pdev) > > struct device *dev = &pdev->dev; > > struct device_node *np = dev->of_node; > > struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); > > - struct hw_pci hw = { > > - .nr_controllers = 1, > > - .private_data = (void **)&pci, > > - .setup = gen_pci_setup, > > - .map_irq = of_irq_parse_and_map_pci, > > - .ops = &gen_pci_ops, > > - }; > > > > if (!pci) > > return -ENOMEM; > > > > + pci->sys.map_irq = of_irq_parse_and_map_pci, > > + > > type = of_get_property(np, "device_type", NULL); > > if (!type || strcmp(type, "pci")) { > > dev_err(dev, "invalid \"device_type\" %s\n", type); > > @@ -355,7 +344,7 @@ static int gen_pci_probe(struct platform_device *pdev) > > pci->cfg.ops = of_id->data; > > pci->host.dev.parent = dev; > > INIT_LIST_HEAD(&pci->host.windows); > > - INIT_LIST_HEAD(&pci->resources); > > + INIT_LIST_HEAD(&pci->sys.resources); > > > > /* Parse our PCI ranges and request their resources */ > > err = gen_pci_parse_request_of_pci_ranges(pci); > > @@ -369,8 +358,12 @@ static int gen_pci_probe(struct platform_device *pdev) > > return err; > > } > > > > - pci_common_init_dev(dev, &hw); > > - return 0; > > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > > + err = pci_init_single(dev, &pci->sys, NULL, &gen_pci_ops); > > + if (err) > > + gen_pci_release_of_pci_ranges(pci); > > + > > + return err; > > } > > > > static struct platform_driver gen_pci_driver = { > > diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c > > index b1315e197ffb..e1381c0699be 100644 > > --- a/drivers/pci/host/pci-mvebu.c > > +++ b/drivers/pci/host/pci-mvebu.c > > @@ -99,6 +99,7 @@ struct mvebu_pcie_port; > > struct mvebu_pcie { > > struct platform_device *pdev; > > struct mvebu_pcie_port *ports; > > + struct pci_sys_data sysdata; > > struct msi_chip *msi; > > struct resource io; > > char io_name[30]; > > @@ -611,7 +612,7 @@ static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port, > > > > static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys) > > { > > - return sys->private_data; > > + return container_of(sys, struct mvebu_pcie, sysdata); > > } > > > > static struct mvebu_pcie_port *mvebu_pcie_find_port(struct mvebu_pcie *pcie, > > @@ -718,11 +719,26 @@ static struct pci_ops mvebu_pcie_ops = { > > .write = mvebu_pcie_wr_conf, > > }; > > > > -static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > > +/* FIXME: move the code around to avoid these */ > > +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys); > > +static void mvebu_pcie_add_bus(struct pci_bus *bus); > > +static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > > + const struct resource *res, > > + resource_size_t start, > > + resource_size_t size, > > + resource_size_t align); > > + > > +static int mvebu_pcie_enable(struct mvebu_pcie *pcie) > > { > > - struct mvebu_pcie *pcie = sys_to_pcie(sys); > > int i; > > int domain = 0; > > + struct pci_sys_data *sys = &pcie->sysdata; > > + > > + pcie->sysdata = (struct pci_sys_data) { > > + .map_irq = of_irq_parse_and_map_pci, > > + .align_resource = mvebu_pcie_align_resource, > > + .add_bus = mvebu_pcie_add_bus, > > + }; > > > > #ifdef CONFIG_PCI_DOMAINS > > domain = sys->domain; > > @@ -738,11 +754,13 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > > if (request_resource(&iomem_resource, &pcie->mem)) > > return 0; > > > > + INIT_LIST_HEAD(&sys->resources); > > if (resource_size(&pcie->realio) != 0) { > > if (request_resource(&ioport_resource, &pcie->realio)) { > > release_resource(&pcie->mem); > > return 0; > > } > > + > > pci_add_resource_offset(&sys->resources, &pcie->realio, > > sys->io_offset); > > } > > @@ -756,7 +774,9 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) > > mvebu_pcie_setup_hw(port); > > } > > > > - return 1; > > + pci_add_flags(PCI_REASSIGN_ALL_RSRC); > > + return pci_init_single(&pcie->pdev->dev, &pcie->sysdata, > > + mvebu_pcie_scan_bus, &mvebu_pcie_ops); > > } > > > > static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) > > @@ -810,24 +830,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, > > return start; > > } > > > > -static void mvebu_pcie_enable(struct mvebu_pcie *pcie) > > -{ > > - struct hw_pci hw; > > - > > - memset(&hw, 0, sizeof(hw)); > > - > > - hw.nr_controllers = 1; > > - hw.private_data = (void **)&pcie; > > - hw.setup = mvebu_pcie_setup; > > - hw.scan = mvebu_pcie_scan_bus; > > - hw.map_irq = of_irq_parse_and_map_pci; > > - hw.ops = &mvebu_pcie_ops; > > - hw.align_resource = mvebu_pcie_align_resource; > > - hw.add_bus = mvebu_pcie_add_bus; > > - > > - pci_common_init(&hw); > > -} > > - > > /* > > * Looks up the list of register addresses encoded into the reg = > > * <...> property for one that matches the given port/lane. Once > > @@ -1066,9 +1068,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) > > pci_ioremap_io(i, pcie->io.start + i); > > > > mvebu_pcie_msi_enable(pcie); > > - mvebu_pcie_enable(pcie); > > - > > - return 0; > > + return mvebu_pcie_enable(pcie); > > } > > > > static const struct of_device_id mvebu_pcie_of_match_table[] = { > > > -- 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 1/2/2015 5:55 AM, Lorenzo Pieralisi wrote: > Hi Suravee, > > On Mon, Dec 29, 2014 at 07:32:44PM +0000, Suravee Suthikulpanit wrote: >> >Hi, >> > >> >I am not sure if this thread is still alive. I'm trying to see what I >> >can do to help clean up/convert to make the PCI GHC also works for arm64 >> >w/ zero or minimal ifdefs. >> > >> >Please let me know if someone is already working on this. I noticed that >> >Lorenzo's patches has already been in 3.19-rc1, and in Bjorn's >> >pci/domain branch. Otherwise, I'll try to continue the work based on the >> >sample patch from Arnd here. > If I am not mistaken, the only bit missing to remove pci_sys_data (and so > having a generic host controller driver that works on ARM32/64) is generic > MSI management. Lorenzo, Do you mean to remove pci_sys_data from pci-host-generic.c or removing it completely? I assume the former case. So, looking at the current code in the pci-host-generic.c, my understanding is that the: *gen_pci = pci_bus->sysdata->private_data will be changed to: *gen_pci = pci_bus->sysdata Then, we can simply just call pci_scan_root_bus() directly since we no longer need to declare hw_pci for calling pci_common_init_dev(). > I know for certain Marc is working on it, and the solution is WIP, > I think we should prevent adding more churn to pci_sys_data, since > I managed to remove most of the dependencies (domain, mem_offset). Thanks for cleaning up the domain and mem_offset. I saw Marc's irq/msi_domain patch series (http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/msi_domain). My understanding is that deals with associating the newly introduced msi_domain to each device, which replaces the need for pci_bus->msi and hw_pci->msi_ctrl when configure with CONFIG_PCI_MSI_IRQ_DOMAIN (not sure if this would be the plan for all arm32). For ARM32, if not define CONFIG_PCI_MSI_IRQ_DOMAIN, it would still fall back to using the [pci_sys_data|hw_pci]->msi_ctrl. However, I noticed that the hw_pci->msi_controller is not even used for the pci-host-generic. So, this should not be blocking the work to free pci-host-generic from pci_sys_data and hw_pci, as the MSI stuff can go in separately. Am I missing something? > So to sum it up, to have a generic host controller driver for ARM32/64 > we just need to work out how to handle the MSI data, patches will be > on the lists shortly to handle that, please review. > > Thanks, > Lorenzo > Thanks for the update and the summary. I'll help review/test the MSI patch once posted. Thanks, Suravee -- 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 Friday 02 January 2015 12:18:06 Suravee Suthikulanit wrote: > On 1/2/2015 5:55 AM, Lorenzo Pieralisi wrote: > > Hi Suravee, > > > > On Mon, Dec 29, 2014 at 07:32:44PM +0000, Suravee Suthikulpanit wrote: > >> >Hi, > >> > > >> >I am not sure if this thread is still alive. I'm trying to see what I > >> >can do to help clean up/convert to make the PCI GHC also works for arm64 > >> >w/ zero or minimal ifdefs. > >> > > >> >Please let me know if someone is already working on this. I noticed that > >> >Lorenzo's patches has already been in 3.19-rc1, and in Bjorn's > >> >pci/domain branch. Otherwise, I'll try to continue the work based on the > >> >sample patch from Arnd here. > > If I am not mistaken, the only bit missing to remove pci_sys_data (and so > > having a generic host controller driver that works on ARM32/64) is generic > > MSI management. > > Lorenzo, > > Do you mean to remove pci_sys_data from pci-host-generic.c or removing > it completely? I assume the former case. Something inbetween: We should be able to remove pci_sys_data and pci_common_init_dev from all drivers in drivers/pci/host/, but keep them for all drivers in arch/arm/*/pci.c > So, looking at the current code in the pci-host-generic.c, my > understanding is that the: > *gen_pci = pci_bus->sysdata->private_data > will be changed to: > *gen_pci = pci_bus->sysdata > > Then, we can simply just call pci_scan_root_bus() directly since we no > longer need to declare hw_pci for calling pci_common_init_dev(). Right. > > I know for certain Marc is working on it, and the solution is WIP, > > I think we should prevent adding more churn to pci_sys_data, since > > I managed to remove most of the dependencies (domain, mem_offset). > > Thanks for cleaning up the domain and mem_offset. > > I saw Marc's irq/msi_domain patch series > (http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/msi_domain). > > > My understanding is that deals with associating the newly introduced > msi_domain to each device, which replaces the need for pci_bus->msi and > hw_pci->msi_ctrl when configure with CONFIG_PCI_MSI_IRQ_DOMAIN (not sure > if this would be the plan for all arm32). For ARM32, if not define > CONFIG_PCI_MSI_IRQ_DOMAIN, it would still fall back to using the > [pci_sys_data|hw_pci]->msi_ctrl. For all I can tell, we have two cases on ARM regarding MSI: - arch/arm/mach-iop13xx/pci.c uses its own arch_setup_msi_irq/arch_teardown_msi_irq implementation and does not use pci_bus->msi. - everything else that supports MSI has a modern driver with multiplatform support and uses msi_controller. If any platform wants to support GICv2m, we have to use CONFIG_PCI_MSI_IRQ_DOMAIN for all of them, and that seems like the best way forward. 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 Fri, Jan 02, 2015 at 09:09:33PM +0000, Arnd Bergmann wrote: > On Friday 02 January 2015 12:18:06 Suravee Suthikulanit wrote: > > On 1/2/2015 5:55 AM, Lorenzo Pieralisi wrote: > > > Hi Suravee, > > > > > > On Mon, Dec 29, 2014 at 07:32:44PM +0000, Suravee Suthikulpanit wrote: > > >> >Hi, > > >> > > > >> >I am not sure if this thread is still alive. I'm trying to see what I > > >> >can do to help clean up/convert to make the PCI GHC also works for arm64 > > >> >w/ zero or minimal ifdefs. > > >> > > > >> >Please let me know if someone is already working on this. I noticed that > > >> >Lorenzo's patches has already been in 3.19-rc1, and in Bjorn's > > >> >pci/domain branch. Otherwise, I'll try to continue the work based on the > > >> >sample patch from Arnd here. > > > If I am not mistaken, the only bit missing to remove pci_sys_data (and so > > > having a generic host controller driver that works on ARM32/64) is generic > > > MSI management. > > > > Lorenzo, > > > > Do you mean to remove pci_sys_data from pci-host-generic.c or removing > > it completely? I assume the former case. > > Something inbetween: We should be able to remove pci_sys_data and > pci_common_init_dev from all drivers in drivers/pci/host/, but keep them > for all drivers in arch/arm/*/pci.c > > > So, looking at the current code in the pci-host-generic.c, my > > understanding is that the: > > *gen_pci = pci_bus->sysdata->private_data > > will be changed to: > > *gen_pci = pci_bus->sysdata > > > > Then, we can simply just call pci_scan_root_bus() directly since we no > > longer need to declare hw_pci for calling pci_common_init_dev(). > > Right. > > > > I know for certain Marc is working on it, and the solution is WIP, > > > I think we should prevent adding more churn to pci_sys_data, since > > > I managed to remove most of the dependencies (domain, mem_offset). > > > > Thanks for cleaning up the domain and mem_offset. > > > > I saw Marc's irq/msi_domain patch series > > (http://git.kernel.org/cgit/linux/kernel/git/maz/arm-platforms.git/log/?h=irq/msi_domain). > > > > > > My understanding is that deals with associating the newly introduced > > msi_domain to each device, which replaces the need for pci_bus->msi and > > hw_pci->msi_ctrl when configure with CONFIG_PCI_MSI_IRQ_DOMAIN (not sure > > if this would be the plan for all arm32). For ARM32, if not define > > CONFIG_PCI_MSI_IRQ_DOMAIN, it would still fall back to using the > > [pci_sys_data|hw_pci]->msi_ctrl. > > For all I can tell, we have two cases on ARM regarding MSI: > > - arch/arm/mach-iop13xx/pci.c uses its own > arch_setup_msi_irq/arch_teardown_msi_irq implementation and does not > use pci_bus->msi. > > - everything else that supports MSI has a modern driver with multiplatform > support and uses msi_controller. If any platform wants to support GICv2m, > we have to use CONFIG_PCI_MSI_IRQ_DOMAIN for all of them, and that > seems like the best way forward. Yes, I think that's the current situation. With a hook (added by Marc's code) in PCI core to set the msi domain (DT), all we need to do is convert the generic host controller code to pci_scan_root_bus() and remove the pcibios init calls, it is a mechanical change when all patches mentioned above are merged. Thanks, Lorenzo -- 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/arch/arm/include/asm/mach/pci.h b/arch/arm/include/asm/mach/pci.h index 7fc42784becb..fe7e13759ec0 100644 --- a/arch/arm/include/asm/mach/pci.h +++ b/arch/arm/include/asm/mach/pci.h @@ -73,16 +73,22 @@ struct pci_sys_data { /* * Call this with your hw_pci struct to initialise the PCI system. */ -void pci_common_init_dev(struct device *, struct hw_pci *); +void pci_common_init(struct hw_pci *); /* - * Compatibility wrapper for older platforms that do not care about - * passing the parent device. + * Used by modern platforms, only one host allowed. */ -static inline void pci_common_init(struct hw_pci *hw) -{ - pci_common_init_dev(NULL, hw); -} +int pci_common_init_dev(struct device *, struct hw_pci *); + +/* + * Replaces pci_common_init_dev for drivers that want to do the + * initialization simpler and avoid defining hw_pci + */ +int pci_init_single(struct device *parent, + struct pci_sys_data *sys, + struct pci_bus *(*scan)(int nr, struct pci_sys_data *), + struct pci_ops *ops); + /* * Setup early fixed I/O mapping. diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c index 17a26c17f7f5..bccc8703e575 100644 --- a/arch/arm/kernel/bios32.c +++ b/arch/arm/kernel/bios32.c @@ -456,8 +456,7 @@ static int pcibios_init_resources(int busnr, struct pci_sys_data *sys) return 0; } -static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, - struct list_head *head) +static void pcibios_init_hw(struct hw_pci *hw, struct list_head *head) { struct pci_sys_data *sys = NULL; int ret; @@ -494,7 +493,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, if (hw->scan) sys->bus = hw->scan(nr, sys); else - sys->bus = pci_scan_root_bus(parent, sys->busnr, + sys->bus = pci_scan_root_bus(NULL, sys->busnr, hw->ops, sys, &sys->resources); if (!sys->bus) @@ -511,7 +510,7 @@ static void pcibios_init_hw(struct device *parent, struct hw_pci *hw, } } -void pci_common_init_dev(struct device *parent, struct hw_pci *hw) +void pci_common_init(struct hw_pci *hw) { struct pci_sys_data *sys; LIST_HEAD(head); @@ -519,7 +518,7 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw) pci_add_flags(PCI_REASSIGN_ALL_RSRC); if (hw->preinit) hw->preinit(); - pcibios_init_hw(parent, hw, &head); + pcibios_init_hw(hw, &head); if (hw->postinit) hw->postinit(); @@ -559,6 +558,100 @@ void pci_common_init_dev(struct device *parent, struct hw_pci *hw) } } +int pci_init_single(struct device *parent, + struct pci_sys_data *sys, + struct pci_bus *(*scan)(int nr, struct pci_sys_data *), + struct pci_ops *ops) +{ + int ret; + struct pci_bus *bus; + + ret = pcibios_init_resources(0, sys); + if (ret) + return ret; + + if (scan) + bus = scan(0, sys); + else + bus = pci_scan_root_bus(parent, 0, ops, sys, &sys->resources); + + if (!bus) { + dev_err(parent, "PCI: unable to scan bus!"); + return -ENXIO; + } + sys->bus = bus; + + pci_fixup_irqs(pcibios_swizzle, pcibios_map_irq); + + if (!pci_has_flag(PCI_PROBE_ONLY)) { + /* + * Size the bridge windows. + */ + pci_bus_size_bridges(bus); + + /* + * Assign resources. + */ + pci_bus_assign_resources(bus); + } + + /* + * Tell drivers about devices found. + */ + pci_bus_add_devices(bus); + + /* Configure PCI Express settings */ + if (bus && !pci_has_flag(PCI_PROBE_ONLY)) { + struct pci_bus *child; + + list_for_each_entry(child, &bus->children, node) + pcie_bus_configure_settings(child); + } + + return 0; +} + +int pci_common_init_dev(struct device *parent, struct hw_pci *hw) +{ + struct pci_sys_data *sys; + int ret; + + if (hw->nr_controllers != 1 || + hw->preinit || hw->postinit) + return -EINVAL; + + sys = kzalloc(sizeof(struct pci_sys_data), GFP_KERNEL); + if (!sys) + return -ENOMEM; + +#ifdef CONFIG_PCI_DOMAINS + sys->domain = hw->domain; +#endif + sys->swizzle = hw->swizzle; + sys->map_irq = hw->map_irq; + sys->align_resource = hw->align_resource; + sys->add_bus = hw->add_bus; + sys->remove_bus = hw->remove_bus; + INIT_LIST_HEAD(&sys->resources); + + if (hw->private_data) + sys->private_data = hw->private_data[0]; + + pci_add_flags(PCI_REASSIGN_ALL_RSRC); + ret = hw->setup(0, sys); + if (ret == 0) + ret = -ENXIO; + if (ret < 0) + return ret; + + ret = pcibios_init_sysdata(parent, sys, hw->scan, hw->ops); + if (ret) + /* FIXME: undo ->setup */ + kfree(sys); + + return ret; +} + #ifndef CONFIG_PCI_HOST_ITE8152 void pcibios_set_master(struct pci_dev *dev) { diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c index 3d2076f59911..3542a7b740e5 100644 --- a/drivers/pci/host/pci-host-generic.c +++ b/drivers/pci/host/pci-host-generic.c @@ -40,16 +40,20 @@ struct gen_pci_cfg_windows { struct gen_pci { struct pci_host_bridge host; + struct pci_sys_data sys; struct gen_pci_cfg_windows cfg; - struct list_head resources; }; +static inline struct gen_pci *gen_pci_from_sys(struct pci_sys_data *sys) +{ + return container_of(sys, struct gen_pci, sys); +} + static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus, unsigned int devfn, int where) { - struct pci_sys_data *sys = bus->sysdata; - struct gen_pci *pci = sys->private_data; + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); resource_size_t idx = bus->number - pci->cfg.bus_range.start; return pci->cfg.win[idx] + ((devfn << 8) | where); @@ -64,8 +68,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus, unsigned int devfn, int where) { - struct pci_sys_data *sys = bus->sysdata; - struct gen_pci *pci = sys->private_data; + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); resource_size_t idx = bus->number - pci->cfg.bus_range.start; return pci->cfg.win[idx] + ((devfn << 12) | where); @@ -80,8 +83,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val) { void __iomem *addr; - struct pci_sys_data *sys = bus->sysdata; - struct gen_pci *pci = sys->private_data; + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); addr = pci->cfg.ops->map_bus(bus, devfn, where); @@ -103,8 +105,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val) { void __iomem *addr; - struct pci_sys_data *sys = bus->sysdata; - struct gen_pci *pci = sys->private_data; + struct gen_pci *pci = gen_pci_from_sys(bus->sysdata); addr = pci->cfg.ops->map_bus(bus, devfn, where); @@ -181,10 +182,10 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci) { struct pci_host_bridge_window *win; - list_for_each_entry(win, &pci->resources, list) + list_for_each_entry(win, &pci->sys.resources, list) release_resource(win->res); - pci_free_resource_list(&pci->resources); + pci_free_resource_list(&pci->sys.resources); } static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) @@ -237,7 +238,7 @@ static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci) if (err) goto out_release_res; - pci_add_resource_offset(&pci->resources, res, offset); + pci_add_resource_offset(&pci->sys.resources, res, offset); } if (!res_valid) { @@ -306,17 +307,10 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci) } /* Register bus resource */ - pci_add_resource(&pci->resources, bus_range); + pci_add_resource(&pci->sys.resources, bus_range); return 0; } -static int gen_pci_setup(int nr, struct pci_sys_data *sys) -{ - struct gen_pci *pci = sys->private_data; - list_splice_init(&pci->resources, &sys->resources); - return 1; -} - static int gen_pci_probe(struct platform_device *pdev) { int err; @@ -326,17 +320,12 @@ static int gen_pci_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct device_node *np = dev->of_node; struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL); - struct hw_pci hw = { - .nr_controllers = 1, - .private_data = (void **)&pci, - .setup = gen_pci_setup, - .map_irq = of_irq_parse_and_map_pci, - .ops = &gen_pci_ops, - }; if (!pci) return -ENOMEM; + pci->sys.map_irq = of_irq_parse_and_map_pci, + type = of_get_property(np, "device_type", NULL); if (!type || strcmp(type, "pci")) { dev_err(dev, "invalid \"device_type\" %s\n", type); @@ -355,7 +344,7 @@ static int gen_pci_probe(struct platform_device *pdev) pci->cfg.ops = of_id->data; pci->host.dev.parent = dev; INIT_LIST_HEAD(&pci->host.windows); - INIT_LIST_HEAD(&pci->resources); + INIT_LIST_HEAD(&pci->sys.resources); /* Parse our PCI ranges and request their resources */ err = gen_pci_parse_request_of_pci_ranges(pci); @@ -369,8 +358,12 @@ static int gen_pci_probe(struct platform_device *pdev) return err; } - pci_common_init_dev(dev, &hw); - return 0; + pci_add_flags(PCI_REASSIGN_ALL_RSRC); + err = pci_init_single(dev, &pci->sys, NULL, &gen_pci_ops); + if (err) + gen_pci_release_of_pci_ranges(pci); + + return err; } static struct platform_driver gen_pci_driver = { diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c index b1315e197ffb..e1381c0699be 100644 --- a/drivers/pci/host/pci-mvebu.c +++ b/drivers/pci/host/pci-mvebu.c @@ -99,6 +99,7 @@ struct mvebu_pcie_port; struct mvebu_pcie { struct platform_device *pdev; struct mvebu_pcie_port *ports; + struct pci_sys_data sysdata; struct msi_chip *msi; struct resource io; char io_name[30]; @@ -611,7 +612,7 @@ static int mvebu_sw_pci_bridge_write(struct mvebu_pcie_port *port, static inline struct mvebu_pcie *sys_to_pcie(struct pci_sys_data *sys) { - return sys->private_data; + return container_of(sys, struct mvebu_pcie, sysdata); } static struct mvebu_pcie_port *mvebu_pcie_find_port(struct mvebu_pcie *pcie, @@ -718,11 +719,26 @@ static struct pci_ops mvebu_pcie_ops = { .write = mvebu_pcie_wr_conf, }; -static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) +/* FIXME: move the code around to avoid these */ +static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys); +static void mvebu_pcie_add_bus(struct pci_bus *bus); +static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, + const struct resource *res, + resource_size_t start, + resource_size_t size, + resource_size_t align); + +static int mvebu_pcie_enable(struct mvebu_pcie *pcie) { - struct mvebu_pcie *pcie = sys_to_pcie(sys); int i; int domain = 0; + struct pci_sys_data *sys = &pcie->sysdata; + + pcie->sysdata = (struct pci_sys_data) { + .map_irq = of_irq_parse_and_map_pci, + .align_resource = mvebu_pcie_align_resource, + .add_bus = mvebu_pcie_add_bus, + }; #ifdef CONFIG_PCI_DOMAINS domain = sys->domain; @@ -738,11 +754,13 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) if (request_resource(&iomem_resource, &pcie->mem)) return 0; + INIT_LIST_HEAD(&sys->resources); if (resource_size(&pcie->realio) != 0) { if (request_resource(&ioport_resource, &pcie->realio)) { release_resource(&pcie->mem); return 0; } + pci_add_resource_offset(&sys->resources, &pcie->realio, sys->io_offset); } @@ -756,7 +774,9 @@ static int mvebu_pcie_setup(int nr, struct pci_sys_data *sys) mvebu_pcie_setup_hw(port); } - return 1; + pci_add_flags(PCI_REASSIGN_ALL_RSRC); + return pci_init_single(&pcie->pdev->dev, &pcie->sysdata, + mvebu_pcie_scan_bus, &mvebu_pcie_ops); } static struct pci_bus *mvebu_pcie_scan_bus(int nr, struct pci_sys_data *sys) @@ -810,24 +830,6 @@ static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev, return start; } -static void mvebu_pcie_enable(struct mvebu_pcie *pcie) -{ - struct hw_pci hw; - - memset(&hw, 0, sizeof(hw)); - - hw.nr_controllers = 1; - hw.private_data = (void **)&pcie; - hw.setup = mvebu_pcie_setup; - hw.scan = mvebu_pcie_scan_bus; - hw.map_irq = of_irq_parse_and_map_pci; - hw.ops = &mvebu_pcie_ops; - hw.align_resource = mvebu_pcie_align_resource; - hw.add_bus = mvebu_pcie_add_bus; - - pci_common_init(&hw); -} - /* * Looks up the list of register addresses encoded into the reg = * <...> property for one that matches the given port/lane. Once @@ -1066,9 +1068,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) pci_ioremap_io(i, pcie->io.start + i); mvebu_pcie_msi_enable(pcie); - mvebu_pcie_enable(pcie); - - return 0; + return mvebu_pcie_enable(pcie); } static const struct of_device_id mvebu_pcie_of_match_table[] = {