On Tue, Sep 10, 2013 at 11:37:44PM +0800, Huang Rui wrote: > > > +EXPORT_SYMBOL_GPL(usb_amd_remote_wakeup_quirk); > > > > Nothing calls this function, so why have you added it here? > > > > The way you have broken up the patches is a bit odd. You are creating > > functions that aren't called yet, but do not describe this in the > > changelog information, so people reviewing the patch would just assume > > they are not needed at all (hint, like I just did...) > > > > Please provide a better description, and justification, if you create a > > new function, otherwise we will assume it is not needed at all. > > > > Actually, I have a little confused. Assume that we write a core level > function, and it would be called in host controller level such as > xhci-*, ehci-*. Should I split the codes from core and host in two > pathes or write one patch included core and host level updates? No, but in the patch that you add the new function, say that you are going to use it in future patches, otherwise people think it's not needed at all. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html