| Message ID | 20220628024913.1755292-10-tzungbi@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | platform/chrome: Kunit tests and refactor for cros_ec_cmd_xfer() | expand |
On Mon, Jun 27, 2022 at 7:49 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote: > > cros_ec_wait_until_complete() checks `msg->result` for > EC_CMD_GET_COMMS_STATUS. However, it doesn't return standard error codes > like most of others. The callers of cros_ec_send_command() do the mapping. I am not sure if it is a good idea to change that; it may have undesired side effects (such as changing the userspace ABI) for callers of cros_ec_send_command() not expecting this change. It would also result in double mapping in some situations. Guenter > > Use cros_ec_map_error() to align them. > > Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org> > --- > drivers/platform/chrome/cros_ec_proto.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > index 49772a4c5353..5323edddb540 100644 > --- a/drivers/platform/chrome/cros_ec_proto.c > +++ b/drivers/platform/chrome/cros_ec_proto.c > @@ -138,7 +138,7 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t * > { > struct cros_ec_command *msg; > struct ec_response_get_comms_status *status; > - int ret = 0, i; > + int ret = 0, i, mapped; > > msg = kzalloc(sizeof(*msg) + sizeof(*status), GFP_KERNEL); > if (!msg) > @@ -160,8 +160,11 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t * > break; > > *result = msg->result; > - if (msg->result != EC_RES_SUCCESS) > + mapped = cros_ec_map_error(msg->result); > + if (mapped) { > + ret = mapped; > break; > + } > > if (!(status->flags & EC_COMMS_STATUS_PROCESSING)) > break; > -- > 2.37.0.rc0.161.g10f37bed90-goog >
On Wed, Jul 13, 2022 at 11:23:58AM -0700, Guenter Roeck wrote: > On Mon, Jun 27, 2022 at 7:49 PM Tzung-Bi Shih <tzungbi@kernel.org> wrote: > > > > cros_ec_wait_until_complete() checks `msg->result` for > > EC_CMD_GET_COMMS_STATUS. However, it doesn't return standard error codes > > like most of others. > > The callers of cros_ec_send_command() do the mapping. I am not sure if > it is a good idea to change that; it may have undesired side effects > (such as changing the userspace ABI) for callers of > cros_ec_send_command() not expecting this change. It would also result > in double mapping in some situations. Agreed. Let's drop the change.
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 49772a4c5353..5323edddb540 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -138,7 +138,7 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t * { struct cros_ec_command *msg; struct ec_response_get_comms_status *status; - int ret = 0, i; + int ret = 0, i, mapped; msg = kzalloc(sizeof(*msg) + sizeof(*status), GFP_KERNEL); if (!msg) @@ -160,8 +160,11 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t * break; *result = msg->result; - if (msg->result != EC_RES_SUCCESS) + mapped = cros_ec_map_error(msg->result); + if (mapped) { + ret = mapped; break; + } if (!(status->flags & EC_COMMS_STATUS_PROCESSING)) break;
cros_ec_wait_until_complete() checks `msg->result` for EC_CMD_GET_COMMS_STATUS. However, it doesn't return standard error codes like most of others. Use cros_ec_map_error() to align them. Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org> --- drivers/platform/chrome/cros_ec_proto.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)