[PATCH 1/1] cros_ec: Handling EC_CMD_GET_NEXT_EVENT

[Simon Glass]

Hi,

On Mon, 9 Nov 2020 at 14:25, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 11/9/20 10:13 PM, Alper Nebi Yasak wrote:
> > On 09/11/2020 23:34, Heinrich Schuchardt wrote:
> >> With commit 690079767803 ("cros_ec: Support keyboard scanning with
> >> EC_CMD_GET_NEXT_EVENT") check_for_keys() tries to read keyboard
> >> strokes using EC_CMD_GET_NEXT_EVENT. But the sandbox driver does
> >> not understand this command. We need to reply with
> >> -EC_RES_INVALID_COMMAND to force check_for_keys() to fall back to
> >> use EC_CMD_MKBP_STATE. Currently the driver prints
> >>
> >>     ** Unknown EC command 0x67
> >>
> >> in this case. With the patch the message is suppressed.
> >>
> >> In a future patch we should upgrade the sandbox driver to provide
> >> EC_CMD_GET_NEXT_EVENT support.
> >>
> >> Fixes: 690079767803 ("cros_ec: Support keyboard scanning with EC_CMD_GET_NEXT_EVENT")
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >> ---
> >> process_cmd() should always return an appropriate negative enum ec_status
> >> in case of an error and not simply -1. But fixing the return values is
> >> beyond the scope of this patch.
> >
> > (Looks to me like -1 is already == -EC_RES_INVALID_COMMAND from
> > include/ec_commands.h definitions, but I'd agree the latter form should
> > be preferred.)
>
> If you look at the complete function, you will find other "return -1;"
> statements where return codes other than -EC_RES_INVALID_COMMAND make
> more sense. E.g. after
>
>     printf("** Unknown flash region %d\n", req->region);
>
> it would be reasonable to return EC_RES_INVALID_PARAM.
>
> Best regards
>
> Heinrich
>
> >
> >> ---
> >>  drivers/misc/cros_ec_sandbox.c | 10 ++++++++++
> >>  1 file changed, 10 insertions(+)

Shall we take Alper's patch to implement this command?

Regards,
Simon