RGBKBD: Move initialization to keyboard backlight

This patch removes initialization code for RGB keyboard and let keyboard
backlight module handle it. This patch also removes RGB keyboard
integration from pwm.c since it's not used or needed.

BUG=b:226215987, b:228525798, 223465912
BRANCH=None
TEST=ectool rgbkbd, pwmsetkblight, pwmgetkblight on Vell & Taniks

Signed-off-by: Daisuke Nojiri <dnojiri@chromium.org>
Change-Id: I01fa445f802607106f2f4a0a8e1469ab759c604b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/3615851
Reviewed-by: Parth Malkan <parthmalkan@google.com>

5 files changed, 35 insertions, 91 deletions

diff --git a/common/keyboard_backlight.c b/common/keyboard_backlight.c
index 4940d9a97e..b1b3671d3d 100644
--- a/common/keyboard_backlight.c
+++ b/common/keyboard_backlight.c
@@ -19,6 +19,7 @@
 
 static struct kblight_conf kblight;
 static int current_percent;
+static uint8_t current_enable;
 
 void __attribute__((weak)) board_kblight_init(void)
 { }
@@ -56,14 +57,23 @@ int kblight_get(void)
 	return current_percent;
 }
 
-int kblight_enable(int enable)
+static void kblight_enable_deferred(void)
 {
 #ifdef CONFIG_KBLIGHT_ENABLE_PIN
-	gpio_set_level(GPIO_EN_KEYBOARD_BACKLIGHT, enable);
+	gpio_set_level(GPIO_EN_KEYBOARD_BACKLIGHT, current_enable);
 #endif
 	if (!kblight.drv || !kblight.drv->enable)
-		return -1;
-	return kblight.drv->enable(enable);
+		return;
+	kblight.drv->enable(current_enable);
+}
+DECLARE_DEFERRED(kblight_enable_deferred);
+
+int kblight_enable(int enable)
+{
+	current_enable = enable;
+	/* Need to defer i2c in case it's called from an interrupt handler. */
+	hook_call_deferred(&kblight_enable_deferred_data, 0);
+	return EC_SUCCESS;
 }
 
 int kblight_get_enabled(void)
@@ -104,6 +114,7 @@ static void keyboard_backlight_init(void)
 }
 DECLARE_HOOK(HOOK_INIT, keyboard_backlight_init, HOOK_PRIO_DEFAULT);
 
+#ifdef HAS_TASK_CHIPSET
 static void kblight_suspend(void)
 {
 	kblight_enable(0);
@@ -118,12 +129,15 @@ static void kblight_resume(void)
 	}
 }
 DECLARE_HOOK(HOOK_CHIPSET_RESUME, kblight_resume, HOOK_PRIO_DEFAULT);
+#endif  // HAS_TASK_CHIPSET
 
+#ifdef CONFIG_LID_SWITCH
 static void kblight_lid_change(void)
 {
 	kblight_enable(lid_is_open() && current_percent);
 }
 DECLARE_HOOK(HOOK_LID_CHANGE, kblight_lid_change, HOOK_PRIO_DEFAULT);
+#endif
 
 /*
  * Console and host commands
diff --git a/common/pwm.c b/common/pwm.c
index 7699405f3d..a4edfdd5a5 100644
--- a/common/pwm.c
+++ b/common/pwm.c
@@ -9,7 +9,6 @@
 #include "hooks.h"
 #include "host_command.h"
 #include "pwm.h"
-#include "rgb_keyboard.h"
 #include "util.h"
 
 #ifdef CONFIG_PWM
@@ -65,18 +64,10 @@ 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 (IS_ENABLED(CONFIG_RGB_KEYBOARD) &&
-			(p->pwm_type == EC_PWM_TYPE_KB_LIGHT)) {
-		uint8_t gcc = DIV_ROUND_NEAREST(p->duty * RGBKBD_MAX_GCC_LEVEL,
-						EC_PWM_MAX_DUTY);
-		if (rgbkbd_set_global_brightness(gcc))
-			return EC_RES_ERROR;
-	} else {
-		if (get_target_channel(&channel, p->pwm_type, p->index))
-			return EC_RES_INVALID_PARAM;
-		pwm_set_raw_duty(channel, p->duty);
-		pwm_enable(channel, p->duty > 0);
-	}
+	if (get_target_channel(&channel, p->pwm_type, p->index))
+		return EC_RES_INVALID_PARAM;
+	pwm_set_raw_duty(channel, p->duty);
+	pwm_enable(channel, p->duty > 0);
 
 	return EC_RES_SUCCESS;
 }
@@ -90,22 +81,12 @@ host_command_pwm_get_duty(struct host_cmd_handler_args *args)
 	const struct ec_params_pwm_get_duty *p = args->params;
 	struct ec_response_pwm_get_duty *r = args->response;
 
-	if (IS_ENABLED(CONFIG_RGB_KEYBOARD) &&
-			(p->pwm_type == EC_PWM_TYPE_KB_LIGHT)) {
-		uint8_t gcc;
-
-		if (rgbkbd_get_global_brightness(&gcc))
-			return EC_RES_ERROR;
-		r->duty = DIV_ROUND_NEAREST(gcc * EC_PWM_MAX_DUTY,
-					    RGBKBD_MAX_GCC_LEVEL);
-	} else {
-		enum pwm_channel channel;
+	enum pwm_channel channel;
 
-		if (get_target_channel(&channel, p->pwm_type, p->index))
-			return EC_RES_INVALID_PARAM;
+	if (get_target_channel(&channel, p->pwm_type, p->index))
+		return EC_RES_INVALID_PARAM;
 
-		r->duty = pwm_get_raw_duty(channel);
-	}
+	r->duty = pwm_get_raw_duty(channel);
 
 	args->response_size = sizeof(*r);
 
diff --git a/common/rgb_keyboard.c b/common/rgb_keyboard.c
index b5a8bfae40..d909d3ad09 100644
--- a/common/rgb_keyboard.c
+++ b/common/rgb_keyboard.c
@@ -234,7 +234,7 @@ void rgbkbd_init_lookup_table(void)
 	 */
 }
 
-int rgbkbd_set_global_brightness(uint8_t gcc)
+static int rgbkbd_set_global_brightness(uint8_t gcc)
 {
 	int e, grid;
 	int rv = EC_SUCCESS;
@@ -249,8 +249,6 @@ int rgbkbd_set_global_brightness(uint8_t gcc)
 			rv = e;
 			continue;
 		}
-
-		ctx->gcc = gcc;
 	}
 
 	CPRINTS("Set GCC to %u", gcc);
@@ -259,21 +257,14 @@ int rgbkbd_set_global_brightness(uint8_t gcc)
 	return rv;
 }
 
-int rgbkbd_get_global_brightness(uint8_t *gcc)
-{
-	*gcc = rgbkbds[0].gcc;
-
-	return EC_SUCCESS;
-}
-
-__overridable void board_enable_rgb_keyboard(bool enable) {}
-
 static int rgbkbd_init(void)
 {
 	int rv = EC_SUCCESS;
 	int e, i;
 	bool updated = false;
 
+	rgbkbd_init_lookup_table();
+
 	for (i = 0; i < rgbkbd_count; i++) {
 		struct rgbkbd *ctx = &rgbkbds[i];
 
@@ -342,16 +333,6 @@ static int rgbkbd_kblight_set(int percent)
 	return rgbkbd_set_global_brightness(gcc);
 }
 
-static int rgbkbd_kblight_get(void)
-{
-	uint8_t gcc;
-
-	if (rgbkbd_get_global_brightness(&gcc))
-		return 0;
-
-	return DIV_ROUND_NEAREST(gcc * 100, RGBKBD_MAX_GCC_LEVEL);
-}
-
 static int rgbkbd_get_enabled(void)
 {
 	return rgbkbds[0].state >= RGBKBD_STATE_ENABLED;
@@ -360,14 +341,8 @@ static int rgbkbd_get_enabled(void)
 const struct kblight_drv kblight_rgbkbd = {
 	.init = rgbkbd_init,
 	.set = rgbkbd_kblight_set,
-	.get = rgbkbd_kblight_get,
-	/*
-	 * We need to let RGBKBD manage enable/disable the backlight to keep
-	 * the LEDs under the control of RGBKBD. Registering NULL also avoids
-	 * ASSERT(!in_interrupt_context()) failure in task.c called from
-	 * rgbkbd_enable API.
-	 */
-	.enable = NULL,
+	.get = NULL,
+	.enable = rgbkbd_enable,
 	.get_enabled = rgbkbd_get_enabled,
 };
 
@@ -375,13 +350,6 @@ void rgbkbd_task(void *u)
 {
 	uint32_t event;
 
-	board_enable_rgb_keyboard(true);
-
-	rgbkbd_init_lookup_table();
-	rgbkbd_init();
-	rgbkbd_enable(1);
-	rgbkbd_set_global_brightness(0x80);
-
 	while (1) {
 		event = task_wait_event(100 * MSEC);
 		if (IS_ENABLED(CONFIG_RGB_KEYBOARD_DEBUG))
diff --git a/include/rgb_keyboard.h b/include/rgb_keyboard.h
index 2c6a550e7b..3593ab6f0e 100644
--- a/include/rgb_keyboard.h
+++ b/include/rgb_keyboard.h
@@ -39,8 +39,6 @@ struct rgbkbd {
 	const struct rgbkbd_cfg * const cfg;
 	/* Current state of the port */
 	enum rgbkbd_state state;
-	/* Global current control (a.k.a. backlight brightness) */
-	uint8_t gcc;
 	/* Buffer containing color info for each dot. */
 	struct rgb_s *buf;
 };
@@ -142,11 +140,6 @@ extern const uint8_t rgbkbd_hsize;
 extern const uint8_t rgbkbd_vsize;
 
 /*
- * Called to power on or off the RGB keyboard module.
- */
-__override_proto void board_enable_rgb_keyboard(bool enable);
-
-/*
  * rgbkbd_map describes a mapping from key IDs to LED IDs.
  *
  * Multiple keys can be mapped to one LED and one key can be mapped to multiple
@@ -178,15 +171,6 @@ __override_proto void board_enable_rgb_keyboard(bool enable);
 extern const uint8_t rgbkbd_map[];
 extern const size_t rgbkbd_map_size;
 
-/**
- * Set/get global brightness of the RGB keyboard.
- *
- * @param  gcc  Brightness level 0 ~ RGBKBD_MAX_GCC_LEVEL.
- * @return enum ec_error_list;
- */
-int rgbkbd_set_global_brightness(uint8_t gcc);
-int rgbkbd_get_global_brightness(uint8_t *gcc);
-
 /*
  * Driver for keyboard_backlight.
  */
diff --git a/test/rgb_keyboard.c b/test/rgb_keyboard.c
index 014f32b014..935c60165b 100644
--- a/test/rgb_keyboard.c
+++ b/test/rgb_keyboard.c
@@ -116,10 +116,14 @@ static int test_drv_set_gcc(struct rgbkbd *ctx, uint8_t level)
 	return EC_SUCCESS;
 }
 
+void rgbkbd_init_lookup_table(void);
+
 static int test_rgbkbd_map(void)
 {
 	union rgbkbd_coord_u8 led;
 
+	rgbkbd_init_lookup_table();
+
 	led.u8 = rgbkbd_map[rgbkbd_table[0]];
 	zassert_equal(RGBKBD_COORD(led.coord.x, led.coord.y),
 		      RGBKBD_DELM, "key[0] -> None");
@@ -164,13 +168,6 @@ static int test_rgbkbd_startup(void)
 	/* Let RGBKBD task run. */
 	task_wait_event(-1);
 
-	zassert_equal(mock_state.count_drv_init, rgbkbd_count,
-		      "init() called");
-	zassert_equal(mock_state.count_drv_set_gcc, rgbkbd_count,
-		      "set_gcc() called");
-	zassert_equal(mock_state.count_drv_set_scale, rgbkbd_count,
-		      "set_scale() called");
-
 	/* Check 'DOT' demo. */
 	for (x = 0; x < rgbkbd_hsize; x++) {
 		g = x / rgbkbds[0].cfg->col_len;