Messages in this thread | | | Date | Tue, 21 Nov 2017 19:54:08 -0800 | From | Benson Leung <> | Subject | Re: [PATCH] mfd: cros ec: spi: Fix "in progress" error signaling |
| |
Hi Shawn,
On Wed, Sep 27, 2017 at 02:35:27PM -0700, Shawn Nematbakhsh wrote: > For host commands that take a long time to process, cros ec can return > early by signaling a EC_RES_IN_PROGRESS result. The host must then poll > status with EC_CMD_GET_COMMS_STATUS until completion of the command. > > None of the above applies when data link errors are encountered. When > errors such as EC_SPI_PAST_END are encountered during command > transmission, it usually means the command was not received by the EC. > Treating such errors as if they were 'EC_RES_IN_PROGRESS' results is > almost always the wrong decision, and can result in host commands > silently being lost. > > Reported-and-tested-by: Jon Hunter <jonathanh@nvidia.com> > Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Acked-by: Benson Leung <bleung@chromium.org>
> --- > drivers/mfd/cros_ec_spi.c | 52 ++++++++++++++++++++++------------------------- > 1 file changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/mfd/cros_ec_spi.c b/drivers/mfd/cros_ec_spi.c > index c9714072e224..d019b5b00b67 100644 > --- a/drivers/mfd/cros_ec_spi.c > +++ b/drivers/mfd/cros_ec_spi.c > @@ -377,6 +377,7 @@ static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev, > u8 *ptr; > u8 *rx_buf; > u8 sum; > + u8 rx_byte; > int ret = 0, final_ret; > > len = cros_ec_prepare_tx(ec_dev, ec_msg); > @@ -421,25 +422,22 @@ static int cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev, > if (!ret) { > /* Verify that EC can process command */ > for (i = 0; i < len; i++) { > - switch (rx_buf[i]) { > - case EC_SPI_PAST_END: > - case EC_SPI_RX_BAD_DATA: > - case EC_SPI_NOT_READY: > - ret = -EAGAIN; > - ec_msg->result = EC_RES_IN_PROGRESS; > - default: > + rx_byte = rx_buf[i]; > + if (rx_byte == EC_SPI_PAST_END || > + rx_byte == EC_SPI_RX_BAD_DATA || > + rx_byte == EC_SPI_NOT_READY) { > + ret = -EREMOTEIO; > break; > } > - if (ret) > - break; > } > - if (!ret) > - ret = cros_ec_spi_receive_packet(ec_dev, > - ec_msg->insize + sizeof(*response)); > - } else { > - dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret); > } > > + if (!ret) > + ret = cros_ec_spi_receive_packet(ec_dev, > + ec_msg->insize + sizeof(*response)); > + else > + dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret); > + > final_ret = terminate_request(ec_dev); > > spi_bus_unlock(ec_spi->spi->master); > @@ -508,6 +506,7 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, > int i, len; > u8 *ptr; > u8 *rx_buf; > + u8 rx_byte; > int sum; > int ret = 0, final_ret; > > @@ -544,25 +543,22 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev, > if (!ret) { > /* Verify that EC can process command */ > for (i = 0; i < len; i++) { > - switch (rx_buf[i]) { > - case EC_SPI_PAST_END: > - case EC_SPI_RX_BAD_DATA: > - case EC_SPI_NOT_READY: > - ret = -EAGAIN; > - ec_msg->result = EC_RES_IN_PROGRESS; > - default: > + rx_byte = rx_buf[i]; > + if (rx_byte == EC_SPI_PAST_END || > + rx_byte == EC_SPI_RX_BAD_DATA || > + rx_byte == EC_SPI_NOT_READY) { > + ret = -EREMOTEIO; > break; > } > - if (ret) > - break; > } > - if (!ret) > - ret = cros_ec_spi_receive_response(ec_dev, > - ec_msg->insize + EC_MSG_TX_PROTO_BYTES); > - } else { > - dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret); > } > > + if (!ret) > + ret = cros_ec_spi_receive_response(ec_dev, > + ec_msg->insize + EC_MSG_TX_PROTO_BYTES); > + else > + dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret); > + > final_ret = terminate_request(ec_dev); > > spi_bus_unlock(ec_spi->spi->master); > -- > 2.12.2 >
-- Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org [unhandled content-type:application/pgp-signature] | |