From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751518AbdKVDyO (ORCPT ); Tue, 21 Nov 2017 22:54:14 -0500 Received: from mail-it0-f48.google.com ([209.85.214.48]:42102 "EHLO mail-it0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751409AbdKVDyN (ORCPT ); Tue, 21 Nov 2017 22:54:13 -0500 X-Google-Smtp-Source: AGs4zMb4SguRWxtqc7J2j+hg1E7obgQwIGRbO6VqT2u8tasz15+gUu9Apnts2XIXIIja3gl1hPHVBw== Date: Tue, 21 Nov 2017 19:54:08 -0800 From: Benson Leung To: Shawn Nematbakhsh Cc: linux-kernel@vger.kernel.org, lee.jones@linaro.org, jonathanh@nvidia.com, briannorris@chromium.org, bleung@google.com, bleung@chromium.org Subject: Re: [PATCH] mfd: cros ec: spi: Fix "in progress" error signaling Message-ID: <20171122035408.GA121077@decatoncale.mtv.corp.google.com> References: <20170927213527.31416-1-shawnn@chromium.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="OgqxwSJOaUobr8KG" Content-Disposition: inline In-Reply-To: <20170927213527.31416-1-shawnn@chromium.org> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --OgqxwSJOaUobr8KG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. >=20 > 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. >=20 > Reported-and-tested-by: Jon Hunter > Signed-off-by: Shawn Nematbakhsh Acked-by: Benson Leung > --- > drivers/mfd/cros_ec_spi.c | 52 ++++++++++++++++++++++-------------------= ------ > 1 file changed, 24 insertions(+), 28 deletions(-) >=20 > 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 =3D 0, final_ret; > =20 > len =3D cros_ec_prepare_tx(ec_dev, ec_msg); > @@ -421,25 +422,22 @@ static int cros_ec_pkt_xfer_spi(struct cros_ec_devi= ce *ec_dev, > if (!ret) { > /* Verify that EC can process command */ > for (i =3D 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 =3D -EAGAIN; > - ec_msg->result =3D EC_RES_IN_PROGRESS; > - default: > + rx_byte =3D rx_buf[i]; > + if (rx_byte =3D=3D EC_SPI_PAST_END || > + rx_byte =3D=3D EC_SPI_RX_BAD_DATA || > + rx_byte =3D=3D EC_SPI_NOT_READY) { > + ret =3D -EREMOTEIO; > break; > } > - if (ret) > - break; > } > - if (!ret) > - ret =3D cros_ec_spi_receive_packet(ec_dev, > - ec_msg->insize + sizeof(*response)); > - } else { > - dev_err(ec_dev->dev, "spi transfer failed: %d\n", ret); > } > =20 > + if (!ret) > + ret =3D 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 =3D terminate_request(ec_dev); > =20 > 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 =3D 0, final_ret; > =20 > @@ -544,25 +543,22 @@ static int cros_ec_cmd_xfer_spi(struct cros_ec_devi= ce *ec_dev, > if (!ret) { > /* Verify that EC can process command */ > for (i =3D 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 =3D -EAGAIN; > - ec_msg->result =3D EC_RES_IN_PROGRESS; > - default: > + rx_byte =3D rx_buf[i]; > + if (rx_byte =3D=3D EC_SPI_PAST_END || > + rx_byte =3D=3D EC_SPI_RX_BAD_DATA || > + rx_byte =3D=3D EC_SPI_NOT_READY) { > + ret =3D -EREMOTEIO; > break; > } > - if (ret) > - break; > } > - if (!ret) > - ret =3D 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); > } > =20 > + if (!ret) > + ret =3D 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 =3D terminate_request(ec_dev); > =20 > spi_bus_unlock(ec_spi->spi->master); > --=20 > 2.12.2 >=20 --=20 Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org --OgqxwSJOaUobr8KG Content-Type: application/pgp-signature; name="signature.asc" Content-Description: Digital signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJaFPTgAAoJEB8J9XsKL+ZYv0cQAI0MbqxHkCneyjhg/8vwlxxT 8Wzw147HO7eiFmIklHv3BinUmk2HhfVH1Wv8RboSylFyUCx57xO4QGECSvF0D5yk JtctDX2f43jr20FNCBmJPga829H2EayvNQ5e5B9gqw2Qm7v84LkUExSHhjphyvRv IxX938duTvPeb1r1VOBHzXd95UWac7vkha8koea8y/6nz6qnEcdKUG5H4/4AKxNn v4N+PJ4ouuqjos/MgxkASUMM0bAHptQbe9twekZt5Af25gNBj8vHV5nfdoeXdhJy McC0HdML3FuTD562VVuOFtqlb/vl4Ow2dNBR0bm0uzpjAjQsTVLUPlZcu/yo6g4D AhLwmVvIw3ttcetloLT41TJG9+dRBwbCC59DlBM5WS3DgX6KoZSvKxMGi0TlXxxu 3JzCwTlSYag7aBaRXgzRxxQKUuC50vJzw/X1R5xH728eORsQvab7Vi0ElrHUHT8a n0D3FiaBkMoXBeaaliCi4rqmF9vC+m5XeLvpaBFOHY24/YUBfeJYmXBl1So5lYoC VdXgPmTDAiq5++QoFtRBCJruqxNoJ8oIC1ycomn4z8ZBl7ahpd2bv8fTS2fv0Plh bnVDG3WmFZwkeK1Il96VHtA5o+bub17jNgptsqlkaOYUCXBuOUR/tpDhqCF4poFK oZSrFRHaDsoWy63Uq0qV =iE4i -----END PGP SIGNATURE----- --OgqxwSJOaUobr8KG--