platform/chrome: wilco_ec: Standardize mailbox interface

The current API for the wilco EC mailbox interface is bad.

It assumes that most messages sent to the EC follow a similar structure,
with a command byte in MBOX[0], followed by a junk byte, followed by
actual data. This doesn't happen in several cases, such as setting the
RTC time, using the raw debugfs interface, and reading or writing
properties such as the Peak Shift policy (this last to be submitted soon).

Similarly for the response message from the EC, the current interface
assumes that the first byte of data is always 0, and the second byte
is unused. However, in both setting and getting the RTC time, in the
debugfs interface, and for reading and writing properties, this isn't
true.

The current way to resolve this is to use WILCO_EC_FLAG_RAW* flags to
specify when and when not to skip these initial bytes in the sent and
received message. They are confusing and used so much that they are
normal, and not exceptions. In addition, the first byte of
response in the debugfs interface is still always skipped, which is
weird, since this raw interface should be giving the entire result.

Additionally, sent messages assume the first byte is a command, and so
struct wilco_ec_message contains the "command" field. In setting or
getting properties however, the first byte is not a command, and so this
field has to be filled with a byte that isn't actually a command. This
is again inconsistent.

wilco_ec_message contains a result field as well, copied from
wilco_ec_response->result. The message result field should be removed:
if the message fails, the cause is already logged, and the callers are
alerted. They will never care about the actual state of the result flag.

These flags and different cases make the wilco_ec_transfer() function,
used in wilco_ec_mailbox(), really gross, dealing with a bunch of
different cases. It's difficult to figure out what it is doing.

Finally, making these assumptions about the structure of a message make
it so that the messages do not correspond well with the specification
for the EC's mailbox interface. For instance, this interface
specification may say that MBOX[9] in the received message contains
some information, but the calling code needs to remember that the first
byte of response is always skipped, and because it didn't set the
RESPONSE_RAW flag, the next byte is also skipped, so this information
is actually contained within wilco_ec_message->response_data[7]. This
makes it difficult to maintain this code in the future.

To fix these problems this patch standardizes the mailbox interface by:
- Removing the WILCO_EC_FLAG_RAW* flags
- Removing the command and reserved_raw bytes from wilco_ec_request
- Removing the mbox0 byte from wilco_ec_response
- Simplifying wilco_ec_transfer() because of these changes
- Gives the callers of wilco_ec_mailbox() the responsibility of exactly
  and consistently defining the structure of the mailbox request and
  response
- Removing command and result from wilco_ec_message.

This results in the reduction of total code, and makes it much more
maintainable and understandable.

Signed-off-by: Nick Crews <ncrews@chromium.org>
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>

1 files changed, 21 insertions, 11 deletions

diff --git a/Documentation/ABI/testing/debugfs-wilco-ec b/Documentation/ABI/testing/debugfs-wilco-ec
index f814f112e213..7ff6b45be703 100644
--- a/Documentation/ABI/testing/debugfs-wilco-ec
+++ b/Documentation/ABI/testing/debugfs-wilco-ec
@@ -4,20 +4,30 @@ KernelVersion:	5.1
 Description:
 		Write and read raw mailbox commands to the EC.
 
-		For writing:
-		Bytes 0-1 indicate the message type:
-			00 F0 = Execute Legacy Command
-			00 F2 = Read/Write NVRAM Property
-		Byte 2 provides the command code
-		Bytes 3+ consist of the data passed in the request
+		You can write a hexadecimal sentence to raw, and that series of
+		bytes will be sent to the EC. Then, you can read the bytes of
+		response by reading from raw.
 
-		At least three bytes are required, for the msg type and command,
-		with additional bytes optional for additional data.
+		For writing, bytes 0-1 indicate the message type, one of enum
+		wilco_ec_msg_type. Byte 2+ consist of the data passed in the
+		request, starting at MBOX[0]
+
+		At least three bytes are required for writing, two for the type
+		and at least a single byte of data. Only the first
+		EC_MAILBOX_DATA_SIZE bytes of MBOX will be used.
 
 		Example:
 		// Request EC info type 3 (EC firmware build date)
-		$ echo 00 f0 38 00 03 00 > raw
+		// Corresponds with sending type 0x00f0 with
+		// MBOX = [38, 00, 03, 00]
+		$ echo 00 f0 38 00 03 00 > /sys/kernel/debug/wilco_ec/raw
 		// View the result. The decoded ASCII result "12/21/18" is
 		// included after the raw hex.
-		$ cat raw
-		00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  .12/21/18.8...
+		// Corresponds with MBOX = [00, 00, 31, 32, 2f, 32, 31, 38, ...]
+		$ cat /sys/kernel/debug/wilco_ec/raw
+		00 00 31 32 2f 32 31 2f 31 38 00 38 00 01 00 2f 00  ..12/21/18.8...
+
+		Note that the first 32 bytes of the received MBOX[] will be
+		printed, even if some of the data is junk. It is up to you to
+		know how many of the first bytes of data are the actual
+		response.