[PATCH 0/7] PCI: Use default pcibios_enable_device()

[PATCH 0/7] PCI: Use default pcibios_enable_device()

[Bjorn Helgaas]

Several architectures implement a simple pcibios_enable_device() that looks
like this:

  int pcibios_enable_device(struct pci_dev *dev, int mask)
  {
         return pci_enable_resources(dev, mask);
  }

This puts a weak version of that in the PCI core and removes the
corresponding implementations from the architectures.

s390 is slightly less trivial but the same general idea.

I'd like to do the same for all the other architectures, but some (arm,
cris, m68k, mips, unicore32, xtensa, and parts of sparc and tile) don't
seem to use pci_claim_resource(), so BAR resource r->parent pointers
probably aren't set, so I can't change them yet.

---

Bjorn Helgaas (7):
      PCI: Add "weak" generic pcibios_enable_device() implementation
      alpha/PCI: Use default pcibios_enable_device()
      microblaze/PCI: Use default pcibios_enable_device()
      sh/PCI: Use default pcibios_enable_device()
      sparc/PCI: Use default pcibios_enable_device() (Leon only)
      tile PCI RC: Use default pcibios_enable_device()
      s390/PCI: Use generic pci_enable_resources()


 arch/alpha/kernel/pci.c          |    6 ------
 arch/microblaze/pci/pci-common.c |    5 -----
 arch/s390/pci/pci.c              |   13 +------------
 arch/sh/drivers/pci/pci.c        |    5 -----
 arch/sparc/kernel/leon_pci.c     |    5 -----
 arch/tile/kernel/pci_gx.c        |   12 ------------
 drivers/pci/pci.c                |    5 +++++
 7 files changed, 6 insertions(+), 45 deletions(-)

[PATCH 1/7] PCI: Add "weak" generic pcibios_enable_device() implementation

[Bjorn Helgaas]

Many architectures implement pcibios_enable_device() the same way, so
provide a default implementation in the core.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
 drivers/pci/pci.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index dc9ce62be7aa..c3ce3d61091c 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1185,6 +1185,11 @@ int pci_load_and_free_saved_state(struct pci_dev *dev,
 }
 EXPORT_SYMBOL_GPL(pci_load_and_free_saved_state);
 
+int __weak pcibios_enable_device(struct pci_dev *dev, int bars)
+{
+	return pci_enable_resources(dev, bars);
+}
+
 static int do_pci_enable_device(struct pci_dev *dev, int bars)
 {
 	int err;

[PATCH 2/7] alpha/PCI: Use default pcibios_enable_device()

[Bjorn Helgaas]

We don't need anything arch-specific in pcibios_enable_device(), so drop
the arch implementation and use the default generic one.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: linux-alpha@vger.kernel.org
---
 arch/alpha/kernel/pci.c |    6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/alpha/kernel/pci.c b/arch/alpha/kernel/pci.c
index edb4e0097b75..076c35cd6cde 100644
--- a/arch/alpha/kernel/pci.c
+++ b/arch/alpha/kernel/pci.c
@@ -254,12 +254,6 @@ void pcibios_fixup_bus(struct pci_bus *bus)
 	}
 }
 
-int
-pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	return pci_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

[PATCH 3/7] microblaze/PCI: Use default pcibios_enable_device()

[Bjorn Helgaas]

We don't need anything arch-specific in pcibios_enable_device(), so drop
the arch implementation and use the default generic one.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Michal Simek <monstr@monstr.eu>
CC: microblaze-uclinux@itee.uq.edu.au
---
 arch/microblaze/pci/pci-common.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/microblaze/pci/pci-common.c b/arch/microblaze/pci/pci-common.c
index 66804adcacf0..70996cc66aa2 100644
--- a/arch/microblaze/pci/pci-common.c
+++ b/arch/microblaze/pci/pci-common.c
@@ -1294,11 +1294,6 @@ void pcibios_finish_adding_to_bus(struct pci_bus *bus)
 }
 EXPORT_SYMBOL_GPL(pcibios_finish_adding_to_bus);
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	return pci_enable_resources(dev, mask);
-}
-
 static void pcibios_setup_phb_resources(struct pci_controller *hose,
 					struct list_head *resources)
 {

[PATCH 4/7] sh/PCI: Use default pcibios_enable_device()

[Bjorn Helgaas]

We don't need anything arch-specific in pcibios_enable_device(), so drop
the arch implementation and use the default generic one.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Paul Mundt <lethal@linux-sh.org>
CC: linux-sh@vger.kernel.org
---
 arch/sh/drivers/pci/pci.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/sh/drivers/pci/pci.c b/arch/sh/drivers/pci/pci.c
index 60ed3e1c4b75..1bc09ee7948f 100644
--- a/arch/sh/drivers/pci/pci.c
+++ b/arch/sh/drivers/pci/pci.c
@@ -186,11 +186,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return start;
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	return pci_enable_resources(dev, mask);
-}
-
 static void __init
 pcibios_bus_report_status_early(struct pci_channel *hose,
 				int top_bus, int current_bus,

[PATCH 5/7] sparc/PCI: Use default pcibios_enable_device() (Leon only)

[Bjorn Helgaas]

We don't need anything arch-specific in pcibios_enable_device() so drop
the arch implementation and use the default generic one.

Note that sparc has two pcibios_enable_device() implementations other than
the one removed here.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Daniel Hellstrom <daniel@gaisler.com
---
 arch/sparc/kernel/leon_pci.c |    5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/sparc/kernel/leon_pci.c b/arch/sparc/kernel/leon_pci.c
index 88aaaa57bb64..e16c4157e1ae 100644
--- a/arch/sparc/kernel/leon_pci.c
+++ b/arch/sparc/kernel/leon_pci.c
@@ -99,11 +99,6 @@ resource_size_t pcibios_align_resource(void *data, const struct resource *res,
 	return res->start;
 }
 
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	return pci_enable_resources(dev, mask);
-}
-
 /* in/out routines taken from pcic.c
  *
  * This probably belongs here rather than ioport.c because

[PATCH 6/7] tile PCI RC: Use default pcibios_enable_device()

[Bjorn Helgaas]

We don't need anything arch-specific in pcibios_enable_device(), so drop
the arch implementation and use the default generic one.

Note: pci_enable_resources() checks that r->parent is non-NULL, which
basically checks that pci_claim_resource() or request_resource() has been
called for each BAR.  I don't see where that happens for tile, but this
patch doesn't change that behavior, so if it worked before, it should still
work.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Chris Metcalf <cmetcalf@tilera.com>
---
 arch/tile/kernel/pci_gx.c |   12 ------------
 1 file changed, 12 deletions(-)

diff --git a/arch/tile/kernel/pci_gx.c b/arch/tile/kernel/pci_gx.c
index a97a6452b812..077b7bc437e5 100644
--- a/arch/tile/kernel/pci_gx.c
+++ b/arch/tile/kernel/pci_gx.c
@@ -1065,18 +1065,6 @@ char *__init pcibios_setup(char *str)
 }
 
 /*
- * Enable memory address decoding, as appropriate, for the
- * device described by the 'dev' struct.
- *
- * This is called from the generic PCI layer, and can be called
- * for bridges or endpoints.
- */
-int pcibios_enable_device(struct pci_dev *dev, int mask)
-{
-	return pci_enable_resources(dev, mask);
-}
-
-/*
  * Called for each device after PCI setup is done.
  * We initialize the PCI device capabilities conservatively, assuming that
  * all devices can only address the 32-bit DMA space. The exception here is

[PATCH 7/7] s390/PCI: Use generic pci_enable_resources()

[Bjorn Helgaas]

The generic pci_enable_resources() does essentially the same thing as the
code in the s390 version of pcibios_enable_device().

There are differences, but I don't think any of them are a problem.  The
generic code:

  - Checks everything up to PCI_NUM_RESOURCES, not PCI_BAR_COUNT (6), so
    we'll now check the ROM resource, IOV resources, and bridge windows.

  - Checks for res->flags & IORESOURCE_UNSET.  The s390 code never sets
    IORESOURCE_UNSET, so this isn't a problem.

  - Checks res->parent.  The s390 pcibios_add_device() calls
    pci_claim_resource() on all BARs (except ROM, IOV, and bridge windows)
    so this isn't a problem either.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Martin Schwidefsky <schwidefsky@de.ibm.com>
CC: Heiko Carstens <heiko.carstens@de.ibm.com>
CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
CC: Jan Glauber <jang@linux.vnet.ibm.com>
CC: linux390@de.ibm.com
---
 arch/s390/pci/pci.c |   13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 66670ff262a0..7d5fcaed3361 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -695,18 +695,7 @@ int pcibios_enable_device(struct pci_dev *pdev, int mask)
 	zpci_fmb_enable_device(zdev);
 	zpci_map_resources(zdev);
 
-	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
-		res = &pdev->resource[i];
-
-		if (res->flags & IORESOURCE_IO)
-			return -EINVAL;
-
-		if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	pci_write_config_word(pdev, PCI_COMMAND, cmd);
-	return 0;
+	return pci_enable_resources(pdev, mask);
 }
 
 void pcibios_disable_device(struct pci_dev *pdev)

Re: [PATCH 7/7] s390/PCI: Use generic pci_enable_resources()

[Sebastian Ott]

Hello Bjorn,

On Wed, 26 Feb 2014, Bjorn Helgaas wrote:

> The generic pci_enable_resources() does essentially the same thing as the
> code in the s390 version of pcibios_enable_device().
> 
> There are differences, but I don't think any of them are a problem.  The
> generic code:
> 
>   - Checks everything up to PCI_NUM_RESOURCES, not PCI_BAR_COUNT (6), so
>     we'll now check the ROM resource, IOV resources, and bridge windows.
> 
>   - Checks for res->flags & IORESOURCE_UNSET.  The s390 code never sets
>     IORESOURCE_UNSET, so this isn't a problem.
> 
>   - Checks res->parent.  The s390 pcibios_add_device() calls
>     pci_claim_resource() on all BARs (except ROM, IOV, and bridge windows)
>     so this isn't a problem either.
> 
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Martin Schwidefsky <schwidefsky@de.ibm.com>
> CC: Heiko Carstens <heiko.carstens@de.ibm.com>
> CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
> CC: Jan Glauber <jang@linux.vnet.ibm.com>
> CC: linux390@de.ibm.com
> ---
>  arch/s390/pci/pci.c |   13 +------------
>  1 file changed, 1 insertion(+), 12 deletions(-)
> 
> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> index 66670ff262a0..7d5fcaed3361 100644
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -695,18 +695,7 @@ int pcibios_enable_device(struct pci_dev *pdev, int mask)
>  	zpci_fmb_enable_device(zdev);
>  	zpci_map_resources(zdev);
> 
> -	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> -	for (i = 0; i < PCI_BAR_COUNT; i++) {
> -		res = &pdev->resource[i];
> -
> -		if (res->flags & IORESOURCE_IO)
> -			return -EINVAL;
> -
> -		if (res->flags & IORESOURCE_MEM)
> -			cmd |= PCI_COMMAND_MEMORY;
> -	}
> -	pci_write_config_word(pdev, PCI_COMMAND, cmd);
> -	return 0;
> +	return pci_enable_resources(pdev, mask);
>  }
> 
>  void pcibios_disable_device(struct pci_dev *pdev)
> 

Let's also remove some now unused variables (updated patch below). Other
than that:

Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>

Regards,
Sebastian
---
s390/PCI: Use generic pci_enable_resources()

The generic pci_enable_resources() does essentially the same thing as the
code in the s390 version of pcibios_enable_device().

There are differences, but I don't think any of them are a problem.  The
generic code:

  - Checks everything up to PCI_NUM_RESOURCES, not PCI_BAR_COUNT (6), so
    we'll now check the ROM resource, IOV resources, and bridge windows.

  - Checks for res->flags & IORESOURCE_UNSET.  The s390 code never sets
    IORESOURCE_UNSET, so this isn't a problem.

  - Checks res->parent.  The s390 pcibios_add_device() calls
    pci_claim_resource() on all BARs (except ROM, IOV, and bridge windows)
    so this isn't a problem either.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
CC: Martin Schwidefsky <schwidefsky@de.ibm.com>
CC: Heiko Carstens <heiko.carstens@de.ibm.com>
CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
CC: Jan Glauber <jang@linux.vnet.ibm.com>
CC: linux390@de.ibm.com
---
 arch/s390/pci/pci.c |   16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -686,27 +686,13 @@ int pcibios_add_device(struct pci_dev *p
 int pcibios_enable_device(struct pci_dev *pdev, int mask)
 {
 	struct zpci_dev *zdev = get_zdev(pdev);
-	struct resource *res;
-	u16 cmd;
-	int i;
 
 	zdev->pdev = pdev;
 	zpci_debug_init_device(zdev);
 	zpci_fmb_enable_device(zdev);
 	zpci_map_resources(zdev);
 
-	pci_read_config_word(pdev, PCI_COMMAND, &cmd);
-	for (i = 0; i < PCI_BAR_COUNT; i++) {
-		res = &pdev->resource[i];
-
-		if (res->flags & IORESOURCE_IO)
-			return -EINVAL;
-
-		if (res->flags & IORESOURCE_MEM)
-			cmd |= PCI_COMMAND_MEMORY;
-	}
-	pci_write_config_word(pdev, PCI_COMMAND, cmd);
-	return 0;
+	return pci_enable_resources(pdev, mask);
 }
 
 void pcibios_disable_device(struct pci_dev *pdev)

Re: [PATCH 7/7] s390/PCI: Use generic pci_enable_resources()

[Bjorn Helgaas]

On Mon, Mar 3, 2014 at 6:35 AM, Sebastian Ott <sebott@linux.vnet.ibm.com> wrote:
> Hello Bjorn,
>
> On Wed, 26 Feb 2014, Bjorn Helgaas wrote:
>
>> The generic pci_enable_resources() does essentially the same thing as the
>> code in the s390 version of pcibios_enable_device().
>>
>> There are differences, but I don't think any of them are a problem.  The
>> generic code:
>>
>>   - Checks everything up to PCI_NUM_RESOURCES, not PCI_BAR_COUNT (6), so
>>     we'll now check the ROM resource, IOV resources, and bridge windows.
>>
>>   - Checks for res->flags & IORESOURCE_UNSET.  The s390 code never sets
>>     IORESOURCE_UNSET, so this isn't a problem.
>>
>>   - Checks res->parent.  The s390 pcibios_add_device() calls
>>     pci_claim_resource() on all BARs (except ROM, IOV, and bridge windows)
>>     so this isn't a problem either.
>>
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> CC: Martin Schwidefsky <schwidefsky@de.ibm.com>
>> CC: Heiko Carstens <heiko.carstens@de.ibm.com>
>> CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
>> CC: Jan Glauber <jang@linux.vnet.ibm.com>
>> CC: linux390@de.ibm.com
>> ---
>>  arch/s390/pci/pci.c |   13 +------------
>>  1 file changed, 1 insertion(+), 12 deletions(-)
>>
>> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
>> index 66670ff262a0..7d5fcaed3361 100644
>> --- a/arch/s390/pci/pci.c
>> +++ b/arch/s390/pci/pci.c
>> @@ -695,18 +695,7 @@ int pcibios_enable_device(struct pci_dev *pdev, int mask)
>>       zpci_fmb_enable_device(zdev);
>>       zpci_map_resources(zdev);
>>
>> -     pci_read_config_word(pdev, PCI_COMMAND, &cmd);
>> -     for (i = 0; i < PCI_BAR_COUNT; i++) {
>> -             res = &pdev->resource[i];
>> -
>> -             if (res->flags & IORESOURCE_IO)
>> -                     return -EINVAL;
>> -
>> -             if (res->flags & IORESOURCE_MEM)
>> -                     cmd |= PCI_COMMAND_MEMORY;
>> -     }
>> -     pci_write_config_word(pdev, PCI_COMMAND, cmd);
>> -     return 0;
>> +     return pci_enable_resources(pdev, mask);
>>  }
>>
>>  void pcibios_disable_device(struct pci_dev *pdev)
>>
>
> Let's also remove some now unused variables (updated patch below). Other
> than that:
>
> Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>

Hi Sebastian,

Sorry, that was sloppy of me to leave those unused variables.  I
updated the patch and added your ack.  Thanks!

Bjorn

> ---
> s390/PCI: Use generic pci_enable_resources()
>
> The generic pci_enable_resources() does essentially the same thing as the
> code in the s390 version of pcibios_enable_device().
>
> There are differences, but I don't think any of them are a problem.  The
> generic code:
>
>   - Checks everything up to PCI_NUM_RESOURCES, not PCI_BAR_COUNT (6), so
>     we'll now check the ROM resource, IOV resources, and bridge windows.
>
>   - Checks for res->flags & IORESOURCE_UNSET.  The s390 code never sets
>     IORESOURCE_UNSET, so this isn't a problem.
>
>   - Checks res->parent.  The s390 pcibios_add_device() calls
>     pci_claim_resource() on all BARs (except ROM, IOV, and bridge windows)
>     so this isn't a problem either.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Martin Schwidefsky <schwidefsky@de.ibm.com>
> CC: Heiko Carstens <heiko.carstens@de.ibm.com>
> CC: Sebastian Ott <sebott@linux.vnet.ibm.com>
> CC: Jan Glauber <jang@linux.vnet.ibm.com>
> CC: linux390@de.ibm.com
> ---
>  arch/s390/pci/pci.c |   16 +---------------
>  1 file changed, 1 insertion(+), 15 deletions(-)
>
> --- a/arch/s390/pci/pci.c
> +++ b/arch/s390/pci/pci.c
> @@ -686,27 +686,13 @@ int pcibios_add_device(struct pci_dev *p
>  int pcibios_enable_device(struct pci_dev *pdev, int mask)
>  {
>         struct zpci_dev *zdev = get_zdev(pdev);
> -       struct resource *res;
> -       u16 cmd;
> -       int i;
>
>         zdev->pdev = pdev;
>         zpci_debug_init_device(zdev);
>         zpci_fmb_enable_device(zdev);
>         zpci_map_resources(zdev);
>
> -       pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> -       for (i = 0; i < PCI_BAR_COUNT; i++) {
> -               res = &pdev->resource[i];
> -
> -               if (res->flags & IORESOURCE_IO)
> -                       return -EINVAL;
> -
> -               if (res->flags & IORESOURCE_MEM)
> -                       cmd |= PCI_COMMAND_MEMORY;
> -       }
> -       pci_write_config_word(pdev, PCI_COMMAND, cmd);
> -       return 0;
> +       return pci_enable_resources(pdev, mask);
>  }
>
>  void pcibios_disable_device(struct pci_dev *pdev)
>

Re: [PATCH 6/7] tile PCI RC: Use default pcibios_enable_device()

[Chris Metcalf]

On 2/26/2014 6:42 PM, Bjorn Helgaas wrote:
> We don't need anything arch-specific in pcibios_enable_device(), so drop
> the arch implementation and use the default generic one.
>
> Note: pci_enable_resources() checks that r->parent is non-NULL, which
> basically checks that pci_claim_resource() or request_resource() has been
> called for each BAR.  I don't see where that happens for tile, but this
> patch doesn't change that behavior, so if it worked before, it should still
> work.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> CC: Chris Metcalf <cmetcalf@tilera.com>
> ---
>  arch/tile/kernel/pci_gx.c |   12 ------------
>  1 file changed, 12 deletions(-)
>

Acked-by: Chris Metcalf <cmetcalf@tilera.com>

-- 
Chris Metcalf, Tilera Corp.
http://www.tilera.com

Re: [PATCH 0/7] PCI: Use default pcibios_enable_device()

[Bjorn Helgaas]

On Wed, Feb 26, 2014 at 4:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Several architectures implement a simple pcibios_enable_device() that looks
> like this:
>
>   int pcibios_enable_device(struct pci_dev *dev, int mask)
>   {
>          return pci_enable_resources(dev, mask);
>   }
>
> This puts a weak version of that in the PCI core and removes the
> corresponding implementations from the architectures.
>
> s390 is slightly less trivial but the same general idea.
>
> I'd like to do the same for all the other architectures, but some (arm,
> cris, m68k, mips, unicore32, xtensa, and parts of sparc and tile) don't
> seem to use pci_claim_resource(), so BAR resource r->parent pointers
> probably aren't set, so I can't change them yet.
>
> ---
>
> Bjorn Helgaas (7):
>       PCI: Add "weak" generic pcibios_enable_device() implementation
>       alpha/PCI: Use default pcibios_enable_device()
>       microblaze/PCI: Use default pcibios_enable_device()
>       sh/PCI: Use default pcibios_enable_device()
>       sparc/PCI: Use default pcibios_enable_device() (Leon only)
>       tile PCI RC: Use default pcibios_enable_device()
>       s390/PCI: Use generic pci_enable_resources()
>
>
>  arch/alpha/kernel/pci.c          |    6 ------
>  arch/microblaze/pci/pci-common.c |    5 -----
>  arch/s390/pci/pci.c              |   13 +------------
>  arch/sh/drivers/pci/pci.c        |    5 -----
>  arch/sparc/kernel/leon_pci.c     |    5 -----
>  arch/tile/kernel/pci_gx.c        |   12 ------------
>  drivers/pci/pci.c                |    5 +++++
>  7 files changed, 6 insertions(+), 45 deletions(-)

I applied these to pci/resource for v3.15.