Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1672099 > unrolled thread

[PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again

Started byArnd Bergmann <arnd@arndb.de>
First post2017-06-22 00:00 +0200
Last post2017-07-02 23:20 +0200
Articles 5 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again Arnd Bergmann <arnd@arndb.de> - 2017-06-22 00:00 +0200
    Re: [PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-06-22 11:40 +0200
      Re: [PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again Bjorn Helgaas <helgaas@kernel.org> - 2017-06-28 01:20 +0200
        Re: [PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2017-07-01 16:00 +0200
          Re: [PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again Bjorn Helgaas <helgaas@kernel.org> - 2017-07-02 23:20 +0200

#1672099 — [PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again

FromArnd Bergmann <arnd@arndb.de>
Date2017-06-22 00:00 +0200
Subject[PATCH 2/4] ARM/PCI: mv78xx0: pass pci_ops correctly again
Message-ID<tUVPk-3BI-29@gated-at.bofh.it>
We used to pass the operations when calling pci_scan_root_bus, but
that argument was removed:

arch/arm/mach-mv78xx0/pcie.c:175:23: error: 'pcie_ops' defined but not used [-Werror=unused-variable]

Setting it in pci_hw should address this.

Fixes: ace27646da8e ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-mv78xx0/pcie.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
index 2b406e909a43..cb7f95c9d8eb 100644
--- a/arch/arm/mach-mv78xx0/pcie.c
+++ b/arch/arm/mach-mv78xx0/pcie.c
@@ -214,6 +214,7 @@ static int __init mv78xx0_pcie_map_irq(const struct pci_dev *dev, u8 slot,
 }
 
 static struct hw_pci mv78xx0_pci __initdata = {
+	.ops		= &pcie_ops,
 	.nr_controllers	= 8,
 	.preinit	= mv78xx0_pcie_preinit,
 	.setup		= mv78xx0_pcie_setup,
-- 
2.9.0

[toc] | [next] | [standalone]

#1672472

FromLorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Date2017-06-22 11:40 +0200
Message-ID<tV6KK-2Nq-25@gated-at.bofh.it>
In reply to#1672099
On Wed, Jun 21, 2017 at 11:53:00PM +0200, Arnd Bergmann wrote:
> We used to pass the operations when calling pci_scan_root_bus, but
> that argument was removed:
> 
> arch/arm/mach-mv78xx0/pcie.c:175:23: error: 'pcie_ops' defined but not used [-Werror=unused-variable]
> 
> Setting it in pci_hw should address this.

No unfortunately it does not. The way I structured the code the
bridge set-up is carried out in the struct hw_pci.scan() callback and
for mv78xx0 it was not done properly.

As said in the other thread we need a pci_fixup_irqs() removal v3 to
address these issues, given that on some host bridges v2 it is still
untested.

Lorenzo

> Fixes: ace27646da8e ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/mach-mv78xx0/pcie.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
> index 2b406e909a43..cb7f95c9d8eb 100644
> --- a/arch/arm/mach-mv78xx0/pcie.c
> +++ b/arch/arm/mach-mv78xx0/pcie.c
> @@ -214,6 +214,7 @@ static int __init mv78xx0_pcie_map_irq(const struct pci_dev *dev, u8 slot,
>  }
>  
>  static struct hw_pci mv78xx0_pci __initdata = {
> +	.ops		= &pcie_ops,
>  	.nr_controllers	= 8,
>  	.preinit	= mv78xx0_pcie_preinit,
>  	.setup		= mv78xx0_pcie_setup,
> -- 
> 2.9.0
>

[toc] | [prev] | [next] | [standalone]

#1676257

FromBjorn Helgaas <helgaas@kernel.org>
Date2017-06-28 01:20 +0200
Message-ID<tX7W2-7nm-13@gated-at.bofh.it>
In reply to#1672472
On Thu, Jun 22, 2017 at 10:38:55AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Jun 21, 2017 at 11:53:00PM +0200, Arnd Bergmann wrote:
> > We used to pass the operations when calling pci_scan_root_bus, but
> > that argument was removed:
> > 
> > arch/arm/mach-mv78xx0/pcie.c:175:23: error: 'pcie_ops' defined but not used [-Werror=unused-variable]
> > 
> > Setting it in pci_hw should address this.
> 
> No unfortunately it does not. The way I structured the code the
> bridge set-up is carried out in the struct hw_pci.scan() callback and
> for mv78xx0 it was not done properly.
> 
> As said in the other thread we need a pci_fixup_irqs() removal v3 to
> address these issues, given that on some host bridges v2 it is still
> untested.
> 
> Lorenzo
> 
> > Fixes: ace27646da8e ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > ---
> >  arch/arm/mach-mv78xx0/pcie.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
> > index 2b406e909a43..cb7f95c9d8eb 100644
> > --- a/arch/arm/mach-mv78xx0/pcie.c
> > +++ b/arch/arm/mach-mv78xx0/pcie.c
> > @@ -214,6 +214,7 @@ static int __init mv78xx0_pcie_map_irq(const struct pci_dev *dev, u8 slot,
> >  }
> >  
> >  static struct hw_pci mv78xx0_pci __initdata = {
> > +	.ops		= &pcie_ops,
> >  	.nr_controllers	= 8,
> >  	.preinit	= mv78xx0_pcie_preinit,
> >  	.setup		= mv78xx0_pcie_setup,

I provisionally made the following change for mv78xx0, based on similar
hunks for dove, iop13xx, and orion5x.

This is on my pci/enumeration branch:

https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/enumeration&id=31ddd0eab68e19cf3dfc14321c99ff0b95cda4ad

Please take a look and see if it makes sense.


diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
index 2b406e909a43..f793ebbd7539 100644
--- a/arch/arm/mach-mv78xx0/pcie.c
+++ b/arch/arm/mach-mv78xx0/pcie.c
@@ -201,6 +201,7 @@ static int __init mv78xx0_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
 		return -EINVAL;
 	}
 
+	bridge->ops = &pcie_ops;
 	return pci_scan_root_bus_bridge(bridge);
 }

[toc] | [prev] | [next] | [standalone]

#1679367

FromLorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Date2017-07-01 16:00 +0200
Message-ID<tYr6h-3TJ-3@gated-at.bofh.it>
In reply to#1676257
Hi Bjorn,

On Tue, Jun 27, 2017 at 06:11:43PM -0500, Bjorn Helgaas wrote:
> On Thu, Jun 22, 2017 at 10:38:55AM +0100, Lorenzo Pieralisi wrote:
> > On Wed, Jun 21, 2017 at 11:53:00PM +0200, Arnd Bergmann wrote:
> > > We used to pass the operations when calling pci_scan_root_bus, but
> > > that argument was removed:
> > > 
> > > arch/arm/mach-mv78xx0/pcie.c:175:23: error: 'pcie_ops' defined but not used [-Werror=unused-variable]
> > > 
> > > Setting it in pci_hw should address this.
> > 
> > No unfortunately it does not. The way I structured the code the
> > bridge set-up is carried out in the struct hw_pci.scan() callback and
> > for mv78xx0 it was not done properly.
> > 
> > As said in the other thread we need a pci_fixup_irqs() removal v3 to
> > address these issues, given that on some host bridges v2 it is still
> > untested.
> > 
> > Lorenzo
> > 
> > > Fixes: ace27646da8e ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")
> > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > ---
> > >  arch/arm/mach-mv78xx0/pcie.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
> > > index 2b406e909a43..cb7f95c9d8eb 100644
> > > --- a/arch/arm/mach-mv78xx0/pcie.c
> > > +++ b/arch/arm/mach-mv78xx0/pcie.c
> > > @@ -214,6 +214,7 @@ static int __init mv78xx0_pcie_map_irq(const struct pci_dev *dev, u8 slot,
> > >  }
> > >  
> > >  static struct hw_pci mv78xx0_pci __initdata = {
> > > +	.ops		= &pcie_ops,
> > >  	.nr_controllers	= 8,
> > >  	.preinit	= mv78xx0_pcie_preinit,
> > >  	.setup		= mv78xx0_pcie_setup,
> 
> I provisionally made the following change for mv78xx0, based on similar
> hunks for dove, iop13xx, and orion5x.
> 
> This is on my pci/enumeration branch:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/enumeration&id=31ddd0eab68e19cf3dfc14321c99ff0b95cda4ad
> 
> Please take a look and see if it makes sense.

It makes code compile but it is missing some initializations.

Patch below, to be folded in commit

e592c4a52cb8 ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")

in your pci/irq-fixups branch, I did that and recompiled (I can't test
this code) and everything seems fine.

Thanks a lot !

Lorenzo

-- >8 --
diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
index f793ebb..636d84b 100644
--- a/arch/arm/mach-mv78xx0/pcie.c
+++ b/arch/arm/mach-mv78xx0/pcie.c
@@ -196,12 +196,19 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup);
 
 static int __init mv78xx0_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
 {
+	struct pci_sys_data *sys = pci_host_bridge_priv(bridge);
+
 	if (nr >= num_pcie_ports) {
 		BUG();
 		return -EINVAL;
 	}
 
+	list_splice_init(&sys->resources, &bridge->windows);
+	bridge->dev.parent = NULL;
+	bridge->sysdata = sys;
+	bridge->busnr = sys->busnr;
 	bridge->ops = &pcie_ops;
+
 	return pci_scan_root_bus_bridge(bridge);
 }
 
-- 
2.10.0

[toc] | [prev] | [next] | [standalone]

#1679563

FromBjorn Helgaas <helgaas@kernel.org>
Date2017-07-02 23:20 +0200
Message-ID<tYUrE-7OP-19@gated-at.bofh.it>
In reply to#1679367
On Sat, Jul 01, 2017 at 02:54:57PM +0100, Lorenzo Pieralisi wrote:
> Hi Bjorn,
> 
> On Tue, Jun 27, 2017 at 06:11:43PM -0500, Bjorn Helgaas wrote:
> > On Thu, Jun 22, 2017 at 10:38:55AM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Jun 21, 2017 at 11:53:00PM +0200, Arnd Bergmann wrote:
> > > > We used to pass the operations when calling pci_scan_root_bus, but
> > > > that argument was removed:
> > > > 
> > > > arch/arm/mach-mv78xx0/pcie.c:175:23: error: 'pcie_ops' defined but not used [-Werror=unused-variable]
> > > > 
> > > > Setting it in pci_hw should address this.
> > > 
> > > No unfortunately it does not. The way I structured the code the
> > > bridge set-up is carried out in the struct hw_pci.scan() callback and
> > > for mv78xx0 it was not done properly.
> > > 
> > > As said in the other thread we need a pci_fixup_irqs() removal v3 to
> > > address these issues, given that on some host bridges v2 it is still
> > > untested.
> > > 
> > > Lorenzo
> > > 
> > > > Fixes: ace27646da8e ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")
> > > > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> > > > ---
> > > >  arch/arm/mach-mv78xx0/pcie.c | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > > 
> > > > diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
> > > > index 2b406e909a43..cb7f95c9d8eb 100644
> > > > --- a/arch/arm/mach-mv78xx0/pcie.c
> > > > +++ b/arch/arm/mach-mv78xx0/pcie.c
> > > > @@ -214,6 +214,7 @@ static int __init mv78xx0_pcie_map_irq(const struct pci_dev *dev, u8 slot,
> > > >  }
> > > >  
> > > >  static struct hw_pci mv78xx0_pci __initdata = {
> > > > +	.ops		= &pcie_ops,
> > > >  	.nr_controllers	= 8,
> > > >  	.preinit	= mv78xx0_pcie_preinit,
> > > >  	.setup		= mv78xx0_pcie_setup,
> > 
> > I provisionally made the following change for mv78xx0, based on similar
> > hunks for dove, iop13xx, and orion5x.
> > 
> > This is on my pci/enumeration branch:
> > 
> > https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/enumeration&id=31ddd0eab68e19cf3dfc14321c99ff0b95cda4ad
> > 
> > Please take a look and see if it makes sense.
> 
> It makes code compile but it is missing some initializations.
> 
> Patch below, to be folded in commit
> 
> e592c4a52cb8 ("ARM/PCI: Convert PCI scan API to pci_scan_root_bus_bridge()")
> 
> in your pci/irq-fixups branch, I did that and recompiled (I can't test
> this code) and everything seems fine.

Folded into pci/irq-fixups, thanks!

> -- >8 --
> diff --git a/arch/arm/mach-mv78xx0/pcie.c b/arch/arm/mach-mv78xx0/pcie.c
> index f793ebb..636d84b 100644
> --- a/arch/arm/mach-mv78xx0/pcie.c
> +++ b/arch/arm/mach-mv78xx0/pcie.c
> @@ -196,12 +196,19 @@ DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MARVELL, PCI_ANY_ID, rc_pci_fixup);
>  
>  static int __init mv78xx0_pcie_scan_bus(int nr, struct pci_host_bridge *bridge)
>  {
> +	struct pci_sys_data *sys = pci_host_bridge_priv(bridge);
> +
>  	if (nr >= num_pcie_ports) {
>  		BUG();
>  		return -EINVAL;
>  	}
>  
> +	list_splice_init(&sys->resources, &bridge->windows);
> +	bridge->dev.parent = NULL;
> +	bridge->sysdata = sys;
> +	bridge->busnr = sys->busnr;
>  	bridge->ops = &pcie_ops;
> +
>  	return pci_scan_root_bus_bridge(bridge);
>  }
>  
> -- 
> 2.10.0
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web