pwm: Increse PWM duty resolution to 16bits

The current PWM interface allows for a pwm duty setting ranging from
0 to 100, resulting in a very coarse adjustment. To alleviate the
problem, the interface now allows for a pwm duty setting in the range
of 0 to 65535.

BUG=chromium:615109
BRANCH=None
TEST=Manual on chell.
`ectool pwmsetduty 1 65535` - Verify LCD backlight goes to 100%
`ectool pwmgetduty 1` - Prints 65535
`ectool pwmsetduty 1 0` - Verify LCD backlight goes to 0%
`ectool pwmgetduty 1` - Prints 0

terminal pwmduty tests:
>pwmduty
PWM channels:
  0: 100%
  1: 62%
  2: 100%
  3: 80%
> pwmduty 1 50
Setting channel 1 to 50%
  1: 50%
> pwmduty 1 0
Setting channel 1 to 0%
  1: disabled
> pwmduty 1 100
Setting channel 1 to 100%
  1: 100%
> pwmduty 1 raw 0
Setting channel 1 to raw 0%
  1: disabled
> pwmduty 1 raw 65535
Setting channel 1 to raw 65535%
  1: 65535
> pwmduty
PWM channels:
  0: 100%
  1: 100%
  2: 100%
  3: 80%
> pwmduty 1 raw 30000
Setting channel 1 to raw 30000%
  1: 30000
> pwmduty
PWM channels:
  0: 100%
  1: 46%
  2: 100%
  3: 80%

Change-Id: I299b77585f3988e72d9ac918bdde7dc5fa3df6de
Reviewed-on: https://chromium-review.googlesource.com/374481
Commit-Ready: Shawn N <shawnn@chromium.org>
Tested-by: Sam Hurst <shurst@google.com>
Reviewed-by: Shawn N <shawnn@chromium.org>

4 files changed, 105 insertions, 74 deletions

diff --git a/board/kevin/board.c b/board/kevin/board.c
index 6293bcc980..44db50ac9e 100644
--- a/board/kevin/board.c
+++ b/board/kevin/board.c
@@ -694,13 +694,13 @@ static void pwm_displight_restore_state(void)
 	prev = (const int *)system_get_jump_tag(PWM_DISPLIGHT_SYSJUMP_TAG,
 						&version, &size);
 	if (prev && version == PWM_HOOK_VERSION && size == sizeof(*prev))
-		pwm_set_duty(PWM_CH_DISPLIGHT, *prev);
+		pwm_set_raw_duty(PWM_CH_DISPLIGHT, *prev);
 }
 DECLARE_HOOK(HOOK_INIT, pwm_displight_restore_state, HOOK_PRIO_INIT_PWM + 1);
 
 static void pwm_displight_preserve_state(void)
 {
-	int pwm_displight_duty = pwm_get_duty(PWM_CH_DISPLIGHT);
+	int pwm_displight_duty = pwm_get_raw_duty(PWM_CH_DISPLIGHT);
 
 	system_add_jump_tag(PWM_DISPLIGHT_SYSJUMP_TAG, PWM_HOOK_VERSION,
 			    sizeof(pwm_displight_duty), &pwm_displight_duty);
diff --git a/chip/npcx/pwm.c b/chip/npcx/pwm.c
index 0f30e3cad4..9d16687ddf 100644
--- a/chip/npcx/pwm.c
+++ b/chip/npcx/pwm.c
@@ -8,8 +8,11 @@
  * On this chip, the PWM logic is implemented by the hardware FAN modules.
  */
 
+#include "assert.h"
 #include "clock.h"
 #include "clock_chip.h"
+#include "console.h"
+#include "ec_commands.h"
 #include "fan.h"
 #include "gpio.h"
 #include "hooks.h"
@@ -17,7 +20,6 @@
 #include "pwm_chip.h"
 #include "registers.h"
 #include "util.h"
-#include "console.h"
 
 #if !(DEBUG_PWM)
 #define CPRINTS(...)
@@ -43,23 +45,21 @@ enum npcx_pwm_heartbeat_mode {
 	NPCX_PWM_HBM_UNDEF     = 0xFF
 };
 
-/* Default duty cycle resolution */
-#define DUTY_CYCLE_RESOLUTION 100
-
 /**
  * Set PWM operation clock.
  *
  * @param   ch      operation channel
  * @param   freq    desired PWM frequency
- * @param   res     resolution for duty cycle
  * @notes   changed when initialization
  */
-void pwm_set_freq(enum pwm_channel ch, uint32_t freq, uint32_t res)
+static void pwm_set_freq(enum pwm_channel ch, uint32_t freq)
 {
 	int mdl = pwm_channels[ch].channel;
-	uint32_t prescaler_divider = 0;
+	uint32_t prescaler_divider;
 	uint32_t clock;
 
+	assert(freq != 0);
+
 	/* Disable PWM for module configuration */
 	pwm_enable(ch, 0);
 
@@ -74,25 +74,20 @@ void pwm_set_freq(enum pwm_channel ch, uint32_t freq, uint32_t res)
 		clock = clock_get_apb2_freq();
 
 	/*
-	 * Using PWM Frequency and Resolution we calculate
-	 * prescaler for input clock
+	 * Based on freq = clock / ((ctr + 1) * (prsc + 1))
+	 *   where:  prsc = prescaler_divider
+	 *           ctr  = MAX_DUTY_CYCLE
 	 */
-	prescaler_divider = ((clock / freq)/res);
-
-	/* Set clock prescaler divider to PWM module*/
-	if (prescaler_divider >= 1)
-		prescaler_divider = prescaler_divider - 1;
-	if (prescaler_divider > 0xFFFF)
-		prescaler_divider = 0xFFFF;
+	prescaler_divider = (clock / ((EC_PWM_MAX_DUTY + 1) * freq)) - 1;
 
 	/* Configure computed prescaler and resolution */
 	NPCX_PRSC(mdl) = (uint16_t)prescaler_divider;
 
 	/* Set PWM cycle time */
-	NPCX_CTR(mdl) = res - 1;
+	NPCX_CTR(mdl) = EC_PWM_MAX_DUTY - 1;
 
 	/* Set the duty cycle to 0% since DCR > CTR */
-	NPCX_DCR(mdl) = res;
+	NPCX_DCR(mdl) = EC_PWM_MAX_DUTY;
 }
 
 /**
@@ -131,40 +126,34 @@ int pwm_get_enabled(enum pwm_channel ch)
  */
 void pwm_set_duty(enum pwm_channel ch, int percent)
 {
+	/* Convert 16 bit duty to percent on [0, 100] */
+	pwm_set_raw_duty(ch, (percent * EC_PWM_MAX_DUTY) / 100);
+}
+
+/**
+ * Set PWM duty cycle.
+ *
+ * @param   ch      operation channel
+ * @param   duty    cycle duty
+ * @return  none
+ */
+void pwm_set_raw_duty(enum pwm_channel ch, uint16_t duty)
+{
 	int mdl = pwm_channels[ch].channel;
-	uint32_t dc_res = 0;
-	uint16_t dc_cnt = 0;
 
-	/* Checking duty value first */
-	if (percent < 0)
-		percent = 0;
-	else if (percent > 100)
-		percent = 100;
-	CPRINTS("pwm%d, set duty=%d", mdl, percent);
+	CPRINTS("pwm%d, set duty=%d", mdl, duty);
 
 	/* Assume the fan control is active high and invert it ourselves */
 	UPDATE_BIT(NPCX_PWMCTL(mdl), NPCX_PWMCTL_INVP,
 			(pwm_channels[ch].flags & PWM_CONFIG_ACTIVE_LOW));
 
-	dc_res = NPCX_CTR(mdl) + 1;
-	dc_cnt = (percent*dc_res)/100;
 	CPRINTS("freq=0x%x", pwm_channels[ch].freq);
-	CPRINTS("duty_cycle_res=%d", dc_res);
-	CPRINTS("duty_cycle_cnt=%d", dc_cnt);
-
+	CPRINTS("duty_cycle_cnt=%d", duty);
 
 	/* Set the duty cycle */
-	if (percent > 0) {
-		if (percent == 100)
-			NPCX_DCR(mdl) = NPCX_CTR(mdl);
-		else
-			NPCX_DCR(mdl) = (dc_cnt - 1);
-		pwm_enable(ch, 1);
-	} else {
-		/* Output low since DCR > CTR */
-		NPCX_DCR(mdl) = NPCX_CTR(mdl) + 1;
-		pwm_enable(ch, 0);
-	}
+	NPCX_DCR(mdl) = (uint16_t)duty;
+
+	pwm_enable(ch, !!duty);
 }
 
 /**
@@ -175,12 +164,24 @@ void pwm_set_duty(enum pwm_channel ch, int percent)
  */
 int pwm_get_duty(enum pwm_channel ch)
 {
+	return DIV_ROUND_NEAREST(pwm_get_raw_duty(ch) * 100, EC_PWM_MAX_DUTY);
+}
+
+/**
+ * Get PWM duty cycle.
+ *
+ * @param   ch  operation channel
+ * @return  duty cycle
+ */
+uint16_t pwm_get_raw_duty(enum pwm_channel ch)
+{
 	int mdl = pwm_channels[ch].channel;
-	/* Return percent */
-	if ((!pwm_get_enabled(ch)) || (NPCX_DCR(mdl) > NPCX_CTR(mdl)))
+
+	/* Return duty */
+	if (!pwm_get_enabled(ch))
 		return 0;
 	else
-		return ((NPCX_DCR(mdl) + 1) * 100) / (NPCX_CTR(mdl) + 1);
+		return NPCX_DCR(mdl);
 }
 
 /**
@@ -212,8 +213,7 @@ void pwm_config(enum pwm_channel ch)
 			(pwm_channels[ch].flags & PWM_CONFIG_DSLEEP));
 
 	/* Set PWM operation frequency */
-	pwm_set_freq(ch, pwm_channels[ch].freq, DUTY_CYCLE_RESOLUTION);
-
+	pwm_set_freq(ch, pwm_channels[ch].freq);
 }
 
 /**
diff --git a/common/pwm.c b/common/pwm.c
index c7fb627ec7..761d1dad25 100644
--- a/common/pwm.c
+++ b/common/pwm.c
@@ -40,25 +40,30 @@ static int get_target_channel(enum pwm_channel *channel, int type, int index)
 	return *channel >= PWM_CH_COUNT;
 }
 
-/*
- * TODO(crbug.com/615109): These host commands use 16 bit duty cycle, but
- * all of our internal code uses percent on [0, 100]. Convert internal
- * functions to use 16 bit duty and remove the conversions below.
- */
-static int host_command_pwm_set_duty(struct host_cmd_handler_args *args)
+__attribute__((weak)) void pwm_set_raw_duty(enum pwm_channel ch, uint16_t duty)
 {
-	const struct ec_params_pwm_set_duty *p = args->params;
-	enum pwm_channel channel;
 	int percent;
 
 	/* Convert 16 bit duty to percent on [0, 100] */
-	percent = DIV_ROUND_NEAREST(p->duty * 100, EC_PWM_MAX_DUTY);
+	percent = DIV_ROUND_NEAREST((uint32_t)duty * 100, 65535);
+	pwm_set_duty(ch, percent);
+}
+
+__attribute__((weak)) uint16_t pwm_get_raw_duty(enum pwm_channel ch)
+{
+	return (pwm_get_duty(ch) * 65535) / 100;
+}
+
+static int host_command_pwm_set_duty(struct host_cmd_handler_args *args)
+{
+	const struct ec_params_pwm_set_duty *p = args->params;
+	enum pwm_channel channel;
 
 	if (get_target_channel(&channel, p->pwm_type, p->index))
 		return EC_RES_INVALID_PARAM;
 
-	pwm_set_duty(channel, percent);
-	pwm_enable(channel, percent > 0);
+	pwm_set_raw_duty(channel, p->duty);
+	pwm_enable(channel, p->duty > 0);
 
 	return EC_RES_SUCCESS;
 }
@@ -76,8 +81,7 @@ static int host_command_pwm_get_duty(struct host_cmd_handler_args *args)
 	if (get_target_channel(&channel, p->pwm_type, p->index))
 		return EC_RES_INVALID_PARAM;
 
-	/* Convert percent on [0, 100] to 16 bit duty */
-	r->duty = pwm_get_duty(channel) * EC_PWM_MAX_DUTY / 100;
+	r->duty = pwm_get_raw_duty(channel);
 	args->response_size = sizeof(*r);
 
 	return EC_RES_SUCCESS;
@@ -91,24 +95,29 @@ DECLARE_HOST_COMMAND(EC_CMD_PWM_GET_DUTY,
  *
  * @param ch		Channel to print.
  */
-static void print_channel(enum pwm_channel ch)
+static void print_channel(enum pwm_channel ch, int max_duty)
 {
 	if (pwm_get_enabled(ch))
-		ccprintf("  %d: %d%%\n", ch, pwm_get_duty(ch));
+		if (max_duty == 100)
+			ccprintf("  %d: %d%%\n", ch, pwm_get_duty(ch));
+		else
+			ccprintf("  %d: %d\n", ch, pwm_get_raw_duty(ch));
 	else
 		ccprintf("  %d: disabled\n", ch);
 }
 
 static int cc_pwm_duty(int argc, char **argv)
 {
-	int percent = 0;
+	int value = 0;
+	int max_duty = 100;
 	int ch;
 	char *e;
+	char *raw;
 
 	if (argc < 2) {
 		ccprintf("PWM channels:\n");
 		for (ch = 0; ch < PWM_CH_COUNT; ch++)
-			print_channel(ch);
+			print_channel(ch, max_duty);
 		return EC_SUCCESS;
 	}
 
@@ -117,26 +126,38 @@ static int cc_pwm_duty(int argc, char **argv)
 		return EC_ERROR_PARAM1;
 
 	if (argc > 2) {
-		percent = strtoi(argv[2], &e, 0);
-		if (*e || percent > 100) {
+		raw = argv[2];
+		if (!strcasecmp(raw, "raw")) {
+			/* use raw duty */
+			value = strtoi(argv[3], &e, 0);
+			max_duty = EC_PWM_MAX_DUTY;
+		} else {
+			/* use percent duty */
+			value = strtoi(argv[2], &e, 0);
+			max_duty = 100;
+		}
+
+		if (*e || value > max_duty) {
 			/* Bad param */
-			return EC_ERROR_PARAM1;
-		} else if (percent < 0) {
+			return EC_ERROR_PARAM2;
+		} else if (value < 0) {
 			/* Negative = disable */
 			pwm_enable(ch, 0);
 		} else {
-			ccprintf("Setting channel %d to %d%%\n", ch, percent);
+			ccprintf("Setting channel %d to%s%d%%\n",
+				ch, (max_duty == 100) ? " " : " raw ", value);
 			pwm_enable(ch, 1);
-			pwm_set_duty(ch, percent);
+			(max_duty == 100) ? pwm_set_duty(ch, value) :
+				pwm_set_raw_duty(ch, value);
 		}
 	}
 
-	print_channel(ch);
+	print_channel(ch, max_duty);
 
 	return EC_SUCCESS;
 }
 DECLARE_CONSOLE_COMMAND(pwmduty, cc_pwm_duty,
-			"[channel [<percent> | -1=disable]]",
+			"[channel [<percent> | -1=disable] | [raw <value>]]",
 			"Get/set PWM duty cycles ");
 #endif /* CONFIG_PWM */
 
diff --git a/include/pwm.h b/include/pwm.h
index 08cea7e43a..9c349d1fb6 100644
--- a/include/pwm.h
+++ b/include/pwm.h
@@ -20,6 +20,16 @@ void pwm_enable(enum pwm_channel ch, int enabled);
 int pwm_get_enabled(enum pwm_channel ch);
 
 /**
+ * Set PWM channel duty cycle (0-65535).
+ */
+void pwm_set_raw_duty(enum pwm_channel ch, uint16_t duty);
+
+/**
+ * Get PWM channel duty cycle.
+ */
+uint16_t pwm_get_raw_duty(enum pwm_channel ch);
+
+/**
  * Set PWM channel duty cycle (0-100).
  */
 void pwm_set_duty(enum pwm_channel ch, int percent);