| Message ID | 1452620173-4905-6-git-send-email-bharatku@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Ping > This patch does required modifications to microblaze PCI subsystem, to > work with generic driver (drivers/pci/host/pcie-xilinx.c) on Microblaze > and Zynq. > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> > --- > Changes: > Modified pcibios_fixup_bus in pci-common.c, as per generic architecuture. > Modified pcibios_align_resource in pci-common.c, as per generic > architecuture. > Modified pcibios_get_phb_of_node function in pci-common.c, to remove > dependency on struct pci_controller. > Removed pci_domain_nr in pci-common.c, instead using generic code. > Added pcibios_add_device in pci-common.c, as per generic architecuture. > Adding Kernel configuration in arch/microblaze as required for generic PCI > domains. > Added kernel configuration for driver to support Microblaze. > --- > arch/microblaze/Kconfig | 3 ++ > arch/microblaze/pci/pci-common.c | 61 +++++++++++++++-------------------- > ----- > drivers/pci/host/Kconfig | 2 +- > 3 files changed, 27 insertions(+), 39 deletions(-) > > diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig > index 0bce820..c3702b9 100644 > --- a/arch/microblaze/Kconfig > +++ b/arch/microblaze/Kconfig > @@ -271,6 +271,9 @@ config PCI > config PCI_DOMAINS > def_bool PCI > > +config PCI_DOMAINS_GENERIC > + def_bool PCI_DOMAINS > + > config PCI_SYSCALL > def_bool PCI > > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci- > common.c > index ae838ed..bc72856 100644 > --- a/arch/microblaze/pci/pci-common.c > +++ b/arch/microblaze/pci/pci-common.c > @@ -123,17 +123,6 @@ unsigned long pci_address_to_pio(phys_addr_t > address) > } > EXPORT_SYMBOL_GPL(pci_address_to_pio); > > -/* > - * Return the domain number for this bus. > - */ > -int pci_domain_nr(struct pci_bus *bus) > -{ > - struct pci_controller *hose = pci_bus_to_host(bus); > - > - return hose->global_number; > -} > -EXPORT_SYMBOL(pci_domain_nr); > - > /* This routine is meant to be used early during boot, when the > * PCI bus numbers have not yet been assigned, and you need to > * issue PCI config cycles to an OF device. > @@ -863,26 +852,10 @@ void pcibios_setup_bus_devices(struct pci_bus > *bus) > > void pcibios_fixup_bus(struct pci_bus *bus) > { > - /* When called from the generic PCI probe, read PCI<->PCI bridge > - * bases. This is -not- called when generating the PCI tree from > - * the OF device-tree. > - */ > - if (bus->self != NULL) > - pci_read_bridge_bases(bus); > - > - /* Now fixup the bus bus */ > - pcibios_setup_bus_self(bus); > - > - /* Now fixup devices on that bus */ > - pcibios_setup_bus_devices(bus); > + /* nothing to do */ > } > EXPORT_SYMBOL(pcibios_fixup_bus); > > -static int skip_isa_ioresource_align(struct pci_dev *dev) > -{ > - return 0; > -} > - > /* > * We need to avoid collisions with `mirrored' VGA ports > * and other strange ISA hardware, so we always want the > @@ -899,20 +872,20 @@ static int skip_isa_ioresource_align(struct pci_dev > *dev) > resource_size_t pcibios_align_resource(void *data, const struct resource > *res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > resource_size_t start = res->start; > > - if (res->flags & IORESOURCE_IO) { > - if (skip_isa_ioresource_align(dev)) > - return start; > - if (start & 0x300) > - start = (start + 0x3ff) & ~0x3ff; > - } > - > return start; > } > EXPORT_SYMBOL(pcibios_align_resource); > > +int pcibios_add_device(struct pci_dev *dev) > +{ > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > + > + return 0; > +} > +EXPORT_SYMBOL(pcibios_add_device); > + > /* > * Reparent resource children of pr that conflict with res > * under res, and make res replace those children. > @@ -1335,9 +1308,21 @@ static void pcibios_setup_phb_resources(struct > pci_controller *hose, > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) > { > - struct pci_controller *hose = bus->sysdata; > + struct device_node *np; > + > + for_each_node_by_type(np, "pci") { > + const void *prop; > + unsigned int bus_min; > + > + prop = of_get_property(np, "bus-range", NULL); > + if (!prop) > + continue; > + bus_min = be32_to_cpup(prop); > + if (bus->number == bus_min) > + return np; > + } > > - return of_node_get(hose->dn); > + return NULL; > } > > static void pcibios_scan_phb(struct pci_controller *hose) > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > index d5e58ba..7c56c2e 100644 > --- a/drivers/pci/host/Kconfig > +++ b/drivers/pci/host/Kconfig > @@ -79,7 +79,7 @@ config PCI_KEYSTONE > > config PCIE_XILINX > bool "Xilinx AXI PCIe host bridge support" > - depends on ARCH_ZYNQ > + depends on ARCH_ZYNQ || MICROBLAZE > help > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe > Host Bridge driver. > -- > 2.1.1
Hi Bharat, On Wed, Feb 03, 2016 at 03:40:21PM +0000, Bharat Kumar Gogada wrote: > Ping You said you were going to do another revision, so I'm waiting for r3. > > This patch does required modifications to microblaze PCI subsystem, to > > work with generic driver (drivers/pci/host/pcie-xilinx.c) on Microblaze > > and Zynq. > > > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> > > --- > > Changes: > > Modified pcibios_fixup_bus in pci-common.c, as per generic architecuture. > > Modified pcibios_align_resource in pci-common.c, as per generic > > architecuture. > > Modified pcibios_get_phb_of_node function in pci-common.c, to remove > > dependency on struct pci_controller. > > Removed pci_domain_nr in pci-common.c, instead using generic code. > > Added pcibios_add_device in pci-common.c, as per generic architecuture. > > Adding Kernel configuration in arch/microblaze as required for generic PCI > > domains. > > Added kernel configuration for driver to support Microblaze. > > --- > > arch/microblaze/Kconfig | 3 ++ > > arch/microblaze/pci/pci-common.c | 61 +++++++++++++++-------------------- > > ----- > > drivers/pci/host/Kconfig | 2 +- > > 3 files changed, 27 insertions(+), 39 deletions(-) > > > > diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig > > index 0bce820..c3702b9 100644 > > --- a/arch/microblaze/Kconfig > > +++ b/arch/microblaze/Kconfig > > @@ -271,6 +271,9 @@ config PCI > > config PCI_DOMAINS > > def_bool PCI > > > > +config PCI_DOMAINS_GENERIC > > + def_bool PCI_DOMAINS > > + > > config PCI_SYSCALL > > def_bool PCI > > > > diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci- > > common.c > > index ae838ed..bc72856 100644 > > --- a/arch/microblaze/pci/pci-common.c > > +++ b/arch/microblaze/pci/pci-common.c > > @@ -123,17 +123,6 @@ unsigned long pci_address_to_pio(phys_addr_t > > address) > > } > > EXPORT_SYMBOL_GPL(pci_address_to_pio); > > > > -/* > > - * Return the domain number for this bus. > > - */ > > -int pci_domain_nr(struct pci_bus *bus) > > -{ > > - struct pci_controller *hose = pci_bus_to_host(bus); > > - > > - return hose->global_number; > > -} > > -EXPORT_SYMBOL(pci_domain_nr); > > - > > /* This routine is meant to be used early during boot, when the > > * PCI bus numbers have not yet been assigned, and you need to > > * issue PCI config cycles to an OF device. > > @@ -863,26 +852,10 @@ void pcibios_setup_bus_devices(struct pci_bus > > *bus) > > > > void pcibios_fixup_bus(struct pci_bus *bus) > > { > > - /* When called from the generic PCI probe, read PCI<->PCI bridge > > - * bases. This is -not- called when generating the PCI tree from > > - * the OF device-tree. > > - */ > > - if (bus->self != NULL) > > - pci_read_bridge_bases(bus); > > - > > - /* Now fixup the bus bus */ > > - pcibios_setup_bus_self(bus); > > - > > - /* Now fixup devices on that bus */ > > - pcibios_setup_bus_devices(bus); > > + /* nothing to do */ > > } > > EXPORT_SYMBOL(pcibios_fixup_bus); > > > > -static int skip_isa_ioresource_align(struct pci_dev *dev) > > -{ > > - return 0; > > -} > > - > > /* > > * We need to avoid collisions with `mirrored' VGA ports > > * and other strange ISA hardware, so we always want the > > @@ -899,20 +872,20 @@ static int skip_isa_ioresource_align(struct pci_dev > > *dev) > > resource_size_t pcibios_align_resource(void *data, const struct resource > > *res, > > resource_size_t size, resource_size_t align) > > { > > - struct pci_dev *dev = data; > > resource_size_t start = res->start; > > > > - if (res->flags & IORESOURCE_IO) { > > - if (skip_isa_ioresource_align(dev)) > > - return start; > > - if (start & 0x300) > > - start = (start + 0x3ff) & ~0x3ff; > > - } > > - > > return start; > > } > > EXPORT_SYMBOL(pcibios_align_resource); > > > > +int pcibios_add_device(struct pci_dev *dev) > > +{ > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(pcibios_add_device); > > + > > /* > > * Reparent resource children of pr that conflict with res > > * under res, and make res replace those children. > > @@ -1335,9 +1308,21 @@ static void pcibios_setup_phb_resources(struct > > pci_controller *hose, > > > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) > > { > > - struct pci_controller *hose = bus->sysdata; > > + struct device_node *np; > > + > > + for_each_node_by_type(np, "pci") { > > + const void *prop; > > + unsigned int bus_min; > > + > > + prop = of_get_property(np, "bus-range", NULL); > > + if (!prop) > > + continue; > > + bus_min = be32_to_cpup(prop); > > + if (bus->number == bus_min) > > + return np; > > + } > > > > - return of_node_get(hose->dn); > > + return NULL; > > } > > > > static void pcibios_scan_phb(struct pci_controller *hose) > > diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig > > index d5e58ba..7c56c2e 100644 > > --- a/drivers/pci/host/Kconfig > > +++ b/drivers/pci/host/Kconfig > > @@ -79,7 +79,7 @@ config PCI_KEYSTONE > > > > config PCIE_XILINX > > bool "Xilinx AXI PCIe host bridge support" > > - depends on ARCH_ZYNQ > > + depends on ARCH_ZYNQ || MICROBLAZE > > help > > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe > > Host Bridge driver. > > -- > > 2.1.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
> You said you were going to do another revision, so I'm waiting for r3. Hi Bjorn, Do you have any comments on this patch (5/5), I will address other patches comments along with this patch, if any, in v3. Bharat > > > > This patch does required modifications to microblaze PCI subsystem, > > > to work with generic driver (drivers/pci/host/pcie-xilinx.c) on > > > Microblaze and Zynq. > > > > > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > > > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> > > > --- > > > Changes: > > > Modified pcibios_fixup_bus in pci-common.c, as per generic > architecuture. > > > Modified pcibios_align_resource in pci-common.c, as per generic > > > architecuture. > > > Modified pcibios_get_phb_of_node function in pci-common.c, to > remove > > > dependency on struct pci_controller. > > > Removed pci_domain_nr in pci-common.c, instead using generic code. > > > Added pcibios_add_device in pci-common.c, as per generic > architecuture. > > > Adding Kernel configuration in arch/microblaze as required for > > > generic PCI domains. > > > Added kernel configuration for driver to support Microblaze. > > > --- > > > arch/microblaze/Kconfig | 3 ++ > > > arch/microblaze/pci/pci-common.c | 61 > > > +++++++++++++++-------------------- > > > ----- > > > drivers/pci/host/Kconfig | 2 +- > > > 3 files changed, 27 insertions(+), 39 deletions(-) > > > > > > diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig index > > > 0bce820..c3702b9 100644 > > > --- a/arch/microblaze/Kconfig > > > +++ b/arch/microblaze/Kconfig > > > @@ -271,6 +271,9 @@ config PCI > > > config PCI_DOMAINS > > > def_bool PCI > > > > > > +config PCI_DOMAINS_GENERIC > > > + def_bool PCI_DOMAINS > > > + > > > config PCI_SYSCALL > > > def_bool PCI > > > > > > diff --git a/arch/microblaze/pci/pci-common.c > > > b/arch/microblaze/pci/pci- common.c index ae838ed..bc72856 100644 > > > --- a/arch/microblaze/pci/pci-common.c > > > +++ b/arch/microblaze/pci/pci-common.c > > > @@ -123,17 +123,6 @@ unsigned long pci_address_to_pio(phys_addr_t > > > address) > > > } > > > EXPORT_SYMBOL_GPL(pci_address_to_pio); > > > > > > -/* > > > - * Return the domain number for this bus. > > > - */ > > > -int pci_domain_nr(struct pci_bus *bus) -{ > > > - struct pci_controller *hose = pci_bus_to_host(bus); > > > - > > > - return hose->global_number; > > > -} > > > -EXPORT_SYMBOL(pci_domain_nr); > > > - > > > /* This routine is meant to be used early during boot, when the > > > * PCI bus numbers have not yet been assigned, and you need to > > > * issue PCI config cycles to an OF device. > > > @@ -863,26 +852,10 @@ void pcibios_setup_bus_devices(struct pci_bus > > > *bus) > > > > > > void pcibios_fixup_bus(struct pci_bus *bus) { > > > - /* When called from the generic PCI probe, read PCI<->PCI bridge > > > - * bases. This is -not- called when generating the PCI tree from > > > - * the OF device-tree. > > > - */ > > > - if (bus->self != NULL) > > > - pci_read_bridge_bases(bus); > > > - > > > - /* Now fixup the bus bus */ > > > - pcibios_setup_bus_self(bus); > > > - > > > - /* Now fixup devices on that bus */ > > > - pcibios_setup_bus_devices(bus); > > > + /* nothing to do */ > > > } > > > EXPORT_SYMBOL(pcibios_fixup_bus); > > > > > > -static int skip_isa_ioresource_align(struct pci_dev *dev) -{ > > > - return 0; > > > -} > > > - > > > /* > > > * We need to avoid collisions with `mirrored' VGA ports > > > * and other strange ISA hardware, so we always want the @@ -899,20 > > > +872,20 @@ static int skip_isa_ioresource_align(struct pci_dev > > > *dev) > > > resource_size_t pcibios_align_resource(void *data, const struct > > > resource *res, > > > resource_size_t size, resource_size_t align) { > > > - struct pci_dev *dev = data; > > > resource_size_t start = res->start; > > > > > > - if (res->flags & IORESOURCE_IO) { > > > - if (skip_isa_ioresource_align(dev)) > > > - return start; > > > - if (start & 0x300) > > > - start = (start + 0x3ff) & ~0x3ff; > > > - } > > > - > > > return start; > > > } > > > EXPORT_SYMBOL(pcibios_align_resource); > > > > > > +int pcibios_add_device(struct pci_dev *dev) { > > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(pcibios_add_device); > > > + > > > /* > > > * Reparent resource children of pr that conflict with res > > > * under res, and make res replace those children. > > > @@ -1335,9 +1308,21 @@ static void > > > pcibios_setup_phb_resources(struct > > > pci_controller *hose, > > > > > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) { > > > - struct pci_controller *hose = bus->sysdata; > > > + struct device_node *np; > > > + > > > + for_each_node_by_type(np, "pci") { > > > + const void *prop; > > > + unsigned int bus_min; > > > + > > > + prop = of_get_property(np, "bus-range", NULL); > > > + if (!prop) > > > + continue; > > > + bus_min = be32_to_cpup(prop); > > > + if (bus->number == bus_min) > > > + return np; > > > + } > > > > > > - return of_node_get(hose->dn); > > > + return NULL; > > > } > > > > > > static void pcibios_scan_phb(struct pci_controller *hose) diff > > > --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index > > > d5e58ba..7c56c2e 100644 > > > --- a/drivers/pci/host/Kconfig > > > +++ b/drivers/pci/host/Kconfig > > > @@ -79,7 +79,7 @@ config PCI_KEYSTONE > > > > > > config PCIE_XILINX > > > bool "Xilinx AXI PCIe host bridge support" > > > - depends on ARCH_ZYNQ > > > + depends on ARCH_ZYNQ || MICROBLAZE > > > help > > > Say 'Y' here if you want kernel to support the Xilinx AXI PCIe > > > Host Bridge driver. > > > -- > > > 2.1.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
[+cc Ben, pcibios_get_phb_of_node() question] On Tue, Jan 12, 2016 at 11:06:13PM +0530, Bharat Kumar Gogada wrote: > This patch does required modifications to microblaze PCI subsystem, to > work with generic driver (drivers/pci/host/pcie-xilinx.c) on Microblaze > and Zynq. > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> >... > resource_size_t pcibios_align_resource(void *data, const struct resource *res, > resource_size_t size, resource_size_t align) > { > - struct pci_dev *dev = data; > resource_size_t start = res->start; > > - if (res->flags & IORESOURCE_IO) { > - if (skip_isa_ioresource_align(dev)) > - return start; > - if (start & 0x300) > - start = (start + 0x3ff) & ~0x3ff; > - } > - > return start; "return res->start;" is sufficient; no need for a temporary variable. > } > EXPORT_SYMBOL(pcibios_align_resource); > > +int pcibios_add_device(struct pci_dev *dev) > +{ > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > + > + return 0; > +} > +EXPORT_SYMBOL(pcibios_add_device); > + > /* > * Reparent resource children of pr that conflict with res > * under res, and make res replace those children. > @@ -1335,9 +1308,21 @@ static void pcibios_setup_phb_resources(struct pci_controller *hose, > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) > { > - struct pci_controller *hose = bus->sysdata; > + struct device_node *np; > + > + for_each_node_by_type(np, "pci") { > + const void *prop; > + unsigned int bus_min; > + > + prop = of_get_property(np, "bus-range", NULL); > + if (!prop) > + continue; > + bus_min = be32_to_cpup(prop); > + if (bus->number == bus_min) > + return np; > + } > > - return of_node_get(hose->dn); > + return NULL; Hmmm. The old microblaze code ("return of_node_get(hose->dn);") is basically the same as the mips and powerpc versions. The new code is basically the same as the x86 version. I like the generic weak version in drivers/pci/of.c because it doesn't use any arch-specific data, and it looks like if we just set the struct device.of_node members correctly, everything should Just Work. But Ben added both the generic and the x86 versions the same day, so there must be some complication: 98d9f30c820d ("pci/of: Match PCI devices to OF nodes dynamically") 3d5fe5a65af9 ("x86/devicetree: Use generic PCI <-> OF matching") So I guess my question is, why do we need a microblaze-specific version at all? Bjorn
[+cc Ben for real this time] On Wed, Feb 03, 2016 at 10:32:07AM -0600, Bjorn Helgaas wrote: > [+cc Ben, pcibios_get_phb_of_node() question] > > On Tue, Jan 12, 2016 at 11:06:13PM +0530, Bharat Kumar Gogada wrote: > > This patch does required modifications to microblaze PCI subsystem, to > > work with generic driver (drivers/pci/host/pcie-xilinx.c) on Microblaze > > and Zynq. > > > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> > >... > > > resource_size_t pcibios_align_resource(void *data, const struct resource *res, > > resource_size_t size, resource_size_t align) > > { > > - struct pci_dev *dev = data; > > resource_size_t start = res->start; > > > > - if (res->flags & IORESOURCE_IO) { > > - if (skip_isa_ioresource_align(dev)) > > - return start; > > - if (start & 0x300) > > - start = (start + 0x3ff) & ~0x3ff; > > - } > > - > > return start; > > "return res->start;" is sufficient; no need for a temporary variable. > > > } > > EXPORT_SYMBOL(pcibios_align_resource); > > > > +int pcibios_add_device(struct pci_dev *dev) > > +{ > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > > + > > + return 0; > > +} > > +EXPORT_SYMBOL(pcibios_add_device); > > + > > /* > > * Reparent resource children of pr that conflict with res > > * under res, and make res replace those children. > > @@ -1335,9 +1308,21 @@ static void pcibios_setup_phb_resources(struct pci_controller *hose, > > > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) > > { > > - struct pci_controller *hose = bus->sysdata; > > + struct device_node *np; > > + > > + for_each_node_by_type(np, "pci") { > > + const void *prop; > > + unsigned int bus_min; > > + > > + prop = of_get_property(np, "bus-range", NULL); > > + if (!prop) > > + continue; > > + bus_min = be32_to_cpup(prop); > > + if (bus->number == bus_min) > > + return np; > > + } > > > > - return of_node_get(hose->dn); > > + return NULL; > > Hmmm. The old microblaze code ("return of_node_get(hose->dn);") is > basically the same as the mips and powerpc versions. The new code is > basically the same as the x86 version. > > I like the generic weak version in drivers/pci/of.c because it doesn't > use any arch-specific data, and it looks like if we just set the > struct device.of_node members correctly, everything should Just Work. > > But Ben added both the generic and the x86 versions the same day, so > there must be some complication: > > 98d9f30c820d ("pci/of: Match PCI devices to OF nodes dynamically") > 3d5fe5a65af9 ("x86/devicetree: Use generic PCI <-> OF matching") > > So I guess my question is, why do we need a microblaze-specific > version at all? > > Bjorn > -- > 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
> Subject: Re: [PATCH V2 5/5] Microblaze: Modifying microblaze PCI subsytem > to support generic Xilinx AXI PCIe Host Bridge IP driver > > [+cc Ben for real this time] > > On Wed, Feb 03, 2016 at 10:32:07AM -0600, Bjorn Helgaas wrote: > > [+cc Ben, pcibios_get_phb_of_node() question] > > > > On Tue, Jan 12, 2016 at 11:06:13PM +0530, Bharat Kumar Gogada wrote: > > > This patch does required modifications to microblaze PCI subsystem, > > > to work with generic driver (drivers/pci/host/pcie-xilinx.c) on > > > Microblaze and Zynq. > > > > > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > > > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> ... > > > > > resource_size_t pcibios_align_resource(void *data, const struct resource > *res, > > > resource_size_t size, resource_size_t align) { > > > - struct pci_dev *dev = data; > > > resource_size_t start = res->start; > > > > > > - if (res->flags & IORESOURCE_IO) { > > > - if (skip_isa_ioresource_align(dev)) > > > - return start; > > > - if (start & 0x300) > > > - start = (start + 0x3ff) & ~0x3ff; > > > - } > > > - > > > return start; > > > > "return res->start;" is sufficient; no need for a temporary variable. > > Agreed will address in next patch. > > > } > > > EXPORT_SYMBOL(pcibios_align_resource); > > > > > > +int pcibios_add_device(struct pci_dev *dev) { > > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(pcibios_add_device); > > > + > > > /* > > > * Reparent resource children of pr that conflict with res > > > * under res, and make res replace those children. > > > @@ -1335,9 +1308,21 @@ static void > > > pcibios_setup_phb_resources(struct pci_controller *hose, > > > > > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) { > > > - struct pci_controller *hose = bus->sysdata; > > > + struct device_node *np; > > > + > > > + for_each_node_by_type(np, "pci") { > > > + const void *prop; > > > + unsigned int bus_min; > > > + > > > + prop = of_get_property(np, "bus-range", NULL); > > > + if (!prop) > > > + continue; > > > + bus_min = be32_to_cpup(prop); > > > + if (bus->number == bus_min) > > > + return np; > > > + } > > > > > > - return of_node_get(hose->dn); > > > + return NULL; > > > > Hmmm. The old microblaze code ("return of_node_get(hose->dn);") is > > basically the same as the mips and powerpc versions. The new code is > > basically the same as the x86 version. > > > > I like the generic weak version in drivers/pci/of.c because it doesn't > > use any arch-specific data, and it looks like if we just set the > > struct device.of_node members correctly, everything should Just Work. > > > > But Ben added both the generic and the x86 versions the same day, so > > there must be some complication: > > > > 98d9f30c820d ("pci/of: Match PCI devices to OF nodes dynamically") > > 3d5fe5a65af9 ("x86/devicetree: Use generic PCI <-> OF matching") > > > > So I guess my question is, why do we need a microblaze-specific > > version at all? > > I did not notice the weak version in /pci/of.c, I have tested with weak version also and it is working. We might not need this microblaze specific version, but will wait for ben's reply. Bharat
On Thu, Feb 04, 2016 at 05:49:20AM +0000, Bharat Kumar Gogada wrote: > > Subject: Re: [PATCH V2 5/5] Microblaze: Modifying microblaze PCI subsytem > > to support generic Xilinx AXI PCIe Host Bridge IP driver > > > > [+cc Ben for real this time] > > > > On Wed, Feb 03, 2016 at 10:32:07AM -0600, Bjorn Helgaas wrote: > > > [+cc Ben, pcibios_get_phb_of_node() question] > > > > > > On Tue, Jan 12, 2016 at 11:06:13PM +0530, Bharat Kumar Gogada wrote: > > > > This patch does required modifications to microblaze PCI subsystem, > > > > to work with generic driver (drivers/pci/host/pcie-xilinx.c) on > > > > Microblaze and Zynq. > > > > > > > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > > > > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> ... > > > > > > > resource_size_t pcibios_align_resource(void *data, const struct resource > > *res, > > > > resource_size_t size, resource_size_t align) { > > > > - struct pci_dev *dev = data; > > > > resource_size_t start = res->start; > > > > > > > > - if (res->flags & IORESOURCE_IO) { > > > > - if (skip_isa_ioresource_align(dev)) > > > > - return start; > > > > - if (start & 0x300) > > > > - start = (start + 0x3ff) & ~0x3ff; > > > > - } > > > > - > > > > return start; > > > > > > "return res->start;" is sufficient; no need for a temporary variable. > > > > Agreed will address in next patch. > > > > } > > > > EXPORT_SYMBOL(pcibios_align_resource); > > > > > > > > +int pcibios_add_device(struct pci_dev *dev) { > > > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > > > > + > > > > + return 0; > > > > +} > > > > +EXPORT_SYMBOL(pcibios_add_device); > > > > + > > > > /* > > > > * Reparent resource children of pr that conflict with res > > > > * under res, and make res replace those children. > > > > @@ -1335,9 +1308,21 @@ static void > > > > pcibios_setup_phb_resources(struct pci_controller *hose, > > > > > > > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) { > > > > - struct pci_controller *hose = bus->sysdata; > > > > + struct device_node *np; > > > > + > > > > + for_each_node_by_type(np, "pci") { > > > > + const void *prop; > > > > + unsigned int bus_min; > > > > + > > > > + prop = of_get_property(np, "bus-range", NULL); > > > > + if (!prop) > > > > + continue; > > > > + bus_min = be32_to_cpup(prop); > > > > + if (bus->number == bus_min) > > > > + return np; > > > > + } > > > > > > > > - return of_node_get(hose->dn); > > > > + return NULL; > > > > > > Hmmm. The old microblaze code ("return of_node_get(hose->dn);") is > > > basically the same as the mips and powerpc versions. The new code is > > > basically the same as the x86 version. > > > > > > I like the generic weak version in drivers/pci/of.c because it doesn't > > > use any arch-specific data, and it looks like if we just set the > > > struct device.of_node members correctly, everything should Just Work. > > > > > > But Ben added both the generic and the x86 versions the same day, so > > > there must be some complication: > > > > > > 98d9f30c820d ("pci/of: Match PCI devices to OF nodes dynamically") > > > 3d5fe5a65af9 ("x86/devicetree: Use generic PCI <-> OF matching") > > > > > > So I guess my question is, why do we need a microblaze-specific > > > version at all? > > > > I did not notice the weak version in /pci/of.c, I have tested with > weak version also and it is working. We might not need this > microblaze specific version, but will wait for ben's reply. If the generic version works, and you don't need the microblaze- specific version, just remove it and we'll get this wrapped up. No need to wait for Ben. Bjorn
> Subject: Re: [PATCH V2 5/5] Microblaze: Modifying microblaze PCI subsytem > to support generic Xilinx AXI PCIe Host Bridge IP driver > > On Thu, Feb 04, 2016 at 05:49:20AM +0000, Bharat Kumar Gogada wrote: > > > Subject: Re: [PATCH V2 5/5] Microblaze: Modifying microblaze PCI > > > subsytem to support generic Xilinx AXI PCIe Host Bridge IP driver > > > > > > [+cc Ben for real this time] > > > > > > On Wed, Feb 03, 2016 at 10:32:07AM -0600, Bjorn Helgaas wrote: > > > > [+cc Ben, pcibios_get_phb_of_node() question] > > > > > > > > On Tue, Jan 12, 2016 at 11:06:13PM +0530, Bharat Kumar Gogada wrote: > > > > > This patch does required modifications to microblaze PCI > > > > > subsystem, to work with generic driver > > > > > (drivers/pci/host/pcie-xilinx.c) on Microblaze and Zynq. > > > > > > > > > > Signed-off-by: Bharat Kumar Gogada <bharatku@xilinx.com> > > > > > Signed-off-by: Ravi Kiran Gummaluri <rgummal@xilinx.com> ... > > > > > > > > > resource_size_t pcibios_align_resource(void *data, const struct > > > > > resource > > > *res, > > > > > resource_size_t size, resource_size_t > align) { > > > > > - struct pci_dev *dev = data; > > > > > resource_size_t start = res->start; > > > > > > > > > > - if (res->flags & IORESOURCE_IO) { > > > > > - if (skip_isa_ioresource_align(dev)) > > > > > - return start; > > > > > - if (start & 0x300) > > > > > - start = (start + 0x3ff) & ~0x3ff; > > > > > - } > > > > > - > > > > > return start; > > > > > > > > "return res->start;" is sufficient; no need for a temporary variable. > > > > > > Agreed will address in next patch. > > > > > } > > > > > EXPORT_SYMBOL(pcibios_align_resource); > > > > > > > > > > +int pcibios_add_device(struct pci_dev *dev) { > > > > > + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); > > > > > + > > > > > + return 0; > > > > > +} > > > > > +EXPORT_SYMBOL(pcibios_add_device); > > > > > + > > > > > /* > > > > > * Reparent resource children of pr that conflict with res > > > > > * under res, and make res replace those children. > > > > > @@ -1335,9 +1308,21 @@ static void > > > > > pcibios_setup_phb_resources(struct pci_controller *hose, > > > > > > > > > > struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) > { > > > > > - struct pci_controller *hose = bus->sysdata; > > > > > + struct device_node *np; > > > > > + > > > > > + for_each_node_by_type(np, "pci") { > > > > > + const void *prop; > > > > > + unsigned int bus_min; > > > > > + > > > > > + prop = of_get_property(np, "bus-range", NULL); > > > > > + if (!prop) > > > > > + continue; > > > > > + bus_min = be32_to_cpup(prop); > > > > > + if (bus->number == bus_min) > > > > > + return np; > > > > > + } > > > > > > > > > > - return of_node_get(hose->dn); > > > > > + return NULL; > > > > > > > > Hmmm. The old microblaze code ("return of_node_get(hose->dn);") > > > > is basically the same as the mips and powerpc versions. The new > > > > code is basically the same as the x86 version. > > > > > > > > I like the generic weak version in drivers/pci/of.c because it > > > > doesn't use any arch-specific data, and it looks like if we just > > > > set the struct device.of_node members correctly, everything should > Just Work. > > > > > > > > But Ben added both the generic and the x86 versions the same day, > > > > so there must be some complication: > > > > > > > > 98d9f30c820d ("pci/of: Match PCI devices to OF nodes dynamically") > > > > 3d5fe5a65af9 ("x86/devicetree: Use generic PCI <-> OF matching") > > > > > > > > So I guess my question is, why do we need a microblaze-specific > > > > version at all? > > > > > > I did not notice the weak version in /pci/of.c, I have tested with > > weak version also and it is working. We might not need this > > microblaze specific version, but will wait for ben's reply. > > If the generic version works, and you don't need the microblaze- specific > version, just remove it and we'll get this wrapped up. No need to wait for > Ben. > Ok Bjorn, I will remove it and send next series of patches. Bharat
diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig index 0bce820..c3702b9 100644 --- a/arch/microblaze/Kconfig +++ b/arch/microblaze/Kconfig @@ -271,6 +271,9 @@ config PCI config PCI_DOMAINS def_bool PCI +config PCI_DOMAINS_GENERIC + def_bool PCI_DOMAINS + config PCI_SYSCALL def_bool PCI diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c index ae838ed..bc72856 100644 --- a/arch/microblaze/pci/pci-common.c +++ b/arch/microblaze/pci/pci-common.c @@ -123,17 +123,6 @@ unsigned long pci_address_to_pio(phys_addr_t address) } EXPORT_SYMBOL_GPL(pci_address_to_pio); -/* - * Return the domain number for this bus. - */ -int pci_domain_nr(struct pci_bus *bus) -{ - struct pci_controller *hose = pci_bus_to_host(bus); - - return hose->global_number; -} -EXPORT_SYMBOL(pci_domain_nr); - /* This routine is meant to be used early during boot, when the * PCI bus numbers have not yet been assigned, and you need to * issue PCI config cycles to an OF device. @@ -863,26 +852,10 @@ void pcibios_setup_bus_devices(struct pci_bus *bus) void pcibios_fixup_bus(struct pci_bus *bus) { - /* When called from the generic PCI probe, read PCI<->PCI bridge - * bases. This is -not- called when generating the PCI tree from - * the OF device-tree. - */ - if (bus->self != NULL) - pci_read_bridge_bases(bus); - - /* Now fixup the bus bus */ - pcibios_setup_bus_self(bus); - - /* Now fixup devices on that bus */ - pcibios_setup_bus_devices(bus); + /* nothing to do */ } EXPORT_SYMBOL(pcibios_fixup_bus); -static int skip_isa_ioresource_align(struct pci_dev *dev) -{ - return 0; -} - /* * We need to avoid collisions with `mirrored' VGA ports * and other strange ISA hardware, so we always want the @@ -899,20 +872,20 @@ static int skip_isa_ioresource_align(struct pci_dev *dev) resource_size_t pcibios_align_resource(void *data, const struct resource *res, resource_size_t size, resource_size_t align) { - struct pci_dev *dev = data; resource_size_t start = res->start; - if (res->flags & IORESOURCE_IO) { - if (skip_isa_ioresource_align(dev)) - return start; - if (start & 0x300) - start = (start + 0x3ff) & ~0x3ff; - } - return start; } EXPORT_SYMBOL(pcibios_align_resource); +int pcibios_add_device(struct pci_dev *dev) +{ + dev->irq = of_irq_parse_and_map_pci(dev, 0, 0); + + return 0; +} +EXPORT_SYMBOL(pcibios_add_device); + /* * Reparent resource children of pr that conflict with res * under res, and make res replace those children. @@ -1335,9 +1308,21 @@ static void pcibios_setup_phb_resources(struct pci_controller *hose, struct device_node *pcibios_get_phb_of_node(struct pci_bus *bus) { - struct pci_controller *hose = bus->sysdata; + struct device_node *np; + + for_each_node_by_type(np, "pci") { + const void *prop; + unsigned int bus_min; + + prop = of_get_property(np, "bus-range", NULL); + if (!prop) + continue; + bus_min = be32_to_cpup(prop); + if (bus->number == bus_min) + return np; + } - return of_node_get(hose->dn); + return NULL; } static void pcibios_scan_phb(struct pci_controller *hose) diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig index d5e58ba..7c56c2e 100644 --- a/drivers/pci/host/Kconfig +++ b/drivers/pci/host/Kconfig @@ -79,7 +79,7 @@ config PCI_KEYSTONE config PCIE_XILINX bool "Xilinx AXI PCIe host bridge support" - depends on ARCH_ZYNQ + depends on ARCH_ZYNQ || MICROBLAZE help Say 'Y' here if you want kernel to support the Xilinx AXI PCIe Host Bridge driver.