Re: [PATCH 1/2] memory: tegra: Add missing dependencies

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2564 bytes --]

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.

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.

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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]