[v5,6/9] PCI: Move resource distribution for a single bridge outside of the loop

Message ID 20180416103453.46232-7-mika.westerberg@linux.intel.com
State Changes Requested
Delegated to: Bjorn Helgaas
Headers show
Series
  • PCI: Fixes and cleanups for native PCIe and ACPI hotplug
Related show

Commit Message

Mika Westerberg April 16, 2018, 10:34 a.m.
There is a special case where there is only single bridge on the bus. In
that case we just assign all resources to it. Currently this is done as
a part of the resource distribution loop but it does not have to be
there, and moving it outside actually improves readability.

While there we can add hotplug_bridges == 1 && normal_bridges == 0 to
the same block because they are dealt the same way.

Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/pci/setup-bus.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

Comments

Bjorn Helgaas April 24, 2018, 11:05 p.m. | #1
On Mon, Apr 16, 2018 at 01:34:50PM +0300, Mika Westerberg wrote:
> There is a special case where there is only single bridge on the bus. In
> that case we just assign all resources to it. Currently this is done as
> a part of the resource distribution loop but it does not have to be
> there, and moving it outside actually improves readability.
> 
> While there we can add hotplug_bridges == 1 && normal_bridges == 0 to
> the same block because they are dealt the same way.
> 
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Reviewed-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> ---
>  drivers/pci/setup-bus.c | 28 +++++++++++++++++-----------
>  1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index eb3059fb7f63..a1b63c7745d0 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1947,6 +1947,22 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>  	mmio_start = mmio_res->start;
>  	mmio_pref_start = mmio_pref_res->start;
>  
> +	/*
> +	 * There is only one bridge on the bus so it gets all available
> +	 * resources which it can then distribute to the possible
> +	 * hotplug bridges below.
> +	 */
> +	if ((hotplug_bridges == 0 && normal_bridges == 1) ||
> +	    (hotplug_bridges == 1 && normal_bridges == 0)) {

  if (hotplug_bridges + normal_bridges == 1) {

Don't repost just for this; I can fold it in if you agree.

> +		dev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
> +		if (dev->subordinate) {
> +			pci_bus_distribute_available_resources(dev->subordinate,
> +				add_list, available_io, available_mmio,
> +				available_mmio_pref);
> +		}
> +		return;
> +	}
> +
>  	/*
>  	 * Go over devices on this bus and distribute the remaining
>  	 * resource space between hotplug bridges.
> @@ -1959,17 +1975,7 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
>  		if (!b)
>  			continue;
>  
> -		if (!hotplug_bridges && normal_bridges == 1) {
> -			/*
> -			 * There is only one bridge on the bus (upstream
> -			 * port) so it gets all available resources
> -			 * which it can then distribute to the possible
> -			 * hotplug bridges below.
> -			 */
> -			pci_bus_distribute_available_resources(b, add_list,
> -				available_io, available_mmio,
> -				available_mmio_pref);
> -		} else if (dev->is_hotplug_bridge) {
> +		if (dev->is_hotplug_bridge) {
>  			resource_size_t io, mmio, mmio_pref;
>  
>  			/*
> -- 
> 2.16.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg April 25, 2018, 7:29 a.m. | #2
On Tue, Apr 24, 2018 at 06:05:58PM -0500, Bjorn Helgaas wrote:
> > +	/*
> > +	 * There is only one bridge on the bus so it gets all available
> > +	 * resources which it can then distribute to the possible
> > +	 * hotplug bridges below.
> > +	 */
> > +	if ((hotplug_bridges == 0 && normal_bridges == 1) ||
> > +	    (hotplug_bridges == 1 && normal_bridges == 0)) {
> 
>   if (hotplug_bridges + normal_bridges == 1) {
> 
> Don't repost just for this; I can fold it in if you agree.

I agree, looks better like that. Thanks!

Patch

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index eb3059fb7f63..a1b63c7745d0 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1947,6 +1947,22 @@  static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 	mmio_start = mmio_res->start;
 	mmio_pref_start = mmio_pref_res->start;
 
+	/*
+	 * There is only one bridge on the bus so it gets all available
+	 * resources which it can then distribute to the possible
+	 * hotplug bridges below.
+	 */
+	if ((hotplug_bridges == 0 && normal_bridges == 1) ||
+	    (hotplug_bridges == 1 && normal_bridges == 0)) {
+		dev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
+		if (dev->subordinate) {
+			pci_bus_distribute_available_resources(dev->subordinate,
+				add_list, available_io, available_mmio,
+				available_mmio_pref);
+		}
+		return;
+	}
+
 	/*
 	 * Go over devices on this bus and distribute the remaining
 	 * resource space between hotplug bridges.
@@ -1959,17 +1975,7 @@  static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 		if (!b)
 			continue;
 
-		if (!hotplug_bridges && normal_bridges == 1) {
-			/*
-			 * There is only one bridge on the bus (upstream
-			 * port) so it gets all available resources
-			 * which it can then distribute to the possible
-			 * hotplug bridges below.
-			 */
-			pci_bus_distribute_available_resources(b, add_list,
-				available_io, available_mmio,
-				available_mmio_pref);
-		} else if (dev->is_hotplug_bridge) {
+		if (dev->is_hotplug_bridge) {
 			resource_size_t io, mmio, mmio_pref;
 
 			/*