[PATCH] cros_ec: Support keyboard scanning with EC_CMD_GET_NEXT_EVENT

[Simon Glass]

Hi Alper,

On Mon, 9 Nov 2020 at 17:55, Alper Nebi Yasak <alpernebiyasak at gmail.com> wrote:
>
> On 09/11/2020 22:37, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 9 Nov 2020 at 12:34, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> On 10/30/20 6:25 PM, Alper Nebi Yasak wrote:
> >>> The cros_ec_keyb driver currently uses EC_CMD_MKBP_STATE to scan the
> >>> keyboard, but this host command was superseded by EC_CMD_GET_NEXT_EVENT
> >>
> >> This patch has been applied to origin/master.
> >>
> >> Now when I compile sandbox_defconfig and run './u-boot -D' it spits out
> >> zillions of
> >>
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>    ** Unknown EC command 0x67
> >>
> >> When I revert the patch the messages are gone.
> >>
> >> So something is really missing in this patch.
> >
> > Test coverage, for a start!
> >
> > I think the EC emulator needs to be updated for the new command.
> >
> > Alper, can you please take a look?
> >
>
> I can get the messages to stop with the following, does it look good to
> you?
>
>         case EC_CMD_ENTERING_MODE:
>                 len = 0;
>                 break;
> +       case EC_CMD_GET_NEXT_EVENT: {
> +               struct ec_response_get_next_event *resp = resp_data;
> +               resp->event_type = EC_MKBP_EVENT_KEY_MATRIX;
> +               cros_ec_keyscan(ec, resp->data.key_matrix);
> +               len = sizeof(*resp);
> +               break;
> +       }
>         default:
>                 printf("   ** Unknown EC command %#02x\n", req_hdr->command);
>                 return -1;
>
> That's more or less only what the cros-ec-keyb counterpart expects. But
> it doesn't test the -EC_RES_UNAVAILABLE thing or the fallback to the old
> method.

Yes that looks good. We don't need to test the fallback for now as
this is just an emulator anyway.

Regards,
Simon