Re: [RESEND PATCH v5 3/5] acerhdf: Use bang-bang thermal governor

[Darren Hart <dvhart@infradead.org>]

On Thu, Dec 04, 2014 at 08:21:06AM +0100, Peter Feuerer wrote:
> Hi again,
> 
> Peter Feuerer writes:
> 
> >Hi Darren,
> >
> >thank you very much for your reply.
> >
> >
> >Darren Hart writes:
> >
> >>On Fri, Nov 28, 2014 at 03:20:50PM +0100, Peter Feuerer wrote:
> >>>acerhdf has been doing an on-off fan control using hysteresis by
> >>>post-manipulating the outcome of thermal subsystem trip point handling.
> >>>This patch enables acerhdf to use the bang-bang governor, which is
> >>>intended for on-off controlled fans.
> >>>
> >>>Cc: platform-driver-x86@vger.kernel.org
> >>>Cc: Darren Hart <dvhart@infradead.org>
> >>>Cc: Andrew Morton <akpm@linux-foundation.org>
> >>>CC: Zhang Rui <rui.zhang@intel.com>
> >>>Cc: Andreas Mohr <andi@lisas.de>
> >>>Cc: Javi Merino <javi.merino@arm.com>
> >>>Acked-and-tested-by: Borislav Petkov <bp@suse.de>
> >>>Signed-off-by: Peter Feuerer <peter@piie.net>
> >>>---
> >>> drivers/platform/x86/Kconfig   |  3 ++-
> >>> drivers/platform/x86/acerhdf.c | 36 +++++++++++++++++++++++++++++++-----
> >>> 2 files changed, 33 insertions(+), 6 deletions(-)
> >>>
> >>>diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> >>>index a2eabe6..c173266 100644
> >>>--- a/drivers/platform/x86/Kconfig
> >>>+++ b/drivers/platform/x86/Kconfig
> >>>@@ -38,7 +38,8 @@ config ACER_WMI
> >>> config ACERHDF
> >>> 	tristate "Acer Aspire One temperature and fan driver"
> >>>-	depends on THERMAL && ACPI
> >>>+	select THERMAL_GOV_BANG_BANG
> >>
> >>So we use select sparingly as it does implicit things.
> >>
> >>I checked the THERMAL_GOV_BANG_BANG Kconfig entry, and the help says acerhdf
> >>already depends on it (which it doesn't appear to). Any particular reason to add
> >>select here instead of adding it as a depends.
> >>
> >>Why did you drop THERMAL?
> >
> >I had it like this in my first version of patches:
> >+ depends on THERMAL && ACPI && THERMAL_GOV_BANG_BANG
> >
> >But after some discussion with lkml community we ended up with the select
> >line and dropped THERMAL dependency, as it is implied by
> >THEMAL_GOV_BANG_BANG.  I'm not so experienced with Kconfig, so I must rely
> >on the statements of the community in this case.
> 
> Just found the link about this discussion I had with Rui and Boris:
> http://linux-kernel.2935.n7.nabble.com/PATCH-0-4-acerhdf-thermal-adding-new-models-and-appropriate-governor-tp848572p908256.html
> 

Rui,

I see you said you agree with the approach to "select THERMAL_GOV_BANG_BANG",
and I'm OK with that. But that doesn't ensure, as far as I can see, a
sufficient set of Kconfig dependencies for ACERHDF as THERMAL_GOV_BANG_BANG
doesn't in turn select THERMAL.

While it's unlikely to get ACERHDF available without THERMAL enabled, it does
look possible. Should this driver also select or depends on THERMAL?

Maybe:

depends on ACPI
depends on THERMAL
select THERMAL_GOV_BANG_BANG

?

> [...]
> 
> -- 
> kind regards,
> --peter;
> 

-- 
Darren Hart
Intel Open Source Technology Center