linux-kernel - Re: [PATCH v4 14/16] PM / devfreq: tegra: Enable COMPILE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <df040294-37d1-064c-009e-ba2fcac90364@gmail.com>
Date:   Tue, 4 Jun 2019 18:15:36 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Thierry Reding <thierry.reding@...il.com>
Cc:     Jonathan Hunter <jonathanh@...dia.com>,
        MyungJoo Ham <myungjoo.ham@...sung.com>,
        Kyungmin Park <kyungmin.park@...sung.com>,
        Chanwoo Choi <cw00.choi@...sung.com>,
        Tomeu Vizoso <tomeu.vizoso@...labora.com>,
        linux-pm@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 14/16] PM / devfreq: tegra: Enable COMPILE_TEST for the
 driver

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@...sung.com>
>>>>>>> Signed-off-by: Dmitry Osipenko <digetx@...il.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@...dia.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@....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@....at>
> 	    Acked-by: Arnd Bergmann <arnd@...db.de>
> 	    Cc: Al Viro <viro@...iv.linux.org.uk>
> 	    Signed-off-by: Andrew Morton <akpm@...ux-foundation.org>
> 	    Signed-off-by: Linus Torvalds <torvalds@...ux-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 :)