All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] eisa: fix eisa with PCI
@ 2013-03-28  4:28 Yinghai Lu
  2013-03-28  4:28 ` [PATCH 1/2] eisa, PCI: Fix bus res reference Yinghai Lu
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Yinghai Lu @ 2013-03-28  4:28 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Bjorn Helgaas, Matthew Whitehead
  Cc: linux-kernel

Looks like pci eisa bridge support is broken for a while.

one is root io resource reference, and other one is pnp related.

Please check if we can put them in v3.9.
	eisa, PCI: Fix bus res reference
	eisa, PCI: init eisa early before pnp step in

Thanks

Yinghai Lu

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

* [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-28  4:28 [PATCH 0/2] eisa: fix eisa with PCI Yinghai Lu
@ 2013-03-28  4:28 ` Yinghai Lu
  2013-03-28 12:52   ` Bjorn Helgaas
  2013-03-29 18:10   ` Bjorn Helgaas
  2013-03-28  4:28 ` [PATCH 2/2] eisa, PCI: init eisa early before pnp step in Yinghai Lu
  2013-03-29 15:16 ` [PATCH 0/2] eisa: fix eisa with PCI Bjorn Helgaas
  2 siblings, 2 replies; 18+ messages in thread
From: Yinghai Lu @ 2013-03-28  4:28 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Bjorn Helgaas, Matthew Whitehead
  Cc: linux-kernel, Yinghai Lu, stable

Matthem found that 3.8.3 is having problems with an old (ancient)
PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
He identified the 82375, but doesn't assign the struct resource *res
pointer inside the struct eisa_root_device, and panics.

After looking at pci_eisa_init(), found it referring bus resource
directly instead of pci_bus_resource_n().

After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
address offset), bus->resource[] is not used for pci root bus any more.

Fix it by using pci_bus_resource_n() and correct idx for root bus.

Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: stable@kernel.org

---
 drivers/eisa/pci_eisa.c |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Index: linux-2.6/drivers/eisa/pci_eisa.c
===================================================================
--- linux-2.6.orig/drivers/eisa/pci_eisa.c
+++ linux-2.6/drivers/eisa/pci_eisa.c
@@ -22,7 +22,8 @@ static struct eisa_root_device pci_eisa_
 static int __init pci_eisa_init(struct pci_dev *pdev,
 				const struct pci_device_id *ent)
 {
-	int rc;
+	int rc, n = 0;
+	struct resource *bus_res;
 
 	if ((rc = pci_enable_device (pdev))) {
 		printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
@@ -30,9 +31,12 @@ static int __init pci_eisa_init(struct p
 		return rc;
 	}
 
+	if (pci_is_root_bus(pdev->bus))
+		n = PCI_BRIDGE_RESOURCE_NUM;
+	bus_res = pci_bus_resource_n(pdev->bus, n);
 	pci_eisa_root.dev              = &pdev->dev;
-	pci_eisa_root.res	       = pdev->bus->resource[0];
-	pci_eisa_root.bus_base_addr    = pdev->bus->resource[0]->start;
+	pci_eisa_root.res	       = bus_res;
+	pci_eisa_root.bus_base_addr    = bus_res->start;
 	pci_eisa_root.slots	       = EISA_MAX_SLOTS;
 	pci_eisa_root.dma_mask         = pdev->dma_mask;
 	dev_set_drvdata(pci_eisa_root.dev, &pci_eisa_root);

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

* [PATCH 2/2] eisa, PCI: init eisa early before pnp step in
  2013-03-28  4:28 [PATCH 0/2] eisa: fix eisa with PCI Yinghai Lu
  2013-03-28  4:28 ` [PATCH 1/2] eisa, PCI: Fix bus res reference Yinghai Lu
@ 2013-03-28  4:28 ` Yinghai Lu
  2013-03-29 22:22   ` Bjorn Helgaas
  2013-03-29 15:16 ` [PATCH 0/2] eisa: fix eisa with PCI Bjorn Helgaas
  2 siblings, 1 reply; 18+ messages in thread
From: Yinghai Lu @ 2013-03-28  4:28 UTC (permalink / raw)
  To: Linus Torvalds, Andrew Morton, Bjorn Helgaas, Matthew Whitehead
  Cc: linux-kernel, Yinghai Lu, stable

Mathhew reported kernels fail the pci_eisa probe and are later successful
with the virtual_eisa_root_init force probe without slot0.

The reason for that is: pnp probing is early than pci_eisa_init get called
as pci_eisa_init is called via pci_driver.

pnp 00:0f has 0xc80 - 0xc84 reserved.
[    9.700409] pnp 00:0f: [io  0x0c80-0x0c84]

so eisa_probe will fail from pci_eisa_init
				==>eisa_root_register
					==>eisa_probe path.
as force_probe is not set in pci_eisa_root, it will bail early when
slot0 is not probed and initialized.

Try to use subsys_initcall_sync instead, and will keep following sequence:
	pci_subsys_init
	pci_eisa_init_early
	pnpacpi_init/isapnp_init

After this patch eisa can be initialized properly, and pnp overlapping
resource will not be reserved.
[   10.104434] system 00:0f: [io  0x0c80-0x0c84] could not be reserved

Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Cc: stable@kernel.org

---
 drivers/eisa/pci_eisa.c |   41 ++++++++++++++++++++++-------------------
 1 file changed, 22 insertions(+), 19 deletions(-)

Index: linux-2.6/drivers/eisa/pci_eisa.c
===================================================================
--- linux-2.6.orig/drivers/eisa/pci_eisa.c
+++ linux-2.6/drivers/eisa/pci_eisa.c
@@ -19,8 +19,7 @@
 /* There is only *one* pci_eisa device per machine, right ? */
 static struct eisa_root_device pci_eisa_root;
 
-static int __init pci_eisa_init(struct pci_dev *pdev,
-				const struct pci_device_id *ent)
+static int __init pci_eisa_init(struct pci_dev *pdev)
 {
 	int rc, n = 0;
 	struct resource *bus_res;
@@ -49,22 +48,26 @@ static int __init pci_eisa_init(struct p
 	return 0;
 }
 
-static struct pci_device_id pci_eisa_pci_tbl[] = {
-	{ PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
-	  PCI_CLASS_BRIDGE_EISA << 8, 0xffff00, 0 },
-	{ 0, }
-};
-
-static struct pci_driver __refdata pci_eisa_driver = {
-	.name		= "pci_eisa",
-	.id_table	= pci_eisa_pci_tbl,
-	.probe		= pci_eisa_init,
-};
-
-static int __init pci_eisa_init_module (void)
+/*
+ * We have to call pci_eisa_init_early() before pnpacpi_init()/isapnp_init().
+ *   Otherwise pnp resource will get enabled early and could prevent eisa
+ *   to be initialized.
+ * Also need to make sure pci_eisa_init_early() is called after
+ * x86/pci_subsys_init().
+ * So need to use subsys_initcall_sync with it.
+ */
+static int __init pci_eisa_init_early(void)
 {
-	return pci_register_driver (&pci_eisa_driver);
-}
+	struct pci_dev *dev = NULL;
+	int ret;
+
+	for_each_pci_dev(dev)
+		if ((dev->class >> 8) == PCI_CLASS_BRIDGE_EISA) {
+			ret = pci_eisa_init(dev);
+			if (ret)
+				return ret;
+		}
 
-device_initcall(pci_eisa_init_module);
-MODULE_DEVICE_TABLE(pci, pci_eisa_pci_tbl);
+	return 0;
+}
+subsys_initcall_sync(pci_eisa_init_early);

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-28  4:28 ` [PATCH 1/2] eisa, PCI: Fix bus res reference Yinghai Lu
@ 2013-03-28 12:52   ` Bjorn Helgaas
  2013-03-28 15:16     ` Yinghai Lu
  2013-03-29 18:10   ` Bjorn Helgaas
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-03-28 12:52 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel, stable

On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Matthem found that 3.8.3 is having problems with an old (ancient)
> PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
> He identified the 82375, but doesn't assign the struct resource *res
> pointer inside the struct eisa_root_device, and panics.
>
> After looking at pci_eisa_init(), found it referring bus resource
> directly instead of pci_bus_resource_n().
>
> After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
> and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
> address offset), bus->resource[] is not used for pci root bus any more.
>
> Fix it by using pci_bus_resource_n() and correct idx for root bus.

Please include URLs for the problem reports for these problems
(bugzilla or mailing list discussion).

> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: stable@kernel.org

You are consistently using the wrong stable email address.  It should
be "stable@vger.kernel.org".  I assume you see a bounce every time; at
least, I get a bounce when I reply to your messages.

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-28 12:52   ` Bjorn Helgaas
@ 2013-03-28 15:16     ` Yinghai Lu
  2013-03-28 16:14       ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Yinghai Lu @ 2013-03-28 15:16 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel, stable

On Thu, Mar 28, 2013 at 5:52 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Matthem found that 3.8.3 is having problems with an old (ancient)
>> PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
>> He identified the 82375, but doesn't assign the struct resource *res
>> pointer inside the struct eisa_root_device, and panics.
>>
>> After looking at pci_eisa_init(), found it referring bus resource
>> directly instead of pci_bus_resource_n().
>>
>> After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
>> and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
>> address offset), bus->resource[] is not used for pci root bus any more.
>>
>> Fix it by using pci_bus_resource_n() and correct idx for root bus.
>
> Please include URLs for the problem reports for these problems
> (bugzilla or mailing list discussion).

Matthew sent private mail to me, and I sent him test patch to see
if it fixes the problem. then I posted patches here after that.

>
>> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
>> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: stable@kernel.org
>
> You are consistently using the wrong stable email address.  It should
> be "stable@vger.kernel.org".  I assume you see a bounce every time; at
> least, I get a bounce when I reply to your messages.

I thought that is tag only, and stable maintainer  will search that from
Linus's tree.
but git send-mail will pick up Cc from the patch.

So stable@kernel.org will never get fixed?

Yinghai

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-28 15:16     ` Yinghai Lu
@ 2013-03-28 16:14       ` Bjorn Helgaas
  2013-03-28 16:54         ` Yinghai Lu
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-03-28 16:14 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel

On Thu, Mar 28, 2013 at 9:16 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Thu, Mar 28, 2013 at 5:52 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> Matthem found that 3.8.3 is having problems with an old (ancient)
>>> PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
>>> He identified the 82375, but doesn't assign the struct resource *res
>>> pointer inside the struct eisa_root_device, and panics.
>>>
>>> After looking at pci_eisa_init(), found it referring bus resource
>>> directly instead of pci_bus_resource_n().
>>>
>>> After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
>>> and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
>>> address offset), bus->resource[] is not used for pci root bus any more.
>>>
>>> Fix it by using pci_bus_resource_n() and correct idx for root bus.
>>
>> Please include URLs for the problem reports for these problems
>> (bugzilla or mailing list discussion).
>
> Matthew sent private mail to me, and I sent him test patch to see
> if it fixes the problem. then I posted patches here after that.

Please add "cc: linux-pci@vger.kernel.org" when you respond to private
mail like that.  As far as I'm concerned, if it doesn't appear in a
public email archive, it didn't happen.  I hate dealing with patches
out of the blue like this that don't have any context.  If it's on a
mailing list, other people with the same problem have some hope of
finding this solution.

>>> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
>>> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>> Cc: stable@kernel.org
>>
>> You are consistently using the wrong stable email address.  It should
>> be "stable@vger.kernel.org".  I assume you see a bounce every time; at
>> least, I get a bounce when I reply to your messages.
>
> I thought that is tag only, and stable maintainer  will search that from
> Linus's tree.
> but git send-mail will pick up Cc from the patch.
>
> So stable@kernel.org will never get fixed?

What's to fix?  The doc (Documentation/stable_kernel_rules.txt) says
to use "Cc: stable@vger.kernel.org" and using "Cc: stable@kernel.org"
causes bounces.  Just use the documented address like everybody else
and everything will be fine.

If you want "stable@kernel.org" to work, first make the kernel.org
change, then change Documentation/stable_kernel_rules.txt, *then*
start using it in your patches.

Bjorn

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-28 16:14       ` Bjorn Helgaas
@ 2013-03-28 16:54         ` Yinghai Lu
  0 siblings, 0 replies; 18+ messages in thread
From: Yinghai Lu @ 2013-03-28 16:54 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel

On Thu, Mar 28, 2013 at 9:14 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>
>> So stable@kernel.org will never get fixed?
>
> What's to fix?  The doc (Documentation/stable_kernel_rules.txt) says
> to use "Cc: stable@vger.kernel.org" and using "Cc: stable@kernel.org"
> causes bounces.  Just use the documented address like everybody else
> and everything will be fine.

I don't think that I'm the only one is still using stable@kernel.org
in patch cc.
please check git log -p in recent commits in linus tree.

>
> If you want "stable@kernel.org" to work, first make the kernel.org
> change, then change Documentation/stable_kernel_rules.txt, *then*
> start using it in your patches.

stable@kernel.org is not restored after last kernel.org get breached.

And I thought stable@vger.kernel.org should be just temporary one.

Yinghai

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

* Re: [PATCH 0/2] eisa: fix eisa with PCI
  2013-03-28  4:28 [PATCH 0/2] eisa: fix eisa with PCI Yinghai Lu
  2013-03-28  4:28 ` [PATCH 1/2] eisa, PCI: Fix bus res reference Yinghai Lu
  2013-03-28  4:28 ` [PATCH 2/2] eisa, PCI: init eisa early before pnp step in Yinghai Lu
@ 2013-03-29 15:16 ` Bjorn Helgaas
  2013-04-01 21:09   ` Bjorn Helgaas
  2 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-03-29 15:16 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	linux-pci

[+cc linux-pci]

On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Looks like pci eisa bridge support is broken for a while.
>
> one is root io resource reference, and other one is pnp related.
>
> Please check if we can put them in v3.9.
>         eisa, PCI: Fix bus res reference
>         eisa, PCI: init eisa early before pnp step in

Both of these patches are to drivers/eisa/pci_eisa.c.  I don't see an
EISA-specific maintainer, and I caused at least one of these problems,
so I'll push these through my PCI tree unless somebody objects.

Bjorn

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-28  4:28 ` [PATCH 1/2] eisa, PCI: Fix bus res reference Yinghai Lu
  2013-03-28 12:52   ` Bjorn Helgaas
@ 2013-03-29 18:10   ` Bjorn Helgaas
  2013-03-29 19:10     ` Yinghai Lu
  1 sibling, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-03-29 18:10 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel, stable

On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Matthem found that 3.8.3 is having problems with an old (ancient)
> PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
> He identified the 82375, but doesn't assign the struct resource *res
> pointer inside the struct eisa_root_device, and panics.
>
> After looking at pci_eisa_init(), found it referring bus resource
> directly instead of pci_bus_resource_n().
>
> After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
> and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
> address offset), bus->resource[] is not used for pci root bus any more.
>
> Fix it by using pci_bus_resource_n() and correct idx for root bus.
>
> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: stable@kernel.org
>
> ---
>  drivers/eisa/pci_eisa.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> Index: linux-2.6/drivers/eisa/pci_eisa.c
> ===================================================================
> --- linux-2.6.orig/drivers/eisa/pci_eisa.c
> +++ linux-2.6/drivers/eisa/pci_eisa.c
> @@ -22,7 +22,8 @@ static struct eisa_root_device pci_eisa_
>  static int __init pci_eisa_init(struct pci_dev *pdev,
>                                 const struct pci_device_id *ent)
>  {
> -       int rc;
> +       int rc, n = 0;
> +       struct resource *bus_res;
>
>         if ((rc = pci_enable_device (pdev))) {
>                 printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
> @@ -30,9 +31,12 @@ static int __init pci_eisa_init(struct p
>                 return rc;
>         }
>
> +       if (pci_is_root_bus(pdev->bus))
> +               n = PCI_BRIDGE_RESOURCE_NUM;
> +       bus_res = pci_bus_resource_n(pdev->bus, n);

I haven't figured out why the pci_is_root_bus() test is here.  Can you
explain, maybe with a sample dmesg log and "lspci -vvx" output?

Doesn't the 82375 subtractively decode this PCI memory space, the same
way a subtractive-decode P2P bridge does?  Can we  make this code look
like the "if (dev->transparent)" block in pci_read_bridge_bases(),
just breaking after the first resource we find, since
eisa_root_register() can only deal with a single resource on the EISA
bus?

Bjorn

>         pci_eisa_root.dev              = &pdev->dev;
> -       pci_eisa_root.res              = pdev->bus->resource[0];
> -       pci_eisa_root.bus_base_addr    = pdev->bus->resource[0]->start;
> +       pci_eisa_root.res              = bus_res;
> +       pci_eisa_root.bus_base_addr    = bus_res->start;
>         pci_eisa_root.slots            = EISA_MAX_SLOTS;
>         pci_eisa_root.dma_mask         = pdev->dma_mask;
>         dev_set_drvdata(pci_eisa_root.dev, &pci_eisa_root);

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-29 18:10   ` Bjorn Helgaas
@ 2013-03-29 19:10     ` Yinghai Lu
  2013-03-29 22:30       ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Yinghai Lu @ 2013-03-29 19:10 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel, stable

On Fri, Mar 29, 2013 at 11:10 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Matthem found that 3.8.3 is having problems with an old (ancient)
>> PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
>> He identified the 82375, but doesn't assign the struct resource *res
>> pointer inside the struct eisa_root_device, and panics.
>>
>> After looking at pci_eisa_init(), found it referring bus resource
>> directly instead of pci_bus_resource_n().
>>
>> After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
>> and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
>> address offset), bus->resource[] is not used for pci root bus any more.
>>
>> Fix it by using pci_bus_resource_n() and correct idx for root bus.
>>
>> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
>> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: stable@kernel.org
>>
>> ---
>>  drivers/eisa/pci_eisa.c |   10 +++++++---
>>  1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> Index: linux-2.6/drivers/eisa/pci_eisa.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/eisa/pci_eisa.c
>> +++ linux-2.6/drivers/eisa/pci_eisa.c
>> @@ -22,7 +22,8 @@ static struct eisa_root_device pci_eisa_
>>  static int __init pci_eisa_init(struct pci_dev *pdev,
>>                                 const struct pci_device_id *ent)
>>  {
>> -       int rc;
>> +       int rc, n = 0;
>> +       struct resource *bus_res;
>>
>>         if ((rc = pci_enable_device (pdev))) {
>>                 printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
>> @@ -30,9 +31,12 @@ static int __init pci_eisa_init(struct p
>>                 return rc;
>>         }
>>
>> +       if (pci_is_root_bus(pdev->bus))
>> +               n = PCI_BRIDGE_RESOURCE_NUM;
>> +       bus_res = pci_bus_resource_n(pdev->bus, n);
>
> I haven't figured out why the pci_is_root_bus() test is here.  Can you
> explain, maybe with a sample dmesg log and "lspci -vvx" output?

Just forwarded two boot.log.

for  root bus, pci_bus_resource_n[0, PCI_BRIDGE_RESOURCE_NUM) all
equal to 0.

>
> Doesn't the 82375 subtractively decode this PCI memory space, the same
> way a subtractive-decode P2P bridge does?  Can we  make this code look
> like the "if (dev->transparent)" block in pci_read_bridge_bases(),
> just breaking after the first resource we find, since
> eisa_root_register() can only deal with a single resource on the EISA
> bus?

they are using extra resource in pci_eisa_root.eisa_root_res to prevent multiple
registering between pci_eisa and virtual platform.

and later eisa resource does get registered under system root ioport resource.

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

* Re: [PATCH 2/2] eisa, PCI: init eisa early before pnp step in
  2013-03-28  4:28 ` [PATCH 2/2] eisa, PCI: init eisa early before pnp step in Yinghai Lu
@ 2013-03-29 22:22   ` Bjorn Helgaas
  2013-03-30  0:18     ` Yinghai Lu
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-03-29 22:22 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	Rafael J. Wysocki

[+cc Rafael, just FYI since it involves PNP resources]

On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> Mathhew reported kernels fail the pci_eisa probe and are later successful
> with the virtual_eisa_root_init force probe without slot0.
>
> The reason for that is: pnp probing is early than pci_eisa_init get called
> as pci_eisa_init is called via pci_driver.
>
> pnp 00:0f has 0xc80 - 0xc84 reserved.
> [    9.700409] pnp 00:0f: [io  0x0c80-0x0c84]
>
> so eisa_probe will fail from pci_eisa_init
>                                 ==>eisa_root_register
>                                         ==>eisa_probe path.
> as force_probe is not set in pci_eisa_root, it will bail early when
> slot0 is not probed and initialized.
>
> Try to use subsys_initcall_sync instead, and will keep following sequence:
>         pci_subsys_init
>         pci_eisa_init_early
>         pnpacpi_init/isapnp_init

Is this a regression?  This must have worked at one time, but it seems
like we've had pnpacpi_init/isapnp_init/pnpbios_init before PCI
drivers for quite a while.

> After this patch eisa can be initialized properly, and pnp overlapping
> resource will not be reserved.
> [   10.104434] system 00:0f: [io  0x0c80-0x0c84] could not be reserved

I think we should add logging in eisa_request_resources() similar to
what drivers/pnp/system.c does so it's more clear what resources EISA
is using.

> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> Cc: stable@kernel.org
>
> ---
>  drivers/eisa/pci_eisa.c |   41 ++++++++++++++++++++++-------------------
>  1 file changed, 22 insertions(+), 19 deletions(-)
>
> Index: linux-2.6/drivers/eisa/pci_eisa.c
> ===================================================================
> --- linux-2.6.orig/drivers/eisa/pci_eisa.c
> +++ linux-2.6/drivers/eisa/pci_eisa.c
> @@ -19,8 +19,7 @@
>  /* There is only *one* pci_eisa device per machine, right ? */
>  static struct eisa_root_device pci_eisa_root;
>
> -static int __init pci_eisa_init(struct pci_dev *pdev,
> -                               const struct pci_device_id *ent)
> +static int __init pci_eisa_init(struct pci_dev *pdev)
>  {
>         int rc, n = 0;
>         struct resource *bus_res;
> @@ -49,22 +48,26 @@ static int __init pci_eisa_init(struct p
>         return 0;
>  }
>
> -static struct pci_device_id pci_eisa_pci_tbl[] = {
> -       { PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
> -         PCI_CLASS_BRIDGE_EISA << 8, 0xffff00, 0 },
> -       { 0, }
> -};
> -
> -static struct pci_driver __refdata pci_eisa_driver = {
> -       .name           = "pci_eisa",
> -       .id_table       = pci_eisa_pci_tbl,
> -       .probe          = pci_eisa_init,
> -};
> -
> -static int __init pci_eisa_init_module (void)
> +/*
> + * We have to call pci_eisa_init_early() before pnpacpi_init()/isapnp_init().
> + *   Otherwise pnp resource will get enabled early and could prevent eisa
> + *   to be initialized.
> + * Also need to make sure pci_eisa_init_early() is called after
> + * x86/pci_subsys_init().
> + * So need to use subsys_initcall_sync with it.
> + */
> +static int __init pci_eisa_init_early(void)
>  {
> -       return pci_register_driver (&pci_eisa_driver);
> -}
> +       struct pci_dev *dev = NULL;
> +       int ret;
> +
> +       for_each_pci_dev(dev)
> +               if ((dev->class >> 8) == PCI_CLASS_BRIDGE_EISA) {
> +                       ret = pci_eisa_init(dev);
> +                       if (ret)
> +                               return ret;
> +               }
>
> -device_initcall(pci_eisa_init_module);
> -MODULE_DEVICE_TABLE(pci, pci_eisa_pci_tbl);
> +       return 0;
> +}
> +subsys_initcall_sync(pci_eisa_init_early);

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

* Re: [PATCH 1/2] eisa, PCI: Fix bus res reference
  2013-03-29 19:10     ` Yinghai Lu
@ 2013-03-29 22:30       ` Bjorn Helgaas
  0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2013-03-29 22:30 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel, stable

On Fri, Mar 29, 2013 at 12:10:37PM -0700, Yinghai Lu wrote:
> On Fri, Mar 29, 2013 at 11:10 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> > On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> >> Matthem found that 3.8.3 is having problems with an old (ancient)
> >> PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
> >> He identified the 82375, but doesn't assign the struct resource *res
> >> pointer inside the struct eisa_root_device, and panics.
> >>
> >> After looking at pci_eisa_init(), found it referring bus resource
> >> directly instead of pci_bus_resource_n().
> >>
> >> After commit 45ca9e97 (PCI: add helpers for building PCI bus resource lists)
> >> and commit 0efd5aab (PCI: add struct pci_host_bridge_window with CPU/bus
> >> address offset), bus->resource[] is not used for pci root bus any more.
> >>
> >> Fix it by using pci_bus_resource_n() and correct idx for root bus.
> >>
> >> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
> >> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
> >> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> >> Cc: stable@kernel.org
> >>
> >> ---
> >>  drivers/eisa/pci_eisa.c |   10 +++++++---
> >>  1 file changed, 7 insertions(+), 3 deletions(-)
> >>
> >> Index: linux-2.6/drivers/eisa/pci_eisa.c
> >> ===================================================================
> >> --- linux-2.6.orig/drivers/eisa/pci_eisa.c
> >> +++ linux-2.6/drivers/eisa/pci_eisa.c
> >> @@ -22,7 +22,8 @@ static struct eisa_root_device pci_eisa_
> >>  static int __init pci_eisa_init(struct pci_dev *pdev,
> >>                                 const struct pci_device_id *ent)
> >>  {
> >> -       int rc;
> >> +       int rc, n = 0;
> >> +       struct resource *bus_res;
> >>
> >>         if ((rc = pci_enable_device (pdev))) {
> >>                 printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
> >> @@ -30,9 +31,12 @@ static int __init pci_eisa_init(struct p
> >>                 return rc;
> >>         }
> >>
> >> +       if (pci_is_root_bus(pdev->bus))
> >> +               n = PCI_BRIDGE_RESOURCE_NUM;
> >> +       bus_res = pci_bus_resource_n(pdev->bus, n);
> >
> > I haven't figured out why the pci_is_root_bus() test is here.  Can you
> > explain, maybe with a sample dmesg log and "lspci -vvx" output?
> 
> Just forwarded two boot.log.
> 
> for  root bus, pci_bus_resource_n[0, PCI_BRIDGE_RESOURCE_NUM) all
> equal to 0.

Thanks.  The patch below is what I was thinking.  That should work
even in this scenario, I think, and it's nicer if we don't have to
have this root bus internal knowledge here.


diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c
index cdae207..3e9dee9 100644
--- a/drivers/eisa/pci_eisa.c
+++ b/drivers/eisa/pci_eisa.c
@@ -22,7 +22,8 @@ static struct eisa_root_device pci_eisa_root;
 static int __init pci_eisa_init(struct pci_dev *pdev,
 				const struct pci_device_id *ent)
 {
-	int rc;
+	int rc, i;
+	struct resource *res;
 
 	if ((rc = pci_enable_device (pdev))) {
 		printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
@@ -30,9 +31,28 @@ static int __init pci_eisa_init(struct pci_dev *pdev,
 		return rc;
 	}
 
+	/*
+	 * The Intel 82375 PCI-EISA bridge is a subtractive-decode PCI
+	 * device, so the resources available on EISA are the same as those
+	 * available on the 82375 bus.  This works the same as a PCI-PCI
+	 * bridge in subtractive-decode mode (see pci_read_bridge_bases()).
+	 * We assume other PCI-EISA bridges are similar.
+	 *
+	 * eisa_root_register() can only deal with a single resource, so we
+	 * use the first valid one.
+	 */
+	pci_bus_for_each_resource(pdev->bus, res, i)
+		if (res)
+			break;
+
+	if (!res) {
+		dev_err(&pdev->dev, "No resources available\n");
+		return -1;
+	}
+
 	pci_eisa_root.dev              = &pdev->dev;
-	pci_eisa_root.res	       = pdev->bus->resource[0];
-	pci_eisa_root.bus_base_addr    = pdev->bus->resource[0]->start;
+	pci_eisa_root.res	       = res;
+	pci_eisa_root.bus_base_addr    = res->start;
 	pci_eisa_root.slots	       = EISA_MAX_SLOTS;
 	pci_eisa_root.dma_mask         = pdev->dma_mask;
 	dev_set_drvdata(pci_eisa_root.dev, &pci_eisa_root);

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

* Re: [PATCH 2/2] eisa, PCI: init eisa early before pnp step in
  2013-03-29 22:22   ` Bjorn Helgaas
@ 2013-03-30  0:18     ` Yinghai Lu
  2013-04-01 18:15       ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Yinghai Lu @ 2013-03-30  0:18 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	Rafael J. Wysocki

On Fri, Mar 29, 2013 at 3:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc Rafael, just FYI since it involves PNP resources]
>
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Mathhew reported kernels fail the pci_eisa probe and are later successful
>> with the virtual_eisa_root_init force probe without slot0.
>>
>> The reason for that is: pnp probing is early than pci_eisa_init get called
>> as pci_eisa_init is called via pci_driver.
>>
>> pnp 00:0f has 0xc80 - 0xc84 reserved.
>> [    9.700409] pnp 00:0f: [io  0x0c80-0x0c84]
>>
>> so eisa_probe will fail from pci_eisa_init
>>                                 ==>eisa_root_register
>>                                         ==>eisa_probe path.
>> as force_probe is not set in pci_eisa_root, it will bail early when
>> slot0 is not probed and initialized.
>>
>> Try to use subsys_initcall_sync instead, and will keep following sequence:
>>         pci_subsys_init
>>         pci_eisa_init_early
>>         pnpacpi_init/isapnp_init
>
> Is this a regression?  This must have worked at one time, but it seems
> like we've had pnpacpi_init/isapnp_init/pnpbios_init before PCI
> drivers for quite a while.

Yes.

>
>> After this patch eisa can be initialized properly, and pnp overlapping
>> resource will not be reserved.
>> [   10.104434] system 00:0f: [io  0x0c80-0x0c84] could not be reserved
>
> I think we should add logging in eisa_request_resources() similar to
> what drivers/pnp/system.c does so it's more clear what resources EISA
> is using.

Those eisa resource does get registered into io port resource tree.

>
>> Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
>> Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>> Cc: stable@kernel.org
>>
>> ---
>>  drivers/eisa/pci_eisa.c |   41 ++++++++++++++++++++++-------------------
>>  1 file changed, 22 insertions(+), 19 deletions(-)
>>
>> Index: linux-2.6/drivers/eisa/pci_eisa.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/eisa/pci_eisa.c
>> +++ linux-2.6/drivers/eisa/pci_eisa.c
>> @@ -19,8 +19,7 @@
>>  /* There is only *one* pci_eisa device per machine, right ? */
>>  static struct eisa_root_device pci_eisa_root;
>>
>> -static int __init pci_eisa_init(struct pci_dev *pdev,
>> -                               const struct pci_device_id *ent)
>> +static int __init pci_eisa_init(struct pci_dev *pdev)
>>  {
>>         int rc, n = 0;
>>         struct resource *bus_res;
>> @@ -49,22 +48,26 @@ static int __init pci_eisa_init(struct p
>>         return 0;
>>  }
>>
>> -static struct pci_device_id pci_eisa_pci_tbl[] = {
>> -       { PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
>> -         PCI_CLASS_BRIDGE_EISA << 8, 0xffff00, 0 },
>> -       { 0, }
>> -};
>> -
>> -static struct pci_driver __refdata pci_eisa_driver = {
>> -       .name           = "pci_eisa",
>> -       .id_table       = pci_eisa_pci_tbl,
>> -       .probe          = pci_eisa_init,
>> -};
>> -
>> -static int __init pci_eisa_init_module (void)
>> +/*
>> + * We have to call pci_eisa_init_early() before pnpacpi_init()/isapnp_init().
>> + *   Otherwise pnp resource will get enabled early and could prevent eisa
>> + *   to be initialized.
>> + * Also need to make sure pci_eisa_init_early() is called after
>> + * x86/pci_subsys_init().
>> + * So need to use subsys_initcall_sync with it.
>> + */
>> +static int __init pci_eisa_init_early(void)
>>  {
>> -       return pci_register_driver (&pci_eisa_driver);
>> -}
>> +       struct pci_dev *dev = NULL;
>> +       int ret;
>> +
>> +       for_each_pci_dev(dev)
>> +               if ((dev->class >> 8) == PCI_CLASS_BRIDGE_EISA) {
>> +                       ret = pci_eisa_init(dev);
>> +                       if (ret)
>> +                               return ret;
>> +               }
>>
>> -device_initcall(pci_eisa_init_module);
>> -MODULE_DEVICE_TABLE(pci, pci_eisa_pci_tbl);
>> +       return 0;
>> +}
>> +subsys_initcall_sync(pci_eisa_init_early);

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

* Re: [PATCH 2/2] eisa, PCI: init eisa early before pnp step in
  2013-03-30  0:18     ` Yinghai Lu
@ 2013-04-01 18:15       ` Bjorn Helgaas
  2013-04-01 18:25         ` Yinghai Lu
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-04-01 18:15 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	Rafael J. Wysocki

On Fri, Mar 29, 2013 at 6:18 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Mar 29, 2013 at 3:22 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> [+cc Rafael, just FYI since it involves PNP resources]
>>
>> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> Mathhew reported kernels fail the pci_eisa probe and are later successful
>>> with the virtual_eisa_root_init force probe without slot0.
>>>
>>> The reason for that is: pnp probing is early than pci_eisa_init get called
>>> as pci_eisa_init is called via pci_driver.
>>>
>>> pnp 00:0f has 0xc80 - 0xc84 reserved.
>>> [    9.700409] pnp 00:0f: [io  0x0c80-0x0c84]
>>>
>>> so eisa_probe will fail from pci_eisa_init
>>>                                 ==>eisa_root_register
>>>                                         ==>eisa_probe path.
>>> as force_probe is not set in pci_eisa_root, it will bail early when
>>> slot0 is not probed and initialized.
>>>
>>> Try to use subsys_initcall_sync instead, and will keep following sequence:
>>>         pci_subsys_init
>>>         pci_eisa_init_early
>>>         pnpacpi_init/isapnp_init
>>
>> Is this a regression?  This must have worked at one time, but it seems
>> like we've had pnpacpi_init/isapnp_init/pnpbios_init before PCI
>> drivers for quite a while.
>
> Yes.

Do you know when the regression occurred?  If you do, I'll add that
info to the "stable" tag.

Bjorn

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

* Re: [PATCH 2/2] eisa, PCI: init eisa early before pnp step in
  2013-04-01 18:15       ` Bjorn Helgaas
@ 2013-04-01 18:25         ` Yinghai Lu
  2013-04-01 18:52           ` Bjorn Helgaas
  0 siblings, 1 reply; 18+ messages in thread
From: Yinghai Lu @ 2013-04-01 18:25 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	Rafael J. Wysocki

On Mon, Apr 1, 2013 at 11:15 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> >> Is this a regression?  This must have worked at one time, but it seems
> >> like we've had pnpacpi_init/isapnp_init/pnpbios_init before PCI
> >> drivers for quite a while.
> >
> > Yes.
>
> Do you know when the regression occurred?  If you do, I'll add that
> info to the "stable" tag.

No, I don't.

Looking at the current code, it should be quite before kernel was using git.
as first drivers/Makefile in git already has pnp before eisa.

Do we need to dig before 2.6.12 and 2.4?

Yinghai

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

* Re: [PATCH 2/2] eisa, PCI: init eisa early before pnp step in
  2013-04-01 18:25         ` Yinghai Lu
@ 2013-04-01 18:52           ` Bjorn Helgaas
  0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2013-04-01 18:52 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	Rafael J. Wysocki

On Mon, Apr 1, 2013 at 12:25 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Apr 1, 2013 at 11:15 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> >> Is this a regression?  This must have worked at one time, but it seems
>> >> like we've had pnpacpi_init/isapnp_init/pnpbios_init before PCI
>> >> drivers for quite a while.
>> >
>> > Yes.
>>
>> Do you know when the regression occurred?  If you do, I'll add that
>> info to the "stable" tag.
>
> No, I don't.
>
> Looking at the current code, it should be quite before kernel was using git.
> as first drivers/Makefile in git already has pnp before eisa.
>
> Do we need to dig before 2.6.12 and 2.4?

No, I don't think we need to worry about fixing kernels that old.

But I do wonder whether we fully understand the cause.  I know EISA is
old and obsolete, but it's a little hard to believe that it's been
broken since 2.6.12 and nobody noticed.

Bjorn

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

* Re: [PATCH 0/2] eisa: fix eisa with PCI
  2013-03-29 15:16 ` [PATCH 0/2] eisa: fix eisa with PCI Bjorn Helgaas
@ 2013-04-01 21:09   ` Bjorn Helgaas
       [not found]     ` <707419406.5567095.1364921996288.JavaMail.root@redhat.com>
  0 siblings, 1 reply; 18+ messages in thread
From: Bjorn Helgaas @ 2013-04-01 21:09 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Linus Torvalds, Andrew Morton, Matthew Whitehead, linux-kernel,
	linux-pci

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

On Fri, Mar 29, 2013 at 9:16 AM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc linux-pci]
>
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> Looks like pci eisa bridge support is broken for a while.
>>
>> one is root io resource reference, and other one is pnp related.
>>
>> Please check if we can put them in v3.9.
>>         eisa, PCI: Fix bus res reference
>>         eisa, PCI: init eisa early before pnp step in
>
> Both of these patches are to drivers/eisa/pci_eisa.c.  I don't see an
> EISA-specific maintainer, and I caused at least one of these problems,
> so I'll push these through my PCI tree unless somebody objects.

Hi Matthew,

Can you test either the attached patch (against v3.9-rc4) or the git
tree at http://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/log/?h=pci/yinghai-eisa
and send me the complete dmesg log?

If all goes well, I'll ask Linus to pull these before v3.9.

Thanks!

Bjorn

[-- Attachment #2: eisa.patch --]
[-- Type: application/octet-stream, Size: 11443 bytes --]

commit 2cfda637e29ce9e3df31b59f64516b2e571cc985
Author: Yinghai Lu <yinghai@kernel.org>
Date:   Mon Apr 1 11:48:59 2013 -0600

    EISA/PCI: Fix bus res reference
    
    Matthew found that 3.8.3 is having problems with an old (ancient)
    PCI-to-EISA bridge, the Intel 82375. It worked with the 3.2 kernel.
    He identified the 82375, but doesn't assign the struct resource *res
    pointer inside the struct eisa_root_device, and panics.
    
    pci_eisa_init() was using bus->resource[] directly instead of
    pci_bus_resource_n().  The bus->resource[] array is a PCI-internal
    implementation detail, and after commit 45ca9e97 (PCI: add helpers for
    building PCI bus resource lists) and commit 0efd5aab (PCI: add struct
    pci_host_bridge_window with CPU/bus address offset), bus->resource[] is not
    used for PCI root buses any more.
    
    The 82375 is a subtractive-decode PCI device, so handle it the same
    way we handle PCI-PCI bridges in subtractive-decode mode in
    pci_read_bridge_bases().
    
    [bhelgaas: changelog]
    Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
    Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Cc: stable@vger.kernel.org	# v3.3+

diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c
index cdae207..ef5c3ec 100644
--- a/drivers/eisa/pci_eisa.c
+++ b/drivers/eisa/pci_eisa.c
@@ -22,7 +22,8 @@ static struct eisa_root_device pci_eisa_root;
 static int __init pci_eisa_init(struct pci_dev *pdev,
 				const struct pci_device_id *ent)
 {
-	int rc;
+	int rc, i;
+	struct resource *res, *bus_res = NULL;
 
 	if ((rc = pci_enable_device (pdev))) {
 		printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
@@ -30,9 +31,30 @@ static int __init pci_eisa_init(struct pci_dev *pdev,
 		return rc;
 	}
 
+	/*
+	 * The Intel 82375 PCI-EISA bridge is a subtractive-decode PCI
+	 * device, so the resources available on EISA are the same as those
+	 * available on the 82375 bus.  This works the same as a PCI-PCI
+	 * bridge in subtractive-decode mode (see pci_read_bridge_bases()).
+	 * We assume other PCI-EISA bridges are similar.
+	 *
+	 * eisa_root_register() can only deal with a single io port resource,
+	*  so we use the first valid io port resource.
+	 */
+	pci_bus_for_each_resource(pdev->bus, res, i)
+		if (res && (res->flags & IORESOURCE_IO)) {
+			bus_res = res;
+			break;
+		}
+
+	if (!bus_res) {
+		dev_err(&pdev->dev, "No resources available\n");
+		return -1;
+	}
+
 	pci_eisa_root.dev              = &pdev->dev;
-	pci_eisa_root.res	       = pdev->bus->resource[0];
-	pci_eisa_root.bus_base_addr    = pdev->bus->resource[0]->start;
+	pci_eisa_root.res	       = bus_res;
+	pci_eisa_root.bus_base_addr    = bus_res->start;
 	pci_eisa_root.slots	       = EISA_MAX_SLOTS;
 	pci_eisa_root.dma_mask         = pdev->dma_mask;
 	dev_set_drvdata(pci_eisa_root.dev, &pci_eisa_root);

commit c5fb301ae83bec6892e54984e6ec765c47df8e10
Author: Yinghai Lu <yinghai@kernel.org>
Date:   Wed Mar 27 21:28:05 2013 -0700

    EISA/PCI: Init EISA early, before PNP
    
    Matthew reported kernels fail the pci_eisa probe and are later successful
    with the virtual_eisa_root_init force probe without slot0.
    
    The reason for that is: PNP probing is before pci_eisa_init gets called
    as pci_eisa_init is called via pci_driver.
    
    pnp 00:0f has 0xc80 - 0xc84 reserved.
    [    9.700409] pnp 00:0f: [io  0x0c80-0x0c84]
    
    so eisa_probe will fail from pci_eisa_init
    				==>eisa_root_register
    					==>eisa_probe path.
    as force_probe is not set in pci_eisa_root, it will bail early when
    slot0 is not probed and initialized.
    
    Try to use subsys_initcall_sync instead, and will keep following sequence:
    	pci_subsys_init
    	pci_eisa_init_early
    	pnpacpi_init/isapnp_init
    
    After this patch EISA can be initialized properly, and PNP overlapping
    resource will not be reserved.
    [   10.104434] system 00:0f: [io  0x0c80-0x0c84] could not be reserved
    
    Reported-by: Matthew Whitehead <mwhitehe@redhat.com>
    Tested-by: Matthew Whitehead <mwhitehe@redhat.com>
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Cc: stable@vger.kernel.org

diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c
index ef5c3ec..6c3fca9 100644
--- a/drivers/eisa/pci_eisa.c
+++ b/drivers/eisa/pci_eisa.c
@@ -19,8 +19,7 @@
 /* There is only *one* pci_eisa device per machine, right ? */
 static struct eisa_root_device pci_eisa_root;
 
-static int __init pci_eisa_init(struct pci_dev *pdev,
-				const struct pci_device_id *ent)
+static int __init pci_eisa_init(struct pci_dev *pdev)
 {
 	int rc, i;
 	struct resource *res, *bus_res = NULL;
@@ -67,22 +66,26 @@ static int __init pci_eisa_init(struct pci_dev *pdev,
 	return 0;
 }
 
-static struct pci_device_id pci_eisa_pci_tbl[] = {
-	{ PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
-	  PCI_CLASS_BRIDGE_EISA << 8, 0xffff00, 0 },
-	{ 0, }
-};
+/*
+ * We have to call pci_eisa_init_early() before pnpacpi_init()/isapnp_init().
+ *   Otherwise pnp resource will get enabled early and could prevent eisa
+ *   to be initialized.
+ * Also need to make sure pci_eisa_init_early() is called after
+ * x86/pci_subsys_init().
+ * So need to use subsys_initcall_sync with it.
+ */
+static int __init pci_eisa_init_early(void)
+{
+	struct pci_dev *dev = NULL;
+	int ret;
 
-static struct pci_driver __refdata pci_eisa_driver = {
-	.name		= "pci_eisa",
-	.id_table	= pci_eisa_pci_tbl,
-	.probe		= pci_eisa_init,
-};
+	for_each_pci_dev(dev)
+		if ((dev->class >> 8) == PCI_CLASS_BRIDGE_EISA) {
+			ret = pci_eisa_init(dev);
+			if (ret)
+				return ret;
+		}
 
-static int __init pci_eisa_init_module (void)
-{
-	return pci_register_driver (&pci_eisa_driver);
+	return 0;
 }
-
-device_initcall(pci_eisa_init_module);
-MODULE_DEVICE_TABLE(pci, pci_eisa_pci_tbl);
+subsys_initcall_sync(pci_eisa_init_early);

commit d0e36df69863bfa054a4703406a737b0476e68e8
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Mon Apr 1 14:16:56 2013 -0600

    EISA: Use dev_printk() when possible
    
    Use dev_printk() when possible to make messages more useful.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
index 806c77b..0b06df4 100644
--- a/drivers/eisa/eisa-bus.c
+++ b/drivers/eisa/eisa-bus.c
@@ -314,22 +314,22 @@ static int __init eisa_probe(struct eisa_root_device *root)
 {
         int i, c;
 	struct eisa_device *edev;
+	char *enabled_str;
 
-	printk(KERN_INFO "EISA: Probing bus %d at %s\n",
-	       root->bus_nr, dev_name(root->dev));
+	dev_info(root->dev, "Probing EISA bus %d\n", root->bus_nr);
 
 	/* First try to get hold of slot 0. If there is no device
 	 * here, simply fail, unless root->force_probe is set. */
 	
 	edev = kzalloc(sizeof(*edev), GFP_KERNEL);
 	if (!edev) {
-		printk(KERN_ERR "EISA: Couldn't allocate mainboard slot\n");
+		dev_err(root->dev, "EISA: Couldn't allocate mainboard slot\n");
 		return -ENOMEM;
 	}
 		
 	if (eisa_request_resources(root, edev, 0)) {
-		printk(KERN_WARNING \
-		       "EISA: Cannot allocate resource for mainboard\n");
+		dev_warn(root->dev,
+		         "EISA: Cannot allocate resource for mainboard\n");
 		kfree(edev);
 		if (!root->force_probe)
 			return -EBUSY;
@@ -344,11 +344,11 @@ static int __init eisa_probe(struct eisa_root_device *root)
 		goto force_probe;
 	}
 
-	printk(KERN_INFO "EISA: Mainboard %s detected.\n", edev->id.sig);
+	dev_info(&edev->dev, "EISA: Mainboard %s detected\n", edev->id.sig);
 
 	if (eisa_register_device(edev)) {
-		printk(KERN_ERR "EISA: Failed to register %s\n",
-		       edev->id.sig);
+		dev_err(&edev->dev, "EISA: Failed to register %s\n",
+		        edev->id.sig);
 		eisa_release_resources(edev);
 		kfree(edev);
 	}
@@ -358,14 +358,15 @@ static int __init eisa_probe(struct eisa_root_device *root)
         for (c = 0, i = 1; i <= root->slots; i++) {
 		edev = kzalloc(sizeof(*edev), GFP_KERNEL);
 		if (!edev) {
-			printk(KERN_ERR "EISA: Out of memory for slot %d\n", i);
+			dev_err(root->dev, "EISA: Out of memory for slot %d\n",
+				i);
 			continue;
 		}
 
 		if (eisa_request_resources(root, edev, i)) {
-			printk(KERN_WARNING \
-			       "Cannot allocate resource for EISA slot %d\n",
-			       i);
+			dev_warn(root->dev,
+			         "Cannot allocate resource for EISA slot %d\n",
+			         i);
 			kfree(edev);
 			continue;
 		}
@@ -375,38 +376,30 @@ static int __init eisa_probe(struct eisa_root_device *root)
 			kfree(edev);
 			continue;
 		}
-		
-		printk(KERN_INFO "EISA: slot %d : %s detected",
-		       i, edev->id.sig);
-			
-		switch (edev->state) {
-		case EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED:
-			printk(" (forced enabled)");
-			break;
-
-		case EISA_CONFIG_FORCED:
-			printk(" (forced disabled)");
-			break;
-
-		case 0:
-			printk(" (disabled)");
-			break;
-		}
-			
-		printk (".\n");
+
+		if (edev->state == (EISA_CONFIG_ENABLED | EISA_CONFIG_FORCED))
+			enabled_str = " (forced enabled)";
+		else if (edev->state == EISA_CONFIG_FORCED)
+			enabled_str = " (forced disabled)";
+		else if (edev->state == 0)
+			enabled_str = " (disabled)";
+		else
+			enabled_str = "";
+
+		dev_info(&edev->dev, "EISA: slot %d: %s detected%s\n", i,
+			 edev->id.sig, enabled_str);
 
 		c++;
 
 		if (eisa_register_device(edev)) {
-			printk(KERN_ERR "EISA: Failed to register %s\n",
-			       edev->id.sig);
+			dev_err(&edev->dev, "EISA: Failed to register %s\n",
+			        edev->id.sig);
 			eisa_release_resources(edev);
 			kfree(edev);
 		}
         }
 
-	printk(KERN_INFO "EISA: Detected %d card%s.\n", c, c == 1 ? "" : "s");
-
+	dev_info(root->dev, "EISA: Detected %d card%s\n", c, c == 1 ? "" : "s");
 	return 0;
 }
 
diff --git a/drivers/eisa/pci_eisa.c b/drivers/eisa/pci_eisa.c
index 6c3fca9..a333bf3 100644
--- a/drivers/eisa/pci_eisa.c
+++ b/drivers/eisa/pci_eisa.c
@@ -25,8 +25,7 @@ static int __init pci_eisa_init(struct pci_dev *pdev)
 	struct resource *res, *bus_res = NULL;
 
 	if ((rc = pci_enable_device (pdev))) {
-		printk (KERN_ERR "pci_eisa : Could not enable device %s\n",
-			pci_name(pdev));
+		dev_err(&pdev->dev, "Could not enable device\n");
 		return rc;
 	}
 
@@ -59,7 +58,7 @@ static int __init pci_eisa_init(struct pci_dev *pdev)
 	dev_set_drvdata(pci_eisa_root.dev, &pci_eisa_root);
 
 	if (eisa_root_register (&pci_eisa_root)) {
-		printk (KERN_ERR "pci_eisa : Could not register EISA root\n");
+		dev_err(&pdev->dev, "Could not register EISA root\n");
 		return -1;
 	}
 

commit 3a94edb7ba59d3ca30a3a54d8de628f1ccf004e4
Author: Bjorn Helgaas <bhelgaas@google.com>
Date:   Mon Apr 1 14:40:05 2013 -0600

    EISA: Log device resources in dmesg
    
    Note the resources consumed by EISA devices in dmesg, similar to what
    we already do for PCI and PNP devices.
    
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>

diff --git a/drivers/eisa/eisa-bus.c b/drivers/eisa/eisa-bus.c
index 0b06df4..a6fbf98 100644
--- a/drivers/eisa/eisa-bus.c
+++ b/drivers/eisa/eisa-bus.c
@@ -288,6 +288,7 @@ static int __init eisa_request_resources(struct eisa_root_device *root,
 			edev->res[i].flags = IORESOURCE_BUSY;
 		}
 
+		dev_printk(KERN_DEBUG, &edev->dev, "%pR\n", &edev->res[i]);
 		if (request_resource(root->res, &edev->res[i]))
 			goto failed;
 	}

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

* Re: [PATCH 0/2] eisa: fix eisa with PCI
       [not found]             ` <1729353177.11186718.1365018482308.JavaMail.root@redhat.com>
@ 2013-04-03 20:07               ` Bjorn Helgaas
  0 siblings, 0 replies; 18+ messages in thread
From: Bjorn Helgaas @ 2013-04-03 20:07 UTC (permalink / raw)
  To: Matthew Whitehead; +Cc: Yinghai Lu, linux-pci

On Wed, Apr 3, 2013 at 1:48 PM, Matthew Whitehead <mwhitehe@redhat.com> wrote:

> It looks like it fixed /proc/ioports, but demsg is still (null). The boot log is attached.

Huh, I don't see the "(null)" in your patch4_boot.log.  Here's what I see:

[    1.816154] pci 0000:00:07.0: Probing EISA bus 0
[    1.820819] eisa 00:00: [io  0x0c80-0x0c83]
[    1.824092] eisa 00:00: EISA: Mainboard ALR01A4 detected
[    1.833195] eisa 00:04: [io  0x4000-0x40ff]
[    1.836097] eisa 00:04: [io  0x4400-0x44ff]
[    1.840084] eisa 00:04: [io  0x4800-0x48ff]
[    1.844084] eisa 00:04: [io  0x4c00-0x4cff]
[    1.848082] eisa 00:04: EISA: slot 4: TCM5093 detected
[    1.852947] eisa 00:06: [io  0x6000-0x60ff]
[    1.856092] eisa 00:06: [io  0x6400-0x64ff]
[    1.860082] eisa 00:06: [io  0x6800-0x68ff]
[    1.864083] eisa 00:06: [io  0x6c00-0x6cff]
[    1.868083] eisa 00:06: EISA: slot 6: HWP1940 detected (forced enabled)
[    1.873097] eisa 00:07: [io  0x7000-0x70ff]
[    1.876097] eisa 00:07: [io  0x7400-0x74ff]
[    1.880083] eisa 00:07: [io  0x7800-0x78ff]
[    1.884082] eisa 00:07: [io  0x7c00-0x7cff]
[    1.888090] eisa 00:07: EISA: slot 7: TCM5920 detected
[    1.892702] pci 0000:00:07.0: EISA: Detected 3 cards

This now matches exactly what you have in /proc/ioports, which is
good.  Previously there were a bunch of these:

[    1.796731]  (null): [io  0x1000-0x10ff]
[    1.800077]  (null): [io  0x1400-0x14ff]
...

and I couldn't figure out where they came from.  But those were an
artifact of setting up the resources before checking to see whether
there was a device there.  When we discovered there was no device
there, we discarded these resources, so it wasn't harmful, but dmesg
was still ugly.  Now that we check for a device first, we skip the
resources for empty slots 1, 2, 3, 5, and 8, which is much nicer.

Thanks for all your testing!

I put the bugfixes (Yinghai's patches) in my "for-linus" branch for
v3.9.  The other printk fixes and such don't fix any actual bugs, so
I'll put them in my "next" branch for merging into v3.10.

Bjorn

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

end of thread, other threads:[~2013-04-03 20:07 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-28  4:28 [PATCH 0/2] eisa: fix eisa with PCI Yinghai Lu
2013-03-28  4:28 ` [PATCH 1/2] eisa, PCI: Fix bus res reference Yinghai Lu
2013-03-28 12:52   ` Bjorn Helgaas
2013-03-28 15:16     ` Yinghai Lu
2013-03-28 16:14       ` Bjorn Helgaas
2013-03-28 16:54         ` Yinghai Lu
2013-03-29 18:10   ` Bjorn Helgaas
2013-03-29 19:10     ` Yinghai Lu
2013-03-29 22:30       ` Bjorn Helgaas
2013-03-28  4:28 ` [PATCH 2/2] eisa, PCI: init eisa early before pnp step in Yinghai Lu
2013-03-29 22:22   ` Bjorn Helgaas
2013-03-30  0:18     ` Yinghai Lu
2013-04-01 18:15       ` Bjorn Helgaas
2013-04-01 18:25         ` Yinghai Lu
2013-04-01 18:52           ` Bjorn Helgaas
2013-03-29 15:16 ` [PATCH 0/2] eisa: fix eisa with PCI Bjorn Helgaas
2013-04-01 21:09   ` Bjorn Helgaas
     [not found]     ` <707419406.5567095.1364921996288.JavaMail.root@redhat.com>
     [not found]       ` <20130402172441.GA22478@google.com>
     [not found]         ` <979994016.6599283.1364940427573.JavaMail.root@redhat.com>
     [not found]           ` <20130403000731.GA27329@google.com>
     [not found]             ` <1729353177.11186718.1365018482308.JavaMail.root@redhat.com>
2013-04-03 20:07               ` Bjorn Helgaas

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.