All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI driver to use insert_resource_conflict() to claim resources
@ 2011-07-28 11:28 Deng-Cheng Zhu
  2011-07-28 11:28 ` [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate Deng-Cheng Zhu
  2011-07-28 11:28 ` [PATCH 2/2] kernel/resource: enrich the comment for insert_resource_conflict() Deng-Cheng Zhu
  0 siblings, 2 replies; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-07-28 11:28 UTC (permalink / raw)
  To: jbarnes, torvalds
  Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dengcheng.zhu

Use insert_resource_conflict() instead of request_resource_conflict() in
pci_claim_resource() to work with the conflict resource which can be the parent
of the resource to be claimed.

Deng-Cheng Zhu (2):
  PCI: make pci_claim_resource() work with conflict resources as
    appropriate
  kernel/resource: enrich the comment for insert_resource_conflict()

 drivers/pci/setup-res.c |    2 +-
 kernel/resource.c       |    4 +++-
 2 files changed, 4 insertions(+), 2 deletions(-)


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

* [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-28 11:28 [PATCH 0/2] PCI driver to use insert_resource_conflict() to claim resources Deng-Cheng Zhu
@ 2011-07-28 11:28 ` Deng-Cheng Zhu
  2011-07-28 11:53   ` Ralf Baechle
  2011-07-28 15:59   ` Bjorn Helgaas
  2011-07-28 11:28 ` [PATCH 2/2] kernel/resource: enrich the comment for insert_resource_conflict() Deng-Cheng Zhu
  1 sibling, 2 replies; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-07-28 11:28 UTC (permalink / raw)
  To: jbarnes, torvalds
  Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dengcheng.zhu

In resolving a network driver issue with the MIPS Malta platform, the root
cause was traced into pci_claim_resource():

MIPS System Controller's PCI I/O resources stay in 0x1000-0xffffff. When
PCI quirks start claiming resources using request_resource_conflict(),
collisions happen and -EBUSY is returned, thereby rendering the onboard AMD
PCnet32 NIC unaware of quirks' region and preventing the NIC from functioning.
For PCI quirks, PIIX4 ACPI is expected to claim 0x1000-0x103f, and PIIX4 SMB to
claim 0x1100-0x110f, both of which fall into the MSC I/O range. Certainly, we
can increase the start point of this range in arch/mips/mti-malta/malta-pci.c to
avoid the collisions. But a fix in here looks more justified, though it seems to
have a wider impact. Using insert_xxx as opposed to request_xxx will register
PCI quirks' resources as children of MSC I/O and return OK, instead of seeing
collisions which are actually resolvable.

Signed-off-by: Deng-Cheng Zhu <dengcheng.zhu@gmail.com>
---
 drivers/pci/setup-res.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index bc0e6ee..40d767e 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -102,7 +102,7 @@ int pci_claim_resource(struct pci_dev *dev, int resource)
 		return -EINVAL;
 	}
 
-	conflict = request_resource_conflict(root, res);
+	conflict = insert_resource_conflict(root, res);
 	if (conflict) {
 		dev_info(&dev->dev,
 			 "address space collision: %pR conflicts with %s %pR\n",
-- 
1.7.1


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

* [PATCH 2/2] kernel/resource: enrich the comment for insert_resource_conflict()
  2011-07-28 11:28 [PATCH 0/2] PCI driver to use insert_resource_conflict() to claim resources Deng-Cheng Zhu
  2011-07-28 11:28 ` [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate Deng-Cheng Zhu
@ 2011-07-28 11:28 ` Deng-Cheng Zhu
  1 sibling, 0 replies; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-07-28 11:28 UTC (permalink / raw)
  To: jbarnes, torvalds
  Cc: linux-pci, linux-kernel, linux-mips, eyal, zenon, dengcheng.zhu

It helps people better understand how this function works.

Signed-off-by: Deng-Cheng Zhu <dengcheng.zhu@gmail.com>
---
 kernel/resource.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index 3ff4017..5406ecf 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -617,7 +617,9 @@ static struct resource * __insert_resource(struct resource *parent, struct resou
  * happens. If a conflict happens, and the conflicting resources
  * entirely fit within the range of the new resource, then the new
  * resource is inserted and the conflicting resources become children of
- * the new resource.
+ * the new resource. Also, if the new resource entirely fits within the range
+ * of a conflicting resource without overlapping the latter's children, then
+ * the new resource is inserted too and becomes a child of the conflicting one.
  */
 struct resource *insert_resource_conflict(struct resource *parent, struct resource *new)
 {
-- 
1.7.1


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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-28 11:28 ` [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate Deng-Cheng Zhu
@ 2011-07-28 11:53   ` Ralf Baechle
  2011-07-29  5:33     ` Deng-Cheng Zhu
  2011-07-29  6:32     ` Deng-Cheng Zhu
  2011-07-28 15:59   ` Bjorn Helgaas
  1 sibling, 2 replies; 11+ messages in thread
From: Ralf Baechle @ 2011-07-28 11:53 UTC (permalink / raw)
  To: Deng-Cheng Zhu
  Cc: jbarnes, torvalds, linux-pci, linux-kernel, linux-mips, eyal, zenon

On Thu, Jul 28, 2011 at 07:28:31PM +0800, Deng-Cheng Zhu wrote:

> In resolving a network driver issue with the MIPS Malta platform, the root
> cause was traced into pci_claim_resource():
> 
> MIPS System Controller's PCI I/O resources stay in 0x1000-0xffffff. When
> PCI quirks start claiming resources using request_resource_conflict(),
> collisions happen and -EBUSY is returned, thereby rendering the onboard AMD
> PCnet32 NIC unaware of quirks' region and preventing the NIC from functioning.
> For PCI quirks, PIIX4 ACPI is expected to claim 0x1000-0x103f, and PIIX4 SMB to
> claim 0x1100-0x110f, both of which fall into the MSC I/O range. Certainly, we
> can increase the start point of this range in arch/mips/mti-malta/malta-pci.c to
> avoid the collisions. But a fix in here looks more justified, though it seems to
> have a wider impact. Using insert_xxx as opposed to request_xxx will register
> PCI quirks' resources as children of MSC I/O and return OK, instead of seeing
> collisions which are actually resolvable.

This used to work in the past; do you know which commit broke the resource
handling for the NIC?

  Ralf

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-28 11:28 ` [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate Deng-Cheng Zhu
  2011-07-28 11:53   ` Ralf Baechle
@ 2011-07-28 15:59   ` Bjorn Helgaas
  2011-07-29  6:35     ` Deng-Cheng Zhu
  1 sibling, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2011-07-28 15:59 UTC (permalink / raw)
  To: Deng-Cheng Zhu
  Cc: jbarnes, torvalds, linux-pci, linux-kernel, linux-mips, eyal,
	zenon, Ralf Baechle

On Thu, Jul 28, 2011 at 5:28 AM, Deng-Cheng Zhu <dengcheng.zhu@gmail.com> wrote:
> In resolving a network driver issue with the MIPS Malta platform, the root
> cause was traced into pci_claim_resource():
>
> MIPS System Controller's PCI I/O resources stay in 0x1000-0xffffff. When
> PCI quirks start claiming resources using request_resource_conflict(),
> collisions happen and -EBUSY is returned, thereby rendering the onboard AMD
> PCnet32 NIC unaware of quirks' region and preventing the NIC from functioning.
> For PCI quirks, PIIX4 ACPI is expected to claim 0x1000-0x103f, and PIIX4 SMB to
> claim 0x1100-0x110f, both of which fall into the MSC I/O range. Certainly, we
> can increase the start point of this range in arch/mips/mti-malta/malta-pci.c to
> avoid the collisions. But a fix in here looks more justified, though it seems to
> have a wider impact. Using insert_xxx as opposed to request_xxx will register
> PCI quirks' resources as children of MSC I/O and return OK, instead of seeing
> collisions which are actually resolvable.

What's the collision?  Can we see the dmesg log (which should have
that information) and maybe the /proc/ioports contents?  Did something
change the order in which we claim resources, so things that used to
work now cause conflicts?

I think insert_resource() (where the newly-inserted resource can
become the parent of something that was previously inserted) is sort
of a hack, and the fact that we need it is telling us that we're doing
things in the wrong order.  It's nicer when we can discover and claim
resources in a top-down hierarchical way.  But I recognize that may
not always be possible, or at least not convenient.

Bjorn

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-28 11:53   ` Ralf Baechle
@ 2011-07-29  5:33     ` Deng-Cheng Zhu
  2011-07-29  6:32     ` Deng-Cheng Zhu
  1 sibling, 0 replies; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-07-29  5:33 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: jbarnes, torvalds, linux-pci, linux-kernel, linux-mips, eyal, zenon

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

I noticed that at 79896cf42f Linus changed the function from
insert_resource()
to request_resource() (and later evolved into request_resource_conflict())
and
he explained the reason. So, in the NIC's case, the problem is that in
pci_claim_resource() the function pci_find_parent_resource() returns the
root
(0x0-0xffffff) rather than the MSC PCI I/O (0x1000-0xffffff). So
request_resource_conflict() for PCI quirks (0x1000-0x103f and 0x1100-0x110f)
will simply return an error, coz these 2 regions 'conflict' with MSC PCI
I/O.
Instead, insert_resource_conflict() will also find the collisions but
register
quirks as children of MSC PCI I/O (is this supposed to be correct?) and
return
a success.


Deng-Cheng

2011/7/28 Ralf Baechle <ralf@linux-mips.org>

> On Thu, Jul 28, 2011 at 07:28:31PM +0800, Deng-Cheng Zhu wrote:
>
> > In resolving a network driver issue with the MIPS Malta platform, the
> root
> > cause was traced into pci_claim_resource():
> >
> > MIPS System Controller's PCI I/O resources stay in 0x1000-0xffffff. When
> > PCI quirks start claiming resources using request_resource_conflict(),
> > collisions happen and -EBUSY is returned, thereby rendering the onboard
> AMD
> > PCnet32 NIC unaware of quirks' region and preventing the NIC from
> functioning.
> > For PCI quirks, PIIX4 ACPI is expected to claim 0x1000-0x103f, and PIIX4
> SMB to
> > claim 0x1100-0x110f, both of which fall into the MSC I/O range.
> Certainly, we
> > can increase the start point of this range in
> arch/mips/mti-malta/malta-pci.c to
> > avoid the collisions. But a fix in here looks more justified, though it
> seems to
> > have a wider impact. Using insert_xxx as opposed to request_xxx will
> register
> > PCI quirks' resources as children of MSC I/O and return OK, instead of
> seeing
> > collisions which are actually resolvable.
>
> This used to work in the past; do you know which commit broke the resource
> handling for the NIC?
>
>  Ralf
>

[-- Attachment #2: Type: text/html, Size: 2414 bytes --]

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-28 11:53   ` Ralf Baechle
  2011-07-29  5:33     ` Deng-Cheng Zhu
@ 2011-07-29  6:32     ` Deng-Cheng Zhu
  2011-07-29 17:35       ` Bjorn Helgaas
  1 sibling, 1 reply; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-07-29  6:32 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: jbarnes, torvalds, linux-pci, linux-kernel, linux-mips, eyal, zenon

I noticed that at 79896cf42f Linus changed the function from insert_resource()
to request_resource() (and later evolved into request_resource_conflict()) and
he explained the reason. So, in the NIC's case, the problem is that in
pci_claim_resource() the function pci_find_parent_resource() returns the root
(0x0-0xffffff) rather than the MSC PCI I/O (0x1000-0xffffff). So
request_resource_conflict() for PCI quirks (0x1000-0x103f and 0x1100-0x110f)
will simply return an error, coz these 2 regions 'conflict' with MSC PCI I/O.
Instead, insert_resource_conflict() will also find the collisions but register
quirks as children of MSC PCI I/O (is this supposed to be correct?) and return
a success.


Deng-Cheng

2011/7/28 Ralf Baechle <ralf@linux-mips.org>
>
> On Thu, Jul 28, 2011 at 07:28:31PM +0800, Deng-Cheng Zhu wrote:
>
> > In resolving a network driver issue with the MIPS Malta platform, the root
> > cause was traced into pci_claim_resource():
> >
> > MIPS System Controller's PCI I/O resources stay in 0x1000-0xffffff. When
> > PCI quirks start claiming resources using request_resource_conflict(),
> > collisions happen and -EBUSY is returned, thereby rendering the onboard AMD
> > PCnet32 NIC unaware of quirks' region and preventing the NIC from functioning.
> > For PCI quirks, PIIX4 ACPI is expected to claim 0x1000-0x103f, and PIIX4 SMB to
> > claim 0x1100-0x110f, both of which fall into the MSC I/O range. Certainly, we
> > can increase the start point of this range in arch/mips/mti-malta/malta-pci.c to
> > avoid the collisions. But a fix in here looks more justified, though it seems to
> > have a wider impact. Using insert_xxx as opposed to request_xxx will register
> > PCI quirks' resources as children of MSC I/O and return OK, instead of seeing
> > collisions which are actually resolvable.
>
> This used to work in the past; do you know which commit broke the resource
> handling for the NIC?
>
>  Ralf

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-28 15:59   ` Bjorn Helgaas
@ 2011-07-29  6:35     ` Deng-Cheng Zhu
  0 siblings, 0 replies; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-07-29  6:35 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: jbarnes, torvalds, linux-pci, linux-kernel, linux-mips, eyal,
	zenon, Ralf Baechle

Well, here are dmesg and /proc/ioports *BEFORE* applying the patch:

...
...
pci 0000:00:0a.0: [8086:7110] type 0 class 0x000601
pci 0000:00:0a.1: [8086:7111] type 0 class 0x000101
pci 0000:00:0a.1: reg 20: [io  0x1240-0x124f]
pci 0000:00:0a.2: [8086:7112] type 0 class 0x000c03
pci 0000:00:0a.2: reg 20: [io  0x1220-0x123f]
pci 0000:00:0a.3: [8086:7113] type 0 class 0x000680
pci 0000:00:0a.3: address space collision: [io  0x1000-0x103f]
conflicts with MSC PCI I/O [io  0x1000-0xffffff]
pci 0000:00:0a.3: address space collision: [io  0x1100-0x110f]
conflicts with MSC PCI I/O [io  0x1000-0xffffff]
pci 0000:00:0b.0: [1022:2000] type 0 class 0x000200
pci 0000:00:0b.0: reg 10: [io  0x1200-0x121f]
pci 0000:00:0b.0: reg 14: [mem 0x10000000-0x1000001f]
pci 0000:00:0b.0: reg 30: [mem 0x00000000-0x000fffff pref]
pci 0000:00:0b.0: PME# supported from D0 D3hot D3cold
pci 0000:00:0b.0: PME# disabled
pci 0000:00:11.0: [153f:0001] type 0 class 0x000600
pci 0000:00:11.0: reg 10: [mem 0x00000000-0x0fffffff]
pci 0000:00:0a.3: BAR 13: [io  0x1000-0x103f] has bogus alignment
pci 0000:00:0a.3: BAR 14: [io  0x1100-0x110f] has bogus alignment
pci 0000:00:0b.0: BAR 6: assigned [mem 0x10000000-0x100fffff pref]
pci 0000:00:0a.2: BAR 4: assigned [io  0x1000-0x101f]
pci 0000:00:0a.2: BAR 4: set to [io  0x1000-0x101f] (PCI address
[0x1000-0x101f])
pci 0000:00:0b.0: BAR 0: assigned [io  0x1020-0x103f]
pci 0000:00:0b.0: BAR 0: set to [io  0x1020-0x103f] (PCI address
[0x1020-0x103f])
pci 0000:00:0b.0: BAR 1: assigned [mem 0x10100000-0x1010001f]
pci 0000:00:0b.0: BAR 1: set to [mem 0x10100000-0x1010001f] (PCI
address [0x10100000-0x1010001f])
pci 0000:00:0a.1: BAR 4: assigned [io  0x1040-0x104f]
pci 0000:00:0a.1: BAR 4: set to [io  0x1040-0x104f] (PCI address
[0x1040-0x104f])
...
...
pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
pcnet32: No access methods
...
...

-sh-4.0# cat /proc/ioports
00000000-0000001f : dma1
00000020-00000021 : pic1
00000040-0000005f : timer
00000060-0000006f : keyboard
00000070-00000077 : rtc_cmos
00000080-0000008f : dma page reg
000000a0-000000a1 : pic2
000000c0-000000df : dma2
00000170-00000177 : piix
000001f0-000001f7 : piix
000002f8-000002ff : serial
00000376-00000376 : piix
000003f6-000003f6 : piix
000003f8-000003ff : serial
00001000-00ffffff : MSC PCI I/O
  00001000-0000101f : 0000:00:0a.2
  00001020-0000103f : 0000:00:0b.0
  00001040-0000104f : 0000:00:0a.1
    00001040-0000104f : piix

And *AFTER* applying the patch:

...
...
pci 0000:00:0a.0: [8086:7110] type 0 class 0x000601
pci 0000:00:0a.1: [8086:7111] type 0 class 0x000101
pci 0000:00:0a.1: reg 20: [io  0x1240-0x124f]
pci 0000:00:0a.2: [8086:7112] type 0 class 0x000c03
pci 0000:00:0a.2: reg 20: [io  0x1220-0x123f]
pci 0000:00:0a.3: [8086:7113] type 0 class 0x000680
pci 0000:00:0a.3: quirk: [io  0x1000-0x103f] claimed by PIIX4 ACPI
pci 0000:00:0a.3: quirk: [io  0x1100-0x110f] claimed by PIIX4 SMB
pci 0000:00:0b.0: [1022:2000] type 0 class 0x000200
pci 0000:00:0b.0: reg 10: [io  0x1200-0x121f]
pci 0000:00:0b.0: reg 14: [mem 0x10000000-0x1000001f]
pci 0000:00:0b.0: reg 30: [mem 0x00000000-0x000fffff pref]
pci 0000:00:0b.0: PME# supported from D0 D3hot D3cold
pci 0000:00:0b.0: PME# disabled
pci 0000:00:11.0: [153f:0001] type 0 class 0x000600
pci 0000:00:11.0: reg 10: [mem 0x00000000-0x0fffffff]
pci 0000:00:0b.0: BAR 6: assigned [mem 0x10000000-0x100fffff pref]
pci 0000:00:0a.2: BAR 4: assigned [io  0x1040-0x105f]
pci 0000:00:0a.2: BAR 4: set to [io  0x1040-0x105f] (PCI address
[0x1040-0x105f])
pci 0000:00:0b.0: BAR 0: assigned [io  0x1060-0x107f]
pci 0000:00:0b.0: BAR 0: set to [io  0x1060-0x107f] (PCI address
[0x1060-0x107f])
pci 0000:00:0b.0: BAR 1: assigned [mem 0x10100000-0x1010001f]
pci 0000:00:0b.0: BAR 1: set to [mem 0x10100000-0x1010001f] (PCI
address [0x10100000-0x1010001f])
pci 0000:00:0a.1: BAR 4: assigned [io  0x1080-0x108f]
pci 0000:00:0a.1: BAR 4: set to [io  0x1080-0x108f] (PCI address
[0x1080-0x108f])
...
...
pcnet32: pcnet32.c:v1.35 21.Apr.2008 tsbogend@alpha.franken.de
pcnet32: PCnet/FAST III 79C973 at 0x1060, 00:d0:a0:00:06:72 assigned IRQ 10
pcnet32: Found PHY 0000:6b60 at address 30
pcnet32: eth0: registered as PCnet/FAST III 79C973
pcnet32: 1 cards_found
...
...

-sh-4.0# cat /proc/ioports
00000000-0000001f : dma1
00000020-00000021 : pic1
00000040-0000005f : timer
00000060-0000006f : keyboard
00000070-00000077 : rtc_cmos
00000080-0000008f : dma page reg
000000a0-000000a1 : pic2
000000c0-000000df : dma2
00000170-00000177 : piix
000001f0-000001f7 : piix
000002f8-000002ff : serial
00000376-00000376 : piix
000003f6-000003f6 : piix
000003f8-000003ff : serial
00001000-00ffffff : MSC PCI I/O
  00001000-0000103f : 0000:00:0a.3
  00001040-0000105f : 0000:00:0a.2
  00001060-0000107f : 0000:00:0b.0
    00001060-0000107f : pcnet32_probe_pci
  00001080-0000108f : 0000:00:0a.1
    00001080-0000108f : piix
  00001100-0000110f : 0000:00:0a.3

> Did something change the order in which we claim resources, so things that
> used to work now cause conflicts?

It used to work (as I see on 2.6.29) because the function insert_resource() was
used. The /proc/ioports of 2.6.29 has exactly the same contents as that of the
patched kernel.

> I think insert_resource() (where the newly-inserted resource can become the
> parent of something that was previously inserted) is sort of a hack...

Yes, agree, though in this case the newly-inserted resource actually becomes the
child of a previously inserted one, the MIPS System Controller's PCI I/O. So,
talking about sticking to using request_resource_conflict() as apposed to
insert_resource_conflict(), the problem is, like what I mentioned in my reply to
Ralf, why pci_find_parent_resource() does not return MSC PCI I/O but return the
root. If MSC PCI I/O is supposed to be the parent of PCI quirks, then something
is wrong somewhere else. Or else, the start point of MSC PCI I/O might be
raised to avoid the collisions.


Deng-Cheng

2011/7/28 Bjorn Helgaas <bhelgaas@google.com>:
> On Thu, Jul 28, 2011 at 5:28 AM, Deng-Cheng Zhu <dengcheng.zhu@gmail.com> wrote:
>> In resolving a network driver issue with the MIPS Malta platform, the root
>> cause was traced into pci_claim_resource():
>>
>> MIPS System Controller's PCI I/O resources stay in 0x1000-0xffffff. When
>> PCI quirks start claiming resources using request_resource_conflict(),
>> collisions happen and -EBUSY is returned, thereby rendering the onboard AMD
>> PCnet32 NIC unaware of quirks' region and preventing the NIC from functioning.
>> For PCI quirks, PIIX4 ACPI is expected to claim 0x1000-0x103f, and PIIX4 SMB to
>> claim 0x1100-0x110f, both of which fall into the MSC I/O range. Certainly, we
>> can increase the start point of this range in arch/mips/mti-malta/malta-pci.c to
>> avoid the collisions. But a fix in here looks more justified, though it seems to
>> have a wider impact. Using insert_xxx as opposed to request_xxx will register
>> PCI quirks' resources as children of MSC I/O and return OK, instead of seeing
>> collisions which are actually resolvable.
>
> What's the collision?  Can we see the dmesg log (which should have
> that information) and maybe the /proc/ioports contents?  Did something
> change the order in which we claim resources, so things that used to
> work now cause conflicts?
>
> I think insert_resource() (where the newly-inserted resource can
> become the parent of something that was previously inserted) is sort
> of a hack, and the fact that we need it is telling us that we're doing
> things in the wrong order.  It's nicer when we can discover and claim
> resources in a top-down hierarchical way.  But I recognize that may
> not always be possible, or at least not convenient.
>
> Bjorn
>

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-29  6:32     ` Deng-Cheng Zhu
@ 2011-07-29 17:35       ` Bjorn Helgaas
  2011-08-01 10:13         ` Deng-Cheng Zhu
  0 siblings, 1 reply; 11+ messages in thread
From: Bjorn Helgaas @ 2011-07-29 17:35 UTC (permalink / raw)
  To: Deng-Cheng Zhu
  Cc: Ralf Baechle, jbarnes, torvalds, linux-pci, linux-kernel,
	linux-mips, eyal, zenon

On Fri, Jul 29, 2011 at 12:32 AM, Deng-Cheng Zhu
<dengcheng.zhu@gmail.com> wrote:
> I noticed that at 79896cf42f Linus changed the function from insert_resource()
> to request_resource() (and later evolved into request_resource_conflict()) and
> he explained the reason. So, in the NIC's case, the problem is that in
> pci_claim_resource() the function pci_find_parent_resource() returns the root
> (0x0-0xffffff) rather than the MSC PCI I/O (0x1000-0xffffff).

This seems like the real problem: PCI has the wrong idea of the
resources available on bus 00.  The pci_bus->resource[0] for bus 00
points to ioport_resource (the default put there by pci_create_bus()),
when it should point to to msc_io_resource instead.

Some architectures fill in the pci_bus->resource[] array directly for
host bridges (for examples, try 'grep -r "resource\[0\] = " arch/').
On x86 and ia64, we use pci_bus_remove_resources() and
pci_bus_add_resource(), and I'd prefer that style for new code because
it hides some ugly implementation details.

I'm a little puzzled that we don't see this problem on more
architectures.  The grep above only found a few arches that update the
root bus resources.  I would expect most of the ones it didn't find to
be broken the same way Malta is.

Bjorn

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-07-29 17:35       ` Bjorn Helgaas
@ 2011-08-01 10:13         ` Deng-Cheng Zhu
  2011-08-01 15:21           ` Bjorn Helgaas
  0 siblings, 1 reply; 11+ messages in thread
From: Deng-Cheng Zhu @ 2011-08-01 10:13 UTC (permalink / raw)
  To: Bjorn Helgaas, Ralf Baechle
  Cc: jbarnes, torvalds, linux-pci, linux-kernel, linux-mips, eyal, zenon

It was found that PCI quirks claim resources (by calling pci_claim_resource())
*BEFORE* pcibios_fixup_bus() is called. In pcibios_fixup_bus(),
pci_bus->resource[0] for the root bus DOES point to msc_io_resource. If PCI
quirks do the resource claim after the arch-defined pcibios_fixup_bus() being
called, then the problem with Malta goes away.

So, it looks like 2 solutions out there:

1) To manage the call sequence. This seems not a desired one as it affects other
arches.

2) To raise the start point of the system controller's io_resource in
mips_pcibios_init() in arch/mips/mti-malta/malta-pci.c. This will place PCI
quirks' resources at the same level of the system controller's resources.

Ralf and Bjorn, which one sounds good to you?


Deng-Cheng


2011/7/30 Bjorn Helgaas <bhelgaas@google.com>:
> On Fri, Jul 29, 2011 at 12:32 AM, Deng-Cheng Zhu
> <dengcheng.zhu@gmail.com> wrote:
>> I noticed that at 79896cf42f Linus changed the function from insert_resource()
>> to request_resource() (and later evolved into request_resource_conflict()) and
>> he explained the reason. So, in the NIC's case, the problem is that in
>> pci_claim_resource() the function pci_find_parent_resource() returns the root
>> (0x0-0xffffff) rather than the MSC PCI I/O (0x1000-0xffffff).
>
> This seems like the real problem: PCI has the wrong idea of the
> resources available on bus 00.  The pci_bus->resource[0] for bus 00
> points to ioport_resource (the default put there by pci_create_bus()),
> when it should point to to msc_io_resource instead.
>
> Some architectures fill in the pci_bus->resource[] array directly for
> host bridges (for examples, try 'grep -r "resource\[0\] = " arch/').
> On x86 and ia64, we use pci_bus_remove_resources() and
> pci_bus_add_resource(), and I'd prefer that style for new code because
> it hides some ugly implementation details.
>
> I'm a little puzzled that we don't see this problem on more
> architectures.  The grep above only found a few arches that update the
> root bus resources.  I would expect most of the ones it didn't find to
> be broken the same way Malta is.
>
> Bjorn
>

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

* Re: [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate
  2011-08-01 10:13         ` Deng-Cheng Zhu
@ 2011-08-01 15:21           ` Bjorn Helgaas
  0 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2011-08-01 15:21 UTC (permalink / raw)
  To: Deng-Cheng Zhu
  Cc: Ralf Baechle, jbarnes, torvalds, linux-pci, linux-kernel,
	linux-mips, eyal, zenon

On Mon, Aug 1, 2011 at 4:13 AM, Deng-Cheng Zhu <dengcheng.zhu@gmail.com> wrote:
> It was found that PCI quirks claim resources (by calling pci_claim_resource())
> *BEFORE* pcibios_fixup_bus() is called. In pcibios_fixup_bus(),
> pci_bus->resource[0] for the root bus DOES point to msc_io_resource. If PCI
> quirks do the resource claim after the arch-defined pcibios_fixup_bus() being
> called, then the problem with Malta goes away.

Oh, I see.  pcibios_fixup_bus() copies the hose resources to the root
bus pci_bus structure.  I think that's bogus because we have the
interval between mips_pcibios_init() and pcibios_fixup_bus() where the
root bus resources are incorrect.

I think it would be better to set up the resources correctly right
away, as we do on x86.  In fact, I'm dubious about pci_create_bus()
filling in ioport_resource and iomem_resource as defaults -- that's
never what we really want there, and we have to rely on the arch
coming back later to fix it up.

I'd like to see some sort of restructuring there so we could pass in a
list of resources at the time we create the bus.

Bjorn

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

end of thread, other threads:[~2011-08-01 15:21 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-07-28 11:28 [PATCH 0/2] PCI driver to use insert_resource_conflict() to claim resources Deng-Cheng Zhu
2011-07-28 11:28 ` [PATCH 1/2] PCI: make pci_claim_resource() work with conflict resources as appropriate Deng-Cheng Zhu
2011-07-28 11:53   ` Ralf Baechle
2011-07-29  5:33     ` Deng-Cheng Zhu
2011-07-29  6:32     ` Deng-Cheng Zhu
2011-07-29 17:35       ` Bjorn Helgaas
2011-08-01 10:13         ` Deng-Cheng Zhu
2011-08-01 15:21           ` Bjorn Helgaas
2011-07-28 15:59   ` Bjorn Helgaas
2011-07-29  6:35     ` Deng-Cheng Zhu
2011-07-28 11:28 ` [PATCH 2/2] kernel/resource: enrich the comment for insert_resource_conflict() Deng-Cheng Zhu

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