Re: [PATCH 1/2] pci: generic: Use the pci_scan_root_bus instead of pci_common_init_dev

[Lorenzo Pieralisi]

On Mon, Feb 09, 2015 at 03:22:29PM +0000, Suravee Suthikulpanit wrote:
> Replacing pci_common_init_dev with pci_scan_root_bus, and remove reference
> to struct pci_sys_data and pci_hw, which is specific to ARM32. This allows
> the PCI host generic driver to also usable in ARM64 architecture.
> 
> Cc: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Will Deacon <will.deacon@xxxxxxx>
> Cc: Marc Zyngier <marc.zyngier@xxxxxxx>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Cc: Catalin Marinas <Catalin.Marinas@xxxxxxx>
> Cc: Liviu Dudau <Liviu.Dudau@xxxxxxx>
> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx>
> 
> ---
>  drivers/pci/host/Kconfig            |  2 +-
>  drivers/pci/host/pci-host-generic.c | 33 ++++++++++-----------------------
>  2 files changed, 11 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index c4b6568..739bab0 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -53,7 +53,7 @@ config PCI_RCAR_GEN2_PCIE
>  
>  config PCI_HOST_GENERIC
>  	bool "Generic PCI host controller"
> -	depends on ARM && OF
> +	depends on (ARM||ARM64) && 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 6eb1aa7..93f64e4 100644
> --- a/drivers/pci/host/pci-host-generic.c
> +++ b/drivers/pci/host/pci-host-generic.c
> @@ -48,8 +48,7 @@ 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 = bus->sysdata;
>  	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
>  
>  	return pci->cfg.win[idx] + ((devfn << 8) | where);
> @@ -64,8 +63,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 = bus->sysdata;
>  	resource_size_t idx = bus->number - pci->cfg.bus_range->start;
>  
>  	return pci->cfg.win[idx] + ((devfn << 12) | where);
> @@ -80,8 +78,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 = bus->sysdata;
>  
>  	addr = pci->cfg.ops->map_bus(bus, devfn, where);
>  
> @@ -103,8 +100,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 = bus->sysdata;
>  
>  	addr = pci->cfg.ops->map_bus(bus, devfn, where);
>  
> @@ -244,13 +240,6 @@ static int gen_pci_parse_map_cfg_windows(struct gen_pci *pci)
>  	return 0;
>  }
>  
> -static int gen_pci_setup(int nr, struct pci_sys_data *sys)
> -{
> -	struct gen_pci *pci = sys->private_data;
> -	list_splice_init(&pci->resources, &sys->resources);
> -	return 1;
> -}
> -
>  static int gen_pci_probe(struct platform_device *pdev)
>  {
>  	int err;
> @@ -260,13 +249,6 @@ static int gen_pci_probe(struct platform_device *pdev)
>  	struct device *dev = &pdev->dev;
>  	struct device_node *np = dev->of_node;
>  	struct gen_pci *pci = devm_kzalloc(dev, sizeof(*pci), GFP_KERNEL);
> -	struct hw_pci hw = {
> -		.nr_controllers	= 1,
> -		.private_data	= (void **)&pci,
> -		.setup		= gen_pci_setup,
> -		.map_irq	= of_irq_parse_and_map_pci,
> -		.ops		= &gen_pci_ops,
> -	};
>  
>  	if (!pci)
>  		return -ENOMEM;
> @@ -303,7 +285,12 @@ static int gen_pci_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	pci_common_init_dev(dev, &hw);
> +	if (!pci_scan_root_bus(&pdev->dev, pci->cfg.bus_range->start,
> +			       &gen_pci_ops, pci, &pci->resources)) {
> +		dev_err(&pdev->dev, "failed to enable PCIe ports\n");
> +		return -ENODEV;
> +	}

We need to assign resources if !PCI_PROBE_ONLY. And to do that we can't
use pci_scan_root_bus() because it adds devices before resources can be
assigned.

Furthermore, the ARM32 pcibios_align_resource() implementation requires
pci_sys_data, so we _still_ rely on pci_common_init_dev to create one
for us.

There are patches in flight to change this behaviour, so for this
series to be applicable there is still some code to be merged first,
we are getting there, bear with us.

Lorenzo

> +
>  	return 0;
>  }
>  
> -- 
> 2.1.0
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/