| Message ID | CAHLCerPxpCVZONujNhjgQ-QKkiibKW1xYoFWOO9dGkLSO--hPQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
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.
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