diff mbox

pci: generic host: make it more generic

Message ID 1415702040-2790-1-git-send-email-ming.lei@canonical.com
State Changes Requested
Headers show

Commit Message

Ming Lei Nov. 11, 2014, 10:33 a.m. UTC
This patch converts the generic host controller driver
into a more generic one, and basically don't need
platform's pcibios support, and use DT based generic
APIs to parse resource and remap IO port.

This patch has been tested on both ARMv7 and ARMv8
VM, and in theroy it should support other ARCHs too.

With this patch, virtio PCI block, network and scsi devices
can work well on ARMv7 and ARMv8 VM.

QEMU needs below patches for runing the test:

- Rob Herring's "Add generic PCI host device" patches
	http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
	http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03483.html
- Alvise Rigo's "Add Generic PCI host device update"
	http://marc.info/?l=qemu-devel&m=140506329920172&w=2

For ARMv8, cpu model of "host" and "cortex-a57" is missed
in Alvise Rigo's patchset, otherwise ARM64 VM can't boot.

All these QEMU patches can be found in below tree:

	http://kernel.ubuntu.com/git?p=ming/qemu.git;a=shortlog;h=refs/heads/arm64-pci-test

Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 drivers/pci/host/Kconfig            |    2 +-
 drivers/pci/host/pci-host-generic.c |  196 +++++++++++------------------------
 2 files changed, 63 insertions(+), 135 deletions(-)

Comments

Lorenzo Pieralisi Nov. 11, 2014, 12:29 p.m. UTC | #1
On Tue, Nov 11, 2014 at 10:33:59AM +0000, Ming Lei wrote:
> This patch converts the generic host controller driver
> into a more generic one, and basically don't need
> platform's pcibios support, and use DT based generic
> APIs to parse resource and remap IO port.
> 
> This patch has been tested on both ARMv7 and ARMv8
> VM, and in theroy it should support other ARCHs too.

In theory, yes.

> With this patch, virtio PCI block, network and scsi devices
> can work well on ARMv7 and ARMv8 VM.
> 
> QEMU needs below patches for runing the test:
> 
> - Rob Herring's "Add generic PCI host device" patches
> 	http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
> 	http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03483.html
> - Alvise Rigo's "Add Generic PCI host device update"
> 	http://marc.info/?l=qemu-devel&m=140506329920172&w=2
> 
> For ARMv8, cpu model of "host" and "cortex-a57" is missed
> in Alvise Rigo's patchset, otherwise ARM64 VM can't boot.
> 
> All these QEMU patches can be found in below tree:
> 
> 	http://kernel.ubuntu.com/git?p=ming/qemu.git;a=shortlog;h=refs/heads/arm64-pci-test
> 
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>  drivers/pci/host/Kconfig            |    2 +-
>  drivers/pci/host/pci-host-generic.c |  196 +++++++++++------------------------
>  2 files changed, 63 insertions(+), 135 deletions(-)
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index 3dc25fa..885034d 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -50,7 +50,7 @@ config PCI_RCAR_GEN2_PCIE
>  
>  config PCI_HOST_GENERIC
>  	bool "Generic PCI host controller"
> -	depends on ARM && OF
> +	depends on OF
>  	help
>  	  Say Y here if you want to support a simple generic PCI host
>  	  controller, such as the one emulated by kvmtool.
> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
> index 3d2076f..00d0ca3 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -44,12 +44,14 @@ struct gen_pci {
>  	struct list_head			resources;
>  };
>  
> +/* fake sysdata for cheating ARCH's pcibios code */
> +static char	gen_sysdata[256];

I take this as a joke and it does not make me laugh.

>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>  					     unsigned int devfn,
>  					     int where)
>  {
> -	struct pci_sys_data *sys = bus->sysdata;
> -	struct gen_pci *pci = sys->private_data;
> +	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>  	resource_size_t idx = bus->number - pci->cfg.bus_range.start;
>  
>  	return pci->cfg.win[idx] + ((devfn << 8) | where);
> @@ -64,8 +66,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
>  					      unsigned int devfn,
>  					      int where)
>  {
> -	struct pci_sys_data *sys = bus->sysdata;
> -	struct gen_pci *pci = sys->private_data;
> +	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>  	resource_size_t idx = bus->number - pci->cfg.bus_range.start;
>  
>  	return pci->cfg.win[idx] + ((devfn << 12) | where);
> @@ -80,8 +81,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn,
>  				int where, int size, u32 *val)
>  {
>  	void __iomem *addr;
> -	struct pci_sys_data *sys = bus->sysdata;
> -	struct gen_pci *pci = sys->private_data;
> +	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>  
>  	addr = pci->cfg.ops->map_bus(bus, devfn, where);
>  
> @@ -103,8 +103,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
>  				 int where, int size, u32 val)
>  {
>  	void __iomem *addr;
> -	struct pci_sys_data *sys = bus->sysdata;
> -	struct gen_pci *pci = sys->private_data;
> +	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>  
>  	addr = pci->cfg.ops->map_bus(bus, devfn, where);
>  
> @@ -138,45 +137,6 @@ static const struct of_device_id gen_pci_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, gen_pci_of_match);
>  
> -static int gen_pci_calc_io_offset(struct device *dev,
> -				  struct of_pci_range *range,
> -				  struct resource *res,
> -				  resource_size_t *offset)
> -{
> -	static atomic_t wins = ATOMIC_INIT(0);
> -	int err, idx, max_win;
> -	unsigned int window;
> -
> -	if (!PAGE_ALIGNED(range->cpu_addr))
> -		return -EINVAL;
> -
> -	max_win = (IO_SPACE_LIMIT + 1) / SZ_64K;
> -	idx = atomic_inc_return(&wins);
> -	if (idx > max_win)
> -		return -ENOSPC;
> -
> -	window = (idx - 1) * SZ_64K;
> -	err = pci_ioremap_io(window, range->cpu_addr);
> -	if (err)
> -		return err;
> -
> -	of_pci_range_to_resource(range, dev->of_node, res);
> -	res->start = window;
> -	res->end = res->start + range->size - 1;
> -	*offset = window - range->pci_addr;
> -	return 0;
> -}
> -
> -static int gen_pci_calc_mem_offset(struct device *dev,
> -				   struct of_pci_range *range,
> -				   struct resource *res,
> -				   resource_size_t *offset)
> -{
> -	of_pci_range_to_resource(range, dev->of_node, res);
> -	*offset = range->cpu_addr - range->pci_addr;
> -	return 0;
> -}
> -
>  static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
>  {
>  	struct pci_host_bridge_window *win;
> @@ -187,72 +147,6 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
>  	pci_free_resource_list(&pci->resources);
>  }
>  
> -static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
> -{
> -	struct of_pci_range range;
> -	struct of_pci_range_parser parser;
> -	int err, res_valid = 0;
> -	struct device *dev = pci->host.dev.parent;
> -	struct device_node *np = dev->of_node;
> -
> -	if (of_pci_range_parser_init(&parser, np)) {
> -		dev_err(dev, "missing \"ranges\" property\n");
> -		return -EINVAL;
> -	}
> -
> -	for_each_of_pci_range(&parser, &range) {
> -		struct resource *parent, *res;
> -		resource_size_t offset;
> -		u32 restype = range.flags & IORESOURCE_TYPE_BITS;
> -
> -		res = devm_kmalloc(dev, sizeof(*res), GFP_KERNEL);
> -		if (!res) {
> -			err = -ENOMEM;
> -			goto out_release_res;
> -		}
> -
> -		switch (restype) {
> -		case IORESOURCE_IO:
> -			parent = &ioport_resource;
> -			err = gen_pci_calc_io_offset(dev, &range, res, &offset);
> -			break;
> -		case IORESOURCE_MEM:
> -			parent = &iomem_resource;
> -			err = gen_pci_calc_mem_offset(dev, &range, res, &offset);
> -			res_valid |= !(res->flags & IORESOURCE_PREFETCH || err);
> -			break;
> -		default:
> -			err = -EINVAL;
> -			continue;
> -		}
> -
> -		if (err) {
> -			dev_warn(dev,
> -				 "error %d: failed to add resource [type 0x%x, %lld bytes]\n",
> -				 err, restype, range.size);
> -			continue;
> -		}
> -
> -		err = request_resource(parent, res);
> -		if (err)
> -			goto out_release_res;
> -
> -		pci_add_resource_offset(&pci->resources, res, offset);
> -	}
> -
> -	if (!res_valid) {
> -		dev_err(dev, "non-prefetchable memory resource required\n");
> -		err = -EINVAL;
> -		goto out_release_res;
> -	}
> -
> -	return 0;
> -
> -out_release_res:
> -	gen_pci_release_of_pci_ranges(pci);
> -	return err;
> -}
> -
>  static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  {
>  	int err;
> @@ -305,16 +199,33 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  			return -ENOMEM;
>  	}
>  
> -	/* Register bus resource */
> -	pci_add_resource(&pci->resources, bus_range);
>  	return 0;
>  }
>  
> -static int gen_pci_setup(int nr, struct pci_sys_data *sys)
> +static int gen_pci_map_ranges(struct list_head *res,
> +		resource_size_t io_base)
>  {
> -	struct gen_pci *pci = sys->private_data;
> -	list_splice_init(&pci->resources, &sys->resources);
> -	return 1;
> +	struct pci_host_bridge_window *window;
> +	int ret;
> +
> +	list_for_each_entry(window, res, list) {
> +		struct resource *res = window->res;
> +		u64 restype = resource_type(res);
> +
> +		switch (restype) {
> +		case IORESOURCE_IO:
> +			ret = pci_remap_iospace(res, io_base);
> +			if (ret < 0)
> +				return ret;
> +			break;
> +		case IORESOURCE_MEM:
> +		case IORESOURCE_BUS:
> +			break;
> +		default:
> +			return -EINVAL;
> +		}
> +	}
> +	return 0;
>  }

http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html

Patch above is already queued and applies most of the changes you have posted
above.

>  static int gen_pci_probe(struct platform_device *pdev)
> @@ -325,14 +236,11 @@ static int gen_pci_probe(struct platform_device *pdev)
>  	const int *prop;
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
> +	resource_size_t iobase = 0;
>  	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> -	struct hw_pci hw = {
> -		.nr_controllers	= 1,
> -		.private_data	= (void **)&pci,
> -		.setup		= gen_pci_setup,
> -		.map_irq	= of_irq_parse_and_map_pci,
> -		.ops		= &gen_pci_ops,
> -	};
> +	struct pci_bus *bus;
> +	struct pci_dev *pci_dev = NULL;
> +	bool probe_only = false;
>  
>  	if (!pci)
>  		return -ENOMEM;
> @@ -346,9 +254,9 @@ static int gen_pci_probe(struct platform_device *pdev)
>  	prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
>  	if (prop) {
>  		if (*prop)
> -			pci_add_flags(PCI_PROBE_ONLY);
> +			probe_only = true;
>  		else
> -			pci_clear_flags(PCI_PROBE_ONLY);
> +			probe_only = false;
>  	}

How can this possibly work ? If firmware assigns resources, they
should not be enabled, and this is done in pcibios code by checking
the PCI_PROBE_ONLY flag (pcibios_enable_device()).
Now you are removing the flag, how can pcibios_enable_device() work on
platforms that are PROBE_ONLY ? I do not see any additional patch, so I
am confused.

>  
>  	of_id = of_match_node(gen_pci_of_match, np);
> @@ -357,20 +265,40 @@ static int gen_pci_probe(struct platform_device *pdev)
>  	INIT_LIST_HEAD(&pci->host.windows);
>  	INIT_LIST_HEAD(&pci->resources);
>  
> -	/* Parse our PCI ranges and request their resources */
> -	err = gen_pci_parse_request_of_pci_ranges(pci);
> +	err = of_pci_get_host_bridge_resources(np, 0, 0xff,
> +			&pci->resources, &iobase);
>  	if (err)
>  		return err;
>  
>  	/* Parse and map our Configuration Space windows */
>  	err = gen_pci_parse_map_cfg_windows(pci);
> -	if (err) {
> -		gen_pci_release_of_pci_ranges(pci);
> -		return err;
> +	if (err)
> +		goto fail;
> +
> +	err = gen_pci_map_ranges(&pci->resources, iobase);
> +	if (err)
> +		goto fail;
> +
> +	err = -ENOMEM;
> +	platform_set_drvdata(pdev, pci);
> +	bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, gen_sysdata,
> +				&pci->resources);
> +	if (!bus)
> +		goto fail;
> +
> +	for_each_pci_dev(pci_dev)
> +		pci_dev->irq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
> +
> +	if (!probe_only) {
> +		pci_bus_size_bridges(bus);
> +		pci_bus_assign_resources(bus);
> +		pci_bus_add_devices(bus);
>  	}

Do you realize that pci_scan_root_bus() already calls pci_bus_add_devices() ?
On top of that:

+	if (!probe_only) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);

Should be called in pci_scan_root_bus() before pci_bus_add_devices(), if
things are working for you at the moment it is out of sheer chance (and
unfortunately that's the case for other host controllers IMO).

I am working on adding the code above to pci_scan_root_bus() implementation
this is a generic issue that we need to solve, there are already patches
on the list to remove the dependency on pci_sys_data (I mean properly)
and move the code to generic pci_scan_root_bus() implementation.

Overall, we are already working to achieve what this patch is trying to
achieve, I will keep you posted.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ming Lei Nov. 11, 2014, 2:02 p.m. UTC | #2
On Tue, Nov 11, 2014 at 8:29 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Tue, Nov 11, 2014 at 10:33:59AM +0000, Ming Lei wrote:
>> This patch converts the generic host controller driver
>> into a more generic one, and basically don't need
>> platform's pcibios support, and use DT based generic
>> APIs to parse resource and remap IO port.
>>
>> This patch has been tested on both ARMv7 and ARMv8
>> VM, and in theroy it should support other ARCHs too.
>
> In theory, yes.
>
>> With this patch, virtio PCI block, network and scsi devices
>> can work well on ARMv7 and ARMv8 VM.
>>
>> QEMU needs below patches for runing the test:
>>
>> - Rob Herring's "Add generic PCI host device" patches
>>       http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03482.html
>>       http://lists.gnu.org/archive/html/qemu-devel/2014-06/msg03483.html
>> - Alvise Rigo's "Add Generic PCI host device update"
>>       http://marc.info/?l=qemu-devel&m=140506329920172&w=2
>>
>> For ARMv8, cpu model of "host" and "cortex-a57" is missed
>> in Alvise Rigo's patchset, otherwise ARM64 VM can't boot.
>>
>> All these QEMU patches can be found in below tree:
>>
>>       http://kernel.ubuntu.com/git?p=ming/qemu.git;a=shortlog;h=refs/heads/arm64-pci-test
>>
>> Signed-off-by: Ming Lei <ming.lei@canonical.com>
>> ---
>>  drivers/pci/host/Kconfig            |    2 +-
>>  drivers/pci/host/pci-host-generic.c |  196 +++++++++++------------------------
>>  2 files changed, 63 insertions(+), 135 deletions(-)
>>
>> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
>> index 3dc25fa..885034d 100644
>> --- a/drivers/pci/host/Kconfig
>> +++ b/drivers/pci/host/Kconfig
>> @@ -50,7 +50,7 @@ config PCI_RCAR_GEN2_PCIE
>>
>>  config PCI_HOST_GENERIC
>>       bool "Generic PCI host controller"
>> -     depends on ARM && OF
>> +     depends on OF
>>       help
>>         Say Y here if you want to support a simple generic PCI host
>>         controller, such as the one emulated by kvmtool.
>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
>> index 3d2076f..00d0ca3 100644
>> --- a/drivers/pci/host/pci-host-generic.c
>> +++ b/drivers/pci/host/pci-host-generic.c
>> @@ -44,12 +44,14 @@ struct gen_pci {
>>       struct list_head                        resources;
>>  };
>>
>> +/* fake sysdata for cheating ARCH's pcibios code */
>> +static char  gen_sysdata[256];
>
> I take this as a joke and it does not make me laugh.

One motivation of the patch is to remove ARCH or platform dependency,
that is why the above faked sysdata is introduced, but pci_sysdata is a
generic issue.

>
>>  static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
>>                                            unsigned int devfn,
>>                                            int where)
>>  {
>> -     struct pci_sys_data *sys = bus->sysdata;
>> -     struct gen_pci *pci = sys->private_data;
>> +     struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>>       resource_size_t idx = bus->number - pci->cfg.bus_range.start;
>>
>>       return pci->cfg.win[idx] + ((devfn << 8) | where);
>> @@ -64,8 +66,7 @@ static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
>>                                             unsigned int devfn,
>>                                             int where)
>>  {
>> -     struct pci_sys_data *sys = bus->sysdata;
>> -     struct gen_pci *pci = sys->private_data;
>> +     struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>>       resource_size_t idx = bus->number - pci->cfg.bus_range.start;
>>
>>       return pci->cfg.win[idx] + ((devfn << 12) | where);
>> @@ -80,8 +81,7 @@ static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn,
>>                               int where, int size, u32 *val)
>>  {
>>       void __iomem *addr;
>> -     struct pci_sys_data *sys = bus->sysdata;
>> -     struct gen_pci *pci = sys->private_data;
>> +     struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>>
>>       addr = pci->cfg.ops->map_bus(bus, devfn, where);
>>
>> @@ -103,8 +103,7 @@ static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
>>                                int where, int size, u32 val)
>>  {
>>       void __iomem *addr;
>> -     struct pci_sys_data *sys = bus->sysdata;
>> -     struct gen_pci *pci = sys->private_data;
>> +     struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
>>
>>       addr = pci->cfg.ops->map_bus(bus, devfn, where);
>>
>> @@ -138,45 +137,6 @@ static const struct of_device_id gen_pci_of_match[] = {
>>  };
>>  MODULE_DEVICE_TABLE(of, gen_pci_of_match);
>>
>> -static int gen_pci_calc_io_offset(struct device *dev,
>> -                               struct of_pci_range *range,
>> -                               struct resource *res,
>> -                               resource_size_t *offset)
>> -{
>> -     static atomic_t wins = ATOMIC_INIT(0);
>> -     int err, idx, max_win;
>> -     unsigned int window;
>> -
>> -     if (!PAGE_ALIGNED(range->cpu_addr))
>> -             return -EINVAL;
>> -
>> -     max_win = (IO_SPACE_LIMIT + 1) / SZ_64K;
>> -     idx = atomic_inc_return(&wins);
>> -     if (idx > max_win)
>> -             return -ENOSPC;
>> -
>> -     window = (idx - 1) * SZ_64K;
>> -     err = pci_ioremap_io(window, range->cpu_addr);
>> -     if (err)
>> -             return err;
>> -
>> -     of_pci_range_to_resource(range, dev->of_node, res);
>> -     res->start = window;
>> -     res->end = res->start + range->size - 1;
>> -     *offset = window - range->pci_addr;
>> -     return 0;
>> -}
>> -
>> -static int gen_pci_calc_mem_offset(struct device *dev,
>> -                                struct of_pci_range *range,
>> -                                struct resource *res,
>> -                                resource_size_t *offset)
>> -{
>> -     of_pci_range_to_resource(range, dev->of_node, res);
>> -     *offset = range->cpu_addr - range->pci_addr;
>> -     return 0;
>> -}
>> -
>>  static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
>>  {
>>       struct pci_host_bridge_window *win;
>> @@ -187,72 +147,6 @@ static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
>>       pci_free_resource_list(&pci->resources);
>>  }
>>
>> -static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
>> -{
>> -     struct of_pci_range range;
>> -     struct of_pci_range_parser parser;
>> -     int err, res_valid = 0;
>> -     struct device *dev = pci->host.dev.parent;
>> -     struct device_node *np = dev->of_node;
>> -
>> -     if (of_pci_range_parser_init(&parser, np)) {
>> -             dev_err(dev, "missing \"ranges\" property\n");
>> -             return -EINVAL;
>> -     }
>> -
>> -     for_each_of_pci_range(&parser, &range) {
>> -             struct resource *parent, *res;
>> -             resource_size_t offset;
>> -             u32 restype = range.flags & IORESOURCE_TYPE_BITS;
>> -
>> -             res = devm_kmalloc(dev, sizeof(*res), GFP_KERNEL);
>> -             if (!res) {
>> -                     err = -ENOMEM;
>> -                     goto out_release_res;
>> -             }
>> -
>> -             switch (restype) {
>> -             case IORESOURCE_IO:
>> -                     parent = &ioport_resource;
>> -                     err = gen_pci_calc_io_offset(dev, &range, res, &offset);
>> -                     break;
>> -             case IORESOURCE_MEM:
>> -                     parent = &iomem_resource;
>> -                     err = gen_pci_calc_mem_offset(dev, &range, res, &offset);
>> -                     res_valid |= !(res->flags & IORESOURCE_PREFETCH || err);
>> -                     break;
>> -             default:
>> -                     err = -EINVAL;
>> -                     continue;
>> -             }
>> -
>> -             if (err) {
>> -                     dev_warn(dev,
>> -                              "error %d: failed to add resource [type 0x%x, %lld bytes]\n",
>> -                              err, restype, range.size);
>> -                     continue;
>> -             }
>> -
>> -             err = request_resource(parent, res);
>> -             if (err)
>> -                     goto out_release_res;
>> -
>> -             pci_add_resource_offset(&pci->resources, res, offset);
>> -     }
>> -
>> -     if (!res_valid) {
>> -             dev_err(dev, "non-prefetchable memory resource required\n");
>> -             err = -EINVAL;
>> -             goto out_release_res;
>> -     }
>> -
>> -     return 0;
>> -
>> -out_release_res:
>> -     gen_pci_release_of_pci_ranges(pci);
>> -     return err;
>> -}
>> -
>>  static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>>  {
>>       int err;
>> @@ -305,16 +199,33 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>>                       return -ENOMEM;
>>       }
>>
>> -     /* Register bus resource */
>> -     pci_add_resource(&pci->resources, bus_range);
>>       return 0;
>>  }
>>
>> -static int gen_pci_setup(int nr, struct pci_sys_data *sys)
>> +static int gen_pci_map_ranges(struct list_head *res,
>> +             resource_size_t io_base)
>>  {
>> -     struct gen_pci *pci = sys->private_data;
>> -     list_splice_init(&pci->resources, &sys->resources);
>> -     return 1;
>> +     struct pci_host_bridge_window *window;
>> +     int ret;
>> +
>> +     list_for_each_entry(window, res, list) {
>> +             struct resource *res = window->res;
>> +             u64 restype = resource_type(res);
>> +
>> +             switch (restype) {
>> +             case IORESOURCE_IO:
>> +                     ret = pci_remap_iospace(res, io_base);
>> +                     if (ret < 0)
>> +                             return ret;
>> +                     break;
>> +             case IORESOURCE_MEM:
>> +             case IORESOURCE_BUS:
>> +                     break;
>> +             default:
>> +                     return -EINVAL;
>> +             }
>> +     }
>> +     return 0;
>>  }
>
> http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
>
> Patch above is already queued and applies most of the changes you have posted
> above.

I should have looked at list first before writing the patch, :-(

Could you make it more generic so that other ARCHs(at least ARM64)
can benefit from it too?

>
>>  static int gen_pci_probe(struct platform_device *pdev)
>> @@ -325,14 +236,11 @@ static int gen_pci_probe(struct platform_device *pdev)
>>       const int *prop;
>>       struct device *dev = &pdev->dev;
>>       struct device_node *np = dev->of_node;
>> +     resource_size_t iobase = 0;
>>       struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
>> -     struct hw_pci hw = {
>> -             .nr_controllers = 1,
>> -             .private_data   = (void **)&pci,
>> -             .setup          = gen_pci_setup,
>> -             .map_irq        = of_irq_parse_and_map_pci,
>> -             .ops            = &gen_pci_ops,
>> -     };
>> +     struct pci_bus *bus;
>> +     struct pci_dev *pci_dev = NULL;
>> +     bool probe_only = false;
>>
>>       if (!pci)
>>               return -ENOMEM;
>> @@ -346,9 +254,9 @@ static int gen_pci_probe(struct platform_device *pdev)
>>       prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
>>       if (prop) {
>>               if (*prop)
>> -                     pci_add_flags(PCI_PROBE_ONLY);
>> +                     probe_only = true;
>>               else
>> -                     pci_clear_flags(PCI_PROBE_ONLY);
>> +                     probe_only = false;
>>       }
>
> How can this possibly work ? If firmware assigns resources, they
> should not be enabled, and this is done in pcibios code by checking
> the PCI_PROBE_ONLY flag (pcibios_enable_device()).
> Now you are removing the flag, how can pcibios_enable_device() work on
> platforms that are PROBE_ONLY ? I do not see any additional patch, so I
> am confused.
>>
>>       of_id = of_match_node(gen_pci_of_match, np);
>> @@ -357,20 +265,40 @@ static int gen_pci_probe(struct platform_device *pdev)
>>       INIT_LIST_HEAD(&pci->host.windows);
>>       INIT_LIST_HEAD(&pci->resources);
>>
>> -     /* Parse our PCI ranges and request their resources */
>> -     err = gen_pci_parse_request_of_pci_ranges(pci);
>> +     err = of_pci_get_host_bridge_resources(np, 0, 0xff,
>> +                     &pci->resources, &iobase);
>>       if (err)
>>               return err;
>>
>>       /* Parse and map our Configuration Space windows */
>>       err = gen_pci_parse_map_cfg_windows(pci);
>> -     if (err) {
>> -             gen_pci_release_of_pci_ranges(pci);
>> -             return err;
>> +     if (err)
>> +             goto fail;
>> +
>> +     err = gen_pci_map_ranges(&pci->resources, iobase);
>> +     if (err)
>> +             goto fail;
>> +
>> +     err = -ENOMEM;
>> +     platform_set_drvdata(pdev, pci);
>> +     bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, gen_sysdata,
>> +                             &pci->resources);
>> +     if (!bus)
>> +             goto fail;
>> +
>> +     for_each_pci_dev(pci_dev)
>> +             pci_dev->irq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
>> +
>> +     if (!probe_only) {
>> +             pci_bus_size_bridges(bus);
>> +             pci_bus_assign_resources(bus);
>> +             pci_bus_add_devices(bus);
>>       }
>
> Do you realize that pci_scan_root_bus() already calls pci_bus_add_devices() ?

Yes, it can be removed, but basically that is what the ARM pcibios code
is doing, :-)

> On top of that:
>
> +       if (!probe_only) {
> +               pci_bus_size_bridges(bus);
> +               pci_bus_assign_resources(bus);
> +               pci_bus_add_devices(bus);
>
> Should be called in pci_scan_root_bus() before pci_bus_add_devices(), if
> things are working for you at the moment it is out of sheer chance (and
> unfortunately that's the case for other host controllers IMO).
>
> I am working on adding the code above to pci_scan_root_bus() implementation
> this is a generic issue that we need to solve, there are already patches
> on the list to remove the dependency on pci_sys_data (I mean properly)
> and move the code to generic pci_scan_root_bus() implementation.

Nice to know you are working on it.

>
> Overall, we are already working to achieve what this patch is trying to
> achieve, I will keep you posted.

OK, thanks.
--
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 Nov. 11, 2014, 6:24 p.m. UTC | #3
On Tue, Nov 11, 2014 at 02:02:20PM +0000, Ming Lei wrote:

[...]

> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
> >
> > Patch above is already queued and applies most of the changes you have posted
> > above.
> 
> I should have looked at list first before writing the patch, :-(
> 
> Could you make it more generic so that other ARCHs(at least ARM64)
> can benefit from it too?

That's our current goal, we are carrying out clean-ups to remove arch
dependency and move code to the generic layer PCI layer.

Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ming Lei Nov. 12, 2014, 2:12 a.m. UTC | #4
On Wed, Nov 12, 2014 at 2:24 AM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Tue, Nov 11, 2014 at 02:02:20PM +0000, Ming Lei wrote:
>
> [...]
>
>> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
>> >
>> > Patch above is already queued and applies most of the changes you have posted
>> > above.
>>
>> I should have looked at list first before writing the patch, :-(
>>
>> Could you make it more generic so that other ARCHs(at least ARM64)
>> can benefit from it too?
>
> That's our current goal, we are carrying out clean-ups to remove arch
> dependency and move code to the generic layer PCI layer.

OK, I am glad to test them after you post them out.

At least, the 'struct gen_pci' pointer can be put in driver data
of the platform_device now, which may remove dependency on
pci_sysdata in this driver.  But ARCH's pcibios code still may
access 'pci_sysdata', do you have patches or solution to handle
this issue?

Do you think it is doable to introduce a option(from module
parameter, or device tree, ...) to let the driver and PCI core
ignore/bypass all pcibios handling for the generic bus?  If that
is OK, it can provide an approach to reach the goal a bit easy.
In reality, generic host controller should be only used by VM,
and VM's firmware shouldn't be very complicated and I think
the option can meet most of demands.

Thanks,
Ming Lei
--
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
Bjorn Helgaas Nov. 12, 2014, 2:47 a.m. UTC | #5
On Tue, Nov 11, 2014 at 7:12 PM, Ming Lei <ming.lei@canonical.com> wrote:
> ...
> Do you think it is doable to introduce a option(from module
> parameter, or device tree, ...) to let the driver and PCI core
> ignore/bypass all pcibios handling for the generic bus?

No, I don't think so.
--
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
Ming Lei Nov. 12, 2014, 8:58 a.m. UTC | #6
On Wed, Nov 12, 2014 at 10:47 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Tue, Nov 11, 2014 at 7:12 PM, Ming Lei <ming.lei@canonical.com> wrote:
>> ...
>> Do you think it is doable to introduce a option(from module
>> parameter, or device tree, ...) to let the driver and PCI core
>> ignore/bypass all pcibios handling for the generic bus?
>
> No, I don't think so.

Could you explain a bit? In VM world, it might be reasonable to
claim that there is no pcibios service or pcibios service needn't.

IMO it isn't easy to make a cross-arch PCI generic controller
driver if pcibios handling has to be involved.

Thanks,
Ming Lei
--
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 Nov. 12, 2014, 9:58 a.m. UTC | #7
On Wed, Nov 12, 2014 at 02:12:12AM +0000, Ming Lei wrote:
> On Wed, Nov 12, 2014 at 2:24 AM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Tue, Nov 11, 2014 at 02:02:20PM +0000, Ming Lei wrote:
> >
> > [...]
> >
> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
> >> >
> >> > Patch above is already queued and applies most of the changes you have posted
> >> > above.
> >>
> >> I should have looked at list first before writing the patch, :-(
> >>
> >> Could you make it more generic so that other ARCHs(at least ARM64)
> >> can benefit from it too?
> >
> > That's our current goal, we are carrying out clean-ups to remove arch
> > dependency and move code to the generic layer PCI layer.
> 
> OK, I am glad to test them after you post them out.
> 
> At least, the 'struct gen_pci' pointer can be put in driver data
> of the platform_device now, which may remove dependency on
> pci_sysdata in this driver.  But ARCH's pcibios code still may
> access 'pci_sysdata', do you have patches or solution to handle
> this issue?

I will consider adding struct gen_pci pointer as platform device
driver data, we are working on removing pci_sys_data dependency in the ARM
pcibios backend, and I do not think we are far from achieving that.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ming Lei Nov. 12, 2014, 10:51 a.m. UTC | #8
On Wed, Nov 12, 2014 at 5:58 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Nov 12, 2014 at 02:12:12AM +0000, Ming Lei wrote:
>> On Wed, Nov 12, 2014 at 2:24 AM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>> > On Tue, Nov 11, 2014 at 02:02:20PM +0000, Ming Lei wrote:
>> >
>> > [...]
>> >
>> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
>> >> >
>> >> > Patch above is already queued and applies most of the changes you have posted
>> >> > above.
>> >>
>> >> I should have looked at list first before writing the patch, :-(
>> >>
>> >> Could you make it more generic so that other ARCHs(at least ARM64)
>> >> can benefit from it too?
>> >
>> > That's our current goal, we are carrying out clean-ups to remove arch
>> > dependency and move code to the generic layer PCI layer.
>>
>> OK, I am glad to test them after you post them out.
>>
>> At least, the 'struct gen_pci' pointer can be put in driver data
>> of the platform_device now, which may remove dependency on
>> pci_sysdata in this driver.  But ARCH's pcibios code still may
>> access 'pci_sysdata', do you have patches or solution to handle
>> this issue?
>
> I will consider adding struct gen_pci pointer as platform device
> driver data, we are working on removing pci_sys_data dependency in the ARM
> pcibios backend, and I do not think we are far from achieving that.

Other ARCHs has sort of pcibios backend too, so that said it still
depends on ARM or ARM64, doesn't it?

Thanks,
--
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 Nov. 12, 2014, 12:32 p.m. UTC | #9
On Wed, Nov 12, 2014 at 10:51:31AM +0000, Ming Lei wrote:
> On Wed, Nov 12, 2014 at 5:58 PM, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> > On Wed, Nov 12, 2014 at 02:12:12AM +0000, Ming Lei wrote:
> >> On Wed, Nov 12, 2014 at 2:24 AM, Lorenzo Pieralisi
> >> <lorenzo.pieralisi@arm.com> wrote:
> >> > On Tue, Nov 11, 2014 at 02:02:20PM +0000, Ming Lei wrote:
> >> >
> >> > [...]
> >> >
> >> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
> >> >> >
> >> >> > Patch above is already queued and applies most of the changes you have posted
> >> >> > above.
> >> >>
> >> >> I should have looked at list first before writing the patch, :-(
> >> >>
> >> >> Could you make it more generic so that other ARCHs(at least ARM64)
> >> >> can benefit from it too?
> >> >
> >> > That's our current goal, we are carrying out clean-ups to remove arch
> >> > dependency and move code to the generic layer PCI layer.
> >>
> >> OK, I am glad to test them after you post them out.
> >>
> >> At least, the 'struct gen_pci' pointer can be put in driver data
> >> of the platform_device now, which may remove dependency on
> >> pci_sysdata in this driver.  But ARCH's pcibios code still may
> >> access 'pci_sysdata', do you have patches or solution to handle
> >> this issue?
> >
> > I will consider adding struct gen_pci pointer as platform device
> > driver data, we are working on removing pci_sys_data dependency in the ARM
> > pcibios backend, and I do not think we are far from achieving that.
> 
> Other ARCHs has sort of pcibios backend too, so that said it still
> depends on ARM or ARM64, doesn't it?

Yes, so ? If you think there is a way to make this host controller work
on all arches you fancy using it on with no arch dependency in a proper
way step up to the plate and post the code, we will duly review it.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ming Lei Nov. 13, 2014, 1:56 a.m. UTC | #10
On Wed, Nov 12, 2014 at 8:32 PM, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
> On Wed, Nov 12, 2014 at 10:51:31AM +0000, Ming Lei wrote:
>> On Wed, Nov 12, 2014 at 5:58 PM, Lorenzo Pieralisi
>> <lorenzo.pieralisi@arm.com> wrote:
>> > On Wed, Nov 12, 2014 at 02:12:12AM +0000, Ming Lei wrote:
>> >> On Wed, Nov 12, 2014 at 2:24 AM, Lorenzo Pieralisi
>> >> <lorenzo.pieralisi@arm.com> wrote:
>> >> > On Tue, Nov 11, 2014 at 02:02:20PM +0000, Ming Lei wrote:
>> >> >
>> >> > [...]
>> >> >
>> >> >> > http://lists.infradead.org/pipermail/linux-arm-kernel/2014-October/296535.html
>> >> >> >
>> >> >> > Patch above is already queued and applies most of the changes you have posted
>> >> >> > above.
>> >> >>
>> >> >> I should have looked at list first before writing the patch, :-(
>> >> >>
>> >> >> Could you make it more generic so that other ARCHs(at least ARM64)
>> >> >> can benefit from it too?
>> >> >
>> >> > That's our current goal, we are carrying out clean-ups to remove arch
>> >> > dependency and move code to the generic layer PCI layer.
>> >>
>> >> OK, I am glad to test them after you post them out.
>> >>
>> >> At least, the 'struct gen_pci' pointer can be put in driver data
>> >> of the platform_device now, which may remove dependency on
>> >> pci_sysdata in this driver.  But ARCH's pcibios code still may
>> >> access 'pci_sysdata', do you have patches or solution to handle
>> >> this issue?
>> >
>> > I will consider adding struct gen_pci pointer as platform device
>> > driver data, we are working on removing pci_sys_data dependency in the ARM
>> > pcibios backend, and I do not think we are far from achieving that.
>>
>> Other ARCHs has sort of pcibios backend too, so that said it still
>> depends on ARM or ARM64, doesn't it?
>
> Yes, so ? If you think there is a way to make this host controller work

I mean suppose the driver/device is only for ARM and ARM64, it
should be easy to support, and it may not be necessary to remove
pci_sys_data dependency in the ARM, such as:

- make a different code path for ARM64
- or like this patch to fake sysdata for ARM
- or what ever

> on all arches you fancy using it on with no arch dependency in a proper
> way step up to the plate and post the code, we will duly review it.

The only way I thought of is to bypass pcibios handling for the
generic host controller, but it may not be agreed.

Thanks,
Ming Lei
--
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
Bjorn Helgaas Nov. 13, 2014, 3:58 a.m. UTC | #11
On Wed, Nov 12, 2014 at 1:58 AM, Ming Lei <ming.lei@canonical.com> wrote:
> On Wed, Nov 12, 2014 at 10:47 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Tue, Nov 11, 2014 at 7:12 PM, Ming Lei <ming.lei@canonical.com> wrote:
>>> ...
>>> Do you think it is doable to introduce a option(from module
>>> parameter, or device tree, ...) to let the driver and PCI core
>>> ignore/bypass all pcibios handling for the generic bus?
>>
>> No, I don't think so.
>
> Could you explain a bit? In VM world, it might be reasonable to
> claim that there is no pcibios service or pcibios service needn't.
>
> IMO it isn't easy to make a cross-arch PCI generic controller
> driver if pcibios handling has to be involved.

I'm not going to add kernel or module parameters to change the way the
basic code works.  We need a way to factor the differences so the PCI
core code is the same for all arches, and the differences are hidden
in the arch-specific code.

That's basically what the pcibios interfaces do.  I suspect they were
originally named "pcibios" because there's a "PCI BIOS" spec that
tells you how to make BIOS calls to enumerate devices, access config
space, manage interrupts, etc.  But "pcibios" is now just the name of
interfaces to arch-specific functionality.  Most arches don't have a
BIOS, and most pcibios_*() functions don't make BIOS calls.

If you have a new arch that has no "pcibios service" (I don't know
exactly what you mean by that), you can still implement the
pcibios_*() interfaces used by the PCI core.  If some of these
interfaces don't actually do anything on your arch, that's fine; they
can be no-ops.

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

Patch

diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index 3dc25fa..885034d 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -50,7 +50,7 @@  config PCI_RCAR_GEN2_PCIE
 
 config PCI_HOST_GENERIC
 	bool "Generic PCI host controller"
-	depends on ARM && OF
+	depends on OF
 	help
 	  Say Y here if you want to support a simple generic PCI host
 	  controller, such as the one emulated by kvmtool.
diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c
index 3d2076f..00d0ca3 100644
--- a/drivers/pci/host/pci-host-generic.c
+++ b/drivers/pci/host/pci-host-generic.c
@@ -44,12 +44,14 @@  struct gen_pci {
 	struct list_head			resources;
 };
 
+/* fake sysdata for cheating ARCH's pcibios code */
+static char	gen_sysdata[256];
+
 static void __iomem *gen_pci_map_cfg_bus_cam(struct pci_bus *bus,
 					     unsigned int devfn,
 					     int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
 	resource_size_t idx = bus->number - pci->cfg.bus_range.start;
 
 	return pci->cfg.win[idx] + ((devfn << 8) | where);
@@ -64,8 +66,7 @@  static void __iomem *gen_pci_map_cfg_bus_ecam(struct pci_bus *bus,
 					      unsigned int devfn,
 					      int where)
 {
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
 	resource_size_t idx = bus->number - pci->cfg.bus_range.start;
 
 	return pci->cfg.win[idx] + ((devfn << 12) | where);
@@ -80,8 +81,7 @@  static int gen_pci_config_read(struct pci_bus *bus, unsigned int devfn,
 				int where, int size, u32 *val)
 {
 	void __iomem *addr;
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
 
 	addr = pci->cfg.ops->map_bus(bus, devfn, where);
 
@@ -103,8 +103,7 @@  static int gen_pci_config_write(struct pci_bus *bus, unsigned int devfn,
 				 int where, int size, u32 val)
 {
 	void __iomem *addr;
-	struct pci_sys_data *sys = bus->sysdata;
-	struct gen_pci *pci = sys->private_data;
+	struct gen_pci *pci = dev_get_drvdata(bus->dev.parent->parent);
 
 	addr = pci->cfg.ops->map_bus(bus, devfn, where);
 
@@ -138,45 +137,6 @@  static const struct of_device_id gen_pci_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, gen_pci_of_match);
 
-static int gen_pci_calc_io_offset(struct device *dev,
-				  struct of_pci_range *range,
-				  struct resource *res,
-				  resource_size_t *offset)
-{
-	static atomic_t wins = ATOMIC_INIT(0);
-	int err, idx, max_win;
-	unsigned int window;
-
-	if (!PAGE_ALIGNED(range->cpu_addr))
-		return -EINVAL;
-
-	max_win = (IO_SPACE_LIMIT + 1) / SZ_64K;
-	idx = atomic_inc_return(&wins);
-	if (idx > max_win)
-		return -ENOSPC;
-
-	window = (idx - 1) * SZ_64K;
-	err = pci_ioremap_io(window, range->cpu_addr);
-	if (err)
-		return err;
-
-	of_pci_range_to_resource(range, dev->of_node, res);
-	res->start = window;
-	res->end = res->start + range->size - 1;
-	*offset = window - range->pci_addr;
-	return 0;
-}
-
-static int gen_pci_calc_mem_offset(struct device *dev,
-				   struct of_pci_range *range,
-				   struct resource *res,
-				   resource_size_t *offset)
-{
-	of_pci_range_to_resource(range, dev->of_node, res);
-	*offset = range->cpu_addr - range->pci_addr;
-	return 0;
-}
-
 static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
 {
 	struct pci_host_bridge_window *win;
@@ -187,72 +147,6 @@  static void gen_pci_release_of_pci_ranges(struct gen_pci *pci)
 	pci_free_resource_list(&pci->resources);
 }
 
-static int gen_pci_parse_request_of_pci_ranges(struct gen_pci *pci)
-{
-	struct of_pci_range range;
-	struct of_pci_range_parser parser;
-	int err, res_valid = 0;
-	struct device *dev = pci->host.dev.parent;
-	struct device_node *np = dev->of_node;
-
-	if (of_pci_range_parser_init(&parser, np)) {
-		dev_err(dev, "missing \"ranges\" property\n");
-		return -EINVAL;
-	}
-
-	for_each_of_pci_range(&parser, &range) {
-		struct resource *parent, *res;
-		resource_size_t offset;
-		u32 restype = range.flags & IORESOURCE_TYPE_BITS;
-
-		res = devm_kmalloc(dev, sizeof(*res), GFP_KERNEL);
-		if (!res) {
-			err = -ENOMEM;
-			goto out_release_res;
-		}
-
-		switch (restype) {
-		case IORESOURCE_IO:
-			parent = &ioport_resource;
-			err = gen_pci_calc_io_offset(dev, &range, res, &offset);
-			break;
-		case IORESOURCE_MEM:
-			parent = &iomem_resource;
-			err = gen_pci_calc_mem_offset(dev, &range, res, &offset);
-			res_valid |= !(res->flags & IORESOURCE_PREFETCH || err);
-			break;
-		default:
-			err = -EINVAL;
-			continue;
-		}
-
-		if (err) {
-			dev_warn(dev,
-				 "error %d: failed to add resource [type 0x%x, %lld bytes]\n",
-				 err, restype, range.size);
-			continue;
-		}
-
-		err = request_resource(parent, res);
-		if (err)
-			goto out_release_res;
-
-		pci_add_resource_offset(&pci->resources, res, offset);
-	}
-
-	if (!res_valid) {
-		dev_err(dev, "non-prefetchable memory resource required\n");
-		err = -EINVAL;
-		goto out_release_res;
-	}
-
-	return 0;
-
-out_release_res:
-	gen_pci_release_of_pci_ranges(pci);
-	return err;
-}
-
 static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 {
 	int err;
@@ -305,16 +199,33 @@  static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
 			return -ENOMEM;
 	}
 
-	/* Register bus resource */
-	pci_add_resource(&pci->resources, bus_range);
 	return 0;
 }
 
-static int gen_pci_setup(int nr, struct pci_sys_data *sys)
+static int gen_pci_map_ranges(struct list_head *res,
+		resource_size_t io_base)
 {
-	struct gen_pci *pci = sys->private_data;
-	list_splice_init(&pci->resources, &sys->resources);
-	return 1;
+	struct pci_host_bridge_window *window;
+	int ret;
+
+	list_for_each_entry(window, res, list) {
+		struct resource *res = window->res;
+		u64 restype = resource_type(res);
+
+		switch (restype) {
+		case IORESOURCE_IO:
+			ret = pci_remap_iospace(res, io_base);
+			if (ret < 0)
+				return ret;
+			break;
+		case IORESOURCE_MEM:
+		case IORESOURCE_BUS:
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
+	return 0;
 }
 
 static int gen_pci_probe(struct platform_device *pdev)
@@ -325,14 +236,11 @@  static int gen_pci_probe(struct platform_device *pdev)
 	const int *prop;
 	struct device *dev = &pdev->dev;
 	struct device_node *np = dev->of_node;
+	resource_size_t iobase = 0;
 	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
-	struct hw_pci hw = {
-		.nr_controllers	= 1,
-		.private_data	= (void **)&pci,
-		.setup		= gen_pci_setup,
-		.map_irq	= of_irq_parse_and_map_pci,
-		.ops		= &gen_pci_ops,
-	};
+	struct pci_bus *bus;
+	struct pci_dev *pci_dev = NULL;
+	bool probe_only = false;
 
 	if (!pci)
 		return -ENOMEM;
@@ -346,9 +254,9 @@  static int gen_pci_probe(struct platform_device *pdev)
 	prop = of_get_property(of_chosen, "linux,pci-probe-only", NULL);
 	if (prop) {
 		if (*prop)
-			pci_add_flags(PCI_PROBE_ONLY);
+			probe_only = true;
 		else
-			pci_clear_flags(PCI_PROBE_ONLY);
+			probe_only = false;
 	}
 
 	of_id = of_match_node(gen_pci_of_match, np);
@@ -357,20 +265,40 @@  static int gen_pci_probe(struct platform_device *pdev)
 	INIT_LIST_HEAD(&pci->host.windows);
 	INIT_LIST_HEAD(&pci->resources);
 
-	/* Parse our PCI ranges and request their resources */
-	err = gen_pci_parse_request_of_pci_ranges(pci);
+	err = of_pci_get_host_bridge_resources(np, 0, 0xff,
+			&pci->resources, &iobase);
 	if (err)
 		return err;
 
 	/* Parse and map our Configuration Space windows */
 	err = gen_pci_parse_map_cfg_windows(pci);
-	if (err) {
-		gen_pci_release_of_pci_ranges(pci);
-		return err;
+	if (err)
+		goto fail;
+
+	err = gen_pci_map_ranges(&pci->resources, iobase);
+	if (err)
+		goto fail;
+
+	err = -ENOMEM;
+	platform_set_drvdata(pdev, pci);
+	bus = pci_scan_root_bus(dev, 0, &gen_pci_ops, gen_sysdata,
+				&pci->resources);
+	if (!bus)
+		goto fail;
+
+	for_each_pci_dev(pci_dev)
+		pci_dev->irq = of_irq_parse_and_map_pci(pci_dev, 0, 0);
+
+	if (!probe_only) {
+		pci_bus_size_bridges(bus);
+		pci_bus_assign_resources(bus);
+		pci_bus_add_devices(bus);
 	}
 
-	pci_common_init_dev(dev, &hw);
 	return 0;
+ fail:
+	gen_pci_release_of_pci_ranges(pci);
+	return err;
 }
 
 static struct platform_driver gen_pci_driver = {