From 62244e609b20350a237f97d37546c51869a54a71 Mon Sep 17 00:00:00 2001 From: Daisuke Nojiri Date: Sat, 6 Mar 2021 10:09:24 -0800 Subject: PCHG: Handle reset event Currently, PCHG assumes PCHG chips are reset only on POR and ignores reset events. This can cause the state machine to be in an unexpected state when a reset happens asynchronously. This patch allows PCHG to handle chip reset events. It also makes the task explicitly reset PCHG chips at start-up so that everything will start in known & clean states. BUG=b:181745891,b:181036152,b:173235954,b:183151376 BRANCH=trogdor TEST=Verify PCHG behaves expectedly across cold reset, warm reset, suspend & resume. Repeat the test with and without stylus. Signed-off-by: Daisuke Nojiri Change-Id: Ia3dd1fe7ebc8dd6f4ee8149a4c25918922143fc3 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2741282 Reviewed-by: Vincent Palatin (cherry picked from commit 2c703a08e866e3c3efedd8bb05d3de2323fa201c) Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2787470 --- common/peripheral_charger.c | 48 +++++++++++++++++++++++++++++++++++--------- driver/nfc/ctn730.c | 11 +++++++++- include/peripheral_charger.h | 3 +++ 3 files changed, 52 insertions(+), 10 deletions(-) diff --git a/common/peripheral_charger.c b/common/peripheral_charger.c index cb9a6e1f3e..05dde52a40 100644 --- a/common/peripheral_charger.c +++ b/common/peripheral_charger.c @@ -49,6 +49,7 @@ static const char *_text_event(enum pchg_event event) static const char * const event_names[] = { [PCHG_EVENT_NONE] = "NONE", [PCHG_EVENT_IRQ] = "IRQ", + [PCHG_EVENT_RESET] = "RESET", [PCHG_EVENT_INITIALIZED] = "INITIALIZED", [PCHG_EVENT_ENABLED] = "ENABLED", [PCHG_EVENT_DISABLED] = "DISABLED", @@ -70,6 +71,18 @@ static const char *_text_event(enum pchg_event event) return event_names[event]; } +static enum pchg_state pchg_reset(struct pchg *ctx) +{ + mutex_lock(&ctx->mtx); + queue_init(&ctx->events); + mutex_unlock(&ctx->mtx); + atomic_clear(&ctx->irq); + + /* When fw update is implemented, this will be the branch point. */ + pchg_queue_event(ctx, PCHG_EVENT_INITIALIZE); + return PCHG_STATE_RESET; +} + static enum pchg_state pchg_initialize(struct pchg *ctx, enum pchg_state state) { int rv = ctx->cfg->drv->init(ctx); @@ -94,6 +107,9 @@ static enum pchg_state pchg_state_reset(struct pchg *ctx) enum pchg_state state = PCHG_STATE_RESET; switch (ctx->event) { + case PCHG_EVENT_RESET: + state = pchg_reset(ctx); + break; case PCHG_EVENT_INITIALIZE: state = pchg_initialize(ctx, state); break; @@ -121,6 +137,9 @@ static enum pchg_state pchg_state_initialized(struct pchg *ctx) return state; switch (ctx->event) { + case PCHG_EVENT_RESET: + state = pchg_reset(ctx); + break; case PCHG_EVENT_INITIALIZE: state = pchg_initialize(ctx, state); break; @@ -147,6 +166,9 @@ static enum pchg_state pchg_state_enabled(struct pchg *ctx) int rv; switch (ctx->event) { + case PCHG_EVENT_RESET: + state = pchg_reset(ctx); + break; case PCHG_EVENT_INITIALIZE: state = pchg_initialize(ctx, state); break; @@ -185,6 +207,9 @@ static enum pchg_state pchg_state_detected(struct pchg *ctx) int rv; switch (ctx->event) { + case PCHG_EVENT_RESET: + state = pchg_reset(ctx); + break; case PCHG_EVENT_INITIALIZE: state = pchg_initialize(ctx, state); break; @@ -222,6 +247,9 @@ static enum pchg_state pchg_state_charging(struct pchg *ctx) int rv; switch (ctx->event) { + case PCHG_EVENT_RESET: + pchg_reset(ctx); + break; case PCHG_EVENT_INITIALIZE: state = pchg_initialize(ctx, state); break; @@ -346,7 +374,7 @@ static void pchg_startup(void) for (p = 0; p < pchg_count; p++) { ctx = &pchgs[p]; - pchg_queue_event(ctx, PCHG_EVENT_INITIALIZE); + ctx->cfg->drv->reset(ctx); gpio_enable_interrupt(ctx->cfg->irq_pin); } @@ -376,8 +404,8 @@ void pchg_task(void *u) struct pchg *ctx; int p; - /* In case we arrive here after power-on (for late sysjump) */ if (chipset_in_state(CHIPSET_STATE_ON)) + /* We are here after power-on (because of late sysjump). */ pchg_startup(); while (true) { @@ -450,7 +478,7 @@ static int cc_pchg(int argc, char **argv) port = strtoi(argv[1], &end, 0); if (*end || port < 0 || port >= pchg_count) - return EC_ERROR_PARAM2; + return EC_ERROR_PARAM1; ctx = &pchgs[port]; if (argc == 2) { @@ -460,15 +488,16 @@ static int cc_pchg(int argc, char **argv) return EC_SUCCESS; } - if (!strcasecmp(argv[2], "init")) { + if (!strcasecmp(argv[2], "reset")) + pchg_queue_event(ctx, PCHG_EVENT_RESET); + else if (!strcasecmp(argv[2], "init")) pchg_queue_event(ctx, PCHG_EVENT_INITIALIZE); - } else if (!strcasecmp(argv[2], "enable")) { + else if (!strcasecmp(argv[2], "enable")) pchg_queue_event(ctx, PCHG_EVENT_ENABLE); - } else if (!strcasecmp(argv[2], "disable")) { + else if (!strcasecmp(argv[2], "disable")) pchg_queue_event(ctx, PCHG_EVENT_DISABLE); - } else { - return EC_ERROR_PARAM1; - } + else + return EC_ERROR_PARAM2; task_wake(TASK_ID_PCHG); @@ -477,6 +506,7 @@ static int cc_pchg(int argc, char **argv) DECLARE_CONSOLE_COMMAND(pchg, cc_pchg, " [init/enable/disable]" "\n\t" + "\n\t reset" "\n\t init" "\n\t enable" "\n\t disable", diff --git a/driver/nfc/ctn730.c b/driver/nfc/ctn730.c index 8e5dd2ea20..75487fa701 100644 --- a/driver/nfc/ctn730.c +++ b/driver/nfc/ctn730.c @@ -252,6 +252,14 @@ static int _send_command(struct pchg *ctx, const struct ctn730_msg *cmd) return rv; } +static int ctn730_reset(struct pchg *ctx) +{ + gpio_set_level(GPIO_WLC_NRST_CONN, 0); + msleep(1); + gpio_set_level(GPIO_WLC_NRST_CONN, 1); + return EC_SUCCESS_IN_PROGRESS; +} + static int ctn730_init(struct pchg *ctx) { uint8_t buf[CTN730_MESSAGE_BUFFER_SIZE]; @@ -376,7 +384,7 @@ static int _process_payload_event(struct pchg *ctx, struct ctn730_msg *res) */ msleep(5); } else if (buf[0] == WLC_HOST_CTRL_RESET_EVT_DOWNLOAD_MODE) { - ctx->event = PCHG_EVENT_NONE; + ctx->event = PCHG_EVENT_RESET; } else { return EC_ERROR_INVAL; } @@ -539,6 +547,7 @@ exit: } const struct pchg_drv ctn730_drv = { + .reset = ctn730_reset, .init = ctn730_init, .enable = ctn730_enable, .get_event = ctn730_get_event, diff --git a/include/peripheral_charger.h b/include/peripheral_charger.h index 62b1b0ac01..26b7b98a2e 100644 --- a/include/peripheral_charger.h +++ b/include/peripheral_charger.h @@ -73,6 +73,7 @@ enum pchg_event { PCHG_EVENT_IRQ, /* External Events */ + PCHG_EVENT_RESET, PCHG_EVENT_INITIALIZED, PCHG_EVENT_ENABLED, PCHG_EVENT_DISABLED, @@ -140,6 +141,8 @@ struct pchg { * Peripheral charger driver */ struct pchg_drv { + /* Reset charger chip. */ + int (*reset)(struct pchg *ctx); /* Initialize the charger. */ int (*init)(struct pchg *ctx); /* Enable/disable the charger. */ -- cgit v1.2.1