diff mbox

[v5,2/5] PCI: designware: Add ARM64 support

Message ID 1437794486-21134-3-git-send-email-wangzhou1@hisilicon.com
State Superseded
Headers show

Commit Message

Zhou Wang July 25, 2015, 3:21 a.m. UTC
This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
each PCIe host driver which is based on pcie-designware. This patch also try
to use of_pci_get_host_bridge_resources for ARM32 and ARM64 according to the
suggestion for Gabriele[1]

This patch is based on Gabriele's patch about of_pci_range fix[2]

I have compiled the driver with multi_v7_defconfig. However, I don't have
ARM32 PCIe related board to do test. It will be appreciated if someone could
help to test it.

Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>

[1] http://www.spinics.net/lists/linux-pci/msg42194.html
[2] https://patchwork.ozlabs.org/patch/495018/
---
 drivers/pci/host/pci-dra7xx.c      |   1 +
 drivers/pci/host/pci-exynos.c      |   2 +-
 drivers/pci/host/pci-imx6.c        |   2 +-
 drivers/pci/host/pci-keystone-dw.c |   2 +-
 drivers/pci/host/pci-keystone.c    |   2 +-
 drivers/pci/host/pci-layerscape.c  |   2 +-
 drivers/pci/host/pcie-designware.c | 217 +++++++++++++------------------------
 drivers/pci/host/pcie-designware.h |  10 +-
 drivers/pci/host/pcie-spear13xx.c  |   2 +-
 9 files changed, 86 insertions(+), 154 deletions(-)

Comments

Zhou Wang July 28, 2015, 6:21 a.m. UTC | #1
On 2015/7/25 11:21, Zhou Wang wrote:
> This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
> function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
> move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
> each PCIe host driver which is based on pcie-designware. This patch also try
> to use of_pci_get_host_bridge_resources for ARM32 and ARM64 according to the
> suggestion for Gabriele[1]
> 
> This patch is based on Gabriele's patch about of_pci_range fix[2]
> 
> I have compiled the driver with multi_v7_defconfig. However, I don't have
> ARM32 PCIe related board to do test. It will be appreciated if someone could
> help to test it.
>

Hi James,

If you have time, could you help to test this patch on i.MX 6Quad board?
You need apply Gabriele's patch before applying this patch.

It will be very appreciate and helpful if we can get test result from you.

Thanks and Regards,
Zhou

> Signed-off-by: Zhou Wang <wangzhou1@hisilicon.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Gabriele Paoloni <gabriele.paoloni@huawei.com>
> 
> [1] http://www.spinics.net/lists/linux-pci/msg42194.html
> [2] https://patchwork.ozlabs.org/patch/495018/
> ---
>  drivers/pci/host/pci-dra7xx.c      |   1 +
>  drivers/pci/host/pci-exynos.c      |   2 +-
>  drivers/pci/host/pci-imx6.c        |   2 +-
>  drivers/pci/host/pci-keystone-dw.c |   2 +-
>  drivers/pci/host/pci-keystone.c    |   2 +-
>  drivers/pci/host/pci-layerscape.c  |   2 +-
>  drivers/pci/host/pcie-designware.c | 217 +++++++++++++------------------------
>  drivers/pci/host/pcie-designware.h |  10 +-
>  drivers/pci/host/pcie-spear13xx.c  |   2 +-
>  9 files changed, 86 insertions(+), 154 deletions(-)
> 
> diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
> index 80db09e..69364e8 100644
> --- a/drivers/pci/host/pci-dra7xx.c
> +++ b/drivers/pci/host/pci-dra7xx.c
> @@ -275,6 +275,7 @@ static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
>  
>  	pp = &dra7xx->pp;
>  	pp->dev = dev;
> +	pp->root_bus_nr = 0;
>  	pp->ops = &dra7xx_pcie_host_ops;
>  
>  	pp->irq = platform_get_irq(pdev, 1);
> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
> index f9f468d..9771bb0 100644
> --- a/drivers/pci/host/pci-exynos.c
> +++ b/drivers/pci/host/pci-exynos.c
> @@ -530,7 +530,7 @@ static int __init exynos_add_pcie_port(struct pcie_port *pp,
>  		}
>  	}
>  
> -	pp->root_bus_nr = -1;
> +	pp->root_bus_nr = 0;
>  	pp->ops = &exynos_pcie_host_ops;
>  
>  	ret = dw_pcie_host_init(pp);
> diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
> index 233a196..bec256c 100644
> --- a/drivers/pci/host/pci-imx6.c
> +++ b/drivers/pci/host/pci-imx6.c
> @@ -551,7 +551,7 @@ static int __init imx6_add_pcie_port(struct pcie_port *pp,
>  		}
>  	}
>  
> -	pp->root_bus_nr = -1;
> +	pp->root_bus_nr = 0;
>  	pp->ops = &imx6_pcie_host_ops;
>  
>  	ret = dw_pcie_host_init(pp);
> diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
> index f34892e..b1e4135 100644
> --- a/drivers/pci/host/pci-keystone-dw.c
> +++ b/drivers/pci/host/pci-keystone-dw.c
> @@ -327,7 +327,7 @@ static void ks_dw_pcie_clear_dbi_mode(void __iomem *reg_virt)
>  void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
>  {
>  	struct pcie_port *pp = &ks_pcie->pp;
> -	u32 start = pp->mem.start, end = pp->mem.end;
> +	u32 start = pp->mem->start, end = pp->mem->end;
>  	int i, tr_size;
>  
>  	/* Disable BARs for inbound access */
> diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
> index 734da58..8113832 100644
> --- a/drivers/pci/host/pci-keystone.c
> +++ b/drivers/pci/host/pci-keystone.c
> @@ -309,7 +309,7 @@ static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
>  			return ret;
>  	}
>  
> -	pp->root_bus_nr = -1;
> +	pp->root_bus_nr = 0;
>  	pp->ops = &keystone_pcie_host_ops;
>  	ret = ks_dw_pcie_host_init(ks_pcie, ks_pcie->msi_intc_np);
>  	if (ret) {
> diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
> index b2328ea1..79ff08c 100644
> --- a/drivers/pci/host/pci-layerscape.c
> +++ b/drivers/pci/host/pci-layerscape.c
> @@ -106,7 +106,7 @@ static int ls_add_pcie_port(struct ls_pcie *pcie)
>  	pp = &pcie->pp;
>  	pp->dev = pcie->dev;
>  	pp->dbi_base = pcie->dbi;
> -	pp->root_bus_nr = -1;
> +	pp->root_bus_nr = 0;
>  	pp->ops = &ls_pcie_host_ops;
>  
>  	ret = dw_pcie_host_init(pp);
> diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
> index 69486be..6092c84 100644
> --- a/drivers/pci/host/pcie-designware.c
> +++ b/drivers/pci/host/pcie-designware.c
> @@ -11,6 +11,7 @@
>   * published by the Free Software Foundation.
>   */
>  
> +#include <linux/hardirq.h>
>  #include <linux/irq.h>
>  #include <linux/irqdomain.h>
>  #include <linux/kernel.h>
> @@ -69,16 +70,7 @@
>  #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
>  #define PCIE_ATU_UPPER_TARGET		0x91C
>  
> -static struct hw_pci dw_pci;
> -
> -static unsigned long global_io_offset;
> -
> -static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
> -{
> -	BUG_ON(!sys->private_data);
> -
> -	return sys->private_data;
> -}
> +static struct pci_ops dw_pcie_ops;
>  
>  int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val)
>  {
> @@ -255,7 +247,7 @@ static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
>  static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
>  {
>  	int irq, pos0, i;
> -	struct pcie_port *pp = sys_to_pcie(desc->dev->bus->sysdata);
> +	struct pcie_port *pp = desc->dev->bus->sysdata;
>  
>  	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
>  				       order_base_2(no_irqs));
> @@ -298,7 +290,7 @@ static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
>  {
>  	int irq, pos;
>  	struct msi_msg msg;
> -	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
> +	struct pcie_port *pp = pdev->bus->sysdata;
>  
>  	if (desc->msi_attrib.is_msix)
>  		return -EINVAL;
> @@ -327,7 +319,7 @@ static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
>  {
>  	struct irq_data *data = irq_get_irq_data(irq);
>  	struct msi_desc *msi = irq_data_get_msi(data);
> -	struct pcie_port *pp = sys_to_pcie(msi->dev->bus->sysdata);
> +	struct pcie_port *pp = msi->dev->bus->sysdata;
>  
>  	clear_irq_range(pp, irq, 1, data->hwirq);
>  }
> @@ -359,21 +351,19 @@ static const struct irq_domain_ops msi_domain_ops = {
>  	.map = dw_pcie_msi_map,
>  };
>  
> -int dw_pcie_host_init(struct pcie_port *pp)
> +int __init dw_pcie_host_init(struct pcie_port *pp)
>  {
>  	struct device_node *np = pp->dev->of_node;
>  	struct platform_device *pdev = to_platform_device(pp->dev);
> -	struct of_pci_range range;
> -	struct of_pci_range_parser parser;
> +	struct pci_bus *bus;
>  	struct resource *cfg_res;
> -	u32 val, na, ns;
> +	LIST_HEAD(res);
> +	u32 val, ns;
>  	const __be32 *addrp;
>  	int i, index, ret;
> +	struct resource_entry *win;
>  
> -	/* Find the address cell size and the number of cells in order to get
> -	 * the untranslated address.
> -	 */
> -	of_property_read_u32(np, "#address-cells", &na);
> +	/* Find the number of cells in order to get the untranslated address */
>  	ns = of_n_size_cells(np);
>  
>  	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> @@ -392,78 +382,62 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  		dev_err(pp->dev, "missing *config* reg space\n");
>  	}
>  
> -	if (of_pci_range_parser_init(&parser, np)) {
> -		dev_err(pp->dev, "missing ranges property\n");
> -		return -EINVAL;
> -	}
> +	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
> +	if (ret)
> +		return ret;
>  
>  	/* Get the I/O and memory ranges from DT */
> -	for_each_of_pci_range(&parser, &range) {
> -		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
> -
> -		if (restype == IORESOURCE_IO) {
> -			of_pci_range_to_resource(&range, np, &pp->io);
> -			pp->io.name = "I/O";
> -			pp->io.start = max_t(resource_size_t,
> -					     PCIBIOS_MIN_IO,
> -					     range.pci_addr + global_io_offset);
> -			pp->io.end = min_t(resource_size_t,
> -					   IO_SPACE_LIMIT,
> -					   range.pci_addr + range.size
> -					   + global_io_offset - 1);
> -			pp->io_size = resource_size(&pp->io);
> -			pp->io_bus_addr = range.pci_addr;
> -			pp->io_base = range.cpu_addr;
> -
> -			/* Find the untranslated IO space address */
> -			pp->io_mod_base = of_read_number(parser.range -
> -							 parser.np + na, ns);
> -		}
> -		if (restype == IORESOURCE_MEM) {
> -			of_pci_range_to_resource(&range, np, &pp->mem);
> -			pp->mem.name = "MEM";
> -			pp->mem_size = resource_size(&pp->mem);
> -			pp->mem_bus_addr = range.pci_addr;
> -
> -			/* Find the untranslated MEM space address */
> -			pp->mem_mod_base = of_read_number(parser.range -
> -							  parser.np + na, ns);
> -		}
> -		if (restype == 0) {
> -			of_pci_range_to_resource(&range, np, &pp->cfg);
> -			pp->cfg0_size = resource_size(&pp->cfg)/2;
> -			pp->cfg1_size = resource_size(&pp->cfg)/2;
> -			pp->cfg0_base = pp->cfg.start;
> -			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
> +	resource_list_for_each_entry(win, &res) {
> +		switch (resource_type(win->res)) {
> +		case IORESOURCE_IO:
> +			pp->io = win->res;
> +			pp->io->name = "I/O";
> +			pp->io_size = resource_size(pp->io);
> +			pp->io_bus_addr = pp->io->start - win->offset;
> +			pp->io_mod_base = win->__res.start;
> +			ret = pci_remap_iospace(pp->io, pp->io_base);
> +			if (ret) {
> +				dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
> +					 ret, pp->io);
> +				continue;
> +			}
> +			break;
> +		case IORESOURCE_MEM:
> +			pp->mem = win->res;
> +			pp->mem->name = "MEM";
> +			pp->mem_size = resource_size(pp->mem);
> +			pp->mem_bus_addr = pp->mem->start - win->offset;
> +			pp->mem_mod_base = win->__res.start;
> +			break;
> +		case 0:
> +			pp->cfg = win->res;
> +			pp->cfg0_size = resource_size(pp->cfg)/2;
> +			pp->cfg1_size = resource_size(pp->cfg)/2;
> +			pp->cfg0_base = pp->cfg->start;
> +			pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
>  
>  			/* Find the untranslated configuration space address */
> -			pp->cfg0_mod_base = of_read_number(parser.range -
> -							   parser.np + na, ns);
> -			pp->cfg1_mod_base = pp->cfg0_mod_base +
> -					    pp->cfg0_size;
> +			pp->cfg0_mod_base = win->__res.start;
> +			pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
> +			break;
> +		case IORESOURCE_BUS:
> +			pp->busn = win->res;
> +			break;
> +		default:
> +			continue;
>  		}
>  	}
>  
> -	ret = of_pci_parse_bus_range(np, &pp->busn);
> -	if (ret < 0) {
> -		pp->busn.name = np->name;
> -		pp->busn.start = 0;
> -		pp->busn.end = 0xff;
> -		pp->busn.flags = IORESOURCE_BUS;
> -		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
> -			ret, &pp->busn);
> -	}
> -
>  	if (!pp->dbi_base) {
> -		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> -					resource_size(&pp->cfg));
> +		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
> +					resource_size(pp->cfg));
>  		if (!pp->dbi_base) {
>  			dev_err(pp->dev, "error with ioremap\n");
>  			return -ENOMEM;
>  		}
>  	}
>  
> -	pp->mem_base = pp->mem.start;
> +	pp->mem_base = pp->mem->start;
>  
>  	if (!pp->va_cfg0_base) {
>  		pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
> @@ -524,15 +498,28 @@ int dw_pcie_host_init(struct pcie_port *pp)
>  	val |= PORT_LOGIC_SPEED_CHANGE;
>  	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>  
> -#ifdef CONFIG_PCI_MSI
> -	dw_pcie_msi_chip.dev = pp->dev;
> -	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
> +	bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
> +			      pp, &res);
> +	if (!bus)
> +		return -ENOMEM;
> +
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +	bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
> +#else
> +	bus->msi = &dw_pcie_msi_chip;
>  #endif
>  
> -	dw_pci.nr_controllers = 1;
> -	dw_pci.private_data = (void **)&pp;
> +	pci_scan_child_bus(bus);
> +	if (pp->ops->scan_bus)
> +		pp->ops->scan_bus(pp);
>  
> -	pci_common_init_dev(pp->dev, &dw_pci);
> +#ifdef CONFIG_ARM
> +	/* support old dtbs that incorrectly describe IRQs */
> +	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> +#endif
> +
> +	pci_assign_unassigned_bus_resources(bus);
> +	pci_bus_add_devices(bus);
>  
>  	return 0;
>  }
> @@ -633,7 +620,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
>  static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>  			int size, u32 *val)
>  {
> -	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
> +	struct pcie_port *pp = bus->sysdata;
>  	int ret;
>  
>  	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
> @@ -657,7 +644,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>  static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>  			int where, int size, u32 val)
>  {
> -	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
> +	struct pcie_port *pp = bus->sysdata;
>  	int ret;
>  
>  	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
> @@ -681,62 +668,6 @@ static struct pci_ops dw_pcie_ops = {
>  	.write = dw_pcie_wr_conf,
>  };
>  
> -static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
> -{
> -	struct pcie_port *pp;
> -
> -	pp = sys_to_pcie(sys);
> -
> -	if (global_io_offset < SZ_1M && pp->io_size > 0) {
> -		sys->io_offset = global_io_offset - pp->io_bus_addr;
> -		pci_ioremap_io(global_io_offset, pp->io_base);
> -		global_io_offset += SZ_64K;
> -		pci_add_resource_offset(&sys->resources, &pp->io,
> -					sys->io_offset);
> -	}
> -
> -	sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
> -	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
> -	pci_add_resource(&sys->resources, &pp->busn);
> -
> -	return 1;
> -}
> -
> -static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> -{
> -	struct pci_bus *bus;
> -	struct pcie_port *pp = sys_to_pcie(sys);
> -
> -	pp->root_bus_nr = sys->busnr;
> -	bus = pci_scan_root_bus(pp->dev, sys->busnr,
> -				  &dw_pcie_ops, sys, &sys->resources);
> -	if (!bus)
> -		return NULL;
> -
> -	if (bus && pp->ops->scan_bus)
> -		pp->ops->scan_bus(pp);
> -
> -	return bus;
> -}
> -
> -static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> -{
> -	struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
> -	int irq;
> -
> -	irq = of_irq_parse_and_map_pci(dev, slot, pin);
> -	if (!irq)
> -		irq = pp->irq;
> -
> -	return irq;
> -}
> -
> -static struct hw_pci dw_pci = {
> -	.setup		= dw_pcie_setup,
> -	.scan		= dw_pcie_scan_bus,
> -	.map_irq	= dw_pcie_map_irq,
> -};
> -
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>  	u32 val;
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index d0bbd27..efac57d 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -34,7 +34,7 @@ struct pcie_port {
>  	u64			cfg1_mod_base;
>  	void __iomem		*va_cfg1_base;
>  	u32			cfg1_size;
> -	u64			io_base;
> +	resource_size_t		io_base;
>  	u64			io_mod_base;
>  	phys_addr_t		io_bus_addr;
>  	u32			io_size;
> @@ -42,10 +42,10 @@ struct pcie_port {
>  	u64			mem_mod_base;
>  	phys_addr_t		mem_bus_addr;
>  	u32			mem_size;
> -	struct resource		cfg;
> -	struct resource		io;
> -	struct resource		mem;
> -	struct resource		busn;
> +	struct resource		*cfg;
> +	struct resource		*io;
> +	struct resource		*mem;
> +	struct resource		*busn;
>  	int			irq;
>  	u32			lanes;
>  	struct pcie_host_ops	*ops;
> diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
> index c49fbdc..03eb204 100644
> --- a/drivers/pci/host/pcie-spear13xx.c
> +++ b/drivers/pci/host/pcie-spear13xx.c
> @@ -286,7 +286,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
>  		return ret;
>  	}
>  
> -	pp->root_bus_nr = -1;
> +	pp->root_bus_nr = 0;
>  	pp->ops = &spear13xx_pcie_host_ops;
>  
>  	ret = dw_pcie_host_init(pp);
> 


--
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
Lorenzo Pieralisi July 29, 2015, 5:24 p.m. UTC | #2
On Sat, Jul 25, 2015 at 04:21:23AM +0100, Zhou Wang wrote:
> This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
> function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
> move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
> each PCIe host driver which is based on pcie-designware. This patch also try

Memory for ports is kzalloc'ed, so there is no need to zero it. I still
think that you should explain the root_bus_nr setting to 0 a bit
better, why you make the change and why it is safe.

[...]

> -int dw_pcie_host_init(struct pcie_port *pp)
> +int __init dw_pcie_host_init(struct pcie_port *pp)
>  {
>         struct device_node *np = pp->dev->of_node;
>         struct platform_device *pdev = to_platform_device(pp->dev);
> -       struct of_pci_range range;
> -       struct of_pci_range_parser parser;
> +       struct pci_bus *bus;
>         struct resource *cfg_res;
> -       u32 val, na, ns;
> +       LIST_HEAD(res);
> +       u32 val, ns;
>         const __be32 *addrp;
>         int i, index, ret;
> +       struct resource_entry *win;
> 
> -       /* Find the address cell size and the number of cells in order to get
> -        * the untranslated address.
> -        */
> -       of_property_read_u32(np, "#address-cells", &na);
> +       /* Find the number of cells in order to get the untranslated address */
>         ns = of_n_size_cells(np);
> 
>         cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> @@ -392,78 +382,62 @@ int dw_pcie_host_init(struct pcie_port *pp)
>                 dev_err(pp->dev, "missing *config* reg space\n");
>         }
> 
> -       if (of_pci_range_parser_init(&parser, np)) {
> -               dev_err(pp->dev, "missing ranges property\n");
> -               return -EINVAL;
> -       }
> +       ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
> +       if (ret)
> +               return ret;
> 
>         /* Get the I/O and memory ranges from DT */
> -       for_each_of_pci_range(&parser, &range) {
> -               unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
> -
> -               if (restype == IORESOURCE_IO) {
> -                       of_pci_range_to_resource(&range, np, &pp->io);
> -                       pp->io.name = "I/O";
> -                       pp->io.start = max_t(resource_size_t,
> -                                            PCIBIOS_MIN_IO,
> -                                            range.pci_addr + global_io_offset);
> -                       pp->io.end = min_t(resource_size_t,
> -                                          IO_SPACE_LIMIT,
> -                                          range.pci_addr + range.size
> -                                          + global_io_offset - 1);
> -                       pp->io_size = resource_size(&pp->io);
> -                       pp->io_bus_addr = range.pci_addr;
> -                       pp->io_base = range.cpu_addr;
> -
> -                       /* Find the untranslated IO space address */
> -                       pp->io_mod_base = of_read_number(parser.range -
> -                                                        parser.np + na, ns);
> -               }
> -               if (restype == IORESOURCE_MEM) {
> -                       of_pci_range_to_resource(&range, np, &pp->mem);
> -                       pp->mem.name = "MEM";
> -                       pp->mem_size = resource_size(&pp->mem);
> -                       pp->mem_bus_addr = range.pci_addr;
> -
> -                       /* Find the untranslated MEM space address */
> -                       pp->mem_mod_base = of_read_number(parser.range -
> -                                                         parser.np + na, ns);
> -               }
> -               if (restype == 0) {
> -                       of_pci_range_to_resource(&range, np, &pp->cfg);
> -                       pp->cfg0_size = resource_size(&pp->cfg)/2;
> -                       pp->cfg1_size = resource_size(&pp->cfg)/2;
> -                       pp->cfg0_base = pp->cfg.start;
> -                       pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
> +       resource_list_for_each_entry(win, &res) {
> +               switch (resource_type(win->res)) {
> +               case IORESOURCE_IO:
> +                       pp->io = win->res;
> +                       pp->io->name = "I/O";
> +                       pp->io_size = resource_size(pp->io);
> +                       pp->io_bus_addr = pp->io->start - win->offset;
> +                       pp->io_mod_base = win->__res.start;
> +                       ret = pci_remap_iospace(pp->io, pp->io_base);
> +                       if (ret) {
> +                               dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
> +                                        ret, pp->io);
> +                               continue;
> +                       }
> +                       break;
> +               case IORESOURCE_MEM:
> +                       pp->mem = win->res;
> +                       pp->mem->name = "MEM";
> +                       pp->mem_size = resource_size(pp->mem);
> +                       pp->mem_bus_addr = pp->mem->start - win->offset;
> +                       pp->mem_mod_base = win->__res.start;
> +                       break;
> +               case 0:
> +                       pp->cfg = win->res;
> +                       pp->cfg0_size = resource_size(pp->cfg)/2;
> +                       pp->cfg1_size = resource_size(pp->cfg)/2;
> +                       pp->cfg0_base = pp->cfg->start;
> +                       pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
> 
>                         /* Find the untranslated configuration space address */
> -                       pp->cfg0_mod_base = of_read_number(parser.range -
> -                                                          parser.np + na, ns);
> -                       pp->cfg1_mod_base = pp->cfg0_mod_base +
> -                                           pp->cfg0_size;
> +                       pp->cfg0_mod_base = win->__res.start;
> +                       pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
> +                       break;
> +               case IORESOURCE_BUS:
> +                       pp->busn = win->res;
> +                       break;
> +               default:
> +                       continue;
>                 }
>         }
> 
> -       ret = of_pci_parse_bus_range(np, &pp->busn);
> -       if (ret < 0) {
> -               pp->busn.name = np->name;
> -               pp->busn.start = 0;
> -               pp->busn.end = 0xff;
> -               pp->busn.flags = IORESOURCE_BUS;
> -               dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
> -                       ret, &pp->busn);
> -       }
> -
>         if (!pp->dbi_base) {
> -               pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
> -                                       resource_size(&pp->cfg));
> +               pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
> +                                       resource_size(pp->cfg));
>                 if (!pp->dbi_base) {
>                         dev_err(pp->dev, "error with ioremap\n");
>                         return -ENOMEM;
>                 }
>         }
> 
> -       pp->mem_base = pp->mem.start;
> +       pp->mem_base = pp->mem->start;
> 
>         if (!pp->va_cfg0_base) {
>                 pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
> @@ -524,15 +498,28 @@ int dw_pcie_host_init(struct pcie_port *pp)
>         val |= PORT_LOGIC_SPEED_CHANGE;
>         dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
> 
> -#ifdef CONFIG_PCI_MSI
> -       dw_pcie_msi_chip.dev = pp->dev;

Is it safe to remove the dev assignment ?

> -       dw_pci.msi_ctrl = &dw_pcie_msi_chip;
> +       bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
> +                             pp, &res);
> +       if (!bus)
> +               return -ENOMEM;
> +
> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
> +       bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
> +#else
> +       bus->msi = &dw_pcie_msi_chip;
>  #endif

For the records, this patch conflicts with my MSI series:

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/360461.html

Conflict resolution is easy but it is worth keeping in mind.

Thanks !
Lorenzo

> 
> -       dw_pci.nr_controllers = 1;
> -       dw_pci.private_data = (void **)&pp;
> +       pci_scan_child_bus(bus);
> +       if (pp->ops->scan_bus)
> +               pp->ops->scan_bus(pp);
> 
> -       pci_common_init_dev(pp->dev, &dw_pci);
> +#ifdef CONFIG_ARM
> +       /* support old dtbs that incorrectly describe IRQs */
> +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
> +#endif
> +
> +       pci_assign_unassigned_bus_resources(bus);
> +       pci_bus_add_devices(bus);
> 
>         return 0;
>  }
> @@ -633,7 +620,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
>  static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>                         int size, u32 *val)
>  {
> -       struct pcie_port *pp = sys_to_pcie(bus->sysdata);
> +       struct pcie_port *pp = bus->sysdata;
>         int ret;
> 
>         if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
> @@ -657,7 +644,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>  static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>                         int where, int size, u32 val)
>  {
> -       struct pcie_port *pp = sys_to_pcie(bus->sysdata);
> +       struct pcie_port *pp = bus->sysdata;
>         int ret;
> 
>         if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
> @@ -681,62 +668,6 @@ static struct pci_ops dw_pcie_ops = {
>         .write = dw_pcie_wr_conf,
>  };
> 
> -static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
> -{
> -       struct pcie_port *pp;
> -
> -       pp = sys_to_pcie(sys);
> -
> -       if (global_io_offset < SZ_1M && pp->io_size > 0) {
> -               sys->io_offset = global_io_offset - pp->io_bus_addr;
> -               pci_ioremap_io(global_io_offset, pp->io_base);
> -               global_io_offset += SZ_64K;
> -               pci_add_resource_offset(&sys->resources, &pp->io,
> -                                       sys->io_offset);
> -       }
> -
> -       sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
> -       pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
> -       pci_add_resource(&sys->resources, &pp->busn);
> -
> -       return 1;
> -}
> -
> -static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
> -{
> -       struct pci_bus *bus;
> -       struct pcie_port *pp = sys_to_pcie(sys);
> -
> -       pp->root_bus_nr = sys->busnr;
> -       bus = pci_scan_root_bus(pp->dev, sys->busnr,
> -                                 &dw_pcie_ops, sys, &sys->resources);
> -       if (!bus)
> -               return NULL;
> -
> -       if (bus && pp->ops->scan_bus)
> -               pp->ops->scan_bus(pp);
> -
> -       return bus;
> -}
> -
> -static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> -{
> -       struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
> -       int irq;
> -
> -       irq = of_irq_parse_and_map_pci(dev, slot, pin);
> -       if (!irq)
> -               irq = pp->irq;
> -
> -       return irq;
> -}
> -
> -static struct hw_pci dw_pci = {
> -       .setup          = dw_pcie_setup,
> -       .scan           = dw_pcie_scan_bus,
> -       .map_irq        = dw_pcie_map_irq,
> -};
> -
>  void dw_pcie_setup_rc(struct pcie_port *pp)
>  {
>         u32 val;
> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
> index d0bbd27..efac57d 100644
> --- a/drivers/pci/host/pcie-designware.h
> +++ b/drivers/pci/host/pcie-designware.h
> @@ -34,7 +34,7 @@ struct pcie_port {
>         u64                     cfg1_mod_base;
>         void __iomem            *va_cfg1_base;
>         u32                     cfg1_size;
> -       u64                     io_base;
> +       resource_size_t         io_base;
>         u64                     io_mod_base;
>         phys_addr_t             io_bus_addr;
>         u32                     io_size;
> @@ -42,10 +42,10 @@ struct pcie_port {
>         u64                     mem_mod_base;
>         phys_addr_t             mem_bus_addr;
>         u32                     mem_size;
> -       struct resource         cfg;
> -       struct resource         io;
> -       struct resource         mem;
> -       struct resource         busn;
> +       struct resource         *cfg;
> +       struct resource         *io;
> +       struct resource         *mem;
> +       struct resource         *busn;
>         int                     irq;
>         u32                     lanes;
>         struct pcie_host_ops    *ops;
> diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
> index c49fbdc..03eb204 100644
> --- a/drivers/pci/host/pcie-spear13xx.c
> +++ b/drivers/pci/host/pcie-spear13xx.c
> @@ -286,7 +286,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
>                 return ret;
>         }
> 
> -       pp->root_bus_nr = -1;
> +       pp->root_bus_nr = 0;
>         pp->ops = &spear13xx_pcie_host_ops;
> 
>         ret = dw_pcie_host_init(pp);
> --
> 1.9.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
> 
--
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
Zhou Wang July 30, 2015, 3:17 a.m. UTC | #3
On 2015/7/30 1:24, Lorenzo Pieralisi wrote:
> On Sat, Jul 25, 2015 at 04:21:23AM +0100, Zhou Wang wrote:
>> This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
>> function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
>> move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
>> each PCIe host driver which is based on pcie-designware. This patch also try
> 
> Memory for ports is kzalloc'ed, so there is no need to zero it. I still
> think that you should explain the root_bus_nr setting to 0 a bit
> better, why you make the change and why it is safe.
>

Hi Lorenzo,

This patch deletes dw_pcie_scan_bus and pass root_bus_nr directly to
pci_create_root_bus.

In past, we use:
pci_common_init_dev
	-> pcibios_init_hw
		-> hw->scan (dw_pcie_scan_bus)
to pass 0 to root_bus_nr in struct pcie_port. so I set root_bus_nr
in each driver which is based on dw to 0.

> [...]
> 
>> -int dw_pcie_host_init(struct pcie_port *pp)
>> +int __init dw_pcie_host_init(struct pcie_port *pp)
>>  {
>>         struct device_node *np = pp->dev->of_node;
>>         struct platform_device *pdev = to_platform_device(pp->dev);
>> -       struct of_pci_range range;
>> -       struct of_pci_range_parser parser;
>> +       struct pci_bus *bus;
>>         struct resource *cfg_res;
>> -       u32 val, na, ns;
>> +       LIST_HEAD(res);
>> +       u32 val, ns;
>>         const __be32 *addrp;
>>         int i, index, ret;
>> +       struct resource_entry *win;
>>
>> -       /* Find the address cell size and the number of cells in order to get
>> -        * the untranslated address.
>> -        */
>> -       of_property_read_u32(np, "#address-cells", &na);
>> +       /* Find the number of cells in order to get the untranslated address */
>>         ns = of_n_size_cells(np);
>>
>>         cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>> @@ -392,78 +382,62 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>                 dev_err(pp->dev, "missing *config* reg space\n");
>>         }
>>
>> -       if (of_pci_range_parser_init(&parser, np)) {
>> -               dev_err(pp->dev, "missing ranges property\n");
>> -               return -EINVAL;
>> -       }
>> +       ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
>> +       if (ret)
>> +               return ret;
>>
>>         /* Get the I/O and memory ranges from DT */
>> -       for_each_of_pci_range(&parser, &range) {
>> -               unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
>> -
>> -               if (restype == IORESOURCE_IO) {
>> -                       of_pci_range_to_resource(&range, np, &pp->io);
>> -                       pp->io.name = "I/O";
>> -                       pp->io.start = max_t(resource_size_t,
>> -                                            PCIBIOS_MIN_IO,
>> -                                            range.pci_addr + global_io_offset);
>> -                       pp->io.end = min_t(resource_size_t,
>> -                                          IO_SPACE_LIMIT,
>> -                                          range.pci_addr + range.size
>> -                                          + global_io_offset - 1);
>> -                       pp->io_size = resource_size(&pp->io);
>> -                       pp->io_bus_addr = range.pci_addr;
>> -                       pp->io_base = range.cpu_addr;
>> -
>> -                       /* Find the untranslated IO space address */
>> -                       pp->io_mod_base = of_read_number(parser.range -
>> -                                                        parser.np + na, ns);
>> -               }
>> -               if (restype == IORESOURCE_MEM) {
>> -                       of_pci_range_to_resource(&range, np, &pp->mem);
>> -                       pp->mem.name = "MEM";
>> -                       pp->mem_size = resource_size(&pp->mem);
>> -                       pp->mem_bus_addr = range.pci_addr;
>> -
>> -                       /* Find the untranslated MEM space address */
>> -                       pp->mem_mod_base = of_read_number(parser.range -
>> -                                                         parser.np + na, ns);
>> -               }
>> -               if (restype == 0) {
>> -                       of_pci_range_to_resource(&range, np, &pp->cfg);
>> -                       pp->cfg0_size = resource_size(&pp->cfg)/2;
>> -                       pp->cfg1_size = resource_size(&pp->cfg)/2;
>> -                       pp->cfg0_base = pp->cfg.start;
>> -                       pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
>> +       resource_list_for_each_entry(win, &res) {
>> +               switch (resource_type(win->res)) {
>> +               case IORESOURCE_IO:
>> +                       pp->io = win->res;
>> +                       pp->io->name = "I/O";
>> +                       pp->io_size = resource_size(pp->io);
>> +                       pp->io_bus_addr = pp->io->start - win->offset;
>> +                       pp->io_mod_base = win->__res.start;
>> +                       ret = pci_remap_iospace(pp->io, pp->io_base);
>> +                       if (ret) {
>> +                               dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
>> +                                        ret, pp->io);
>> +                               continue;
>> +                       }
>> +                       break;
>> +               case IORESOURCE_MEM:
>> +                       pp->mem = win->res;
>> +                       pp->mem->name = "MEM";
>> +                       pp->mem_size = resource_size(pp->mem);
>> +                       pp->mem_bus_addr = pp->mem->start - win->offset;
>> +                       pp->mem_mod_base = win->__res.start;
>> +                       break;
>> +               case 0:
>> +                       pp->cfg = win->res;
>> +                       pp->cfg0_size = resource_size(pp->cfg)/2;
>> +                       pp->cfg1_size = resource_size(pp->cfg)/2;
>> +                       pp->cfg0_base = pp->cfg->start;
>> +                       pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
>>
>>                         /* Find the untranslated configuration space address */
>> -                       pp->cfg0_mod_base = of_read_number(parser.range -
>> -                                                          parser.np + na, ns);
>> -                       pp->cfg1_mod_base = pp->cfg0_mod_base +
>> -                                           pp->cfg0_size;
>> +                       pp->cfg0_mod_base = win->__res.start;
>> +                       pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
>> +                       break;
>> +               case IORESOURCE_BUS:
>> +                       pp->busn = win->res;
>> +                       break;
>> +               default:
>> +                       continue;
>>                 }
>>         }
>>
>> -       ret = of_pci_parse_bus_range(np, &pp->busn);
>> -       if (ret < 0) {
>> -               pp->busn.name = np->name;
>> -               pp->busn.start = 0;
>> -               pp->busn.end = 0xff;
>> -               pp->busn.flags = IORESOURCE_BUS;
>> -               dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
>> -                       ret, &pp->busn);
>> -       }
>> -
>>         if (!pp->dbi_base) {
>> -               pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
>> -                                       resource_size(&pp->cfg));
>> +               pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
>> +                                       resource_size(pp->cfg));
>>                 if (!pp->dbi_base) {
>>                         dev_err(pp->dev, "error with ioremap\n");
>>                         return -ENOMEM;
>>                 }
>>         }
>>
>> -       pp->mem_base = pp->mem.start;
>> +       pp->mem_base = pp->mem->start;
>>
>>         if (!pp->va_cfg0_base) {
>>                 pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
>> @@ -524,15 +498,28 @@ int dw_pcie_host_init(struct pcie_port *pp)
>>         val |= PORT_LOGIC_SPEED_CHANGE;
>>         dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
>>
>> -#ifdef CONFIG_PCI_MSI
>> -       dw_pcie_msi_chip.dev = pp->dev;
> 
> Is it safe to remove the dev assignment ?
>

I am not sure about this. But from James' test of v3 series, it seems that
it is OK for i.MX 6Quad board at least.

Maybe we'd better to add this in next version.

>> -       dw_pci.msi_ctrl = &dw_pcie_msi_chip;
>> +       bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
>> +                             pp, &res);
>> +       if (!bus)
>> +               return -ENOMEM;
>> +
>> +#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
>> +       bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
>> +#else
>> +       bus->msi = &dw_pcie_msi_chip;
>>  #endif
> 
> For the records, this patch conflicts with my MSI series:
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/360461.html
> 
> Conflict resolution is easy but it is worth keeping in mind.
>

Thanks for reminding this.

Best Regards,
Zhou

> Thanks !
> Lorenzo
> 
>>
>> -       dw_pci.nr_controllers = 1;
>> -       dw_pci.private_data = (void **)&pp;
>> +       pci_scan_child_bus(bus);
>> +       if (pp->ops->scan_bus)
>> +               pp->ops->scan_bus(pp);
>>
>> -       pci_common_init_dev(pp->dev, &dw_pci);
>> +#ifdef CONFIG_ARM
>> +       /* support old dtbs that incorrectly describe IRQs */
>> +       pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
>> +#endif
>> +
>> +       pci_assign_unassigned_bus_resources(bus);
>> +       pci_bus_add_devices(bus);
>>
>>         return 0;
>>  }
>> @@ -633,7 +620,7 @@ static int dw_pcie_valid_config(struct pcie_port *pp,
>>  static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>>                         int size, u32 *val)
>>  {
>> -       struct pcie_port *pp = sys_to_pcie(bus->sysdata);
>> +       struct pcie_port *pp = bus->sysdata;
>>         int ret;
>>
>>         if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
>> @@ -657,7 +644,7 @@ static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
>>  static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
>>                         int where, int size, u32 val)
>>  {
>> -       struct pcie_port *pp = sys_to_pcie(bus->sysdata);
>> +       struct pcie_port *pp = bus->sysdata;
>>         int ret;
>>
>>         if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
>> @@ -681,62 +668,6 @@ static struct pci_ops dw_pcie_ops = {
>>         .write = dw_pcie_wr_conf,
>>  };
>>
>> -static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
>> -{
>> -       struct pcie_port *pp;
>> -
>> -       pp = sys_to_pcie(sys);
>> -
>> -       if (global_io_offset < SZ_1M && pp->io_size > 0) {
>> -               sys->io_offset = global_io_offset - pp->io_bus_addr;
>> -               pci_ioremap_io(global_io_offset, pp->io_base);
>> -               global_io_offset += SZ_64K;
>> -               pci_add_resource_offset(&sys->resources, &pp->io,
>> -                                       sys->io_offset);
>> -       }
>> -
>> -       sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
>> -       pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
>> -       pci_add_resource(&sys->resources, &pp->busn);
>> -
>> -       return 1;
>> -}
>> -
>> -static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
>> -{
>> -       struct pci_bus *bus;
>> -       struct pcie_port *pp = sys_to_pcie(sys);
>> -
>> -       pp->root_bus_nr = sys->busnr;
>> -       bus = pci_scan_root_bus(pp->dev, sys->busnr,
>> -                                 &dw_pcie_ops, sys, &sys->resources);
>> -       if (!bus)
>> -               return NULL;
>> -
>> -       if (bus && pp->ops->scan_bus)
>> -               pp->ops->scan_bus(pp);
>> -
>> -       return bus;
>> -}
>> -
>> -static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
>> -{
>> -       struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
>> -       int irq;
>> -
>> -       irq = of_irq_parse_and_map_pci(dev, slot, pin);
>> -       if (!irq)
>> -               irq = pp->irq;
>> -
>> -       return irq;
>> -}
>> -
>> -static struct hw_pci dw_pci = {
>> -       .setup          = dw_pcie_setup,
>> -       .scan           = dw_pcie_scan_bus,
>> -       .map_irq        = dw_pcie_map_irq,
>> -};
>> -
>>  void dw_pcie_setup_rc(struct pcie_port *pp)
>>  {
>>         u32 val;
>> diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
>> index d0bbd27..efac57d 100644
>> --- a/drivers/pci/host/pcie-designware.h
>> +++ b/drivers/pci/host/pcie-designware.h
>> @@ -34,7 +34,7 @@ struct pcie_port {
>>         u64                     cfg1_mod_base;
>>         void __iomem            *va_cfg1_base;
>>         u32                     cfg1_size;
>> -       u64                     io_base;
>> +       resource_size_t         io_base;
>>         u64                     io_mod_base;
>>         phys_addr_t             io_bus_addr;
>>         u32                     io_size;
>> @@ -42,10 +42,10 @@ struct pcie_port {
>>         u64                     mem_mod_base;
>>         phys_addr_t             mem_bus_addr;
>>         u32                     mem_size;
>> -       struct resource         cfg;
>> -       struct resource         io;
>> -       struct resource         mem;
>> -       struct resource         busn;
>> +       struct resource         *cfg;
>> +       struct resource         *io;
>> +       struct resource         *mem;
>> +       struct resource         *busn;
>>         int                     irq;
>>         u32                     lanes;
>>         struct pcie_host_ops    *ops;
>> diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
>> index c49fbdc..03eb204 100644
>> --- a/drivers/pci/host/pcie-spear13xx.c
>> +++ b/drivers/pci/host/pcie-spear13xx.c
>> @@ -286,7 +286,7 @@ static int spear13xx_add_pcie_port(struct pcie_port *pp,
>>                 return ret;
>>         }
>>
>> -       pp->root_bus_nr = -1;
>> +       pp->root_bus_nr = 0;
>>         pp->ops = &spear13xx_pcie_host_ops;
>>
>>         ret = dw_pcie_host_init(pp);
>> --
>> 1.9.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
>>
> 
> .
> 


--
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
James Morse Aug. 4, 2015, 9:34 a.m. UTC | #4
On 28/07/15 07:21, Zhou Wang wrote:
> On 2015/7/25 11:21, Zhou Wang wrote:
>> This patch tries to unify ARM32 and ARM64 PCIe in designware driver. Delete
>> function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct hw_pci,
>> move related operations to dw_pcie_host_init. Also set pp->root_bus_nr = 0 in
>> each PCIe host driver which is based on pcie-designware. This patch also try
>> to use of_pci_get_host_bridge_resources for ARM32 and ARM64 according to the
>> suggestion for Gabriele[1]
>>
>> This patch is based on Gabriele's patch about of_pci_range fix[2]
>>
>> I have compiled the driver with multi_v7_defconfig. However, I don't have
>> ARM32 PCIe related board to do test. It will be appreciated if someone could
>> help to test it.
>>
> 
> Hi James,
> 
> If you have time, could you help to test this patch on i.MX 6Quad board?
> You need apply Gabriele's patch before applying this patch.
> 
> It will be very appreciate and helpful if we can get test result from you.

Applying patches 1 and 2, from v5, onto 4.2-rc5, I still get the same
problem as before: config cycles to enumerate the second bus aren't
working. (good news - I have a workaround)

Output from dmesg below, the lines 'dw_pcie_cfg_read(0xf0180000, 0x0, 0x4,
=0x8878086)' were added by me, 0x8878086 is the intel wireless card
attached to the board.

From v4.2-rc5:
----------------------------------------------------------------------------------
imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [io 0x1000-0xffff]
pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
pci_bus 0000:00: root bus resource [bus 00-ff]
pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
pci 0000:00:00.0: IOMMU is currently not supported for PCI
pci 0000:00:00.0: supports D1
pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
PCI: bus0: Fast back to back transfers disabled
dw_pcie_cfg_read(0xf0180000, 0x0, 0x4, =0x8878086)
pci 0000:01:00.0: [8086:0887] type 00 class 0x028000
dw_pcie_cfg_read(0xf0180000, 0x0, 0x4, =0x8878086)
pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00001fff 64bit]
pci 0000:01:00.0: IOMMU is currently not supported for PCI
pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
PCI: bus1: Fast back to back transfers disabled
pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
pci 0000:00:00.0: BAR 6: assigned [mem 0x01200000-0x0120ffff pref
]
pci 0000:01:00.0: BAR 0: assigned [mem 0x01100000-0x01101fff 64bi
t]
pci 0000:00:00.0: PCI bridge to [bus 01]
pci 0000:00:00.0:bridge window [mem 0x01100000-0x011fffff]
pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
----------------------------------------------------------------------------------

And then with your two patches:
----------------------------------------------------------------------------------
PCI host bridge /soc/pcie@0x01000000 ranges:
No bus range found for /soc/pcie@0x01000000, using [bus 00-ff]
err 0x01f00000..0x01f7ffff -> 0x01f00000
IO 0x01f80000..0x01f8ffff -> 0x00000000
MEM 0x01000000..0x01efffff -> 0x01000000
imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
pci_bus 0000:00: root bus resource [bus 00-ff]
pci_bus 0000:00: root bus resource [??? 0x01f00000-0x01f7ffff fla
gs 0x0]
pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
pci 0000:00:00.0: IOMMU is currently not supported for PCI
pci 0000:00:00.0: supports D1
pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
PCI: bus0: Fast back to back transfers disabled
dw_pcie_cfg_read(0xf0180000, 0x0, 0x4, =0x0)
PCI: bus1: Fast back to back transfers enabled
pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
pci 0000:00:00.0: BAR 6: assigned [mem 0x01100000-0x0110ffff pref
]
pci 0000:00:00.0: PCI bridge to [bus 01]
pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
----------------------------------------------------------------------------------

Root-cause appears to be that the designware driver relies on ATU for
config and IO accesses. dw_pcie_rd_other_conf() does the appropriate magic,
but with your patches 'pp->cfg0_base' is NULL, despite being correctly
initialised in dw_pcie_host_init().

dw_pcie_host_init() initialises the pp->cfg* values correctly after its call:
>  platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");

But the new code introduced by patch 2 then walks the whole resource_list
and re-initialises the pp->cfg* values. The fault occurs at:
> /* Find the untranslated configuration space address */
> pp->cfg0_mod_base = win->__res.start

where win->__res is uninitialised. The comment in linux/resource_ext.h says
this is the 'default storage for res', so its not valid to assume it
contains different values to win->res. (in this case, it contains no useful
values).

The workaround is to remove the re-initialisation of the pp->cfg* values,
as they were already correctly initialised earlier. However, other resource
types are accessing __res directly ... which is probably not correct.

I need to read-up on what these 'untranslated' addresses are for...



James


--
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
Gabriele Paoloni Aug. 4, 2015, 10:23 a.m. UTC | #5
Hi James

Please see "[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range"

I think if you apply this patch your problem should be solved...

If you follow the discussion you see that this patch is going to be part 
of the next designware patchset...

Wang Zhou said "You need apply Gabriele's patch before applying this patch."
but he didn't specify which one and obviously this patch should have been part
of the patch-set

Sorry for the confusion 

Gab

> -----Original Message-----
> From: James Morse [mailto:james.morse@arm.com]
> Sent: Tuesday, August 04, 2015 10:35 AM
> To: Wangzhou (B)
> Cc: Bjorn Helgaas; Jingoo Han; Pratyush Anand; Arnd Bergmann; Gabriele
> Paoloni; Lorenzo Pieralisi; Liviu Dudau; thomas.petazzoni@free-
> electrons.com; Jason Cooper; robh@kernel.org; linux-pci@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org; devicetree@vger.kernel.org;
> Yuanzhichang; Zhudacai; zhangjukuo; qiuzhenfa; liudongdong (C);
> qiujiang; Kangfenglong; Liguozhu (Kenneth)
> Subject: Re: [PATCH v5 2/5] PCI: designware: Add ARM64 support
> 
> On 28/07/15 07:21, Zhou Wang wrote:
> > On 2015/7/25 11:21, Zhou Wang wrote:
> >> This patch tries to unify ARM32 and ARM64 PCIe in designware driver.
> Delete
> >> function dw_pcie_setup, dw_pcie_scan_bus, dw_pcie_map_irq and struct
> hw_pci,
> >> move related operations to dw_pcie_host_init. Also set pp-
> >root_bus_nr = 0 in
> >> each PCIe host driver which is based on pcie-designware. This patch
> also try
> >> to use of_pci_get_host_bridge_resources for ARM32 and ARM64
> according to the
> >> suggestion for Gabriele[1]
> >>
> >> This patch is based on Gabriele's patch about of_pci_range fix[2]
> >>
> >> I have compiled the driver with multi_v7_defconfig. However, I don't
> have
> >> ARM32 PCIe related board to do test. It will be appreciated if
> someone could
> >> help to test it.
> >>
> >
> > Hi James,
> >
> > If you have time, could you help to test this patch on i.MX 6Quad
> board?
> > You need apply Gabriele's patch before applying this patch.
> >
> > It will be very appreciate and helpful if we can get test result from
> you.
> 
> Applying patches 1 and 2, from v5, onto 4.2-rc5, I still get the same
> problem as before: config cycles to enumerate the second bus aren't
> working. (good news - I have a workaround)
> 
> Output from dmesg below, the lines 'dw_pcie_cfg_read(0xf0180000, 0x0,
> 0x4,
> =0x8878086)' were added by me, 0x8878086 is the intel wireless card
> attached to the board.
> 
> From v4.2-rc5:
> -----------------------------------------------------------------------
> -----------
> imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [io 0x1000-0xffff]
> pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
> pci_bus 0000:00: root bus resource [bus 00-ff]
> pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
> pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
> pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
> pci 0000:00:00.0: IOMMU is currently not supported for PCI
> pci 0000:00:00.0: supports D1
> pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
> PCI: bus0: Fast back to back transfers disabled
> dw_pcie_cfg_read(0xf0180000, 0x0, 0x4, =0x8878086)
> pci 0000:01:00.0: [8086:0887] type 00 class 0x028000
> dw_pcie_cfg_read(0xf0180000, 0x0, 0x4, =0x8878086)
> pci 0000:01:00.0: reg 0x10: [mem 0x00000000-0x00001fff 64bit]
> pci 0000:01:00.0: IOMMU is currently not supported for PCI
> pci 0000:01:00.0: PME# supported from D0 D3hot D3cold
> PCI: bus1: Fast back to back transfers disabled
> pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
> pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
> pci 0000:00:00.0: BAR 8: assigned [mem 0x01100000-0x011fffff]
> pci 0000:00:00.0: BAR 6: assigned [mem 0x01200000-0x0120ffff pref
> ]
> pci 0000:01:00.0: BAR 0: assigned [mem 0x01100000-0x01101fff 64bi
> t]
> pci 0000:00:00.0: PCI bridge to [bus 01]
> pci 0000:00:00.0:bridge window [mem 0x01100000-0x011fffff]
> pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
> pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
> pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
> -----------------------------------------------------------------------
> -----------
> 
> And then with your two patches:
> -----------------------------------------------------------------------
> -----------
> PCI host bridge /soc/pcie@0x01000000 ranges:
> No bus range found for /soc/pcie@0x01000000, using [bus 00-ff]
> err 0x01f00000..0x01f7ffff -> 0x01f00000
> IO 0x01f80000..0x01f8ffff -> 0x00000000
> MEM 0x01000000..0x01efffff -> 0x01000000
> imx6q-pcie 1ffc000.pcie: PCI host bridge to bus 0000:00
> pci_bus 0000:00: root bus resource [bus 00-ff]
> pci_bus 0000:00: root bus resource [??? 0x01f00000-0x01f7ffff fla
> gs 0x0]
> pci_bus 0000:00: root bus resource [io 0x0000-0xffff]
> pci_bus 0000:00: root bus resource [mem 0x01000000-0x01efffff]
> pci 0000:00:00.0: [16c3:abcd] type 01 class 0x060400
> pci 0000:00:00.0: reg 0x10: [mem 0x00000000-0x000fffff]
> pci 0000:00:00.0: reg 0x38: [mem 0x00000000-0x0000ffff pref]
> pci 0000:00:00.0: IOMMU is currently not supported for PCI
> pci 0000:00:00.0: supports D1
> pci 0000:00:00.0: PME# supported from D0 D1 D3hot D3cold
> PCI: bus0: Fast back to back transfers disabled
> dw_pcie_cfg_read(0xf0180000, 0x0, 0x4, =0x0)
> PCI: bus1: Fast back to back transfers enabled
> pci 0000:00:00.0: BAR 0: assigned [mem 0x01000000-0x010fffff]
> pci 0000:00:00.0: BAR 6: assigned [mem 0x01100000-0x0110ffff pref
> ]
> pci 0000:00:00.0: PCI bridge to [bus 01]
> pcieport 0000:00:00.0: Signaling PME through PCIe PME interrupt
> pcie_pme 0000:00:00.0:pcie01: service driver pcie_pme loaded
> -----------------------------------------------------------------------
> -----------
> 
> Root-cause appears to be that the designware driver relies on ATU for
> config and IO accesses. dw_pcie_rd_other_conf() does the appropriate
> magic,
> but with your patches 'pp->cfg0_base' is NULL, despite being correctly
> initialised in dw_pcie_host_init().
> 
> dw_pcie_host_init() initialises the pp->cfg* values correctly after its
> call:
> >  platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
> 
> But the new code introduced by patch 2 then walks the whole
> resource_list
> and re-initialises the pp->cfg* values. The fault occurs at:
> > /* Find the untranslated configuration space address */
> > pp->cfg0_mod_base = win->__res.start
> 
> where win->__res is uninitialised. The comment in linux/resource_ext.h
> says
> this is the 'default storage for res', so its not valid to assume it
> contains different values to win->res. (in this case, it contains no
> useful
> values).
> 
> The workaround is to remove the re-initialisation of the pp->cfg*
> values,
> as they were already correctly initialised earlier. However, other
> resource
> types are accessing __res directly ... which is probably not correct.
> 
> I need to read-up on what these 'untranslated' addresses are for...
> 
> 
> 
> James
> 

--
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
James Morse Aug. 4, 2015, 10:40 a.m. UTC | #6
On 04/08/15 11:23, Gabriele Paoloni wrote:
> Hi James
> 
> Please see "[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range"
> 
> I think if you apply this patch your problem should be solved...
> 
> If you follow the discussion you see that this patch is going to be part 
> of the next designware patchset...

Yes I just spotted that series after continuing through my email backlog.

> Wang Zhou said "You need apply Gabriele's patch before applying this patch."
> but he didn't specify which one and obviously this patch should have been part
> of the patch-set

I assumed he meant your patch 1 in the same series, (given the reply was to
patch 2).

With the '[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range'
applied before patches and 1 and 2 of this series - pcie on the
'Freescale i.MX6 Quad SABRE Lite Board' works fine.

I can rescan the bus, load firmware, list nearby APs, and even get MSIs
coming from the card.

Tested-by: James Morse <james.morse@arm.com>



James
--
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
Gabriele Paoloni Aug. 4, 2015, 10:43 a.m. UTC | #7
> -----Original Message-----
> From: James Morse [mailto:james.morse@arm.com]
> Sent: Tuesday, August 04, 2015 11:40 AM
> To: Gabriele Paoloni; Wangzhou (B)
> Cc: Bjorn Helgaas; Jingoo Han; Pratyush Anand; Arnd Bergmann; Lorenzo
> Pieralisi; Liviu Dudau; thomas.petazzoni@free-electrons.com; Jason
> Cooper; robh@kernel.org; linux-pci@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; Yuanzhichang;
> Zhudacai; zhangjukuo; qiuzhenfa; liudongdong (C); qiujiang;
> Kangfenglong; Liguozhu (Kenneth)
> Subject: Re: [PATCH v5 2/5] PCI: designware: Add ARM64 support
> 
> On 04/08/15 11:23, Gabriele Paoloni wrote:
> > Hi James
> >
> > Please see "[PATCH v6] PCI: Store PCIe bus address in struct
> of_pci_range"
> >
> > I think if you apply this patch your problem should be solved...
> >
> > If you follow the discussion you see that this patch is going to be
> part
> > of the next designware patchset...
> 
> Yes I just spotted that series after continuing through my email
> backlog.
> 
> > Wang Zhou said "You need apply Gabriele's patch before applying this
> patch."
> > but he didn't specify which one and obviously this patch should have
> been part
> > of the patch-set
> 
> I assumed he meant your patch 1 in the same series, (given the reply
> was to
> patch 2).
> 
> With the '[PATCH v6] PCI: Store PCIe bus address in struct
> of_pci_range'
> applied before patches and 1 and 2 of this series - pcie on the
> 'Freescale i.MX6 Quad SABRE Lite Board' works fine.
> 
> I can rescan the bus, load firmware, list nearby APs, and even get MSIs
> coming from the card.

This is Great NEWS! Thanks a Lot!

> 
> Tested-by: James Morse <james.morse@arm.com>
> 
> 
> 
> James
--
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
Zhou Wang Aug. 5, 2015, 1:40 a.m. UTC | #8
On 2015/8/4 18:40, James Morse wrote:
> On 04/08/15 11:23, Gabriele Paoloni wrote:
>> Hi James
>>
>> Please see "[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range"
>>
>> I think if you apply this patch your problem should be solved...
>>
>> If you follow the discussion you see that this patch is going to be part 
>> of the next designware patchset...
> 
> Yes I just spotted that series after continuing through my email backlog.
> 
>> Wang Zhou said "You need apply Gabriele's patch before applying this patch."
>> but he didn't specify which one and obviously this patch should have been part
>> of the patch-set
> 
> I assumed he meant your patch 1 in the same series, (given the reply was to
> patch 2).
> 
> With the '[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range'
> applied before patches and 1 and 2 of this series - pcie on the
> 'Freescale i.MX6 Quad SABRE Lite Board' works fine.
> 
> I can rescan the bus, load firmware, list nearby APs, and even get MSIs
> coming from the card.
> 
> Tested-by: James Morse <james.morse@arm.com>
> 
> 
> 
> James
> 
> .
>

Hi James,

Many thanks for your help to test!

I need apologize for not giving a clear message. What I mean is applying
Gab's '[PATCH v6] PCI: Store PCIe bus address in struct of_pci_range'.
I am very sorry for wasting your time :(

Above Gab's patch will be merged in my next series. Hope that will make
thing clearer.

It is good that these patches work fine on your board! Hope we can get
more tests on other ARM32 boards.

Thanks again,
Zhou

.

--
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 mbox

Patch

diff --git a/drivers/pci/host/pci-dra7xx.c b/drivers/pci/host/pci-dra7xx.c
index 80db09e..69364e8 100644
--- a/drivers/pci/host/pci-dra7xx.c
+++ b/drivers/pci/host/pci-dra7xx.c
@@ -275,6 +275,7 @@  static int __init dra7xx_add_pcie_port(struct dra7xx_pcie *dra7xx,
 
 	pp = &dra7xx->pp;
 	pp->dev = dev;
+	pp->root_bus_nr = 0;
 	pp->ops = &dra7xx_pcie_host_ops;
 
 	pp->irq = platform_get_irq(pdev, 1);
diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c
index f9f468d..9771bb0 100644
--- a/drivers/pci/host/pci-exynos.c
+++ b/drivers/pci/host/pci-exynos.c
@@ -530,7 +530,7 @@  static int __init exynos_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &exynos_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-imx6.c b/drivers/pci/host/pci-imx6.c
index 233a196..bec256c 100644
--- a/drivers/pci/host/pci-imx6.c
+++ b/drivers/pci/host/pci-imx6.c
@@ -551,7 +551,7 @@  static int __init imx6_add_pcie_port(struct pcie_port *pp,
 		}
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &imx6_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pci-keystone-dw.c b/drivers/pci/host/pci-keystone-dw.c
index f34892e..b1e4135 100644
--- a/drivers/pci/host/pci-keystone-dw.c
+++ b/drivers/pci/host/pci-keystone-dw.c
@@ -327,7 +327,7 @@  static void ks_dw_pcie_clear_dbi_mode(void __iomem *reg_virt)
 void ks_dw_pcie_setup_rc_app_regs(struct keystone_pcie *ks_pcie)
 {
 	struct pcie_port *pp = &ks_pcie->pp;
-	u32 start = pp->mem.start, end = pp->mem.end;
+	u32 start = pp->mem->start, end = pp->mem->end;
 	int i, tr_size;
 
 	/* Disable BARs for inbound access */
diff --git a/drivers/pci/host/pci-keystone.c b/drivers/pci/host/pci-keystone.c
index 734da58..8113832 100644
--- a/drivers/pci/host/pci-keystone.c
+++ b/drivers/pci/host/pci-keystone.c
@@ -309,7 +309,7 @@  static int __init ks_add_pcie_port(struct keystone_pcie *ks_pcie,
 			return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &keystone_pcie_host_ops;
 	ret = ks_dw_pcie_host_init(ks_pcie, ks_pcie->msi_intc_np);
 	if (ret) {
diff --git a/drivers/pci/host/pci-layerscape.c b/drivers/pci/host/pci-layerscape.c
index b2328ea1..79ff08c 100644
--- a/drivers/pci/host/pci-layerscape.c
+++ b/drivers/pci/host/pci-layerscape.c
@@ -106,7 +106,7 @@  static int ls_add_pcie_port(struct ls_pcie *pcie)
 	pp = &pcie->pp;
 	pp->dev = pcie->dev;
 	pp->dbi_base = pcie->dbi;
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &ls_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);
diff --git a/drivers/pci/host/pcie-designware.c b/drivers/pci/host/pcie-designware.c
index 69486be..6092c84 100644
--- a/drivers/pci/host/pcie-designware.c
+++ b/drivers/pci/host/pcie-designware.c
@@ -11,6 +11,7 @@ 
  * published by the Free Software Foundation.
  */
 
+#include <linux/hardirq.h>
 #include <linux/irq.h>
 #include <linux/irqdomain.h>
 #include <linux/kernel.h>
@@ -69,16 +70,7 @@ 
 #define PCIE_ATU_FUNC(x)		(((x) & 0x7) << 16)
 #define PCIE_ATU_UPPER_TARGET		0x91C
 
-static struct hw_pci dw_pci;
-
-static unsigned long global_io_offset;
-
-static inline struct pcie_port *sys_to_pcie(struct pci_sys_data *sys)
-{
-	BUG_ON(!sys->private_data);
-
-	return sys->private_data;
-}
+static struct pci_ops dw_pcie_ops;
 
 int dw_pcie_cfg_read(void __iomem *addr, int where, int size, u32 *val)
 {
@@ -255,7 +247,7 @@  static void dw_pcie_msi_set_irq(struct pcie_port *pp, int irq)
 static int assign_irq(int no_irqs, struct msi_desc *desc, int *pos)
 {
 	int irq, pos0, i;
-	struct pcie_port *pp = sys_to_pcie(desc->dev->bus->sysdata);
+	struct pcie_port *pp = desc->dev->bus->sysdata;
 
 	pos0 = bitmap_find_free_region(pp->msi_irq_in_use, MAX_MSI_IRQS,
 				       order_base_2(no_irqs));
@@ -298,7 +290,7 @@  static int dw_msi_setup_irq(struct msi_controller *chip, struct pci_dev *pdev,
 {
 	int irq, pos;
 	struct msi_msg msg;
-	struct pcie_port *pp = sys_to_pcie(pdev->bus->sysdata);
+	struct pcie_port *pp = pdev->bus->sysdata;
 
 	if (desc->msi_attrib.is_msix)
 		return -EINVAL;
@@ -327,7 +319,7 @@  static void dw_msi_teardown_irq(struct msi_controller *chip, unsigned int irq)
 {
 	struct irq_data *data = irq_get_irq_data(irq);
 	struct msi_desc *msi = irq_data_get_msi(data);
-	struct pcie_port *pp = sys_to_pcie(msi->dev->bus->sysdata);
+	struct pcie_port *pp = msi->dev->bus->sysdata;
 
 	clear_irq_range(pp, irq, 1, data->hwirq);
 }
@@ -359,21 +351,19 @@  static const struct irq_domain_ops msi_domain_ops = {
 	.map = dw_pcie_msi_map,
 };
 
-int dw_pcie_host_init(struct pcie_port *pp)
+int __init dw_pcie_host_init(struct pcie_port *pp)
 {
 	struct device_node *np = pp->dev->of_node;
 	struct platform_device *pdev = to_platform_device(pp->dev);
-	struct of_pci_range range;
-	struct of_pci_range_parser parser;
+	struct pci_bus *bus;
 	struct resource *cfg_res;
-	u32 val, na, ns;
+	LIST_HEAD(res);
+	u32 val, ns;
 	const __be32 *addrp;
 	int i, index, ret;
+	struct resource_entry *win;
 
-	/* Find the address cell size and the number of cells in order to get
-	 * the untranslated address.
-	 */
-	of_property_read_u32(np, "#address-cells", &na);
+	/* Find the number of cells in order to get the untranslated address */
 	ns = of_n_size_cells(np);
 
 	cfg_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
@@ -392,78 +382,62 @@  int dw_pcie_host_init(struct pcie_port *pp)
 		dev_err(pp->dev, "missing *config* reg space\n");
 	}
 
-	if (of_pci_range_parser_init(&parser, np)) {
-		dev_err(pp->dev, "missing ranges property\n");
-		return -EINVAL;
-	}
+	ret = of_pci_get_host_bridge_resources(np, 0, 0xff, &res, &pp->io_base);
+	if (ret)
+		return ret;
 
 	/* Get the I/O and memory ranges from DT */
-	for_each_of_pci_range(&parser, &range) {
-		unsigned long restype = range.flags & IORESOURCE_TYPE_BITS;
-
-		if (restype == IORESOURCE_IO) {
-			of_pci_range_to_resource(&range, np, &pp->io);
-			pp->io.name = "I/O";
-			pp->io.start = max_t(resource_size_t,
-					     PCIBIOS_MIN_IO,
-					     range.pci_addr + global_io_offset);
-			pp->io.end = min_t(resource_size_t,
-					   IO_SPACE_LIMIT,
-					   range.pci_addr + range.size
-					   + global_io_offset - 1);
-			pp->io_size = resource_size(&pp->io);
-			pp->io_bus_addr = range.pci_addr;
-			pp->io_base = range.cpu_addr;
-
-			/* Find the untranslated IO space address */
-			pp->io_mod_base = of_read_number(parser.range -
-							 parser.np + na, ns);
-		}
-		if (restype == IORESOURCE_MEM) {
-			of_pci_range_to_resource(&range, np, &pp->mem);
-			pp->mem.name = "MEM";
-			pp->mem_size = resource_size(&pp->mem);
-			pp->mem_bus_addr = range.pci_addr;
-
-			/* Find the untranslated MEM space address */
-			pp->mem_mod_base = of_read_number(parser.range -
-							  parser.np + na, ns);
-		}
-		if (restype == 0) {
-			of_pci_range_to_resource(&range, np, &pp->cfg);
-			pp->cfg0_size = resource_size(&pp->cfg)/2;
-			pp->cfg1_size = resource_size(&pp->cfg)/2;
-			pp->cfg0_base = pp->cfg.start;
-			pp->cfg1_base = pp->cfg.start + pp->cfg0_size;
+	resource_list_for_each_entry(win, &res) {
+		switch (resource_type(win->res)) {
+		case IORESOURCE_IO:
+			pp->io = win->res;
+			pp->io->name = "I/O";
+			pp->io_size = resource_size(pp->io);
+			pp->io_bus_addr = pp->io->start - win->offset;
+			pp->io_mod_base = win->__res.start;
+			ret = pci_remap_iospace(pp->io, pp->io_base);
+			if (ret) {
+				dev_warn(pp->dev, "error %d: failed to map resource %pR\n",
+					 ret, pp->io);
+				continue;
+			}
+			break;
+		case IORESOURCE_MEM:
+			pp->mem = win->res;
+			pp->mem->name = "MEM";
+			pp->mem_size = resource_size(pp->mem);
+			pp->mem_bus_addr = pp->mem->start - win->offset;
+			pp->mem_mod_base = win->__res.start;
+			break;
+		case 0:
+			pp->cfg = win->res;
+			pp->cfg0_size = resource_size(pp->cfg)/2;
+			pp->cfg1_size = resource_size(pp->cfg)/2;
+			pp->cfg0_base = pp->cfg->start;
+			pp->cfg1_base = pp->cfg->start + pp->cfg0_size;
 
 			/* Find the untranslated configuration space address */
-			pp->cfg0_mod_base = of_read_number(parser.range -
-							   parser.np + na, ns);
-			pp->cfg1_mod_base = pp->cfg0_mod_base +
-					    pp->cfg0_size;
+			pp->cfg0_mod_base = win->__res.start;
+			pp->cfg1_mod_base = pp->cfg0_mod_base + pp->cfg0_size;
+			break;
+		case IORESOURCE_BUS:
+			pp->busn = win->res;
+			break;
+		default:
+			continue;
 		}
 	}
 
-	ret = of_pci_parse_bus_range(np, &pp->busn);
-	if (ret < 0) {
-		pp->busn.name = np->name;
-		pp->busn.start = 0;
-		pp->busn.end = 0xff;
-		pp->busn.flags = IORESOURCE_BUS;
-		dev_dbg(pp->dev, "failed to parse bus-range property: %d, using default %pR\n",
-			ret, &pp->busn);
-	}
-
 	if (!pp->dbi_base) {
-		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg.start,
-					resource_size(&pp->cfg));
+		pp->dbi_base = devm_ioremap(pp->dev, pp->cfg->start,
+					resource_size(pp->cfg));
 		if (!pp->dbi_base) {
 			dev_err(pp->dev, "error with ioremap\n");
 			return -ENOMEM;
 		}
 	}
 
-	pp->mem_base = pp->mem.start;
+	pp->mem_base = pp->mem->start;
 
 	if (!pp->va_cfg0_base) {
 		pp->va_cfg0_base = devm_ioremap(pp->dev, pp->cfg0_base,
@@ -524,15 +498,28 @@  int dw_pcie_host_init(struct pcie_port *pp)
 	val |= PORT_LOGIC_SPEED_CHANGE;
 	dw_pcie_wr_own_conf(pp, PCIE_LINK_WIDTH_SPEED_CONTROL, 4, val);
 
-#ifdef CONFIG_PCI_MSI
-	dw_pcie_msi_chip.dev = pp->dev;
-	dw_pci.msi_ctrl = &dw_pcie_msi_chip;
+	bus = pci_create_root_bus(pp->dev, pp->root_bus_nr, &dw_pcie_ops,
+			      pp, &res);
+	if (!bus)
+		return -ENOMEM;
+
+#ifdef CONFIG_GENERIC_MSI_IRQ_DOMAIN
+	bus->msi = container_of(&pp->irq_domain, struct msi_controller, domain);
+#else
+	bus->msi = &dw_pcie_msi_chip;
 #endif
 
-	dw_pci.nr_controllers = 1;
-	dw_pci.private_data = (void **)&pp;
+	pci_scan_child_bus(bus);
+	if (pp->ops->scan_bus)
+		pp->ops->scan_bus(pp);
 
-	pci_common_init_dev(pp->dev, &dw_pci);
+#ifdef CONFIG_ARM
+	/* support old dtbs that incorrectly describe IRQs */
+	pci_fixup_irqs(pci_common_swizzle, of_irq_parse_and_map_pci);
+#endif
+
+	pci_assign_unassigned_bus_resources(bus);
+	pci_bus_add_devices(bus);
 
 	return 0;
 }
@@ -633,7 +620,7 @@  static int dw_pcie_valid_config(struct pcie_port *pp,
 static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 			int size, u32 *val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0) {
@@ -657,7 +644,7 @@  static int dw_pcie_rd_conf(struct pci_bus *bus, u32 devfn, int where,
 static int dw_pcie_wr_conf(struct pci_bus *bus, u32 devfn,
 			int where, int size, u32 val)
 {
-	struct pcie_port *pp = sys_to_pcie(bus->sysdata);
+	struct pcie_port *pp = bus->sysdata;
 	int ret;
 
 	if (dw_pcie_valid_config(pp, bus, PCI_SLOT(devfn)) == 0)
@@ -681,62 +668,6 @@  static struct pci_ops dw_pcie_ops = {
 	.write = dw_pcie_wr_conf,
 };
 
-static int dw_pcie_setup(int nr, struct pci_sys_data *sys)
-{
-	struct pcie_port *pp;
-
-	pp = sys_to_pcie(sys);
-
-	if (global_io_offset < SZ_1M && pp->io_size > 0) {
-		sys->io_offset = global_io_offset - pp->io_bus_addr;
-		pci_ioremap_io(global_io_offset, pp->io_base);
-		global_io_offset += SZ_64K;
-		pci_add_resource_offset(&sys->resources, &pp->io,
-					sys->io_offset);
-	}
-
-	sys->mem_offset = pp->mem.start - pp->mem_bus_addr;
-	pci_add_resource_offset(&sys->resources, &pp->mem, sys->mem_offset);
-	pci_add_resource(&sys->resources, &pp->busn);
-
-	return 1;
-}
-
-static struct pci_bus *dw_pcie_scan_bus(int nr, struct pci_sys_data *sys)
-{
-	struct pci_bus *bus;
-	struct pcie_port *pp = sys_to_pcie(sys);
-
-	pp->root_bus_nr = sys->busnr;
-	bus = pci_scan_root_bus(pp->dev, sys->busnr,
-				  &dw_pcie_ops, sys, &sys->resources);
-	if (!bus)
-		return NULL;
-
-	if (bus && pp->ops->scan_bus)
-		pp->ops->scan_bus(pp);
-
-	return bus;
-}
-
-static int dw_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
-{
-	struct pcie_port *pp = sys_to_pcie(dev->bus->sysdata);
-	int irq;
-
-	irq = of_irq_parse_and_map_pci(dev, slot, pin);
-	if (!irq)
-		irq = pp->irq;
-
-	return irq;
-}
-
-static struct hw_pci dw_pci = {
-	.setup		= dw_pcie_setup,
-	.scan		= dw_pcie_scan_bus,
-	.map_irq	= dw_pcie_map_irq,
-};
-
 void dw_pcie_setup_rc(struct pcie_port *pp)
 {
 	u32 val;
diff --git a/drivers/pci/host/pcie-designware.h b/drivers/pci/host/pcie-designware.h
index d0bbd27..efac57d 100644
--- a/drivers/pci/host/pcie-designware.h
+++ b/drivers/pci/host/pcie-designware.h
@@ -34,7 +34,7 @@  struct pcie_port {
 	u64			cfg1_mod_base;
 	void __iomem		*va_cfg1_base;
 	u32			cfg1_size;
-	u64			io_base;
+	resource_size_t		io_base;
 	u64			io_mod_base;
 	phys_addr_t		io_bus_addr;
 	u32			io_size;
@@ -42,10 +42,10 @@  struct pcie_port {
 	u64			mem_mod_base;
 	phys_addr_t		mem_bus_addr;
 	u32			mem_size;
-	struct resource		cfg;
-	struct resource		io;
-	struct resource		mem;
-	struct resource		busn;
+	struct resource		*cfg;
+	struct resource		*io;
+	struct resource		*mem;
+	struct resource		*busn;
 	int			irq;
 	u32			lanes;
 	struct pcie_host_ops	*ops;
diff --git a/drivers/pci/host/pcie-spear13xx.c b/drivers/pci/host/pcie-spear13xx.c
index c49fbdc..03eb204 100644
--- a/drivers/pci/host/pcie-spear13xx.c
+++ b/drivers/pci/host/pcie-spear13xx.c
@@ -286,7 +286,7 @@  static int spear13xx_add_pcie_port(struct pcie_port *pp,
 		return ret;
 	}
 
-	pp->root_bus_nr = -1;
+	pp->root_bus_nr = 0;
 	pp->ops = &spear13xx_pcie_host_ops;
 
 	ret = dw_pcie_host_init(pp);