[PATCH -v2 0/9] PCI : bridge resource reallocation patchset -- followup

[PATCH -v2 0/9] PCI : bridge resource reallocation patchset -- followup

[Yinghai Lu]

c16c722: PCI: only enable pci realloc when SRIOV bar is not assigned
868faa5: PCI: print out suggestion about using pci=realloc
ace6e3c: PCI: Make pci bridge reallocating enabled/disabled
3cf1011: PCI: Retry on type IORESOURCE_IO allocation.
b6d30ee: PCI: Skip reset cardbus assigned resource during pci bus rescan
2bc1cf0: PCI: Fix cardbus bridge resources as optional size handling
e6c495b: PCI: Disable cardbus bridge MEM1 pref CTL
d1e2d75: PCI: Fix /sys warning when sriov enabled card is hot removed
29834f2: pci: Fix pci cardbus removal

are left over after Jesse pickup more of them.
it will try to auto detect if need to use pci=realloc, and print out suggestion.

including fixing some pci carbus handling.

could get from

	git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci3

-v2: add two patches before, it will fix pci cardbus removal, and srov removal
	warning
	the first one should be with v3.3.

Thanks

Yinghai

 Documentation/kernel-parameters.txt |    8 ++-
 drivers/pci/iov.c                   |    7 ++-
 drivers/pci/pci.c                   |    4 +-
 drivers/pci/pci.h                   |    2 +-
 drivers/pci/remove.c                |   28 +++++--
 drivers/pci/setup-bus.c             |  159 +++++++++++++++++++++++++----------
 6 files changed, 153 insertions(+), 55 deletions(-)

[PATCH 1/9] pci: Fix pci cardbus removal

[Yinghai Lu]

During test busn_res allocation with cardbus, found pci card removal is not
working anymore, and it turns out it is broken by:

|commit 79cc9601c3e42b4f0650fe7e69132ebce7ab48f9
|Date:   Tue Nov 22 21:06:53 2011 -0800
|
|    PCI: Only call pci_stop_bus_device() one time for child devices at remove

that patch changed pci_remove_behind_bridge behavoir that yenta_carbus depends.

Restore the old behavoir of pci_remove_behind_bridge by:
1. rename pci_remove_behind_bridge to __pci_remove_behind_bridge, and let
	__pci_remove_bus_device() call it instead.
2. add pci-stop_befind_bridge that will stop device under bridge
3. add back pci_remove_behind_bridge that will stop and remove device
	under bridge.

This one is for v3.3

-v2: update commit description a little bit.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/remove.c |   28 ++++++++++++++++++++++------
 1 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index bd70f23..82f8ae5 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -77,6 +77,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 }
 EXPORT_SYMBOL(pci_remove_bus);
 
+static void __pci_remove_behind_bridge(struct pci_dev *dev);
 /**
  * pci_remove_bus_device - remove a PCI device and any children
  * @dev: the device to remove
@@ -94,7 +95,7 @@ static void __pci_remove_bus_device(struct pci_dev *dev)
 	if (dev->subordinate) {
 		struct pci_bus *b = dev->subordinate;
 
-		pci_remove_behind_bridge(dev);
+		__pci_remove_behind_bridge(dev);
 		pci_remove_bus(b);
 		dev->subordinate = NULL;
 	}
@@ -107,6 +108,24 @@ void pci_remove_bus_device(struct pci_dev *dev)
 	__pci_remove_bus_device(dev);
 }
 
+static void __pci_remove_behind_bridge(struct pci_dev *dev)
+{
+	struct list_head *l, *n;
+
+	if (dev->subordinate)
+		list_for_each_safe(l, n, &dev->subordinate->devices)
+			__pci_remove_bus_device(pci_dev_b(l));
+}
+
+static void pci_stop_behind_bridge(struct pci_dev *dev)
+{
+	struct list_head *l, *n;
+
+	if (dev->subordinate)
+		list_for_each_safe(l, n, &dev->subordinate->devices)
+			pci_stop_bus_device(pci_dev_b(l));
+}
+
 /**
  * pci_remove_behind_bridge - remove all devices behind a PCI bridge
  * @dev: PCI bridge device
@@ -117,11 +136,8 @@ void pci_remove_bus_device(struct pci_dev *dev)
  */
 void pci_remove_behind_bridge(struct pci_dev *dev)
 {
-	struct list_head *l, *n;
-
-	if (dev->subordinate)
-		list_for_each_safe(l, n, &dev->subordinate->devices)
-			__pci_remove_bus_device(pci_dev_b(l));
+	pci_stop_behind_bridge(dev);
+	__pci_remove_behind_bridge(dev);
 }
 
 static void pci_stop_bus_devices(struct pci_bus *bus)
-- 
1.7.7

Re: [PATCH 1/9] pci: Fix pci cardbus removal

[Jesse Barnes]

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

On Sat,  4 Feb 2012 22:55:00 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> During test busn_res allocation with cardbus, found pci card removal is not
> working anymore, and it turns out it is broken by:
> 
> |commit 79cc9601c3e42b4f0650fe7e69132ebce7ab48f9
> |Date:   Tue Nov 22 21:06:53 2011 -0800
> |
> |    PCI: Only call pci_stop_bus_device() one time for child devices at remove
> 
> that patch changed pci_remove_behind_bridge behavoir that yenta_carbus depends.
> 
> Restore the old behavoir of pci_remove_behind_bridge by:
> 1. rename pci_remove_behind_bridge to __pci_remove_behind_bridge, and let
> 	__pci_remove_bus_device() call it instead.
> 2. add pci-stop_befind_bridge that will stop device under bridge
> 3. add back pci_remove_behind_bridge that will stop and remove device
> 	under bridge.
> 
> This one is for v3.3

Applied to -fixes with Dominik's tested-by.  A nice cleanup on top
might be to rename pci_remove_behind_bridge to
pci_stop_remove_behind_bridge or something to indicate that it's
calling stop each time as well...

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 2/9] PCI: Fix /sys warning when sriov enabled card is hot removed

[Yinghai Lu]

During recent strick checking about sysfs_remove from Eric.
it will spit more bitter warning.

For SRIOV hotplug, we are calling pci_stop_dev() for VF at first.
(after we update pci_stop_bus_devices).

that pci_stop_dev will calling device_unregiste for that VF, so that directory
in VF is removed already.

We double checking if that VF dir in /sys is there, before try removing that
 physfn link.

Signed-of-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/iov.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
index 0321fa3..dfc7d65 100644
--- a/drivers/pci/iov.c
+++ b/drivers/pci/iov.c
@@ -173,7 +173,12 @@ static void virtfn_remove(struct pci_dev *dev, int id, int reset)
 
 	sprintf(buf, "virtfn%u", id);
 	sysfs_remove_link(&dev->dev.kobj, buf);
-	sysfs_remove_link(&virtfn->dev.kobj, "physfn");
+	/*
+	 * pci_stop_dev() could be called for this virtfn before already
+	 *  so directory for the virtfn is removed before.
+	 */
+	if (virtfn->dev.kobj.sd)
+		sysfs_remove_link(&virtfn->dev.kobj, "physfn");
 
 	mutex_lock(&iov->dev->sriov->lock);
 	pci_remove_bus_device(virtfn);
-- 
1.7.7

Re: [PATCH 2/9] PCI: Fix /sys warning when sriov enabled card is hot removed

[Jesse Barnes]

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

On Sat,  4 Feb 2012 22:55:01 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> During recent strick checking about sysfs_remove from Eric.
> it will spit more bitter warning.
> 
> For SRIOV hotplug, we are calling pci_stop_dev() for VF at first.
> (after we update pci_stop_bus_devices).
> 
> that pci_stop_dev will calling device_unregiste for that VF, so that directory
> in VF is removed already.
> 
> We double checking if that VF dir in /sys is there, before try removing that
>  physfn link.
> 
> Signed-of-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/iov.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
> index 0321fa3..dfc7d65 100644
> --- a/drivers/pci/iov.c
> +++ b/drivers/pci/iov.c
> @@ -173,7 +173,12 @@ static void virtfn_remove(struct pci_dev *dev, int id, int reset)
>  
>  	sprintf(buf, "virtfn%u", id);
>  	sysfs_remove_link(&dev->dev.kobj, buf);
> -	sysfs_remove_link(&virtfn->dev.kobj, "physfn");
> +	/*
> +	 * pci_stop_dev() could be called for this virtfn before already
> +	 *  so directory for the virtfn is removed before.
> +	 */
> +	if (virtfn->dev.kobj.sd)
> +		sysfs_remove_link(&virtfn->dev.kobj, "physfn");
>  
>  	mutex_lock(&iov->dev->sriov->lock);
>  	pci_remove_bus_device(virtfn);

Cleaned up the commit & comment text a bit and put into -next.  Please
check it out.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 3/9] PCI: Disable cardbus bridge MEM1 pref CTL

[Yinghai Lu]

Some BIOS enable both pref for MEM0 and MEM1.

but we assume MEM1 is non-pref...

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 090217a..d5897c3 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	if (realloc_head)
 		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
 
+	/* MEM1 must not be pref mmio */
+	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
+	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
+		ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
+		pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
+		pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
+	}
+
 	/*
 	 * Check whether prefetchable memory is supported
 	 * by this bridge.
-- 
1.7.7

Re: [PATCH 3/9] PCI: Disable cardbus bridge MEM1 pref CTL

[Jesse Barnes]

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

On Sat,  4 Feb 2012 22:55:02 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> Some BIOS enable both pref for MEM0 and MEM1.
> 
> but we assume MEM1 is non-pref...
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/setup-bus.c |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 090217a..d5897c3 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>  	if (realloc_head)
>  		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>  
> +	/* MEM1 must not be pref mmio */
> +	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
> +	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
> +		ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
> +		pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
> +		pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
> +	}
> +
>  	/*
>  	 * Check whether prefetchable memory is supported
>  	 * by this bridge.

Does this actually fix any bugs?  Dominik, have you tested it?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 3/9] PCI: Disable cardbus bridge MEM1 pref CTL

[Yinghai Lu]

On Fri, Feb 10, 2012 at 12:46 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Sat,  4 Feb 2012 22:55:02 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
>
>> Some BIOS enable both pref for MEM0 and MEM1.
>>
>> but we assume MEM1 is non-pref...
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/setup-bus.c |    8 ++++++++
>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 090217a..d5897c3 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>>       if (realloc_head)
>>               add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>>
>> +     /* MEM1 must not be pref mmio */
>> +     pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>> +     if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
>> +             ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
>> +             pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
>> +             pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>> +     }
>> +
>>       /*
>>        * Check whether prefetchable memory is supported
>>        * by this bridge.
>
> Does this actually fix any bugs?

at least, one of laptop have this problem.

> Dominik, have you tested it?

he tested my for-pci3 and for-pci-busn-alloc branches, both branches
have this patch.

and it does not cause problem to him.

Yinghai

Re: [PATCH 3/9] PCI: Disable cardbus bridge MEM1 pref CTL

[Bjorn Helgaas]

On Fri, Feb 10, 2012 at 12:54 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Feb 10, 2012 at 12:46 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> On Sat,  4 Feb 2012 22:55:02 -0800
>> Yinghai Lu <yinghai@kernel.org> wrote:
>>
>>> Some BIOS enable both pref for MEM0 and MEM1.
>>>
>>> but we assume MEM1 is non-pref...
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>  drivers/pci/setup-bus.c |    8 ++++++++
>>>  1 files changed, 8 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>>> index 090217a..d5897c3 100644
>>> --- a/drivers/pci/setup-bus.c
>>> +++ b/drivers/pci/setup-bus.c
>>> @@ -914,6 +914,14 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>>>       if (realloc_head)
>>>               add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>>>
>>> +     /* MEM1 must not be pref mmio */
>>> +     pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>>> +     if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
>>> +             ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
>>> +             pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
>>> +             pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>>> +     }
>>> +
>>>       /*
>>>        * Check whether prefetchable memory is supported
>>>        * by this bridge.
>>
>> Does this actually fix any bugs?
>
> at least, one of laptop have this problem.

Rogério Brito's Clevo M5X0JE laptop seems to boot with both CardBus
mem apertures set to prefetchable.  I think there's evidence of this
in one of the video boot logs Rogério mentioned in this thread:
https://lkml.org/lkml/2012/1/6/343 because I noticed the same thing
and fixed a nearby bug in lspci.

I looked for it, but didn't find it.  I think it would be useful if
Yinghai could locate it, transcribe it into a bugzilla report, and
mention the bugzilla URL in this commit log.

I think Rogério did try the patch, but I don't know whether he
observed different behavior.  I think it *does* fix a bug; it's just
that his box has more serious problems that we haven't figured out
yet.

Bjorn

[PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Yinghai Lu]

We should not set the requested size to -2.

that will confuse the resource list sorting with align when SIZEALIGN is used.

Change to STARTALIGN and pass align from start.

We are safe to do that just as we do that regular pci bridge.

In the long run, we should just treat cardbus like regular pci bridge.

Also fix when realloc is not passed, should keep the requested size.

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index d5897c3..3b3932a 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 {
 	struct pci_dev *bridge = bus->self;
 	struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
+	resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
 	u16 ctrl;
 
 	/*
 	 * Reserve some resources for CardBus.  We reserve
 	 * a fixed amount of bus space for CardBus bridges.
 	 */
-	b_res[0].start = 0;
-	b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
-	if (realloc_head)
-		add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size, 0 /* dont care */);
+	b_res[0].start = pci_cardbus_io_size;
+	b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
+	b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
+	if (realloc_head) {
+		b_res[0].end -= pci_cardbus_io_size;
+		add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
+				pci_cardbus_io_size);
+	}
 
-	b_res[1].start = 0;
-	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
-	if (realloc_head)
-		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
+	b_res[1].start = pci_cardbus_io_size;
+	b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
+	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
+	if (realloc_head) {
+		b_res[1].end -= pci_cardbus_io_size;
+		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
+				 pci_cardbus_io_size);
+	}
 
 	/* MEM1 must not be pref mmio */
 	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
@@ -939,28 +948,28 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	 * twice the size.
 	 */
 	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
-		b_res[2].start = 0;
-		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
-		if (realloc_head)
-			add_to_list(realloc_head, bridge, b_res+2, pci_cardbus_mem_size, 0 /* dont care */);
+		b_res[2].start = pci_cardbus_mem_size;
+		b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
+		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
+				  IORESOURCE_STARTALIGN;
+		if (realloc_head) {
+			b_res[2].end -= pci_cardbus_mem_size;
+			add_to_list(realloc_head, bridge, b_res+2,
+				 pci_cardbus_mem_size, pci_cardbus_mem_size);
+		}
 
-		b_res[3].start = 0;
-		b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
-		if (realloc_head)
-			add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
-	} else {
-		b_res[3].start = 0;
-		b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
-		if (realloc_head)
-			add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
+		/* reduce that to half */
+		b_res_3_size = pci_cardbus_mem_size;
 	}
 
-	/* set the size of the resource to zero, so that the resource does not
-	 * get assigned during required-resource allocation cycle but gets assigned
-	 * during the optional-resource allocation cycle.
- 	 */
-	b_res[0].start = b_res[1].start = b_res[2].start = b_res[3].start = 1;
-	b_res[0].end = b_res[1].end = b_res[2].end = b_res[3].end = 0;
+	b_res[3].start = pci_cardbus_mem_size;
+	b_res[3].end = b_res[3].start + b_res_3_size - 1;
+	b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
+	if (realloc_head) {
+		b_res[3].end -= b_res_3_size;
+		add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
+				 pci_cardbus_mem_size);
+	}
 }
 
 void __ref __pci_bus_size_bridges(struct pci_bus *bus,
-- 
1.7.7

Re: [PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Ram Pai]

On Sat, Feb 04, 2012 at 10:55:03PM -0800, Yinghai Lu wrote:
> We should not set the requested size to -2.
> 
> that will confuse the resource list sorting with align when SIZEALIGN is used.
> 
> Change to STARTALIGN and pass align from start.
> 
> We are safe to do that just as we do that regular pci bridge.
> 
> In the long run, we should just treat cardbus like regular pci bridge.
> 
> Also fix when realloc is not passed, should keep the requested size.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/setup-bus.c |   63 ++++++++++++++++++++++++++--------------------
>  1 files changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index d5897c3..3b3932a 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>  {
>  	struct pci_dev *bridge = bus->self;
>  	struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
> +	resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
>  	u16 ctrl;
> 
>  	/*
>  	 * Reserve some resources for CardBus.  We reserve
>  	 * a fixed amount of bus space for CardBus bridges.
>  	 */
> -	b_res[0].start = 0;
> -	b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
> -	if (realloc_head)
> -		add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size, 0 /* dont care */);
> +	b_res[0].start = pci_cardbus_io_size;
> +	b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
> +	b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
> +	if (realloc_head) {
> +		b_res[0].end -= pci_cardbus_io_size;
> +		add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
> +				pci_cardbus_io_size);
> +	}
> 
> -	b_res[1].start = 0;
> -	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
> -	if (realloc_head)
> -		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
> +	b_res[1].start = pci_cardbus_io_size;
> +	b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
> +	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
> +	if (realloc_head) {
> +		b_res[1].end -= pci_cardbus_io_size;
> +		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
> +				 pci_cardbus_io_size);
> +	}
> 
>  	/* MEM1 must not be pref mmio */
>  	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
> @@ -939,28 +948,28 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>  	 * twice the size.
>  	 */
>  	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
> -		b_res[2].start = 0;
> -		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
> -		if (realloc_head)
> -			add_to_list(realloc_head, bridge, b_res+2, pci_cardbus_mem_size, 0 /* dont care */);
> +		b_res[2].start = pci_cardbus_mem_size;
> +		b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
> +		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
> +				  IORESOURCE_STARTALIGN;
> +		if (realloc_head) {
> +			b_res[2].end -= pci_cardbus_mem_size;
> +			add_to_list(realloc_head, bridge, b_res+2,
> +				 pci_cardbus_mem_size, pci_cardbus_mem_size);
> +		}
> 
> -		b_res[3].start = 0;
> -		b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> -		if (realloc_head)
> -			add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);

The b_res_3_size has to be reduced to half here.
Otherwise it will try allocate 2*pci_cardbus_mem_size to the BAR3..

		b_res_3_size = pci_cardbus_mem_size;


> -	} else {
> -		b_res[3].start = 0;
> -		b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> -		if (realloc_head)
> -			add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
> +		/* reduce that to half */
> +		b_res_3_size = pci_cardbus_mem_size;

b_res3_3_size should not be updated here, since BAR3 has to be allocated 2*pci_cardbus_mem_size
resource.

RP


>  	}
> 
> -	/* set the size of the resource to zero, so that the resource does not
> -	 * get assigned during required-resource allocation cycle but gets assigned
> -	 * during the optional-resource allocation cycle.
> - 	 */
> -	b_res[0].start = b_res[1].start = b_res[2].start = b_res[3].start = 1;
> -	b_res[0].end = b_res[1].end = b_res[2].end = b_res[3].end = 0;
> +	b_res[3].start = pci_cardbus_mem_size;
> +	b_res[3].end = b_res[3].start + b_res_3_size - 1;
> +	b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
> +	if (realloc_head) {
> +		b_res[3].end -= b_res_3_size;
> +		add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
> +				 pci_cardbus_mem_size);
> +	}
>  }
> 
>  void __ref __pci_bus_size_bridges(struct pci_bus *bus,
> -- 
> 1.7.7

Re: [PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Yinghai Lu]

On Tue, Feb 7, 2012 at 8:35 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> On Sat, Feb 04, 2012 at 10:55:03PM -0800, Yinghai Lu wrote:
>> We should not set the requested size to -2.
>>
>> that will confuse the resource list sorting with align when SIZEALIGN is used.
>>
>> Change to STARTALIGN and pass align from start.
>>
>> We are safe to do that just as we do that regular pci bridge.
>>
>> In the long run, we should just treat cardbus like regular pci bridge.
>>
>> Also fix when realloc is not passed, should keep the requested size.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/setup-bus.c |   63 ++++++++++++++++++++++++++--------------------
>>  1 files changed, 36 insertions(+), 27 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index d5897c3..3b3932a 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>>  {
>>       struct pci_dev *bridge = bus->self;
>>       struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
>> +     resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
>>       u16 ctrl;
>>
>>       /*
>>        * Reserve some resources for CardBus.  We reserve
>>        * a fixed amount of bus space for CardBus bridges.
>>        */
>> -     b_res[0].start = 0;
>> -     b_res[0].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
>> -     if (realloc_head)
>> -             add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size, 0 /* dont care */);
>> +     b_res[0].start = pci_cardbus_io_size;
>> +     b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
>> +     b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
>> +     if (realloc_head) {
>> +             b_res[0].end -= pci_cardbus_io_size;
>> +             add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
>> +                             pci_cardbus_io_size);
>> +     }
>>
>> -     b_res[1].start = 0;
>> -     b_res[1].flags |= IORESOURCE_IO | IORESOURCE_SIZEALIGN;
>> -     if (realloc_head)
>> -             add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size, 0 /* dont care */);
>> +     b_res[1].start = pci_cardbus_io_size;
>> +     b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
>> +     b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
>> +     if (realloc_head) {
>> +             b_res[1].end -= pci_cardbus_io_size;
>> +             add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
>> +                              pci_cardbus_io_size);
>> +     }
>>
>>       /* MEM1 must not be pref mmio */
>>       pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
>> @@ -939,28 +948,28 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
>>        * twice the size.
>>        */
>>       if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
>> -             b_res[2].start = 0;
>> -             b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH | IORESOURCE_SIZEALIGN;
>> -             if (realloc_head)
>> -                     add_to_list(realloc_head, bridge, b_res+2, pci_cardbus_mem_size, 0 /* dont care */);
>> +             b_res[2].start = pci_cardbus_mem_size;
>> +             b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
>> +             b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
>> +                               IORESOURCE_STARTALIGN;
>> +             if (realloc_head) {
>> +                     b_res[2].end -= pci_cardbus_mem_size;
>> +                     add_to_list(realloc_head, bridge, b_res+2,
>> +                              pci_cardbus_mem_size, pci_cardbus_mem_size);
>> +             }
>>
>> -             b_res[3].start = 0;
>> -             b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
>> -             if (realloc_head)
>> -                     add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
>
> The b_res_3_size has to be reduced to half here.
> Otherwise it will try allocate 2*pci_cardbus_mem_size to the BAR3..
>
>                b_res_3_size = pci_cardbus_mem_size;
>
>
>> -     } else {
>> -             b_res[3].start = 0;
>> -             b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
>> -             if (realloc_head)
>> -                     add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
>> +             /* reduce that to half */
>> +             b_res_3_size = pci_cardbus_mem_size;
>
> b_res3_3_size should not be updated here, since BAR3 has to be allocated 2*pci_cardbus_mem_size
> resource.
>

looks like you did not read the patch correctly

that else {} get removed already in this patch

please check segment after patch...

static void pci_bus_size_cardbus(struct pci_bus *bus,
			struct list_head *realloc_head)
{
	struct pci_dev *bridge = bus->self;
	struct resource *b_res = &bridge->resource[PCI_BRIDGE_RESOURCES];
	resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
	u16 ctrl;

	/*
	 * Reserve some resources for CardBus.  We reserve
	 * a fixed amount of bus space for CardBus bridges.
	 */
	b_res[0].start = pci_cardbus_io_size;
	b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
	b_res[0].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
	if (realloc_head) {
		b_res[0].end -= pci_cardbus_io_size;
		add_to_list(realloc_head, bridge, b_res, pci_cardbus_io_size,
				pci_cardbus_io_size);
	}

	b_res[1].start = pci_cardbus_io_size;
	b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
	if (realloc_head) {
		b_res[1].end -= pci_cardbus_io_size;
		add_to_list(realloc_head, bridge, b_res+1, pci_cardbus_io_size,
				 pci_cardbus_io_size);
	}

	/* MEM1 must not be pref mmio */
	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
		ctrl &= ~PCI_CB_BRIDGE_CTL_PREFETCH_MEM1;
		pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
		pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
	}

	/*
	 * Check whether prefetchable memory is supported
	 * by this bridge.
	 */
	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
	if (!(ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0)) {
		ctrl |= PCI_CB_BRIDGE_CTL_PREFETCH_MEM0;
		pci_write_config_word(bridge, PCI_CB_BRIDGE_CONTROL, ctrl);
		pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
	}

	/*
	 * If we have prefetchable memory support, allocate
	 * two regions.  Otherwise, allocate one region of
	 * twice the size.
	 */
	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
		b_res[2].start = pci_cardbus_mem_size;
		b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
				  IORESOURCE_STARTALIGN;
		if (realloc_head) {
			b_res[2].end -= pci_cardbus_mem_size;
			add_to_list(realloc_head, bridge, b_res+2,
				 pci_cardbus_mem_size, pci_cardbus_mem_size);
		}

		/* reduce that to half */
		b_res_3_size = pci_cardbus_mem_size;
	}

	b_res[3].start = pci_cardbus_mem_size;
	b_res[3].end = b_res[3].start + b_res_3_size - 1;
	b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
	if (realloc_head) {
		b_res[3].end -= b_res_3_size;
		add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
				 pci_cardbus_mem_size);
	}
}

Re: [PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Ram Pai]

On Tue, Feb 07, 2012 at 08:48:36PM -0800, Yinghai Lu wrote:
> On Tue, Feb 7, 2012 at 8:35 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> > On Sat, Feb 04, 2012 at 10:55:03PM -0800, Yinghai Lu wrote:
> >> We should not set the requested size to -2.
> >>
> >> that will confuse the resource list sorting with align when SIZEALIGN is used.
> >>
> >> Change to STARTALIGN and pass align from start.
> >>
> >> We are safe to do that just as we do that regular pci bridge.
> >>
> >> In the long run, we should just treat cardbus like regular pci bridge.
> >>
> >> Also fix when realloc is not passed, should keep the requested size.
> >>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >> ---
> >>  drivers/pci/setup-bus.c |   63 ++++++++++++++++++++++++++--------------------
> >>  1 files changed, 36 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> >> index d5897c3..3b3932a 100644
> >> --- a/drivers/pci/setup-bus.c
> >> +++ b/drivers/pci/setup-bus.c
> >> @@ -898,21 +898,30 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,

..snip

> >> -             b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> >> -             if (realloc_head)
> >> -                     add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size, 0 /* dont care */);
> >
> > The b_res_3_size has to be reduced to half here.
> > Otherwise it will try allocate 2*pci_cardbus_mem_size to the BAR3..
> >
> >                b_res_3_size = pci_cardbus_mem_size;
> >
> >
> >> -     } else {
> >> -             b_res[3].start = 0;
> >> -             b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_SIZEALIGN;
> >> -             if (realloc_head)
> >> -                     add_to_list(realloc_head, bridge, b_res+3, pci_cardbus_mem_size * 2, 0 /* dont care */);
> >> +             /* reduce that to half */
> >> +             b_res_3_size = pci_cardbus_mem_size;
> >
> > b_res3_3_size should not be updated here, since BAR3 has to be allocated 2*pci_cardbus_mem_size
> > resource.
> >
> 
> looks like you did not read the patch correctly
> 
> that else {} get removed already in this patch
> 
> please check segment after patch...
> 	/*

..snip..
> 	 * If we have prefetchable memory support, allocate
> 	 * two regions.  Otherwise, allocate one region of
> 	 * twice the size.
> 	 */
> 	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
> 		b_res[2].start = pci_cardbus_mem_size;
> 		b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
> 		b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
> 				  IORESOURCE_STARTALIGN;
> 		if (realloc_head) {
> 			b_res[2].end -= pci_cardbus_mem_size;
> 			add_to_list(realloc_head, bridge, b_res+2,
> 				 pci_cardbus_mem_size, pci_cardbus_mem_size);
> 		}
> 
> 		/* reduce that to half */
> 		b_res_3_size = pci_cardbus_mem_size;
> 	}

ACK. Ok. got it. This looks correct.
RP

Re: [PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Yinghai Lu]

On Tue, Feb 7, 2012 at 9:01 PM, Ram Pai <linuxram@us.ibm.com> wrote:
>
> ..snip..
>>        * If we have prefetchable memory support, allocate
>>        * two regions.  Otherwise, allocate one region of
>>        * twice the size.
>>        */
>>       if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
>>               b_res[2].start = pci_cardbus_mem_size;
>>               b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
>>               b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
>>                                 IORESOURCE_STARTALIGN;
>>               if (realloc_head) {
>>                       b_res[2].end -= pci_cardbus_mem_size;
>>                       add_to_list(realloc_head, bridge, b_res+2,
>>                                pci_cardbus_mem_size, pci_cardbus_mem_size);
>>               }
>>
>>               /* reduce that to half */
>>               b_res_3_size = pci_cardbus_mem_size;
>>       }
>
> ACK. Ok. got it. This looks correct.

Good, So we have

Acked-by: Ram Pai <linuxram@us.ibm.com>
Tested-by: Dominik Brodowski <linux@dominikbrodowski.net>

Hope Jesse will not miss to add them

Thanks

Yinghai

Re: [PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Jesse Barnes]

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

On Tue, 7 Feb 2012 22:11:22 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> On Tue, Feb 7, 2012 at 9:01 PM, Ram Pai <linuxram@us.ibm.com> wrote:
> >
> > ..snip..
> >>        * If we have prefetchable memory support, allocate
> >>        * two regions.  Otherwise, allocate one region of
> >>        * twice the size.
> >>        */
> >>       if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
> >>               b_res[2].start = pci_cardbus_mem_size;
> >>               b_res[2].end = b_res[2].start + pci_cardbus_mem_size - 1;
> >>               b_res[2].flags |= IORESOURCE_MEM | IORESOURCE_PREFETCH |
> >>                                 IORESOURCE_STARTALIGN;
> >>               if (realloc_head) {
> >>                       b_res[2].end -= pci_cardbus_mem_size;
> >>                       add_to_list(realloc_head, bridge, b_res+2,
> >>                                pci_cardbus_mem_size, pci_cardbus_mem_size);
> >>               }
> >>
> >>               /* reduce that to half */
> >>               b_res_3_size = pci_cardbus_mem_size;
> >>       }
> >
> > ACK. Ok. got it. This looks correct.
> 
> Good, So we have
> 
> Acked-by: Ram Pai <linuxram@us.ibm.com>
> Tested-by: Dominik Brodowski <linux@dominikbrodowski.net>
> 
> Hope Jesse will not miss to add them

Yeah looks fine.  I'd like to see an ack from Dominik on 5/9 as well
though.  Can you include the acks and tested-bys when you re-post 3-9
with bug references and any other comments included?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 4/9] PCI: Fix cardbus bridge resources as optional size handling

[Yinghai Lu]

On Fri, Feb 10, 2012 at 12:52 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

>> > ACK. Ok. got it. This looks correct.
>>
>> Good, So we have
>>
>> Acked-by: Ram Pai <linuxram@us.ibm.com>
>> Tested-by: Dominik Brodowski <linux@dominikbrodowski.net>
>>
>> Hope Jesse will not miss to add them
>
> Yeah looks fine.  I'd like to see an ack from Dominik on 5/9 as well
> though.  Can you include the acks and tested-bys when you re-post 3-9
> with bug references and any other comments included?

sure. Will send them updated acks and etc.

Yinghai

[PATCH 5/9] PCI: Skip reset cardbus assigned resource during pci bus rescan

[Yinghai Lu]

otherwise when rescan is used for cardbus, assigned resource will get
cleared.

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 3b3932a..2991a89 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -901,6 +901,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	resource_size_t b_res_3_size = pci_cardbus_mem_size * 2;
 	u16 ctrl;
 
+	if (b_res[0].parent)
+		goto handle_b_res_1;
 	/*
 	 * Reserve some resources for CardBus.  We reserve
 	 * a fixed amount of bus space for CardBus bridges.
@@ -914,6 +916,9 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 				pci_cardbus_io_size);
 	}
 
+handle_b_res_1:
+	if (b_res[1].parent)
+		goto handle_b_res_2;
 	b_res[1].start = pci_cardbus_io_size;
 	b_res[1].end = b_res[1].start + pci_cardbus_io_size - 1;
 	b_res[1].flags |= IORESOURCE_IO | IORESOURCE_STARTALIGN;
@@ -923,6 +928,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 				 pci_cardbus_io_size);
 	}
 
+handle_b_res_2:
 	/* MEM1 must not be pref mmio */
 	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
 	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM1) {
@@ -942,6 +948,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 		pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
 	}
 
+	if (b_res[2].parent)
+		goto handle_b_res_3;
 	/*
 	 * If we have prefetchable memory support, allocate
 	 * two regions.  Otherwise, allocate one region of
@@ -962,6 +970,9 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 		b_res_3_size = pci_cardbus_mem_size;
 	}
 
+handle_b_res_3:
+	if (b_res[3].parent)
+		goto handle_done;
 	b_res[3].start = pci_cardbus_mem_size;
 	b_res[3].end = b_res[3].start + b_res_3_size - 1;
 	b_res[3].flags |= IORESOURCE_MEM | IORESOURCE_STARTALIGN;
@@ -970,6 +981,9 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 		add_to_list(realloc_head, bridge, b_res+3, b_res_3_size,
 				 pci_cardbus_mem_size);
 	}
+
+handle_done:
+	;
 }
 
 void __ref __pci_bus_size_bridges(struct pci_bus *bus,
-- 
1.7.7

[PATCH 6/9] PCI: Retry on type IORESOURCE_IO allocation.

[Yinghai Lu]

During reenabling pci reallocating for pci bridge by clean the small size in
bridge and assign with requested + optional size for first several try,
Ram mention could have problem with one case
	https://bugzilla.kernel.org/show_bug.cgi?id=15960

After checking the booting log in
	https://lkml.org/lkml/2010/4/19/44
	[regression, bisected] Xonar DX invalid PCI I/O range since 977d17bb174

We should not stop too early for io ports.
	Apr 19 10:19:38 [kernel] pci 0000:04:00.0: BAR 7: can't assign io (size 0x4000)
	Apr 19 10:19:38 [kernel] pci 0000:05:01.0: BAR 8: assigned [mem 0x80400000-0x805fffff]
	Apr 19 10:19:38 [kernel] pci 0000:05:01.0: BAR 7: can't assign io (size 0x2000)
	Apr 19 10:19:38 [kernel] pci 0000:05:02.0: BAR 7: can't assign io (size 0x1000)
	Apr 19 10:19:38 [kernel] pci 0000:05:03.0: BAR 7: can't assign io (size 0x1000)
	Apr 19 10:19:38 [kernel] pci 0000:08:00.0: BAR 7: can't assign io (size 0x1000)
	Apr 19 10:19:38 [kernel] pci 0000:09:04.0: BAR 0: can't assign io (size 0x100)
and clear 00:1c.0 to retry again.

The patch remove the IORESOUCE_IO checking, and try one more time.
and we will have chance to get allocation for 00:1c.0 io port range because
from 0x4000 to 0x8000 could be used.

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 2991a89..162edfb 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1292,7 +1292,6 @@ pci_assign_unassigned_resources(void)
 	struct pci_dev_resource *fail_res;
 	unsigned long type_mask = IORESOURCE_IO | IORESOURCE_MEM |
 				  IORESOURCE_PREFETCH;
-	unsigned long failed_type;
 	int pci_try_num = 1;
 
 	/* don't realloc if asked to do so */
@@ -1327,16 +1326,7 @@ again:
 	if (list_empty(&fail_head))
 		goto enable_and_dump;
 
-	failed_type = 0;
-	list_for_each_entry(fail_res, &fail_head, list)
-		failed_type |= fail_res->flags;
-
-	/*
-	 * io port are tight, don't try extra
-	 * or if reach the limit, don't want to try more
-	 */
-	failed_type &= type_mask;
-	if ((failed_type == IORESOURCE_IO) || (tried_times >= pci_try_num)) {
+	if (tried_times >= pci_try_num) {
 		free_list(&fail_head);
 		goto enable_and_dump;
 	}
-- 
1.7.7

[PATCH 7/9] PCI: Make pci bridge reallocating enabled/disabled

[Yinghai Lu]

Let the user could enable and disable with pci=realloc=on or pci=realloc=off

Also
1. move variable and functions near the place they are used.
2. change macro to function
3. change related functions and variable to static and _init
4. update parameter description accordingly.

-v2: still honor pci=realloc, and treat it as pci=realloc=on
     also use enum instead of ...

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 Documentation/kernel-parameters.txt |    8 ++++++--
 drivers/pci/pci.c                   |    4 +++-
 drivers/pci/pci.h                   |    2 +-
 drivers/pci/setup-bus.c             |   33 ++++++++++++++++++++++++++-------
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 033d4e6..e11e5dd 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2109,8 +2109,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 				the default.
 				off: Turn ECRC off
 				on: Turn ECRC on.
-		realloc		reallocate PCI resources if allocations done by BIOS
-				are erroneous.
+		realloc=	Enable/disable reallocating PCI bridge resources
+				if allocations done by BIOS are too small to fit
+				resources required by children devices.
+				off: Turn realloc off
+				on: Turn realloc on
+		realloc		same as realloc=on
 
 	pcie_aspm=	[PCIE] Forcibly enable or disable PCIe Active State Power
 			Management.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 9c89447..09cb1f2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3744,8 +3744,10 @@ static int __init pci_setup(char *str)
 				pci_no_msi();
 			} else if (!strcmp(str, "noaer")) {
 				pci_no_aer();
+			} else if (!strncmp(str, "realloc=", 8)) {
+				pci_realloc_get_opt(str + 8);
 			} else if (!strncmp(str, "realloc", 7)) {
-				pci_realloc();
+				pci_realloc_get_opt("on");
 			} else if (!strcmp(str, "nodomains")) {
 				pci_no_domains();
 			} else if (!strncmp(str, "cbiosize=", 9)) {
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index aaf7ff8..f862a5a 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -148,7 +148,7 @@ static inline void pci_no_msi(void) { }
 static inline void pci_msi_init_pci_dev(struct pci_dev *dev) { }
 #endif
 
-extern void pci_realloc(void);
+void pci_realloc_get_opt(char *);
 
 static inline int pci_no_d1d2(struct pci_dev *dev)
 {
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 162edfb..22454c5 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -48,13 +48,6 @@ static void free_list(struct list_head *head)
 	}
 }
 
-int pci_realloc_enable = 0;
-#define pci_realloc_enabled() pci_realloc_enable
-void pci_realloc(void)
-{
-	pci_realloc_enable = 1;
-}
-
 /**
  * add_to_list() - add a new resource tracker to the list
  * @head:	Head of the list
@@ -1273,6 +1266,32 @@ static int __init pci_get_max_depth(void)
 	return depth;
 }
 
+/*
+ * -1: undefined, will auto detect later
+ *  0: disabled by user
+ *  1: enabled by user
+ *  2: enabled by auto detect
+ */
+enum enable_type {
+	enable_not_set = -1,
+	enable_no_user,
+	enable_no_detected,
+	enable_yes_user,
+	enable_yes_detected,
+};
+
+static enum enable_type pci_realloc_enable __initdata = enable_not_set;
+void __init pci_realloc_get_opt(char *str)
+{
+	if (!strncmp(str, "off", 3))
+		pci_realloc_enable = enable_no_user;
+	else if (!strncmp(str, "on", 2))
+		pci_realloc_enable = enable_yes_user;
+}
+static bool __init pci_realloc_enabled(void)
+{
+	return pci_realloc_enable >= enable_yes_user;
+}
 
 /*
  * first try will not touch pci bridge res
-- 
1.7.7

Re: [PATCH 7/9] PCI: Make pci bridge reallocating enabled/disabled

[Jesse Barnes]

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

On Sat,  4 Feb 2012 22:55:06 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> Let the user could enable and disable with pci=realloc=on or pci=realloc=off
> 
> Also
> 1. move variable and functions near the place they are used.
> 2. change macro to function
> 3. change related functions and variable to static and _init
> 4. update parameter description accordingly.
> 
> -v2: still honor pci=realloc, and treat it as pci=realloc=on
>      also use enum instead of ...
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  Documentation/kernel-parameters.txt |    8 ++++++--
>  drivers/pci/pci.c                   |    4 +++-
>  drivers/pci/pci.h                   |    2 +-
>  drivers/pci/setup-bus.c             |   33 ++++++++++++++++++++++++++-------
>  4 files changed, 36 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index 033d4e6..e11e5dd 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2109,8 +2109,12 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
>  				the default.
>  				off: Turn ECRC off
>  				on: Turn ECRC on.
> -		realloc		reallocate PCI resources if allocations done by BIOS
> -				are erroneous.
> +		realloc=	Enable/disable reallocating PCI bridge resources
> +				if allocations done by BIOS are too small to fit
> +				resources required by children devices.
> +				off: Turn realloc off
> +				on: Turn realloc on
> +		realloc		same as realloc=on

"too small to accommodate resources required by all child devices"

Looks good otherwise.

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 8/9] PCI: print out suggestion about using pci=realloc

[Yinghai Lu]

let user know they could try if pci=realloc could help.

Suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/setup-bus.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 22454c5..9526038 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1346,6 +1346,9 @@ again:
 		goto enable_and_dump;
 
 	if (tried_times >= pci_try_num) {
+		if (pci_realloc_enable == enable_not_set)
+			printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
+
 		free_list(&fail_head);
 		goto enable_and_dump;
 	}
-- 
1.7.7

Re: [PATCH 8/9] PCI: print out suggestion about using pci=realloc

[Jesse Barnes]

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

On Sat,  4 Feb 2012 22:55:07 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> let user know they could try if pci=realloc could help.
> 
> Suggested-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/setup-bus.c |    3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 22454c5..9526038 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1346,6 +1346,9 @@ again:
>  		goto enable_and_dump;
>  
>  	if (tried_times >= pci_try_num) {
> +		if (pci_realloc_enable == enable_not_set)
> +			printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
> +
>  		free_list(&fail_head);
>  		goto enable_and_dump;
>  	}

"Some PCI device resources are unassigned, try booting with pci=realloc"

That said, I wonder if this will cause more problems than it solves.
In some cases the BIOS may not allocate all resources, but this will be
harmless since the user won't actually use the device, or the driver
for the device won't even use the BARs in question anyway (as happens a
lot for IO regions).

-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH 9/9] PCI: only enable pci realloc when SRIOV bar is not assigned

[Yinghai Lu]

If bios does not assign those BAR or wrong address, then kernel will
try to do pci realloc.

in that case, user still can use pci=realloc=off to override it.

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 9526038..520f256 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1293,6 +1293,31 @@ static bool __init pci_realloc_enabled(void)
 	return pci_realloc_enable >= enable_yes_user;
 }
 
+static void __init pci_realloc_detect(void)
+{
+	struct pci_dev *dev = NULL;
+
+	if (pci_realloc_enable != enable_not_set)
+		return;
+
+#ifdef CONFIG_PCI_IOV
+	for_each_pci_dev(dev) {
+		int i;
+
+		for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
+			struct resource *r = &dev->resource[i];
+
+			/* Not assigned, or rejected by kernel */
+			if (r->flags && !r->start) {
+				pci_realloc_enable = enable_yes_detected;
+
+				return;
+			}
+		}
+	}
+#endif
+}
+
 /*
  * first try will not touch pci bridge res
  * second  and later try will clear small leaf bridge res
@@ -1314,6 +1339,7 @@ pci_assign_unassigned_resources(void)
 	int pci_try_num = 1;
 
 	/* don't realloc if asked to do so */
+	pci_realloc_detect();
 	if (pci_realloc_enabled()) {
 		int max_depth = pci_get_max_depth();
 
@@ -1348,6 +1374,8 @@ again:
 	if (tried_times >= pci_try_num) {
 		if (pci_realloc_enable == enable_not_set)
 			printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
+		else if (pci_realloc_enable == enable_yes_detected)
+			printk(KERN_INFO "Automatically enabled pci realloc, if you have problem, please try to boot with pci=realloc=off\n");
 
 		free_list(&fail_head);
 		goto enable_and_dump;
-- 
1.7.7

Re: [PATCH 9/9] PCI: only enable pci realloc when SRIOV bar is not assigned

[Jesse Barnes]

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

On Sat,  4 Feb 2012 22:55:08 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> If bios does not assign those BAR or wrong address, then kernel will
> try to do pci realloc.
> 
> in that case, user still can use pci=realloc=off to override it.
> 
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/setup-bus.c |   28 ++++++++++++++++++++++++++++
>  1 files changed, 28 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 9526038..520f256 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1293,6 +1293,31 @@ static bool __init pci_realloc_enabled(void)
>  	return pci_realloc_enable >= enable_yes_user;
>  }
>  
> +static void __init pci_realloc_detect(void)
> +{
> +	struct pci_dev *dev = NULL;
> +
> +	if (pci_realloc_enable != enable_not_set)
> +		return;
> +
> +#ifdef CONFIG_PCI_IOV
> +	for_each_pci_dev(dev) {
> +		int i;
> +
> +		for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
> +			struct resource *r = &dev->resource[i];
> +
> +			/* Not assigned, or rejected by kernel */
> +			if (r->flags && !r->start) {
> +				pci_realloc_enable = enable_yes_detected;
> +
> +				return;
> +			}
> +		}
> +	}
> +#endif
> +}
> +
>  /*
>   * first try will not touch pci bridge res
>   * second  and later try will clear small leaf bridge res
> @@ -1314,6 +1339,7 @@ pci_assign_unassigned_resources(void)
>  	int pci_try_num = 1;
>  
>  	/* don't realloc if asked to do so */
> +	pci_realloc_detect();
>  	if (pci_realloc_enabled()) {
>  		int max_depth = pci_get_max_depth();
>  
> @@ -1348,6 +1374,8 @@ again:
>  	if (tried_times >= pci_try_num) {
>  		if (pci_realloc_enable == enable_not_set)
>  			printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
> +		else if (pci_realloc_enable == enable_yes_detected)
> +			printk(KERN_INFO "Automatically enabled pci realloc, if you have problem, please try to boot with pci=realloc=off\n");
>  
>  		free_list(&fail_head);
>  		goto enable_and_dump;

Not sure I like this one either.  Distros will want to enable IOV by
default, but in most configs users still won't use it, and many BIOSes
won't bother to allocate IOV space.  So defaulting to trying to do so
will likely cause a lot of unnecessary re-allocation.

So distro guidance would be appreciated here for the default behavior.
-- 
Jesse Barnes, Intel Open Source Technology Center

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 9/9] PCI: only enable pci realloc when SRIOV bar is not assigned

[Yinghai Lu]

On Fri, Feb 10, 2012 at 1:00 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Sat,  4 Feb 2012 22:55:08 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
>
>> If bios does not assign those BAR or wrong address, then kernel will
>> try to do pci realloc.
>>
>> in that case, user still can use pci=realloc=off to override it.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/setup-bus.c |   28 ++++++++++++++++++++++++++++
>>  1 files changed, 28 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
>> index 9526038..520f256 100644
>> --- a/drivers/pci/setup-bus.c
>> +++ b/drivers/pci/setup-bus.c
>> @@ -1293,6 +1293,31 @@ static bool __init pci_realloc_enabled(void)
>>       return pci_realloc_enable >= enable_yes_user;
>>  }
>>
>> +static void __init pci_realloc_detect(void)
>> +{
>> +     struct pci_dev *dev = NULL;
>> +
>> +     if (pci_realloc_enable != enable_not_set)
>> +             return;
>> +
>> +#ifdef CONFIG_PCI_IOV
>> +     for_each_pci_dev(dev) {
>> +             int i;
>> +
>> +             for (i = PCI_IOV_RESOURCES; i <= PCI_IOV_RESOURCE_END; i++) {
>> +                     struct resource *r = &dev->resource[i];
>> +
>> +                     /* Not assigned, or rejected by kernel */
>> +                     if (r->flags && !r->start) {
>> +                             pci_realloc_enable = enable_yes_detected;
>> +
>> +                             return;
>> +                     }
>> +             }
>> +     }
>> +#endif
>> +}
>> +
>>  /*
>>   * first try will not touch pci bridge res
>>   * second  and later try will clear small leaf bridge res
>> @@ -1314,6 +1339,7 @@ pci_assign_unassigned_resources(void)
>>       int pci_try_num = 1;
>>
>>       /* don't realloc if asked to do so */
>> +     pci_realloc_detect();
>>       if (pci_realloc_enabled()) {
>>               int max_depth = pci_get_max_depth();
>>
>> @@ -1348,6 +1374,8 @@ again:
>>       if (tried_times >= pci_try_num) {
>>               if (pci_realloc_enable == enable_not_set)
>>                       printk(KERN_INFO "Some pci devices resources are not assigned, please try to boot with pci=realloc\n");
>> +             else if (pci_realloc_enable == enable_yes_detected)
>> +                     printk(KERN_INFO "Automatically enabled pci realloc, if you have problem, please try to boot with pci=realloc=off\n");
>>
>>               free_list(&fail_head);
>>               goto enable_and_dump;
>
> Not sure I like this one either.  Distros will want to enable IOV by
> default, but in most configs users still won't use it, and many BIOSes
> won't bother to allocate IOV space.  So defaulting to trying to do so
> will likely cause a lot of unnecessary re-allocation.
>
> So distro guidance would be appreciated here for the default behavior.

Or do you prefer to:

if all SRIOV bar are not assigned, We will not enable SRIOV feature even?

Yinghai