Re: [PATCH v4 06/12] PCI: Introduce pci_rescan_bus()

[Greg KH]

On Thu, Mar 19, 2009 at 11:49:38AM -0600, Alex Chiang wrote:
> * Greg KH <greg@xxxxxxxxx>:
> > On Thu, Mar 19, 2009 at 11:05:48AM -0600, Alex Chiang wrote:
> > > * Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>:
> > > > On Wed, 18 Mar 2009 16:39:56 -0600 Alex Chiang <achiang@xxxxxx> wrote:
> > > > 
> > > > >  EXPORT_SYMBOL(pci_add_new_bus);
> > > > >  EXPORT_SYMBOL(pci_scan_slot);
> > > > >  EXPORT_SYMBOL(pci_scan_bridge);
> > > > >  EXPORT_SYMBOL_GPL(pci_scan_child_bus);
> > > > > +EXPORT_SYMBOL_GPL(pci_rescan_bus);
> > > > 
> > > > uhm, is there any rationale for the seemingly-random mixup of export types
> > > > in this interface?
> > > 
> > > History?
> > > 
> > > I've no clue why we're mixing EXPORT_SYMBOL and EXPORT_SYMBOL_GPL.
> > 
> > I do :)
> > 
> > New PCI core exports were added with _GPL, it's the older ones that were
> > left at the "normal" level.
> > 
> > > git-blame says that the mishmash existed pre-git.
> > > 
> > > For my new interface, I thought that EXPORT_SYMBOL_GPL would be
> > > most appropriate, but maybe not?
> > 
> > Yes it is, pci hotplug exports has always been EXPORT_SYMBOL_GPL(), in
> > fact I think it was the first in-tree user of this marking.
> > 
> > > What would you like me to do? Make them all the same?
> > 
> > New ones should be _GPL() please.
> > 
> > But please put the export in the proper place, checkpatch.pl will
> > complain if you do not.  Which means you didn't run it on your patches
> > :)
> 
> Andrew already yelled at me privately about checkpatch.
> 
> I went over and ran it on all my patches, and that was indeed one
> of the complaints.
> 
> But when it comes to file consistency vs "letter of the law" I
> like to keep things consistent (even if broken) in the file.
> 
> I could go back and move all those EXPORT_SYMBOL declarations to
> the "proper" place. Is that too much noise though?

Too much noise for this patch, yes.

Put things in the right place for new patches/changes.

If you want to go back and just make a "fix this file to be checkpatch
clean" patch, that's fine as well, but don't merge it with other changes
at the same time, as that makes it pretty unreviewable.

Hope this helps,

greg k-h
--
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/