[RESEND PATCH v5 0/5] acerhdf: adding new models, appropriate governor and minor clean up

[RESEND PATCH v5 0/5] acerhdf: adding new models, appropriate governor and minor clean up

[Peter Feuerer]

Hi Darren,

please apply this series of patches.

It is intended to:

  * Introduce "manual mode" support (Patch 1 & 2), which is needed to control
    the fan of a few new models.

  * Add an appropriate thermal governor (Patch 3).  Manipulating and
    fiddling around with the step-wise governor has been a very fragile thing
    in the past and as it broke again, I used the opportunity to add a two
    point thermal governor which implements the actual fan handling required by
    acerhdf and puts from my point of view things straight.

  * Do some minor clean up like:
      - adding second trip point for critical temperature (Patch 4)
      - removing _t suffix from struct which isn't typedef and replace unsigned
        char by u8 (Patch 5)

Thanks and kind regards,
peter 

Peter Feuerer (5):
  acerhdf: Adding support for "manual mode"
  acerhdf: Adding support for new models
  acerhdf: Use bang-bang thermal governor
  acerhdf: added critical trip point
  acerhdf: minor clean up

 drivers/platform/x86/Kconfig   |   3 +-
 drivers/platform/x86/acerhdf.c | 265 ++++++++++++++++++++++++++---------------
 2 files changed, 172 insertions(+), 96 deletions(-)

-- 
2.1.3

[RESEND PATCH v5 1/5] acerhdf: Adding support for "manual mode"

[Peter Feuerer]

Some Acer models require an additional command to turn off the fan after
bios mode has been enabled.  Adding new section in bios table to allow
support for those models, by writing an extra "manual mode" register.

Cc: platform-driver-x86@vger.kernel.org
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andreas Mohr <andi@lisas.de>
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Feuerer <peter@piie.net>
---
 drivers/platform/x86/acerhdf.c | 186 ++++++++++++++++++++++-------------------
 1 file changed, 102 insertions(+), 84 deletions(-)

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index f94467c..2c58f10 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -50,7 +50,7 @@
  */
 #undef START_IN_KERNEL_MODE
 
-#define DRV_VER "0.5.26"
+#define DRV_VER "0.5.29"
 
 /*
  * According to the Atom N270 datasheet,
@@ -119,6 +119,17 @@ struct fancmd {
 	u8 cmd_auto;
 };
 
+struct manualcmd {
+	u8 mreg;
+	u8 moff;
+};
+
+/* default register and command to disable fan in manual mode */
+static const struct manualcmd mcmd = {
+	.mreg = 0x94,
+	.moff = 0xff,
+};
+
 /* BIOS settings */
 struct bios_settings_t {
 	const char *vendor;
@@ -127,105 +138,106 @@ struct bios_settings_t {
 	unsigned char fanreg;
 	unsigned char tempreg;
 	struct fancmd cmd;
+	int mcmd_enable;
 };
 
 /* Register addresses and values for different BIOS versions */
 static const struct bios_settings_t bios_tbl[] = {
 	/* AOA110 */
-	{"Acer", "AOA110", "v0.3109", 0x55, 0x58, {0x1f, 0x00} },
-	{"Acer", "AOA110", "v0.3114", 0x55, 0x58, {0x1f, 0x00} },
-	{"Acer", "AOA110", "v0.3301", 0x55, 0x58, {0xaf, 0x00} },
-	{"Acer", "AOA110", "v0.3304", 0x55, 0x58, {0xaf, 0x00} },
-	{"Acer", "AOA110", "v0.3305", 0x55, 0x58, {0xaf, 0x00} },
-	{"Acer", "AOA110", "v0.3307", 0x55, 0x58, {0xaf, 0x00} },
-	{"Acer", "AOA110", "v0.3308", 0x55, 0x58, {0x21, 0x00} },
-	{"Acer", "AOA110", "v0.3309", 0x55, 0x58, {0x21, 0x00} },
-	{"Acer", "AOA110", "v0.3310", 0x55, 0x58, {0x21, 0x00} },
+	{"Acer", "AOA110", "v0.3109", 0x55, 0x58, {0x1f, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3114", 0x55, 0x58, {0x1f, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3301", 0x55, 0x58, {0xaf, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3304", 0x55, 0x58, {0xaf, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3305", 0x55, 0x58, {0xaf, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3307", 0x55, 0x58, {0xaf, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3308", 0x55, 0x58, {0x21, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3309", 0x55, 0x58, {0x21, 0x00}, 0},
+	{"Acer", "AOA110", "v0.3310", 0x55, 0x58, {0x21, 0x00}, 0},
 	/* AOA150 */
-	{"Acer", "AOA150", "v0.3114", 0x55, 0x58, {0x1f, 0x00} },
-	{"Acer", "AOA150", "v0.3301", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AOA150", "v0.3304", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AOA150", "v0.3305", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AOA150", "v0.3307", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AOA150", "v0.3308", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AOA150", "v0.3309", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AOA150", "v0.3310", 0x55, 0x58, {0x20, 0x00} },
+	{"Acer", "AOA150", "v0.3114", 0x55, 0x58, {0x1f, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3301", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3304", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3305", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3307", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3308", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3309", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AOA150", "v0.3310", 0x55, 0x58, {0x20, 0x00}, 0},
 	/* LT1005u */
-	{"Acer", "LT-10Q", "v0.3310", 0x55, 0x58, {0x20, 0x00} },
+	{"Acer", "LT-10Q", "v0.3310", 0x55, 0x58, {0x20, 0x00}, 0},
 	/* Acer 1410 */
-	{"Acer", "Aspire 1410", "v0.3108", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v0.3113", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v0.3115", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v0.3117", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v0.3119", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v0.3120", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v1.3204", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v1.3303", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v1.3308", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v1.3310", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1410", "v1.3314", 0x55, 0x58, {0x9e, 0x00} },
+	{"Acer", "Aspire 1410", "v0.3108", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v0.3113", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v0.3115", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v0.3117", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v0.3119", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v0.3120", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v1.3204", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v1.3303", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v1.3308", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v1.3310", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1410", "v1.3314", 0x55, 0x58, {0x9e, 0x00}, 0},
 	/* Acer 1810xx */
-	{"Acer", "Aspire 1810TZ", "v0.3108", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v0.3108", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v0.3113", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v0.3113", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v0.3115", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v0.3115", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v0.3117", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v0.3117", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v0.3119", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v0.3119", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v0.3120", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v0.3120", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v1.3204", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v1.3204", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v1.3303", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v1.3303", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v1.3308", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v1.3308", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v1.3310", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v1.3310", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810TZ", "v1.3314", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1810T",  "v1.3314", 0x55, 0x58, {0x9e, 0x00} },
+	{"Acer", "Aspire 1810TZ", "v0.3108", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v0.3108", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v0.3113", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v0.3113", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v0.3115", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v0.3115", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v0.3117", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v0.3117", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v0.3119", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v0.3119", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v0.3120", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v0.3120", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v1.3204", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v1.3204", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v1.3303", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v1.3303", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v1.3308", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v1.3308", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v1.3310", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v1.3310", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810TZ", "v1.3314", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1810T",  "v1.3314", 0x55, 0x58, {0x9e, 0x00}, 0},
 	/* Acer 531 */
-	{"Acer", "AO531h", "v0.3104", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AO531h", "v0.3201", 0x55, 0x58, {0x20, 0x00} },
-	{"Acer", "AO531h", "v0.3304", 0x55, 0x58, {0x20, 0x00} },
+	{"Acer", "AO531h", "v0.3104", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AO531h", "v0.3201", 0x55, 0x58, {0x20, 0x00}, 0},
+	{"Acer", "AO531h", "v0.3304", 0x55, 0x58, {0x20, 0x00}, 0},
 	/* Acer 751 */
-	{"Acer", "AO751h", "V0.3212", 0x55, 0x58, {0x21, 0x00} },
+	{"Acer", "AO751h", "V0.3212", 0x55, 0x58, {0x21, 0x00}, 0},
 	/* Acer 1825 */
-	{"Acer", "Aspire 1825PTZ", "V1.3118", 0x55, 0x58, {0x9e, 0x00} },
-	{"Acer", "Aspire 1825PTZ", "V1.3127", 0x55, 0x58, {0x9e, 0x00} },
+	{"Acer", "Aspire 1825PTZ", "V1.3118", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Acer", "Aspire 1825PTZ", "V1.3127", 0x55, 0x58, {0x9e, 0x00}, 0},
 	/* Acer TravelMate 7730 */
-	{"Acer", "TravelMate 7730G", "v0.3509", 0x55, 0x58, {0xaf, 0x00} },
+	{"Acer", "TravelMate 7730G", "v0.3509", 0x55, 0x58, {0xaf, 0x00}, 0},
 	/* Gateway */
-	{"Gateway", "AOA110", "v0.3103",  0x55, 0x58, {0x21, 0x00} },
-	{"Gateway", "AOA150", "v0.3103",  0x55, 0x58, {0x20, 0x00} },
-	{"Gateway", "LT31",   "v1.3103",  0x55, 0x58, {0x9e, 0x00} },
-	{"Gateway", "LT31",   "v1.3201",  0x55, 0x58, {0x9e, 0x00} },
-	{"Gateway", "LT31",   "v1.3302",  0x55, 0x58, {0x9e, 0x00} },
-	{"Gateway", "LT31",   "v1.3303t", 0x55, 0x58, {0x9e, 0x00} },
+	{"Gateway", "AOA110", "v0.3103",  0x55, 0x58, {0x21, 0x00}, 0},
+	{"Gateway", "AOA150", "v0.3103",  0x55, 0x58, {0x20, 0x00}, 0},
+	{"Gateway", "LT31",   "v1.3103",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Gateway", "LT31",   "v1.3201",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Gateway", "LT31",   "v1.3302",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Gateway", "LT31",   "v1.3303t", 0x55, 0x58, {0x9e, 0x00}, 0},
 	/* Packard Bell */
-	{"Packard Bell", "DOA150",  "v0.3104",  0x55, 0x58, {0x21, 0x00} },
-	{"Packard Bell", "DOA150",  "v0.3105",  0x55, 0x58, {0x20, 0x00} },
-	{"Packard Bell", "AOA110",  "v0.3105",  0x55, 0x58, {0x21, 0x00} },
-	{"Packard Bell", "AOA150",  "v0.3105",  0x55, 0x58, {0x20, 0x00} },
-	{"Packard Bell", "ENBFT",   "V1.3118",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "ENBFT",   "V1.3127",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v1.3303",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v0.3120",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v0.3108",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v0.3113",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v0.3115",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v0.3117",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v0.3119",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMU",   "v1.3204",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMA",   "v1.3201",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMA",   "v1.3302",  0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTMA",   "v1.3303t", 0x55, 0x58, {0x9e, 0x00} },
-	{"Packard Bell", "DOTVR46", "v1.3308",  0x55, 0x58, {0x9e, 0x00} },
+	{"Packard Bell", "DOA150",  "v0.3104",  0x55, 0x58, {0x21, 0x00}, 0},
+	{"Packard Bell", "DOA150",  "v0.3105",  0x55, 0x58, {0x20, 0x00}, 0},
+	{"Packard Bell", "AOA110",  "v0.3105",  0x55, 0x58, {0x21, 0x00}, 0},
+	{"Packard Bell", "AOA150",  "v0.3105",  0x55, 0x58, {0x20, 0x00}, 0},
+	{"Packard Bell", "ENBFT",   "V1.3118",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "ENBFT",   "V1.3127",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v1.3303",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v0.3120",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v0.3108",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v0.3113",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v0.3115",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v0.3117",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v0.3119",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMU",   "v1.3204",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMA",   "v1.3201",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMA",   "v1.3302",  0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTMA",   "v1.3303t", 0x55, 0x58, {0x9e, 0x00}, 0},
+	{"Packard Bell", "DOTVR46", "v1.3308",  0x55, 0x58, {0x9e, 0x00}, 0},
 	/* pewpew-terminator */
-	{"", "", "", 0, 0, {0, 0} }
+	{"", "", "", 0, 0, {0, 0}, 0}
 };
 
 static const struct bios_settings_t *bios_cfg __read_mostly;
@@ -275,6 +287,12 @@ static void acerhdf_change_fanstate(int state)
 	fanstate = state;
 
 	ec_write(bios_cfg->fanreg, cmd);
+
+	if (bios_cfg->mcmd_enable && state == ACERHDF_FAN_OFF) {
+		if (verbose)
+			pr_notice("turning off fan manually\n");
+		ec_write(mcmd.mreg, mcmd.moff);
+	}
 }
 
 static void acerhdf_check_param(struct thermal_zone_device *thermal)
-- 
2.1.3

[RESEND PATCH v5 2/5] acerhdf: Adding support for new models

[Peter Feuerer]

added following new models:
   * Aspire 5755G
   * AO521
   * AO751h
   * Aspire One 753
   * Extensa 5420
   * Aspire 5315
   * Aspire 5739G
   * TravelMate TM8573T

Cc: platform-driver-x86@vger.kernel.org
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andreas Mohr <andi@lisas.de>
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Feuerer <peter@piie.net>
---
 drivers/platform/x86/acerhdf.c | 25 ++++++++++++++++++++++++-
 1 file changed, 24 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 2c58f10..f30767f 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -50,7 +50,7 @@
  */
 #undef START_IN_KERNEL_MODE
 
-#define DRV_VER "0.5.29"
+#define DRV_VER "0.5.30"
 
 /*
  * According to the Atom N270 datasheet,
@@ -199,17 +199,33 @@ static const struct bios_settings_t bios_tbl[] = {
 	{"Acer", "Aspire 1810T",  "v1.3310", 0x55, 0x58, {0x9e, 0x00}, 0},
 	{"Acer", "Aspire 1810TZ", "v1.3314", 0x55, 0x58, {0x9e, 0x00}, 0},
 	{"Acer", "Aspire 1810T",  "v1.3314", 0x55, 0x58, {0x9e, 0x00}, 0},
+	/* Acer 5755G */
+	{"Acer", "Aspire 5755G",  "V1.20",   0xab, 0xb4, {0x00, 0x08}, 0},
+	{"Acer", "Aspire 5755G",  "V1.21",   0xab, 0xb3, {0x00, 0x08}, 0},
+	/* Acer 521 */
+	{"Acer", "AO521", "V1.11", 0x55, 0x58, {0x1f, 0x00}, 0},
 	/* Acer 531 */
 	{"Acer", "AO531h", "v0.3104", 0x55, 0x58, {0x20, 0x00}, 0},
 	{"Acer", "AO531h", "v0.3201", 0x55, 0x58, {0x20, 0x00}, 0},
 	{"Acer", "AO531h", "v0.3304", 0x55, 0x58, {0x20, 0x00}, 0},
 	/* Acer 751 */
+	{"Acer", "AO751h", "V0.3206", 0x55, 0x58, {0x21, 0x00}, 0},
 	{"Acer", "AO751h", "V0.3212", 0x55, 0x58, {0x21, 0x00}, 0},
+	/* Acer 753 */
+	{"Acer", "Aspire One 753", "V1.24", 0x93, 0xac, {0x14, 0x04}, 1},
 	/* Acer 1825 */
 	{"Acer", "Aspire 1825PTZ", "V1.3118", 0x55, 0x58, {0x9e, 0x00}, 0},
 	{"Acer", "Aspire 1825PTZ", "V1.3127", 0x55, 0x58, {0x9e, 0x00}, 0},
+	/* Acer Extensa 5420 */
+	{"Acer", "Extensa 5420", "V1.17", 0x93, 0xac, {0x14, 0x04}, 1},
+	/* Acer Aspire 5315 */
+	{"Acer", "Aspire 5315", "V1.19", 0x93, 0xac, {0x14, 0x04}, 1},
+	/* Acer Aspire 5739 */
+	{"Acer", "Aspire 5739G", "V1.3311", 0x55, 0x58, {0x20, 0x00}, 0},
 	/* Acer TravelMate 7730 */
 	{"Acer", "TravelMate 7730G", "v0.3509", 0x55, 0x58, {0xaf, 0x00}, 0},
+	/* Acer TravelMate TM8573T */
+	{"Acer", "TM8573T", "V1.13", 0x93, 0xa8, {0x14, 0x04}, 1},
 	/* Gateway */
 	{"Gateway", "AOA110", "v0.3103",  0x55, 0x58, {0x21, 0x00}, 0},
 	{"Gateway", "AOA150", "v0.3103",  0x55, 0x58, {0x20, 0x00}, 0},
@@ -741,9 +757,15 @@ MODULE_ALIAS("dmi:*:*Acer*:pnAOA*:");
 MODULE_ALIAS("dmi:*:*Acer*:pnAO751h*:");
 MODULE_ALIAS("dmi:*:*Acer*:pnAspire*1410*:");
 MODULE_ALIAS("dmi:*:*Acer*:pnAspire*1810*:");
+MODULE_ALIAS("dmi:*:*Acer*:pnAspire*5755G:");
 MODULE_ALIAS("dmi:*:*Acer*:pnAspire*1825PTZ:");
+MODULE_ALIAS("dmi:*:*Acer*:pnAO521*:");
 MODULE_ALIAS("dmi:*:*Acer*:pnAO531*:");
+MODULE_ALIAS("dmi:*:*Acer*:pnAspire*5739G:");
+MODULE_ALIAS("dmi:*:*Acer*:pnAspire*One*753:");
+MODULE_ALIAS("dmi:*:*Acer*:pnAspire*5315:");
 MODULE_ALIAS("dmi:*:*Acer*:TravelMate*7730G:");
+MODULE_ALIAS("dmi:*:*Acer*:TM8573T:");
 MODULE_ALIAS("dmi:*:*Gateway*:pnAOA*:");
 MODULE_ALIAS("dmi:*:*Gateway*:pnLT31*:");
 MODULE_ALIAS("dmi:*:*Packard*Bell*:pnAOA*:");
@@ -752,6 +774,7 @@ MODULE_ALIAS("dmi:*:*Packard*Bell*:pnDOTMU*:");
 MODULE_ALIAS("dmi:*:*Packard*Bell*:pnENBFT*:");
 MODULE_ALIAS("dmi:*:*Packard*Bell*:pnDOTMA*:");
 MODULE_ALIAS("dmi:*:*Packard*Bell*:pnDOTVR46*:");
+MODULE_ALIAS("dmi:*:*Acer*:pnExtensa 5420*:");
 
 module_init(acerhdf_init);
 module_exit(acerhdf_exit);
-- 
2.1.3

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

[Peter Feuerer]

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
+	depends on ACPI
 	---help---
 	  This is a driver for Acer Aspire One netbooks. It allows to access
 	  the temperature sensor and to control the fan.
diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index f30767f..7fe7dbf 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -50,7 +50,7 @@
  */
 #undef START_IN_KERNEL_MODE
 
-#define DRV_VER "0.5.30"
+#define DRV_VER "0.7.0"
 
 /*
  * According to the Atom N270 datasheet,
@@ -258,6 +258,14 @@ static const struct bios_settings_t bios_tbl[] = {
 
 static const struct bios_settings_t *bios_cfg __read_mostly;
 
+/*
+ * this struct is used to instruct thermal layer to use bang_bang instead of
+ * default governor for acerhdf
+ */
+static struct thermal_zone_params acerhdf_zone_params = {
+	.governor_name = "bang_bang",
+};
+
 static int acerhdf_get_temp(int *temp)
 {
 	u8 read_temp;
@@ -439,6 +447,17 @@ static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
 	return 0;
 }
 
+static int acerhdf_get_trip_hyst(struct thermal_zone_device *thermal, int trip,
+				 unsigned long *temp)
+{
+	if (trip != 0)
+		return -EINVAL;
+
+	*temp = fanon - fanoff;
+
+	return 0;
+}
+
 static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
 				 unsigned long *temp)
 {
@@ -463,6 +482,7 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
 	.get_mode = acerhdf_get_mode,
 	.set_mode = acerhdf_set_mode,
 	.get_trip_type = acerhdf_get_trip_type,
+	.get_trip_hyst = acerhdf_get_trip_hyst,
 	.get_trip_temp = acerhdf_get_trip_temp,
 	.get_crit_temp = acerhdf_get_crit_temp,
 };
@@ -515,9 +535,7 @@ static int acerhdf_set_cur_state(struct thermal_cooling_device *cdev,
 	}
 
 	if (state == 0) {
-		/* turn fan off only if below fanoff temperature */
-		if ((cur_state == ACERHDF_FAN_AUTO) &&
-		    (cur_temp < fanoff))
+		if (cur_state == ACERHDF_FAN_AUTO)
 			acerhdf_change_fanstate(ACERHDF_FAN_OFF);
 	} else {
 		if (cur_state == ACERHDF_FAN_OFF)
@@ -696,11 +714,19 @@ static int acerhdf_register_thermal(void)
 		return -EINVAL;
 
 	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
-					      &acerhdf_dev_ops, NULL, 0,
+					      &acerhdf_dev_ops,
+					      &acerhdf_zone_params, 0,
 					      (kernelmode) ? interval*1000 : 0);
 	if (IS_ERR(thz_dev))
 		return -EINVAL;
 
+	if (strcmp(thz_dev->governor->name,
+				acerhdf_zone_params.governor_name)) {
+		pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
+				acerhdf_zone_params.governor_name);
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
-- 
2.1.3

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

[Darren Hart]

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?

> +	depends on ACPI
>  	---help---
>  	  This is a driver for Acer Aspire One netbooks. It allows to access
>  	  the temperature sensor and to control the fan.
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index f30767f..7fe7dbf 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -50,7 +50,7 @@
>   */
>  #undef START_IN_KERNEL_MODE
>  
> -#define DRV_VER "0.5.30"
> +#define DRV_VER "0.7.0"
>  
>  /*
>   * According to the Atom N270 datasheet,
> @@ -258,6 +258,14 @@ static const struct bios_settings_t bios_tbl[] = {
>  
>  static const struct bios_settings_t *bios_cfg __read_mostly;
>  
> +/*
> + * this struct is used to instruct thermal layer to use bang_bang instead of
> + * default governor for acerhdf

Please use sentence punctuation, particularly for block comments.

/*
 * This struct...
 * default ... for acerhdf.
 */

> + */
> +static struct thermal_zone_params acerhdf_zone_params = {
> +	.governor_name = "bang_bang",
> +};
> +
>  static int acerhdf_get_temp(int *temp)
>  {
>  	u8 read_temp;
> @@ -439,6 +447,17 @@ static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
>  	return 0;
>  }
>  
> +static int acerhdf_get_trip_hyst(struct thermal_zone_device *thermal, int trip,
> +				 unsigned long *temp)
> +{
> +	if (trip != 0)
> +		return -EINVAL;
> +
> +	*temp = fanon - fanoff;
> +
> +	return 0;
> +}
> +
>  static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
>  				 unsigned long *temp)
>  {
> @@ -463,6 +482,7 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
>  	.get_mode = acerhdf_get_mode,
>  	.set_mode = acerhdf_set_mode,
>  	.get_trip_type = acerhdf_get_trip_type,
> +	.get_trip_hyst = acerhdf_get_trip_hyst,
>  	.get_trip_temp = acerhdf_get_trip_temp,
>  	.get_crit_temp = acerhdf_get_crit_temp,
>  };
> @@ -515,9 +535,7 @@ static int acerhdf_set_cur_state(struct thermal_cooling_device *cdev,
>  	}
>  
>  	if (state == 0) {
> -		/* turn fan off only if below fanoff temperature */
> -		if ((cur_state == ACERHDF_FAN_AUTO) &&
> -		    (cur_temp < fanoff))
> +		if (cur_state == ACERHDF_FAN_AUTO)
>  			acerhdf_change_fanstate(ACERHDF_FAN_OFF);
>  	} else {
>  		if (cur_state == ACERHDF_FAN_OFF)
> @@ -696,11 +714,19 @@ static int acerhdf_register_thermal(void)
>  		return -EINVAL;
>  
>  	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
> -					      &acerhdf_dev_ops, NULL, 0,
> +					      &acerhdf_dev_ops,
> +					      &acerhdf_zone_params, 0,
>  					      (kernelmode) ? interval*1000 : 0);
>  	if (IS_ERR(thz_dev))
>  		return -EINVAL;
>  
> +	if (strcmp(thz_dev->governor->name,
> +				acerhdf_zone_params.governor_name)) {
> +		pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
> +				acerhdf_zone_params.governor_name);

We've ensured it has to be compiled in, so that really can't be it. Right?

> +		return -EINVAL;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.1.3
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

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

[Peter Feuerer]

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.


>> +	depends on ACPI
>>  	---help---
>>  	  This is a driver for Acer Aspire One netbooks. It allows to access
>>  	  the temperature sensor and to control the fan.
>> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
>> index f30767f..7fe7dbf 100644
>> --- a/drivers/platform/x86/acerhdf.c
>> +++ b/drivers/platform/x86/acerhdf.c
>> @@ -50,7 +50,7 @@
>>   */
>>  #undef START_IN_KERNEL_MODE
>>  
>> -#define DRV_VER "0.5.30"
>> +#define DRV_VER "0.7.0"
>>  
>>  /*
>>   * According to the Atom N270 datasheet,
>> @@ -258,6 +258,14 @@ static const struct bios_settings_t bios_tbl[] = {
>>  
>>  static const struct bios_settings_t *bios_cfg __read_mostly;
>>  
>> +/*
>> + * this struct is used to instruct thermal layer to use bang_bang instead of
>> + * default governor for acerhdf
> 
> Please use sentence punctuation, particularly for block comments.

ok.


 
> /*
>  * This struct...
>  * default ... for acerhdf.
>  */
> 
>> + */
>> +static struct thermal_zone_params acerhdf_zone_params = {
>> +	.governor_name = "bang_bang",
>> +};
>> +
>>  static int acerhdf_get_temp(int *temp)
>>  {
>>  	u8 read_temp;
>> @@ -439,6 +447,17 @@ static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
>>  	return 0;
>>  }
>>  
>> +static int acerhdf_get_trip_hyst(struct thermal_zone_device *thermal, int trip,
>> +				 unsigned long *temp)
>> +{
>> +	if (trip != 0)
>> +		return -EINVAL;
>> +
>> +	*temp = fanon - fanoff;
>> +
>> +	return 0;
>> +}
>> +
>>  static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
>>  				 unsigned long *temp)
>>  {
>> @@ -463,6 +482,7 @@ static struct thermal_zone_device_ops acerhdf_dev_ops = {
>>  	.get_mode = acerhdf_get_mode,
>>  	.set_mode = acerhdf_set_mode,
>>  	.get_trip_type = acerhdf_get_trip_type,
>> +	.get_trip_hyst = acerhdf_get_trip_hyst,
>>  	.get_trip_temp = acerhdf_get_trip_temp,
>>  	.get_crit_temp = acerhdf_get_crit_temp,
>>  };
>> @@ -515,9 +535,7 @@ static int acerhdf_set_cur_state(struct thermal_cooling_device *cdev,
>>  	}
>>  
>>  	if (state == 0) {
>> -		/* turn fan off only if below fanoff temperature */
>> -		if ((cur_state == ACERHDF_FAN_AUTO) &&
>> -		    (cur_temp < fanoff))
>> +		if (cur_state == ACERHDF_FAN_AUTO)
>>  			acerhdf_change_fanstate(ACERHDF_FAN_OFF);
>>  	} else {
>>  		if (cur_state == ACERHDF_FAN_OFF)
>> @@ -696,11 +714,19 @@ static int acerhdf_register_thermal(void)
>>  		return -EINVAL;
>>  
>>  	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
>> -					      &acerhdf_dev_ops, NULL, 0,
>> +					      &acerhdf_dev_ops,
>> +					      &acerhdf_zone_params, 0,
>>  					      (kernelmode) ? interval*1000 : 0);
>>  	if (IS_ERR(thz_dev))
>>  		return -EINVAL;
>>  
>> +	if (strcmp(thz_dev->governor->name,
>> +				acerhdf_zone_params.governor_name)) {
>> +		pr_err("Didn't get thermal governor %s, perhaps not compiled into thermal subsystem.\n",
>> +				acerhdf_zone_params.governor_name);
> 
> We've ensured it has to be compiled in, so that really can't be it. Right?

Yes, you are right, but if it is no real blocker for you, I'd like to keep this code in as I'm providing the acerhdf.c code in a separate package for debugging / testing too and people could use the code without the Kconfig.

-- 
kind regards,
--peter;

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

[Peter Feuerer]

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

[...]

-- 
kind regards,
--peter;

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

[Darren Hart]

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

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

[Peter Feuerer]

Hi Darren,

I know you are busy and I don't want to stress you, but have you had time to 
look to this?  I'd be really happy to solve all issues in time and have 
those patches finally applied.

kind regards,
--peter;


Peter Feuerer writes:

> 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
> 
> [...]
> 
> -- 
> kind regards,
> --peter;

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

[Darren Hart]

On Mon, Dec 08, 2014 at 08:45:58AM +0100, Peter Feuerer wrote:
> Hi Darren,
> 
> I know you are busy and I don't want to stress you, but have you had time to
> look to this?  I'd be really happy to solve all issues in time and have
> those patches finally applied.

Not yet. I've been traveling to Mexico and wrapping up the current for-next
patches. FYI, also dealing with a very sick kid these last couple of days, so
I've been rather tied up. I hope to get to these in the next few days.

You've provided the link to the discussion which is what I needed, thanks.

Thanks,

Darren

> 
> kind regards,
> --peter;
> 
> 
> Peter Feuerer writes:
> 
> >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
> >
> >[...]
> >
> >-- 
> >kind regards,
> >--peter;
> 

-- 
Darren Hart
Intel Open Source Technology Center

[RESEND PATCH v5 4/5] acerhdf: added critical trip point

[Peter Feuerer]

added critical trip point which represents the temperature limit.
Added return -EINVAL in case wrong trip point is provided.

Cc: platform-driver-x86@vger.kernel.org
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andreas Mohr <andi@lisas.de>
Cc: Borislav Petkov <bp@suse.de>
Cc: Javi Merino <javi.merino@arm.com>
Signed-off-by: Peter Feuerer <peter@piie.net>
---
 drivers/platform/x86/acerhdf.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 7fe7dbf..91b16c8 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -443,6 +443,10 @@ static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
 {
 	if (trip == 0)
 		*type = THERMAL_TRIP_ACTIVE;
+	else if (trip == 1)
+		*type = THERMAL_TRIP_CRITICAL;
+	else
+		return -EINVAL;
 
 	return 0;
 }
@@ -463,6 +467,10 @@ static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
 {
 	if (trip == 0)
 		*temp = fanon;
+	else if (trip == 1)
+		*temp = ACERHDF_TEMP_CRIT;
+	else
+		return -EINVAL;
 
 	return 0;
 }
@@ -713,7 +721,7 @@ static int acerhdf_register_thermal(void)
 	if (IS_ERR(cl_dev))
 		return -EINVAL;
 
-	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
+	thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
 					      &acerhdf_dev_ops,
 					      &acerhdf_zone_params, 0,
 					      (kernelmode) ? interval*1000 : 0);
-- 
2.1.3

Re: [RESEND PATCH v5 4/5] acerhdf: added critical trip point

[Darren Hart]

On Fri, Nov 28, 2014 at 03:20:51PM +0100, Peter Feuerer wrote:
> added critical trip point which represents the temperature limit.

Nitpic, Add ^

> Added return -EINVAL in case wrong trip point is provided.

Add (we are going to add it with this patch, it wasn't added previously). It's a
nitpic. But this is more consistent with typical commit message language.

I might normally just fix that upon commit, but since I had a few questions on
the last one, if that results in a V2, please correct this as well.

> 
> Cc: platform-driver-x86@vger.kernel.org
> Cc: Darren Hart <dvhart@infradead.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andreas Mohr <andi@lisas.de>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Javi Merino <javi.merino@arm.com>
> Signed-off-by: Peter Feuerer <peter@piie.net>
> ---
>  drivers/platform/x86/acerhdf.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
> index 7fe7dbf..91b16c8 100644
> --- a/drivers/platform/x86/acerhdf.c
> +++ b/drivers/platform/x86/acerhdf.c
> @@ -443,6 +443,10 @@ static int acerhdf_get_trip_type(struct thermal_zone_device *thermal, int trip,
>  {
>  	if (trip == 0)
>  		*type = THERMAL_TRIP_ACTIVE;
> +	else if (trip == 1)
> +		*type = THERMAL_TRIP_CRITICAL;
> +	else
> +		return -EINVAL;
>  
>  	return 0;
>  }
> @@ -463,6 +467,10 @@ static int acerhdf_get_trip_temp(struct thermal_zone_device *thermal, int trip,
>  {
>  	if (trip == 0)
>  		*temp = fanon;
> +	else if (trip == 1)
> +		*temp = ACERHDF_TEMP_CRIT;
> +	else
> +		return -EINVAL;
>  
>  	return 0;
>  }
> @@ -713,7 +721,7 @@ static int acerhdf_register_thermal(void)
>  	if (IS_ERR(cl_dev))
>  		return -EINVAL;
>  
> -	thz_dev = thermal_zone_device_register("acerhdf", 1, 0, NULL,
> +	thz_dev = thermal_zone_device_register("acerhdf", 2, 0, NULL,
>  					      &acerhdf_dev_ops,
>  					      &acerhdf_zone_params, 0,
>  					      (kernelmode) ? interval*1000 : 0);
> -- 
> 2.1.3
> 
> 

-- 
Darren Hart
Intel Open Source Technology Center

[RESEND PATCH v5 5/5] acerhdf: minor clean up

[Peter Feuerer]

  * renamed bios_settings_t to bios_settings, as it is no typedef
  * replaced "unsigned char" by u8 in bios_settings struct for better
    readability.

Cc: platform-driver-x86@vger.kernel.org
Cc: Darren Hart <dvhart@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andreas Mohr <andi@lisas.de>
Acked-by: Borislav Petkov <bp@suse.de>
Signed-off-by: Peter Feuerer <peter@piie.net>
---
 drivers/platform/x86/acerhdf.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/acerhdf.c b/drivers/platform/x86/acerhdf.c
index 91b16c8..c07623c 100644
--- a/drivers/platform/x86/acerhdf.c
+++ b/drivers/platform/x86/acerhdf.c
@@ -131,18 +131,18 @@ static const struct manualcmd mcmd = {
 };
 
 /* BIOS settings */
-struct bios_settings_t {
+struct bios_settings {
 	const char *vendor;
 	const char *product;
 	const char *version;
-	unsigned char fanreg;
-	unsigned char tempreg;
+	u8 fanreg;
+	u8 tempreg;
 	struct fancmd cmd;
 	int mcmd_enable;
 };
 
 /* Register addresses and values for different BIOS versions */
-static const struct bios_settings_t bios_tbl[] = {
+static const struct bios_settings bios_tbl[] = {
 	/* AOA110 */
 	{"Acer", "AOA110", "v0.3109", 0x55, 0x58, {0x1f, 0x00}, 0},
 	{"Acer", "AOA110", "v0.3114", 0x55, 0x58, {0x1f, 0x00}, 0},
@@ -256,7 +256,7 @@ static const struct bios_settings_t bios_tbl[] = {
 	{"", "", "", 0, 0, {0, 0}, 0}
 };
 
-static const struct bios_settings_t *bios_cfg __read_mostly;
+static const struct bios_settings *bios_cfg __read_mostly;
 
 /*
  * this struct is used to instruct thermal layer to use bang_bang instead of
@@ -619,7 +619,7 @@ static int str_starts_with(const char *str, const char *start)
 static int acerhdf_check_hardware(void)
 {
 	char const *vendor, *version, *product;
-	const struct bios_settings_t *bt = NULL;
+	const struct bios_settings *bt = NULL;
 
 	/* get BIOS data */
 	vendor  = dmi_get_system_info(DMI_SYS_VENDOR);
-- 
2.1.3

Re: [RESEND PATCH v5 0/5] acerhdf: adding new models, appropriate governor and minor clean up

[Peter Feuerer]

Hi Darren / platform-driver-x86 list,

Peter Feuerer writes:
> please apply this series of patches.

did you find time for my patchseries already?
Is there any problem with it?

-- 
kind regards,
--peter;

Re: [RESEND PATCH v5 0/5] acerhdf: adding new models, appropriate governor and minor clean up

[Darren Hart]

On Fri, Nov 28, 2014 at 03:20:47PM +0100, Peter Feuerer wrote:
> Hi Darren,
> 
> please apply this series of patches.

Applied to testing, and given the level of review and testing already received,
I will include in for-next for a late merge-window submission.

Thanks,

-- 
Darren Hart
Intel Open Source Technology Center

Re: [RESEND PATCH v5 0/5] acerhdf: adding new models, appropriate governor and minor clean up

[Darren Hart]

On Fri, Nov 28, 2014 at 03:20:47PM +0100, Peter Feuerer wrote:
> Hi Darren,
> 
> please apply this series of patches.
> 
> It is intended to:
> 
>   * Introduce "manual mode" support (Patch 1 & 2), which is needed to control
>     the fan of a few new models.
> 
>   * Add an appropriate thermal governor (Patch 3).  Manipulating and
>     fiddling around with the step-wise governor has been a very fragile thing
>     in the past and as it broke again, I used the opportunity to add a two
>     point thermal governor which implements the actual fan handling required by
>     acerhdf and puts from my point of view things straight.
> 
>   * Do some minor clean up like:
>       - adding second trip point for critical temperature (Patch 4)
>       - removing _t suffix from struct which isn't typedef and replace unsigned
>         char by u8 (Patch 5)
> 
> Thanks and kind regards,
> peter 
> 
> Peter Feuerer (5):
>   acerhdf: Adding support for "manual mode"
>   acerhdf: Adding support for new models
>   acerhdf: Use bang-bang thermal governor
>   acerhdf: added critical trip point
>   acerhdf: minor clean up

Queued to for-next. Thanks!

-- 
Darren Hart
Intel Open Source Technology Center