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

[Krzysztof Kozlowski]

On 11/06/2021 15:40, Dmitry Osipenko wrote:
> 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.

OF_RESERVED_MEM is not selectable, so it cannot be disabled for regular
builds.

When I proposed to add dependencies, I indeed did not check whether
these are selectable symbols. My advise was not accurate.

Usually we do not add dependencies to each driver on each kernel
non-selectable feature. It would be too much (depends on HAS_IOMEM,
REGMAP, OF, MFD_SYSCON, OF_RESERVED_MEM and probably many more)... There
are of course exceptions and mistakes (I think I should not add
OF_ADDRESS to OMAP_GPMC).

If such features are non-selectable, usually architecture enforces them
so the driver does not need to. For compile testing, these features
should come with stubs. If there are no stubs, the dependency for
compile testing could be added:
ARCH_TEGRA_210_SOC || (COMPILE_TEST && OF_RESERVED_MEM)

We add the dependencies for selectable options which driver needs, e.g.
DEVFREQ, CPUFREQ, PM_xxx

Since you proposed already to add stubs for OF_RESERVED_MEM, adding
dependency on it does not bring any benefit.

Best regards,
Krzysztof