[PATCH 1/1] cros_ec: Handling EC_CMD_GET_NEXT_EVENT

[Alper Nebi Yasak]

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.)

> ---
>  drivers/misc/cros_ec_sandbox.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c
> index a191f061b8..ff7f782742 100644
> --- a/drivers/misc/cros_ec_sandbox.c
> +++ b/drivers/misc/cros_ec_sandbox.c
> @@ -460,6 +460,16 @@ static int process_cmd(struct ec_state *ec,
>  	case EC_CMD_ENTERING_MODE:
>  		len = 0;
>  		break;
> +	case EC_CMD_GET_NEXT_EVENT:
> +		/*
> +		 * TODO:
> +		 * This driver emulates an old keyboard device supporting
> +		 * EC_CMD_MKBP_STATE. Current Chrome OS keyboards use
> +		 * EC_CMD_GET_NEXT_EVENT. Cf.
> +		 * "mkbp: Add support for buttons and switches"
> +		 * https://chromium.googlesource.com/chromiumos/platform/ec/+/87a071941b89e3f7fd3eb329b682e60b3fbd6c73
> +		 */
> +		return -EC_RES_INVALID_COMMAND;

I'll try implementing the TODO, sorry for the fallout.

>  	default:
>  		printf("   ** Unknown EC command %#02x\n", req_hdr->command);
>  		return -1;
> --
> 2.28.0
>