diff mbox

[RFC,2/4] PCI: generic: Add support for ARM64 and MSI(x)

Message ID 2148776.X8NPqiYA6S@wuerfel (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Arnd Bergmann Oct. 22, 2014, 8:52 p.m. UTC
On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> 
> [...]
> 
> > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > removed if we change the arm32 pcibios_init_hw function to call the new
> > interfaces that set the domain number.
> 
> I wished, but it is a bit more complicated than I thought unfortunately,
> mostly because some drivers, eg cns3xxx set the domain numbers
> statically in pci_sys_data and this sets a chain of dependency that is
> not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> the domain number (in pci_sys_data) in a way that clashes with the
> generic domain_nr implementation, I need to give it more thought.

Just had a look at that driver, shouldn't be too hard to change, see below.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>



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

Comments

Liviu Dudau Oct. 23, 2014, 9:13 a.m. UTC | #1
On Wed, Oct 22, 2014 at 09:52:19PM +0100, Arnd Bergmann wrote:
> On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> > On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> > 
> > [...]
> > 
> > > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > > removed if we change the arm32 pcibios_init_hw function to call the new
> > > interfaces that set the domain number.
> > 
> > I wished, but it is a bit more complicated than I thought unfortunately,
> > mostly because some drivers, eg cns3xxx set the domain numbers
> > statically in pci_sys_data and this sets a chain of dependency that is
> > not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> > the domain number (in pci_sys_data) in a way that clashes with the
> > generic domain_nr implementation, I need to give it more thought.
> 
> Just had a look at that driver, shouldn't be too hard to change, see below.

I like this!

One thing though ...

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
> index 45d6bd09e6ef..aa4b9d7c52fd 100644
> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -30,18 +30,15 @@ struct cns3xxx_pcie {
>  	unsigned int irqs[2];
>  	struct resource res_io;
>  	struct resource res_mem;
> -	struct hw_pci hw_pci;
> -
> +	int port;
>  	bool linked;
>  };
>  
> -static struct cns3xxx_pcie cns3xxx_pcie[]; /* forward decl. */
> -
>  static struct cns3xxx_pcie *sysdata_to_cnspci(void *sysdata)
>  {
>  	struct pci_sys_data *root = sysdata;
>  
> -	return &cns3xxx_pcie[root->domain];
> +	return root->private_data;
>  }
>  
>  static struct cns3xxx_pcie *pdev_to_cnspci(const struct pci_dev *dev)
> @@ -192,13 +189,7 @@ static struct cns3xxx_pcie cns3xxx_pcie[] = {
>  			.flags = IORESOURCE_MEM,
>  		},
>  		.irqs = { IRQ_CNS3XXX_PCIE0_RC, IRQ_CNS3XXX_PCIE0_DEVICE, },
> -		.hw_pci = {
> -			.domain = 0,
> -			.nr_controllers = 1,
> -			.ops = &cns3xxx_pcie_ops,
> -			.setup = cns3xxx_pci_setup,
> -			.map_irq = cns3xxx_pcie_map_irq,
> -		},
> +		.port = 0,
>  	},
>  	[1] = {
>  		.host_regs = (void __iomem *)CNS3XXX_PCIE1_HOST_BASE_VIRT,
> @@ -217,19 +208,13 @@ static struct cns3xxx_pcie cns3xxx_pcie[] = {
>  			.flags = IORESOURCE_MEM,
>  		},
>  		.irqs = { IRQ_CNS3XXX_PCIE1_RC, IRQ_CNS3XXX_PCIE1_DEVICE, },
> -		.hw_pci = {
> -			.domain = 1,
> -			.nr_controllers = 1,
> -			.ops = &cns3xxx_pcie_ops,
> -			.setup = cns3xxx_pci_setup,
> -			.map_irq = cns3xxx_pcie_map_irq,
> -		},
> +		.port = 1,
>  	},
>  };
>  
>  static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
>  {
> -	int port = cnspci->hw_pci.domain;
> +	int port = cnspci->port;
>  	u32 reg;
>  	unsigned long time;
>  
> @@ -260,9 +245,10 @@ static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
>  
>  static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci)
>  {
> -	int port = cnspci->hw_pci.domain;
> +	int port = cnspci->port;
>  	struct pci_sys_data sd = {
>  		.domain = port,
> +		.private_data = cnspci,
>  	};
>  	struct pci_bus bus = {
>  		.number = 0,
> @@ -323,6 +309,14 @@ static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
>  void __init cns3xxx_pcie_init_late(void)
>  {
>  	int i;
> +	void *private_data;
> +	struct hw_pci hw_pci = {
> +		.nr_controllers = 1,
> +		.ops = &cns3xxx_pcie_ops,
> +		.setup = cns3xxx_pci_setup,
> +		.map_irq = cns3xxx_pcie_map_irq,
> +		.private_data = &private_data,
> +	};
>  
>  	pcibios_min_io = 0;
>  	pcibios_min_mem = 0;
> @@ -335,7 +329,9 @@ void __init cns3xxx_pcie_init_late(void)
>  		cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
>  		cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
>  		cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
> -		pci_common_init(&cns3xxx_pcie[i].hw_pci);
> +		hw_pci->domain = i;
> +		private_data = &cns3xxx_pcie[i];

Is this dance with pointers absolutely necessary? Does gcc though dishes at you
for doing hw_pci->private_data = &cns3xxx_pcie[i] directly?

Best regards,
Liviu

> +		pci_common_init(&hw_pci);
>  	}
>  
>  	pci_assign_unassigned_resources();
> 
> 
> 
>
Lorenzo Pieralisi Oct. 23, 2014, 11:27 a.m. UTC | #2
On Thu, Oct 23, 2014 at 10:13:09AM +0100, Liviu Dudau wrote:
> On Wed, Oct 22, 2014 at 09:52:19PM +0100, Arnd Bergmann wrote:
> > On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> > > On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> > > 
> > > [...]
> > > 
> > > > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > > > removed if we change the arm32 pcibios_init_hw function to call the new
> > > > interfaces that set the domain number.
> > > 
> > > I wished, but it is a bit more complicated than I thought unfortunately,
> > > mostly because some drivers, eg cns3xxx set the domain numbers
> > > statically in pci_sys_data and this sets a chain of dependency that is
> > > not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> > > the domain number (in pci_sys_data) in a way that clashes with the
> > > generic domain_nr implementation, I need to give it more thought.
> > 
> > Just had a look at that driver, shouldn't be too hard to change, see below.
> 
> I like this!
> 
> One thing though ...

I like it too, it is one way of removing the artificial domain dependency
from this driver.

I think that by removing that, we could switch to CONFIG_PCI_DOMAINS_GENERIC
on ARM32. I will remove the dependency in drivers/pci/host/pci-mvebu.c
introduced by commit 2613ba48. pci_sys_data.domain is always 0 in that
driver so its usefulness is doubtful, comments welcome, copied Jason in
if he has comments.

[...]

> > @@ -323,6 +309,14 @@ static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
> >  void __init cns3xxx_pcie_init_late(void)
> >  {
> >  	int i;
> > +	void *private_data;
> > +	struct hw_pci hw_pci = {
> > +		.nr_controllers = 1,
> > +		.ops = &cns3xxx_pcie_ops,
> > +		.setup = cns3xxx_pci_setup,
> > +		.map_irq = cns3xxx_pcie_map_irq,
> > +		.private_data = &private_data,
> > +	};
> >  
> >  	pcibios_min_io = 0;
> >  	pcibios_min_mem = 0;
> > @@ -335,7 +329,9 @@ void __init cns3xxx_pcie_init_late(void)
> >  		cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
> >  		cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
> >  		cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
> > -		pci_common_init(&cns3xxx_pcie[i].hw_pci);
> > +		hw_pci->domain = i;

+		hw_pci.domain = i;

I will remove this since if we move to generic domains it is useless to
pass the value through hw_pci.

> > +		private_data = &cns3xxx_pcie[i];
> 
> Is this dance with pointers absolutely necessary? Does gcc though dishes at you
> for doing hw_pci->private_data = &cns3xxx_pcie[i] directly?

You can't, hw_pci.private_data is void **.

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
Jason Gunthorpe Oct. 23, 2014, 4:52 p.m. UTC | #3
On Thu, Oct 23, 2014 at 12:27:31PM +0100, Lorenzo Pieralisi wrote:

> I think that by removing that, we could switch to CONFIG_PCI_DOMAINS_GENERIC
> on ARM32. I will remove the dependency in drivers/pci/host/pci-mvebu.c
> introduced by commit 2613ba48. pci_sys_data.domain is always 0 in that
> driver so its usefulness is doubtful, comments welcome, copied Jason in
> if he has comments.

pcie-mvebu is like all the other new drivers, each top level DT node
that introduces the interface should have a unique domain number. It
would be very strange (and currently unsupported by the driver) to
ever have more than 1 mvebu top level node in any DT.

Jason
--
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 Oct. 27, 2014, 4:10 p.m. UTC | #4
On Thu, Oct 23, 2014 at 05:52:06PM +0100, Jason Gunthorpe wrote:
> On Thu, Oct 23, 2014 at 12:27:31PM +0100, Lorenzo Pieralisi wrote:
> 
> > I think that by removing that, we could switch to CONFIG_PCI_DOMAINS_GENERIC
> > on ARM32. I will remove the dependency in drivers/pci/host/pci-mvebu.c
> > introduced by commit 2613ba48. pci_sys_data.domain is always 0 in that
> > driver so its usefulness is doubtful, comments welcome, copied Jason in
> > if he has comments.
> 
> pcie-mvebu is like all the other new drivers, each top level DT node
> that introduces the interface should have a unique domain number. It
> would be very strange (and currently unsupported by the driver) to
> ever have more than 1 mvebu top level node in any DT.

Which as a matter of fact I should take as pci_sys_data.domain is
useless on pci-mvebu.c, since that value will always be 0 (at least it
is in the current driver):

#ifdef CONFIG_PCI_DOMAINS
	domain = sys->domain;
#endif

Am I missing something ? Is that domain number meant to be used for anything
else ?

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
Bjorn Helgaas Nov. 5, 2014, 11:39 p.m. UTC | #5
On Wed, Oct 22, 2014 at 10:52:19PM +0200, Arnd Bergmann wrote:
> On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> > On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> > 
> > [...]
> > 
> > > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > > removed if we change the arm32 pcibios_init_hw function to call the new
> > > interfaces that set the domain number.
> > 
> > I wished, but it is a bit more complicated than I thought unfortunately,
> > mostly because some drivers, eg cns3xxx set the domain numbers
> > statically in pci_sys_data and this sets a chain of dependency that is
> > not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> > the domain number (in pci_sys_data) in a way that clashes with the
> > generic domain_nr implementation, I need to give it more thought.
> 
> Just had a look at that driver, shouldn't be too hard to change, see below.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

This patch is in my patchwork, but it lacks a topic & changelog and I'm not
sure of its state, so I'm going to drop it for now.  Please post it again
if you want me to do something with it.  I guess it only touches arch/arm,
so it would probably be merged via your tree anyway.

Bjorn

> diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
> index 45d6bd09e6ef..aa4b9d7c52fd 100644
> --- a/arch/arm/mach-cns3xxx/pcie.c
> +++ b/arch/arm/mach-cns3xxx/pcie.c
> @@ -30,18 +30,15 @@ struct cns3xxx_pcie {
>  	unsigned int irqs[2];
>  	struct resource res_io;
>  	struct resource res_mem;
> -	struct hw_pci hw_pci;
> -
> +	int port;
>  	bool linked;
>  };
>  
> -static struct cns3xxx_pcie cns3xxx_pcie[]; /* forward decl. */
> -
>  static struct cns3xxx_pcie *sysdata_to_cnspci(void *sysdata)
>  {
>  	struct pci_sys_data *root = sysdata;
>  
> -	return &cns3xxx_pcie[root->domain];
> +	return root->private_data;
>  }
>  
>  static struct cns3xxx_pcie *pdev_to_cnspci(const struct pci_dev *dev)
> @@ -192,13 +189,7 @@ static struct cns3xxx_pcie cns3xxx_pcie[] = {
>  			.flags = IORESOURCE_MEM,
>  		},
>  		.irqs = { IRQ_CNS3XXX_PCIE0_RC, IRQ_CNS3XXX_PCIE0_DEVICE, },
> -		.hw_pci = {
> -			.domain = 0,
> -			.nr_controllers = 1,
> -			.ops = &cns3xxx_pcie_ops,
> -			.setup = cns3xxx_pci_setup,
> -			.map_irq = cns3xxx_pcie_map_irq,
> -		},
> +		.port = 0,
>  	},
>  	[1] = {
>  		.host_regs = (void __iomem *)CNS3XXX_PCIE1_HOST_BASE_VIRT,
> @@ -217,19 +208,13 @@ static struct cns3xxx_pcie cns3xxx_pcie[] = {
>  			.flags = IORESOURCE_MEM,
>  		},
>  		.irqs = { IRQ_CNS3XXX_PCIE1_RC, IRQ_CNS3XXX_PCIE1_DEVICE, },
> -		.hw_pci = {
> -			.domain = 1,
> -			.nr_controllers = 1,
> -			.ops = &cns3xxx_pcie_ops,
> -			.setup = cns3xxx_pci_setup,
> -			.map_irq = cns3xxx_pcie_map_irq,
> -		},
> +		.port = 1,
>  	},
>  };
>  
>  static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
>  {
> -	int port = cnspci->hw_pci.domain;
> +	int port = cnspci->port;
>  	u32 reg;
>  	unsigned long time;
>  
> @@ -260,9 +245,10 @@ static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
>  
>  static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci)
>  {
> -	int port = cnspci->hw_pci.domain;
> +	int port = cnspci->port;
>  	struct pci_sys_data sd = {
>  		.domain = port,
> +		.private_data = cnspci,
>  	};
>  	struct pci_bus bus = {
>  		.number = 0,
> @@ -323,6 +309,14 @@ static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
>  void __init cns3xxx_pcie_init_late(void)
>  {
>  	int i;
> +	void *private_data;
> +	struct hw_pci hw_pci = {
> +		.nr_controllers = 1,
> +		.ops = &cns3xxx_pcie_ops,
> +		.setup = cns3xxx_pci_setup,
> +		.map_irq = cns3xxx_pcie_map_irq,
> +		.private_data = &private_data,
> +	};
>  
>  	pcibios_min_io = 0;
>  	pcibios_min_mem = 0;
> @@ -335,7 +329,9 @@ void __init cns3xxx_pcie_init_late(void)
>  		cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
>  		cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
>  		cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
> -		pci_common_init(&cns3xxx_pcie[i].hw_pci);
> +		hw_pci->domain = i;
> +		private_data = &cns3xxx_pcie[i];
> +		pci_common_init(&hw_pci);
>  	}
>  
>  	pci_assign_unassigned_resources();
> 
> 
--
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
Arnd Bergmann Nov. 6, 2014, 12:05 a.m. UTC | #6
On Wednesday 05 November 2014 16:39:21 Bjorn Helgaas wrote:
> On Wed, Oct 22, 2014 at 10:52:19PM +0200, Arnd Bergmann wrote:
> > On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> > > On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> > > 
> > > [...]
> > > 
> > > > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > > > removed if we change the arm32 pcibios_init_hw function to call the new
> > > > interfaces that set the domain number.
> > > 
> > > I wished, but it is a bit more complicated than I thought unfortunately,
> > > mostly because some drivers, eg cns3xxx set the domain numbers
> > > statically in pci_sys_data and this sets a chain of dependency that is
> > > not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> > > the domain number (in pci_sys_data) in a way that clashes with the
> > > generic domain_nr implementation, I need to give it more thought.
> > 
> > Just had a look at that driver, shouldn't be too hard to change, see below.
> > 
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> This patch is in my patchwork, but it lacks a topic & changelog and I'm not
> sure of its state, so I'm going to drop it for now.  Please post it again
> if you want me to do something with it.  I guess it only touches arch/arm,
> so it would probably be merged via your tree anyway.

Lorenzo has posted an updated version as

"arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain",
and a second patch that depends on it. That is the version we should be
merging, though I'm not sure through which tree.

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lorenzo Pieralisi Nov. 6, 2014, 9:52 a.m. UTC | #7
On Thu, Nov 06, 2014 at 12:05:48AM +0000, Arnd Bergmann wrote:
> On Wednesday 05 November 2014 16:39:21 Bjorn Helgaas wrote:
> > On Wed, Oct 22, 2014 at 10:52:19PM +0200, Arnd Bergmann wrote:
> > > On Wednesday 22 October 2014 16:59:14 Lorenzo Pieralisi wrote:
> > > > On Wed, Oct 01, 2014 at 10:38:45AM +0100, Arnd Bergmann wrote:
> > > > 
> > > > [...]
> > > > 
> > > > > The arm32 implementations of pci_domain_nr/pci_proc_domain can probably be
> > > > > removed if we change the arm32 pcibios_init_hw function to call the new
> > > > > interfaces that set the domain number.
> > > > 
> > > > I wished, but it is a bit more complicated than I thought unfortunately,
> > > > mostly because some drivers, eg cns3xxx set the domain numbers
> > > > statically in pci_sys_data and this sets a chain of dependency that is
> > > > not easy to untangle. I think cns3xxx is the only legacy driver that "uses"
> > > > the domain number (in pci_sys_data) in a way that clashes with the
> > > > generic domain_nr implementation, I need to give it more thought.
> > > 
> > > Just had a look at that driver, shouldn't be too hard to change, see below.
> > > 
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > 
> > This patch is in my patchwork, but it lacks a topic & changelog and I'm not
> > sure of its state, so I'm going to drop it for now.  Please post it again
> > if you want me to do something with it.  I guess it only touches arch/arm,
> > so it would probably be merged via your tree anyway.
> 
> Lorenzo has posted an updated version as
> 
> "arm: cns3xxx: pci: remove artificial dependency on pci_sys_data domain",
> and a second patch that depends on it. That is the version we should be
> merging, though I'm not sure through which tree.

I am posting a v2 shortly, let's discuss the best way to merge it then.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/arm/mach-cns3xxx/pcie.c b/arch/arm/mach-cns3xxx/pcie.c
index 45d6bd09e6ef..aa4b9d7c52fd 100644
--- a/arch/arm/mach-cns3xxx/pcie.c
+++ b/arch/arm/mach-cns3xxx/pcie.c
@@ -30,18 +30,15 @@  struct cns3xxx_pcie {
 	unsigned int irqs[2];
 	struct resource res_io;
 	struct resource res_mem;
-	struct hw_pci hw_pci;
-
+	int port;
 	bool linked;
 };
 
-static struct cns3xxx_pcie cns3xxx_pcie[]; /* forward decl. */
-
 static struct cns3xxx_pcie *sysdata_to_cnspci(void *sysdata)
 {
 	struct pci_sys_data *root = sysdata;
 
-	return &cns3xxx_pcie[root->domain];
+	return root->private_data;
 }
 
 static struct cns3xxx_pcie *pdev_to_cnspci(const struct pci_dev *dev)
@@ -192,13 +189,7 @@  static struct cns3xxx_pcie cns3xxx_pcie[] = {
 			.flags = IORESOURCE_MEM,
 		},
 		.irqs = { IRQ_CNS3XXX_PCIE0_RC, IRQ_CNS3XXX_PCIE0_DEVICE, },
-		.hw_pci = {
-			.domain = 0,
-			.nr_controllers = 1,
-			.ops = &cns3xxx_pcie_ops,
-			.setup = cns3xxx_pci_setup,
-			.map_irq = cns3xxx_pcie_map_irq,
-		},
+		.port = 0,
 	},
 	[1] = {
 		.host_regs = (void __iomem *)CNS3XXX_PCIE1_HOST_BASE_VIRT,
@@ -217,19 +208,13 @@  static struct cns3xxx_pcie cns3xxx_pcie[] = {
 			.flags = IORESOURCE_MEM,
 		},
 		.irqs = { IRQ_CNS3XXX_PCIE1_RC, IRQ_CNS3XXX_PCIE1_DEVICE, },
-		.hw_pci = {
-			.domain = 1,
-			.nr_controllers = 1,
-			.ops = &cns3xxx_pcie_ops,
-			.setup = cns3xxx_pci_setup,
-			.map_irq = cns3xxx_pcie_map_irq,
-		},
+		.port = 1,
 	},
 };
 
 static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
 {
-	int port = cnspci->hw_pci.domain;
+	int port = cnspci->port;
 	u32 reg;
 	unsigned long time;
 
@@ -260,9 +245,10 @@  static void __init cns3xxx_pcie_check_link(struct cns3xxx_pcie *cnspci)
 
 static void __init cns3xxx_pcie_hw_init(struct cns3xxx_pcie *cnspci)
 {
-	int port = cnspci->hw_pci.domain;
+	int port = cnspci->port;
 	struct pci_sys_data sd = {
 		.domain = port,
+		.private_data = cnspci,
 	};
 	struct pci_bus bus = {
 		.number = 0,
@@ -323,6 +309,14 @@  static int cns3xxx_pcie_abort_handler(unsigned long addr, unsigned int fsr,
 void __init cns3xxx_pcie_init_late(void)
 {
 	int i;
+	void *private_data;
+	struct hw_pci hw_pci = {
+		.nr_controllers = 1,
+		.ops = &cns3xxx_pcie_ops,
+		.setup = cns3xxx_pci_setup,
+		.map_irq = cns3xxx_pcie_map_irq,
+		.private_data = &private_data,
+	};
 
 	pcibios_min_io = 0;
 	pcibios_min_mem = 0;
@@ -335,7 +329,9 @@  void __init cns3xxx_pcie_init_late(void)
 		cns3xxx_pwr_soft_rst(0x1 << PM_SOFT_RST_REG_OFFST_PCIE(i));
 		cns3xxx_pcie_check_link(&cns3xxx_pcie[i]);
 		cns3xxx_pcie_hw_init(&cns3xxx_pcie[i]);
-		pci_common_init(&cns3xxx_pcie[i].hw_pci);
+		hw_pci->domain = i;
+		private_data = &cns3xxx_pcie[i];
+		pci_common_init(&hw_pci);
 	}
 
 	pci_assign_unassigned_resources();