Re: linux-next: build warning after merge of the tegra tree

From: Dmitry Osipenko <digetx@gmail.com>
To: Thierry Reding <treding@nvidia.com>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
	Colin Cross <ccross@android.com>, Olof Johansson <olof@lixom.net>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux Next Mailing List <linux-next@vger.kernel.org>
Subject: Re: linux-next: build warning after merge of the tegra tree
Date: Thu, 3 Jun 2021 17:37:34 +0300	[thread overview]
Message-ID: <a5ed712a-3fb8-57c8-8738-090bfc23e7be@gmail.com> (raw)
In-Reply-To: <YLjomqomVuJ3QZNC@orome.fritz.box>

03.06.2021 17:35, Thierry Reding пишет:
> On Thu, Jun 03, 2021 at 05:03:38PM +0300, Dmitry Osipenko wrote:
>> 03.06.2021 15:18, Thierry Reding пишет:
>>> On Thu, Jun 03, 2021 at 05:01:48AM +0300, Dmitry Osipenko wrote:
>>>> 03.06.2021 03:35, Stephen Rothwell пишет:
>>>>> Hi all,
>>>>>
>>>>> After merging the tegra tree, today's linux-next build (x86_64
>>>>> allmodconfig) produced this warning:
>>>>>
>>>>> WARNING: unmet direct dependencies detected for TEGRA210_EMC_TABLE
>>>>>   Depends on [n]: MEMORY [=y] && TEGRA_MC [=y] && ARCH_TEGRA_210_SOC [=n]
>>>>>   Selected by [m]:
>>>>>   - TEGRA210_EMC [=m] && MEMORY [=y] && TEGRA_MC [=y] && (ARCH_TEGRA_210_SOC [=n] || COMPILE_TEST [=y])
>>>>>
>>>>> Probably introduced by commit
>>>>>
>>>>>   08decdd5b448 ("memory: tegra: Enable compile testing for all drivers")
>>>>>
>>>>
>>>> Thank you. This is a new warning to me, apparently this case wasn't previously tested by kernel build bot.
>>>>
>>>> Perhaps this should fix it:
>>>>
>>>> diff --git a/drivers/memory/tegra/Kconfig b/drivers/memory/tegra/Kconfig
>>>> index 71bba2345bce..3f2fa7750118 100644
>>>> --- a/drivers/memory/tegra/Kconfig
>>>> +++ b/drivers/memory/tegra/Kconfig
>>>> @@ -47,7 +47,6 @@ config TEGRA124_EMC
>>>>  
>>>>  config TEGRA210_EMC_TABLE
>>>>  	bool
>>>> -	depends on ARCH_TEGRA_210_SOC
>>>
>>> Why not just add a || COMPILE_TEST like we do for TEGRA210_EMC? Because
>>> TEGRA210_EMC being pulled in under COMPILE_TEST (and then pulling in
>>> TEGRA210_EMC_TABLE which is missing the alternative path) seems to be
>>> the root cause for this.
>>
>> The anonymous Kconfig entry is unavailable by default, it can be only
>> selected by other entry, IIUC. In our case the TEGRA210_EMC_TABLE is
>> selected by TEGRA210_EMC, hence additional dependencies aren't needed
>> for TEGRA210_EMC_TABLE.
> 
> The code guarded by TEGRA210_EMC_TABLE makes use of some symbols that
> are only available if ARCH_TEGRA_210_SOC is also defined. If we don't
> list the dependencies via Kconfig this could lead to a problem where
> somebody selected TEGRA210_EMC_TABLE without having a dependency on
> ARCH_TEGRA_210_SOC, which would then lead to a build error.
> 
> If we do represent the dependency in Kconfig, we'll get a warning like
> the above during the configuration step and the offending Kconfig option
> will end up disabled, and avoid the build failure.
> 
> Granted, this could be caught during patch review, and yes, there's not
> technically a need to encode this using Kconfig dependencies, but at the
> same time there's also no reason not to use the safeguards we have at
> our disposal to avoid this in a more automated way.
> 
> I'd prefer to stick with the explicit dependency in Kconfig, so I've
> updated the patch to match the dependencies to that of TEGRA210_EMC.

I don't mind if you prefer this explicit approach more. Thank you for
the fix.