Re: [PATCH v4 14/16] PM / devfreq: tegra: Enable COMPILE_TEST for the driver

Dmitry Osipenko
2019-06-04T15:15:45Z

04.06.2019 17:50, Thierry Reding пишет:
> On Tue, Jun 04, 2019 at 05:41:53PM +0300, Dmitry Osipenko wrote:
>> 04.06.2019 17:18, Thierry Reding пишет:
>>> On Tue, Jun 04, 2019 at 04:10:31PM +0200, Thierry Reding wrote:
>>>> On Tue, Jun 04, 2019 at 04:53:17PM +0300, Dmitry Osipenko wrote:
>>>>> 04.06.2019 14:20, Thierry Reding пишет:
>>>>>> On Thu, May 02, 2019 at 02:38:13AM +0300, Dmitry Osipenko wrote:
>>>>>>> The driver's compilation doesn't have any specific dependencies, hence
>>>>>>> the COMPILE_TEST option can be supported in Kconfig.
>>>>>>>
>>>>>>> Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
>>>>>>> ---
>>>>>>> drivers/devfreq/Kconfig | 2 +-
>>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/devfreq/Kconfig b/drivers/devfreq/Kconfig
>>>>>>> index 56db9dc05edb..a6bba6e1e7d9 100644
>>>>>>> --- a/drivers/devfreq/Kconfig
>>>>>>> +++ b/drivers/devfreq/Kconfig
>>>>>>> @@ -93,7 +93,7 @@ config ARM_EXYNOS_BUS_DEVFREQ
>>>>>>>
>>>>>>> config ARM_TEGRA_DEVFREQ
>>>>>>> tristate "NVIDIA Tegra30/114/124/210 DEVFREQ Driver"
>>>>>>> - depends on ARCH_TEGRA
>>>>>>> + depends on ARCH_TEGRA || COMPILE_TEST
>>>>>>> select PM_OPP
>>>>>>> help
>>>>>>> This adds the DEVFREQ driver for the Tegra family of SoCs.
>>>>>>
>>>>>> You need to be careful with these. You're using I/O register accessors,
>>>>>> which are not supported on the UM architecture, for example.
>>>>>>
>>>>>> This may end up getting flagged during build testing.
>>>>>
>>>>> We have similar cases in other drivers and it doesn't cause any known
>>>>> problems because (I think) build-bots are aware of this detail. Hence
>>>>
>>>> I don't understand how the build-bots would be aware of this detail.
>>>> Unless you explicitly state what the dependencies are, how would the
>>>> build-bots know? Perhaps there's some logic built-in somewhere that I
>>>> don't know about?
>>>
>>> So looks like COMPILE_TEST has a !UML dependency, so this might just
>>> work.
>>>
>>> Acked-by: Thierry Reding <treding@nvidia.com>
>>>
>>
>> Thank you very much for the clarification! Certainly that would caused
>> problems already since there are such cases all over the kernel,
>> including Tegra drivers.
>
> In the cases that I'm aware of we used to explicitly list all the
> dependencies that would've otherwise been pulled in by the ARCH_TEGRA
> dependency to make sure there were no issues.
>
> Now that we've been discussing this I vaguely recall a discussion about
> the only real issue nowadays being HAS_IOMEM and since that's only
> missing on UML, that may have been the reason for why the !UML
> dependency was added.
>
> Yes, looks like that was it:
>
> commit bc083a64b6c035135c0f80718f9e9192cc0867c6
> Author: Richard Weinberger <richard@nod.at>
> Date: Tue Aug 2 14:03:27 2016 -0700
>
> init/Kconfig: make COMPILE_TEST depend on !UML
>
> UML is a bit special since it does not have iomem nor dma. That means a
> lot of drivers will not build if they miss a dependency on HAS_IOMEM.
> s390 used to have the same issues but since it gained PCI support UML is
> the only stranger.
>
> We are tired of patching dozens of new drivers after every merge window
> just to un-break allmod/yesconfig UML builds. One could argue that a
> decent driver has to know on what it depends and therefore a missing
> HAS_IOMEM dependency is a clear driver bug. But the dependency not
> obvious and not everyone does UML builds with COMPILE_TEST enabled when
> developing a device driver.
>
> A possible solution to make these builds succeed on UML would be
> providing stub functions for ioremap() and friends which fail upon
> runtime. Another one is simply disabling COMPILE_TEST for UML. Since
> it is the least hassle and does not force use to fake iomem support
> let's do the latter.
>
> Link: http://lkml.kernel.org/r/1466152995-28367-1-git-send-email-richard@nod.at
> Signed-off-by: Richard Weinberger <richard@nod.at>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

That was a wise solution. Thank you very much again for the detailed
comment! Very appreciate that!

> Oh wow... almost three years now.

Please don't pay much attention to the time, it never stops. Live in the
moment :)