* 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; 3+ 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@vger.kernel.org, linux-pci@vger.kernel.org
[-- 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] 3+ messages in thread