On Fri, Sep 14, 2012 at 12:55 PM, Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx> wrote: > On Fri, Sep 07, 2012 at 10:22:28AM -0700, Bjorn Helgaas wrote: >> On Fri, Sep 7, 2012 at 10:00 AM, Thierry Reding >> <thierry.reding@xxxxxxxxxxxxxxxxx> wrote: >> > On Fri, Sep 07, 2012 at 10:19:46AM -0600, Stephen Warren wrote: >> >> On 08/15/2012 01:42 PM, Bjorn Helgaas wrote: >> >> > On Wed, Aug 15, 2012 at 1:28 PM, Thierry Reding >> >> > <thierry.reding@xxxxxxxxxxxxxxxxx> wrote: >> >> >> On Wed, Aug 15, 2012 at 10:06:27AM -0700, Bjorn Helgaas wrote: >> >> >>> On Thu, Jul 26, 2012 at 12:55 PM, Thierry Reding >> >> >>> <thierry.reding@xxxxxxxxxxxxxxxxx> wrote: >> >> >>>> When using deferred driver probing, PCI host controller drivers may >> >> >>>> actually require this function after the init stage. >> >> >>>> >> >> >>>> Signed-off-by: Thierry Reding <thierry.reding@xxxxxxxxxxxxxxxxx> >> >> >>>> --- >> >> >>>> Changes in v3: >> >> >>>> - none >> >> >>>> >> >> >>>> Changes in v2: >> >> >>>> - use __devinit annotations >> >> >>> >> >> >>> Your original patch removed __init completely. Here you change it to >> >> >>> __devinit. That means we decide whether to discard the function based >> >> >>> on whether CONFIG_HOTPLUG is supported. But I think your point is not >> >> >>> about hotplug; it's merely that we should be able to scan a PCI bus >> >> >>> after init-time. We ought to be able to do a late PCI scan even if >> >> >>> hotplug is not supported. >> >> >>> >> >> >>> Therefore, I'd be inclined to remove __init completely unless you have >> >> >>> another reason for preferring __devinit. >> >> >> >> >> >> I thought __devinit would resolve to nothing if HOTPLUG is defined and >> >> >> __init otherwise. That seemed more appropriate. However you are right >> >> >> that it is useful to always have it available, so I'm fine with removing >> >> >> the annotations altogether. Do you want me to follow up with a patch? Or >> >> >> can you just take the first version? I'm not sure if it still applies. >> >> > >> >> > You're right about how __devinit works. It's just that I don't think >> >> > hotplug is actually relevant here. We're trying to make >> >> > pci_fixup_irqs() work after init, whether it's because of hotplug or >> >> > simply because the arch scans host bridges after init. >> >> > >> >> > I applied this to my "next" branch. Thanks! >> >> >> >> Bjorn, I don't see this patch in next-20120907. Did it get dropped for >> >> some reason? >> > >> > Yes, it turns out that dropping the annotations causes lots of section >> > mismatches on other architectures. See here[0] for the details. I think >> > the solution to the issue would be to either remove HOTPLUG altogether >> > and drop __devinit and __devexit annotations or update all architectures >> > to fix these warnings. I think Bjorn and I settled on the latter because >> > it's obviously less intrusive. I've been busy building toolchains for >> > all the PCI architectures and I think I have all of them. I'll just need >> > some more time to build, find and fix any remaining section mismatches. >> >> Greg KH is actively removing CONFIG_HOTPLUG altogether -- see >> https://lkml.org/lkml/2012/9/4/489 >> >> That will make __devinit resolve to nothing in all cases, and we'll >> eventually remove __devinit completely. So we don't want to convert >> __init to __devinit; we have to remove the __init altogether. I think >> that means we have to update all arches first to avoid the section >> mismatches. >> >> So I think Thierry is on the right track: >> 1) Change all arch pcibios_update_irq() implementations (and >> probably a few other things) to be non-__init >> 2) Change pci_fixup_irqs() to be non-__init > > Okay this wasn't as much work as I had thought. It turns out that there > very few dependencies other than pcibios_update_irq(). Actually none at > all. In addition some of the architectures already had these annotated > with __devinit. Luckily those were the ones I wasn't able to build a > cross-compiler for. Note that I resolved to converting all annotations > from __init to __devinit first. I'll try to find out what exactly Greg > is planning to do. If it turns out that the plan is to just remove the > __devinit incrementally I could just drop all of these. Otherwise maybe > a better option would be to leave them in and remove them all at once > when the HOTPLUG symbol is removed. > > Furthermore it seems like almost all implementations are the same, so I > was going to include a patch that moves the common implementation to the > PCI core and make it a weak symbol so architectures would still have the > possibility to override it. > > The only exception to this is 64-bit SPARC, which apparently doesn't do > anything in pcibios_update_irq(). I wonder if it would hurt to just use > the generic implementation anyway, which just sets a byte in the > configuration space. That should work regardless of architecture, right? > > Unicore32 and ARM also output a debugging message and maybe it would be > okay to include that with the generic implementation as well. Do you > have any objections? Sounds good to me. DaveM is really responsive -- if you propose using the generic implementation on SPARC, I bet he'd either ack it or tell you why SPARC needs to be special. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
