diff options
author | Hyungwoo Yang <hyungwoo.yang@intel.com> | 2019-05-29 22:57:58 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2019-06-06 17:52:48 +0000 |
commit | e12b71b1fe82af7bc804004147f33da5e29cced1 (patch) | |
tree | ede97ddb996cf2dcbb44e07dd0ee636355cc5384 | |
parent | 57361b1afa507c39d46cb4ae916fec38204ad9a0 (diff) | |
download | chrome-ec-e12b71b1fe82af7bc804004147f33da5e29cced1.tar.gz |
ish: fix to follow HECI spec
Some of current implementaion doesn't follow HECI spec.
This patch fixes those wrong implementation.
BRANCH=none
BUG=b:133300213
TEST=Tested on Arcada platform, HECI bus&clients are working fine.
Change-Id: I3a7e242d3414187342aca33aa1efc789e66af105
Signed-off-by: Hyungwoo Yang <hyungwoo.yang@intel.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1637140
Reviewed-by: Jack Rosenthal <jrosenth@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Commit-Queue: Jack Rosenthal <jrosenth@chromium.org>
-rw-r--r-- | chip/ish/heci.c | 110 |
1 files changed, 71 insertions, 39 deletions
diff --git a/chip/ish/heci.c b/chip/ish/heci.c index 4fe6fd6ada..146aa175f5 100644 --- a/chip/ish/heci.c +++ b/chip/ish/heci.c @@ -37,8 +37,15 @@ struct heci_msg { uint8_t payload[HECI_IPC_PAYLOAD_SIZE]; } __packed; -/* address for Host Bus */ -#define HECI_HBM_ADDRESS 0 +/* HECI addresses */ +#define HECI_HBM_ADDRESS 0 /* HECI Bus Message */ +#define HECI_DYN_CLIENT_ADDR_START 0x20 /* Dynamic client start addr */ + +/* A fw client has the same value for both handle and fw address */ +#define TO_FW_ADDR(handle) ((uintptr_t)(handle)) +#define TO_HECI_HANDLE(fw_addr) ((heci_handle_t)(uintptr_t)(fw_addr)) +/* convert client fw address to client context index */ +#define TO_CLIENT_CTX_IDX(fw_addr) ((fw_addr) - HECI_DYN_CLIENT_ADDR_START) /* should be less than HECI_INVALID_HANDLE - 1 */ BUILD_ASSERT(HECI_MAX_NUM_OF_CLIENTS < 0x0FE); @@ -78,11 +85,10 @@ struct heci_bus_context heci_bus_ctx = { .ipc_handle = IPC_INVALID_HANDLE, }; -/* host generates client fw address starting from 1 */ static inline struct heci_client_context * heci_get_client_context(const uint8_t fw_addr) { - return &heci_bus_ctx.client_ctxs[fw_addr - 1]; + return &heci_bus_ctx.client_ctxs[TO_CLIENT_CTX_IDX(fw_addr)]; } static inline struct heci_client_connect * @@ -100,7 +106,9 @@ static inline int heci_is_client_connected(const uint8_t fw_addr) static inline int heci_is_valid_client_addr(const uint8_t fw_addr) { - return fw_addr > 0 && fw_addr <= heci_bus_ctx.num_of_clients; + uint8_t cli_idx = TO_CLIENT_CTX_IDX(fw_addr); + + return cli_idx < heci_bus_ctx.num_of_clients; } static inline int heci_is_valid_handle(const heci_handle_t handle) @@ -108,9 +116,6 @@ static inline int heci_is_valid_handle(const heci_handle_t handle) return heci_is_valid_client_addr((uintptr_t)(handle)); } -#define TO_FW_ADDR(handle) ((uintptr_t)(handle)) -#define TO_HECI_HANDLE(fw_addr) ((heci_handle_t)(uintptr_t)(fw_addr)) - /* find heci device that contains this system state device in it */ #define ss_device_to_heci_client_context(ss_dev) \ ((struct heci_client_context *)((void *)(ss_dev) - \ @@ -159,6 +164,7 @@ struct system_state_callbacks heci_ss_cbs = { heci_handle_t heci_register_client(const struct heci_client *client) { int ret; + heci_handle_t handle; struct heci_client_context *cli_ctx; if (client == NULL || client->cbs == NULL) @@ -178,14 +184,14 @@ heci_handle_t heci_register_client(const struct heci_client *client) if (client->max_msg_size > HECI_MAX_MSG_SIZE) return HECI_INVALID_HANDLE; + /* create handle with the same value of fw address */ + handle = (heci_handle_t)(heci_bus_ctx.num_of_clients + + HECI_DYN_CLIENT_ADDR_START); cli_ctx = &heci_bus_ctx.client_ctxs[heci_bus_ctx.num_of_clients++]; - cli_ctx->client = client; if (client->cbs->initialize) { - ret = client->cbs->initialize( - (heci_handle_t)heci_bus_ctx.num_of_clients); - + ret = client->cbs->initialize(handle); if (ret) { heci_bus_ctx.num_of_clients--; return HECI_INVALID_HANDLE; @@ -197,7 +203,7 @@ heci_handle_t heci_register_client(const struct heci_client *client) ss_subsys_register_client(&cli_ctx->ss_device); } - return (heci_handle_t)heci_bus_ctx.num_of_clients; + return handle; } static void heci_build_hbm_header(struct heci_header *hdr, uint32_t length) @@ -206,7 +212,7 @@ static void heci_build_hbm_header(struct heci_header *hdr, uint32_t length) hdr->host_addr = HECI_HBM_ADDRESS; hdr->length = length; /* payload of hbm is less than IPC payload */ - hdr->length |= (uint16_t)1 << HECI_MSG_CMPL_SHIFT; + hdr->length |= (uint16_t)1 << HECI_MSG_CMPL_SHIFT; } static void heci_build_fixed_client_header(struct heci_header *hdr, @@ -217,7 +223,7 @@ static void heci_build_fixed_client_header(struct heci_header *hdr, hdr->host_addr = 0; hdr->length = length; /* Fixed client payload < IPC payload */ - hdr->length |= (uint16_t)1 << HECI_MSG_CMPL_SHIFT; + hdr->length |= (uint16_t)1 << HECI_MSG_CMPL_SHIFT; } static int heci_send_heci_msg_timestamp(struct heci_msg *msg, @@ -551,15 +557,16 @@ static int handle_enum_req(struct hbm_enum_req *enum_req) memset(enum_res, 0, sizeof(*enum_res)); /* - * current host driver implementation doesn't allow to use bit0 - * due to its fw address generation scheme. + * fw address 0 is reserved for HECI Bus Message + * fw address 1 ~ 0x1f are reserved for fixed clients + * fw address 0x20 ~ 0xFF is for dynamic clients * bit-0 set -> fw address "0", bit-1 set -> fw address "1" - * but fw address "0" is used for HECI bus management only. - * so the bitmap should not use bit-0 */ - for (i = 0; i < heci_bus_ctx.num_of_clients; i++) { - enum_res->valid_addresses[(i + 1) / BITS_PER_ELEMENT] |= - 1 << ((i + 1) & (BITS_PER_ELEMENT - 1)); + for (i = HECI_DYN_CLIENT_ADDR_START; + i < heci_bus_ctx.num_of_clients + HECI_DYN_CLIENT_ADDR_START; + i++) { + enum_res->valid_addresses[i / BITS_PER_ELEMENT] |= + 1 << (i & (BITS_PER_ELEMENT - 1)); } heci_send_heci_msg(&heci_msg); @@ -657,6 +664,9 @@ static int handle_client_connect_req( if (!heci_is_valid_client_addr(client_connect_req->fw_addr)) { client_connect_res->status = HECI_CONNECT_STATUS_CLIENT_NOT_FOUND; + } else if (!client_connect_req->host_addr) { + client_connect_res->status = + HECI_CONNECT_STATUS_INVALID_PARAMETER; } else { connect = heci_get_client_connect(client_connect_req->fw_addr); if (connect->is_connected) { @@ -669,7 +679,10 @@ static int handle_client_connect_req( } heci_send_heci_msg(&heci_msg); - heci_send_flow_control(client_connect_req->fw_addr); + + /* no error, send flow control */ + if (!client_connect_res->status) + heci_send_flow_control(client_connect_req->fw_addr); return EC_SUCCESS; } @@ -755,7 +768,7 @@ static int handle_client_disconnect_req( struct heci_client_context *cli_ctx; struct heci_client_connect *connect; const struct heci_client_callbacks *cbs; - uint8_t fw_addr; + uint8_t fw_addr, host_addr; CPRINTS("Got HECI disconnect request"); @@ -769,23 +782,29 @@ static int handle_client_disconnect_req( memset(client_disconnect_res, 0, sizeof(*client_disconnect_res)); fw_addr = client_disconnect_req->fw_addr; + host_addr = client_disconnect_req->host_addr; client_disconnect_res->fw_addr = fw_addr; - client_disconnect_res->host_addr = client_disconnect_req->host_addr; + client_disconnect_res->host_addr = host_addr; if (!heci_is_valid_client_addr(fw_addr) || !heci_is_client_connected(fw_addr)) { client_disconnect_res->status = HECI_CONNECT_STATUS_CLIENT_NOT_FOUND; } else { connect = heci_get_client_connect(fw_addr); - cli_ctx = heci_get_client_context(fw_addr); - cbs = cli_ctx->client->cbs; - mutex_lock(&connect->lock); - if (connect->is_connected) { - cbs->disconnected(TO_HECI_HANDLE(fw_addr)); - connect->is_connected = 0; + if (connect->host_addr != host_addr) { + client_disconnect_res->status = + HECI_CONNECT_STATUS_INVALID_PARAMETER; + } else { + cli_ctx = heci_get_client_context(fw_addr); + cbs = cli_ctx->client->cbs; + mutex_lock(&connect->lock); + if (connect->is_connected) { + cbs->disconnected(TO_HECI_HANDLE(fw_addr)); + connect->is_connected = 0; + } + mutex_unlock(&connect->lock); } - mutex_unlock(&connect->lock); } heci_send_heci_msg(&heci_msg); @@ -944,17 +963,30 @@ static void heci_handle_hbm(struct hbm_h2i *h2i, size_t length) static void heci_handle_heci_msg(struct heci_msg *heci_msg, size_t msg_length) { - if (heci_msg->hdr.fw_addr) { + if (!heci_msg->hdr.host_addr) { + /* + * message for HECI bus or a fixed client should fit + * into one IPC message + */ + if (!HECI_MSG_IS_COMPLETED(heci_msg->hdr.length)) { + CPRINTS("message not completed"); + return; + } + if (heci_msg->hdr.fw_addr == HECI_FIXED_SYSTEM_STATE_ADDR) heci_handle_system_state_msg( - heci_msg->payload, - HECI_MSG_LENGTH(heci_msg->hdr.length) - ); + heci_msg->payload, + HECI_MSG_LENGTH(heci_msg->hdr.length)); + else if (!heci_msg->hdr.fw_addr) + /* HECI Bus Message(fw_addr == 0 && host_addr == 0) */ + heci_handle_hbm((struct hbm_h2i *)heci_msg->payload, + HECI_MSG_LENGTH(heci_msg->hdr.length)); else - heci_handle_client_msg(heci_msg, msg_length); + CPRINTS("not supported fixed client(%d)", + heci_msg->hdr.fw_addr); } else { - heci_handle_hbm((struct hbm_h2i *)heci_msg->payload, - HECI_MSG_LENGTH(heci_msg->hdr.length)); + /* host_addr != 0 : Msg for Dynamic client */ + heci_handle_client_msg(heci_msg, msg_length); } } |