On Friday 16 October 2015 14:26:27 Stephen Boyd wrote: > On 10/16, Arnd Bergmann wrote: > > On Friday 16 October 2015 13:04:17 Stephen Boyd wrote: > > > On 10/16, Arnd Bergmann wrote: > > > > On Friday 16 October 2015 09:56:30 Stephen Boyd wrote: > > > > > > > > > > Can you share your .config? It looks like there are stubs for these, so > > > > > I'm lost how we got undefined references. > > > > > > > > > http://pastebin.com/HtrC510p > > > > > > > > The problem is CONFIG_HWSPINLOCK=m && CONFIG_QCOM_SMEM=y. Sorry for sending > > > > an incomplete patch description, I had not noticed those wrapper functions. > > > > > > > > My patch fixes the issue, but does not allow you to build QCOM_SMEM without > > > > HWSPINLOCK. If we want that configuration to be valid, we need one of > > > > the two changes below: > > > > > > > > diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig > > > > index eec76141d9b9..cae0ffa19bca 100644 > > > > --- a/drivers/soc/qcom/Kconfig > > > > +++ b/drivers/soc/qcom/Kconfig > > > > @@ -22,7 +22,7 @@ config QCOM_PM > > > > config QCOM_SMEM > > > > tristate "Qualcomm Shared Memory Manager (SMEM)" > > > > depends on ARCH_QCOM > > > > - depends on HWSPINLOCK > > > > + depends on HWSPINLOCK && !HWSPINLOCK > > > > > > What does this mean? > > > > This is the magic way in Kconfig to express what we want ;-) > > > > It says it depends on either HWSPINLOCK being disabled (!HWSPINLOCK), > > or it depends on HWSPINLOCK. The latter implies that QCOM_SMEM cannot > > be built-in when HWSPINLOCK=m, but they are allowed to both be built-in. > > > > I think you mean OR then? > > depends on HWSPINLOCK || !HWSPINLOCK Yes, sorry about that. > If I have HWSPINLOCK=y and try menuconfig with your patch, the > smem config isn't selectable. But if HWSPINLOCK=m then the smem > config is visible and smem can only be a module. Ok, I have to admit that I don't even know how Kconfig interprets (FOO && !FOO), so that's probably the result of my typo. > > > > > > help > > > > Say y here to enable support for the Qualcomm Shared Memory Manager. > > > > The driver provides an interface to items in a heap shared among all > > > > diff --git a/include/linux/hwspinlock.h b/include/linux/hwspinlock.h > > > > index 859d673d98c8..cdfd9fd2ba11 100644 > > > > --- a/include/linux/hwspinlock.h > > > > +++ b/include/linux/hwspinlock.h > > > > @@ -59,7 +59,7 @@ struct hwspinlock_pdata { > > > > int base_id; > > > > }; > > > > > > > > -#if defined(CONFIG_HWSPINLOCK) || defined(CONFIG_HWSPINLOCK_MODULE) > > > > +#if IS_REACHABLE(CONFIG_HWSPINLOCK) > > > > > > > > int hwspin_lock_register(struct hwspinlock_device *bank, struct device *dev, > > > > const struct hwspinlock_ops *ops, int base_id, int num_locks); > > > > > > > > > > I'd prefer this patch. Is there some way to capture that > > > QCOM_SMEM must be a module if HWSPINLOCK is a module unless we're > > > doing a compile test, in which case we don't care? > > > > The problem here is that while we avoid the build error, we get the > > counterintuitive behavior that you can have the HWSPINLOCK module > > loaded but the built-in QCOM_SMEM module does not use it because > > it wasn't reachable at compile time. > > > > Agreed, which is why I suggested depends on HWSPINLOCK || > COMPILE_TEST + the IS_REACHABLE patch. That way if hwspinlock=y, > smem can be y or m, if hwspinlock=m, smem can only be m, and if > hwspinlock=n then smem can only be y or m if it has > COMPILE_TEST=y. In the last case the IS_REACHABLE ifdef will "do > the right thing" and make it so that HWSPINLOCK symbols are > stubbed out if hwspinlock=m and smem=y or hwspinlock=n and > smem=y/m. Ah, I see. The downside is still that other users of HWSPINLOCK might now accidentally build but not work if HWSPINLOCK=m. > I don't think we ever care about a kernel that actually runs when > COMPILE_TEST=y, so this seems to be ok. Right, for the QCOM_SMEM support that is clearly ok as long as you have the Kconfig logic there. Arnd -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
