On Mon, Dec 16, 2013 at 1:07 PM, David Cohen <david.a.cohen@xxxxxxxxxxxxxxx> wrote: > This code was partially based on Mark Brown's previous work. > > Signed-off-by: David Cohen <david.a.cohen@xxxxxxxxxxxxxxx> > Signed-off-by: Fei Yang <fei.yang@xxxxxxxxx> > Cc: Mark F. Brown <mark.f.brown@xxxxxxxxx> > Cc: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx> I know this has already been merged to Linus' tree, but it looks funny to me. > --- a/arch/x86/platform/intel-mid/intel_mid_weak_decls.h > +++ b/arch/x86/platform/intel-mid/intel_mid_weak_decls.h > @@ -16,3 +16,4 @@ > */ > extern void * __cpuinit get_penwell_ops(void) __attribute__((weak)); > extern void * __cpuinit get_cloverview_ops(void) __attribute__((weak)); > +extern void * __init get_tangier_ops(void) __attribute__((weak)); We should use "__weak" instead of the gcc-specific "__attribute__((weak))". I don't think it's a good idea to use __weak on a declaration in a header file. If there are ever multiple definitions of the symbol, they are *all* made weak symbols, and one is chosen based on link order, which is error-prone. I only see one definition now, but the whole point of weak is to allow multiple definitions, so this looks like a problem waiting to happen. See 10629d711ed, for example. It look me a bit to figure out that these get_*_ops() functions are used by INTEL_MID_OPS_INIT, which constructs the name using a macro, so grep/cscope/etc. don't see any users. A comment pointing to INTEL_MID_OPS_INIT would be helpful. What's the reason for making these symbols weak? Normally we use weak to make a generic default version of a function, while allowing architectures to replace the default with their own version if necessary. But I don't see that happening here. Maybe I'm just missing it, like I missed the uses of get_tangier_ops(), et al. Bjorn -- 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
