All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tzung-Bi Shih <tzungbi@kernel.org>
To: bleung@chromium.org, groeck@chromium.org
Cc: chrome-platform@lists.linux.dev, tzungbi@kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 00/10] platform/chrome: Kunit tests and refactor for cros_ec_cmd_xfer()
Date: Mon, 18 Jul 2022 05:09:04 +0000	[thread overview]
Message-ID: <20220718050914.2267370-1-tzungbi@kernel.org> (raw)

The 1st patch fixes an issue that cros_kunit_ec_xfer_mock() could return
garbage bytes for `msg->result` if the mock list is empty.

The 2nd ~ 6th patches add Kunit tests and refactors for cros_ec_cmd_xfer().

The last 4 patches change the behavior a bit by altering return codes.


Asynchronous mechanism in EC protocol:
EC returns EC_RES_IN_PROGRESS if the host command needs more time to
complete.  It saves the result into `saved_result` once the command
completed[1].

By design, AP should send another command EC_CMD_RESEND_RESPONSE for
getting the result from the previous pending command[2].  The mechanism
was only designed for commands that don't need responses[3].

However, the kernel code doesn't have such logic when dealing with
EC_RES_IN_PROGRESS.

The series doesn't fix it but leave it as is.  I doubt there is no existing
use case.

[1]: https://crrev.com/9b80051e01872d8cae86fff999b5d31a0bea985b/common/host_command.c#113
[2]: https://crrev.com/9b80051e01872d8cae86fff999b5d31a0bea985b/common/host_command.c#748
[3]: https://crrev.com/9b80051e01872d8cae86fff999b5d31a0bea985b/common/host_command.c#126


Return value overridden in cros_ec_send_command() if asynchronous:
By original intention, cros_ec_send_command() returns number of available
bytes of input payload.

When it falls into asynchronous path (i.e. EC_RES_IN_PROGRESS), both return
value and `msg->result` will be overridden by the subsequent
EC_CMD_GET_COMMS_STATUS.

The series doesn't fix it but leave it as is.

Tzung-Bi Shih (10):
  platform/chrome: cros_kunit_util: add default value for `msg->result`
  platform/chrome: cros_ec_proto: add "cros_ec_" prefix to
    send_command()
  platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_cmd_xfer()
  platform/chrome: cros_ec_proto: add Kunit tests for
    cros_ec_send_command()
  platform/chrome: cros_ec_proto: separate cros_ec_xfer_command()
  platform/chrome: cros_ec_proto: separate cros_ec_wait_until_complete()
  platform/chrome: cros_ec_proto: change Kunit expectation when timed
    out
  platform/chrome: cros_ec_proto: return -EAGAIN when retries timed out
  platform/chrome: cros_ec_proto: add Kunit test for empty payload
  platform/chrome: cros_ec_proto: return -EPROTO if empty payload

 drivers/platform/chrome/cros_ec_proto.c      | 102 +++--
 drivers/platform/chrome/cros_ec_proto_test.c | 443 +++++++++++++++++++
 drivers/platform/chrome/cros_kunit_util.c    |  27 +-
 drivers/platform/chrome/cros_kunit_util.h    |   5 +
 4 files changed, 530 insertions(+), 47 deletions(-)

Changes from v1 (https://patchwork.kernel.org/project/chrome-platform/cover/20220628024913.1755292-1-tzungbi@kernel.org/):
- Drop "platform/chrome: cros_ec_proto: change Kunit expectation for EC errors"
  and "platform/chrome: cros_ec_proto: return standard error codes for EC
  errors" because they could break existing ABI.
- Add "platform/chrome: cros_kunit_util: add default value for `msg->result`".

-- 
2.37.0.170.g444d1eabd0-goog


             reply	other threads:[~2022-07-18  5:10 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-07-18  5:09 Tzung-Bi Shih [this message]
2022-07-18  5:09 ` [PATCH v2 01/10] platform/chrome: cros_kunit_util: add default value for `msg->result` Tzung-Bi Shih
2022-07-18 13:35   ` Guenter Roeck
2022-07-19  4:00     ` Tzung-Bi Shih
2022-07-19 21:27       ` Guenter Roeck
2022-07-20  1:00         ` Tzung-Bi Shih
2022-07-18  5:09 ` [PATCH v2 02/10] platform/chrome: cros_ec_proto: add "cros_ec_" prefix to send_command() Tzung-Bi Shih
2022-07-18  5:09 ` [PATCH v2 03/10] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_cmd_xfer() Tzung-Bi Shih
2022-07-18  5:09 ` [PATCH v2 04/10] platform/chrome: cros_ec_proto: add Kunit tests for cros_ec_send_command() Tzung-Bi Shih
2022-07-18  5:09 ` [PATCH v2 05/10] platform/chrome: cros_ec_proto: separate cros_ec_xfer_command() Tzung-Bi Shih
2022-07-18  5:09 ` [PATCH v2 06/10] platform/chrome: cros_ec_proto: separate cros_ec_wait_until_complete() Tzung-Bi Shih
2022-07-18 13:32   ` Guenter Roeck
2022-07-18  5:09 ` [PATCH v2 07/10] platform/chrome: cros_ec_proto: change Kunit expectation when timed out Tzung-Bi Shih
2022-07-18  5:09 ` [PATCH v2 08/10] platform/chrome: cros_ec_proto: return -EAGAIN when retries " Tzung-Bi Shih
2022-07-18 13:33   ` Guenter Roeck
2022-07-18  5:09 ` [PATCH v2 09/10] platform/chrome: cros_ec_proto: add Kunit test for empty payload Tzung-Bi Shih
2022-07-18 13:36   ` Guenter Roeck
2022-07-18  5:09 ` [PATCH v2 10/10] platform/chrome: cros_ec_proto: return -EPROTO if " Tzung-Bi Shih
2022-07-20  1:30 ` [PATCH v2 00/10] platform/chrome: Kunit tests and refactor for cros_ec_cmd_xfer() patchwork-bot+chrome-platform
2022-07-21  8:50 ` patchwork-bot+chrome-platform

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220718050914.2267370-1-tzungbi@kernel.org \
    --to=tzungbi@kernel.org \
    --cc=bleung@chromium.org \
    --cc=chrome-platform@lists.linux.dev \
    --cc=groeck@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.