Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1457520 > unrolled thread

[PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI

Started byLeo Yan <leo.yan@linaro.org>
First post2016-08-08 05:40 +0200
Last post2016-08-09 03:30 +0200
Articles 9 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Leo Yan <leo.yan@linaro.org> - 2016-08-08 05:40 +0200
    Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Amit Kucheria <amit.kucheria@verdurent.com> - 2016-08-08 08:00 +0200
      Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Leo Yan <leo.yan@linaro.org> - 2016-08-08 08:50 +0200
        Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Amit Kucheria <amit.kucheria@verdurent.com> - 2016-08-08 16:50 +0200
          Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Leo Yan <leo.yan@linaro.org> - 2016-08-08 17:40 +0200
          Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Daniel Thompson <daniel.thompson@linaro.org> - 2016-08-08 18:00 +0200
            Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Amit Kucheria <amit.kucheria@verdurent.com> - 2016-08-08 20:10 +0200
              Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Daniel Thompson <daniel.thompson@linaro.org> - 2016-08-08 22:40 +0200
                Re: [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI Leo Yan <leo.yan@linaro.org> - 2016-08-09 03:30 +0200

#1457520 — [PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI

FromLeo Yan <leo.yan@linaro.org>
Date2016-08-08 05:40 +0200
Subject[PATCH] clk: Hi6220: enable stub clock driver for ARCH_HISI
Message-ID<s3JzX-4oT-3@gated-at.bofh.it>
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.

This patch is to enable stub clock driver in config for ARCH_HISI.

Reported-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
Signed-off-by: Leo Yan <leo.yan@linaro.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
 	help
 	  Build the Hisilicon Hi6220 stub clock driver.
-- 
1.9.1

[toc] | [next] | [standalone]

#1457543

FromAmit Kucheria <amit.kucheria@verdurent.com>
Date2016-08-08 08:00 +0200
Message-ID<s3LLr-5N8-9@gated-at.bofh.it>
In reply to#1457520
On Mon, Aug 8, 2016 at 9:07 AM, Leo Yan <leo.yan@linaro.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.

Is the stub mandatory for the architecture? Will other SoCs in the
family will use the same stub?


> This patch is to enable stub clock driver in config for ARCH_HISI.
>
> Reported-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
> Signed-off-by: Leo Yan <leo.yan@linaro.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
>         help
>           Build the Hisilicon Hi6220 stub clock driver.
> --
> 1.9.1
>

[toc] | [prev] | [next] | [standalone]

#1457570

FromLeo Yan <leo.yan@linaro.org>
Date2016-08-08 08:50 +0200
Message-ID<s3MxP-6io-21@gated-at.bofh.it>
In reply to#1457543
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@linaro.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@arm.com>
> > Signed-off-by: Leo Yan <leo.yan@linaro.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
> >         help
> >           Build the Hisilicon Hi6220 stub clock driver.
> > --
> > 1.9.1
> >

[toc] | [prev] | [next] | [standalone]

#1457849

FromAmit Kucheria <amit.kucheria@verdurent.com>
Date2016-08-08 16:50 +0200
Message-ID<s3U2m-2BY-13@gated-at.bofh.it>
In reply to#1457570
On Mon, Aug 8, 2016 at 12:12 PM, Leo Yan <leo.yan@linaro.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@linaro.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@arm.com>
>> > Signed-off-by: Leo Yan <leo.yan@linaro.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?

>> >         help
>> >           Build the Hisilicon Hi6220 stub clock driver.
>> > --
>> > 1.9.1
>> >

[toc] | [prev] | [next] | [standalone]

#1457862

FromLeo Yan <leo.yan@linaro.org>
Date2016-08-08 17:40 +0200
Message-ID<s3UOJ-3af-5@gated-at.bofh.it>
In reply to#1457849
On Mon, Aug 08, 2016 at 08:12:21PM +0530, Amit Kucheria wrote:

[...]

> >> > 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?

ARM_HISI_ACPU_CPUFREQ has been removed by 3920be471ce7f "cpufreq:
hisilicon: Use generic platdev driver". By default now platforms are
using drivers/cpufreq/cpufreq-dt-platdev.c.

> >> >         help
> >> >           Build the Hisilicon Hi6220 stub clock driver.
> >> > --
> >> > 1.9.1
> >> >

[toc] | [prev] | [next] | [standalone]

#1457871

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2016-08-08 18:00 +0200
Message-ID<s3V86-3gZ-7@gated-at.bofh.it>
In reply to#1457849
On 08/08/16 15:42, Amit Kucheria wrote:
> On Mon, Aug 8, 2016 at 12:12 PM, Leo Yan <leo.yan@linaro.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@linaro.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@arm.com>
>>>> Signed-off-by: Leo Yan <leo.yan@linaro.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.

[toc] | [prev] | [next] | [standalone]

#1457966

FromAmit Kucheria <amit.kucheria@verdurent.com>
Date2016-08-08 20:10 +0200
Message-ID<s3X9U-4P8-25@gated-at.bofh.it>
In reply to#1457871
On Mon, Aug 8, 2016 at 9:23 PM, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
> On 08/08/16 15:42, Amit Kucheria wrote:
>>
>> On Mon, Aug 8, 2016 at 12:12 PM, Leo Yan <leo.yan@linaro.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@linaro.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@arm.com>
>>>>> Signed-off-by: Leo Yan <leo.yan@linaro.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.

No concern actually - on the contrary, this dependency needs to be
fixed urgently. Just trying to figure out if there is a way to manage
the dependency chain in one place and including the root config option
into defconfig.

(Thermal driver) ---- dep ---> (cpufreq driver)  ----- dep --->
(hi2660 clock stub driver) ---- dep -----> (common hi6220 clock
driver)

My earlier patch focused on enabling the stub driver in the case the
thermal driver was enabled (and subsequently turning on HISI_THERMAL
in defconfig). The EAS profiling usecase to prevent thermal-throttling
from kicking in is a bad default to have in the kernel, IMO - it can
be easily achieved by just changing the thermal thresholds.

Something like the following, with HISI_THERMAL added to defconfig
would give a "stable" kernel on Hikey.

diff --git i/drivers/thermal/Kconfig w/drivers/thermal/Kconfig
index 2d702ca..77597a5 100644
--- i/drivers/thermal/Kconfig
+++ w/drivers/thermal/Kconfig
@@ -177,8 +177,11 @@ config THERMAL_EMULATION

 config HISI_THERMAL
  tristate "Hisilicon thermal driver"
-   depends on (ARCH_HISI && CPU_THERMAL && OF) || COMPILE_TEST
+ depends on (ARCH_HISI && OF) || COMPILE_TEST
  depends on HAS_IOMEM
+ select CPU_THERMAL
+ select CPUFREQ_DT
+ select STUB_CLK_HI6220
  help
    Enable this to plug hisilicon's thermal sensor driver into the Linux
    thermal framework. cpufreq is used as the cooling device to throttle

[toc] | [prev] | [next] | [standalone]

#1458225

FromDaniel Thompson <daniel.thompson@linaro.org>
Date2016-08-08 22:40 +0200
Message-ID<s3Zv3-6cB-17@gated-at.bofh.it>
In reply to#1457966
On 08/08/16 19:02, Amit Kucheria wrote:
>>>>>> This patch is to enable stub clock driver in config for ARCH_HISI.
>>>>>>
>>>>>> Reported-by: Dietmar Eggemann <dietmar.eggemann@arm.com>
>>>>>> Signed-off-by: Leo Yan <leo.yan@linaro.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.
>
> No concern actually - on the contrary, this dependency needs to be
> fixed urgently. Just trying to figure out if there is a way to manage
> the dependency chain in one place and including the root config option
> into defconfig.
>
> (Thermal driver) ---- dep ---> (cpufreq driver)  ----- dep --->
> (hi2660 clock stub driver) ---- dep -----> (common hi6220 clock
> driver)

This strikes me as solving a different problem.

The thermal driver is not the only client of cpufreq. We'd *like* a 
system where CPUFREQ_DT will work regardless of whether the thermal 
driver is enabled. That can only be achieved by setting a default in 
drivers/clk/hisilicon/Kconfig .

In other words I don't think your and Leo's patches are in conflict.


> My earlier patch focused on enabling the stub driver in the case the
> thermal driver was enabled (and subsequently turning on HISI_THERMAL
> in defconfig). The EAS profiling usecase to prevent thermal-throttling
> from kicking in is a bad default to have in the kernel, IMO - it can
> be easily achieved by just changing the thermal thresholds.
>
> Something like the following, with HISI_THERMAL added to defconfig
> would give a "stable" kernel on Hikey.
>
> diff --git i/drivers/thermal/Kconfig w/drivers/thermal/Kconfig
> index 2d702ca..77597a5 100644
> --- i/drivers/thermal/Kconfig
> +++ w/drivers/thermal/Kconfig
> @@ -177,8 +177,11 @@ config THERMAL_EMULATION
>
>  config HISI_THERMAL
>   tristate "Hisilicon thermal driver"
> -   depends on (ARCH_HISI && CPU_THERMAL && OF) || COMPILE_TEST
> + depends on (ARCH_HISI && OF) || COMPILE_TEST
>   depends on HAS_IOMEM
> + select CPU_THERMAL
> + select CPUFREQ_DT
> + select STUB_CLK_HI6220

I'm actually a little uncomfortable having a thermal sensor dictate what 
cooling devices are used to react to its temperature reading. The link 
between sensors and cooling devices comes from DT.

However I admit there are other platforms (IMX and DB8500) that accept 
the same build time diktat from their thermal sensors.


Daniel.

[toc] | [prev] | [next] | [standalone]

#1458362

FromLeo Yan <leo.yan@linaro.org>
Date2016-08-09 03:30 +0200
Message-ID<s441H-OA-9@gated-at.bofh.it>
In reply to#1458225
On Mon, Aug 08, 2016 at 09:36:32PM +0100, Daniel Thompson wrote:

[...]

> >My earlier patch focused on enabling the stub driver in the case the
> >thermal driver was enabled (and subsequently turning on HISI_THERMAL
> >in defconfig). The EAS profiling usecase to prevent thermal-throttling
> >from kicking in is a bad default to have in the kernel, IMO - it can
> >be easily achieved by just changing the thermal thresholds.
> >
> >Something like the following, with HISI_THERMAL added to defconfig
> >would give a "stable" kernel on Hikey.
> >
> >diff --git i/drivers/thermal/Kconfig w/drivers/thermal/Kconfig
> >index 2d702ca..77597a5 100644
> >--- i/drivers/thermal/Kconfig
> >+++ w/drivers/thermal/Kconfig
> >@@ -177,8 +177,11 @@ config THERMAL_EMULATION
> >
> > config HISI_THERMAL
> >  tristate "Hisilicon thermal driver"
> >-   depends on (ARCH_HISI && CPU_THERMAL && OF) || COMPILE_TEST
> >+ depends on (ARCH_HISI && OF) || COMPILE_TEST
> >  depends on HAS_IOMEM
> >+ select CPU_THERMAL
> >+ select CPUFREQ_DT
> >+ select STUB_CLK_HI6220
> 
> I'm actually a little uncomfortable having a thermal sensor dictate
> what cooling devices are used to react to its temperature reading.
> The link between sensors and cooling devices comes from DT.
> 
> However I admit there are other platforms (IMX and DB8500) that
> accept the same build time diktat from their thermal sensors.

For thermal enabling on Hikey with CPU cooling device, how about below
change? I checked arch/arm/configs/multi_v7_defconfig, both
CONFIG_CPUFREQ_DT and CONFIG_CPU_THERMAL have been enabled in it.
These two drivers are quite common and used by many ARM platforms.

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 0555b7c..f65336f 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -78,6 +78,7 @@ CONFIG_COMPAT=y
 CONFIG_CPU_IDLE=y
 CONFIG_ARM_CPUIDLE=y
 CONFIG_CPU_FREQ=y
+CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_BIG_LITTLE_CPUFREQ=y
 CONFIG_ARM_SCPI_CPUFREQ=y
 CONFIG_NET=y
@@ -217,6 +218,7 @@ CONFIG_SENSORS_INA2XX=m
 CONFIG_SENSORS_ARM_SCPI=y
 CONFIG_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
+CONFIG_CPU_THERMAL=y
 CONFIG_EXYNOS_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_RENESAS_WDT=y
diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig
index 2d702ca..91ebab3 100644
--- a/drivers/thermal/Kconfig
+++ b/drivers/thermal/Kconfig
@@ -177,8 +177,10 @@ config THERMAL_EMULATION
 
 config HISI_THERMAL
 	tristate "Hisilicon thermal driver"
-	depends on (ARCH_HISI && CPU_THERMAL && OF) || COMPILE_TEST
+	depends on ARCH_HISI || COMPILE_TEST
 	depends on HAS_IOMEM
+	depends on OF
+	default y
 	help
 	  Enable this to plug hisilicon's thermal sensor driver into the Linux
 	  thermal framework. cpufreq is used as the cooling device to throttle

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web