diff mbox

[3/3] arm/arm64: pci: remove arch specific pcibios_enable_device()

Message ID 1447779838-15585-3-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Lorenzo Pieralisi Nov. 17, 2015, 5:03 p.m. UTC
The arm/arm64 pcibios_enable_device() implementations exist solely
to prevent enabling PCI resources on PROBE_ONLY systems, since
on those systems the PCI resources are currently not claimed (ie
validated and inserted in the PCI resource tree) therefore they can
not be enabled since this would trigger PCI set-ups failures.

By introducing resources claiming in the PCI host controllers set-ups
that have PROBE_ONLY as a probe option, there is no need for arch specific
pcibios_enable_device() implementations anymore in that the kernel can
rely on the generic pcibios_enable_device() implementation without
resorting to arch specific code to work around the missing resources
claiming enumeration step.

This patch removes the pcibios_enable_device() implementations from
the arm/arm64 arch back-ends.

Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/kernel/bios32.c | 12 ------------
 arch/arm64/kernel/pci.c  | 13 -------------
 2 files changed, 25 deletions(-)

Comments

Will Deacon Nov. 23, 2015, 1:49 p.m. UTC | #1
On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> The arm/arm64 pcibios_enable_device() implementations exist solely
> to prevent enabling PCI resources on PROBE_ONLY systems, since
> on those systems the PCI resources are currently not claimed (ie
> validated and inserted in the PCI resource tree) therefore they can
> not be enabled since this would trigger PCI set-ups failures.
> 
> By introducing resources claiming in the PCI host controllers set-ups
> that have PROBE_ONLY as a probe option, there is no need for arch specific
> pcibios_enable_device() implementations anymore in that the kernel can
> rely on the generic pcibios_enable_device() implementation without
> resorting to arch specific code to work around the missing resources
> claiming enumeration step.
> 
> This patch removes the pcibios_enable_device() implementations from
> the arm/arm64 arch back-ends.
> 
> Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Russell King <linux@arm.linux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> ---
>  arch/arm/kernel/bios32.c | 12 ------------
>  arch/arm64/kernel/pci.c  | 13 -------------
>  2 files changed, 25 deletions(-)

Acked-by: Will Deacon <will.deacon@arm.com>

Will
--
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 Jan. 20, 2016, 4:22 p.m. UTC | #2
On Mon, Nov 23, 2015 at 01:49:21PM +0000, Will Deacon wrote:
> On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> > The arm/arm64 pcibios_enable_device() implementations exist solely
> > to prevent enabling PCI resources on PROBE_ONLY systems, since
> > on those systems the PCI resources are currently not claimed (ie
> > validated and inserted in the PCI resource tree) therefore they can
> > not be enabled since this would trigger PCI set-ups failures.
> > 
> > By introducing resources claiming in the PCI host controllers set-ups
> > that have PROBE_ONLY as a probe option, there is no need for arch specific
> > pcibios_enable_device() implementations anymore in that the kernel can
> > rely on the generic pcibios_enable_device() implementation without
> > resorting to arch specific code to work around the missing resources
> > claiming enumeration step.
> > 
> > This patch removes the pcibios_enable_device() implementations from
> > the arm/arm64 arch back-ends.
> > 
> > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > Cc: Arnd Bergmann <arnd@arndb.de>
> > Cc: Will Deacon <will.deacon@arm.com>
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: Russell King <linux@arm.linux.org.uk>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > ---
> >  arch/arm/kernel/bios32.c | 12 ------------
> >  arch/arm64/kernel/pci.c  | 13 -------------
> >  2 files changed, 25 deletions(-)
> 
> Acked-by: Will Deacon <will.deacon@arm.com>

Thanks, unfortunately I spotted that ARM platforms can set
PCI_PROBE_ONLY also via command line (pcibios_setup()), which means
that I have to add resource claiming to all ARM PCI controllers that
check PCI_PROBE_ONLY (inclusive of ARM bios32) to really make sure we
can apply this patch or alternatevely we add the resource claiming
to the pcibios_fixup_bus() callback (but we can claim resources only if
PCI_PROBE_ONLY is set lest we trigger regressions) which would be the
simpler solution.

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 Feb. 25, 2016, 3:24 p.m. UTC | #3
Hi Lorenzo,

On Wed, Jan 20, 2016 at 04:22:15PM +0000, Lorenzo Pieralisi wrote:
> On Mon, Nov 23, 2015 at 01:49:21PM +0000, Will Deacon wrote:
> > On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> > > The arm/arm64 pcibios_enable_device() implementations exist solely
> > > to prevent enabling PCI resources on PROBE_ONLY systems, since
> > > on those systems the PCI resources are currently not claimed (ie
> > > validated and inserted in the PCI resource tree) therefore they can
> > > not be enabled since this would trigger PCI set-ups failures.
> > > 
> > > By introducing resources claiming in the PCI host controllers set-ups
> > > that have PROBE_ONLY as a probe option, there is no need for arch specific
> > > pcibios_enable_device() implementations anymore in that the kernel can
> > > rely on the generic pcibios_enable_device() implementation without
> > > resorting to arch specific code to work around the missing resources
> > > claiming enumeration step.
> > > 
> > > This patch removes the pcibios_enable_device() implementations from
> > > the arm/arm64 arch back-ends.
> > > 
> > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > Cc: Will Deacon <will.deacon@arm.com>
> > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > Cc: Russell King <linux@arm.linux.org.uk>
> > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > ---
> > >  arch/arm/kernel/bios32.c | 12 ------------
> > >  arch/arm64/kernel/pci.c  | 13 -------------
> > >  2 files changed, 25 deletions(-)
> > 
> > Acked-by: Will Deacon <will.deacon@arm.com>
> 
> Thanks, unfortunately I spotted that ARM platforms can set
> PCI_PROBE_ONLY also via command line (pcibios_setup()), which means
> that I have to add resource claiming to all ARM PCI controllers that
> check PCI_PROBE_ONLY (inclusive of ARM bios32) to really make sure we
> can apply this patch or alternatevely we add the resource claiming
> to the pcibios_fixup_bus() callback (but we can claim resources only if
> PCI_PROBE_ONLY is set lest we trigger regressions) which would be the
> simpler solution.

So where are we?  The above response sounds like this series still
needs a little tweaking?

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
Lorenzo Pieralisi Feb. 25, 2016, 4:05 p.m. UTC | #4
Hi Bjorn,

On Thu, Feb 25, 2016 at 09:24:17AM -0600, Bjorn Helgaas wrote:
> Hi Lorenzo,
> 
> On Wed, Jan 20, 2016 at 04:22:15PM +0000, Lorenzo Pieralisi wrote:
> > On Mon, Nov 23, 2015 at 01:49:21PM +0000, Will Deacon wrote:
> > > On Tue, Nov 17, 2015 at 05:03:58PM +0000, Lorenzo Pieralisi wrote:
> > > > The arm/arm64 pcibios_enable_device() implementations exist solely
> > > > to prevent enabling PCI resources on PROBE_ONLY systems, since
> > > > on those systems the PCI resources are currently not claimed (ie
> > > > validated and inserted in the PCI resource tree) therefore they can
> > > > not be enabled since this would trigger PCI set-ups failures.
> > > > 
> > > > By introducing resources claiming in the PCI host controllers set-ups
> > > > that have PROBE_ONLY as a probe option, there is no need for arch specific
> > > > pcibios_enable_device() implementations anymore in that the kernel can
> > > > rely on the generic pcibios_enable_device() implementation without
> > > > resorting to arch specific code to work around the missing resources
> > > > claiming enumeration step.
> > > > 
> > > > This patch removes the pcibios_enable_device() implementations from
> > > > the arm/arm64 arch back-ends.
> > > > 
> > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> > > > Cc: Arnd Bergmann <arnd@arndb.de>
> > > > Cc: Will Deacon <will.deacon@arm.com>
> > > > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > > > Cc: Russell King <linux@arm.linux.org.uk>
> > > > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > > > ---
> > > >  arch/arm/kernel/bios32.c | 12 ------------
> > > >  arch/arm64/kernel/pci.c  | 13 -------------
> > > >  2 files changed, 25 deletions(-)
> > > 
> > > Acked-by: Will Deacon <will.deacon@arm.com>
> > 
> > Thanks, unfortunately I spotted that ARM platforms can set
> > PCI_PROBE_ONLY also via command line (pcibios_setup()), which means
> > that I have to add resource claiming to all ARM PCI controllers that
> > check PCI_PROBE_ONLY (inclusive of ARM bios32) to really make sure we
> > can apply this patch or alternatevely we add the resource claiming
> > to the pcibios_fixup_bus() callback (but we can claim resources only if
> > PCI_PROBE_ONLY is set lest we trigger regressions) which would be the
> > simpler solution.
> 
> So where are we?  The above response sounds like this series still
> needs a little tweaking?

Yes, I rewrote the generic PCI layer for claiming resources (Tomasz's
posted it in its ACPI PCI ARM64 series):

https://lkml.org/lkml/2016/2/16/412

My only concern is removing PCI_PROBE_ONLY handling from arm 32 bit code,
since I noticed it can be set on the command line there, claiming
resources in pcibios arm 32 should do the trick, I will move the ARM
32-bit handling in a separate patch (hopefully someone can help me test
it on affected platforms (?), I will ask Russell the best way to do 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
diff mbox

Patch

diff --git a/arch/arm/kernel/bios32.c b/arch/arm/kernel/bios32.c
index 6551d28..9c998d5 100644
--- a/arch/arm/kernel/bios32.c
+++ b/arch/arm/kernel/bios32.c
@@ -590,18 +590,6 @@  resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 			enum pci_mmap_state mmap_state, int write_combine)
 {
diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
index b3d098b..4095379 100644
--- a/arch/arm64/kernel/pci.c
+++ b/arch/arm64/kernel/pci.c
@@ -38,19 +38,6 @@  resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return res->start;
 }
 
-/**
- * pcibios_enable_device - Enable I/O and memory.
- * @dev: PCI device to be enabled
- * @mask: bitmask of BARs to enable
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	if (pci_has_flag(PCI_PROBE_ONLY))
-		return 0;
-
-	return pci_enable_resources(dev, mask);
-}
-
 /*
  * Try to assign the IRQ number from DT when adding a new device
  */