All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] PCI: refactor io size calculation code
  2011-02-15  1:43 [PATCH 0/4] PCI: preallocate resource patch series Ram Pai
@ 2011-02-15  1:43 ` Ram Pai
  0 siblings, 0 replies; 5+ messages in thread
From: Ram Pai @ 2011-02-15  1:43 UTC (permalink / raw)
  To: linux-pci, jbarnes
  Cc: Ram Pai, linux-kernel, clemens, Yinghai Lu, Linus Torvalds,
	Bjorn Helgaas

refactor  code  that  calculates  the  io  size in pbus_size_io()
and pbus_mem_io() into separate functions.

Signed-off-by: Ram Pai <linuxram@us.ibm.com>
---
 drivers/pci/setup-bus.c |   66 ++++++++++++++++++++++++++++++-----------------
 1 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 66cb8f4..2121215 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -404,6 +404,43 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus, unsigned lon
 	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;
+	/* To be fixed in 2.5: we should have sort of HAVE_ISA
+	   flag in the struct pci_bus. */
+#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
+	size = (size & 0xff) + ((size & ~0xffUL) << 2);
+#endif
+	size = ALIGN(size + size1, align);
+	if (size < old_size)
+		size = old_size;
+	return size;
+}
+
+static resource_size_t calculate_memsize(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;
+	if (size < old_size)
+		size = old_size;
+	size = ALIGN(size + size1, align);
+	return size;
+}
+
 /* Sizing the IO windows of the PCI-PCI bridge is trivial,
    since these windows have 4K granularity and the IO ranges
    of non-bridge PCI devices are limited to 256 bytes.
@@ -412,7 +449,7 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 {
 	struct pci_dev *dev;
 	struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO);
-	unsigned long size = 0, size1 = 0, old_size;
+	unsigned long size = 0, size1 = 0;
 
 	if (!b_res)
  		return;
@@ -435,19 +472,8 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size)
 				size1 += r_size;
 		}
 	}
-	if (size < min_size)
-		size = min_size;
-	old_size = resource_size(b_res);
-	if (old_size == 1)
-		old_size = 0;
-/* To be fixed in 2.5: we should have sort of HAVE_ISA
-   flag in the struct pci_bus. */
-#if defined(CONFIG_ISA) || defined(CONFIG_EISA)
-	size = (size & 0xff) + ((size & ~0xffUL) << 2);
-#endif
-	size = ALIGN(size + size1, 4096);
-	if (size < old_size)
-		size = old_size;
+	size = calculate_iosize(size, min_size, size1,
+			resource_size(b_res), 4096);
 	if (!size) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "
@@ -468,7 +494,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			 unsigned long type, resource_size_t min_size)
 {
 	struct pci_dev *dev;
-	resource_size_t min_align, align, size, old_size;
+	resource_size_t min_align, align, size;
 	resource_size_t aligns[12];	/* Alignments from 1Mb to 2Gb */
 	int order, max_order;
 	struct resource *b_res = find_free_bus_resource(bus, type);
@@ -516,14 +542,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			mem64_mask &= r->flags & IORESOURCE_MEM_64;
 		}
 	}
-	if (size < min_size)
-		size = min_size;
-	old_size = resource_size(b_res);
-	if (old_size == 1)
-		old_size = 0;
-	if (size < old_size)
-		size = old_size;
-
 	align = 0;
 	min_align = 0;
 	for (order = 0; order <= max_order; order++) {
@@ -537,7 +555,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			min_align = align1 >> 1;
 		align += aligns[order];
 	}
-	size = ALIGN(size, min_align);
+	size = calculate_memsize(size, min_size, 0, resource_size(b_res), align);
 	if (!size) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "
-- 
1.6.5.2


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] PCI: refactor io size calculation code
@ 2011-04-11 13:18 Daniel Hellstrom
  2011-04-11 14:42 ` Linus Torvalds
  0 siblings, 1 reply; 5+ messages in thread
From: Daniel Hellstrom @ 2011-04-11 13:18 UTC (permalink / raw)
  To: Ram Pai
  Cc: linux-pci, linux-kernel, Clemens Ladisch, Yinghai Lu,
	Linus Torvalds, Kristoffer Glembo, daniel

>
>
>refactor  code  that  calculates  the  io  size in pbus_size_io()
>and pbus_mem_io() into separate functions.
>
>Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
>---
> drivers/pci/setup-bus.c |   66 ++++++++++++++++++++++++++++++-----------------
> 1 files changed, 42 insertions(+), 24 deletions(-)
>
>diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>index 66cb8f4..2121215 100644
>--- a/drivers/pci/setup-bus.c
>+++ b/drivers/pci/setup-bus.c
>@@ -404,6 +404,43 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus, unsigned lon
> 	return NULL;
> }
>
...

>+
>+static resource_size_t calculate_memsize(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;
>+	if (size < old_size)
>+		size = old_size;
>+	size = ALIGN(size + size1, align);
>+	return size;
>+}
>+
> /* Sizing the IO windows of the PCI-PCI bridge is trivial,
>    since these windows have 4K granularity and the IO ranges
>    of non-bridge PCI devices are limited to 256 bytes.
>
>  
>
...

>@@ -516,14 +542,6 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> 			mem64_mask &= r->flags & IORESOURCE_MEM_64;
> 		}
> 	}
>-	if (size < min_size)
>-		size = min_size;
>-	old_size = resource_size(b_res);
>-	if (old_size == 1)
>-		old_size = 0;
>-	if (size < old_size)
>-		size = old_size;
>-
> 	align = 0;
> 	min_align = 0;
> 	for (order = 0; order <= max_order; order++) {
>@@ -537,7 +555,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> 			min_align = align1 >> 1;
> 		align += aligns[order];
> 	}
>-	size = ALIGN(size, min_align);
>+	size = calculate_memsize(size, min_size, 0, resource_size(b_res), align);
>  
>
On my SPARC32/LEON4 PCI system I get overlapped areas, double mapped 
resources. Some BARs on PCIBUS0 are in the same non-prefetchable memory 
range as the secondary bus PCIBUS1. Changing align to min_align in the 
above call to calculate_memsize() fixes the problem, and the memory 
allocation is the same as with 2.6.36.4 kernel.

I belive this is just a typo.

Best Regards,
Daniel Hellstrom
Aeroflex Gaisler

> 	if (!size) {
> 		if (b_res->start || b_res->end)
> 			dev_info(&bus->self->dev, "disabling bridge window "
>-- 
>1.6.5.2
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] PCI: refactor io size calculation code
  2011-04-11 13:18 [PATCH 1/4] PCI: refactor io size calculation code Daniel Hellstrom
@ 2011-04-11 14:42 ` Linus Torvalds
  2011-04-11 14:53   ` Daniel Hellstrom
  2011-04-11 15:59   ` Ram Pai
  0 siblings, 2 replies; 5+ messages in thread
From: Linus Torvalds @ 2011-04-11 14:42 UTC (permalink / raw)
  To: Daniel Hellstrom
  Cc: Ram Pai, linux-pci, linux-kernel, Clemens Ladisch, Yinghai Lu,
	Kristoffer Glembo

[-- Attachment #1: Type: text/plain, Size: 3059 bytes --]

On Mon, Apr 11, 2011 at 6:18 AM, Daniel Hellstrom <daniel@gaisler.com> wrote:
>In commit 13583b16592a ("PCI: refactor io size calculation code"):
>>
>> refactor  code  that  calculates  the  io  size in pbus_size_io()
>> and pbus_mem_io() into separate functions.
>>
>> Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
>> ---
>> drivers/pci/setup-bus.c |   66
>> ++++++++++++++++++++++++++++++-----------------
>> 1 files changed, 42 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 66cb8f4..2121215 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -404,6 +404,43 @@ static struct resource *find_free_bus_resource(struct
>> pci_bus *bus, unsigned lon
>>        return NULL;
>> }
>>
> ...
>
>> +
>> +static resource_size_t calculate_memsize(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;
>> +       if (size < old_size)
>> +               size = old_size;
>> +       size = ALIGN(size + size1, align);
>> +       return size;
>> +}
>> +
>> /* Sizing the IO windows of the PCI-PCI bridge is trivial,
>>   since these windows have 4K granularity and the IO ranges
>>   of non-bridge PCI devices are limited to 256 bytes.
>>
>>
>
> ...
>
>> @@ -516,14 +542,6 @@ static int pbus_size_mem(struct pci_bus *bus,
>> unsigned long mask,
>>                        mem64_mask &= r->flags & IORESOURCE_MEM_64;
>>                }
>>        }
>> -       if (size < min_size)
>> -               size = min_size;
>> -       old_size = resource_size(b_res);
>> -       if (old_size == 1)
>> -               old_size = 0;
>> -       if (size < old_size)
>> -               size = old_size;
>> -
>>        align = 0;
>>        min_align = 0;
>>        for (order = 0; order <= max_order; order++) {
>> @@ -537,7 +555,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned
>> long mask,
>>                        min_align = align1 >> 1;
>>                align += aligns[order];
>>        }
>> -       size = ALIGN(size, min_align);
>> +       size = calculate_memsize(size, min_size, 0, resource_size(b_res),
>> align);
>>
>
> On my SPARC32/LEON4 PCI system I get overlapped areas, double mapped
> resources. Some BARs on PCIBUS0 are in the same non-prefetchable memory
> range as the secondary bus PCIBUS1. Changing align to min_align in the above
> call to calculate_memsize() fixes the problem, and the memory allocation is
> the same as with 2.6.36.4 kernel.
>
> I belive this is just a typo.

It does seem that way. The original code used 'min_align' and 'align'
itself is meaningless in that place.

Can you confirm that the patch you talk about as fixing things is the attached?

                  Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 827 bytes --]

 drivers/pci/setup-bus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 89d0a6a88df7..ebf51ad1b714 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -676,10 +676,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			min_align = align1 >> 1;
 		align += aligns[order];
 	}
-	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), align);
+	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
 	size1 = !add_size ? size :
 		calculate_memsize(size, min_size+add_size, 0,
-				resource_size(b_res), align);
+				resource_size(b_res), min_align);
 	if (!size0 && !size1) {
 		if (b_res->start || b_res->end)
 			dev_info(&bus->self->dev, "disabling bridge window "

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] PCI: refactor io size calculation code
  2011-04-11 14:42 ` Linus Torvalds
@ 2011-04-11 14:53   ` Daniel Hellstrom
  2011-04-11 15:59   ` Ram Pai
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Hellstrom @ 2011-04-11 14:53 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Ram Pai, linux-pci, linux-kernel, Clemens Ladisch, Yinghai Lu,
	Kristoffer Glembo

Linus Torvalds wrote:

>On Mon, Apr 11, 2011 at 6:18 AM, Daniel Hellstrom <daniel@gaisler.com> wrote:
>  
>
>>In commit 13583b16592a ("PCI: refactor io size calculation code"):
>>    
>>
>>>refactor  code  that  calculates  the  io  size in pbus_size_io()
>>>and pbus_mem_io() into separate functions.
>>>
>>>Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
>>>---
>>>drivers/pci/setup-bus.c |   66
>>>++++++++++++++++++++++++++++++-----------------
>>>1 files changed, 42 insertions(+), 24 deletions(-)
>>>
>>>diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>>index 66cb8f4..2121215 100644
>>>--- a/drivers/pci/setup-bus.c
>>>+++ b/drivers/pci/setup-bus.c
>>>@@ -404,6 +404,43 @@ static struct resource *find_free_bus_resource(struct
>>>pci_bus *bus, unsigned lon
>>>       return NULL;
>>>}
>>>
>>>      
>>>
>>...
>>
>>    
>>
>>>+
>>>+static resource_size_t calculate_memsize(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;
>>>+       if (size < old_size)
>>>+               size = old_size;
>>>+       size = ALIGN(size + size1, align);
>>>+       return size;
>>>+}
>>>+
>>>/* Sizing the IO windows of the PCI-PCI bridge is trivial,
>>>  since these windows have 4K granularity and the IO ranges
>>>  of non-bridge PCI devices are limited to 256 bytes.
>>>
>>>
>>>      
>>>
>>...
>>
>>    
>>
>>>@@ -516,14 +542,6 @@ static int pbus_size_mem(struct pci_bus *bus,
>>>unsigned long mask,
>>>                       mem64_mask &= r->flags & IORESOURCE_MEM_64;
>>>               }
>>>       }
>>>-       if (size < min_size)
>>>-               size = min_size;
>>>-       old_size = resource_size(b_res);
>>>-       if (old_size == 1)
>>>-               old_size = 0;
>>>-       if (size < old_size)
>>>-               size = old_size;
>>>-
>>>       align = 0;
>>>       min_align = 0;
>>>       for (order = 0; order <= max_order; order++) {
>>>@@ -537,7 +555,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned
>>>long mask,
>>>                       min_align = align1 >> 1;
>>>               align += aligns[order];
>>>       }
>>>-       size = ALIGN(size, min_align);
>>>+       size = calculate_memsize(size, min_size, 0, resource_size(b_res),
>>>align);
>>>
>>>      
>>>
>>On my SPARC32/LEON4 PCI system I get overlapped areas, double mapped
>>resources. Some BARs on PCIBUS0 are in the same non-prefetchable memory
>>range as the secondary bus PCIBUS1. Changing align to min_align in the above
>>call to calculate_memsize() fixes the problem, and the memory allocation is
>>the same as with 2.6.36.4 kernel.
>>
>>I belive this is just a typo.
>>    
>>
>
>It does seem that way. The original code used 'min_align' and 'align'
>itself is meaningless in that place.
>  
>
Agree.

>Can you confirm that the patch you talk about as fixing things is the attached?
>
>                  Linus
>  
>
Yes, the below patch works on the system that triggered the problem 
before. The last min_align was added in a later patch, I missed it 
initially.

Thanks,
Daniel

>------------------------------------------------------------------------
>
> drivers/pci/setup-bus.c |    4 ++--
> 1 files changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>index 89d0a6a88df7..ebf51ad1b714 100644
>--- a/drivers/pci/setup-bus.c
>+++ b/drivers/pci/setup-bus.c
>@@ -676,10 +676,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
> 			min_align = align1 >> 1;
> 		align += aligns[order];
> 	}
>-	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), align);
>+	size0 = calculate_memsize(size, min_size, 0, resource_size(b_res), min_align);
> 	size1 = !add_size ? size :
> 		calculate_memsize(size, min_size+add_size, 0,
>-				resource_size(b_res), align);
>+				resource_size(b_res), min_align);
> 	if (!size0 && !size1) {
> 		if (b_res->start || b_res->end)
> 			dev_info(&bus->self->dev, "disabling bridge window "
>  
>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 1/4] PCI: refactor io size calculation code
  2011-04-11 14:42 ` Linus Torvalds
  2011-04-11 14:53   ` Daniel Hellstrom
@ 2011-04-11 15:59   ` Ram Pai
  1 sibling, 0 replies; 5+ messages in thread
From: Ram Pai @ 2011-04-11 15:59 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Daniel Hellstrom, Ram Pai, linux-pci, linux-kernel,
	Clemens Ladisch, Yinghai Lu, Kristoffer Glembo

On Mon, Apr 11, 2011 at 07:42:13AM -0700, Linus Torvalds wrote:
> On Mon, Apr 11, 2011 at 6:18 AM, Daniel Hellstrom <daniel@gaisler.com> wrote:
> >In commit 13583b16592a ("PCI: refactor io size calculation code"):
> >>
> >> refactor  code  that  calculates  the  io  size in pbus_size_io()
> >> and pbus_mem_io() into separate functions.
> >>
> >> Signed-off-by: Ram Pai <linuxram@xxxxxxxxxx>
> >> ---
> >> drivers/pci/setup-bus.c |   66
> >> ++++++++++++++++++++++++++++++-----------------
> >> 1 files changed, 42 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> >> index 66cb8f4..2121215 100644
> >> --- a/drivers/pci/setup-bus.c
> >> +++ b/drivers/pci/setup-bus.c
> >> @@ -404,6 +404,43 @@ static struct resource *find_free_bus_resource(struct
> >> pci_bus *bus, unsigned lon
> >>        return NULL;
> >> }
> >>
> > ...
> >
> >> +
> >> +static resource_size_t calculate_memsize(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;
> >> +       if (size < old_size)
> >> +               size = old_size;
> >> +       size = ALIGN(size + size1, align);
> >> +       return size;
> >> +}
> >> +
> >> /* Sizing the IO windows of the PCI-PCI bridge is trivial,
> >>   since these windows have 4K granularity and the IO ranges
> >>   of non-bridge PCI devices are limited to 256 bytes.
> >>
> >>
> >
> > ...
> >
> >> @@ -516,14 +542,6 @@ static int pbus_size_mem(struct pci_bus *bus,
> >> unsigned long mask,
> >>                        mem64_mask &= r->flags & IORESOURCE_MEM_64;
> >>                }
> >>        }
> >> -       if (size < min_size)
> >> -               size = min_size;
> >> -       old_size = resource_size(b_res);
> >> -       if (old_size == 1)
> >> -               old_size = 0;
> >> -       if (size < old_size)
> >> -               size = old_size;
> >> -
> >>        align = 0;
> >>        min_align = 0;
> >>        for (order = 0; order <= max_order; order++) {
> >> @@ -537,7 +555,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned
> >> long mask,
> >>                        min_align = align1 >> 1;
> >>                align += aligns[order];
> >>        }
> >> -       size = ALIGN(size, min_align);
> >> +       size = calculate_memsize(size, min_size, 0, resource_size(b_res),
> >> align);
> >>
> >
> > On my SPARC32/LEON4 PCI system I get overlapped areas, double mapped
> > resources. Some BARs on PCIBUS0 are in the same non-prefetchable memory
> > range as the secondary bus PCIBUS1. Changing align to min_align in the above
> > call to calculate_memsize() fixes the problem, and the memory allocation is
> > the same as with 2.6.36.4 kernel.
> >
> > I belive this is just a typo.


Yes it is my mistake. I got the parameter wrong. Your fix should correct the problem.

RP

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-04-11 15:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-04-11 13:18 [PATCH 1/4] PCI: refactor io size calculation code Daniel Hellstrom
2011-04-11 14:42 ` Linus Torvalds
2011-04-11 14:53   ` Daniel Hellstrom
2011-04-11 15:59   ` Ram Pai
  -- strict thread matches above, loose matches on Subject: below --
2011-02-15  1:43 [PATCH 0/4] PCI: preallocate resource patch series Ram Pai
2011-02-15  1:43 ` [PATCH 1/4] PCI: refactor io size calculation code Ram Pai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.