linux-kernel - Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH

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]

Date:	Mon, 8 Aug 2016 16:53:19 +0100
From:	Daniel Thompson <daniel.thompson@...aro.org>
To:	Amit Kucheria <amit.kucheria@...durent.com>,
	Leo Yan <leo.yan@...aro.org>
Cc:	Michael Turquette <mturquette@...libre.com>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Jiancheng Xue <xuejiancheng@...ilicon.com>,
	Philipp Zabel <p.zabel@...gutronix.de>,
	Rob Herring <robh@...nel.org>, linux-clk@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Guodong Xu <guodong.xu@...aro.org>
Subject: Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI

On 08/08/16 15:42, Amit Kucheria wrote:
> On Mon, Aug 8, 2016 at 12:12 PM, Leo Yan <leo.yan@...aro.org> wrote:
>> Hi Amit,
>>
>> On Mon, Aug 08, 2016 at 11:23:31AM +0530, Amit Kucheria wrote:
>>> On Mon, Aug 8, 2016 at 9:07 AM, Leo Yan <leo.yan@...aro.org> wrote:
>>>> In current kernel config 'CONFIG_STUB_CLK_HI6220' is disabled by
>>>> default, as result stub clock driver has not been registered and
>>>> CPUFreq driver cannot work.
>>>
>>> I have a related patch that has been pending for a while:
>>> https://lkml.org/lkml/2016/6/20/375 but it was tied only to the
>>> thermal driver.
>>
>> I also have concern this patch may duplicate with yours.
>>
>>> Is the stub mandatory for the architecture? Will other SoCs in the
>>> family will use the same stub?
>>
>> I don't think stub driver is mandartory for archtitecture and it's
>> only used by Hi6220 on Hikey. AFAIK, currently stub driver is only used
>> by CPU frequency change.
>>
>> The logic is:
>>   Thermal cooling device driver
>>       `-> CPUFreq DT driver
>>               `-> Stub clock driver
>>
>> ARM is working on Hikey for EAS profiling, so usually the use case is
>> to enable CPUFreq driver and stub clock driver. Sometimes it only
>> need enable this driver without thermal driver; so this is why we
>> cannot rely on thermal driver to enable stub clock driver.
>>
>> I'm open-minded if you have better idea to enable it.
>>
>>>> This patch is to enable stub clock driver in config for ARCH_HISI.
>>>>
>>>> Reported-by: Dietmar Eggemann <dietmar.eggemann@....com>
>>>> Signed-off-by: Leo Yan <leo.yan@...aro.org>
>>>> ---
>>>>  drivers/clk/hisilicon/Kconfig | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>>
>>>> diff --git a/drivers/clk/hisilicon/Kconfig b/drivers/clk/hisilicon/Kconfig
>>>> index 3f537a0..9e0a95e 100644
>>>> --- a/drivers/clk/hisilicon/Kconfig
>>>> +++ b/drivers/clk/hisilicon/Kconfig
>>>> @@ -23,5 +23,6 @@ config RESET_HISI
>>>>  config STUB_CLK_HI6220
>>>>         bool "Hi6220 Stub Clock Driver"
>>>>         depends on COMMON_CLK_HI6220 && MAILBOX
>>>> +       default ARCH_HISI
>
> Instead of forcing this up on the entire arch, why not restrict it to
> just the cpufreq driver?  ARM_HISI_ACPU_CPUFREQ?

I'm struggling to understand the concern here. default doesn't force 
this option on anything, it just sets the default.

Personally I might prefer 'default y' because it is simpler and involves 
less thinking. However the other drivers in this directory use 'default 
ARCH_HISI' (i.e. they do not default on for a COMPILE_TEST) so copying 
their approach is quite reasonable.


Daniel.