* [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
@ 2020-11-10 14:00 Alper Nebi Yasak
2020-11-10 17:08 ` Heinrich Schuchardt
2020-11-11 14:32 ` Simon Glass
0 siblings, 2 replies; 4+ messages in thread
From: Alper Nebi Yasak @ 2020-11-10 14:00 UTC (permalink / raw)
To: u-boot
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 <xypron.glpk@gmx.de>
Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
---
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(+)
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
2020-11-10 14:00 [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT Alper Nebi Yasak
@ 2020-11-10 17:08 ` Heinrich Schuchardt
2020-11-10 23:18 ` Alper Nebi Yasak
2020-11-11 14:32 ` Simon Glass
1 sibling, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2020-11-10 17:08 UTC (permalink / raw)
To: u-boot
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 <xypron.glpk@gmx.de>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> ---
> 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 <xypron.glpk@gmx.de>
>
> 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;
>
^ permalink raw reply [flat|nested] 4+ messages in thread* [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
2020-11-10 17:08 ` Heinrich Schuchardt
@ 2020-11-10 23:18 ` Alper Nebi Yasak
0 siblings, 0 replies; 4+ messages in thread
From: Alper Nebi Yasak @ 2020-11-10 23:18 UTC (permalink / raw)
To: u-boot
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 <xypron.glpk@gmx.de>
>> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>> ---
>> 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 <xypron.glpk@gmx.de>
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;
>>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT
2020-11-10 14:00 [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT Alper Nebi Yasak
2020-11-10 17:08 ` Heinrich Schuchardt
@ 2020-11-11 14:32 ` Simon Glass
1 sibling, 0 replies; 4+ messages in thread
From: Simon Glass @ 2020-11-11 14:32 UTC (permalink / raw)
To: u-boot
On Tue, 10 Nov 2020 at 07:00, Alper Nebi Yasak <alpernebiyasak@gmail.com> 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 <xypron.glpk@gmx.de>
> Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> ---
> 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 <sjg@chromium.org>
> 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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-11 14:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-10 14:00 [PATCH] sandbox: cros_ec: Basic support for EC_CMD_GET_NEXT_EVENT Alper Nebi Yasak
2020-11-10 17:08 ` Heinrich Schuchardt
2020-11-10 23:18 ` Alper Nebi Yasak
2020-11-11 14:32 ` Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox