Re: [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
On Tue, 10 Nov 2020 at 07:00, Alper Nebi Yasak wrote: > > Since commit 690079767803 ("cros_ec: Support keyboard scanning with > EC_CMD_GET_NEXT_EVENT") the cros-ec-keyb driver has started using this > command, but the sandbox EC emulator does not recognize it and > continuously prints: > > ** Unknown EC command 0x67 > > This patch makes the sandbox driver send basic responses to the command, > but the response only supports keyboard scans for now. > > Fixes: 690079767803 ("cros_ec: Support keyboard scanning with > EC_CMD_GET_NEXT_EVENT") > Reported-by: Heinrich Schuchardt > Signed-off-by: Alper Nebi Yasak > --- > This doesn't test the -EC_RES_UNAVAILABLE part, which looks like an > event queue on the EC side. As far as I understand sandbox is getting a > single keyboard state from SDL after discarding pending events. I think > things would need to be hooked into sandbox_sdl_poll_events, looks > possible but harder. > > Also, now that this command would work, the fallback to the old one > would never trigger and wouldn't be tested. > > drivers/misc/cros_ec_sandbox.c | 8 > 1 file changed, 8 insertions(+) > Reviewed-by: Simon Glass > diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c > index a191f061b898..d72db3eace98 100644 > --- a/drivers/misc/cros_ec_sandbox.c > +++ b/drivers/misc/cros_ec_sandbox.c > @@ -460,6 +460,14 @@ static int process_cmd(struct ec_state *ec, > 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; > -- > 2.29.2 >
Re: [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
On 10/11/2020 20:08, Heinrich Schuchardt wrote: > On 10.11.20 15:00, Alper Nebi Yasak wrote: >> Since commit 690079767803 ("cros_ec: Support keyboard scanning with >> EC_CMD_GET_NEXT_EVENT") the cros-ec-keyb driver has started using this >> command, but the sandbox EC emulator does not recognize it and >> continuously prints: >> >> ** Unknown EC command 0x67 >> >> This patch makes the sandbox driver send basic responses to the command, >> but the response only supports keyboard scans for now. >> >> Fixes: 690079767803 ("cros_ec: Support keyboard scanning with >> EC_CMD_GET_NEXT_EVENT") >> Reported-by: Heinrich Schuchardt >> Signed-off-by: Alper Nebi Yasak >> --- >> This doesn't test the -EC_RES_UNAVAILABLE part, which looks like an >> event queue on the EC side. As far as I understand sandbox is getting a >> single keyboard state from SDL after discarding pending events. I think >> things would need to be hooked into sandbox_sdl_poll_events, looks >> possible but harder. >> >> Also, now that this command would work, the fallback to the old one >> would never trigger and wouldn't be tested. > > Should some of the comment above be merged into the commit message? Yeah, probably. I just defaulted to the notes since I'd be writing in an informal tone and was not sure if leaving those as future work is OK. > Tested-by: Heinrich Schuchardt Thanks. >> >> drivers/misc/cros_ec_sandbox.c | 8 >> 1 file changed, 8 insertions(+) >> >> diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c >> index a191f061b898..d72db3eace98 100644 >> --- a/drivers/misc/cros_ec_sandbox.c >> +++ b/drivers/misc/cros_ec_sandbox.c >> @@ -460,6 +460,14 @@ static int process_cmd(struct ec_state *ec, >> 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; >> >
Re: [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
On 10.11.20 15:00, Alper Nebi Yasak wrote: > Since commit 690079767803 ("cros_ec: Support keyboard scanning with > EC_CMD_GET_NEXT_EVENT") the cros-ec-keyb driver has started using this > command, but the sandbox EC emulator does not recognize it and > continuously prints: > > ** Unknown EC command 0x67 > > This patch makes the sandbox driver send basic responses to the command, > but the response only supports keyboard scans for now. > > Fixes: 690079767803 ("cros_ec: Support keyboard scanning with > EC_CMD_GET_NEXT_EVENT") > Reported-by: Heinrich Schuchardt > Signed-off-by: Alper Nebi Yasak > --- > This doesn't test the -EC_RES_UNAVAILABLE part, which looks like an > event queue on the EC side. As far as I understand sandbox is getting a > single keyboard state from SDL after discarding pending events. I think > things would need to be hooked into sandbox_sdl_poll_events, looks > possible but harder. > > Also, now that this command would work, the fallback to the old one > would never trigger and wouldn't be tested. Should some of the comment above be merged into the commit message? Tested-by: Heinrich Schuchardt > > drivers/misc/cros_ec_sandbox.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c > index a191f061b898..d72db3eace98 100644 > --- a/drivers/misc/cros_ec_sandbox.c > +++ b/drivers/misc/cros_ec_sandbox.c > @@ -460,6 +460,14 @@ static int process_cmd(struct ec_state *ec, > 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; >