diff mbox

[RFC,1/3] MIPS: PCI: Use pci_bus_remove_resources()/pci_bus_add_resource() to set up root resources

Message ID 1314167063-15785-2-git-send-email-dengcheng.zhu@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Deng-Cheng Zhu Aug. 24, 2011, 6:24 a.m. UTC
Use this new style (instead of filling in the pci_bus->resource[] array
directly) to hide some ugly implementation details.

Signed-off-by: Deng-Cheng Zhu <dengcheng.zhu@gmail.com>
---
 arch/mips/pci/pci.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

Comments

Bjorn Helgaas Aug. 24, 2011, 2:35 p.m. UTC | #1
On Wed, Aug 24, 2011 at 12:24 AM, Deng-Cheng Zhu
<dengcheng.zhu@gmail.com> wrote:
> Use this new style (instead of filling in the pci_bus->resource[] array
> directly) to hide some ugly implementation details.
>
> Signed-off-by: Deng-Cheng Zhu <dengcheng.zhu@gmail.com>
> ---
>  arch/mips/pci/pci.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
> index 33bba7b..7473214 100644
> --- a/arch/mips/pci/pci.c
> +++ b/arch/mips/pci/pci.c
> @@ -261,6 +261,14 @@ static void pcibios_fixup_device_resources(struct pci_dev *dev,
>        }
>  }
>
> +static void __devinit
> +pcibios_setup_root_resources(struct pci_bus *bus, struct pci_controller *ctrl)
> +{
> +       pci_bus_remove_resources(bus);
> +       pci_bus_add_resource(bus, ctrl->io_resource, 0);
> +       pci_bus_add_resource(bus, ctrl->mem_resource, 0);
> +}
> +
>  void __devinit pcibios_fixup_bus(struct pci_bus *bus)
>  {
>        /* Propagate hose info into the subordinate devices.  */
> @@ -270,8 +278,7 @@ void __devinit pcibios_fixup_bus(struct pci_bus *bus)
>        struct pci_dev *dev = bus->self;
>
>        if (!dev) {
> -               bus->resource[0] = hose->io_resource;
> -               bus->resource[1] = hose->mem_resource;
> +               pcibios_setup_root_resources(bus, hose);
>        } else if (pci_probe_only &&
>                   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
>                pci_read_bridge_bases(bus);

I don't understand this patch.

Wouldn't it be enough to have [PATCH 2/3], which adds the
pci_create_bus() argument with nobody using it yet, then [PATCH 3/3],
which makes mips supply the new argument, and add a hunk to [PATCH
3/3] that completely removes the bus->resource fixups in
pcibios_fixup_bus() at the same time?

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
Deng-Cheng Zhu Aug. 25, 2011, 6:41 a.m. UTC | #2
2011/8/24 Bjorn Helgaas <bhelgaas@google.com>:
> Wouldn't it be enough to have [PATCH 2/3], which adds the
> pci_create_bus() argument with nobody using it yet, then [PATCH 3/3],
> which makes mips supply the new argument, and add a hunk to [PATCH
> 3/3] that completely removes the bus->resource fixups in
> pcibios_fixup_bus() at the same time?
>
> Bjorn
>

Do you mean to merge this patch to [PATCH 3/3]? OK, that's good!


Deng-Cheng
--
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 Aug. 25, 2011, 3:34 p.m. UTC | #3
On Thu, Aug 25, 2011 at 12:41 AM, Deng-Cheng Zhu
<dengcheng.zhu@gmail.com> wrote:
> 2011/8/24 Bjorn Helgaas <bhelgaas@google.com>:
>> Wouldn't it be enough to have [PATCH 2/3], which adds the
>> pci_create_bus() argument with nobody using it yet, then [PATCH 3/3],
>> which makes mips supply the new argument, and add a hunk to [PATCH
>> 3/3] that completely removes the bus->resource fixups in
>> pcibios_fixup_bus() at the same time?
>>
>> Bjorn
>>
>
> Do you mean to merge this patch to [PATCH 3/3]? OK, that's good!

No, I just mean that I don't see why you need this patch at all.  If
you pass the list of bus resources to pci_create_bus(), there's no
need to fix anything up later.  Or am I missing something?

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
Deng-Cheng Zhu Aug. 25, 2011, 11:14 p.m. UTC | #4
2011/8/25 Bjorn Helgaas <bhelgaas@google.com>:
> No, I just mean that I don't see why you need this patch at all.  If
> you pass the list of bus resources to pci_create_bus(), there's no
> need to fix anything up later.  Or am I missing something?

Well, doing the root resource fixups in here is a *paranoid* way. It's to
deal with the 'unlikely' circumstance where controller_resources() returns
the NULL pointer in pcibios_scanbus() due to memory allocation failure.
Most of the time (always) it's nothing more than repeating the resource
list setup. But maybe we can do something like this:

if (unlikely(!dev && list_empty(&bus->resources))
        pcibios_setup_root_resources(bus, hose);


What do you think?


Deng-Cheng
--
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 Aug. 25, 2011, 11:23 p.m. UTC | #5
On Thu, Aug 25, 2011 at 5:14 PM, Deng-Cheng Zhu <dengcheng.zhu@gmail.com> wrote:
> 2011/8/25 Bjorn Helgaas <bhelgaas@google.com>:
>> No, I just mean that I don't see why you need this patch at all.  If
>> you pass the list of bus resources to pci_create_bus(), there's no
>> need to fix anything up later.  Or am I missing something?
>
> Well, doing the root resource fixups in here is a *paranoid* way. It's to
> deal with the 'unlikely' circumstance where controller_resources() returns
> the NULL pointer in pcibios_scanbus() due to memory allocation failure.
> Most of the time (always) it's nothing more than repeating the resource
> list setup. But maybe we can do something like this:
>
> if (unlikely(!dev && list_empty(&bus->resources))
>        pcibios_setup_root_resources(bus, hose);
>
>
> What do you think?

I don't think it's worth it.  Just check for failure of
controller_resources(), and if it fails, skip the pci_create_bus()
call.  You've already printed a message (you might add the PCI
domain/bus number).

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

Patch

diff --git a/arch/mips/pci/pci.c b/arch/mips/pci/pci.c
index 33bba7b..7473214 100644
--- a/arch/mips/pci/pci.c
+++ b/arch/mips/pci/pci.c
@@ -261,6 +261,14 @@  static void pcibios_fixup_device_resources(struct pci_dev *dev,
 	}
 }
 
+static void __devinit
+pcibios_setup_root_resources(struct pci_bus *bus, struct pci_controller *ctrl)
+{
+	pci_bus_remove_resources(bus);
+	pci_bus_add_resource(bus, ctrl->io_resource, 0);
+	pci_bus_add_resource(bus, ctrl->mem_resource, 0);
+}
+
 void __devinit pcibios_fixup_bus(struct pci_bus *bus)
 {
 	/* Propagate hose info into the subordinate devices.  */
@@ -270,8 +278,7 @@  void __devinit pcibios_fixup_bus(struct pci_bus *bus)
 	struct pci_dev *dev = bus->self;
 
 	if (!dev) {
-		bus->resource[0] = hose->io_resource;
-		bus->resource[1] = hose->mem_resource;
+		pcibios_setup_root_resources(bus, hose);
 	} else if (pci_probe_only &&
 		   (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI) {
 		pci_read_bridge_bases(bus);