pci_bus_for_each_resource, transparent bridges and rsrc_nonstatic.c

[Dominik Brodowski]

Bjorn,

On Mon, Mar 22, 2010 at 09:34:28AM -0700, Bjorn Helgaas wrote:
> On Monday 22 March 2010 06:00:41 am Dominik Brodowski wrote:
> > Komuro reports the following issue:
> > 
> > 00:1e.0 PCI bridge: Intel Corporation 82801 Mobile PCI Bridge (rev 93) (prog-if 01 [Subtractive decode])
> > ...
> > 	Bus: primary=00, secondary=04, subordinate=05, sec-latency=32
> > 	I/O behind bridge: 00001000-00001fff
> > 	Memory behind bridge: d2000000-d40fffff
> > 	Prefetchable memory behind bridge: 00000000d0000000-00000000d1ffffff
> > 
> > 04:06.0 CardBus bridge: Ricoh Co Ltd RL5c476 II (rev b6)
> > 
> > Iterating over the bus (4) resources used to report just IO 0x1000-0x1ffff,
> > MEM d2...-d4..., prefetch d0...-d1..., and that's exactly what we need in
> > drivers/pcmcia/rsrc_nonstatic.c . However, the newly added
> > pci_bus_for_each_resource() seems to return _also_ additional resources,
> > which we need to exclude in drivers/pcmcia/rssrc_nonstatic.c . Any ideas on
> > how to fix this issue?
> 
> This was probably broken by 2fe2abf896c1 or maybe 89a74ecccd1f7, but
> I don't see how yet.
> 
> Before those changes, we had this:
> 
>   #define PCI_BUS_NUM_RESOURCES  16
>   static int nonstatic_autoadd_resources(struct pcmcia_socket *s)
>       ...
>       for (i = 0; i < PCI_BUS_NUM_RESOURCES; i++) {
>           struct resource *root = socket->dev->bus->resource[i];
> 
> so we used to iterate over all 16 possible bus resources, not just four.
> 
> Can you please collect a dmesg log from b16694f70c (just before my
> changes) and another from 7bc5e3f2be32 (just after) so we can compare
> them?
> 
> You can open a kernel bugzilla, assign it to me, and attach the logs
> there, if you want.

Actually, I now think the bug was introduced somewhere else.

We did indeed iterate over all 16 bus resources, so also for the root PCI
bridge resources. However, this used to just skip in the next check:

if (res == &ioport_resource)
	continue;

(or &iomem_resource) as there used to be no specific root I/O / Memory
window resource set. But now, all of the I/O address space is split up into
two, and a (previously non-existing) "PCI Bus 0000:00" appears in
/proc/iomem . Do you know, by chance, which change or patchset specifically
introduced this?

> > > yenta_cardbus 0000:04:06.0: pcmcia: parent PCI bridge I/O window: 0x0 - 0xcf7
> > > yenta_cardbus 0000:04:06.0: pcmcia: parent PCI bridge I/O window: 0xd00 - 0xffff

with just a very small hole in between...

So, what should we do? 

(1) The best way seems to be to skip all resources of type 
PCI_SUBTRACTIVE_DECODE, but this flag is only stored in
struct pci_bus_resource, which is hard to access, and nonexistent for the
first four resources. Actually, the "flag" field is unused otherwise. Could
we set res->flags |= PCI_SUBTRACTIVE_DECODE instead? (it probably needs to
be != 0x1 then, but anyways). Then it's trivial in rsrc_nonstatic.c

(2) An alternative would be to 

diff --git a/drivers/pcmcia/rsrc_nonstatic.c b/drivers/pcmcia/rsrc_nonstatic.c
index ba3a53e..b0e60a1 100644
--- a/drivers/pcmcia/rsrc_nonstatic.c
+++ b/drivers/pcmcia/rsrc_nonstatic.c
@@ -933,7 +933,8 @@ static int nonstatic_autoadd_resources(struct pcmcia_socket *s)
 		return -EINVAL;
 #endif
 
-	pci_bus_for_each_resource(s->cb_dev->bus, res, i) {
+	for (i = 0; i < PCI_BRIDGE_RESOURCE_NUM; i++) {
+		res = s->cb_dev->bus->resource[i];
 		if (!res)
 			continue;
 


What do you think?

Best,
	Dominik