]> www.infradead.org Git - users/hch/configfs.git/commitdiff
platform/chrome: cros_ec_lpc: MEC access can return error code
authorBen Walsh <ben@jubnut.com>
Wed, 5 Jun 2024 06:33:47 +0000 (07:33 +0100)
committerTzung-Bi Shih <tzungbi@kernel.org>
Thu, 6 Jun 2024 03:09:16 +0000 (03:09 +0000)
cros_ec_lpc_io_bytes_mec was returning a u8 checksum of all bytes
read/written, which didn't leave room to indicate errors. Change this
u8 to an int where negative values indicate an error, and non-negative
values are the checksum as before.

Tested-by: Dustin L. Howett <dustin@howett.net>
Signed-off-by: Ben Walsh <ben@jubnut.com>
Link: https://lore.kernel.org/r/20240605063351.14836-2-ben@jubnut.com
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
drivers/platform/chrome/cros_ec_lpc.c
drivers/platform/chrome/cros_ec_lpc_mec.c
drivers/platform/chrome/cros_ec_lpc_mec.h
drivers/platform/chrome/wilco_ec/mailbox.c

index ddfbfec44f4ccfc184c504b9568f5f85e6ee5eb8..7bf13c1d2c670ee96a8857cff92edf5d079be326 100644 (file)
@@ -62,14 +62,16 @@ struct cros_ec_lpc {
 
 /**
  * struct lpc_driver_ops - LPC driver operations
- * @read: Copy length bytes from EC address offset into buffer dest. Returns
- *        the 8-bit checksum of all bytes read.
- * @write: Copy length bytes from buffer msg into EC address offset. Returns
- *         the 8-bit checksum of all bytes written.
+ * @read: Copy length bytes from EC address offset into buffer dest.
+ *        Returns a negative error code on error, or the 8-bit checksum
+ *        of all bytes read.
+ * @write: Copy length bytes from buffer msg into EC address offset.
+ *         Returns a negative error code on error, or the 8-bit checksum
+ *         of all bytes written.
  */
 struct lpc_driver_ops {
-       u8 (*read)(unsigned int offset, unsigned int length, u8 *dest);
-       u8 (*write)(unsigned int offset, unsigned int length, const u8 *msg);
+       int (*read)(unsigned int offset, unsigned int length, u8 *dest);
+       int (*write)(unsigned int offset, unsigned int length, const u8 *msg);
 };
 
 static struct lpc_driver_ops cros_ec_lpc_ops = { };
@@ -78,10 +80,10 @@ static struct lpc_driver_ops cros_ec_lpc_ops = { };
  * A generic instance of the read function of struct lpc_driver_ops, used for
  * the LPC EC.
  */
-static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length,
-                                u8 *dest)
+static int cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length,
+                                 u8 *dest)
 {
-       int sum = 0;
+       u8 sum = 0;
        int i;
 
        for (i = 0; i < length; ++i) {
@@ -97,10 +99,10 @@ static u8 cros_ec_lpc_read_bytes(unsigned int offset, unsigned int length,
  * A generic instance of the write function of struct lpc_driver_ops, used for
  * the LPC EC.
  */
-static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length,
-                                 const u8 *msg)
+static int cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length,
+                                  const u8 *msg)
 {
-       int sum = 0;
+       u8 sum = 0;
        int i;
 
        for (i = 0; i < length; ++i) {
@@ -116,8 +118,8 @@ static u8 cros_ec_lpc_write_bytes(unsigned int offset, unsigned int length,
  * An instance of the read function of struct lpc_driver_ops, used for the
  * MEC variant of LPC EC.
  */
-static u8 cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length,
-                                    u8 *dest)
+static int cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length,
+                                     u8 *dest)
 {
        int in_range = cros_ec_lpc_mec_in_range(offset, length);
 
@@ -135,8 +137,8 @@ static u8 cros_ec_lpc_mec_read_bytes(unsigned int offset, unsigned int length,
  * An instance of the write function of struct lpc_driver_ops, used for the
  * MEC variant of LPC EC.
  */
-static u8 cros_ec_lpc_mec_write_bytes(unsigned int offset, unsigned int length,
-                                     const u8 *msg)
+static int cros_ec_lpc_mec_write_bytes(unsigned int offset, unsigned int length,
+                                      const u8 *msg)
 {
        int in_range = cros_ec_lpc_mec_in_range(offset, length);
 
@@ -154,11 +156,14 @@ static int ec_response_timed_out(void)
 {
        unsigned long one_second = jiffies + HZ;
        u8 data;
+       int ret;
 
        usleep_range(200, 300);
        do {
-               if (!(cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_CMD, 1, &data) &
-                   EC_LPC_STATUS_BUSY_MASK))
+               ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_CMD, 1, &data);
+               if (ret < 0)
+                       return ret;
+               if (!(data & EC_LPC_STATUS_BUSY_MASK))
                        return 0;
                usleep_range(100, 200);
        } while (time_before(jiffies, one_second));
@@ -179,28 +184,41 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
                goto done;
 
        /* Write buffer */
-       cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
+       ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
+       if (ret < 0)
+               goto done;
 
        /* Here we go */
        sum = EC_COMMAND_PROTOCOL_3;
-       cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+       ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+       if (ret < 0)
+               goto done;
 
-       if (ec_response_timed_out()) {
+       ret = ec_response_timed_out();
+       if (ret < 0)
+               goto done;
+       if (ret) {
                dev_warn(ec->dev, "EC response timed out\n");
                ret = -EIO;
                goto done;
        }
 
        /* Check result */
-       msg->result = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+       if (ret < 0)
+               goto done;
+       msg->result = ret;
        ret = cros_ec_check_result(ec, msg);
        if (ret)
                goto done;
 
        /* Read back response */
        dout = (u8 *)&response;
-       sum = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET, sizeof(response),
                                   dout);
+       if (ret < 0)
+               goto done;
+       sum = ret;
 
        msg->result = response.result;
 
@@ -213,9 +231,12 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
        }
 
        /* Read response and process checksum */
-       sum += cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET +
-                                   sizeof(response), response.data_len,
-                                   msg->data);
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PACKET +
+                                  sizeof(response), response.data_len,
+                                  msg->data);
+       if (ret < 0)
+               goto done;
+       sum += ret;
 
        if (sum) {
                dev_err(ec->dev,
@@ -255,32 +276,47 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
        sum = msg->command + args.flags + args.command_version + args.data_size;
 
        /* Copy data and update checksum */
-       sum += cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
-                                    msg->data);
+       ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PARAM, msg->outsize,
+                                   msg->data);
+       if (ret < 0)
+               goto done;
+       sum += ret;
 
        /* Finalize checksum and write args */
        args.checksum = sum;
-       cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
-                             (u8 *)&args);
+       ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_ARGS, sizeof(args),
+                                   (u8 *)&args);
+       if (ret < 0)
+               goto done;
 
        /* Here we go */
        sum = msg->command;
-       cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+       ret = cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_CMD, 1, &sum);
+       if (ret < 0)
+               goto done;
 
-       if (ec_response_timed_out()) {
+       ret = ec_response_timed_out();
+       if (ret < 0)
+               goto done;
+       if (ret) {
                dev_warn(ec->dev, "EC response timed out\n");
                ret = -EIO;
                goto done;
        }
 
        /* Check result */
-       msg->result = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_DATA, 1, &sum);
+       if (ret < 0)
+               goto done;
+       msg->result = ret;
        ret = cros_ec_check_result(ec, msg);
        if (ret)
                goto done;
 
        /* Read back args */
-       cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_ARGS, sizeof(args), (u8 *)&args);
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_ARGS, sizeof(args), (u8 *)&args);
+       if (ret < 0)
+               goto done;
 
        if (args.data_size > msg->insize) {
                dev_err(ec->dev,
@@ -294,8 +330,11 @@ static int cros_ec_cmd_xfer_lpc(struct cros_ec_device *ec,
        sum = msg->command + args.flags + args.command_version + args.data_size;
 
        /* Read response and update checksum */
-       sum += cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PARAM, args.data_size,
-                                   msg->data);
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_HOST_PARAM, args.data_size,
+                                  msg->data);
+       if (ret < 0)
+               goto done;
+       sum += ret;
 
        /* Verify checksum */
        if (args.checksum != sum) {
@@ -320,19 +359,24 @@ static int cros_ec_lpc_readmem(struct cros_ec_device *ec, unsigned int offset,
        int i = offset;
        char *s = dest;
        int cnt = 0;
+       int ret;
 
        if (offset >= EC_MEMMAP_SIZE - bytes)
                return -EINVAL;
 
        /* fixed length */
        if (bytes) {
-               cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
+               ret = cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + offset, bytes, s);
+               if (ret < 0)
+                       return ret;
                return bytes;
        }
 
        /* string */
        for (; i < EC_MEMMAP_SIZE; i++, s++) {
-               cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
+               ret = cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + i, 1, s);
+               if (ret < 0)
+                       return ret;
                cnt++;
                if (!*s)
                        break;
@@ -425,7 +469,9 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
         */
        cros_ec_lpc_ops.read = cros_ec_lpc_mec_read_bytes;
        cros_ec_lpc_ops.write = cros_ec_lpc_mec_write_bytes;
-       cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
+       ret = cros_ec_lpc_ops.read(EC_LPC_ADDR_MEMMAP + EC_MEMMAP_ID, 2, buf);
+       if (ret < 0)
+               return ret;
        if (buf[0] != 'E' || buf[1] != 'C') {
                if (!devm_request_region(dev, ec_lpc->mmio_memory_base, EC_MEMMAP_SIZE,
                                         dev_name(dev))) {
@@ -436,8 +482,10 @@ static int cros_ec_lpc_probe(struct platform_device *pdev)
                /* Re-assign read/write operations for the non MEC variant */
                cros_ec_lpc_ops.read = cros_ec_lpc_read_bytes;
                cros_ec_lpc_ops.write = cros_ec_lpc_write_bytes;
-               cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
-                                    buf);
+               ret = cros_ec_lpc_ops.read(ec_lpc->mmio_memory_base + EC_MEMMAP_ID, 2,
+                                          buf);
+               if (ret < 0)
+                       return ret;
                if (buf[0] != 'E' || buf[1] != 'C') {
                        dev_err(dev, "EC ID not detected\n");
                        return -ENODEV;
index 0d9c79b270ce205a2840a7b13163d80e50ec348a..395dc3a6fb5efcd273acb6002184610ff1a71b85 100644 (file)
@@ -67,11 +67,12 @@ int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length)
  * @length:  Number of bytes to read / write
  * @buf:     Destination / source buffer
  *
- * Return: 8-bit checksum of all bytes read / written
+ * @return:  A negative error code on error, or 8-bit checksum of all
+ *           bytes read / written
  */
-u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
-                           unsigned int offset, unsigned int length,
-                           u8 *buf)
+int cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
+                            unsigned int offset, unsigned int length,
+                            u8 *buf)
 {
        int i = 0;
        int io_addr;
index 9d0521b23e8aedb6624658eb847bc8c822f61859..69670832f187b1963fd13c8ca18b9e7ad8a0cde0 100644 (file)
@@ -64,9 +64,10 @@ int cros_ec_lpc_mec_in_range(unsigned int offset, unsigned int length);
  * @length:  Number of bytes to read / write
  * @buf:     Destination / source buffer
  *
- * @return 8-bit checksum of all bytes read / written
+ * @return:  A negative error code on error, or 8-bit checksum of all
+ *           bytes read / written
  */
-u8 cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
-                           unsigned int offset, unsigned int length, u8 *buf);
+int cros_ec_lpc_io_bytes_mec(enum cros_ec_lpc_mec_io_type io_type,
+                            unsigned int offset, unsigned int length, u8 *buf);
 
 #endif /* __CROS_EC_LPC_MEC_H */
index 0f98358ea824c61367ed854d0b0cc48e011cdad9..4d8273b47cdeeac062a09c8e78df1b20d7d6dc25 100644 (file)
@@ -117,13 +117,17 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec,
                             struct wilco_ec_request *rq)
 {
        struct wilco_ec_response *rs;
-       u8 checksum;
+       int ret;
        u8 flag;
 
        /* Write request header, then data */
-       cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
-       cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
-                                msg->request_data);
+       ret = cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, 0, sizeof(*rq), (u8 *)rq);
+       if (ret < 0)
+               return ret;
+       ret = cros_ec_lpc_io_bytes_mec(MEC_IO_WRITE, sizeof(*rq), msg->request_size,
+                                      msg->request_data);
+       if (ret < 0)
+               return ret;
 
        /* Start the command */
        outb(EC_MAILBOX_START_COMMAND, ec->io_command->start);
@@ -149,10 +153,12 @@ static int wilco_ec_transfer(struct wilco_ec_device *ec,
 
        /* Read back response */
        rs = ec->data_buffer;
-       checksum = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
-                                           sizeof(*rs) + EC_MAILBOX_DATA_SIZE,
-                                           (u8 *)rs);
-       if (checksum) {
+       ret = cros_ec_lpc_io_bytes_mec(MEC_IO_READ, 0,
+                                      sizeof(*rs) + EC_MAILBOX_DATA_SIZE,
+                                      (u8 *)rs);
+       if (ret < 0)
+               return ret;
+       if (ret) {
                dev_dbg(ec->dev, "bad packet checksum 0x%02x\n", rs->checksum);
                return -EBADMSG;
        }