Re: [PATCH] PCI, EDAC: fix ordering assign resource and bus_add

[Bjorn Helgaas]

On Fri, May 30, 2014 at 3:46 PM, Borislav Petkov <bp@xxxxxxxxx> wrote:
> On Fri, May 30, 2014 at 11:01:03AM -0600, Bjorn Helgaas wrote:
>> On Tue, May 07, 2013 at 04:29:31PM -0700, Yinghai Lu wrote:
>> > We should assign unassigned resource before pci_bus_add_device.
>> >
>> > as late one will enable driver and create sysfs file that will need
>> > pci io resources from assign unassigned code.
>> >
>> > Signed-off-by: Yinghai Lu <yinghai@xxxxxxxxxx>
>>
>> Applied to pci/resource for v3.16, thanks!
>
> This is touching drivers/edac/ and I guess I'm fine with it,
>
> Acked-by: Borislav Petkov <bp@xxxxxxx>

Thanks, I should have waited for you.  I was assuming the merge window
would open on Monday, and I hoped for a day or two in -next, but now
it sounds like we might have an -rc8, so I needn't have been in such a
hurry.

>> > ---
>> >  drivers/edac/i82875p_edac.c |    3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > Index: linux-2.6/drivers/edac/i82875p_edac.c
>> > ===================================================================
>> > --- linux-2.6.orig/drivers/edac/i82875p_edac.c
>> > +++ linux-2.6/drivers/edac/i82875p_edac.c
>> > @@ -293,13 +293,14 @@ static int i82875p_setup_overfl_dev(stru
>> >             if (dev == NULL)
>> >                     return 1;
>> >
>> > +           pci_bus_assign_resources(dev->bus);
>> > +
>> >             err = pci_bus_add_device(dev);
>> >             if (err) {
>> >                     i82875p_printk(KERN_ERR,
>> >                             "%s(): pci_bus_add_device() Failed\n",
>> >                             __func__);
>> >             }
>> > -           pci_bus_assign_resources(dev->bus);a
>
>
> ... one question though: how are people using those pci_bus*
> functions supposed to know that pci_bus_assign_resources needs to
> go before pci_bus_add_device? The comment in pci_bus_add_device()
> mentions something about it but wouldn't it be possible to call
> pci_bus_assign_resources() in pci_bus_add_device() when resources are
> not assigned?

Yeah, this is really a mess.  The pci_bus_*() functions (at least
pci_bus_add_device() and pci_bus_assign_resources()) are not really
intended to be called by drivers at all -- they should only be used by
the arch code that drives PCI enumeration.  Eventually I'd like to
even move them out of the arch code and into the PCI core.

In this i82875p case, it looks like this is basically a quirk that
unhides a device.  I think it's sort of dubious to poke at things the
BIOS has explicitly hidden from the OS, but apart from that, if we
turned this into an actual DECLARE_PCI_FIXUP_CLASS_EARLY() quirk on
the 82875_0 device, I think the normal PCI enumeration process would
find the 82875_6 device, and we could get rid of the whole "if (dev ==
NULL)" chunk.

Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/