"Hiremath, Vaibhav" <hvaibhav@xxxxxx> writes: > On Wed, May 09, 2012 at 04:08:09, Hilman, Kevin wrote: >> Vaibhav Hiremath <hvaibhav@xxxxxx> writes: >> >> > The function __omap2_set_globals() can be common across all >> > platforms/architectures, even in case of omap4, internally it >> > calls same set of functions as in __omap2_set_globals() function >> > (except for sdrc). >> >> OK so far. >> >> > This patch adds new config flag SOC_HAS_OMAP2_SDRC to handle sdrc, >> > so that we can reuse same function across omap2/3/4... >> >> But what happens when a single kernel is built that has support for an >> SoC with an SDRC (OMAP4) and one that doesn't (AM33xx)? >> > > As such Nothing...I looking into this direction while implementing. > > In that case, sdrc.c file will be compiled in and execution will jump to > omap2_set_globals_sdrc(). But inside this function, we are already checking > whether the omap2_globals->sdrc and omap2_globals->sms for NULL and then use > it. > > And function omap2_sdrc_init() is also depends on machine, so in case of > Am33xx, it won't get into sdrc execution at all. And in case of omap4, it > will. Then why bother with the #ifdef at all? If it already safe to call on all SoCs, just get rid of the #ifdef all together. >> In that case this new SOC_HAS_OMAP2_SDRC will be set, and >> set_globals_sdrc() will be called even for the SoCs without SDRC. >> >> So, rather than add a new Kconfig option for this, I would rather see >> you using the existing runtime feature check for the SDRC: omap_has_sdrc() >> > > There is NO difference between runtime feature check Vs this patch, refer to the function implementation, > > void __init omap2_set_globals_sdrc(struct omap_globals *omap2_globals) > { > if (omap2_globals->sdrc) > omap2_sdrc_base = omap2_globals->sdrc; > if (omap2_globals->sms) > omap2_sms_base = omap2_globals->sms; > } > > The initialization happens after checking for NULL, so even if you execute it or not, the variable are set to NULL in case of am33xx. > > So I don't find any difference between runtime and this patch. Except readability. Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html