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
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 <[email protected]>
Tested-by: Matthew Whitehead <[email protected]>
Signed-off-by: Yinghai Lu <[email protected]>
Cc: [email protected]
---
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);
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 <[email protected]>
Tested-by: Matthew Whitehead <[email protected]>
Signed-off-by: Yinghai Lu <[email protected]>
Cc: [email protected]
---
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);
On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
> Tested-by: Matthew Whitehead <[email protected]>
> Signed-off-by: Yinghai Lu <[email protected]>
> Cc: [email protected]
You are consistently using the wrong stable email address. It should
be "[email protected]". I assume you see a bounce every time; at
least, I get a bounce when I reply to your messages.
On Thu, Mar 28, 2013 at 5:52 AM, Bjorn Helgaas <[email protected]> wrote:
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
>> Tested-by: Matthew Whitehead <[email protected]>
>> Signed-off-by: Yinghai Lu <[email protected]>
>> Cc: [email protected]
>
> You are consistently using the wrong stable email address. It should
> be "[email protected]". 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 [email protected] will never get fixed?
Yinghai
On Thu, Mar 28, 2013 at 9:16 AM, Yinghai Lu <[email protected]> wrote:
> On Thu, Mar 28, 2013 at 5:52 AM, Bjorn Helgaas <[email protected]> wrote:
>> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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: [email protected]" 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 <[email protected]>
>>> Tested-by: Matthew Whitehead <[email protected]>
>>> Signed-off-by: Yinghai Lu <[email protected]>
>>> Cc: [email protected]
>>
>> You are consistently using the wrong stable email address. It should
>> be "[email protected]". 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 [email protected] will never get fixed?
What's to fix? The doc (Documentation/stable_kernel_rules.txt) says
to use "Cc: [email protected]" and using "Cc: [email protected]"
causes bounces. Just use the documented address like everybody else
and everything will be fine.
If you want "[email protected]" to work, first make the kernel.org
change, then change Documentation/stable_kernel_rules.txt, *then*
start using it in your patches.
Bjorn
On Thu, Mar 28, 2013 at 9:14 AM, Bjorn Helgaas <[email protected]> wrote:
>>
>> So [email protected] will never get fixed?
>
> What's to fix? The doc (Documentation/stable_kernel_rules.txt) says
> to use "Cc: [email protected]" and using "Cc: [email protected]"
> 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 [email protected]
in patch cc.
please check git log -p in recent commits in linus tree.
>
> If you want "[email protected]" to work, first make the kernel.org
> change, then change Documentation/stable_kernel_rules.txt, *then*
> start using it in your patches.
[email protected] is not restored after last kernel.org get breached.
And I thought [email protected] should be just temporary one.
Yinghai
[+cc linux-pci]
On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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
On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
> Tested-by: Matthew Whitehead <[email protected]>
> Signed-off-by: Yinghai Lu <[email protected]>
> Cc: [email protected]
>
> ---
> 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);
On Fri, Mar 29, 2013 at 11:10 AM, Bjorn Helgaas <[email protected]> wrote:
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
>> Tested-by: Matthew Whitehead <[email protected]>
>> Signed-off-by: Yinghai Lu <[email protected]>
>> Cc: [email protected]
>>
>> ---
>> 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.
[+cc Rafael, just FYI since it involves PNP resources]
On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
> Tested-by: Matthew Whitehead <[email protected]>
> Signed-off-by: Yinghai Lu <[email protected]>
> Cc: [email protected]
>
> ---
> 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);
On Fri, Mar 29, 2013 at 12:10:37PM -0700, Yinghai Lu wrote:
> On Fri, Mar 29, 2013 at 11:10 AM, Bjorn Helgaas <[email protected]> wrote:
> > On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
> >> Tested-by: Matthew Whitehead <[email protected]>
> >> Signed-off-by: Yinghai Lu <[email protected]>
> >> Cc: [email protected]
> >>
> >> ---
> >> 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);
On Fri, Mar 29, 2013 at 3:22 PM, Bjorn Helgaas <[email protected]> wrote:
> [+cc Rafael, just FYI since it involves PNP resources]
>
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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 <[email protected]>
>> Tested-by: Matthew Whitehead <[email protected]>
>> Signed-off-by: Yinghai Lu <[email protected]>
>> Cc: [email protected]
>>
>> ---
>> 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);
On Fri, Mar 29, 2013 at 6:18 PM, Yinghai Lu <[email protected]> wrote:
> On Fri, Mar 29, 2013 at 3:22 PM, Bjorn Helgaas <[email protected]> wrote:
>> [+cc Rafael, just FYI since it involves PNP resources]
>>
>> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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
On Mon, Apr 1, 2013 at 11:15 AM, Bjorn Helgaas <[email protected]> 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
On Mon, Apr 1, 2013 at 12:25 PM, Yinghai Lu <[email protected]> wrote:
> On Mon, Apr 1, 2013 at 11:15 AM, Bjorn Helgaas <[email protected]> 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
On Fri, Mar 29, 2013 at 9:16 AM, Bjorn Helgaas <[email protected]> wrote:
> [+cc linux-pci]
>
> On Wed, Mar 27, 2013 at 10:28 PM, Yinghai Lu <[email protected]> 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