Provide multiple fan support within the EC itself

This adds explicit "int fan" args to the exported functions from
common/fan.c: fan_set_percent_needed() and fan_percent_to_rpm(). Within that
file, multiple fans are handled independently.

This is not complete, though. Host commands and sysjump support still only
handle a single fan, so at the moment multiple fans are treated identically
in those cases.

BUG=chrome-os-partner:23530
BRANCH=none
TEST=manual

All boards build, "make runtests" passes.

On a multi-fan system, the EC command "faninfo" displays multiple results:

  > faninfo
  Fan 0 Actual:    0 rpm
  Fan 0 Target:    0 rpm
  Fan 0 Duty:   0%
  Fan 0 Status: 0 (not spinning)
  Fan 0 Mode:   rpm
  Fan 0 Auto:   yes
  Fan 0 Enable: yes

  Fan 1 Actual:    0 rpm
  Fan 1 Target:    0 rpm
  Fan 1 Duty:   0%
  Fan 1 Status: 0 (not spinning)
  Fan 1 Mode:   rpm
  Fan 1 Auto:   no
  Fan 1 Enable: no
  >

and the "fanduty", "fanset", and "fanauto" all require the fan number as the
first arg:

  > fanduty 0 30
  Setting fan 0 duty cycle to 30%
  > fanset 1 2000
  Setting fan 1 rpm target to 2000
  > fanauto 0
  > fanauto 1

On single-fan systems, there is no visible change.

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

8 files changed, 205 insertions, 105 deletions

diff --git a/board/falco/board.c b/board/falco/board.c
index b0a8ece31d..5d90ef3aa6 100644
--- a/board/falco/board.c
+++ b/board/falco/board.c
@@ -259,10 +259,10 @@ int board_discharge_on_ac(int enable)
  * And never turn it off. Bah. That'll do wonders for battery life.
  */
 #ifdef CONFIG_FAN_RPM_CUSTOM
-int fan_percent_to_rpm(int pct)
+int fan_percent_to_rpm(int fan, int pct)
 {
-	const int FAN_MAX = 5050;
-	const int FAN_MIN = 2700;
+	const int FAN_MAX = fans[fan].rpm_max;
+	const int FAN_MIN = fans[fan].rpm_min;
 	const int NUM_STEPS = 7;
 	const int m = 100 * 100 / NUM_STEPS;
 	const int m0 = m / 200;
diff --git a/board/samus/board.c b/board/samus/board.c
index a2e1e14b30..2fccd9d310 100644
--- a/board/samus/board.c
+++ b/board/samus/board.c
@@ -179,9 +179,9 @@ const struct adc_t adc_channels[] = {
 	{"ChargerCurrent", LM4_ADC_SEQ1, 33000, ADC_READ_MAX * 4, 0,
 	 LM4_AIN(11), 0x06 /* IE0 | END0 */, LM4_GPIO_B, (1<<5)},
 
-	/* FIXME: We don't know what to expect here, but it's an analog input
-	 * that's pulled high. We're using it as a battery presence indicator
-	 * for now. We'll return just 0 - ADC_READ_MAX for now.
+	/* TODO: We don't know what to expect here, but it's an analog input
+	 * that's pulled high. We're only using it as a battery presence
+	 * indicator for now, so we'll just return 0 - ADC_READ_MAX.
 	 */
 	{"BatteryTemp", LM4_ADC_SEQ2, 1, 1, 0,
 	 LM4_AIN(10), 0x06 /* IE0 | END0 */, LM4_GPIO_B, (1<<4)},
diff --git a/chip/lm4/fan.c b/chip/lm4/fan.c
index a342a49954..955113ae57 100644
--- a/chip/lm4/fan.c
+++ b/chip/lm4/fan.c
@@ -127,7 +127,7 @@ int fan_is_stalled(int ch)
 		return 0;
 
 	/* Check for stall condition */
-	return (((LM4_FAN_FANSTS >> (2 * ch)) & 0x03) == 0) ? 1 : 0;
+	return fan_get_status(ch) == FAN_STATUS_STOPPED;
 }
 
 void fan_channel_setup(int ch, unsigned int flags)
diff --git a/common/fan.c b/common/fan.c
index 209953f342..874b73537b 100644
--- a/common/fan.c
+++ b/common/fan.c
@@ -11,16 +11,13 @@
 #include "gpio.h"
 #include "hooks.h"
 #include "host_command.h"
+#include "printf.h"
 #include "system.h"
 #include "util.h"
 
-/* HEY - this is temporary! (crosbug.com/p/23530) */
-#define CONFIG_FAN_CH_CPU (fans[0].ch)
-#define HEY0 0
-
 /* True if we're listening to the thermal control task. False if we're setting
  * things manually. */
-static int thermal_control_enabled;
+static int thermal_control_enabled[CONFIG_FANS];
 
 #ifndef CONFIG_FAN_RPM_CUSTOM
 /* This is the default implementation. It's only called over [0,100].
@@ -28,15 +25,15 @@ static int thermal_control_enabled;
  * the way down to zero because most fans won't turn that slowly, so
  * we'll map [1,100] => [FAN_MIN,FAN_MAX], and [0] => "off".
 */
-int fan_percent_to_rpm(int pct)
+int fan_percent_to_rpm(int fan, int pct)
 {
 	int rpm, max, min;
 
 	if (!pct) {
 		rpm = 0;
 	} else {
-		min = fans[HEY0].rpm_min;
-		max = fans[HEY0].rpm_max;
+		min = fans[fan].rpm_min;
+		max = fans[fan].rpm_max;
 		rpm = ((pct - 1) * max + (100 - pct) * min) / 99;
 	}
 
@@ -45,48 +42,48 @@ int fan_percent_to_rpm(int pct)
 #endif	/* CONFIG_FAN_RPM_CUSTOM */
 
 /* The thermal task will only call this function with pct in [0,100]. */
-test_mockable void fan_set_percent_needed(int pct)
+test_mockable void fan_set_percent_needed(int fan, int pct)
 {
 	int rpm;
 
-	if (!thermal_control_enabled)
+	if (!thermal_control_enabled[fan])
 		return;
 
-	rpm = fan_percent_to_rpm(pct);
+	rpm = fan_percent_to_rpm(fan, pct);
 
-	fan_set_rpm_target(CONFIG_FAN_CH_CPU, rpm);
+	fan_set_rpm_target(fans[fan].ch, rpm);
 }
 
-static void set_enabled(int enable)
+static void set_enabled(int fan, int enable)
 {
-	fan_set_enabled(CONFIG_FAN_CH_CPU, enable);
+	fan_set_enabled(fans[fan].ch, enable);
 
-	if (fans[HEY0].enable_gpio >= 0)
-		gpio_set_level(fans[HEY0].enable_gpio, enable);
+	if (fans[fan].enable_gpio >= 0)
+		gpio_set_level(fans[fan].enable_gpio, enable);
 }
 
-static void set_thermal_control_enabled(int enable)
+static void set_thermal_control_enabled(int fan, int enable)
 {
-	thermal_control_enabled	= enable;
+	thermal_control_enabled[fan] = enable;
 
 	/* If controlling the fan, need it in RPM-control mode */
 	if (enable)
-		fan_set_rpm_mode(CONFIG_FAN_CH_CPU, 1);
+		fan_set_rpm_mode(fans[fan].ch, 1);
 }
 
-static void set_duty_cycle(int percent)
+static void set_duty_cycle(int fan, int percent)
 {
 	/* Move the fan to manual control */
-	fan_set_rpm_mode(CONFIG_FAN_CH_CPU, 0);
+	fan_set_rpm_mode(fans[fan].ch, 0);
 
 	/* Always enable the fan */
-	set_enabled(1);
+	set_enabled(fan, 1);
 
 	/* Disable thermal engine automatic fan control. */
-	set_thermal_control_enabled(0);
+	set_thermal_control_enabled(fan, 0);
 
 	/* Set the duty cycle */
-	fan_set_duty(CONFIG_FAN_CH_CPU, percent);
+	fan_set_duty(fans[fan].ch, percent);
 }
 
 /*****************************************************************************/
@@ -94,46 +91,77 @@ static void set_duty_cycle(int percent)
 
 static int cc_fanauto(int argc, char **argv)
 {
-	set_thermal_control_enabled(1);
+	char *e;
+	int fan = 0;
+
+	if (CONFIG_FANS > 1) {
+		if (argc < 2) {
+			ccprintf("fan number is required as the first arg\n");
+			return EC_ERROR_PARAM_COUNT;
+		}
+		fan = strtoi(argv[1], &e, 0);
+		if (*e || fan >= CONFIG_FANS)
+			return EC_ERROR_PARAM1;
+		argc--;
+		argv++;
+	}
+
+	set_thermal_control_enabled(fan, 1);
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(fanauto, cc_fanauto,
-			NULL,
+			"{fan}",
 			"Enable thermal fan control",
 			NULL);
 
+/* Return 0 for off, 1 for on, -1 for unknown */
+static int is_powered(int fan)
+{
+	int is_pgood = -1;
+
+	/* If we have an enable output, see if it's on or off. */
+	if (fans[fan].enable_gpio >= 0)
+		is_pgood = gpio_get_level(fans[fan].enable_gpio);
+	/* If we have a pgood input, it overrides any enable output. */
+	if (fans[fan].pgood_gpio >= 0)
+		is_pgood = gpio_get_level(fans[fan].pgood_gpio);
+
+	return is_pgood;
+}
+
 static int cc_faninfo(int argc, char **argv)
 {
 	static const char * const human_status[] = {
 		"not spinning", "changing", "locked", "frustrated"
 	};
 	int tmp, is_pgood;
-
-	ccprintf("Actual: %4d rpm\n",
-		 fan_get_rpm_actual(CONFIG_FAN_CH_CPU));
-	ccprintf("Target: %4d rpm\n",
-		 fan_get_rpm_target(CONFIG_FAN_CH_CPU));
-	ccprintf("Duty:   %d%%\n",
-		 fan_get_duty(CONFIG_FAN_CH_CPU));
-	tmp = fan_get_status(CONFIG_FAN_CH_CPU);
-	ccprintf("Status: %d (%s)\n", tmp, human_status[tmp]);
-	ccprintf("Mode:   %s\n",
-		 fan_get_rpm_mode(CONFIG_FAN_CH_CPU) ? "rpm" : "duty");
-	ccprintf("Auto:   %s\n", thermal_control_enabled ? "yes" : "no");
-	ccprintf("Enable: %s\n",
-		 fan_get_enabled(CONFIG_FAN_CH_CPU) ? "yes" : "no");
-
-	/* Assume we don't know */
-	is_pgood = -1;
-	/* If we have an enable output, see if it's on or off. */
-	if (fans[HEY0].enable_gpio >= 0)
-		is_pgood = gpio_get_level(fans[HEY0].enable_gpio);
-	/* If we have a pgood input, it overrides any enable output. */
-	if (fans[HEY0].pgood_gpio >= 0)
-		is_pgood = gpio_get_level(fans[HEY0].pgood_gpio);
-	/* If we think we know, say so */
-	if (is_pgood >= 0)
-		ccprintf("Power:  %s\n", is_pgood ? "yes" : "no");
+	int fan;
+	char leader[20] = "";
+	for (fan = 0; fan < CONFIG_FANS; fan++) {
+		if (CONFIG_FANS > 1)
+			snprintf(leader, sizeof(leader), "Fan %d ", fan);
+		if (fan)
+			ccprintf("\n");
+		ccprintf("%sActual: %4d rpm\n", leader,
+			 fan_get_rpm_actual(fans[fan].ch));
+		ccprintf("%sTarget: %4d rpm\n", leader,
+			 fan_get_rpm_target(fans[fan].ch));
+		ccprintf("%sDuty:   %d%%\n", leader,
+			 fan_get_duty(fans[fan].ch));
+		tmp = fan_get_status(fans[fan].ch);
+		ccprintf("%sStatus: %d (%s)\n", leader,
+			 tmp, human_status[tmp]);
+		ccprintf("%sMode:   %s\n", leader,
+			 fan_get_rpm_mode(fans[fan].ch) ? "rpm" : "duty");
+		ccprintf("%sAuto:   %s\n", leader,
+			 thermal_control_enabled[fan] ? "yes" : "no");
+		ccprintf("%sEnable: %s\n", leader,
+			 fan_get_enabled(fans[fan].ch) ? "yes" : "no");
+		is_pgood = is_powered(fan);
+		if (is_pgood >= 0)
+			ccprintf("%sPower:  %s\n", leader,
+				 is_pgood ? "yes" : "no");
+	}
 
 	return EC_SUCCESS;
 }
@@ -146,6 +174,19 @@ static int cc_fanset(int argc, char **argv)
 {
 	int rpm;
 	char *e;
+	int fan = 0;
+
+	if (CONFIG_FANS > 1) {
+		if (argc < 2) {
+			ccprintf("fan number is required as the first arg\n");
+			return EC_ERROR_PARAM_COUNT;
+		}
+		fan = strtoi(argv[1], &e, 0);
+		if (*e || fan >= CONFIG_FANS)
+			return EC_ERROR_PARAM1;
+		argc--;
+		argv++;
+	}
 
 	if (argc < 2)
 		return EC_ERROR_PARAM_COUNT;
@@ -157,28 +198,28 @@ static int cc_fanset(int argc, char **argv)
 			rpm = 0;
 		else if (rpm > 100)
 			rpm = 100;
-		rpm = fan_percent_to_rpm(rpm);
+		rpm = fan_percent_to_rpm(fan, rpm);
 	} else if (*e) {
 		return EC_ERROR_PARAM1;
 	}
 
 	/* Move the fan to automatic control */
-	fan_set_rpm_mode(CONFIG_FAN_CH_CPU, 1);
+	fan_set_rpm_mode(fans[fan].ch, 1);
 
 	/* Always enable the fan */
-	set_enabled(1);
+	set_enabled(fan, 1);
 
 	/* Disable thermal engine automatic fan control. */
-	set_thermal_control_enabled(0);
+	set_thermal_control_enabled(fan, 0);
 
-	fan_set_rpm_target(CONFIG_FAN_CH_CPU, rpm);
+	fan_set_rpm_target(fans[fan].ch, rpm);
 
-	ccprintf("Setting fan rpm target to %d\n", rpm);
+	ccprintf("Setting fan %d rpm target to %d\n", fan, rpm);
 
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(fanset, cc_fanset,
-			"rpm | pct%",
+			"{fan} (rpm | pct%)",
 			"Set fan speed",
 			NULL);
 
@@ -186,6 +227,19 @@ static int cc_fanduty(int argc, char **argv)
 {
 	int percent = 0;
 	char *e;
+	int fan = 0;
+
+	if (CONFIG_FANS > 1) {
+		if (argc < 2) {
+			ccprintf("fan number is required as the first arg\n");
+			return EC_ERROR_PARAM_COUNT;
+		}
+		fan = strtoi(argv[1], &e, 0);
+		if (*e || fan >= CONFIG_FANS)
+			return EC_ERROR_PARAM1;
+		argc--;
+		argv++;
+	}
 
 	if (argc < 2)
 		return EC_ERROR_PARAM_COUNT;
@@ -194,13 +248,13 @@ static int cc_fanduty(int argc, char **argv)
 	if (*e)
 		return EC_ERROR_PARAM1;
 
-	ccprintf("Setting fan duty cycle to %d%%\n", percent);
-	set_duty_cycle(percent);
+	ccprintf("Setting fan %d duty cycle to %d%%\n", fan, percent);
+	set_duty_cycle(fan, percent);
 
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(fanduty, cc_fanduty,
-			"percent",
+			"{fan} percent",
 			"Set fan duty cycle",
 			NULL);
 
@@ -210,8 +264,10 @@ DECLARE_CONSOLE_COMMAND(fanduty, cc_fanduty,
 static int hc_pwm_get_fan_target_rpm(struct host_cmd_handler_args *args)
 {
 	struct ec_response_pwm_get_fan_rpm *r = args->response;
+	int fan = 0;
 
-	r->rpm = fan_get_rpm_target(CONFIG_FAN_CH_CPU);
+	/* TODO(crosbug.com/p/23803) */
+	r->rpm = fan_get_rpm_target(fans[fan].ch);
 	args->response_size = sizeof(*r);
 
 	return EC_RES_SUCCESS;
@@ -223,10 +279,14 @@ DECLARE_HOST_COMMAND(EC_CMD_PWM_GET_FAN_TARGET_RPM,
 static int hc_pwm_set_fan_target_rpm(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_pwm_set_fan_target_rpm *p = args->params;
+	int fan;
 
-	set_thermal_control_enabled(0);
-	fan_set_rpm_mode(CONFIG_FAN_CH_CPU, 1);
-	fan_set_rpm_target(CONFIG_FAN_CH_CPU, p->rpm);
+	/* TODO(crosbug.com/p/23803) */
+	for (fan = 0; fan < CONFIG_FANS; fan++) {
+		set_thermal_control_enabled(fan, 0);
+		fan_set_rpm_mode(fans[fan].ch, 1);
+		fan_set_rpm_target(fans[fan].ch, p->rpm);
+	}
 
 	return EC_RES_SUCCESS;
 }
@@ -237,7 +297,11 @@ DECLARE_HOST_COMMAND(EC_CMD_PWM_SET_FAN_TARGET_RPM,
 static int hc_pwm_set_fan_duty(struct host_cmd_handler_args *args)
 {
 	const struct ec_params_pwm_set_fan_duty *p = args->params;
-	set_duty_cycle(p->percent);
+	int fan;
+
+	/* TODO(crosbug.com/p/23803) */
+	for (fan = 0; fan < CONFIG_FANS; fan++)
+		set_duty_cycle(fan, p->percent);
 
 	return EC_RES_SUCCESS;
 }
@@ -247,7 +311,12 @@ DECLARE_HOST_COMMAND(EC_CMD_PWM_SET_FAN_DUTY,
 
 static int hc_thermal_auto_fan_ctrl(struct host_cmd_handler_args *args)
 {
-	set_thermal_control_enabled(1);
+	int fan;
+
+	/* TODO(crosbug.com/p/23803) */
+	for (fan = 0; fan < CONFIG_FANS; fan++)
+		set_thermal_control_enabled(fan, 1);
+
 	return EC_RES_SUCCESS;
 }
 DECLARE_HOST_COMMAND(EC_CMD_THERMAL_AUTO_FAN_CTRL,
@@ -258,6 +327,12 @@ DECLARE_HOST_COMMAND(EC_CMD_THERMAL_AUTO_FAN_CTRL,
 /*****************************************************************************/
 /* Hooks */
 
+/* We only have a limited number of memory-mapped slots to report fan speed to
+ * the AP. If we have more fans than that, some will be inaccessible. But
+ * if we're using that many fans, we probably have bigger problems.
+ */
+BUILD_ASSERT(CONFIG_FANS <= EC_FAN_SPEED_ENTRIES);
+
 #define PWMFAN_SYSJUMP_TAG 0x5046  /* "PF" */
 #define PWM_HOOK_VERSION 1
 /* Saved PWM state across sysjumps */
@@ -273,6 +348,7 @@ static void pwm_fan_init(void)
 	uint16_t *mapped;
 	int version, size;
 	int i;
+	int fan = 0;
 
 	gpio_config_module(MODULE_PWM_FAN, 1);
 
@@ -283,14 +359,14 @@ static void pwm_fan_init(void)
 		system_get_jump_tag(PWMFAN_SYSJUMP_TAG, &version, &size);
 	if (prev && version == PWM_HOOK_VERSION && size == sizeof(*prev)) {
 		/* Restore previous state. */
-		fan_set_enabled(CONFIG_FAN_CH_CPU, prev->fan_en);
-		fan_set_rpm_target(CONFIG_FAN_CH_CPU, prev->fan_rpm);
+		fan_set_enabled(fans[fan].ch, prev->fan_en);
+		fan_set_rpm_target(fans[fan].ch, prev->fan_rpm);
 	} else {
 		/* Set initial fan speed to maximum */
-		fan_set_duty(CONFIG_FAN_CH_CPU, 100);
+		fan_set_duty(fans[fan].ch, 100);
 	}
 
-	set_thermal_control_enabled(1);
+	set_thermal_control_enabled(fan, 1);
 
 	/* Initialize memory-mapped data */
 	mapped = (uint16_t *)host_get_memmap(EC_MEMMAP_FAN);
@@ -302,27 +378,36 @@ DECLARE_HOOK(HOOK_INIT, pwm_fan_init, HOOK_PRIO_DEFAULT + 1);
 static void pwm_fan_second(void)
 {
 	uint16_t *mapped = (uint16_t *)host_get_memmap(EC_MEMMAP_FAN);
+	int stalled = 0;
+	int fan;
+
+	for (fan = 0; fan < CONFIG_FANS; fan++) {
+		if (fan_is_stalled(fans[fan].ch)) {
+			mapped[fan] = EC_FAN_SPEED_STALLED;
+			stalled = 1;
+			cprintf(CC_PWM, "[%T Fan %d stalled!]\n", fan);
+		} else {
+			mapped[fan] = fan_get_rpm_actual(fans[fan].ch);
+		}
+	}
 
-	if (fan_is_stalled(CONFIG_FAN_CH_CPU)) {
-		mapped[0] = EC_FAN_SPEED_STALLED;
-		/*
-		 * Issue warning.  As we have thermal shutdown
-		 * protection, issuing warning here should be enough.
-		 */
+	/*
+	 * Issue warning.  As we have thermal shutdown
+	 * protection, issuing warning here should be enough.
+	 */
+	if (stalled)
 		host_set_single_event(EC_HOST_EVENT_THERMAL);
-		cprintf(CC_PWM, "[%T Fan stalled!]\n");
-	} else {
-		mapped[0] = fan_get_rpm_actual(CONFIG_FAN_CH_CPU);
-	}
 }
 DECLARE_HOOK(HOOK_SECOND, pwm_fan_second, HOOK_PRIO_DEFAULT);
 
 static void pwm_fan_preserve_state(void)
 {
 	struct pwm_fan_state state;
+	int fan = 0;
 
-	state.fan_en = fan_get_enabled(CONFIG_FAN_CH_CPU);
-	state.fan_rpm = fan_get_rpm_target(CONFIG_FAN_CH_CPU);
+	/* TODO(crosbug.com/p/23530): Still treating all fans as one. */
+	state.fan_en = fan_get_enabled(fans[fan].ch);
+	state.fan_rpm = fan_get_rpm_target(fans[fan].ch);
 
 	system_add_jump_tag(PWMFAN_SYSJUMP_TAG, PWM_HOOK_VERSION,
 			    sizeof(state), &state);
@@ -331,19 +416,27 @@ DECLARE_HOOK(HOOK_SYSJUMP, pwm_fan_preserve_state, HOOK_PRIO_DEFAULT);
 
 static void pwm_fan_resume(void)
 {
-	fan_set_enabled(CONFIG_FAN_CH_CPU, 1);
+	int fan;
+	for (fan = 0; fan < CONFIG_FANS; fan++)
+		fan_set_enabled(fans[fan].ch, 1);
 }
 DECLARE_HOOK(HOOK_CHIPSET_RESUME, pwm_fan_resume, HOOK_PRIO_DEFAULT);
 
 static void pwm_fan_S3_S5(void)
 {
-	/* Take back fan control when the processor shuts down */
-	set_thermal_control_enabled(1);
-	/* For now don't do anything with it. We'll have to turn it on again if
-	 * we need active cooling during heavy battery charging or something.
-	 */
-	fan_set_rpm_target(CONFIG_FAN_CH_CPU, 0);
-	fan_set_enabled(CONFIG_FAN_CH_CPU, 0); /* crosbug.com/p/8097 */
+	int fan;
+
+	/* TODO(crosbug.com/p/23530): Still treating all fans as one. */
+	for (fan = 0; fan < CONFIG_FANS; fan++) {
+		/* Take back fan control when the processor shuts down */
+		set_thermal_control_enabled(fan, 1);
+		/* For now don't do anything with it. We'll have to turn it on
+		 * again if we need active cooling during heavy battery
+		 * charging or something.
+		 */
+		fan_set_rpm_target(fans[fan].ch, 0);
+		fan_set_enabled(fans[fan].ch, 0); /* crosbug.com/p/8097 */
+	}
 }
 DECLARE_HOOK(HOOK_CHIPSET_SUSPEND, pwm_fan_S3_S5, HOOK_PRIO_DEFAULT);
 DECLARE_HOOK(HOOK_CHIPSET_SHUTDOWN, pwm_fan_S3_S5, HOOK_PRIO_DEFAULT);
diff --git a/common/thermal.c b/common/thermal.c
index 92b6e170ab..c7840065de 100644
--- a/common/thermal.c
+++ b/common/thermal.c
@@ -155,8 +155,13 @@ static void thermal_control(void)
 	}
 
 #ifdef CONFIG_FANS
-	/* Max fan needed is what's needed. */
-	fan_set_percent_needed(fmax);
+	/* TODO(crosbug.com/p/23797): For now, we just treat all fans the
+	 * same. It would be better if we could assign different thermal
+	 * profiles to each fan - in case one fan cools the CPU while another
+	 * cools the radios or battery.
+	 */
+	for (i = 0; i < CONFIG_FANS; i++)
+		fan_set_percent_needed(i, fmax);
 #endif
 }
 
diff --git a/include/fan.h b/include/fan.h
index 4c41f2c175..0813535592 100644
--- a/include/fan.h
+++ b/include/fan.h
@@ -32,19 +32,21 @@ extern const struct fan_t fans[];
  * Set the amount of active cooling needed. The thermal control task will call
  * this frequently, and the fan control logic will attempt to provide it.
  *
+ * @param fan   Fan number (index into fans[])
  * @param pct   Percentage of cooling effort needed (0 - 100)
  */
-void fan_set_percent_needed(int pct);		/* HEY: need fan arg */
+void fan_set_percent_needed(int fan, int pct);
 
 /**
  * This function translates the percentage of cooling needed into a target RPM.
  * The default implementation should be sufficient for most needs, but
  * individual boards may provide a custom version if needed (see config.h).
  *
+ * @param fan   Fan number (index into fans[])
  * @param pct   Percentage of cooling effort needed (always in [0,100])
  * Return       Target RPM for fan
  */
-int fan_percent_to_rpm(int pct);		/* HEY: need fan arg */
+int fan_percent_to_rpm(int fan, int pct);
 
 
 /**
diff --git a/test/thermal.c b/test/thermal.c
index deaac2f631..10662ffd7d 100644
--- a/test/thermal.c
+++ b/test/thermal.c
@@ -62,7 +62,7 @@ void host_throttle_cpu(int throttled)
 	host_throttled = throttled;
 }
 
-void fan_set_percent_needed(int pct)
+void fan_set_percent_needed(int fan, int pct)
 {
 	fan_pct = pct;
 }
diff --git a/test/thermal_falco.c b/test/thermal_falco.c
index 1452a12692..885dcfcdfb 100644
--- a/test/thermal_falco.c
+++ b/test/thermal_falco.c
@@ -81,7 +81,7 @@ void host_throttle_cpu(int throttled)
 	host_throttled = throttled;
 }
 
-void fan_set_percent_needed(int pct)
+void fan_set_percent_needed(int fan, int pct)
 {
 	fan_pct = pct;
 }