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

[Dmitry Osipenko]

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@xxxxxxxxxxx>
>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx>
>>>>>>> ---
>>>>>>>  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@xxxxxxxxxx>
>>>
>>
>> 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@xxxxxx>
> 	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@xxxxxx
> 	    Signed-off-by: Richard Weinberger <richard@xxxxxx>
> 	    Acked-by: Arnd Bergmann <arnd@xxxxxxxx>
> 	    Cc: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
> 	    Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> 	    Signed-off-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>

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 :)