[patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Bjorn Helgaas]

There are many implementations of pcibios_enable_resources() that differ
in minor ways that look more like bugs than architectural differences.
This patch series consolidates most of them to use the x86 version.

This series is for discussion only at this point.  I'm interested in
feedback about whether any of the differences are "real" and need to
be preserved.

ARM and PA-RISC, in particular, have interesting differences:
    - ARM always enables bridge devices, which no other arch does
    - PA-RISC always turns on SERR and PARITY, which no other arch does

Should other arches do the same thing, or are these somehow related to
ARM and PA-RISC architecture?

Bjorn

-- 

[patch 1/4] PCI: split pcibios_enable_resources() out of pcibios_enable_device()

[Bjorn Helgaas]

On x86, pcibios_enable_device() is factored into
pcibios_enable_resources() and pcibios_enable_irq().  On several other
architectures, the functional equivalent of pcibios_enable_resources()
is expanded directly inside pcibios_enable_device().

This splits these pcibios_enable_device() implementations to make them
more similar to the x86 implementation.

There should be no functional change from this patch.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

---
 arch/alpha/kernel/pci.c          |    8 +++++++-
 arch/arm/kernel/bios32.c         |    9 +++++++--
 arch/parisc/kernel/pci.c         |    6 +++++-
 arch/powerpc/kernel/pci-common.c |   14 +++++++++-----
 arch/sh/drivers/pci/pci.c        |    7 ++++++-
 arch/sparc64/kernel/pci.c        |    7 ++++++-
 arch/v850/kernel/rte_mb_a_pci.c  |    7 ++++++-
 7 files changed, 46 insertions(+), 12 deletions(-)

Index: work6/arch/alpha/kernel/pci.c
===================================================================
--- work6.orig/arch/alpha/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/alpha/kernel/pci.c	2008-02-18 10:45:14.000000000 -0700
@@ -370,7 +370,7 @@
 #endif
 
 int
-pcibios_enable_device(struct pci_dev *dev, int mask)
+pcibios_enable_resources(struct pci_dev *dev, int mask)
 {
 	u16 cmd, oldcmd;
 	int i;
@@ -396,6 +396,12 @@
 	return 0;
 }
 
+int
+pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pcibios_enable_resources(dev, mask);
+}
+
 /*
  *  If we set up a device for bus mastering, we need to check the latency
  *  timer as certain firmware forgets to set it properly, as seen
Index: work6/arch/arm/kernel/bios32.c
===================================================================
--- work6.orig/arch/arm/kernel/bios32.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/arm/kernel/bios32.c	2008-02-18 10:45:14.000000000 -0700
@@ -655,10 +655,10 @@
 }
 
 /**
- * pcibios_enable_device - Enable I/O and memory.
+ * pcibios_enable_resources - Enable I/O and memory.
  * @dev: PCI device to be enabled
  */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
 {
 	u16 cmd, old_cmd;
 	int idx;
@@ -697,6 +697,11 @@
 	return 0;
 }
 
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pcibios_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)
 {
Index: work6/arch/parisc/kernel/pci.c
===================================================================
--- work6.orig/arch/parisc/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/parisc/kernel/pci.c	2008-02-18 10:45:14.000000000 -0700
@@ -285,7 +285,7 @@
  * Drivers that do not need parity (eg graphics and possibly networking)
  * can clear these bits if they want.
  */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
 {
 	u16 cmd;
 	int idx;
@@ -317,6 +317,10 @@
 	return 0;
 }
 
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pcibios_enable_resources(dev, mask);
+}
 
 /* PA-RISC specific */
 void pcibios_register_hba(struct pci_hba_data *hba)
Index: work6/arch/powerpc/kernel/pci-common.c
===================================================================
--- work6.orig/arch/powerpc/kernel/pci-common.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/powerpc/kernel/pci-common.c	2008-02-18 10:45:14.000000000 -0700
@@ -1153,16 +1153,12 @@
 EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
 #endif /* CONFIG_HOTPLUG */
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
 {
 	u16 cmd, old_cmd;
 	int idx;
 	struct resource *r;
 
-	if (ppc_md.pcibios_enable_device_hook)
-		if (ppc_md.pcibios_enable_device_hook(dev))
-			return -EINVAL;
-
 	pci_read_config_word(dev, PCI_COMMAND, &cmd);
 	old_cmd = cmd;
 	for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
@@ -1193,3 +1189,11 @@
 	return 0;
 }
 
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	if (ppc_md.pcibios_enable_device_hook)
+		if (ppc_md.pcibios_enable_device_hook(dev))
+			return -EINVAL;
+
+	return pcibios_enable_resources(dev, mask);
+}
Index: work6/arch/sh/drivers/pci/pci.c
===================================================================
--- work6.orig/arch/sh/drivers/pci/pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/sh/drivers/pci/pci.c	2008-02-18 10:45:14.000000000 -0700
@@ -131,7 +131,7 @@
 	}
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
 {
 	u16 cmd, old_cmd;
 	int idx;
@@ -163,6 +163,11 @@
 	return 0;
 }
 
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pcibios_enable_resources(dev, mask);
+}
+
 /*
  *  If we set up a device for bus mastering, we need to check and set
  *  the latency timer as it may not be properly set.
Index: work6/arch/sparc64/kernel/pci.c
===================================================================
--- work6.orig/arch/sparc64/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/sparc64/kernel/pci.c	2008-02-18 10:45:14.000000000 -0700
@@ -946,7 +946,7 @@
 {
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
+int pcibios_enable_resources(struct pci_dev *dev, int mask)
 {
 	u16 cmd, oldcmd;
 	int i;
@@ -976,6 +976,11 @@
 	return 0;
 }
 
+int pcibios_enable_device(struct pci_dev *dev, int mask)
+{
+	return pcibios_enable_resources(dev, mask);
+}
+
 void pcibios_resource_to_bus(struct pci_dev *pdev, struct pci_bus_region *region,
 			     struct resource *res)
 {
Index: work6/arch/v850/kernel/rte_mb_a_pci.c
===================================================================
--- work6.orig/arch/v850/kernel/rte_mb_a_pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/v850/kernel/rte_mb_a_pci.c	2008-02-18 10:45:14.000000000 -0700
@@ -217,7 +217,7 @@
 }
 
 \f
-int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
+int __nomods_init pcibios_enable_resources (struct pci_dev *dev, int mask)
 {
 	u16 cmd, old_cmd;
 	int idx;
@@ -245,6 +245,11 @@
 	return 0;
 }
 
+int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
+{
+	return pcibios_enable_resources(dev, mask);
+}
+
 \f
 /* Resource allocation.  */
 static void __devinit pcibios_assign_resources (void)

-- 

Re: [patch 1/4] PCI: split pcibios_enable_resources() out of pcibios_enable_device()

[Benjamin Herrenschmidt]

On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
> plain text document attachment (make-pcibios_enable_resources)
> On x86, pcibios_enable_device() is factored into
> pcibios_enable_resources() and pcibios_enable_irq().  On several other
> architectures, the functional equivalent of pcibios_enable_resources()
> is expanded directly inside pcibios_enable_device().
> 
> This splits these pcibios_enable_device() implementations to make them
> more similar to the x86 implementation.
> 
> There should be no functional change from this patch.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> 
> ---
>  arch/alpha/kernel/pci.c          |    8 +++++++-
>  arch/arm/kernel/bios32.c         |    9 +++++++--
>  arch/parisc/kernel/pci.c         |    6 +++++-
>  arch/powerpc/kernel/pci-common.c |   14 +++++++++-----
>  arch/sh/drivers/pci/pci.c        |    7 ++++++-
>  arch/sparc64/kernel/pci.c        |    7 ++++++-
>  arch/v850/kernel/rte_mb_a_pci.c  |    7 ++++++-
>  7 files changed, 46 insertions(+), 12 deletions(-)
> 
> Index: work6/arch/alpha/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/alpha/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/alpha/kernel/pci.c	2008-02-18 10:45:14.000000000 -0700
> @@ -370,7 +370,7 @@
>  #endif
>  
>  int
> -pcibios_enable_device(struct pci_dev *dev, int mask)
> +pcibios_enable_resources(struct pci_dev *dev, int mask)
>  {
>  	u16 cmd, oldcmd;
>  	int i;
> @@ -396,6 +396,12 @@
>  	return 0;
>  }
>  
> +int
> +pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> +	return pcibios_enable_resources(dev, mask);
> +}
> +
>  /*
>   *  If we set up a device for bus mastering, we need to check the latency
>   *  timer as certain firmware forgets to set it properly, as seen
> Index: work6/arch/arm/kernel/bios32.c
> ===================================================================
> --- work6.orig/arch/arm/kernel/bios32.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/arm/kernel/bios32.c	2008-02-18 10:45:14.000000000 -0700
> @@ -655,10 +655,10 @@
>  }
>  
>  /**
> - * pcibios_enable_device - Enable I/O and memory.
> + * pcibios_enable_resources - Enable I/O and memory.
>   * @dev: PCI device to be enabled
>   */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
>  {
>  	u16 cmd, old_cmd;
>  	int idx;
> @@ -697,6 +697,11 @@
>  	return 0;
>  }
>  
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> +	return pcibios_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)
>  {
> Index: work6/arch/parisc/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/parisc/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/parisc/kernel/pci.c	2008-02-18 10:45:14.000000000 -0700
> @@ -285,7 +285,7 @@
>   * Drivers that do not need parity (eg graphics and possibly networking)
>   * can clear these bits if they want.
>   */
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
>  {
>  	u16 cmd;
>  	int idx;
> @@ -317,6 +317,10 @@
>  	return 0;
>  }
>  
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> +	return pcibios_enable_resources(dev, mask);
> +}
>  
>  /* PA-RISC specific */
>  void pcibios_register_hba(struct pci_hba_data *hba)
> Index: work6/arch/powerpc/kernel/pci-common.c
> ===================================================================
> --- work6.orig/arch/powerpc/kernel/pci-common.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/powerpc/kernel/pci-common.c	2008-02-18 10:45:14.000000000 -0700
> @@ -1153,16 +1153,12 @@
>  EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
>  #endif /* CONFIG_HOTPLUG */
>  
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
>  {
>  	u16 cmd, old_cmd;
>  	int idx;
>  	struct resource *r;
>  
> -	if (ppc_md.pcibios_enable_device_hook)
> -		if (ppc_md.pcibios_enable_device_hook(dev))
> -			return -EINVAL;
> -
>  	pci_read_config_word(dev, PCI_COMMAND, &cmd);
>  	old_cmd = cmd;
>  	for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
> @@ -1193,3 +1189,11 @@
>  	return 0;
>  }
>  
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> +	if (ppc_md.pcibios_enable_device_hook)
> +		if (ppc_md.pcibios_enable_device_hook(dev))
> +			return -EINVAL;
> +
> +	return pcibios_enable_resources(dev, mask);
> +}
> Index: work6/arch/sh/drivers/pci/pci.c
> ===================================================================
> --- work6.orig/arch/sh/drivers/pci/pci.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/sh/drivers/pci/pci.c	2008-02-18 10:45:14.000000000 -0700
> @@ -131,7 +131,7 @@
>  	}
>  }
>  
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
>  {
>  	u16 cmd, old_cmd;
>  	int idx;
> @@ -163,6 +163,11 @@
>  	return 0;
>  }
>  
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> +	return pcibios_enable_resources(dev, mask);
> +}
> +
>  /*
>   *  If we set up a device for bus mastering, we need to check and set
>   *  the latency timer as it may not be properly set.
> Index: work6/arch/sparc64/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/sparc64/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/sparc64/kernel/pci.c	2008-02-18 10:45:14.000000000 -0700
> @@ -946,7 +946,7 @@
>  {
>  }
>  
> -int pcibios_enable_device(struct pci_dev *dev, int mask)
> +int pcibios_enable_resources(struct pci_dev *dev, int mask)
>  {
>  	u16 cmd, oldcmd;
>  	int i;
> @@ -976,6 +976,11 @@
>  	return 0;
>  }
>  
> +int pcibios_enable_device(struct pci_dev *dev, int mask)
> +{
> +	return pcibios_enable_resources(dev, mask);
> +}
> +
>  void pcibios_resource_to_bus(struct pci_dev *pdev, struct pci_bus_region *region,
>  			     struct resource *res)
>  {
> Index: work6/arch/v850/kernel/rte_mb_a_pci.c
> ===================================================================
> --- work6.orig/arch/v850/kernel/rte_mb_a_pci.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/v850/kernel/rte_mb_a_pci.c	2008-02-18 10:45:14.000000000 -0700
> @@ -217,7 +217,7 @@
>  }
>  
>  \f
> -int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
> +int __nomods_init pcibios_enable_resources (struct pci_dev *dev, int mask)
>  {
>  	u16 cmd, old_cmd;
>  	int idx;
> @@ -245,6 +245,11 @@
>  	return 0;
>  }
>  
> +int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
> +{
> +	return pcibios_enable_resources(dev, mask);
> +}
> +
>  \f
>  /* Resource allocation.  */
>  static void __devinit pcibios_assign_resources (void)
> 

[patch 2/4] ppc: make pcibios_enable_device() use pcibios_enable_resources()

[Bjorn Helgaas]

pcibios_enable_device() has an almost verbatim copy of
pcibios_enable_resources(), (the only difference is that
pcibios_enable_resources() turns on PCI_COMMAND_MEMORY if
there's a ROM resource).

The duplication might be intentional, but I don't see any callers
of pcibios_enable_resources() on ppc, so I think it's more
likely a historical accident.

This patch removes the duplication, making pcibios_enable_device()
simply call pcibios_enable_resources() as x86 does.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: work6/arch/ppc/kernel/pci.c
===================================================================
--- work6.orig/arch/ppc/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/ppc/kernel/pci.c	2008-02-18 11:31:23.000000000 -0700
@@ -785,33 +785,11 @@
 
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
 	if (ppc_md.pcibios_enable_device_hook)
 		if (ppc_md.pcibios_enable_device_hook(dev, 0))
 			return -EINVAL;
-		
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx=0; idx<6; idx++) {
-		r = &dev->resource[idx];
-		if (r->flags & IORESOURCE_UNSET) {
-			printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n",
-		       pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
+
+	return pcibios_enable_resources(dev, mask);
 }
 
 struct pci_controller*

-- 

Re: [patch 2/4] ppc: make pcibios_enable_device() use pcibios_enable_resources()

[Benjamin Herrenschmidt]

On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
> plain text document attachment (ppc-pcibios_enable_resources)
> pcibios_enable_device() has an almost verbatim copy of
> pcibios_enable_resources(), (the only difference is that
> pcibios_enable_resources() turns on PCI_COMMAND_MEMORY if
> there's a ROM resource).
> 
> The duplication might be intentional, but I don't see any callers
> of pcibios_enable_resources() on ppc, so I think it's more
> likely a historical accident.
> 
> This patch removes the duplication, making pcibios_enable_device()
> simply call pcibios_enable_resources() as x86 does.
> 
> Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Ack. arch/ppc is being phased out soon anyway.

Ben.

> 
> Index: work6/arch/ppc/kernel/pci.c
> ===================================================================
> --- work6.orig/arch/ppc/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
> +++ work6/arch/ppc/kernel/pci.c	2008-02-18 11:31:23.000000000 -0700
> @@ -785,33 +785,11 @@
>  
>  int pcibios_enable_device(struct pci_dev *dev, int mask)
>  {
> -	u16 cmd, old_cmd;
> -	int idx;
> -	struct resource *r;
> -
>  	if (ppc_md.pcibios_enable_device_hook)
>  		if (ppc_md.pcibios_enable_device_hook(dev, 0))
>  			return -EINVAL;
> -		
> -	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> -	old_cmd = cmd;
> -	for (idx=0; idx<6; idx++) {
> -		r = &dev->resource[idx];
> -		if (r->flags & IORESOURCE_UNSET) {
> -			printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
> -			return -EINVAL;
> -		}
> -		if (r->flags & IORESOURCE_IO)
> -			cmd |= PCI_COMMAND_IO;
> -		if (r->flags & IORESOURCE_MEM)
> -			cmd |= PCI_COMMAND_MEMORY;
> -	}
> -	if (cmd != old_cmd) {
> -		printk("PCI: Enabling device %s (%04x -> %04x)\n",
> -		       pci_name(dev), old_cmd, cmd);
> -		pci_write_config_word(dev, PCI_COMMAND, cmd);
> -	}
> -	return 0;
> +
> +	return pcibios_enable_resources(dev, mask);
>  }
>  
>  struct pci_controller*
> 

[patch 3/4] xtensa: make pcibios_enable_device() use pcibios_enable_resources()

[Bjorn Helgaas]

pcibios_enable_device() has an almost verbatim copy of
pcibios_enable_resources(), (the only difference is that
pcibios_enable_resources() turns on PCI_COMMAND_MEMORY if
there's a ROM resource).

The duplication might be intentional, but I don't see any callers
of pcibios_enable_resources() on xtensa, so I think it's more
likely a historical accident copied from ppc.

This patch removes the duplication, making pcibios_enable_device()
simply call pcibios_enable_resources() as x86 does.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

Index: work6/arch/xtensa/kernel/pci.c
===================================================================
--- work6.orig/arch/xtensa/kernel/pci.c	2008-02-18 10:43:50.000000000 -0700
+++ work6/arch/xtensa/kernel/pci.c	2008-02-18 11:32:12.000000000 -0700
@@ -238,31 +238,7 @@
 
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx=0; idx<6; idx++) {
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because "
-			       "of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n",
-		       pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-
-	return 0;
+	return pcibios_enable_resources(dev, mask);
 }
 
 #ifdef CONFIG_PROC_FS

-- 

[patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Bjorn Helgaas]

There are many implementations of pcibios_enable_resources() that differ
in minor ways that look more like bugs than architectural differences.
This patch consolidates most of them to use the x86 version.

This patch is for discussion only at this point.  I'm interested in
feedback about whether any of the differences are "real" and need to
be preserved.

ARM and PA-RISC, in particular, have interesting differences:
    - ARM always enables bridge devices, which no other arch does
    - PA-RISC always turns on SERR and PARITY, which no other arch does

Should other arches do the same thing, or are these somehow related to
ARM and PA-RISC architecture?


Here's the x86 version, which seems most complete and up-to-date:

    int pcibios_enable_resources(struct pci_dev *dev, int mask)
    {
	u16 cmd, old_cmd;
	int i;
	struct resource *r;

  (0)
	pci_read_config_word(dev, PCI_COMMAND, &cmd);
	old_cmd = cmd;

  (1)	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
  (2)		if (!(mask & (1 << i)))
			continue;

		r = &dev->resource[i];

  (3)		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
			continue;

  (4)		if ((i == PCI_ROM_RESOURCE) &&
				(!(r->flags & IORESOURCE_ROM_ENABLE)))
			continue;

  (5)		if (!r->start && r->end) {
			dev_err(&dev->dev, "device not available because of "
				"resource %d collisions\n", i);
			return -EINVAL;
		}

		if (r->flags & IORESOURCE_IO)
			cmd |= PCI_COMMAND_IO;
		if (r->flags & IORESOURCE_MEM)
			cmd |= PCI_COMMAND_MEMORY;
	}

  (6)
  (7)	if (cmd != old_cmd) {
		dev_info(&dev->dev, "enabling device (%04x -> %04x)\n",
			 old_cmd, cmd);
		pci_write_config_word(dev, PCI_COMMAND, cmd);
	}
	return 0;
    }

Compared with the x86 version, other architectures have the following
functional differences:

    alpha: ignores mask at (2), has no PCI_ROM_RESOURCE check at (4),
	has no collision check at (5)

    arm: checks only 6 resources at (1), has no PCI_ROM_RESOURCE check at (4), 
	always fully enables bridges at (6)

    cris: checks only 6 resources at (1), has a different ROM
	resource check at (4) and (6) that ignores IORESOURCE_ROM_ENABLE

    frv: checks only 6 resources at (1), has a different ROM
	resource check at (4) and (6) that ignores IORESOURCE_ROM_ENABLE

    ia64: checks for NULL dev at (0)

    mips: has no IORESOURCE_{IO,MEM} check at (3), has a different
	ROM resource check at (4) and (6) that ignores IORESOURCE_ROM_ENABLE

    mn10300: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM}
	check at (3), has a different ROM resource check at (4) and (6)
	that ignores IORESOURCE_ROM_ENABLE

    parisc: checks DEVICE_COUNT_RESOURCE (12) instead of PCI_NUM_RESOURCES (11)
	resources at (1), has no IORESOURCE_{IO,MEM} check at (3),
	has no PCI_ROM_RESOURCE check at (4), has no collision check at (5)
	always turns on PCI_COMMAND_SERR | PCI_COMMAND_PARITY at (6),
	writes cmd even if unchanged at (7)

    powerpc: has a different collision check at (5)

    ppc: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
	at (3), has a different ROM resource check at (4) and (6) that
	ignores IORESOURCE_ROM_ENABLE, has a different collision check using
	IORESOURCE_UNSET at (5)

    sh: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
	at (3), has a different ROM resource check at (4) and (6) that
	ignores IORESOURCE_ROM_ENABLE

    sparc64: has no IORESOURCE_{IO,MEM} check at (3), has no PCI_ROM_RESOURCE
	check at (4)

    v850: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
	at (3), has no PCI_ROM_RESOURCE check at (4)

    xtensa: checks only 6 resources at (1), has no IORESOURCE_{IO,MEM} check
	at (3), has a different ROM resource check at (4) and (6) that
	ignores IORESOURCE_ROM_ENABLE

The mips/pmc-sierra implementation of pcibios_enable_resources() is
cluttered with a bunch of titan stuff, so I can't immediately consolidate
it with the others.  So I made the generic version "weak" so pmc-sierra
can override it.

Not-Yet-Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>

---
 arch/alpha/kernel/pci.c                 |   27 -------------
 arch/arm/kernel/bios32.c                |   43 ---------------------
 arch/cris/arch-v32/drivers/pci/bios.c   |   32 ----------------
 arch/frv/mb93090-mb00/pci-frv.c         |   32 ----------------
 arch/ia64/pci/pci.c                     |   42 ---------------------
 arch/mips/pci/pci.c                     |   32 ----------------
 arch/mn10300/unit-asb2305/pci-asb2305.c |   39 -------------------
 arch/parisc/kernel/pci.c                |   41 --------------------
 arch/powerpc/kernel/pci-common.c        |   36 ------------------
 arch/ppc/kernel/pci.c                   |   33 ----------------
 arch/sh/drivers/pci/pci.c               |   32 ----------------
 arch/sparc64/kernel/pci.c               |   30 ---------------
 arch/v850/kernel/rte_mb_a_pci.c         |   28 --------------
 arch/x86/pci/i386.c                     |   38 -------------------
 arch/x86/pci/pci.h                      |    1 
 arch/xtensa/kernel/pci.c                |   31 ---------------
 drivers/pci/Makefile                    |    2 -
 drivers/pci/bios.c                      |   64 ++++++++++++++++++++++++++++++++
 include/linux/pci.h                     |    1 
 19 files changed, 66 insertions(+), 518 deletions(-)

Index: work6/arch/alpha/kernel/pci.c
===================================================================
--- work6.orig/arch/alpha/kernel/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/alpha/kernel/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -370,33 +370,6 @@
 #endif
 
 int
-pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, oldcmd;
-	int i;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	oldcmd = cmd;
-
-	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
-		struct resource *res = &dev->resource[i];
-
-		if (res->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		else if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	if (cmd != oldcmd) {
-		printk(KERN_DEBUG "PCI: Enabling device: (%s), cmd %x\n",
-		       pci_name(dev), cmd);
-		/* Enable the appropriate bits in the PCI command register.  */
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
-int
 pcibios_enable_device(struct pci_dev *dev, int mask)
 {
 	return pcibios_enable_resources(dev, mask);
Index: work6/arch/arm/kernel/bios32.c
===================================================================
--- work6.orig/arch/arm/kernel/bios32.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/arm/kernel/bios32.c	2008-02-18 21:16:38.000000000 -0700
@@ -654,49 +654,6 @@
 	res->start = (start + align - 1) & ~(align - 1);
 }
 
-/**
- * pcibios_enable_resources - Enable I/O and memory.
- * @dev: PCI device to be enabled
- */
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx = 0; idx < 6; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1 << idx)))
-			continue;
-
-		r = dev->resource + idx;
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because"
-			       " of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	/*
-	 * Bridges (eg, cardbus bridges) need to be fully enabled
-	 */
-	if ((dev->class >> 16) == PCI_BASE_CLASS_BRIDGE)
-		cmd |= PCI_COMMAND_IO | PCI_COMMAND_MEMORY;
-
-	if (cmd != old_cmd) {
-		printk("PCI: enabling device %s (%04x -> %04x)\n",
-		       pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
 	return pcibios_enable_resources(dev, mask);
Index: work6/arch/cris/arch-v32/drivers/pci/bios.c
===================================================================
--- work6.orig/arch/cris/arch-v32/drivers/pci/bios.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/cris/arch-v32/drivers/pci/bios.c	2008-02-18 21:16:38.000000000 -0700
@@ -55,38 +55,6 @@
 	}
 }
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for(idx=0; idx<6; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<idx)))
-			continue;
-
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int pcibios_enable_irq(struct pci_dev *dev)
 {
 	dev->irq = EXT_INTR_VECT;
Index: work6/arch/frv/mb93090-mb00/pci-frv.c
===================================================================
--- work6.orig/arch/frv/mb93090-mb00/pci-frv.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/frv/mb93090-mb00/pci-frv.c	2008-02-18 21:16:38.000000000 -0700
@@ -231,38 +231,6 @@
 	pcibios_assign_resources();
 }
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for(idx=0; idx<6; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<idx)))
-			continue;
-
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 /*
  *  If we set up a device for bus mastering, we need to check the latency
  *  timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/ia64/pci/pci.c
===================================================================
--- work6.orig/arch/ia64/pci/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/ia64/pci/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -499,48 +499,6 @@
 	/* ??? FIXME -- record old value for shutdown.  */
 }
 
-static inline int
-pcibios_enable_resources (struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM;
-
-	if (!dev)
-		return -EINVAL;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx=0; idx<PCI_NUM_RESOURCES; idx++) {
-		/* Only set up the desired resources.  */
-		if (!(mask & (1 << idx)))
-			continue;
-
-		r = &dev->resource[idx];
-		if (!(r->flags & type_mask))
-			continue;
-		if ((idx == PCI_ROM_RESOURCE) &&
-				(!(r->flags & IORESOURCE_ROM_ENABLE)))
-			continue;
-		if (!r->start && r->end) {
-			printk(KERN_ERR
-			       "PCI: Device %s not available because of resource collisions\n",
-			       pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int
 pcibios_enable_device (struct pci_dev *dev, int mask)
 {
Index: work6/arch/mips/pci/pci.c
===================================================================
--- work6.orig/arch/mips/pci/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/mips/pci/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -163,38 +163,6 @@
 
 subsys_initcall(pcibios_init);
 
-static int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx=0; idx < PCI_NUM_RESOURCES; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<idx)))
-			continue;
-
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 /*
  *  If we set up a device for bus mastering, we need to check the latency
  *  timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/mn10300/unit-asb2305/pci-asb2305.c
===================================================================
--- work6.orig/arch/mn10300/unit-asb2305/pci-asb2305.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/mn10300/unit-asb2305/pci-asb2305.c	2008-02-18 21:16:38.000000000 -0700
@@ -218,45 +218,6 @@
 	pcibios_allocate_resources(1);
 }
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-
-	for (idx = 0; idx < 6; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1 << idx)))
-			continue;
-
-		r = &dev->resource[idx];
-
-		if (!r->start && r->end) {
-			printk(KERN_ERR
-			       "PCI: Device %s not available because of"
-			       " resource collisions\n",
-			       pci_name(dev));
-			return -EINVAL;
-		}
-
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-
-	if (cmd != old_cmd)
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-
-	return 0;
-}
-
 /*
  *  If we set up a device for bus mastering, we need to check the latency
  *  timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/parisc/kernel/pci.c
===================================================================
--- work6.orig/arch/parisc/kernel/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/parisc/kernel/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -276,47 +276,6 @@
 	/* The caller updates the end field, we don't.  */
 }
 
-
-/*
- * A driver is enabling the device.  We make sure that all the appropriate
- * bits are set to allow the device to operate as the driver is expecting.
- * We enable the port IO and memory IO bits if the device has any BARs of
- * that type, and we enable the PERR and SERR bits unconditionally.
- * Drivers that do not need parity (eg graphics and possibly networking)
- * can clear these bits if they want.
- */
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd;
-	int idx;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-
-	for (idx = 0; idx < DEVICE_COUNT_RESOURCE; idx++) {
-		struct resource *r = &dev->resource[idx];
-
-		/* only setup requested resources */
-		if (!(mask & (1<<idx)))
-			continue;
-
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	cmd |= (PCI_COMMAND_SERR | PCI_COMMAND_PARITY);
-
-#if 0
-	/* If bridge/bus controller has FBB enabled, child must too. */
-	if (dev->bus->bridge_ctl & PCI_BRIDGE_CTL_FAST_BACK)
-		cmd |= PCI_COMMAND_FAST_BACK;
-#endif
-	DBGC("PCIBIOS: Enabling device %s cmd 0x%04x\n", pci_name(dev), cmd);
-	pci_write_config_word(dev, PCI_COMMAND, cmd);
-	return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
 	return pcibios_enable_resources(dev, mask);
Index: work6/arch/powerpc/kernel/pci-common.c
===================================================================
--- work6.orig/arch/powerpc/kernel/pci-common.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/powerpc/kernel/pci-common.c	2008-02-18 21:16:38.000000000 -0700
@@ -1153,42 +1153,6 @@
 EXPORT_SYMBOL_GPL(pcibios_claim_one_bus);
 #endif /* CONFIG_HOTPLUG */
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1 << idx)))
-			continue;
-		r = &dev->resource[idx];
-		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
-			continue;
-		if ((idx == PCI_ROM_RESOURCE) &&
-				(!(r->flags & IORESOURCE_ROM_ENABLE)))
-			continue;
-		if (r->parent == NULL) {
-			printk(KERN_ERR "PCI: Device %s not available because"
-			       " of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n",
-		       pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
 	if (ppc_md.pcibios_enable_device_hook)
Index: work6/arch/ppc/kernel/pci.c
===================================================================
--- work6.orig/arch/ppc/kernel/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/ppc/kernel/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -578,39 +578,6 @@
 }
 
 
-int
-pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx=0; idx<6; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<idx)))
-			continue;
-	
-		r = &dev->resource[idx];
-		if (r->flags & IORESOURCE_UNSET) {
-			printk(KERN_ERR "PCI: Device %s not available because of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n", pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 static int next_controller_index;
 
 struct pci_controller * __init
Index: work6/arch/sh/drivers/pci/pci.c
===================================================================
--- work6.orig/arch/sh/drivers/pci/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/sh/drivers/pci/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -131,38 +131,6 @@
 	}
 }
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for(idx=0; idx<6; idx++) {
-		if (!(mask & (1 << idx)))
-			continue;
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because "
-			       "of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-	if (cmd != old_cmd) {
-		printk(KERN_INFO "PCI: Enabling device %s (%04x -> %04x)\n",
-		       pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
 	return pcibios_enable_resources(dev, mask);
Index: work6/arch/sparc64/kernel/pci.c
===================================================================
--- work6.orig/arch/sparc64/kernel/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/sparc64/kernel/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -946,36 +946,6 @@
 {
 }
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, oldcmd;
-	int i;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	oldcmd = cmd;
-
-	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
-		struct resource *res = &dev->resource[i];
-
-		/* Only set up the requested stuff */
-		if (!(mask & (1<<i)))
-			continue;
-
-		if (res->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-
-	if (cmd != oldcmd) {
-		printk(KERN_DEBUG "PCI: Enabling device: (%s), cmd %x\n",
-		       pci_name(dev), cmd);
-                /* Enable the appropriate bits in the PCI command register.  */
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int pcibios_enable_device(struct pci_dev *dev, int mask)
 {
 	return pcibios_enable_resources(dev, mask);
Index: work6/arch/v850/kernel/rte_mb_a_pci.c
===================================================================
--- work6.orig/arch/v850/kernel/rte_mb_a_pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/v850/kernel/rte_mb_a_pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -217,34 +217,6 @@
 }
 
 \f
-int __nomods_init pcibios_enable_resources (struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx = 0; idx < 6; idx++) {
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available because "
-			       "of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n",
-		       pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 int __nomods_init pcibios_enable_device (struct pci_dev *dev, int mask)
 {
 	return pcibios_enable_resources(dev, mask);
Index: work6/arch/x86/pci/i386.c
===================================================================
--- work6.orig/arch/x86/pci/i386.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/x86/pci/i386.c	2008-02-18 21:16:38.000000000 -0700
@@ -238,44 +238,6 @@
  */
 fs_initcall(pcibios_assign_resources);
 
-int pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for (idx = 0; idx < PCI_NUM_RESOURCES; idx++) {
-		/* Only set up the requested stuff */
-		if (!(mask & (1 << idx)))
-			continue;
-
-		r = &dev->resource[idx];
-		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
-			continue;
-		if ((idx == PCI_ROM_RESOURCE) &&
-				(!(r->flags & IORESOURCE_ROM_ENABLE)))
-			continue;
-		if (!r->start && r->end) {
-			printk(KERN_ERR "PCI: Device %s not available "
-				"because of resource %d collisions\n",
-				pci_name(dev), idx);
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n",
-			pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 /*
  *  If we set up a device for bus mastering, we need to check the latency
  *  timer as certain crappy BIOSes forget to set it properly.
Index: work6/arch/x86/pci/pci.h
===================================================================
--- work6.orig/arch/x86/pci/pci.h	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/x86/pci/pci.h	2008-02-18 21:16:38.000000000 -0700
@@ -44,7 +44,6 @@
 extern unsigned int pcibios_max_latency;
 
 void pcibios_resource_survey(void);
-int pcibios_enable_resources(struct pci_dev *, int);
 
 /* pci-pc.c */
 
Index: work6/arch/xtensa/kernel/pci.c
===================================================================
--- work6.orig/arch/xtensa/kernel/pci.c	2008-02-18 21:16:36.000000000 -0700
+++ work6/arch/xtensa/kernel/pci.c	2008-02-18 21:16:38.000000000 -0700
@@ -91,37 +91,6 @@
 	}
 }
 
-int
-pcibios_enable_resources(struct pci_dev *dev, int mask)
-{
-	u16 cmd, old_cmd;
-	int idx;
-	struct resource *r;
-
-	pci_read_config_word(dev, PCI_COMMAND, &cmd);
-	old_cmd = cmd;
-	for(idx=0; idx<6; idx++) {
-		r = &dev->resource[idx];
-		if (!r->start && r->end) {
-			printk (KERN_ERR "PCI: Device %s not available because "
-				"of resource collisions\n", pci_name(dev));
-			return -EINVAL;
-		}
-		if (r->flags & IORESOURCE_IO)
-			cmd |= PCI_COMMAND_IO;
-		if (r->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	if (dev->resource[PCI_ROM_RESOURCE].start)
-		cmd |= PCI_COMMAND_MEMORY;
-	if (cmd != old_cmd) {
-		printk("PCI: Enabling device %s (%04x -> %04x)\n",
-			pci_name(dev), old_cmd, cmd);
-		pci_write_config_word(dev, PCI_COMMAND, cmd);
-	}
-	return 0;
-}
-
 struct pci_controller * __init pcibios_alloc_controller(void)
 {
 	struct pci_controller *pci_ctrl;
Index: work6/drivers/pci/Makefile
===================================================================
--- work6.orig/drivers/pci/Makefile	2008-02-18 21:16:36.000000000 -0700
+++ work6/drivers/pci/Makefile	2008-02-18 21:16:38.000000000 -0700
@@ -2,7 +2,7 @@
 # Makefile for the PCI bus specific drivers.
 #
 
-obj-y		+= access.o bus.o probe.o remove.o pci.o quirks.o \
+obj-y		+= access.o bios.o bus.o probe.o remove.o pci.o quirks.o \
 			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o
 obj-$(CONFIG_PROC_FS) += proc.o
 
Index: work6/drivers/pci/bios.c
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ work6/drivers/pci/bios.c	2008-02-18 21:16:38.000000000 -0700
@@ -0,0 +1,64 @@
+/*
+ * Generic pcibios routines, derived from x86 version
+ *
+ * Copyright 1993, 1994 Drew Eckhardt
+ *      Visionary Computing
+ *      (Unix and Linux consulting and custom programming)
+ *      Drew@Colorado.EDU
+ *      +1 (303) 786-7975
+ *
+ * Drew's work was sponsored by:
+ *	iX Multiuser Multitasking Magazine
+ *	Hannover, Germany
+ *	hm@ix.de
+ *
+ * Copyright 1997--2000 Martin Mares <mj@ucw.cz>
+ */
+
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/pci.h>
+#include <linux/ioport.h>
+#include <linux/errno.h>
+
+int __attribute__ ((weak)) pcibios_enable_resources(struct pci_dev *dev,
+						    int mask)
+{
+	u16 cmd, old_cmd;
+	int i;
+	struct resource *r;
+
+	pci_read_config_word(dev, PCI_COMMAND, &cmd);
+	old_cmd = cmd;
+
+	for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+		if (!(mask & (1 << i)))
+			continue;
+
+		r = &dev->resource[i];
+
+		if (!(r->flags & (IORESOURCE_IO | IORESOURCE_MEM)))
+			continue;
+		if ((i == PCI_ROM_RESOURCE) &&
+				(!(r->flags & IORESOURCE_ROM_ENABLE)))
+			continue;
+
+		if (!r->start && r->end) {
+			dev_err(&dev->dev, "device not available because of "
+				"resource %d collisions\n", i);
+			return -EINVAL;
+		}
+
+		if (r->flags & IORESOURCE_IO)
+			cmd |= PCI_COMMAND_IO;
+		if (r->flags & IORESOURCE_MEM)
+			cmd |= PCI_COMMAND_MEMORY;
+	}
+
+	if (cmd != old_cmd) {
+		dev_info(&dev->dev, "enabling device (%04x -> %04x)\n",
+			 old_cmd, cmd);
+		pci_write_config_word(dev, PCI_COMMAND, cmd);
+	}
+	return 0;
+}
Index: work6/include/linux/pci.h
===================================================================
--- work6.orig/include/linux/pci.h	2008-02-18 21:16:36.000000000 -0700
+++ work6/include/linux/pci.h	2008-02-18 21:16:38.000000000 -0700
@@ -443,6 +443,7 @@
 extern int no_pci_devices(void);
 
 void pcibios_fixup_bus(struct pci_bus *);
+int pcibios_enable_resources(struct pci_dev *, int mask);
 int __must_check pcibios_enable_device(struct pci_dev *, int mask);
 char *pcibios_setup(char *str);
 

-- 

Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Benjamin Herrenschmidt]

On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
>     powerpc: has a different collision check at (5)

I've always found the collision check dodgy. I tend to want to keep
the way powerpc does it here.

pci_enable_device() should only enable resources that have successfully
been added to the resource tree (that have passed all the collision
check etc...). There is a simple & clear indication of that: res->parent
is non-NULL. I think that is a better check than the test x86 does on
start and end.

That is, whatever the arch code decides to use to decide whether
resources are assigned by firmware or by the first pass assignment code
or not and collide or not, once that phase is finished (which is the
case when calling pcibios_enable_device(), having the resource in the
resource-tree or not is, I believe, the proper way to test whether it's
a useable resource.

Ben.

Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Bjorn Helgaas]

On Monday 18 February 2008 11:31:07 pm Benjamin Herrenschmidt wrote:
> 
> On Mon, 2008-02-18 at 21:39 -0700, Bjorn Helgaas wrote:
> >     powerpc: has a different collision check at (5)
> 
> I've always found the collision check dodgy. I tend to want to keep
> the way powerpc does it here.
> 
> pci_enable_device() should only enable resources that have successfully
> been added to the resource tree (that have passed all the collision
> check etc...). There is a simple & clear indication of that: res->parent
> is non-NULL. I think that is a better check than the test x86 does on
> start and end.
> 
> That is, whatever the arch code decides to use to decide whether
> resources are assigned by firmware or by the first pass assignment code
> or not and collide or not, once that phase is finished (which is the
> case when calling pcibios_enable_device(), having the resource in the
> resource-tree or not is, I believe, the proper way to test whether it's
> a useable resource.

So should x86 adopt that collision check?  I don't hear anything about
actual architecture differences that are behind this implementation
difference.

Bjorn

Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Ivan Kokshaysky]

On Tue, Feb 19, 2008 at 09:11:55AM -0700, Bjorn Helgaas wrote:
> > That is, whatever the arch code decides to use to decide whether
> > resources are assigned by firmware or by the first pass assignment code
> > or not and collide or not, once that phase is finished (which is the
> > case when calling pcibios_enable_device(), having the resource in the
> > resource-tree or not is, I believe, the proper way to test whether it's
> > a useable resource.
> 
> So should x86 adopt that collision check?

Yes, and other arches as well, I believe.

> I don't hear anything about
> actual architecture differences that are behind this implementation
> difference.

On ppc64 "0" can be a valid value for resource start, as far as I know.

Ivan.

Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Benjamin Herrenschmidt]

> > That is, whatever the arch code decides to use to decide whether
> > resources are assigned by firmware or by the first pass assignment code
> > or not and collide or not, once that phase is finished (which is the
> > case when calling pcibios_enable_device(), having the resource in the
> > resource-tree or not is, I believe, the proper way to test whether it's
> > a useable resource.
> 
> So should x86 adopt that collision check?  I don't hear anything about
> actual architecture differences that are behind this implementation
> difference.

Well, on powerpc we do allow under some circumstances a 0 start value
in BARs, which is why I wanted to use a different check.

Ben.

Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Benjamin Herrenschmidt]

> Index: work6/drivers/pci/Makefile
> ===================================================================
> --- work6.orig/drivers/pci/Makefile	2008-02-18 21:16:36.000000000 -0700
> +++ work6/drivers/pci/Makefile	2008-02-18 21:16:38.000000000 -0700
> @@ -2,7 +2,7 @@
>  # Makefile for the PCI bus specific drivers.
>  #
>  
> -obj-y		+= access.o bus.o probe.o remove.o pci.o quirks.o \
> +obj-y		+= access.o bios.o bus.o probe.o remove.o pci.o quirks.o \
>  			pci-driver.o search.o pci-sysfs.o rom.o setup-res.o
>  obj-$(CONFIG_PROC_FS) += proc.o
>  
> Index: work6/drivers/pci/bios.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ work6/drivers/pci/bios.c	2008-02-18 21:16:38.000000000 -070
                        ^^^^^^

Yuck :-)

Please, don't call this bios ... whatever is in this file really has
nothing to do with a "BIOS" in any shape or form :-) I know we used to
call those things pcibios_* but that's really historical...

If you want to make clear it's for "utilities" that can be overriden
by the arch, maybe call it utils.c, or just stick the function in
pci.c, or setup-res.c

Ben.

Re: [patch 4/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Kyle McMartin]

On Mon, Feb 18, 2008 at 09:39:56PM -0700, Bjorn Helgaas wrote:
>     parisc: checks DEVICE_COUNT_RESOURCE (12) instead of PCI_NUM_RESOURCES
>	(11) resources at (1),
>

Good catch.

> has no IORESOURCE_{IO,MEM} check at (3),

What else can it be?

> 	has no PCI_ROM_RESOURCE check at (4), has no collision check at (5)
> 	always turns on PCI_COMMAND_SERR | PCI_COMMAND_PARITY at (6),
> 	writes cmd even if unchanged at (7)
> 

I'll have to check into 4 & 5, there might be a good reason for it. For
instance on a four port HP ethernet card (pci-pci bridge + 4 tulips) all
4 of the rom resources are mapped to the same address, which afaict, is
allowed but breaks things in mysterious and subtle ways.

That said, the parisc pci code is a rats nest...

cheers, Kyle

Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Kyle McMartin]

On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
>     - PA-RISC always turns on SERR and PARITY, which no other arch does
> 

I suspect this is because we set the host bus adapters to hard fail
(HPMC) on detecting an error, since we don't want to
	1) return possibly bogus (-1) data
	2) write code to use the (undocumented) error detection

More to the point, I suspect it's extra paranoia because firmware has
set it up this way. I put in a quick hack to test whether those bits
were set, and they came enabled that way by firmware on all the boxes I
tested. (Disclaimer, I didn't have any easily accessible boxes with
add-on cards installed, so firmware might just set it up for core
devices, and we're making sure its set everywhere.)

cheers, Kyle

Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Russell King]

On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
> There are many implementations of pcibios_enable_resources() that differ
> in minor ways that look more like bugs than architectural differences.
> This patch series consolidates most of them to use the x86 version.
> 
> This series is for discussion only at this point.  I'm interested in
> feedback about whether any of the differences are "real" and need to
> be preserved.
> 
> ARM and PA-RISC, in particular, have interesting differences:
>     - ARM always enables bridge devices, which no other arch does

ARM does this because there is nothing else which would do that - which
means devices behind bridges would be completely inaccessible.

>     - PA-RISC always turns on SERR and PARITY, which no other arch does

ARM also does this, unless pdev_bad_for_parity(dev) is true.  See
ARMs pcibios_fixup_bus().

> Should other arches do the same thing, or are these somehow related to
> ARM and PA-RISC architecture?

I suspect they're architecture specific; I wouldn't like to do either
on x86, but they're either required or preferred on ARM.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Benjamin Herrenschmidt]

On Tue, 2008-02-19 at 08:09 +0000, Russell King wrote:
> On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
> > There are many implementations of pcibios_enable_resources() that differ
> > in minor ways that look more like bugs than architectural differences.
> > This patch series consolidates most of them to use the x86 version.
> > 
> > This series is for discussion only at this point.  I'm interested in
> > feedback about whether any of the differences are "real" and need to
> > be preserved.
> > 
> > ARM and PA-RISC, in particular, have interesting differences:
> >     - ARM always enables bridge devices, which no other arch does
> 
> ARM does this because there is nothing else which would do that - which
> means devices behind bridges would be completely inaccessible.

That's normally done by pci_enable_bridges() called by
pci_assign_unassigned_resources(). (The later has a weird naming, it
does more than just assign unassigned resources in fact).

> >     - PA-RISC always turns on SERR and PARITY, which no other arch does
> 
> ARM also does this, unless pdev_bad_for_parity(dev) is true.  See
> ARMs pcibios_fixup_bus().

While that sounds like a good idea to generalize, I think it should
remain arch stuff tho, not move to generic code.

On some platforms, weirdo firmwares handle error handling and will be
unhappy if the kernel mucks around (such as pSeries).

> > Should other arches do the same thing, or are these somehow related to
> > ARM and PA-RISC architecture?
> 
> I suspect they're architecture specific; I wouldn't like to do either
> on x86, but they're either required or preferred on ARM.

Ben.

Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Bjorn Helgaas]

On Tuesday 19 February 2008 03:08:32 am Benjamin Herrenschmidt wrote:
> On Tue, 2008-02-19 at 08:09 +0000, Russell King wrote:
> > On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
> > > 
> > > ARM and PA-RISC, in particular, have interesting differences:
> > >     - ARM always enables bridge devices, which no other arch does
> > 
> > ARM does this because there is nothing else which would do that - which
> > means devices behind bridges would be completely inaccessible.
> 
> That's normally done by pci_enable_bridges() called by
> pci_assign_unassigned_resources().

alpha, mips, mn10300, powerpc, and x86 use pci_enable_bridges() via
pci_assign_unassigned_resources().  parisc uses pci_enable_bridges()
directly from lba_driver_probe().  I guess the other arches (except
arm) rely on firmware to enable bridge.

I think pci_assign_unassigned_resources() is a bit of an anachronism --
it's a boot-time thing that does it for all root buses at once.  A
more hot-plug oriented scheme would do it like parisc, where every
time we discover a root bridge, we do any necessary resource assignment
and bridge enablement underneath it.

For ARM, maybe that could happen in pcibios_init_hw() or something
similar.

> > >     - PA-RISC always turns on SERR and PARITY, which no other arch does
> > 
> > ARM also does this, unless pdev_bad_for_parity(dev) is true.  See
> > ARMs pcibios_fixup_bus().
> 
> While that sounds like a good idea to generalize, I think it should
> remain arch stuff tho, not move to generic code.
> 
> On some platforms, weirdo firmwares handle error handling and will be
> unhappy if the kernel mucks around (such as pSeries).

I agree the SERR and PARITY stuff probably needs to remain arch code.
But it would be nice to at least have this and the bridge enable done in
similar places across arches.

Bjorn

Re: [patch 0/4] RFC: PCI: consolidate several pcibios_enable_resources() implementations

[Grant Grundler]

On Mon, Feb 18, 2008 at 09:39:52PM -0700, Bjorn Helgaas wrote:
> There are many implementations of pcibios_enable_resources() that differ
> in minor ways that look more like bugs than architectural differences.
> This patch series consolidates most of them to use the x86 version.
> 
> This series is for discussion only at this point.  I'm interested in
> feedback about whether any of the differences are "real" and need to
> be preserved.
> 
> ARM and PA-RISC, in particular, have interesting differences:
>     - ARM always enables bridge devices, which no other arch does
>     - PA-RISC always turns on SERR and PARITY, which no other arch does
> 
> Should other arches do the same thing, or are these somehow related to
> ARM and PA-RISC architecture?

My impression was most x86 BIOS's did NOT turn on SERR/PERR when I added
that code to parisc-linux port (2000 or 2001 so) . HPUX was turning on
SERR/PERR and so I was comfortable the HW was stable and it not crash
under normal use unless something was really broken.

There is certainly nothing architectural specific about SERR/PERR.
I felt (at the time) this is more of a case of "if it's not reported to the
user, we won't get blamed for it not working well."

hth,
grant