[PATCH -v5 0/24] PCI: allocate pci bus num range for unassigned bridge busn + pci rescan cleanup

[PATCH -v5 0/24] PCI: allocate pci bus num range for unassigned bridge busn + pci rescan cleanup

[Yinghai Lu]

cdef5a1: PCI: clean up rescan_bus_bridge_resize
7b6deb4: PCI: make pci_rescan_bus_bridge_resize use pci_scan_bridge instead
876bd4a: PCI: Add pci_bus_add_single_device()
6e5f346: PCI, sysfs: create rescan_bridge under /sys/.../pci/devices/... for pci bridges
3c92113: PCI, sys: Use device_type and attr_groups with pci dev
e6d3c54: PCI, pciehp: Remove not needed bus number range checking
2306e31: PCI: Double checking setting for bus register and bus struct.
daddc90: pcmcia: remove workaround for fixing pci parent bus subordinate
6a88939: PCI: Seperate child bus scanning to two passes overall
60c7d94: PCI: kill pci_fixup_parent_subordinate_busnr()
e9e72be: PCI: Allocate bus range instead of use max blindly
f22af7f: PCI: Strict checking of valid range for bridge
8656ea6: PCI: Probe safe range that we can use for unassigned bridge.
4023fc3: PCI: Add pci_bus_extend/shrink_top()
a066c95: PCI, parisc: Register busn_res for root buses
d069fbb: PCI, powerpc: Register busn_res for root buses
f00e514: PCI, ia64: Register busn_res for root buses
1a203f1: PCI, x86: Register busn_res for root buses
8b1b01b: PCI: Add busn_res tracking in core
a9ccca7: PCI: add /proc/iobusn
ec1cdb3: PCI: Add busn_res operation functions
2a09fbd: Make %pR could handle bus resource with domain
9c83a59: PCI: add busn inline helper
b18cd6a: PCI: Add iobusn_resource

Set up iobusn_resource tree, and register bus number range to it.
Later when need to find bus range, will try to allocate from the tree

Need to test on arches other than x86. esp for ia64 and powerpc that support
  more than on peer root buses.

last five patches are rescan cleanup that make use busn alloc.

could be found at:
	git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git for-pci-busn-alloc

-v2: according to Jesse, split to more small patches.
-v3: address some request from Bjorn. like make use %pR for busn_res debug print
	out, and move the comment change with code change.
-v4: fixes the problem about rescan that Bjorn found.
-v5: add /proc/iobusn that is requested by Bjorn.
     remove old workaround from pciehp.
     add rescan_bridge for pci bridge in /sys

Thanks

Yinghai

 Documentation/ABI/testing/sysfs-bus-pci |   10 +
 arch/ia64/pci/pci.c                     |    2 +
 arch/powerpc/kernel/pci-common.c        |    7 +-
 arch/x86/include/asm/topology.h         |    3 +-
 arch/x86/pci/acpi.c                     |    8 +-
 arch/x86/pci/bus_numa.c                 |    8 +-
 arch/x86/pci/common.c                   |   11 +-
 drivers/parisc/dino.c                   |    2 +
 drivers/parisc/lba_pci.c                |    3 +
 drivers/pci/bus.c                       |   40 +++
 drivers/pci/hotplug/pciehp_pci.c        |   12 +-
 drivers/pci/pci-sysfs.c                 |   59 ++++-
 drivers/pci/pci.h                       |    1 +
 drivers/pci/probe.c                     |  460 +++++++++++++++++++++++++------
 drivers/pci/remove.c                    |    1 +
 drivers/pcmcia/yenta_socket.c           |   75 -----
 include/linux/ioport.h                  |   18 ++
 include/linux/pci.h                     |    9 +
 kernel/resource.c                       |   50 ++++
 lib/vsprintf.c                          |   29 ++-
 20 files changed, 617 insertions(+), 191 deletions(-)

[PATCH 01/24] PCI: Add iobusn_resource

[Yinghai Lu]

also add busn_res into struct pci_bus.

will use them to have bus number resource tree.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/ioport.h |    1 +
 include/linux/pci.h    |    1 +
 kernel/resource.c      |    8 ++++++++
 3 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index e885ba2..6fe9e19 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -136,6 +136,7 @@ struct resource {
 /* PC/ISA/whatever - the normal PC address spaces: IO and memory */
 extern struct resource ioport_resource;
 extern struct resource iomem_resource;
+extern struct resource iobusn_resource;
 
 extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
 extern int request_resource(struct resource *root, struct resource *new);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f8caaab..94ad468 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -419,6 +419,7 @@ struct pci_bus {
 	struct list_head slots;		/* list of slots on this bus */
 	struct resource *resource[PCI_BRIDGE_RESOURCE_NUM];
 	struct list_head resources;	/* address space routed to this bus */
+	struct resource busn_res;	/* track registered bus num range */
 
 	struct pci_ops	*ops;		/* configuration access functions */
 	void		*sysdata;	/* hook for sys-specific extension */
diff --git a/kernel/resource.c b/kernel/resource.c
index 7640b3a..53b42f0 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -38,6 +38,14 @@ struct resource iomem_resource = {
 };
 EXPORT_SYMBOL(iomem_resource);
 
+struct resource iobusn_resource = {
+	.name	= "PCI busn",
+	.start	= 0,
+	.end	= 0xffffff,
+	.flags	= IORESOURCE_BUS,
+};
+EXPORT_SYMBOL(iobusn_resource);
+
 /* constraints to be met while allocating resources */
 struct resource_constraint {
 	resource_size_t min, max, align;
-- 
1.7.7

Re: [PATCH 01/24] PCI: Add iobusn_resource

[Bjorn Helgaas]

On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> also add busn_res into struct pci_bus.
>
> will use them to have bus number resource tree.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> ---
>  include/linux/ioport.h |    1 +
>  include/linux/pci.h    |    1 +
>  kernel/resource.c      |    8 ++++++++
>  3 files changed, 10 insertions(+), 0 deletions(-)
>
> diff --git a/include/linux/ioport.h b/include/linux/ioport.h
> index e885ba2..6fe9e19 100644
> --- a/include/linux/ioport.h
> +++ b/include/linux/ioport.h
> @@ -136,6 +136,7 @@ struct resource {
>  /* PC/ISA/whatever - the normal PC address spaces: IO and memory */
>  extern struct resource ioport_resource;
>  extern struct resource iomem_resource;
> +extern struct resource iobusn_resource;
>
>  extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
>  extern int request_resource(struct resource *root, struct resource *new);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index f8caaab..94ad468 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -419,6 +419,7 @@ struct pci_bus {
>        struct list_head slots;         /* list of slots on this bus */
>        struct resource *resource[PCI_BRIDGE_RESOURCE_NUM];
>        struct list_head resources;     /* address space routed to this bus */
> +       struct resource busn_res;       /* track registered bus num range */
>
>        struct pci_ops  *ops;           /* configuration access functions */
>        void            *sysdata;       /* hook for sys-specific extension */
> diff --git a/kernel/resource.c b/kernel/resource.c
> index 7640b3a..53b42f0 100644
> --- a/kernel/resource.c
> +++ b/kernel/resource.c
> @@ -38,6 +38,14 @@ struct resource iomem_resource = {
>  };
>  EXPORT_SYMBOL(iomem_resource);
>
> +struct resource iobusn_resource = {
> +       .name   = "PCI busn",
> +       .start  = 0,
> +       .end    = 0xffffff,
> +       .flags  = IORESOURCE_BUS,
> +};

I'm not sure this should be global.  iomem_resource and
ioport_resource *are* really global, because they refer to processor
address space that is the same for everybody.  But PCI bus numbers are
specific to PCI.  Some machines don't have PCI at all, and there are
different bus architectures to which this doesn't apply.

The 0-0xffffff range is misleading because it includes both the domain
and the bus number, and it's meaningless to allocate ranges that cross
domain boundaries.  For example, [bus 0x0000f0-0x000120] includes bus
numbers from domain 0000 and domain 0001, which doesn't make any sense
because a bus can only be in one domain.

I think it would make more sense to keep this bus number resource in a
per-host bridge structure.  Then we wouldn't need to include the
domain number at all because the host bridge determines the domain.

> +EXPORT_SYMBOL(iobusn_resource);
> +
>  /* constraints to be met while allocating resources */
>  struct resource_constraint {
>        resource_size_t min, max, align;
> --
> 1.7.7
>

Re: [PATCH 01/24] PCI: Add iobusn_resource

[Yinghai Lu]

On Mon, Feb 6, 2012 at 10:48 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> +struct resource iobusn_resource = {
>> +       .name   = "PCI busn",
>> +       .start  = 0,
>> +       .end    = 0xffffff,
>> +       .flags  = IORESOURCE_BUS,
>> +};
>
> I'm not sure this should be global.  iomem_resource and
> ioport_resource *are* really global, because they refer to processor
> address space that is the same for everybody.  But PCI bus numbers are
> specific to PCI.  Some machines don't have PCI at all, and there are
> different bus architectures to which this doesn't apply.

that does not hurt them.

>
> The 0-0xffffff range is misleading because it includes both the domain
> and the bus number, and it's meaningless to allocate ranges that cross
> domain boundaries.  For example, [bus 0x0000f0-0x000120] includes bus
> numbers from domain 0000 and domain 0001, which doesn't make any sense
> because a bus can only be in one domain.

allocation code will make sure it will be cross the boundary for domain.

>
> I think it would make more sense to keep this bus number resource in a
> per-host bridge structure.  Then we wouldn't need to include the
> domain number at all because the host bridge determines the domain.

not sure. insert the all busn_res of all peer root buses into one
global iobusn_resource
looks more simple.

Yinghai

Re: [PATCH 01/24] PCI: Add iobusn_resource

[Jesse Barnes]

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

On Mon, 6 Feb 2012 10:55:14 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> On Mon, Feb 6, 2012 at 10:48 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> >> +struct resource iobusn_resource = {
> >> +       .name   = "PCI busn",
> >> +       .start  = 0,
> >> +       .end    = 0xffffff,
> >> +       .flags  = IORESOURCE_BUS,
> >> +};
> >
> > I'm not sure this should be global.  iomem_resource and
> > ioport_resource *are* really global, because they refer to processor
> > address space that is the same for everybody.  But PCI bus numbers are
> > specific to PCI.  Some machines don't have PCI at all, and there are
> > different bus architectures to which this doesn't apply.
> 
> that does not hurt them.

Yes but it's superfluous and confusing if you're porting to a new arch
or looking at changes in generic code that may affect you.

> > The 0-0xffffff range is misleading because it includes both the domain
> > and the bus number, and it's meaningless to allocate ranges that cross
> > domain boundaries.  For example, [bus 0x0000f0-0x000120] includes bus
> > numbers from domain 0000 and domain 0001, which doesn't make any sense
> > because a bus can only be in one domain.
> 
> allocation code will make sure it will be cross the boundary for domain.

But that means everyone reading it will do a double take, have to dig
into the implementation, and only then say "ah yeah ok it looks
correct" rather than it being obvious from the fact that the resource
is tracked on a per-domain basis.

> > I think it would make more sense to keep this bus number resource in a
> > per-host bridge structure.  Then we wouldn't need to include the
> > domain number at all because the host bridge determines the domain.
> 
> not sure. insert the all busn_res of all peer root buses into one
> global iobusn_resource
> looks more simple.

In what sense?  Simpler in the sense of your current implementation,
but not simpler at all to someone just reading the code...

-- 
Jesse Barnes, Intel Open Source Technology Center

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

Re: [PATCH 01/24] PCI: Add iobusn_resource

[Yinghai Lu]

On Fri, Feb 10, 2012 at 12:51 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 6 Feb 2012 10:55:14 -0800
> Yinghai Lu <yinghai@kernel.org> wrote:
>
>> On Mon, Feb 6, 2012 at 10:48 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> >> +struct resource iobusn_resource = {
>> >> +       .name   = "PCI busn",
>> >> +       .start  = 0,
>> >> +       .end    = 0xffffff,
>> >> +       .flags  = IORESOURCE_BUS,
>> >> +};
>> >
>> > I'm not sure this should be global.  iomem_resource and
>> > ioport_resource *are* really global, because they refer to processor
>> > address space that is the same for everybody.  But PCI bus numbers are
>> > specific to PCI.  Some machines don't have PCI at all, and there are
>> > different bus architectures to which this doesn't apply.
>>
>> that does not hurt them.
>
> Yes but it's superfluous and confusing if you're porting to a new arch
> or looking at changes in generic code that may affect you.
>
>> > The 0-0xffffff range is misleading because it includes both the domain
>> > and the bus number, and it's meaningless to allocate ranges that cross
>> > domain boundaries.  For example, [bus 0x0000f0-0x000120] includes bus
>> > numbers from domain 0000 and domain 0001, which doesn't make any sense
>> > because a bus can only be in one domain.
>>
>> allocation code will make sure it will be cross the boundary for domain.
>
> But that means everyone reading it will do a double take, have to dig
> into the implementation, and only then say "ah yeah ok it looks
> correct" rather than it being obvious from the fact that the resource
> is tracked on a per-domain basis.
>
>> > I think it would make more sense to keep this bus number resource in a
>> > per-host bridge structure.  Then we wouldn't need to include the
>> > domain number at all because the host bridge determines the domain.
>>
>> not sure. insert the all busn_res of all peer root buses into one
>> global iobusn_resource
>> looks more simple.
>
> In what sense?  Simpler in the sense of your current implementation,
> but not simpler at all to someone just reading the code...

ok, will check if i could drop iobusn_resource.

Yinghai

[PATCH 02/24] PCI: add busn inline helper

[Yinghai Lu]

convert back and forth with busn and domain_nr/bus_nr

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 include/linux/ioport.h |   17 +++++++++++++++++
 1 files changed, 17 insertions(+), 0 deletions(-)

diff --git a/include/linux/ioport.h b/include/linux/ioport.h
index 6fe9e19..f80c0cc 100644
--- a/include/linux/ioport.h
+++ b/include/linux/ioport.h
@@ -138,6 +138,23 @@ extern struct resource ioport_resource;
 extern struct resource iomem_resource;
 extern struct resource iobusn_resource;
 
+static inline int busn_domain_nr(resource_size_t busn)
+{
+	return busn >> 8;
+}
+static inline int busn_bus_nr(resource_size_t busn)
+{
+	return busn & 0xff;
+}
+static inline resource_size_t busn_update_bus_nr(resource_size_t busn, int b_nr)
+{
+	return (busn & ~0xff) | (b_nr & 0xff);
+}
+static inline resource_size_t busn(int d_nr, int b_nr)
+{
+	return  ((d_nr & 0xffff) << 8) | (b_nr & 0xff);
+}
+
 extern struct resource *request_resource_conflict(struct resource *root, struct resource *new);
 extern int request_resource(struct resource *root, struct resource *new);
 extern int release_resource(struct resource *new);
-- 
1.7.7

[PATCH 03/24] Make %pR could handle bus resource with domain

[Yinghai Lu]

So could use %pR for busn_res with domain nr in start/end

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
---
 lib/vsprintf.c |   29 +++++++++++++++++++++++++----
 1 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 8e75003..9397998 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -451,6 +451,12 @@ char *resource_string(char *buf, char *end, struct resource *res,
 		.precision = -1,
 		.flags = SPECIAL | SMALL | ZEROPAD,
 	};
+	static const struct printf_spec domain_spec = {
+		.base = 16,
+		.field_width = 4,
+		.precision = -1,
+		.flags = SMALL | ZEROPAD,
+	};
 	static const struct printf_spec bus_spec = {
 		.base = 16,
 		.field_width = 2,
@@ -507,11 +513,26 @@ char *resource_string(char *buf, char *end, struct resource *res,
 		specp = &mem_spec;
 		decode = 0;
 	}
-	p = number(p, pend, res->start, *specp);
-	if (res->start != res->end) {
-		*p++ = '-';
-		p = number(p, pend, res->end, *specp);
+
+	if (res->flags & IORESOURCE_BUS && busn_domain_nr(res->end)) {
+		p = number(p, pend, busn_domain_nr(res->start), domain_spec);
+		*p++ = ':';
+		p = number(p, pend, busn_bus_nr(res->start), *specp);
+		if (res->start != res->end) {
+			*p++ = '-';
+			p = number(p, pend, busn_domain_nr(res->end),
+					 domain_spec);
+			*p++ = ':';
+			p = number(p, pend, busn_bus_nr(res->end), *specp);
+		}
+	} else {
+		p = number(p, pend, res->start, *specp);
+		if (res->start != res->end) {
+			*p++ = '-';
+			p = number(p, pend, res->end, *specp);
+		}
 	}
+
 	if (decode) {
 		if (res->flags & IORESOURCE_MEM_64)
 			p = string(p, pend, " 64bit", str_spec);
-- 
1.7.7

[PATCH 04/24] PCI: Add busn_res operation functions

[Yinghai Lu]

will use them insert/update busn res in pci_bus

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    3 +++
 2 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a114173..8d4de5e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1622,6 +1622,48 @@ err_out:
 	return NULL;
 }
 
+void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
+{
+	struct resource *res = &b->busn_res;
+	struct resource *parent_res = &iobusn_resource;
+	int ret;
+
+	res->start = busn(pci_domain_nr(b), bus);
+	res->end = busn(pci_domain_nr(b), bus_max);
+	res->flags = IORESOURCE_BUS;
+
+	if (!pci_is_root_bus(b))
+		parent_res = &b->parent->busn_res;
+
+	ret = insert_resource(parent_res, res);
+
+	dev_printk(KERN_DEBUG, &b->dev,
+			"busn_res: %pR %s inserted under %pR\n",
+			res, ret ? "can not be" : "is", parent_res);
+}
+
+void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
+{
+	struct resource *res = &b->busn_res;
+	struct resource old_res = *res;
+
+	res->end = busn_update_bus_nr(res->end, bus_max);
+	dev_printk(KERN_DEBUG, &b->dev,
+			"busn_res: %pR end updated to %pR\n",
+			&old_res, res);
+}
+
+void pci_bus_release_busn_res(struct pci_bus *b)
+{
+	struct resource *res = &b->busn_res;
+	int ret;
+
+	ret = release_resource(res);
+	dev_printk(KERN_DEBUG, &b->dev,
+			"busn_res: %pR %s released\n",
+			res, ret ? "can not be" : "is");
+}
+
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 		struct pci_ops *ops, void *sysdata, struct list_head *resources)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 94ad468..3da935c 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -665,6 +665,9 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 				    struct pci_ops *ops, void *sysdata,
 				    struct list_head *resources);
+void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
+void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
+void pci_bus_release_busn_res(struct pci_bus *b);
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 					     struct pci_ops *ops, void *sysdata,
 					     struct list_head *resources);
-- 
1.7.7

Re: [PATCH 04/24] PCI: Add busn_res operation functions

[Bjorn Helgaas]

On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> will use them insert/update busn res in pci_bus
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>  include/linux/pci.h |    3 +++
>  2 files changed, 45 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index a114173..8d4de5e 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1622,6 +1622,48 @@ err_out:
>        return NULL;
>  }
>
> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
> +{
> +       struct resource *res = &b->busn_res;
> +       struct resource *parent_res = &iobusn_resource;
> +       int ret;
> +
> +       res->start = busn(pci_domain_nr(b), bus);
> +       res->end = busn(pci_domain_nr(b), bus_max);
> +       res->flags = IORESOURCE_BUS;
> +
> +       if (!pci_is_root_bus(b))
> +               parent_res = &b->parent->busn_res;
> +
> +       ret = insert_resource(parent_res, res);
> +
> +       dev_printk(KERN_DEBUG, &b->dev,
> +                       "busn_res: %pR %s inserted under %pR\n",
> +                       res, ret ? "can not be" : "is", parent_res);
> +}
> +
> +void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
> +{
> +       struct resource *res = &b->busn_res;
> +       struct resource old_res = *res;
> +
> +       res->end = busn_update_bus_nr(res->end, bus_max);

I think this design is a mistake.  Here's what you're doing:

  - initialize struct resource (keys are "start" and "end")
  - insert into tree (placed in tree by kernel/resource.c based on
"start" and "end")
  - update "end"

You "know" in this case that the update is safe because the caller has
validated "bus_max."  But that still breaks the kernel/resource.c
encapsulation.  If we change the kernel/resource.c implementation,
this code might break.

I think it would be better to remove the bus resource from the tree,
change its "end," then re-insert it.

> +       dev_printk(KERN_DEBUG, &b->dev,
> +                       "busn_res: %pR end updated to %pR\n",
> +                       &old_res, res);
> +}
> +
> +void pci_bus_release_busn_res(struct pci_bus *b)
> +{
> +       struct resource *res = &b->busn_res;
> +       int ret;
> +
> +       ret = release_resource(res);
> +       dev_printk(KERN_DEBUG, &b->dev,
> +                       "busn_res: %pR %s released\n",
> +                       res, ret ? "can not be" : "is");
> +}
> +
>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>                struct pci_ops *ops, void *sysdata, struct list_head *resources)
>  {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 94ad468..3da935c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -665,6 +665,9 @@ struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
>  struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>                                    struct pci_ops *ops, void *sysdata,
>                                    struct list_head *resources);
> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
> +void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
> +void pci_bus_release_busn_res(struct pci_bus *b);
>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>                                             struct pci_ops *ops, void *sysdata,
>                                             struct list_head *resources);
> --
> 1.7.7
>

Re: [PATCH 04/24] PCI: Add busn_res operation functions

[Yinghai Lu]

On Mon, Feb 6, 2012 at 10:59 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> will use them insert/update busn res in pci_bus
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/probe.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/pci.h |    3 +++
>>  2 files changed, 45 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index a114173..8d4de5e 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1622,6 +1622,48 @@ err_out:
>>        return NULL;
>>  }
>>
>> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
>> +{
>> +       struct resource *res = &b->busn_res;
>> +       struct resource *parent_res = &iobusn_resource;
>> +       int ret;
>> +
>> +       res->start = busn(pci_domain_nr(b), bus);
>> +       res->end = busn(pci_domain_nr(b), bus_max);
>> +       res->flags = IORESOURCE_BUS;
>> +
>> +       if (!pci_is_root_bus(b))
>> +               parent_res = &b->parent->busn_res;
>> +
>> +       ret = insert_resource(parent_res, res);
>> +
>> +       dev_printk(KERN_DEBUG, &b->dev,
>> +                       "busn_res: %pR %s inserted under %pR\n",
>> +                       res, ret ? "can not be" : "is", parent_res);
>> +}
>> +
>> +void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
>> +{
>> +       struct resource *res = &b->busn_res;
>> +       struct resource old_res = *res;
>> +
>> +       res->end = busn_update_bus_nr(res->end, bus_max);
>
> I think this design is a mistake.  Here's what you're doing:
>
>  - initialize struct resource (keys are "start" and "end")
>  - insert into tree (placed in tree by kernel/resource.c based on
> "start" and "end")
>  - update "end"
>
> You "know" in this case that the update is safe because the caller has
> validated "bus_max."  But that still breaks the kernel/resource.c
> encapsulation.  If we change the kernel/resource.c implementation,
> this code might break.

the point is: I only want to reuse allocate_resource() to get right position.
and the code does not depends to kernel/resource.c much.

>
> I think it would be better to remove the bus resource from the tree,
> change its "end," then re-insert it.

how about parent buses that have extended top?

Yinghai

Re: [PATCH 04/24] PCI: Add busn_res operation functions

[Bjorn Helgaas]

On Mon, Feb 6, 2012 at 12:45 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Feb 6, 2012 at 10:59 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> will use them insert/update busn res in pci_bus
>>>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> ---
>>>  drivers/pci/probe.c |   42 ++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/pci.h |    3 +++
>>>  2 files changed, 45 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>> index a114173..8d4de5e 100644
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -1622,6 +1622,48 @@ err_out:
>>>        return NULL;
>>>  }
>>>
>>> +void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
>>> +{
>>> +       struct resource *res = &b->busn_res;
>>> +       struct resource *parent_res = &iobusn_resource;
>>> +       int ret;
>>> +
>>> +       res->start = busn(pci_domain_nr(b), bus);
>>> +       res->end = busn(pci_domain_nr(b), bus_max);
>>> +       res->flags = IORESOURCE_BUS;
>>> +
>>> +       if (!pci_is_root_bus(b))
>>> +               parent_res = &b->parent->busn_res;
>>> +
>>> +       ret = insert_resource(parent_res, res);
>>> +
>>> +       dev_printk(KERN_DEBUG, &b->dev,
>>> +                       "busn_res: %pR %s inserted under %pR\n",
>>> +                       res, ret ? "can not be" : "is", parent_res);
>>> +}
>>> +
>>> +void pci_bus_update_busn_res_end(struct pci_bus *b, int bus_max)
>>> +{
>>> +       struct resource *res = &b->busn_res;
>>> +       struct resource old_res = *res;
>>> +
>>> +       res->end = busn_update_bus_nr(res->end, bus_max);
>>
>> I think this design is a mistake.  Here's what you're doing:
>>
>>  - initialize struct resource (keys are "start" and "end")
>>  - insert into tree (placed in tree by kernel/resource.c based on
>> "start" and "end")
>>  - update "end"
>>
>> You "know" in this case that the update is safe because the caller has
>> validated "bus_max."  But that still breaks the kernel/resource.c
>> encapsulation.  If we change the kernel/resource.c implementation,
>> this code might break.
>
> the point is: I only want to reuse allocate_resource() to get right position.
> and the code does not depends to kernel/resource.c much.
>
>>
>> I think it would be better to remove the bus resource from the tree,
>> change its "end," then re-insert it.
>
> how about parent buses that have extended top?

I don't understand your question.  I assume you mean there's a case
where remove/update/reinsert doesn't work, but I don't see why that
would be a problem.  Can you show an example?

Re: [PATCH 04/24] PCI: Add busn_res operation functions

[Yinghai Lu]

On Sun, Feb 12, 2012 at 3:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>>>
>>> I think it would be better to remove the bus resource from the tree,
>>> change its "end," then re-insert it.
>>
>> how about parent buses that have extended top?
>
> I don't understand your question.  I assume you mean there's a case
> where remove/update/reinsert doesn't work, but I don't see why that
> would be a problem.  Can you show an example?

I mean parent busn_res already had several level's children busn_res.
and every level may have some siblings.
before remove will need to record those resources, to later to put them back.

that just increase not necessary complexity. because we already know
those resource could be extended safely.

Yinghai

Re: [PATCH 04/24] PCI: Add busn_res operation functions

[Bjorn Helgaas]

On Sun, Feb 12, 2012 at 4:03 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sun, Feb 12, 2012 at 3:51 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>>
>>>>
>>>> I think it would be better to remove the bus resource from the tree,
>>>> change its "end," then re-insert it.
>>>
>>> how about parent buses that have extended top?
>>
>> I don't understand your question.  I assume you mean there's a case
>> where remove/update/reinsert doesn't work, but I don't see why that
>> would be a problem.  Can you show an example?
>
> I mean parent busn_res already had several level's children busn_res.
> and every level may have some siblings.
> before remove will need to record those resources, to later to put them back.
>
> that just increase not necessary complexity. because we already know
> those resource could be extended safely.

You're doing surgery on the middle of a relatively complicated data
structure.  Now readers of the code have to trust that not only does
kernel/resource.c work correctly, but they also have to examine this
PCI code to make sure that these alterations are safe.  I know this is
all crystal-clear in your mind, and no doubt it is correct right now,
but I don't think it is a reader-friendly approach.

But I don't expect to convince you, so I'll stop trying :)

Bjorn

[PATCH 05/24] PCI: add /proc/iobusn

[Yinghai Lu]

May help debug purpose.

need to fill root buses name.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |    3 +++
 kernel/resource.c   |   42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 8d4de5e..fabf9d0 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1594,6 +1594,8 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 
 	b->number = b->secondary = bus;
 
+	sprintf(b->name, "PCI Bus %04x:%02x", pci_domain_nr(b), b->number);
+
 	/* Add initial resources to the bus */
 	list_for_each_entry_safe(bus_res, n, resources, list)
 		list_move_tail(&bus_res->list, &b->resources);
@@ -1631,6 +1633,7 @@ void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int bus_max)
 	res->start = busn(pci_domain_nr(b), bus);
 	res->end = busn(pci_domain_nr(b), bus_max);
 	res->flags = IORESOURCE_BUS;
+	res->name = b->name;
 
 	if (!pci_is_root_bus(b))
 		parent_res = &b->parent->busn_res;
diff --git a/kernel/resource.c b/kernel/resource.c
index 53b42f0..1a73a81 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -113,6 +113,30 @@ static const struct seq_operations resource_op = {
 	.show	= r_show,
 };
 
+static int busn_r_show(struct seq_file *m, void *v)
+{
+	struct resource *root = m->private;
+	struct resource *r = v, *p;
+	int depth;
+
+	for (depth = 0, p = r; depth < MAX_IORES_LEVEL; depth++, p = p->parent)
+		if (p->parent == root)
+			break;
+	seq_printf(m, "%*s%04x:%02x-%04x:%02x : %s\n",
+			depth * 2, "",
+			busn_domain_nr(r->start), busn_bus_nr(r->start),
+			busn_domain_nr(r->end), busn_bus_nr(r->end),
+			r->name ? r->name : "<BAD>");
+	return 0;
+}
+
+static const struct seq_operations busn_resource_op = {
+	.start	= r_start,
+	.next	= r_next,
+	.stop	= r_stop,
+	.show	= busn_r_show,
+};
+
 static int ioports_open(struct inode *inode, struct file *file)
 {
 	int res = seq_open(file, &resource_op);
@@ -133,6 +157,16 @@ static int iomem_open(struct inode *inode, struct file *file)
 	return res;
 }
 
+static int iobusn_open(struct inode *inode, struct file *file)
+{
+	int res = seq_open(file, &busn_resource_op);
+	if (!res) {
+		struct seq_file *m = file->private_data;
+		m->private = &iobusn_resource;
+	}
+	return res;
+}
+
 static const struct file_operations proc_ioports_operations = {
 	.open		= ioports_open,
 	.read		= seq_read,
@@ -147,10 +181,18 @@ static const struct file_operations proc_iomem_operations = {
 	.release	= seq_release,
 };
 
+static const struct file_operations proc_iobusn_operations = {
+	.open		= iobusn_open,
+	.read		= seq_read,
+	.llseek		= seq_lseek,
+	.release	= seq_release,
+};
+
 static int __init ioresources_init(void)
 {
 	proc_create("ioports", 0, NULL, &proc_ioports_operations);
 	proc_create("iomem", 0, NULL, &proc_iomem_operations);
+	proc_create("iobusn", 0, NULL, &proc_iobusn_operations);
 	return 0;
 }
 __initcall(ioresources_init);
-- 
1.7.7

[PATCH 06/24] PCI: Add busn_res tracking in core

[Yinghai Lu]

update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
iobusn_resource tree.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c  |   30 ++++++++++++++++++++++++++----
 drivers/pci/remove.c |    1 +
 include/linux/pci.h  |    4 ++++
 3 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index fabf9d0..cdd6e83 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1667,8 +1667,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
 			res, ret ? "can not be" : "is");
 }
 
-struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
-		struct pci_ops *ops, void *sysdata, struct list_head *resources)
+struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
+		int bus_max, struct pci_ops *ops, void *sysdata,
+		struct list_head *resources)
 {
 	struct pci_bus *b;
 
@@ -1676,10 +1677,26 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 	if (!b)
 		return NULL;
 
+	pci_bus_insert_busn_res(b, bus, bus_max);
 	b->subordinate = pci_scan_child_bus(b);
 	pci_bus_add_devices(b);
 	return b;
 }
+EXPORT_SYMBOL(pci_scan_root_bus_max);
+
+struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
+		struct pci_ops *ops, void *sysdata,
+		struct list_head *resources)
+{
+	struct pci_bus *b;
+
+	b = pci_scan_root_bus_max(parent, bus, 255, ops, sysdata, resources);
+
+	if (b)
+		pci_bus_update_busn_res_end(b, b->subordinate);
+
+	return b;
+}
 EXPORT_SYMBOL(pci_scan_root_bus);
 
 /* Deprecated; use pci_scan_root_bus() instead */
@@ -1692,9 +1709,11 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
 	pci_add_resource(&resources, &ioport_resource);
 	pci_add_resource(&resources, &iomem_resource);
 	b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
-	if (b)
+	if (b) {
+		pci_bus_insert_busn_res(b, bus, 255);
 		b->subordinate = pci_scan_child_bus(b);
-	else
+		pci_bus_update_busn_res_end(b, b->subordinate);
+	} else
 		pci_free_resource_list(&resources);
 	return b;
 }
@@ -1710,7 +1729,10 @@ struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *ops,
 	pci_add_resource(&resources, &iomem_resource);
 	b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
 	if (b) {
+		pci_bus_insert_busn_res(b, bus, 255);
 		b->subordinate = pci_scan_child_bus(b);
+		pci_bus_update_busn_res_end(b, b->subordinate);
+
 		pci_bus_add_devices(b);
 	} else {
 		pci_free_resource_list(&resources);
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
index 82f8ae5..5b679d2 100644
--- a/drivers/pci/remove.c
+++ b/drivers/pci/remove.c
@@ -68,6 +68,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
 
 	down_write(&pci_bus_sem);
 	list_del(&pci_bus->node);
+	pci_bus_release_busn_res(pci_bus);
 	up_write(&pci_bus_sem);
 	if (!pci_bus->is_added)
 		return;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 3da935c..d5b6786 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -668,6 +668,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
 void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
 void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
 void pci_bus_release_busn_res(struct pci_bus *b);
+struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
+					     int busmax, struct pci_ops *ops,
+					     void *sysdata,
+					     struct list_head *resources);
 struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
 					     struct pci_ops *ops, void *sysdata,
 					     struct list_head *resources);
-- 
1.7.7

Re: [PATCH 06/24] PCI: Add busn_res tracking in core

[Bjorn Helgaas]

On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
> iobusn_resource tree.
>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> ---
>  drivers/pci/probe.c  |   30 ++++++++++++++++++++++++++----
>  drivers/pci/remove.c |    1 +
>  include/linux/pci.h  |    4 ++++
>  3 files changed, 31 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index fabf9d0..cdd6e83 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1667,8 +1667,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
>                        res, ret ? "can not be" : "is");
>  }
>
> -struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> -               struct pci_ops *ops, void *sysdata, struct list_head *resources)
> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
> +               int bus_max, struct pci_ops *ops, void *sysdata,
> +               struct list_head *resources)
>  {
>        struct pci_bus *b;
>
> @@ -1676,10 +1677,26 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>        if (!b)
>                return NULL;
>
> +       pci_bus_insert_busn_res(b, bus, bus_max);
>        b->subordinate = pci_scan_child_bus(b);
>        pci_bus_add_devices(b);
>        return b;
>  }
> +EXPORT_SYMBOL(pci_scan_root_bus_max);
> +
> +struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> +               struct pci_ops *ops, void *sysdata,
> +               struct list_head *resources)
> +{
> +       struct pci_bus *b;
> +
> +       b = pci_scan_root_bus_max(parent, bus, 255, ops, sysdata, resources);
> +
> +       if (b)
> +               pci_bus_update_busn_res_end(b, b->subordinate);
> +
> +       return b;
> +}
>  EXPORT_SYMBOL(pci_scan_root_bus);
>
>  /* Deprecated; use pci_scan_root_bus() instead */
> @@ -1692,9 +1709,11 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
>        pci_add_resource(&resources, &ioport_resource);
>        pci_add_resource(&resources, &iomem_resource);
>        b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
> -       if (b)
> +       if (b) {
> +               pci_bus_insert_busn_res(b, bus, 255);
>                b->subordinate = pci_scan_child_bus(b);
> -       else
> +               pci_bus_update_busn_res_end(b, b->subordinate);
> +       } else
>                pci_free_resource_list(&resources);
>        return b;
>  }
> @@ -1710,7 +1729,10 @@ struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *ops,
>        pci_add_resource(&resources, &iomem_resource);
>        b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
>        if (b) {
> +               pci_bus_insert_busn_res(b, bus, 255);
>                b->subordinate = pci_scan_child_bus(b);
> +               pci_bus_update_busn_res_end(b, b->subordinate);
> +
>                pci_bus_add_devices(b);
>        } else {
>                pci_free_resource_list(&resources);
> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> index 82f8ae5..5b679d2 100644
> --- a/drivers/pci/remove.c
> +++ b/drivers/pci/remove.c
> @@ -68,6 +68,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
>
>        down_write(&pci_bus_sem);
>        list_del(&pci_bus->node);
> +       pci_bus_release_busn_res(pci_bus);
>        up_write(&pci_bus_sem);
>        if (!pci_bus->is_added)
>                return;
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 3da935c..d5b6786 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -668,6 +668,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>  void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
>  void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
>  void pci_bus_release_busn_res(struct pci_bus *b);
> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
> +                                            int busmax, struct pci_ops *ops,
> +                                            void *sysdata,
> +                                            struct list_head *resources);
>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>                                             struct pci_ops *ops, void *sysdata,
>                                             struct list_head *resources);

Now we have both pci_bus_insert_busn_res() and
pci_scan_root_bus_max().  Why do we need both?  I think it's too much
of a burden on architectures to expect them to understand both.

Maybe some sample pseudo-code using both interfaces would help me
understand what you expect a typical architecture to do.

Bjorn

Re: [PATCH 06/24] PCI: Add busn_res tracking in core

[Yinghai Lu]

On Wed, Feb 8, 2012 at 8:08 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
>> iobusn_resource tree.
>>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> ---
>>  drivers/pci/probe.c  |   30 ++++++++++++++++++++++++++----
>>  drivers/pci/remove.c |    1 +
>>  include/linux/pci.h  |    4 ++++
>>  3 files changed, 31 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index fabf9d0..cdd6e83 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -1667,8 +1667,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
>>                        res, ret ? "can not be" : "is");
>>  }
>>
>> -struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>> -               struct pci_ops *ops, void *sysdata, struct list_head *resources)
>> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
>> +               int bus_max, struct pci_ops *ops, void *sysdata,
>> +               struct list_head *resources)
>>  {
>>        struct pci_bus *b;
>>
>> @@ -1676,10 +1677,26 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>>        if (!b)
>>                return NULL;
>>
>> +       pci_bus_insert_busn_res(b, bus, bus_max);
>>        b->subordinate = pci_scan_child_bus(b);
>>        pci_bus_add_devices(b);
>>        return b;
>>  }
>> +EXPORT_SYMBOL(pci_scan_root_bus_max);
>> +
>> +struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>> +               struct pci_ops *ops, void *sysdata,
>> +               struct list_head *resources)
>> +{
>> +       struct pci_bus *b;
>> +
>> +       b = pci_scan_root_bus_max(parent, bus, 255, ops, sysdata, resources);
>> +
>> +       if (b)
>> +               pci_bus_update_busn_res_end(b, b->subordinate);
>> +
>> +       return b;
>> +}
>>  EXPORT_SYMBOL(pci_scan_root_bus);
>>
>>  /* Deprecated; use pci_scan_root_bus() instead */
>> @@ -1692,9 +1709,11 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
>>        pci_add_resource(&resources, &ioport_resource);
>>        pci_add_resource(&resources, &iomem_resource);
>>        b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
>> -       if (b)
>> +       if (b) {
>> +               pci_bus_insert_busn_res(b, bus, 255);
>>                b->subordinate = pci_scan_child_bus(b);
>> -       else
>> +               pci_bus_update_busn_res_end(b, b->subordinate);
>> +       } else
>>                pci_free_resource_list(&resources);
>>        return b;
>>  }
>> @@ -1710,7 +1729,10 @@ struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *ops,
>>        pci_add_resource(&resources, &iomem_resource);
>>        b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
>>        if (b) {
>> +               pci_bus_insert_busn_res(b, bus, 255);
>>                b->subordinate = pci_scan_child_bus(b);
>> +               pci_bus_update_busn_res_end(b, b->subordinate);
>> +
>>                pci_bus_add_devices(b);
>>        } else {
>>                pci_free_resource_list(&resources);
>> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
>> index 82f8ae5..5b679d2 100644
>> --- a/drivers/pci/remove.c
>> +++ b/drivers/pci/remove.c
>> @@ -68,6 +68,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
>>
>>        down_write(&pci_bus_sem);
>>        list_del(&pci_bus->node);
>> +       pci_bus_release_busn_res(pci_bus);
>>        up_write(&pci_bus_sem);
>>        if (!pci_bus->is_added)
>>                return;
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 3da935c..d5b6786 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -668,6 +668,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
>>  void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
>>  void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
>>  void pci_bus_release_busn_res(struct pci_bus *b);
>> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
>> +                                            int busmax, struct pci_ops *ops,
>> +                                            void *sysdata,
>> +                                            struct list_head *resources);
>>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
>>                                             struct pci_ops *ops, void *sysdata,
>>                                             struct list_head *resources);
>
> Now we have both pci_bus_insert_busn_res() and
> pci_scan_root_bus_max().  Why do we need both?  I think it's too much
> of a burden on architectures to expect them to understand both.
>
> Maybe some sample pseudo-code using both interfaces would help me
> understand what you expect a typical architecture to do.

We need to call pci_bus_insert_busn_res for root bus:
1. after that bus is allocated (...) : that busn_res is in the bus struct.
2. before pci_scan_child_bus

Yinghai

Re: [PATCH 06/24] PCI: Add busn_res tracking in core

[Jesse Barnes]

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

On Wed, 8 Feb 2012 09:26:52 -0800
Yinghai Lu <yinghai@kernel.org> wrote:

> On Wed, Feb 8, 2012 at 8:08 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> >> update pci_scan_root_bus, and pci_scan_bus to insert root bus busn into
> >> iobusn_resource tree.
> >>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >> ---
> >>  drivers/pci/probe.c  |   30 ++++++++++++++++++++++++++----
> >>  drivers/pci/remove.c |    1 +
> >>  include/linux/pci.h  |    4 ++++
> >>  3 files changed, 31 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> >> index fabf9d0..cdd6e83 100644
> >> --- a/drivers/pci/probe.c
> >> +++ b/drivers/pci/probe.c
> >> @@ -1667,8 +1667,9 @@ void pci_bus_release_busn_res(struct pci_bus *b)
> >>                        res, ret ? "can not be" : "is");
> >>  }
> >>
> >> -struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> >> -               struct pci_ops *ops, void *sysdata, struct list_head *resources)
> >> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
> >> +               int bus_max, struct pci_ops *ops, void *sysdata,
> >> +               struct list_head *resources)
> >>  {
> >>        struct pci_bus *b;
> >>
> >> @@ -1676,10 +1677,26 @@ struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> >>        if (!b)
> >>                return NULL;
> >>
> >> +       pci_bus_insert_busn_res(b, bus, bus_max);
> >>        b->subordinate = pci_scan_child_bus(b);
> >>        pci_bus_add_devices(b);
> >>        return b;
> >>  }
> >> +EXPORT_SYMBOL(pci_scan_root_bus_max);
> >> +
> >> +struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> >> +               struct pci_ops *ops, void *sysdata,
> >> +               struct list_head *resources)
> >> +{
> >> +       struct pci_bus *b;
> >> +
> >> +       b = pci_scan_root_bus_max(parent, bus, 255, ops, sysdata, resources);
> >> +
> >> +       if (b)
> >> +               pci_bus_update_busn_res_end(b, b->subordinate);
> >> +
> >> +       return b;
> >> +}
> >>  EXPORT_SYMBOL(pci_scan_root_bus);
> >>
> >>  /* Deprecated; use pci_scan_root_bus() instead */
> >> @@ -1692,9 +1709,11 @@ struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent,
> >>        pci_add_resource(&resources, &ioport_resource);
> >>        pci_add_resource(&resources, &iomem_resource);
> >>        b = pci_create_root_bus(parent, bus, ops, sysdata, &resources);
> >> -       if (b)
> >> +       if (b) {
> >> +               pci_bus_insert_busn_res(b, bus, 255);
> >>                b->subordinate = pci_scan_child_bus(b);
> >> -       else
> >> +               pci_bus_update_busn_res_end(b, b->subordinate);
> >> +       } else
> >>                pci_free_resource_list(&resources);
> >>        return b;
> >>  }
> >> @@ -1710,7 +1729,10 @@ struct pci_bus * __devinit pci_scan_bus(int bus, struct pci_ops *ops,
> >>        pci_add_resource(&resources, &iomem_resource);
> >>        b = pci_create_root_bus(NULL, bus, ops, sysdata, &resources);
> >>        if (b) {
> >> +               pci_bus_insert_busn_res(b, bus, 255);
> >>                b->subordinate = pci_scan_child_bus(b);
> >> +               pci_bus_update_busn_res_end(b, b->subordinate);
> >> +
> >>                pci_bus_add_devices(b);
> >>        } else {
> >>                pci_free_resource_list(&resources);
> >> diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c
> >> index 82f8ae5..5b679d2 100644
> >> --- a/drivers/pci/remove.c
> >> +++ b/drivers/pci/remove.c
> >> @@ -68,6 +68,7 @@ void pci_remove_bus(struct pci_bus *pci_bus)
> >>
> >>        down_write(&pci_bus_sem);
> >>        list_del(&pci_bus->node);
> >> +       pci_bus_release_busn_res(pci_bus);
> >>        up_write(&pci_bus_sem);
> >>        if (!pci_bus->is_added)
> >>                return;
> >> diff --git a/include/linux/pci.h b/include/linux/pci.h
> >> index 3da935c..d5b6786 100644
> >> --- a/include/linux/pci.h
> >> +++ b/include/linux/pci.h
> >> @@ -668,6 +668,10 @@ struct pci_bus *pci_create_root_bus(struct device *parent, int bus,
> >>  void pci_bus_insert_busn_res(struct pci_bus *b, int bus, int busmax);
> >>  void pci_bus_update_busn_res_end(struct pci_bus *b, int busmax);
> >>  void pci_bus_release_busn_res(struct pci_bus *b);
> >> +struct pci_bus * __devinit pci_scan_root_bus_max(struct device *parent, int bus,
> >> +                                            int busmax, struct pci_ops *ops,
> >> +                                            void *sysdata,
> >> +                                            struct list_head *resources);
> >>  struct pci_bus * __devinit pci_scan_root_bus(struct device *parent, int bus,
> >>                                             struct pci_ops *ops, void *sysdata,
> >>                                             struct list_head *resources);
> >
> > Now we have both pci_bus_insert_busn_res() and
> > pci_scan_root_bus_max().  Why do we need both?  I think it's too much
> > of a burden on architectures to expect them to understand both.
> >
> > Maybe some sample pseudo-code using both interfaces would help me
> > understand what you expect a typical architecture to do.
> 
> We need to call pci_bus_insert_busn_res for root bus:
> 1. after that bus is allocated (...) : that busn_res is in the bus struct.
> 2. before pci_scan_child_bus

I agree with Bjorn; at the very least this has a really bad name.  How
are people supposed to know which function to call?  If it's just for
the primary scan at init, then it should be named as such.  But ideally
it would be inside the existing scan_root_bus...

-- 
Jesse Barnes, Intel Open Source Technology Center

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

[PATCH 07/24] PCI, x86: Register busn_res for root buses

[Yinghai Lu]

update x86_pci_root_bus_resources() to get bus_max for non-acpi case.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/topology.h |    3 ++-
 arch/x86/pci/acpi.c             |    8 +++++---
 arch/x86/pci/bus_numa.c         |    8 +++++++-
 arch/x86/pci/common.c           |   11 +++++++----
 4 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/topology.h b/arch/x86/include/asm/topology.h
index b9676ae..ad4060e 100644
--- a/arch/x86/include/asm/topology.h
+++ b/arch/x86/include/asm/topology.h
@@ -172,7 +172,8 @@ static inline void arch_fix_phys_package_id(int num, u32 slot)
 }
 
 struct pci_bus;
-void x86_pci_root_bus_resources(int bus, struct list_head *resources);
+void x86_pci_root_bus_resources(int bus, int *bus_max,
+				struct list_head *resources);
 
 #ifdef CONFIG_SMP
 #define mc_capable()	((boot_cpu_data.x86_max_cores > 1) && \
diff --git a/arch/x86/pci/acpi.c b/arch/x86/pci/acpi.c
index daa4249..b25b5b1 100644
--- a/arch/x86/pci/acpi.c
+++ b/arch/x86/pci/acpi.c
@@ -348,6 +348,7 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
 	struct acpi_device *device = root->device;
 	int domain = root->segment;
 	int busnum = root->secondary.start;
+	int busmax = root->secondary.end;
 	LIST_HEAD(resources);
 	struct pci_bus *bus;
 	struct pci_sysdata *sd;
@@ -410,12 +411,13 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_pci_root *root)
 		 * defaults or native bridge info if we're ignoring _CRS.
 		 */
 		if (!pci_use_crs)
-			x86_pci_root_bus_resources(busnum, &resources);
+			x86_pci_root_bus_resources(busnum, &busmax, &resources);
 		bus = pci_create_root_bus(NULL, busnum, &pci_root_ops, sd,
 					  &resources);
-		if (bus)
+		if (bus) {
+			pci_bus_insert_busn_res(bus, busnum, busmax);
 			bus->subordinate = pci_scan_child_bus(bus);
-		else
+		} else
 			pci_free_resource_list(&resources);
 	}
 
diff --git a/arch/x86/pci/bus_numa.c b/arch/x86/pci/bus_numa.c
index fd3f655..5213cd8 100644
--- a/arch/x86/pci/bus_numa.c
+++ b/arch/x86/pci/bus_numa.c
@@ -7,7 +7,8 @@
 int pci_root_num;
 struct pci_root_info pci_root_info[PCI_ROOT_NR];
 
-void x86_pci_root_bus_resources(int bus, struct list_head *resources)
+void x86_pci_root_bus_resources(int bus, int *bus_max,
+				struct list_head *resources)
 {
 	int i;
 	int j;
@@ -28,6 +29,7 @@ void x86_pci_root_bus_resources(int bus, struct list_head *resources)
 	       bus);
 
 	info = &pci_root_info[i];
+	*bus_max = info->bus_max;
 	for (j = 0; j < info->res_num; j++) {
 		struct resource *res;
 		struct resource *root;
@@ -51,6 +53,10 @@ default_resources:
 	printk(KERN_DEBUG "PCI: root bus %02x: using default resources\n", bus);
 	pci_add_resource(resources, &ioport_resource);
 	pci_add_resource(resources, &iomem_resource);
+	if (!bus)
+		*bus_max = 0xff;
+	else if (!*bus_max)
+		*bus_max = bus;
 }
 
 void __devinit update_res(struct pci_root_info *info, resource_size_t start,
diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 323481e..ce0aefc 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -433,6 +433,7 @@ struct pci_bus * __devinit pcibios_scan_root(int busnum)
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
 	struct pci_sysdata *sd;
+	int bus_max;
 
 	while ((bus = pci_find_next_bus(bus)) != NULL) {
 		if (bus->number == busnum) {
@@ -454,8 +455,9 @@ struct pci_bus * __devinit pcibios_scan_root(int busnum)
 	sd->node = get_mp_bus_to_node(busnum);
 
 	printk(KERN_DEBUG "PCI: Probing PCI hardware (bus %02x)\n", busnum);
-	x86_pci_root_bus_resources(busnum, &resources);
-	bus = pci_scan_root_bus(NULL, busnum, &pci_root_ops, sd, &resources);
+	x86_pci_root_bus_resources(busnum, &bus_max, &resources);
+	bus = pci_scan_root_bus_max(NULL, busnum, bus_max, &pci_root_ops, sd,
+				&resources);
 	if (!bus) {
 		pci_free_resource_list(&resources);
 		kfree(sd);
@@ -643,6 +645,7 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 	LIST_HEAD(resources);
 	struct pci_bus *bus = NULL;
 	struct pci_sysdata *sd;
+	int bus_max;
 
 	/*
 	 * Allocate per-root-bus (not per bus) arch-specific data.
@@ -655,8 +658,8 @@ struct pci_bus * __devinit pci_scan_bus_on_node(int busno, struct pci_ops *ops,
 		return NULL;
 	}
 	sd->node = node;
-	x86_pci_root_bus_resources(busno, &resources);
-	bus = pci_scan_root_bus(NULL, busno, ops, sd, &resources);
+	x86_pci_root_bus_resources(busno, &bus_max, &resources);
+	bus = pci_scan_root_bus_max(NULL, busno, bus_max, ops, sd, &resources);
 	if (!bus) {
 		pci_free_resource_list(&resources);
 		kfree(sd);
-- 
1.7.7

[PATCH 08/24] PCI, ia64: Register busn_res for root buses

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
---
 arch/ia64/pci/pci.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/ia64/pci/pci.c b/arch/ia64/pci/pci.c
index f82f5d4..936b2f1 100644
--- a/arch/ia64/pci/pci.c
+++ b/arch/ia64/pci/pci.c
@@ -331,6 +331,7 @@ pci_acpi_scan_root(struct acpi_pci_root *root)
 	struct acpi_device *device = root->device;
 	int domain = root->segment;
 	int bus = root->secondary.start;
+	int busmax = root->secondary.end;
 	struct pci_controller *controller;
 	unsigned int windows = 0;
 	struct pci_root_info info;
@@ -384,6 +385,7 @@ pci_acpi_scan_root(struct acpi_pci_root *root)
 		return NULL;
 	}
 
+	pci_bus_insert_busn_res(pbus, busnum, busmax);
 	pbus->subordinate = pci_scan_child_bus(pbus);
 	return pbus;
 
-- 
1.7.7

[PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
---
 arch/powerpc/kernel/pci-common.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
index cce98d7..501f29b 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1732,6 +1732,8 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 	bus->secondary = hose->first_busno;
 	hose->bus = bus;
 
+	pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
+
 	/* Get probe mode and perform scan */
 	mode = PCI_PROBE_NORMAL;
 	if (node && ppc_md.pci_probe_mode)
@@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		of_scan_bus(node, bus);
 	}
 
-	if (mode == PCI_PROBE_NORMAL)
+	if (mode == PCI_PROBE_NORMAL) {
+		pci_bus_update_busn_res_end(bus, 255);
 		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+		pci_bus_update_busn_res_end(bus, bus->subordinate);
+	}
 
 	/* Platform gets a chance to do some global fixups before
 	 * we proceed to resource allocation
-- 
1.7.7

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Bjorn Helgaas]

On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: linuxppc-dev@lists.ozlabs.org
> ---
>  arch/powerpc/kernel/pci-common.c |    7 ++++++-
>  1 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
> index cce98d7..501f29b 100644
> --- a/arch/powerpc/kernel/pci-common.c
> +++ b/arch/powerpc/kernel/pci-common.c
> @@ -1732,6 +1732,8 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>        bus->secondary = hose->first_busno;
>        hose->bus = bus;
>
> +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
> +
>        /* Get probe mode and perform scan */
>        mode = PCI_PROBE_NORMAL;
>        if (node && ppc_md.pci_probe_mode)
> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>                of_scan_bus(node, bus);
>        }
>
> -       if (mode == PCI_PROBE_NORMAL)
> +       if (mode == PCI_PROBE_NORMAL) {
> +               pci_bus_update_busn_res_end(bus, 255);
>                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> +               pci_bus_update_busn_res_end(bus, bus->subordinate);
> +       }

The only architecture-specific thing here is discovering the range of
bus numbers below a host bridge.  The architecture should not have to
mess around with pci_bus_update_busn_res_end() like this.  It should
be able to say "here's my bus number range" (and of course the PCI
core can default to 0-255 if the arch doesn't supply a range) and the
core should take care of the rest.

Bjorn

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Yinghai Lu]

On Wed, Feb 8, 2012 at 7:58 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Sat, Feb 4, 2012 at 10:57 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>> Cc: Paul Mackerras <paulus@samba.org>
>> Cc: linuxppc-dev@lists.ozlabs.org
>> ---
>>  arch/powerpc/kernel/pci-common.c |    7 ++++++-
>>  1 files changed, 6 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/pci-common.c b/arch/powerpc/kernel/pci-common.c
>> index cce98d7..501f29b 100644
>> --- a/arch/powerpc/kernel/pci-common.c
>> +++ b/arch/powerpc/kernel/pci-common.c
>> @@ -1732,6 +1732,8 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>>        bus->secondary = hose->first_busno;
>>        hose->bus = bus;
>>
>> +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
>> +
>>        /* Get probe mode and perform scan */
>>        mode = PCI_PROBE_NORMAL;
>>        if (node && ppc_md.pci_probe_mode)
>> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>>                of_scan_bus(node, bus);
>>        }
>>
>> -       if (mode == PCI_PROBE_NORMAL)
>> +       if (mode == PCI_PROBE_NORMAL) {
>> +               pci_bus_update_busn_res_end(bus, 255);
>>                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
>> +               pci_bus_update_busn_res_end(bus, bus->subordinate);
>> +       }
>
> The only architecture-specific thing here is discovering the range of
> bus numbers below a host bridge.  The architecture should not have to
> mess around with pci_bus_update_busn_res_end() like this.  It should
> be able to say "here's my bus number range" (and of course the PCI
> core can default to 0-255 if the arch doesn't supply a range) and the
> core should take care of the rest.

during the pci_scan_child_bus,  child bus busn_res will be inserted
under parent bus busn_res.

So need to make sure parent busn_res.end is bigger enough.


Yinghai

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Benjamin Herrenschmidt]

On Wed, 2012-02-08 at 07:58 -0800, Bjorn Helgaas wrote:
> The only architecture-specific thing here is discovering the range of
> bus numbers below a host bridge.  The architecture should not have to
> mess around with pci_bus_update_busn_res_end() like this.  It should
> be able to say "here's my bus number range" (and of course the PCI
> core can default to 0-255 if the arch doesn't supply a range) and the
> core should take care of the rest. 

So it's a bit messy in here because we deal with several things.

What the firmware gives us is the range it assigned, but that isn't
necessarily the HW limits (almost never is in fact).

In some cases we honor it, for example when in "probe only" mode where
we prevent any reassigning, and in some case, we ignore it and let the
PCI core renumber things (typically because the FW "forgot" to set aside
bus numbers for a cardbus slot for example, that sort of things).

So it's a bit of a tricky situation.

Off the top of my head, I'm pretty sure that most if not all of our PCI
host bridges simply support a full 0...255 range and there is no sharing
between bridges like on x86, they are just different domains.

But I can't vouch 100% for some of the oddball cases like Pegasos or
some freescale gear.

Cheers,
Ben.

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Bjorn Helgaas]

On Wed, Feb 8, 2012 at 2:02 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2012-02-08 at 07:58 -0800, Bjorn Helgaas wrote:
>> The only architecture-specific thing here is discovering the range of
>> bus numbers below a host bridge.  The architecture should not have to
>> mess around with pci_bus_update_busn_res_end() like this.  It should
>> be able to say "here's my bus number range" (and of course the PCI
>> core can default to 0-255 if the arch doesn't supply a range) and the
>> core should take care of the rest.
>
> So it's a bit messy in here because we deal with several things.
>
> What the firmware gives us is the range it assigned, but that isn't
> necessarily the HW limits (almost never is in fact).
>
> In some cases we honor it, for example when in "probe only" mode where
> we prevent any reassigning, and in some case, we ignore it and let the
> PCI core renumber things (typically because the FW "forgot" to set aside
> bus numbers for a cardbus slot for example, that sort of things).
>
> So it's a bit of a tricky situation.
>
> Off the top of my head, I'm pretty sure that most if not all of our PCI
> host bridges simply support a full 0...255 range and there is no sharing
> between bridges like on x86, they are just different domains.

My point is that the interface between the arch and the PCI core
should be simply the arch telling the core "this is the range of bus
numbers you can use."  If the firmware doesn't give you the HW limits,
that's the arch's problem.  If you want to assume 0..255 are
available, again, that's the arch's decision.

But the answer to the question "what bus numbers are available to me"
depends only on the host bridge HW configuration.  It does not depend
on what pci_scan_child_bus() found.  Therefore, I think we can come up
with a design where pci_bus_update_busn_res_end() is unnecessary.

Bjorn

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Benjamin Herrenschmidt]

On Thu, 2012-02-09 at 11:24 -0800, Bjorn Helgaas wrote:
> My point is that the interface between the arch and the PCI core
> should be simply the arch telling the core "this is the range of bus
> numbers you can use."  If the firmware doesn't give you the HW limits,
> that's the arch's problem.  If you want to assume 0..255 are
> available, again, that's the arch's decision.
> 
> But the answer to the question "what bus numbers are available to me"
> depends only on the host bridge HW configuration.  It does not depend
> on what pci_scan_child_bus() found.  Therefore, I think we can come up
> with a design where pci_bus_update_busn_res_end() is unnecessary.

In an ideal world yes. In a world where there are reverse engineered
platforms on which we aren't 100% sure how thing actually work under the
hood and have the code just adapt on "what's there" (and try to fix it
up -sometimes-), thinks can get a bit murky :-)

But yes, I see your point. As for what is the "correct" setting that
needs to be done so that the patch doesn't end up a regression for us,
I'll have to dig into some ancient HW to dbl check a few things. I hope
0...255 will just work but I can't guarantee it.

What I'll probably do is constraint the core to the values in
hose->min/max, and update selected platforms to put 0..255 in there when
I know for sure they can cope.

Cheers,
Ben.

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Jesse Barnes]

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

On Fri, 10 Feb 2012 08:35:58 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Thu, 2012-02-09 at 11:24 -0800, Bjorn Helgaas wrote:
> > My point is that the interface between the arch and the PCI core
> > should be simply the arch telling the core "this is the range of bus
> > numbers you can use."  If the firmware doesn't give you the HW limits,
> > that's the arch's problem.  If you want to assume 0..255 are
> > available, again, that's the arch's decision.
> > 
> > But the answer to the question "what bus numbers are available to me"
> > depends only on the host bridge HW configuration.  It does not depend
> > on what pci_scan_child_bus() found.  Therefore, I think we can come up
> > with a design where pci_bus_update_busn_res_end() is unnecessary.
> 
> In an ideal world yes. In a world where there are reverse engineered
> platforms on which we aren't 100% sure how thing actually work under the
> hood and have the code just adapt on "what's there" (and try to fix it
> up -sometimes-), thinks can get a bit murky :-)
> 
> But yes, I see your point. As for what is the "correct" setting that
> needs to be done so that the patch doesn't end up a regression for us,
> I'll have to dig into some ancient HW to dbl check a few things. I hope
> 0...255 will just work but I can't guarantee it.
> 
> What I'll probably do is constraint the core to the values in
> hose->min/max, and update selected platforms to put 0..255 in there when
> I know for sure they can cope.

But I think the point is, can't we intiialize the busn resource after
the first & last bus numbers have been determined?  E.g. rather than
Yinghai's current:
+	pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
+
 	/* Get probe mode and perform scan */
 	mode = PCI_PROBE_NORMAL;
 	if (node && ppc_md.pci_probe_mode)
@@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		of_scan_bus(node, bus);
 	}
 
-	if (mode == PCI_PROBE_NORMAL)
+	if (mode == PCI_PROBE_NORMAL) {
+		pci_bus_update_busn_res_end(bus, 255);
 		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
+		pci_bus_update_busn_res_end(bus, bus->subordinate);
+	}

we'd have something more like:

 	/* Get probe mode and perform scan */
 	mode = PCI_PROBE_NORMAL;
 	if (node && ppc_md.pci_probe_mode)
@@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
 		of_scan_bus(node, bus);
 	}
 
	if (mode == PCI_PROBE_NORMAL)
 		hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);

+	pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);

since we should have the final bus range by then?  Setting the end to
255 and then changing it again doesn't make sense; and definitely makes
the code hard to follow.

-- 
Jesse Barnes, Intel Open Source Technology Center

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

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Bjorn Helgaas]

On Thu, Feb 23, 2012 at 12:25 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Fri, 10 Feb 2012 08:35:58 +1100
> Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
>> On Thu, 2012-02-09 at 11:24 -0800, Bjorn Helgaas wrote:
>> > My point is that the interface between the arch and the PCI core
>> > should be simply the arch telling the core "this is the range of bus
>> > numbers you can use."  If the firmware doesn't give you the HW limits,
>> > that's the arch's problem.  If you want to assume 0..255 are
>> > available, again, that's the arch's decision.
>> >
>> > But the answer to the question "what bus numbers are available to me"
>> > depends only on the host bridge HW configuration.  It does not depend
>> > on what pci_scan_child_bus() found.  Therefore, I think we can come up
>> > with a design where pci_bus_update_busn_res_end() is unnecessary.
>>
>> In an ideal world yes. In a world where there are reverse engineered
>> platforms on which we aren't 100% sure how thing actually work under the
>> hood and have the code just adapt on "what's there" (and try to fix it
>> up -sometimes-), thinks can get a bit murky :-)
>>
>> But yes, I see your point. As for what is the "correct" setting that
>> needs to be done so that the patch doesn't end up a regression for us,
>> I'll have to dig into some ancient HW to dbl check a few things. I hope
>> 0...255 will just work but I can't guarantee it.
>>
>> What I'll probably do is constraint the core to the values in
>> hose->min/max, and update selected platforms to put 0..255 in there when
>> I know for sure they can cope.
>
> But I think the point is, can't we intiialize the busn resource after
> the first & last bus numbers have been determined?  E.g. rather than
> Yinghai's current:
> +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
> +
>        /* Get probe mode and perform scan */
>        mode = PCI_PROBE_NORMAL;
>        if (node && ppc_md.pci_probe_mode)
> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>                of_scan_bus(node, bus);
>        }
>
> -       if (mode == PCI_PROBE_NORMAL)
> +       if (mode == PCI_PROBE_NORMAL) {
> +               pci_bus_update_busn_res_end(bus, 255);
>                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> +               pci_bus_update_busn_res_end(bus, bus->subordinate);
> +       }
>
> we'd have something more like:
>
>        /* Get probe mode and perform scan */
>        mode = PCI_PROBE_NORMAL;
>        if (node && ppc_md.pci_probe_mode)
> @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
>                of_scan_bus(node, bus);
>        }
>
>        if (mode == PCI_PROBE_NORMAL)
>                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
>
> +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
>
> since we should have the final bus range by then?  Setting the end to
> 255 and then changing it again doesn't make sense; and definitely makes
> the code hard to follow.

I have two issues here:

1) hose->last_busno is currently the highest bus number found by
pci_scan_child_bus().  If I understand correctly,
pci_bus_insert_busn_res() is supposed to update the core's idea of the
host bridge's bus number aperture.  (Actually, I guess it just updates
the *end* of the aperture, since we supply the start directly to
pci_scan_root_bus()).  The aperture and the highest bus number we
found are not related, except that we should have:

    hose->first_busno <= bus->subordinate <= hose->last_busno

If we set the aperture to [first_busno - last_busno], we artificially
prevent some hotplug.

2) We already have a way to add resources to a root bus: the
pci_add_resource() used to add I/O port and MMIO apertures.  I think
it'd be a lot simpler to just use that same interface for the bus
number aperture, e.g.,

    pci_add_resource(&resources, hose->io_space);
    pci_add_resource(&resources, hose->mem_space);
    pci_add_resource(&resources, hose->busnr_space);
    bus = pci_scan_root_bus(dev, next_busno, pci_ops, sysdata, &resources);

This is actually a bit redundant, since "next_busno" should be the
same as hose->busnr_space->start.  So if we adopted this approach, we
might want to eventually drop the "next_busno" argument.

 Bjorn

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Jesse Barnes]

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

On Thu, 23 Feb 2012 12:51:30 -0800
Bjorn Helgaas <bhelgaas@google.com> wrote:

> On Thu, Feb 23, 2012 at 12:25 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Fri, 10 Feb 2012 08:35:58 +1100
> > Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
> >
> >> On Thu, 2012-02-09 at 11:24 -0800, Bjorn Helgaas wrote:
> >> > My point is that the interface between the arch and the PCI core
> >> > should be simply the arch telling the core "this is the range of bus
> >> > numbers you can use."  If the firmware doesn't give you the HW limits,
> >> > that's the arch's problem.  If you want to assume 0..255 are
> >> > available, again, that's the arch's decision.
> >> >
> >> > But the answer to the question "what bus numbers are available to me"
> >> > depends only on the host bridge HW configuration.  It does not depend
> >> > on what pci_scan_child_bus() found.  Therefore, I think we can come up
> >> > with a design where pci_bus_update_busn_res_end() is unnecessary.
> >>
> >> In an ideal world yes. In a world where there are reverse engineered
> >> platforms on which we aren't 100% sure how thing actually work under the
> >> hood and have the code just adapt on "what's there" (and try to fix it
> >> up -sometimes-), thinks can get a bit murky :-)
> >>
> >> But yes, I see your point. As for what is the "correct" setting that
> >> needs to be done so that the patch doesn't end up a regression for us,
> >> I'll have to dig into some ancient HW to dbl check a few things. I hope
> >> 0...255 will just work but I can't guarantee it.
> >>
> >> What I'll probably do is constraint the core to the values in
> >> hose->min/max, and update selected platforms to put 0..255 in there when
> >> I know for sure they can cope.
> >
> > But I think the point is, can't we intiialize the busn resource after
> > the first & last bus numbers have been determined?  E.g. rather than
> > Yinghai's current:
> > +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
> > +
> >        /* Get probe mode and perform scan */
> >        mode = PCI_PROBE_NORMAL;
> >        if (node && ppc_md.pci_probe_mode)
> > @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
> >                of_scan_bus(node, bus);
> >        }
> >
> > -       if (mode == PCI_PROBE_NORMAL)
> > +       if (mode == PCI_PROBE_NORMAL) {
> > +               pci_bus_update_busn_res_end(bus, 255);
> >                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> > +               pci_bus_update_busn_res_end(bus, bus->subordinate);
> > +       }
> >
> > we'd have something more like:
> >
> >        /* Get probe mode and perform scan */
> >        mode = PCI_PROBE_NORMAL;
> >        if (node && ppc_md.pci_probe_mode)
> > @@ -1742,8 +1744,11 @@ void __devinit pcibios_scan_phb(struct pci_controller *hose)
> >                of_scan_bus(node, bus);
> >        }
> >
> >        if (mode == PCI_PROBE_NORMAL)
> >                hose->last_busno = bus->subordinate = pci_scan_child_bus(bus);
> >
> > +       pci_bus_insert_busn_res(bus, hose->first_busno, hose->last_busno);
> >
> > since we should have the final bus range by then?  Setting the end to
> > 255 and then changing it again doesn't make sense; and definitely makes
> > the code hard to follow.
> 
> I have two issues here:
> 
> 1) hose->last_busno is currently the highest bus number found by
> pci_scan_child_bus().  If I understand correctly,
> pci_bus_insert_busn_res() is supposed to update the core's idea of the
> host bridge's bus number aperture.  (Actually, I guess it just updates
> the *end* of the aperture, since we supply the start directly to
> pci_scan_root_bus()).  The aperture and the highest bus number we
> found are not related, except that we should have:
> 
>     hose->first_busno <= bus->subordinate <= hose->last_busno
> 
> If we set the aperture to [first_busno - last_busno], we artificially
> prevent some hotplug.

Oh true, we'll need to allocate any extra bus number space somehow so
that hot plug of bridges is possible in the future w/o renumbering
(until our glorious future when we can move resources on the fly by
stopping drivers).

> 
> 2) We already have a way to add resources to a root bus: the
> pci_add_resource() used to add I/O port and MMIO apertures.  I think
> it'd be a lot simpler to just use that same interface for the bus
> number aperture, e.g.,
> 
>     pci_add_resource(&resources, hose->io_space);
>     pci_add_resource(&resources, hose->mem_space);
>     pci_add_resource(&resources, hose->busnr_space);
>     bus = pci_scan_root_bus(dev, next_busno, pci_ops, sysdata, &resources);
> 
> This is actually a bit redundant, since "next_busno" should be the
> same as hose->busnr_space->start.  So if we adopted this approach, we
> might want to eventually drop the "next_busno" argument.

Yeah that would be nice, the call would certainly make more sense that
way.

-- 
Jesse Barnes, Intel Open Source Technology Center

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

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Yinghai Lu]

On Fri, Feb 24, 2012 at 2:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Thu, 23 Feb 2012 12:51:30 -0800
> Bjorn Helgaas <bhelgaas@google.com> wrote:
>> 2) We already have a way to add resources to a root bus: the
>> pci_add_resource() used to add I/O port and MMIO apertures.  I think
>> it'd be a lot simpler to just use that same interface for the bus
>> number aperture, e.g.,
>>
>>     pci_add_resource(&resources, hose->io_space);
>>     pci_add_resource(&resources, hose->mem_space);
>>     pci_add_resource(&resources, hose->busnr_space);
>>     bus = pci_scan_root_bus(dev, next_busno, pci_ops, sysdata, &resources);
>>
>> This is actually a bit redundant, since "next_busno" should be the
>> same as hose->busnr_space->start.  So if we adopted this approach, we
>> might want to eventually drop the "next_busno" argument.
>
> Yeah that would be nice, the call would certainly make more sense that
> way.

no, I don't think so.

using pci_add_resource will need to create dummy resource abut bus range.

there is lots of pci_scan_root_bus(),  and those user does not bus end
yet before scan.
so could just hide pci_insert_busn_res in pci_scan_root_bus, and
update busn_res end there.

other arch like x86, ia64, powerpc, sparc, will insert exact bus range
between pci_create_root_bus and
pci_scan_child_bus, will not need to update busn_res end.

please check v7 of this patchset.

    	git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
for-pci-busn-alloc

It should be clean and have minimum lines of change.

Thanks

      Yinghai

Re: [PATCH 09/24] PCI, powerpc: Register busn_res for root buses

[Bjorn Helgaas]

On Sat, Feb 25, 2012 at 12:47 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Feb 24, 2012 at 2:24 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
>> On Thu, 23 Feb 2012 12:51:30 -0800
>> Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> 2) We already have a way to add resources to a root bus: the
>>> pci_add_resource() used to add I/O port and MMIO apertures.  I think
>>> it'd be a lot simpler to just use that same interface for the bus
>>> number aperture, e.g.,
>>>
>>>     pci_add_resource(&resources, hose->io_space);
>>>     pci_add_resource(&resources, hose->mem_space);
>>>     pci_add_resource(&resources, hose->busnr_space);
>>>     bus = pci_scan_root_bus(dev, next_busno, pci_ops, sysdata, &resources);
>>>
>>> This is actually a bit redundant, since "next_busno" should be the
>>> same as hose->busnr_space->start.  So if we adopted this approach, we
>>> might want to eventually drop the "next_busno" argument.
>>
>> Yeah that would be nice, the call would certainly make more sense that
>> way.
>
> no, I don't think so.
>
> using pci_add_resource will need to create dummy resource abut bus range.

I don't see the problem here.  The bus number aperture is a
fundamental property of a host bridge, and any firmware or native
bridge driver that tells you about a bridge but doesn't tell you the
bus number aperture is just broken.

Every arch already has struct resources for the MMIO and IO port
regions available on the bus.  ACPI already has a resource for the bus
number range.  It makes sense to me that the arch should supply a bus
number resource.

Conceptually, it's just like the MMIO and IO resources, so it makes
sense to me that the code for bus numbers should look like the code
for MMIO and IO ports.

> there is lots of pci_scan_root_bus(),  and those user does not bus end
> yet before scan.
> so could just hide pci_insert_busn_res in pci_scan_root_bus, and
> update busn_res end there.

pci_scan_child_bus() does NOT tell you the end of the bus number
aperture, and we shouldn't pretend that it does.  It might give you a
lower bound on the end of the aperture (as long as you're willing to
trust the current PCI config and you don't change anything).

> other arch like x86, ia64, powerpc, sparc, will insert exact bus range
> between pci_create_root_bus and
> pci_scan_child_bus, will not need to update busn_res end.
>
> please check v7 of this patchset.
>
>        git://git.kernel.org/pub/scm/linux/kernel/git/yinghai/linux-yinghai.git
> for-pci-busn-alloc

I looked at your git tree, but I can't tell whether what's there is v7
or not and it's too much trouble to try to figure it out.

Bjorn

[PATCH 10/24] PCI, parisc: Register busn_res for root buses

[Yinghai Lu]

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Kyle McMartin <kyle@mcmartin.ca>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-parisc@vger.kernel.org
---
 drivers/parisc/dino.c    |    2 ++
 drivers/parisc/lba_pci.c |    3 +++
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/parisc/dino.c b/drivers/parisc/dino.c
index 7ff10c1..3f2e203 100644
--- a/drivers/parisc/dino.c
+++ b/drivers/parisc/dino.c
@@ -1014,7 +1014,9 @@ static int __init dino_probe(struct parisc_device *dev)
 		return 0;
 	}
 
+	pci_bus_insert_busn_res(bus, dino_current_bus, 255);
 	bus->subordinate = pci_scan_child_bus(bus);
+	pci_bus_update_busn_res_end(bus, bus->subordinate);
 
 	/* This code *depends* on scanning being single threaded
 	 * if it isn't, this global bus number count will fail
diff --git a/drivers/parisc/lba_pci.c b/drivers/parisc/lba_pci.c
index d5f3d75..b58bf8b 100644
--- a/drivers/parisc/lba_pci.c
+++ b/drivers/parisc/lba_pci.c
@@ -1531,6 +1531,9 @@ lba_driver_probe(struct parisc_device *dev)
 		return 0;
 	}
 
+	pci_bus_insert_busn_res(lba_bus, lba_dev->hba.bus_num.start,
+				lba_dev->hba.bus_num.end);
+
 	lba_bus->subordinate = pci_scan_child_bus(lba_bus);
 
 	/* This is in lieu of calling pci_assign_unassigned_resources() */
-- 
1.7.7

[PATCH 11/24] PCI: Add pci_bus_extend/shrink_top()

[Yinghai Lu]

extend or shrink bus and parent buses top (subordinate)

extended range is verified safe range, and stop at recorded parent_res.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index cdd6e83..d2519df 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -624,6 +624,42 @@ static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max)
 	}
 }
 
+static void __devinit pci_bus_update_top(struct pci_bus *parent,
+		 long size, struct resource *parent_res)
+{
+	struct resource *res;
+
+	if (!size)
+		return;
+
+	while (parent) {
+		res = &parent->busn_res;
+		if (res == parent_res)
+			break;
+		res->end += size;
+		parent->subordinate += size;
+		pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS,
+					 parent->subordinate);
+		dev_printk(KERN_DEBUG, &parent->dev,
+				"busn_res: %s %02lx to %pR\n",
+				(size > 0) ? "extended" : "shrunk",
+				abs(size), res);
+		parent = parent->parent;
+	}
+}
+
+static void __devinit pci_bus_extend_top(struct pci_bus *parent,
+		 long size, struct resource *parent_res)
+{
+	pci_bus_update_top(parent, size, parent_res);
+}
+
+static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
+		 long size, struct resource *parent_res)
+{
+	pci_bus_update_top(parent, -size, parent_res);
+}
+
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
-- 
1.7.7

[PATCH 12/24] PCI: Probe safe range that we can use for unassigned bridge.

[Yinghai Lu]

Try to allocate from parent bus busn_res. if can not find any big enough, will try
to extend parent bus top. even the extending is through allocating, after allocating
will pad the range to parent buses top.

When extending happens, We will record the parent_res, so could use it as stopper
for really extend/shrink top later.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index d2519df..7015dcd 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -660,6 +660,116 @@ static void __devinit pci_bus_shrink_top(struct pci_bus *parent,
 	pci_bus_update_top(parent, -size, parent_res);
 }
 
+static resource_size_t __devinit find_res_top_free_size(struct resource *res)
+{
+	resource_size_t n_size;
+	struct resource tmp_res;
+
+	/*
+	 *   find out number below res->end, that we can use at first
+	 *	res->start can not be used.
+	 */
+	n_size = resource_size(res) - 1;
+	memset(&tmp_res, 0, sizeof(struct resource));
+	while (n_size > 0) {
+		int ret;
+
+		ret = allocate_resource(res, &tmp_res, n_size,
+			res->end - n_size + 1, res->end,
+			1, NULL, NULL);
+		if (ret == 0) {
+			release_resource(&tmp_res);
+			break;
+		}
+		n_size--;
+	}
+
+	return n_size;
+}
+
+static int __devinit pci_bridge_probe_busn_res(struct pci_bus *bus,
+			 struct pci_dev *dev, struct resource *busn_res,
+			 resource_size_t needed_size, struct resource **p)
+{
+	int ret = -ENOMEM;
+	resource_size_t n_size;
+	struct pci_bus *parent;
+	struct resource *parent_res = NULL;
+	resource_size_t tmp = bus->busn_res.end + 1;
+	int free_sz = -1;
+
+again:
+	/*
+	 * find bigest range in bus->busn_res that we can use in the middle
+	 *  and we can not use bus->busn_res.start.
+	 */
+	n_size = resource_size(&bus->busn_res) - 1;
+	memset(busn_res, 0, sizeof(struct resource));
+	dev_printk(KERN_DEBUG, &dev->dev,
+			"find free busn in busn_res: %pR\n", &bus->busn_res);
+	while (n_size >= needed_size) {
+		ret = allocate_resource(&bus->busn_res, busn_res, n_size,
+				bus->busn_res.start + 1, bus->busn_res.end,
+				1, NULL, NULL);
+		if (ret == 0) {
+			/* found one, prepare to return */
+			release_resource(busn_res);
+
+			return ret;
+		}
+		n_size--;
+	}
+
+	/* try extend the top of parent bus, find free under top af first */
+	if (free_sz < 0) {
+		free_sz = find_res_top_free_size(&bus->busn_res);
+		dev_printk(KERN_DEBUG, &dev->dev,
+			"found free busn %d in busn_res: %pR top\n",
+				free_sz, &bus->busn_res);
+	}
+	n_size = free_sz;
+
+	/* can not extend cross domain boundary */
+	if ((0xff - busn_bus_nr(bus->busn_res.end)) < (needed_size - n_size))
+		goto reduce_needed_size;
+
+	/* find exteded range */
+	memset(busn_res, 0, sizeof(struct resource));
+	parent = bus->parent;
+	while (parent) {
+		ret = allocate_resource(&parent->busn_res, busn_res,
+			 needed_size - n_size,
+			 tmp, tmp + needed_size - n_size - 1,
+			 1, NULL, NULL);
+		if (ret == 0)
+			break;
+		parent = parent->parent;
+	}
+
+reduce_needed_size:
+	if (ret != 0) {
+		needed_size--;
+		if (!needed_size)
+			return ret;
+
+		goto again;
+	}
+
+	/* save parent_res, we need it as stopper later */
+	parent_res = busn_res->parent;
+
+	/* prepare busn_res for return */
+	release_resource(busn_res);
+	busn_res->start -= n_size;
+
+	/* extend parent bus top*/
+	pci_bus_extend_top(bus, needed_size - n_size, parent_res);
+
+	*p = parent_res;
+
+	return ret;
+}
+
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
-- 
1.7.7

[PATCH 13/24] PCI: Strict checking of valid range for bridge

[Yinghai Lu]

children bridges busn range should be able to be allocated from parent bus range.

to avoid overlapping between sibling bridges on same bus.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 7015dcd..a860acb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -770,6 +770,31 @@ reduce_needed_size:
 	return ret;
 }
 
+static int __devinit pci_bridge_check_busn_broken(struct pci_bus *bus,
+				struct pci_dev *dev,
+				int secondary, int subordinate)
+{
+	int broken = 0;
+
+	struct resource busn_res;
+	int ret;
+
+	memset(&busn_res, 0, sizeof(struct resource));
+	dev_printk(KERN_DEBUG, &dev->dev,
+		 "check if busn %02x-%02x is in busn_res: %pR\n",
+		 secondary, subordinate, &bus->busn_res);
+	ret = allocate_resource(&bus->busn_res, &busn_res,
+			 (subordinate - secondary + 1),
+			 busn(pci_domain_nr(bus), secondary),
+			 busn(pci_domain_nr(bus), subordinate),
+			 1, NULL, NULL);
+	if (ret)
+		broken = 1;
+	else
+		release_resource(&busn_res);
+
+	return broken;
+}
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
-- 
1.7.7

[PATCH 14/24] PCI: Allocate bus range instead of use max blindly

[Yinghai Lu]

every bus have extra busn_res, and linked them toghter to iobusn_resource.

when need to find usable bus number range, try probe from iobusn_resource tree.

To avoid falling to small hole in the middle, we try from 8 spare bus.
if can not find 8 or more in the middle, will try to append 8 on top later.
then if can not append, will try to find 7 from the middle, then will try to append 7 on top.
then if can not append, will try to find 6 from the middle...

for cardbus will only find 4 spare.

if extend from top, at last will shrink back to really needed range...

-v4: fix checking with pci rescan. Found by Bjorn.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |  101 ++++++++++++++++++++++++++++++---------------------
 1 files changed, 60 insertions(+), 41 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a860acb..c4ed9cb 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -809,10 +809,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 {
 	struct pci_bus *child;
 	int is_cardbus = (dev->hdr_type == PCI_HEADER_TYPE_CARDBUS);
-	u32 buses, i, j = 0;
+	u32 buses;
 	u16 bctl;
 	u8 primary, secondary, subordinate;
 	int broken = 0;
+	struct resource *parent_res = NULL;
 
 	pci_read_config_dword(dev, PCI_PRIMARY_BUS, &buses);
 	primary = buses & 0xFF;
@@ -824,10 +825,16 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 
 	/* Check if setup is sensible at all */
 	if (!pass &&
-	    (primary != bus->number || secondary <= bus->number)) {
-		dev_dbg(&dev->dev, "bus configuration invalid, reconfiguring\n");
+	    (primary != bus->number || secondary <= bus->number))
 		broken = 1;
-	}
+
+	/* more strict checking */
+	if (!pass && !broken && !dev->subordinate)
+		broken = pci_bridge_check_busn_broken(bus, dev,
+						   secondary, subordinate);
+
+	if (broken)
+		dev_dbg(&dev->dev, "bus configuration invalid, reconfiguring\n");
 
 	/* Disable MasterAbortMode during probing to avoid reporting
 	   of bus errors (in some architectures) */ 
@@ -860,6 +867,8 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			child->primary = primary;
 			child->subordinate = subordinate;
 			child->bridge_ctl = bctl;
+
+			pci_bus_insert_busn_res(child, secondary, subordinate);
 		}
 
 		cmax = pci_scan_child_bus(child);
@@ -872,6 +881,11 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 		 * We need to assign a number to this bus which we always
 		 * do in the second pass.
 		 */
+		long shrink_size;
+		struct resource busn_res;
+		int ret = -ENOMEM;
+		int old_max = max;
+
 		if (!pass) {
 			if (pcibios_assign_all_busses() || broken)
 				/* Temporarily disable forwarding of the
@@ -888,20 +902,44 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 		/* Clear errors */
 		pci_write_config_word(dev, PCI_STATUS, 0xffff);
 
-		/* Prevent assigning a bus number that already exists.
-		 * This can happen when a bridge is hot-plugged, so in
-		 * this case we only re-scan this bus. */
-		child = pci_find_bus(pci_domain_nr(bus), max+1);
-		if (!child) {
-			child = pci_add_new_bus(bus, dev, ++max);
-			if (!child)
-				goto out;
+		if (dev->subordinate) {
+			/* We get here only for cardbus */
+			child = dev->subordinate;
+			if  (!is_cardbus)
+				dev_warn(&dev->dev,
+				 "rescan scaned bridge as broken one again ?");
+
+			goto out;
 		}
+		/*
+		 * For CardBus bridges, we leave 4 bus numbers
+		 * as cards with a PCI-to-PCI bridge can be
+		 * inserted later.
+		 * other just allocate 8 bus to avoid we fall into
+		 *  small hole in the middle.
+		 */
+		ret = pci_bridge_probe_busn_res(bus, dev, &busn_res,
+				is_cardbus ? (CARDBUS_RESERVE_BUSNR + 1) : 8,
+				&parent_res);
+
+		if (ret != 0)
+			goto out;
+
+		child = pci_add_new_bus(bus, dev, busn_bus_nr(busn_res.start));
+		if (!child)
+			goto out;
+
+		child->subordinate = busn_bus_nr(busn_res.end);
+		pci_bus_insert_busn_res(child, busn_bus_nr(busn_res.start),
+					busn_bus_nr(busn_res.end));
+
 		buses = (buses & 0xff000000)
 		      | ((unsigned int)(child->primary)     <<  0)
 		      | ((unsigned int)(child->secondary)   <<  8)
 		      | ((unsigned int)(child->subordinate) << 16);
 
+		max = child->subordinate;
+
 		/*
 		 * yenta.c forces a secondary latency timer of 176.
 		 * Copy that behaviour here.
@@ -932,43 +970,24 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			 * the real value of max.
 			 */
 			pci_fixup_parent_subordinate_busnr(child, max);
+
 		} else {
-			/*
-			 * For CardBus bridges, we leave 4 bus numbers
-			 * as cards with a PCI-to-PCI bridge can be
-			 * inserted later.
-			 */
-			for (i=0; i<CARDBUS_RESERVE_BUSNR; i++) {
-				struct pci_bus *parent = bus;
-				if (pci_find_bus(pci_domain_nr(bus),
-							max+i+1))
-					break;
-				while (parent->parent) {
-					if ((!pcibios_assign_all_busses()) &&
-					    (parent->subordinate > max) &&
-					    (parent->subordinate <= max+i)) {
-						j = 1;
-					}
-					parent = parent->parent;
-				}
-				if (j) {
-					/*
-					 * Often, there are two cardbus bridges
-					 * -- try to leave one valid bus number
-					 * for each one.
-					 */
-					i /= 2;
-					break;
-				}
-			}
-			max += i;
 			pci_fixup_parent_subordinate_busnr(child, max);
 		}
 		/*
 		 * Set the subordinate bus number to its real value.
 		 */
+		shrink_size = child->subordinate - max;
 		child->subordinate = max;
 		pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
+		pci_bus_update_busn_res_end(child, max);
+
+		/* shrink some back, if we extend top before */
+		if (!is_cardbus && (shrink_size > 0) && parent_res)
+			pci_bus_shrink_top(bus, shrink_size, parent_res);
+
+		if (old_max > max)
+			max = old_max;
 	}
 
 	sprintf(child->name,
-- 
1.7.7

[PATCH 15/24] PCI: kill pci_fixup_parent_subordinate_busnr()

[Yinghai Lu]

Now we can safely extend parent top and shrink them according iobusn_resource tree.

Don't need that any more.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c4ed9cb..0aa5693 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -608,22 +608,6 @@ struct pci_bus *__ref pci_add_new_bus(struct pci_bus *parent, struct pci_dev *de
 	return child;
 }
 
-static void pci_fixup_parent_subordinate_busnr(struct pci_bus *child, int max)
-{
-	struct pci_bus *parent = child->parent;
-
-	/* Attempts to fix that up are really dangerous unless
-	   we're going to re-assign all bus numbers. */
-	if (!pcibios_assign_all_busses())
-		return;
-
-	while (parent->parent && parent->subordinate < max) {
-		parent->subordinate = max;
-		pci_write_config_byte(parent->self, PCI_SUBORDINATE_BUS, max);
-		parent = parent->parent;
-	}
-}
-
 static void __devinit pci_bus_update_top(struct pci_bus *parent,
 		 long size, struct resource *parent_res)
 {
@@ -956,23 +940,7 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 
 		if (!is_cardbus) {
 			child->bridge_ctl = bctl;
-			/*
-			 * Adjust subordinate busnr in parent buses.
-			 * We do this before scanning for children because
-			 * some devices may not be detected if the bios
-			 * was lazy.
-			 */
-			pci_fixup_parent_subordinate_busnr(child, max);
-			/* Now we can scan all subordinate buses... */
 			max = pci_scan_child_bus(child);
-			/*
-			 * now fix it up again since we have found
-			 * the real value of max.
-			 */
-			pci_fixup_parent_subordinate_busnr(child, max);
-
-		} else {
-			pci_fixup_parent_subordinate_busnr(child, max);
 		}
 		/*
 		 * Set the subordinate bus number to its real value.
-- 
1.7.7

[PATCH 16/24] PCI: Seperate child bus scanning to two passes overall

[Yinghai Lu]

In extreme case: Two bridges are properly setup.
       bridge A
               bridge AA
               bridge AB
       bridge B
	       bridge BA
	       bridge BB
   but AA, AB are not setup properly.
   bridge A has small range, and bridge AB could need more, when do the
       first pass0 for bridge A, it will do pass0 and pass1 for AA and AB,
	during that process, it will extend range of A for AB blindly.
	because bridge B is not registered yet.
       that could overlap range that is used by bridge B.

Right way should be:
       do pass0 for all good bridges at first.
So we could do pass0 for bridge B before pass1 for bridge AB.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/probe.c |   50 ++++++++++++++++++++++++++++++++++----------------
 1 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 0aa5693..2117364 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -779,6 +779,9 @@ static int __devinit pci_bridge_check_busn_broken(struct pci_bus *bus,
 
 	return broken;
 }
+
+static unsigned int __devinit __pci_scan_child_bus(struct pci_bus *bus,
+						 int pass);
 /*
  * If it's a bridge, configure it and scan the bus behind it.
  * For CardBus bridges, we don't scan behind as the devices will
@@ -830,11 +833,10 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	    !is_cardbus && !broken) {
 		unsigned int cmax;
 		/*
-		 * Bus already configured by firmware, process it in the first
-		 * pass and just note the configuration.
+		 * Bus already configured by firmware, still process it in two
+		 * passes in extreme case like two adjaced bridges have children
+		 * bridges that are not setup properly.
 		 */
-		if (pass)
-			goto out;
 
 		/*
 		 * If we already got to this bus through a different bridge,
@@ -855,7 +857,7 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 			pci_bus_insert_busn_res(child, secondary, subordinate);
 		}
 
-		cmax = pci_scan_child_bus(child);
+		cmax = __pci_scan_child_bus(child, pass);
 		if (cmax > max)
 			max = cmax;
 		if (child->subordinate > max)
@@ -1651,12 +1653,13 @@ void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
 }
 EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
 
-unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
+static unsigned int __devinit __pci_scan_child_bus(struct pci_bus *bus,
+						 int pass)
 {
-	unsigned int devfn, pass, max = bus->secondary;
+	unsigned int devfn, max = bus->secondary;
 	struct pci_dev *dev;
 
-	dev_dbg(&bus->dev, "scanning bus\n");
+	dev_dbg(&bus->dev, "scanning bus pass %d\n", pass);
 
 	/* Go find them, Rover! */
 	for (devfn = 0; devfn < 0x100; devfn += 8)
@@ -1670,18 +1673,16 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 	 * all PCI-to-PCI bridges on this bus.
 	 */
 	if (!bus->is_added) {
-		dev_dbg(&bus->dev, "fixups for bus\n");
+		dev_dbg(&bus->dev, "fixups for bus pass %d\n", pass);
 		pcibios_fixup_bus(bus);
 		if (pci_is_root_bus(bus))
 			bus->is_added = 1;
 	}
 
-	for (pass=0; pass < 2; pass++)
-		list_for_each_entry(dev, &bus->devices, bus_list) {
-			if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
-			    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
-				max = pci_scan_bridge(bus, dev, max, pass);
-		}
+	list_for_each_entry(dev, &bus->devices, bus_list)
+		if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE ||
+		    dev->hdr_type == PCI_HEADER_TYPE_CARDBUS)
+			max = pci_scan_bridge(bus, dev, max, pass);
 
 	/*
 	 * We've scanned the bus and so we know all about what's on
@@ -1690,7 +1691,24 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
 	 *
 	 * Return how far we've got finding sub-buses.
 	 */
-	dev_dbg(&bus->dev, "bus scan returning with max=%02x\n", max);
+	dev_dbg(&bus->dev, "bus scan returning with max=%02x pass %d\n",
+			max, pass);
+
+	return max;
+}
+
+unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus)
+{
+	int pass;
+	unsigned int max = 0, tmp;
+
+	for (pass = 0; pass < 2;  pass++) {
+		tmp = __pci_scan_child_bus(bus, pass);
+
+		if (tmp > max)
+			max = tmp;
+	}
+
 	return max;
 }
 
-- 
1.7.7

[PATCH 17/24] pcmcia: remove workaround for fixing pci parent bus subordinate

[Yinghai Lu]

Now pci busn allocation code is there, and it will preallocate bus number and it
will make sure parent buses subordinate is right.

So remove workaround here.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Mauro Carvalho Chehab <mchehab@redhat.com>
Cc: Dominik Brodowski <linux@dominikbrodowski.net>
Cc: linux-pcmcia@lists.infradead.org
---
 drivers/pcmcia/yenta_socket.c |   75 -----------------------------------------
 1 files changed, 0 insertions(+), 75 deletions(-)

diff --git a/drivers/pcmcia/yenta_socket.c b/drivers/pcmcia/yenta_socket.c
index 849c0c1..5757cae 100644
--- a/drivers/pcmcia/yenta_socket.c
+++ b/drivers/pcmcia/yenta_socket.c
@@ -1064,79 +1064,6 @@ static void yenta_config_init(struct yenta_socket *socket)
 	config_writew(socket, CB_BRIDGE_CONTROL, bridge);
 }
 
-/**
- * yenta_fixup_parent_bridge - Fix subordinate bus# of the parent bridge
- * @cardbus_bridge: The PCI bus which the CardBus bridge bridges to
- *
- * Checks if devices on the bus which the CardBus bridge bridges to would be
- * invisible during PCI scans because of a misconfigured subordinate number
- * of the parent brige - some BIOSes seem to be too lazy to set it right.
- * Does the fixup carefully by checking how far it can go without conflicts.
- * See http://bugzilla.kernel.org/show_bug.cgi?id=2944 for more information.
- */
-static void yenta_fixup_parent_bridge(struct pci_bus *cardbus_bridge)
-{
-	struct list_head *tmp;
-	unsigned char upper_limit;
-	/*
-	 * We only check and fix the parent bridge: All systems which need
-	 * this fixup that have been reviewed are laptops and the only bridge
-	 * which needed fixing was the parent bridge of the CardBus bridge:
-	 */
-	struct pci_bus *bridge_to_fix = cardbus_bridge->parent;
-
-	/* Check bus numbers are already set up correctly: */
-	if (bridge_to_fix->subordinate >= cardbus_bridge->subordinate)
-		return; /* The subordinate number is ok, nothing to do */
-
-	if (!bridge_to_fix->parent)
-		return; /* Root bridges are ok */
-
-	/* stay within the limits of the bus range of the parent: */
-	upper_limit = bridge_to_fix->parent->subordinate;
-
-	/* check the bus ranges of all silbling bridges to prevent overlap */
-	list_for_each(tmp, &bridge_to_fix->parent->children) {
-		struct pci_bus *silbling = pci_bus_b(tmp);
-		/*
-		 * If the silbling has a higher secondary bus number
-		 * and it's secondary is equal or smaller than our
-		 * current upper limit, set the new upper limit to
-		 * the bus number below the silbling's range:
-		 */
-		if (silbling->secondary > bridge_to_fix->subordinate
-		    && silbling->secondary <= upper_limit)
-			upper_limit = silbling->secondary - 1;
-	}
-
-	/* Show that the wanted subordinate number is not possible: */
-	if (cardbus_bridge->subordinate > upper_limit)
-		dev_printk(KERN_WARNING, &cardbus_bridge->dev,
-			   "Upper limit for fixing this "
-			   "bridge's parent bridge: #%02x\n", upper_limit);
-
-	/* If we have room to increase the bridge's subordinate number, */
-	if (bridge_to_fix->subordinate < upper_limit) {
-
-		/* use the highest number of the hidden bus, within limits */
-		unsigned char subordinate_to_assign =
-			min(cardbus_bridge->subordinate, upper_limit);
-
-		dev_printk(KERN_INFO, &bridge_to_fix->dev,
-			   "Raising subordinate bus# of parent "
-			   "bus (#%02x) from #%02x to #%02x\n",
-			   bridge_to_fix->number,
-			   bridge_to_fix->subordinate, subordinate_to_assign);
-
-		/* Save the new subordinate in the bus struct of the bridge */
-		bridge_to_fix->subordinate = subordinate_to_assign;
-
-		/* and update the PCI config space with the new subordinate */
-		pci_write_config_byte(bridge_to_fix->self,
-			PCI_SUBORDINATE_BUS, bridge_to_fix->subordinate);
-	}
-}
-
 /*
  * Initialize a cardbus controller. Make sure we have a usable
  * interrupt, and that we can map the cardbus area. Fill in the
@@ -1257,8 +1184,6 @@ static int __devinit yenta_probe(struct pci_dev *dev, const struct pci_device_id
 	dev_printk(KERN_INFO, &dev->dev,
 		   "Socket status: %08x\n", cb_readl(socket, CB_SOCKET_STATE));
 
-	yenta_fixup_parent_bridge(dev->subordinate);
-
 	/* Register it with the pcmcia layer.. */
 	ret = pcmcia_register_socket(&socket->socket);
 	if (ret == 0) {
-- 
1.7.7

[PATCH 18/24] PCI: Double checking setting for bus register and bus struct.

[Yinghai Lu]

User could use setpci change bridge's bus register. that could make value of register
and struct is out of sync.
User later will use rescan to see the devices is moving.

In the rescaning, we need to double check the range and remove the old struct at first.

to make thing working user may need have script to remove children devices under bridge
at first, and then use setpci update bus register and then rescan.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2117364..b0c2aa1 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -815,6 +815,46 @@ int __devinit pci_scan_bridge(struct pci_bus *bus, struct pci_dev *dev, int max,
 	    (primary != bus->number || secondary <= bus->number))
 		broken = 1;
 
+	if (!pass && dev->subordinate) {
+		child = dev->subordinate;
+		/*
+		 * User could change bus register in bridge manually with
+		 * setpci and rescan. So double check the setting, and remove
+		 * old structs.  Don't set broken yet, let following check
+		 * to see if the new setting good.
+		 */
+		if (primary != child->primary ||
+		    secondary != child->secondary ||
+		    subordinate != child->subordinate) {
+			dev_info(&dev->dev,
+				"someone changed bus register from pri:%02x, sec:%02x, sub:%02x to pri:%02x, sec:%02x, sub:%02x\n",
+				child->primary, child->secondary, child->subordinate,
+				primary, secondary, subordinate);
+			if (!list_empty(&dev->subordinate->devices)) {
+				u32 old_buses;
+
+				dev_info(&dev->dev,
+				 "but children devices are not removed manually before that.\n");
+				/*
+				 * Try best to remove left children devices
+				 * but we need to set bus register back, otherwise
+				 * We can not access children device and stop them
+				 */
+				old_buses = (buses & 0xff000000)
+				   | ((unsigned int)(child->primary)     <<  0)
+				   | ((unsigned int)(child->secondary)   <<  8)
+				   | ((unsigned int)(child->subordinate) << 16);
+				pci_write_config_dword(dev, PCI_PRIMARY_BUS,
+							 old_buses);
+				pci_remove_behind_bridge(dev);
+				pci_write_config_dword(dev, PCI_PRIMARY_BUS,
+							 buses);
+			}
+			pci_remove_bus(dev->subordinate);
+			dev->subordinate = NULL;
+		}
+	}
+
 	/* more strict checking */
 	if (!pass && !broken && !dev->subordinate)
 		broken = pci_bridge_check_busn_broken(bus, dev,
-- 
1.7.7

[PATCH 19/24] PCI, pciehp: Remove not needed bus number range checking

[Yinghai Lu]

Found hotplug adding one EM with bridge fail, bios only leave one bus range
 for slot.

[ 1169.621444] pciehp: No bus number available for hot-added bridge 0000:55:00.0
[ 1169.633277] pcieport 0000:40:03.0: PCI bridge to [bus 55-55]

With busn_res tracking and allocating, we don't need that checking anymore.

Parent bridges' bus number will be extended safely.

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

diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
index a4031df..df1f3ed 100644
--- a/drivers/pci/hotplug/pciehp_pci.c
+++ b/drivers/pci/hotplug/pciehp_pci.c
@@ -37,18 +37,8 @@
 static int __ref pciehp_add_bridge(struct pci_dev *dev)
 {
 	struct pci_bus *parent = dev->bus;
-	int pass, busnr, start = parent->secondary;
-	int end = parent->subordinate;
+	int pass, busnr = parent->secondary;
 
-	for (busnr = start; busnr <= end; busnr++) {
-		if (!pci_find_bus(pci_domain_nr(parent), busnr))
-			break;
-	}
-	if (busnr-- > end) {
-		err("No bus number available for hot-added bridge %s\n",
-				pci_name(dev));
-		return -1;
-	}
 	for (pass = 0; pass < 2; pass++)
 		busnr = pci_scan_bridge(parent, dev, busnr, pass);
 	if (!dev->subordinate)
-- 
1.7.7

[PATCH 20/24] PCI, sys: Use device_type and attr_groups with pci dev

[Yinghai Lu]

We want to create rescan in sys only for pci bridge instead of all pci dev.

We could use attribute_groups/is_visible method to do that.

Now pci dev does not use device type yet. So add pci_dev_type to take
attr_groups with it.

Add pci_dev_bridge_attrs_are_visible() to control attr_bridge_group only create
attr for bridge.

This is the framework related change, later could attr to bridge_attr_group,
to make those attr only show up on pci bridge device.

Also We could add more attr groups with is_visible to reduce messness in
	pci_create_sysfs_dev_files. ( at least for boot_vga one )

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/pci-sysfs.c |   30 ++++++++++++++++++++++++++++++
 drivers/pci/pci.h       |    1 +
 drivers/pci/probe.c     |    1 +
 3 files changed, 32 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index d972303..124e826 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -1335,3 +1335,33 @@ static int __init pci_sysfs_init(void)
 }
 
 late_initcall(pci_sysfs_init);
+
+static struct attribute *pci_dev_bridge_attrs[] = {
+	NULL,
+};
+
+static umode_t pci_dev_bridge_attrs_are_visible(struct kobject *kobj,
+						struct attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (!pdev->subordinate)
+		return 0;
+
+	return a->mode;
+}
+
+static struct attribute_group pci_dev_bridge_attr_group = {
+	.attrs = pci_dev_bridge_attrs,
+	.is_visible = pci_dev_bridge_attrs_are_visible,
+};
+
+static const struct attribute_group *pci_dev_attr_groups[] = {
+	&pci_dev_bridge_attr_group,
+	NULL,
+};
+
+struct device_type pci_dev_type = {
+	.groups = pci_dev_attr_groups,
+};
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index f862a5a..8d17a1c 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -161,6 +161,7 @@ static inline int pci_no_d1d2(struct pci_dev *dev)
 }
 extern struct device_attribute pci_dev_attrs[];
 extern struct device_attribute pcibus_dev_attrs[];
+extern struct device_type pci_dev_type;
 #ifdef CONFIG_HOTPLUG
 extern struct bus_attribute pci_bus_attrs[];
 #else
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b0c2aa1..61df44d 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1102,6 +1102,7 @@ int pci_setup_device(struct pci_dev *dev)
 	dev->sysdata = dev->bus->sysdata;
 	dev->dev.parent = dev->bus->bridge;
 	dev->dev.bus = &pci_bus_type;
+	dev->dev.type = &pci_dev_type;
 	dev->hdr_type = hdr_type & 0x7f;
 	dev->multifunction = !!(hdr_type & 0x80);
 	dev->error_state = pci_channel_io_normal;
-- 
1.7.7

[PATCH 21/24] PCI, sysfs: create rescan_bridge under /sys/.../pci/devices/... for pci bridges

[Yinghai Lu]

Current code will create rescan for every pci device under parent bus.
that is not right. the device is already there, there is no reason to rescan it.

We could have rescan for pci bridges. less confusing.

Need to move rescan attr to pci dev bridge attribute group.
And We should rescan bridge's secondary bus instead of primary bus.

-v3: Use device_type for pci dev.
-v4: Seperate pci device type change out
-v5: add rescan_bridge for bridge type, and still keep the old rescan.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 Documentation/ABI/testing/sysfs-bus-pci |   10 ++++++++++
 drivers/pci/pci-sysfs.c                 |   24 ++++++++++++++++++++++++
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci
index 34f5110..95f0f37 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci
+++ b/Documentation/ABI/testing/sysfs-bus-pci
@@ -111,6 +111,16 @@ Description:
 		from this part of the device tree.
 		Depends on CONFIG_HOTPLUG.
 
+What:		/sys/bus/pci/devices/.../rescan_bridge
+Date:		February 2012
+Contact:	Linux PCI developers <linux-pci@vger.kernel.org>
+Description:
+		Writing a non-zero value to this attribute will
+		force a rescan of the bridge and all child buses, and
+		re-discover devices removed earlier from this part of
+		the device tree.
+		Depends on CONFIG_HOTPLUG.
+
 What:		/sys/bus/pci/devices/.../reset
 Date:		July 2009
 Contact:	Michael S. Tsirkin <mst@redhat.com>
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 124e826..68caf29 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -325,6 +325,27 @@ dev_rescan_store(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+static ssize_t
+dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
+		 const char *buf, size_t count)
+{
+	unsigned long val;
+	struct pci_dev *pdev = to_pci_dev(dev);
+
+	if (kstrtoul(buf, 0, &val) < 0)
+		return -EINVAL;
+
+	if (val) {
+		mutex_lock(&pci_remove_rescan_mutex);
+		pci_rescan_bus(pdev->subordinate);
+		mutex_unlock(&pci_remove_rescan_mutex);
+	}
+	return count;
+}
+
+static struct device_attribute pci_dev_bridge_rescan_attr =
+	__ATTR(rescan_bridge, (S_IWUSR|S_IWGRP), NULL, dev_bridge_rescan_store);
+
 static void remove_callback(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
@@ -1337,6 +1358,9 @@ static int __init pci_sysfs_init(void)
 late_initcall(pci_sysfs_init);
 
 static struct attribute *pci_dev_bridge_attrs[] = {
+#ifdef CONFIG_HOTPLUG
+	&pci_dev_bridge_rescan_attr.attr,
+#endif
 	NULL,
 };
 
-- 
1.7.7

[PATCH 22/24] PCI: Add pci_bus_add_single_device()

[Yinghai Lu]

will use it to pci_bus_bridge_scan_resize(0 to make bridge will
have pci_bus directory created.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/pci/bus.c   |   40 ++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h |    1 +
 2 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 398f5d8..7b2ae85 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -247,6 +247,46 @@ void pci_bus_add_devices(const struct pci_bus *bus)
 	}
 }
 
+void pci_bus_add_single_device(struct pci_dev *dev)
+{
+	struct pci_bus *child;
+	int retval;
+
+	/* Skip already-added devices */
+	if (!dev->is_added) {
+		retval = pci_bus_add_device(dev);
+		if (retval)
+			dev_err(&dev->dev, "Error adding device, continuing\n");
+	}
+
+	BUG_ON(!dev->is_added);
+
+	child = dev->subordinate;
+	/*
+	 * If there is an unattached subordinate bus, attach
+	 * it and then scan for unattached PCI devices.
+	 */
+	if (child) {
+		if (list_empty(&child->node)) {
+			down_write(&pci_bus_sem);
+			list_add_tail(&child->node, &dev->bus->children);
+			up_write(&pci_bus_sem);
+		}
+		pci_bus_add_devices(child);
+
+		/*
+		 * register the bus with sysfs as the parent is now
+		 * properly registered.
+		 */
+		if (!child->is_added) {
+			retval = pci_bus_add_child(child);
+			if (retval)
+				dev_err(&dev->dev, "Error adding bus, continuing\n");
+		}
+	}
+}
+
+
 void pci_enable_bridges(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d5b6786..8061031 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -659,6 +659,7 @@ void pci_fixup_cardbus(struct pci_bus *);
 void pcibios_scan_specific_bus(int busn);
 extern struct pci_bus *pci_find_bus(int domain, int busnr);
 void pci_bus_add_devices(const struct pci_bus *bus);
+void pci_bus_add_single_device(struct pci_dev *dev);
 struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus,
 				      struct pci_ops *ops, void *sysdata);
 struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata);
-- 
1.7.7

[PATCH 23/24] PCI: make pci_rescan_bus_bridge_resize use pci_scan_bridge instead

[Yinghai Lu]

So after remove all children and then using setpci change bus register and rescan
bridge could use new set bus number.

Otherwise need to rescan parent bus, it would have too much overhead.

also need to use pci_bus_add_single_device to make sure new change bus have directory
 /sys/../.../pci_bus.

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

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 61df44d..e85b823 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1972,14 +1972,16 @@ EXPORT_SYMBOL(pci_scan_bus);
  */
 unsigned int __ref pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
 {
-	unsigned int max;
-	struct pci_bus *bus = bridge->subordinate;
+	unsigned int max = 0;
+	int pass;
+	struct pci_bus *bus = bridge->bus;
 
-	max = pci_scan_child_bus(bus);
+	for (pass = 0; pass < 2; pass++)
+		max = pci_scan_bridge(bus, bridge, max, pass);
 
 	pci_assign_unassigned_bridge_resources(bridge);
 
-	pci_bus_add_devices(bus);
+	pci_bus_add_single_device(bridge);
 
 	return max;
 }
-- 
1.7.7

[PATCH 24/24] PCI: clean up rescan_bus_bridge_resize

[Yinghai Lu]

Should only use it with bridge instead of bus.
it could get new subordinate. so can not use it with reguar bus.

in that case, use may need to make all children devices get removed already.

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

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 68caf29..a3fa64e 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -337,7 +337,7 @@ dev_bridge_rescan_store(struct device *dev, struct device_attribute *attr,
 
 	if (val) {
 		mutex_lock(&pci_remove_rescan_mutex);
-		pci_rescan_bus(pdev->subordinate);
+		pci_rescan_bus_bridge_resize(pdev);
 		mutex_unlock(&pci_remove_rescan_mutex);
 	}
 	return count;
@@ -387,10 +387,7 @@ dev_bus_rescan_store(struct device *dev, struct device_attribute *attr,
 
 	if (val) {
 		mutex_lock(&pci_remove_rescan_mutex);
-		if (!pci_is_root_bus(bus) && list_empty(&bus->devices))
-			pci_rescan_bus_bridge_resize(bus->self);
-		else
-			pci_rescan_bus(bus);
+		pci_rescan_bus(bus);
 		mutex_unlock(&pci_remove_rescan_mutex);
 	}
 	return count;
-- 
1.7.7