From f883354bbad138390142fdeaa554f55ee51551ef Mon Sep 17 00:00:00 2001 From: Bill Richardson Date: Tue, 19 Aug 2014 19:01:29 -0700 Subject: lightbar: correctly revert to the S0/S3/S5 patterns This CL ensures that temporary "one-shot" sequences such as KONAMI, TEST, TAP, etc. will revert to the previous "normal" sequences even when interrupted by other one-shot sequences. This also adds a test for those cases. BUG=chrome-os-partner:29873 BRANCH=ToT TEST=manual make runtests Change-Id: Ie83908731acdf2f7c9108568a1ba047943175d26 Signed-off-by: Bill Richardson Reviewed-on: https://chromium-review.googlesource.com/213230 --- common/lb_common.c | 10 ++++++++ common/lightbar.c | 70 +++++++++++++++++++++++++++++++++++------------------- 2 files changed, 55 insertions(+), 25 deletions(-) (limited to 'common') diff --git a/common/lb_common.c b/common/lb_common.c index 289421d3bd..8803608328 100644 --- a/common/lb_common.c +++ b/common/lb_common.c @@ -68,6 +68,12 @@ static inline uint8_t controller_read(int ctrl_num, uint8_t reg) #define MAX_GREEN 0x55 #define MAX_BLUE 0x67 #endif +#ifdef BOARD_HOST +/* For testing only */ +#define MAX_RED 0xff +#define MAX_GREEN 0xff +#define MAX_BLUE 0xff +#endif /* How we'd like to see the driver chips initialized. The controllers have some * auto-cycling capability, but it's not much use for our purposes. For now, @@ -114,6 +120,10 @@ static const uint8_t led_to_isc[] = { 0x18, 0x15, 0x18, 0x15 }; #ifdef BOARD_SAMUS static const uint8_t led_to_isc[] = { 0x15, 0x18, 0x15, 0x18 }; #endif +#ifdef BOARD_HOST +/* For testing only */ +static const uint8_t led_to_isc[] = { 0x15, 0x18, 0x15, 0x18 }; +#endif /* Scale 0-255 into max value */ static inline uint8_t scale_abs(int val, int max) diff --git a/common/lightbar.c b/common/lightbar.c index d08f12cf0b..e817b38da5 100644 --- a/common/lightbar.c +++ b/common/lightbar.c @@ -563,7 +563,6 @@ static uint32_t sequence_S5S3(void) static uint32_t sequence_S3S5(void) { lb_off(); - WAIT_OR_RET(-1); return 0; } @@ -571,6 +570,7 @@ static uint32_t sequence_S3S5(void) * nothing to do. We'll just wait here until the state changes. */ static uint32_t sequence_S5(void) { + lb_off(); WAIT_OR_RET(-1); return 0; } @@ -644,9 +644,10 @@ static uint32_t sequence_PULSE(void) return TASK_EVENT_CUSTOM(msg); } -/* The host CPU (or someone) is going to poke at the lightbar directly, so we - * don't want the EC messing with it. We'll just sit here and ignore all - * other messages until we're told to continue. */ +/* The AP is going to poke at the lightbar directly, so we don't want the EC + * messing with it. We'll just sit here and ignore all other messages until + * we're told to continue (or until we think the AP is shutting down). + */ static uint32_t sequence_STOP(void) { uint32_t msg; @@ -654,10 +655,12 @@ static uint32_t sequence_STOP(void) do { msg = TASK_EVENT_CUSTOM(task_wait_event(-1)); CPRINTS("LB_stop got pending_msg %d", pending_msg); - } while (msg != PENDING_MSG || pending_msg != LIGHTBAR_RUN); - - /* Q: What should we do if the host shuts down? */ - /* A: Nothing. We could be driving from the EC console. */ + } while (msg != PENDING_MSG || ( + pending_msg != LIGHTBAR_RUN && + pending_msg != LIGHTBAR_S0S3 && + pending_msg != LIGHTBAR_S3 && + pending_msg != LIGHTBAR_S3S5 && + pending_msg != LIGHTBAR_S5)); CPRINTS("LB_stop->running"); return 0; @@ -669,7 +672,10 @@ static uint32_t sequence_RUN(void) return 0; } -/* We shouldn't come here, but if we do it shouldn't hurt anything */ +/* We shouldn't come here, but if we do it shouldn't hurt anything. This + * sequence is to indicate an internal error in the lightbar logic, not an + * error with the Chromebook itself. + */ static uint32_t sequence_ERROR(void) { lb_init(); @@ -759,25 +765,33 @@ static const struct { {4, 0x00, 0x00, 0x00, 100000}, }; -static uint32_t sequence_KONAMI(void) +static uint32_t sequence_KONAMI_inner(void) { int i; - int tmp; - - tmp = lb_get_brightness(); - lb_set_brightness(255); for (i = 0; i < ARRAY_SIZE(konami); i++) { lb_set_rgb(konami[i].led, konami[i].r, konami[i].g, konami[i].b); if (konami[i].delay) - usleep(konami[i].delay); + WAIT_OR_RET(konami[i].delay); } - lb_set_brightness(tmp); return 0; } +static uint32_t sequence_KONAMI(void) +{ + int tmp; + uint32_t r; + + /* Force brightness to max, then restore it */ + tmp = lb_get_brightness(); + lb_set_brightness(255); + r = sequence_KONAMI_inner(); + lb_set_brightness(tmp); + return r; +} + /* Returns 0.0 to 1.0 for val in [min, min + ofs] */ static float range(int val, int min, int ofs) { @@ -846,14 +860,10 @@ static uint32_t sequence_TAP_inner(void) mult * st.p.color[ci].g, mult * st.p.color[ci].b); } - /* - * TODO: Use a different delay function here. Otherwise, - * it's possible that a new sequence (such as KONAMI) can end - * up with TAP as it's previous sequence. It's okay to return - * early from TAP (or not), but we don't want to end up stuck - * in the TAP sequence. - */ + WAIT_OR_RET(st.p.tap_tick_delay); + + /* Return after some time has elapsed */ now = get_time(); if (now.le.lo - start.le.lo > st.p.tap_display_time) break; @@ -867,7 +877,10 @@ static uint32_t sequence_TAP(void) uint32_t r; uint8_t br, save[NUM_LEDS][3]; - /* TODO(crosbug.com/p/29041): do we need more than lb_init() */ + /* TODO(crosbug.com/p/29041): Do we need more than lb_init()? + * Yes. And then we may need to turn it off again, if the AP is still + * off when we're done. + */ lb_init(); lb_on(); @@ -889,6 +902,12 @@ static uint32_t sequence_TAP(void) /* The main lightbar task. It just cycles between various pretty patterns. */ /****************************************************************************/ +/* Distinguish "normal" sequences from one-shot sequences */ +static inline int is_normal_sequence(enum lightbar_sequence seq) +{ + return (seq >= LIGHTBAR_S5 && seq <= LIGHTBAR_S3S5); +} + /* Link each sequence with a command to invoke it. */ struct lightbar_cmd_t { const char * const string; @@ -918,7 +937,8 @@ void lightbar_task(void) CPRINTS("LB msg %d = %s", pending_msg, lightbar_cmds[pending_msg].string); if (st.cur_seq != pending_msg) { - st.prev_seq = st.cur_seq; + if (is_normal_sequence(st.cur_seq)) + st.prev_seq = st.cur_seq; st.cur_seq = pending_msg; } } else { -- cgit v1.2.1