diff mbox

[v3,28/51] PCI: Unifiy calculate_size for io port and mmio

Message ID 1438039809-24957-29-git-send-email-yinghai@kernel.org
State Superseded
Headers show

Commit Message

Yinghai Lu July 27, 2015, 11:29 p.m. UTC
We should check size+size1 with min_size for io port.
For example, when hotplug bridge has two children bridges,
every child bridge will need 0x1000, so size1 will be 0x2000
and size is 0. The min_size for the hotplug bridge is 0x100.
with old version calculate_iosize, we get 0x3000 for final
size because we are using size to compare with min_size. That is
not right, we should use 0x2000 instead.

After this change, calculate_memsize and calculate_iosize
is the same.

Change them to calculate_size.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c | 27 ++++++---------------------
 1 file changed, 6 insertions(+), 21 deletions(-)

Comments

Bjorn Helgaas Aug. 18, 2015, 4:13 a.m. UTC | #1
On Mon, Jul 27, 2015 at 04:29:46PM -0700, Yinghai Lu wrote:
> We should check size+size1 with min_size for io port.
> For example, when hotplug bridge has two children bridges,
> every child bridge will need 0x1000, so size1 will be 0x2000
> and size is 0. The min_size for the hotplug bridge is 0x100.

A min_size of 0x100?  Is that a typo?

> with old version calculate_iosize, we get 0x3000 for final
> size because we are using size to compare with min_size. That is
> not right, we should use 0x2000 instead.

If this fixes a bug, please make one patch that *only* fixes the bug,
and a separate one that unifies but doesn't change the behavior.

> After this change, calculate_memsize and calculate_iosize
> is the same.
> 
> Change them to calculate_size.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/setup-bus.c | 27 ++++++---------------------
>  1 file changed, 6 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 3db2838..aeed716 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1116,23 +1116,7 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus,
>  	return NULL;
>  }
>  
> -static resource_size_t calculate_iosize(resource_size_t size,
> -		resource_size_t min_size,
> -		resource_size_t size1,
> -		resource_size_t old_size,
> -		resource_size_t align)
> -{
> -	if (size < min_size)
> -		size = min_size;
> -	if (old_size == 1)
> -		old_size = 0;
> -	size = ALIGN(size + size1, align);
> -	if (size < old_size)
> -		size = old_size;
> -	return size;
> -}
> -
> -static resource_size_t calculate_memsize(resource_size_t size,
> +static resource_size_t calculate_size(resource_size_t size,
>  		resource_size_t min_size,
>  		resource_size_t old_size,
>  		resource_size_t align)
> @@ -1257,14 +1241,15 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
>  	}
>  
>  	size = size_aligned_for_isa(size);
> -	size0 = calculate_iosize(size, min_size, size1,
> +	size += size1;
> +	size0 = calculate_size(size, min_size,
>  			resource_size(b_res), min_align);
>  	sum_add_size = size_aligned_for_isa(sum_add_size);
>  	sum_add_size += sum_add_size1;
>  	if (sum_add_size < min_sum_size)
>  		sum_add_size = min_sum_size;
>  	size1 = !realloc_head ? size0 :
> -		calculate_iosize(sum_add_size, min_size, 0,
> +		calculate_size(sum_add_size, min_size,
>  			resource_size(b_res), min_align);
>  	if (!size0 && !size1) {
>  		if (b_res->start || b_res->end)
> @@ -1617,7 +1602,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>  	if (size || min_size) {
>  		min_align = calculate_mem_align(&align_test_list, max_align,
>  						size, window_align);
> -		size0 = calculate_memsize(size, min_size,
> +		size0 = calculate_size(size, min_size,
>  				  resource_size(b_res), min_align);
>  	}
>  	free_align_test_list(&align_test_list);
> @@ -1642,7 +1627,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
>  		min_add_align = calculate_mem_align(&align_test_add_list,
>  					max_add_align, sum_add_size,
>  					window_align);
> -		size1 = calculate_memsize(sum_add_size, min_size,
> +		size1 = calculate_size(sum_add_size, min_size,
>  				 resource_size(b_res), min_add_align);
>  	}
>  	free_align_test_list(&align_test_add_list);
> -- 
> 1.8.4.5
> 
--
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
Yinghai Lu Aug. 19, 2015, 6:37 a.m. UTC | #2
On Mon, Aug 17, 2015 at 9:13 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Mon, Jul 27, 2015 at 04:29:46PM -0700, Yinghai Lu wrote:
>> We should check size+size1 with min_size for io port.
>> For example, when hotplug bridge has two children bridges,
>> every child bridge will need 0x1000, so size1 will be 0x2000
>> and size is 0. The min_size for the hotplug bridge is 0x100.
>
> A min_size of 0x100?  Is that a typo?

yes, it is 0x100.

#define DEFAULT_HOTPLUG_IO_SIZE         (256)
/* pci=hpmemsize=nnM,hpiosize=nn can override this */
unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;

and we have

                if (bus->self->is_hotplug_bridge) {
                        min_io_size  = pci_hotplug_io_size;

>
>> with old version calculate_iosize, we get 0x3000 for final
>> size because we are using size to compare with min_size. That is
>> not right, we should use 0x2000 instead.
>
> If this fixes a bug, please make one patch that *only* fixes the bug,
> and a separate one that unifies but doesn't change the behavior.

ok, will separate it into two.
--
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/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3db2838..aeed716 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1116,23 +1116,7 @@  static struct resource *find_free_bus_resource(struct pci_bus *bus,
 	return NULL;
 }
 
-static resource_size_t calculate_iosize(resource_size_t size,
-		resource_size_t min_size,
-		resource_size_t size1,
-		resource_size_t old_size,
-		resource_size_t align)
-{
-	if (size < min_size)
-		size = min_size;
-	if (old_size == 1)
-		old_size = 0;
-	size = ALIGN(size + size1, align);
-	if (size < old_size)
-		size = old_size;
-	return size;
-}
-
-static resource_size_t calculate_memsize(resource_size_t size,
+static resource_size_t calculate_size(resource_size_t size,
 		resource_size_t min_size,
 		resource_size_t old_size,
 		resource_size_t align)
@@ -1257,14 +1241,15 @@  static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 	}
 
 	size = size_aligned_for_isa(size);
-	size0 = calculate_iosize(size, min_size, size1,
+	size += size1;
+	size0 = calculate_size(size, min_size,
 			resource_size(b_res), min_align);
 	sum_add_size = size_aligned_for_isa(sum_add_size);
 	sum_add_size += sum_add_size1;
 	if (sum_add_size < min_sum_size)
 		sum_add_size = min_sum_size;
 	size1 = !realloc_head ? size0 :
-		calculate_iosize(sum_add_size, min_size, 0,
+		calculate_size(sum_add_size, min_size,
 			resource_size(b_res), min_align);
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
@@ -1617,7 +1602,7 @@  static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	if (size || min_size) {
 		min_align = calculate_mem_align(&align_test_list, max_align,
 						size, window_align);
-		size0 = calculate_memsize(size, min_size,
+		size0 = calculate_size(size, min_size,
 				  resource_size(b_res), min_align);
 	}
 	free_align_test_list(&align_test_list);
@@ -1642,7 +1627,7 @@  static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 		min_add_align = calculate_mem_align(&align_test_add_list,
 					max_add_align, sum_add_size,
 					window_align);
-		size1 = calculate_memsize(sum_add_size, min_size,
+		size1 = calculate_size(sum_add_size, min_size,
 				 resource_size(b_res), min_add_align);
 	}
 	free_align_test_list(&align_test_add_list);