lkml.org 
[lkml]   [2020]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 2/2] platform/chrome: cros_ec_proto: check for missing EC_CMD_HOST_EVENT_GET_WAKE_MASK
From
Date
Hi Brian,

Thank you for your patch, I'll take a look soon but I'd like to ask if you can
join the discussion with this patchset [1], specially this one [2]. We're trying
to match EC errors with standard linux kernel errors because we think can be
helpful.

[1] https://lore.kernel.org/patchwork/cover/1276734/
[2] https://lore.kernel.org/patchwork/patch/1276738/

Thanks,
Enric

On 22/7/20 3:57, Brian Norris wrote:
> As with cros_ec_cmd_xfer_status(), etc., it's not enough to simply check
> for the return status of send_command() -- that only covers transport or
> other similarly-fatal errors. One must also check the ->result field, to
> see whether the command really succeeded. If not, we can't use the data
> it returns.
>
> The caller of cros_ec_get_host_event_wake_mask() ignores this, and so
> for example, on EC's where the command is not implemented, we're using
> junk (or in practice, all zeros) for our wake-mask. We should be using a
> non-zero default (currently, it's supposed to be all-1's).
>
> Fix this by checking the ->result field and returning -EPROTO for
> errors.
>
> I might label this as fixing commit 29d99b966d60 ("cros_ec: Don't signal
> wake event for non-wake host events"), except that this fix alone
> actually may make things worse, as it now allows for a lot more spurious
> wakeups. The patch "platform/chrome: cros_ec_proto: ignore battery/AC
> wakeups on old ECs" helps to mitigate this.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> drivers/platform/chrome/cros_ec_proto.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index e93024b55ce8..01a74abe4191 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -208,6 +208,12 @@ static int cros_ec_get_host_event_wake_mask(struct cros_ec_device *ec_dev,
> msg->insize = sizeof(*r);
>
> ret = send_command(ec_dev, msg);
> + if (ret >= 0) {
> + if (msg->result == EC_RES_INVALID_COMMAND)
> + return -ENOTSUPP;
> + if (msg->result != EC_RES_SUCCESS)
> + return -EPROTO;
> + }
> if (ret > 0) {
> r = (struct ec_response_host_event_mask *)msg->data;
> *mask = r->mask;
> @@ -488,6 +494,13 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev)
> BIT(EC_HOST_EVENT_BATTERY_CRITICAL) |
> BIT(EC_HOST_EVENT_PD_MCU) |
> BIT(EC_HOST_EVENT_BATTERY_STATUS));
> + /*
> + * Old ECs may not support this command. Complain about all
> + * other errors.
> + */
> + if (ret != -ENOTSUPP)
> + dev_err(ec_dev->dev,
> + "failed to retrieve wake mask: %d\n", ret);
> }
>
> ret = 0;
>

\
 
 \ /
  Last update: 2020-07-22 12:20    [W:0.057 / U:0.728 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site