The series gets rid of BUG_ON()s in drivers/platform/chrome/. Most of them
can be replaced by returning proper return code.
The 2nd patch makes callers of cros_ec_prepare_tx() to take care of the
return code.
The 3rd patch turns cros_ec_prepare_tx() to return error code if any.
Tzung-Bi Shih (6):
platform/chrome: cros_ec_proto: drop unneeded BUG_ON() in
prepare_packet()
platform/chrome: correct cros_ec_prepare_tx() usage
platform/chrome: cros_ec_proto: drop BUG_ON() in cros_ec_prepare_tx()
platform/chrome: cros_ec_proto: drop BUG_ON() in
cros_ec_get_host_event()
platform/chrome: cros_ec_i2c: drop BUG_ON() in cros_ec_pkt_xfer_i2c()
platform/chrome: cros_ec_spi: drop BUG_ON()
drivers/platform/chrome/cros_ec_i2c.c | 12 ++++++++++--
drivers/platform/chrome/cros_ec_ishtp.c | 4 +++-
drivers/platform/chrome/cros_ec_lpc.c | 2 ++
drivers/platform/chrome/cros_ec_proto.c | 13 ++++++++-----
drivers/platform/chrome/cros_ec_rpmsg.c | 2 ++
drivers/platform/chrome/cros_ec_spi.c | 19 ++++++++++++++-----
6 files changed, 39 insertions(+), 13 deletions(-)
--
2.36.0.512.ge40c2bad7a-goog
cros_ec_prepare_tx() returns either:
- >= 0 for number of prepared bytes.
- < 0 for -errno.
Correct the comment and make sure all callers check the return code.
Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_i2c.c | 2 ++
drivers/platform/chrome/cros_ec_ishtp.c | 4 +++-
drivers/platform/chrome/cros_ec_lpc.c | 2 ++
drivers/platform/chrome/cros_ec_proto.c | 2 +-
drivers/platform/chrome/cros_ec_rpmsg.c | 2 ++
drivers/platform/chrome/cros_ec_spi.c | 4 ++++
6 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
index 22feb0fd4ce7..a4f305f1eb0e 100644
--- a/drivers/platform/chrome/cros_ec_i2c.c
+++ b/drivers/platform/chrome/cros_ec_i2c.c
@@ -89,6 +89,8 @@ static int cros_ec_pkt_xfer_i2c(struct cros_ec_device *ec_dev,
ec_dev->dout++;
ret = cros_ec_prepare_tx(ec_dev, msg);
+ if (ret < 0)
+ goto done;
ec_dev->dout--;
/* send command to EC and read answer */
diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
index 4020b8354bae..cb2031cf7106 100644
--- a/drivers/platform/chrome/cros_ec_ishtp.c
+++ b/drivers/platform/chrome/cros_ec_ishtp.c
@@ -521,7 +521,9 @@ static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
out_msg->hdr.status = 0;
ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
- cros_ec_prepare_tx(ec_dev, msg);
+ rv = cros_ec_prepare_tx(ec_dev, msg);
+ if (rv < 0)
+ goto end_error;
ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
dev_dbg(dev,
diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
index 8eeef85a96b1..7677ab3c0ead 100644
--- a/drivers/platform/chrome/cros_ec_lpc.c
+++ b/drivers/platform/chrome/cros_ec_lpc.c
@@ -147,6 +147,8 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
u8 *dout;
ret = cros_ec_prepare_tx(ec, msg);
+ if (ret < 0)
+ goto done;
/* Write buffer */
cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index db1c8ba43171..2d6d3fbfa905 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -164,7 +164,7 @@ static int send_command(struct cros_ec_device *ec_dev,
* only SPI uses it. Once LPC uses the same protocol it can start using it.
* I2C could use it now, with a refactor of the existing code.
*
- * Return: 0 on success or negative error code.
+ * Return: number of prepared bytes on success or negative error code.
*/
int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
struct cros_ec_command *msg)
diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
index d96d15b8ca94..39d3b50a7c09 100644
--- a/drivers/platform/chrome/cros_ec_rpmsg.c
+++ b/drivers/platform/chrome/cros_ec_rpmsg.c
@@ -89,6 +89,8 @@ static int cros_ec_pkt_xfer_rpmsg(struct cros_ec_device *ec_dev,
ec_msg->result = 0;
len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ if (len < 0)
+ return len;
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
reinit_completion(&ec_rpmsg->xfer_ack);
diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
index 8493af0f680e..589f18e9537d 100644
--- a/drivers/platform/chrome/cros_ec_spi.c
+++ b/drivers/platform/chrome/cros_ec_spi.c
@@ -401,6 +401,8 @@ static int do_cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev,
unsigned long delay;
len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ if (len < 0)
+ return len;
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
/* If it's too soon to do another transaction, wait */
@@ -544,6 +546,8 @@ static int do_cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
unsigned long delay;
len = cros_ec_prepare_tx(ec_dev, ec_msg);
+ if (len < 0)
+ return len;
dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
/* If it's too soon to do another transaction, wait */
--
2.36.0.512.ge40c2bad7a-goog
prepare_packet() gets called if `ec_dev->proto_version` > 2. For now, it
must be equivalent to EC_HOST_REQUEST_VERSION.
Drop the BUG_ON().
Signed-off-by: Tzung-Bi Shih <[email protected]>
---
drivers/platform/chrome/cros_ec_proto.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ac1419881ff3..db1c8ba43171 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -60,7 +60,6 @@ static int prepare_packet(struct cros_ec_device *ec_dev,
int i;
u8 csum = 0;
- BUG_ON(ec_dev->proto_version != EC_HOST_REQUEST_VERSION);
BUG_ON(msg->outsize + sizeof(*request) > ec_dev->dout_size);
out = ec_dev->dout;
--
2.36.0.512.ge40c2bad7a-goog
On Thu, May 12, 2022 at 1:36 AM Tzung-Bi Shih <[email protected]> wrote:
>
> cros_ec_prepare_tx() returns either:
> - >= 0 for number of prepared bytes.
> - < 0 for -errno.
>
> Correct the comment and make sure all callers check the return code.
>
> Signed-off-by: Tzung-Bi Shih <[email protected]>
Makes sense in the context of the next patch.
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/platform/chrome/cros_ec_i2c.c | 2 ++
> drivers/platform/chrome/cros_ec_ishtp.c | 4 +++-
> drivers/platform/chrome/cros_ec_lpc.c | 2 ++
> drivers/platform/chrome/cros_ec_proto.c | 2 +-
> drivers/platform/chrome/cros_ec_rpmsg.c | 2 ++
> drivers/platform/chrome/cros_ec_spi.c | 4 ++++
> 6 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_i2c.c b/drivers/platform/chrome/cros_ec_i2c.c
> index 22feb0fd4ce7..a4f305f1eb0e 100644
> --- a/drivers/platform/chrome/cros_ec_i2c.c
> +++ b/drivers/platform/chrome/cros_ec_i2c.c
> @@ -89,6 +89,8 @@ static int cros_ec_pkt_xfer_i2c(struct cros_ec_device *ec_dev,
>
> ec_dev->dout++;
> ret = cros_ec_prepare_tx(ec_dev, msg);
> + if (ret < 0)
> + goto done;
> ec_dev->dout--;
>
> /* send command to EC and read answer */
> diff --git a/drivers/platform/chrome/cros_ec_ishtp.c b/drivers/platform/chrome/cros_ec_ishtp.c
> index 4020b8354bae..cb2031cf7106 100644
> --- a/drivers/platform/chrome/cros_ec_ishtp.c
> +++ b/drivers/platform/chrome/cros_ec_ishtp.c
> @@ -521,7 +521,9 @@ static int cros_ec_pkt_xfer_ish(struct cros_ec_device *ec_dev,
> out_msg->hdr.status = 0;
>
> ec_dev->dout += OUT_MSG_EC_REQUEST_PREAMBLE;
> - cros_ec_prepare_tx(ec_dev, msg);
> + rv = cros_ec_prepare_tx(ec_dev, msg);
> + if (rv < 0)
> + goto end_error;
> ec_dev->dout -= OUT_MSG_EC_REQUEST_PREAMBLE;
>
> dev_dbg(dev,
> diff --git a/drivers/platform/chrome/cros_ec_lpc.c b/drivers/platform/chrome/cros_ec_lpc.c
> index 8eeef85a96b1..7677ab3c0ead 100644
> --- a/drivers/platform/chrome/cros_ec_lpc.c
> +++ b/drivers/platform/chrome/cros_ec_lpc.c
> @@ -147,6 +147,8 @@ static int cros_ec_pkt_xfer_lpc(struct cros_ec_device *ec,
> u8 *dout;
>
> ret = cros_ec_prepare_tx(ec, msg);
> + if (ret < 0)
> + goto done;
>
> /* Write buffer */
> cros_ec_lpc_ops.write(EC_LPC_ADDR_HOST_PACKET, ret, ec->dout);
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index db1c8ba43171..2d6d3fbfa905 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -164,7 +164,7 @@ static int send_command(struct cros_ec_device *ec_dev,
> * only SPI uses it. Once LPC uses the same protocol it can start using it.
> * I2C could use it now, with a refactor of the existing code.
> *
> - * Return: 0 on success or negative error code.
> + * Return: number of prepared bytes on success or negative error code.
> */
> int cros_ec_prepare_tx(struct cros_ec_device *ec_dev,
> struct cros_ec_command *msg)
> diff --git a/drivers/platform/chrome/cros_ec_rpmsg.c b/drivers/platform/chrome/cros_ec_rpmsg.c
> index d96d15b8ca94..39d3b50a7c09 100644
> --- a/drivers/platform/chrome/cros_ec_rpmsg.c
> +++ b/drivers/platform/chrome/cros_ec_rpmsg.c
> @@ -89,6 +89,8 @@ static int cros_ec_pkt_xfer_rpmsg(struct cros_ec_device *ec_dev,
>
> ec_msg->result = 0;
> len = cros_ec_prepare_tx(ec_dev, ec_msg);
> + if (len < 0)
> + return len;
> dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
>
> reinit_completion(&ec_rpmsg->xfer_ack);
> diff --git a/drivers/platform/chrome/cros_ec_spi.c b/drivers/platform/chrome/cros_ec_spi.c
> index 8493af0f680e..589f18e9537d 100644
> --- a/drivers/platform/chrome/cros_ec_spi.c
> +++ b/drivers/platform/chrome/cros_ec_spi.c
> @@ -401,6 +401,8 @@ static int do_cros_ec_pkt_xfer_spi(struct cros_ec_device *ec_dev,
> unsigned long delay;
>
> len = cros_ec_prepare_tx(ec_dev, ec_msg);
> + if (len < 0)
> + return len;
> dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
>
> /* If it's too soon to do another transaction, wait */
> @@ -544,6 +546,8 @@ static int do_cros_ec_cmd_xfer_spi(struct cros_ec_device *ec_dev,
> unsigned long delay;
>
> len = cros_ec_prepare_tx(ec_dev, ec_msg);
> + if (len < 0)
> + return len;
> dev_dbg(ec_dev->dev, "prepared, len=%d\n", len);
>
> /* If it's too soon to do another transaction, wait */
> --
> 2.36.0.512.ge40c2bad7a-goog
>
On Thu, May 12, 2022 at 1:36 AM Tzung-Bi Shih <[email protected]> wrote:
>
> prepare_packet() gets called if `ec_dev->proto_version` > 2. For now, it
> must be equivalent to EC_HOST_REQUEST_VERSION.
>
> Drop the BUG_ON().
>
> Signed-off-by: Tzung-Bi Shih <[email protected]>
An alternative would be to return -EPROTO, but given the context this
is just a waste of code space.
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/platform/chrome/cros_ec_proto.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index ac1419881ff3..db1c8ba43171 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -60,7 +60,6 @@ static int prepare_packet(struct cros_ec_device *ec_dev,
> int i;
> u8 csum = 0;
>
> - BUG_ON(ec_dev->proto_version != EC_HOST_REQUEST_VERSION);
> BUG_ON(msg->outsize + sizeof(*request) > ec_dev->dout_size);
>
> out = ec_dev->dout;
> --
> 2.36.0.512.ge40c2bad7a-goog
>