11.06.2021 14:00, Thierry Reding пишет: > On Fri, Jun 11, 2021 at 10:21:41AM +0300, Dmitry Osipenko wrote: >> 11.06.2021 09:50, Krzysztof Kozlowski пишет: >>> On 10/06/2021 18:23, Dmitry Osipenko wrote: >>>> 10.06.2021 18:50, Dmitry Osipenko пишет: >>>>> 10.06.2021 09:43, Krzysztof Kozlowski пишет: >>>>>> The stubs might be good idea anyway, but the driver explicitly needs for >>>>>> runtime working reservedmem, so it should select it. >>>>> >>>>> The OF and reservedmem are both selected by the ARCH for the runtime >>>>> use. They may not be selected in the case of compile-testing. >>>>> >>>>> Both OF core and reservedmem provide stubs needed for compile-testing, >>>>> it's only the RESERVEDMEM_OF_DECLARE() that is missing the stub. Adding >>>>> the missing stub should be a more appropriate solution than adding extra >>>>> Kconfig dependencies, IMO. >>> >>> Ah, in such case everything looks good. Stubs is indeed proper choice. >> >> Although, I see that there are only two Kconfigs that have >> OF_RESERVED_MEM, one defines the OF_RESERVED_MEM, the other is QCOM >> Kconfig which depends on OF_RESERVED_MEM. The OF_RESERVED_MEM is enabled >> by default in defconfig. >> >> You're right, we need the Kconfig change to be entirely correct, since >> driver won't work properly without OF_RESERVED_MEM. >> >> config TEGRA210_EMC >> tristate "NVIDIA Tegra210 External Memory Controller driver" >> - depends on ARCH_TEGRA_210_SOC || COMPILE_TEST >> + depends on (ARCH_TEGRA_210_SOC && OF_RESERVED_MEM) || COMPILE_TEST >> >> I will send that change later today. > > That's completely unnecessary. OF_RESERVED_MEM is enabled by default if > OF_EARLY_FLATTREE is enabled, which it is for ARM64 and that is always > enabled for ARCH_TEGRA_210_SOC. But it doesn't stop you from disabling OF_RESERVED_MEM. The Kconfig dependencies should reflect the build and runtime requirements of the driver, otherwise only driver author knows which config options are need. > What Krzysztof had originally proposed, as far as I understand, is to > add "depends on OF_RESERVED_MEM" so that the dependency is always there > (including the COMPILE_TEST case). However, that's a bit problematic, as > I said earlier, because OF_RESERVED_MEM is not user-visible and neither > is OF_EARLY_FLATTREE, so there's no way to enable OF_RESERVED_MEM unless > the architecture selected it, which it doesn't on x86, so it kind of > defeats the purpose of COMPILE_TEST. Indeed, the QCOM driver isn't compile-tested as much as it could be. That driver already shouldn't have any problems with compile-testing, maybe some stubs were missing when driver was originally added. > So I think if this really has to be compile-test enabled, the only way > to do that is to either make this select OF_EARLY_FLATTREE, or add the > stubs. > > Another option would perhaps be to enable OF_UNITTEST along with > COMPILE_TEST, since that also pulls in OF_EARLY_FLATTREE and would allow > this driver to be built even on x86. For the universal compile-testing it should be enough to fix the stub. We may consider other options if it won't be enough, thank you for the suggestions.
