Samus: Handle fan startup in the EC, not the fan controller

The fans on samus have a recommended minimum duty cycle of 20%
while running, but 30% in order to start. We've been using the
EC's built-in fan controller for the start requirement, but it
has a minimum fast-start duty cycle of 50%. It turns out that
that speed is noticeably noisy.

This change handles the startup with logic in the EC instead, so
that the fan only tries to spin at 30% initially (or if it drops
too much below the minimum turning speed).

BUG=chrome-os-partner:33429
BRANCH=ToT,samus
TEST=make buildall -j

Boot the system, let it idle with the browser windows closed, the
browse a bit, then idle. Listen for changes to the fans.

Before, I could hear the fans kick in and out as the AP load
changed. Now it's much quieter.

Change-Id: Id35215520c064eb6843686ec8bb5f3618dac6cf6
Signed-off-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/227658
Reviewed-by: Randall Spangler <rspangler@chromium.org>

14 files changed, 167 insertions, 8 deletions

diff --git a/board/auron/board.c b/board/auron/board.c
index 9c91c8845d..1a56ac5492 100644
--- a/board/auron/board.c
+++ b/board/auron/board.c
@@ -69,6 +69,7 @@ BUILD_ASSERT(ARRAY_SIZE(adc_channels) == ADC_CH_COUNT);
 const struct fan_t fans[] = {
 	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 1000,
+	 .rpm_start = 1000,
 	 .rpm_max = 5050,
 	 .ch = 2,
 	 .pgood_gpio = GPIO_PP5000_PGOOD,
diff --git a/board/falco/board.c b/board/falco/board.c
index 4d04312816..b1bd747ff2 100644
--- a/board/falco/board.c
+++ b/board/falco/board.c
@@ -74,6 +74,7 @@ BUILD_ASSERT(ARRAY_SIZE(adc_channels) == ADC_CH_COUNT);
 const struct fan_t fans[] = {
 	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 1000,
+	 .rpm_start = 1000,
 	 .rpm_max = 5050,
 	 .ch = 2,
 	 .pgood_gpio = GPIO_PP5000_PGOOD,
diff --git a/board/host/fan.c b/board/host/fan.c
index 18127b9eb0..ee7b15ef12 100644
--- a/board/host/fan.c
+++ b/board/host/fan.c
@@ -11,6 +11,7 @@
 const struct fan_t fans[] = {
 	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 1000,
+	 .rpm_start = 1500,
 	 .rpm_max = 5000,
 	 .ch = 0,
 	 .pgood_gpio = -1,
@@ -49,7 +50,7 @@ int fan_get_rpm_mode(int ch)
 	return mock_rpm_mode;
 }
 
-static int mock_rpm;
+int mock_rpm;
 void fan_set_rpm_target(int ch, int rpm)
 {
 	mock_rpm = rpm;
diff --git a/board/link/board.c b/board/link/board.c
index 8e2881a664..3895885877 100644
--- a/board/link/board.c
+++ b/board/link/board.c
@@ -77,6 +77,7 @@ BUILD_ASSERT(ARRAY_SIZE(pwm_channels) == PWM_CH_COUNT);
 const struct fan_t fans[] = {
 	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 1500,
+	 .rpm_start = 1500,
 	 .rpm_max = 9300,
 	 .ch = 0,
 	 .pgood_gpio = GPIO_PGOOD_5VALW,
diff --git a/board/mec1322_evb/board.c b/board/mec1322_evb/board.c
index 7deae735b8..774abe0239 100644
--- a/board/mec1322_evb/board.c
+++ b/board/mec1322_evb/board.c
@@ -30,6 +30,7 @@ BUILD_ASSERT(ARRAY_SIZE(adc_channels) == ADC_CH_COUNT);
 const struct fan_t fans[] = {
 	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 1500,
+	 .rpm_start = 1500,
 	 .rpm_max = 8000,
 	 .ch = 0,
 	 .pgood_gpio = -1,
diff --git a/board/peppy/board.c b/board/peppy/board.c
index 3a3a92d29f..2ab34f5029 100644
--- a/board/peppy/board.c
+++ b/board/peppy/board.c
@@ -69,6 +69,7 @@ BUILD_ASSERT(ARRAY_SIZE(adc_channels) == ADC_CH_COUNT);
 const struct fan_t fans[] = {
 	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 1000,
+	 .rpm_start = 1000,
 	 .rpm_max = 5050,
 	 .ch = 2,
 	 .pgood_gpio = GPIO_PP5000_PGOOD,
diff --git a/board/samus/board.c b/board/samus/board.c
index e6682d4717..68c5146438 100644
--- a/board/samus/board.c
+++ b/board/samus/board.c
@@ -106,15 +106,17 @@ BUILD_ASSERT(ARRAY_SIZE(pwm_channels) == PWM_CH_COUNT);
 
 /* Physical fans. These are logically separate from pwm_channels. */
 const struct fan_t fans[] = {
-	{.flags = FAN_USE_RPM_MODE | FAN_USE_FAST_START,
+	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 2286,
+	 .rpm_start = 3090,
 	 .rpm_max = 6350,
 	 .ch = 2,
 	 .pgood_gpio = -1,
 	 .enable_gpio = -1,
 	},
-	{.flags = FAN_USE_RPM_MODE | FAN_USE_FAST_START,
+	{.flags = FAN_USE_RPM_MODE,
 	 .rpm_min = 2286,
+	 .rpm_start = 3090,
 	 .rpm_max = 6350,
 	 .ch = 3,
 	 .pgood_gpio = -1,
diff --git a/chip/lm4/fan.c b/chip/lm4/fan.c
index 1180d51906..01f561433d 100644
--- a/chip/lm4/fan.c
+++ b/chip/lm4/fan.c
@@ -98,7 +98,7 @@ int fan_get_rpm_target(int ch)
 	return (LM4_FAN_FANCMD(ch) & MAX_RPM) * RPM_SCALE;
 }
 
-void fan_set_rpm_target(int ch, int rpm)
+test_mockable void fan_set_rpm_target(int ch, int rpm)
 {
 	/* Apply fan scaling */
 	if (rpm > 0)
diff --git a/common/fan.c b/common/fan.c
index db0b9a0d86..b99b770580 100644
--- a/common/fan.c
+++ b/common/fan.c
@@ -15,6 +15,10 @@
 #include "system.h"
 #include "util.h"
 
+/* Console output macros */
+#define CPUTS(outstr) cputs(CC_THERMAL, outstr)
+#define CPRINTS(format, args...) cprints(CC_THERMAL, format, ## args)
+
 /* True if we're listening to the thermal control task. False if we're setting
  * things manually. */
 static int thermal_control_enabled[CONFIG_FANS];
@@ -44,14 +48,29 @@ int fan_percent_to_rpm(int fan, int pct)
 /* The thermal task will only call this function with pct in [0,100]. */
 test_mockable void fan_set_percent_needed(int fan, int pct)
 {
-	int rpm;
+	int actual_rpm, new_rpm;
+	static int prev_rpm[CONFIG_FANS];
 
 	if (!thermal_control_enabled[fan])
 		return;
 
-	rpm = fan_percent_to_rpm(fan, pct);
+	new_rpm = fan_percent_to_rpm(fan, pct);
+	actual_rpm = fan_get_rpm_actual(fans[fan].ch);
 
-	fan_set_rpm_target(fans[fan].ch, rpm);
+	/* If we want to turn and the fans are currently significantly below
+	 * the minimum turning speed, we should turn at least as fast as the
+	 * necessary start speed instead. */
+	if (new_rpm &&
+	    actual_rpm < fans[fan].rpm_min * 9 / 10 &&
+	    new_rpm < fans[fan].rpm_start)
+		new_rpm = fans[fan].rpm_start;
+
+	if (new_rpm != prev_rpm[fan]) {
+		CPRINTS("Fan %d %d%% => %d rpm", fan, pct, new_rpm);
+		prev_rpm[fan] = new_rpm;
+	}
+
+	fan_set_rpm_target(fans[fan].ch, new_rpm);
 }
 
 static void set_enabled(int fan, int enable)
diff --git a/include/fan.h b/include/fan.h
index a8352987d8..bd94ba8bd1 100644
--- a/include/fan.h
+++ b/include/fan.h
@@ -11,7 +11,9 @@
 /* Characteristic of each physical fan */
 struct fan_t {
 	unsigned int flags;
+	/* rpm_min is to keep turning. rpm_start is to begin turning */
 	int rpm_min;
+	int rpm_start;
 	int rpm_max;
 	/* Hardware channel number (the meaning is chip-specific) */
 	int ch;
diff --git a/test/build.mk b/test/build.mk
index 593142b58d..ff5e617fbb 100644
--- a/test/build.mk
+++ b/test/build.mk
@@ -31,7 +31,7 @@ test-list-host+=thermal flash queue kb_8042 extpwr_gpio console_edit system
 test-list-host+=sbs_charging adapter host_command thermal_falco led_spring
 test-list-host+=bklight_lid bklight_passthru interrupt timer_dos button
 test-list-host+=motion_lid math_util sbs_charging_v2 battery_get_params_smart
-test-list-host+=lightbar inductive_charging usb_pd
+test-list-host+=lightbar inductive_charging usb_pd fan
 
 adapter-y=adapter.o
 battery_get_params_smart-y=battery_get_params_smart.o
@@ -70,3 +70,4 @@ usb_pd-y=usb_pd.o
 utils-y=utils.o
 battery_get_params_smart-y=battery_get_params_smart.o
 lightbar-y=lightbar.o
+fan-y=fan.o
diff --git a/test/fan.c b/test/fan.c
new file mode 100644
index 0000000000..9a0fa0d225
--- /dev/null
+++ b/test/fan.c
@@ -0,0 +1,108 @@
+/* Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ *
+ * Test thermal engine.
+ */
+
+#include "common.h"
+#include "console.h"
+#include "fan.h"
+#include "hooks.h"
+#include "host_command.h"
+#include "printf.h"
+#include "temp_sensor.h"
+#include "test_util.h"
+#include "thermal.h"
+#include "timer.h"
+#include "util.h"
+
+
+/*****************************************************************************/
+/* Tests */
+
+static int test_fan(void)
+{
+	/* "actual" fan speed from board/host/fan.c */
+	extern int mock_rpm;
+
+	sleep(2);
+
+	/* With nothing else to do, fans default to full-on */
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_max);
+
+	/*
+	 * fan_set_percent_needed() is normally called once a second by the
+	 * thermal task, but we're not using a thermal test in this test so
+	 * we can dink around with the fans without having to wait. The host
+	 * implementation just sets mock_rpm to whatever it's asked for.
+	 */
+
+	/* Off */
+	fan_set_percent_needed(0, 0);
+	TEST_ASSERT(fan_get_rpm_actual(0) == 0);
+	fan_set_percent_needed(0, 0);
+	TEST_ASSERT(fan_get_rpm_actual(0) == 0);
+
+	/* On, but just barely */
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_start);
+	/* fan is above min speed now, so should be set to min */
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_min);
+
+	/* Full speed */
+	fan_set_percent_needed(0, 100);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_max);
+	fan_set_percent_needed(0, 100);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_max);
+
+	/* Slow again */
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_min);
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_min);
+
+	/* Off */
+	fan_set_percent_needed(0, 0);
+	TEST_ASSERT(fan_get_rpm_actual(0) == 0);
+	fan_set_percent_needed(0, 0);
+	TEST_ASSERT(fan_get_rpm_actual(0) == 0);
+
+	/* On, but just barely */
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_start);
+	/* Force the mock_rpm to be slow, to simulate dragging */
+	mock_rpm = fans[0].rpm_min - 105;
+	/* It should keep trying for the start speed */
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_start);
+	/* But we have to keep forcing the mock_rpm back down */
+	mock_rpm = fans[0].rpm_min - 105;
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_start);
+	/* Now let it turn just under rpm_min. Should be okay there. */
+	mock_rpm = fans[0].rpm_min - 10;
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_min);
+	/* Let it go a little faster, still okay */
+	mock_rpm = fans[0].rpm_min + 10;
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_min);
+	/* But if it drops too low, it should go back to the start speed */
+	mock_rpm = fans[0].rpm_min - 105;
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_start);
+	/* And then relax */
+	fan_set_percent_needed(0, 1);
+	TEST_ASSERT(fan_get_rpm_actual(0) == fans[0].rpm_min);
+
+	return EC_SUCCESS;
+}
+
+void run_test(void)
+{
+	RUN_TEST(test_fan);
+
+	test_print_result();
+}
diff --git a/test/fan.tasklist b/test/fan.tasklist
new file mode 100644
index 0000000000..c72de237cd
--- /dev/null
+++ b/test/fan.tasklist
@@ -0,0 +1,17 @@
+/* Copyright (c) 2014 The Chromium OS Authors. All rights reserved.
+ * Use of this source code is governed by a BSD-style license that can be
+ * found in the LICENSE file.
+ */
+
+/**
+ * List of enabled tasks in the priority order
+ *
+ * The first one has the lowest priority.
+ *
+ * For each task, use the macro TASK_TEST(n, r, d, s) where :
+ * 'n' in the name of the task
+ * 'r' in the main routine of the task
+ * 'd' in an opaque parameter passed to the routine at startup
+ * 's' is the stack size in bytes; must be a multiple of 8
+ */
+#define CONFIG_TEST_TASK_LIST   /* No test task */
diff --git a/test/test_config.h b/test/test_config.h
index 9a06858834..303bc54f86 100644
--- a/test/test_config.h
+++ b/test/test_config.h
@@ -104,6 +104,10 @@ int board_discharge_on_ac(int enabled);
 #define I2C_PORT_MASTER 1
 #endif
 
+#ifdef TEST_FAN
+#define CONFIG_FANS 1
+#endif
+
 #ifdef TEST_BUTTON
 #define CONFIG_BUTTON_COUNT 2
 #define CONFIG_KEYBOARD_PROTOCOL_8042