* [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 @ 2018-09-27 9:24 Emil Karlson 2018-09-27 9:30 ` Emil Karlson ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Emil Karlson @ 2018-09-27 9:24 UTC (permalink / raw) To: Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, linux-kernel Cc: Emil Karlson Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events be truncated on many chromebooks so that Left and Right keys on Column 12 were always 0. This commit fixes the issue by restoring the old semantics when the protocol version is 0. --- drivers/platform/chrome/cros_ec_proto.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 398393ab5df8..457e4940dba4 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -519,8 +519,14 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, ret = cros_ec_cmd_xfer(ec_dev, msg); if (ret > 0) { + unsigned int copy_size; + ec_dev->event_size = ret - 1; - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); + if (!version) + copy_size = sizeof(struct ec_response_get_next_event); + else + copy_size = ec_dev->event_size; + memcpy(&ec_dev->event_data, msg->data, copy_size); } return ret; -- 2.19.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 2018-09-27 9:24 [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Karlson @ 2018-09-27 9:30 ` Emil Karlson 2018-09-28 12:15 ` Neil Armstrong 2018-10-09 10:13 ` Lee Jones 2 siblings, 0 replies; 12+ messages in thread From: Emil Karlson @ 2018-09-27 9:30 UTC (permalink / raw) To: Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, linux-kernel To note I have almost no idea what I am doing and ended up wondering, whether the messages are always truncated and it only shows on version 0, because the messages are not padded to longer lenght. Alternatively the ret could possibly be used as copy length without the -1, right now I am unable to test with version 1, possibly it requires update of cros-ec firmware. Best regards -Emil On Thu, Sep 27, 2018 at 12:24 PM Emil Karlson <jekarlson@gmail.com> wrote: > > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > be truncated on many chromebooks so that Left and Right keys on Column 12 were > always 0. This commit fixes the issue by restoring the old semantics when the > protocol version is 0. > --- > drivers/platform/chrome/cros_ec_proto.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > index 398393ab5df8..457e4940dba4 100644 > --- a/drivers/platform/chrome/cros_ec_proto.c > +++ b/drivers/platform/chrome/cros_ec_proto.c > @@ -519,8 +519,14 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, > > ret = cros_ec_cmd_xfer(ec_dev, msg); > if (ret > 0) { > + unsigned int copy_size; > + > ec_dev->event_size = ret - 1; > - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); > + if (!version) > + copy_size = sizeof(struct ec_response_get_next_event); > + else > + copy_size = ec_dev->event_size; > + memcpy(&ec_dev->event_data, msg->data, copy_size); > } > > return ret; > -- > 2.19.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 2018-09-27 9:24 [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Karlson 2018-09-27 9:30 ` Emil Karlson @ 2018-09-28 12:15 ` Neil Armstrong 2018-09-28 17:08 ` [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer Emil Karlson 2018-09-28 20:27 ` [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Renner Berthing 2018-10-09 10:13 ` Lee Jones 2 siblings, 2 replies; 12+ messages in thread From: Neil Armstrong @ 2018-09-28 12:15 UTC (permalink / raw) To: Emil Karlson, Emil Renner Berthing, Stefan Adolfsson, linux-kernel, lee.jones, bleung, olof, Enric Balletbo i Serra Hi All, On 27/09/2018 11:24, Emil Karlson wrote: > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > be truncated on many chromebooks so that Left and Right keys on Column 12 were > always 0. This commit fixes the issue by restoring the old semantics when the > protocol version is 0. OK for me, can someone confirms it fixes the issue ?? Thanks, Neil > --- > drivers/platform/chrome/cros_ec_proto.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > index 398393ab5df8..457e4940dba4 100644 > --- a/drivers/platform/chrome/cros_ec_proto.c > +++ b/drivers/platform/chrome/cros_ec_proto.c > @@ -519,8 +519,14 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, > > ret = cros_ec_cmd_xfer(ec_dev, msg); > if (ret > 0) { > + unsigned int copy_size; > + > ec_dev->event_size = ret - 1; > - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); > + if (!version) > + copy_size = sizeof(struct ec_response_get_next_event); > + else > + copy_size = ec_dev->event_size; > + memcpy(&ec_dev->event_data, msg->data, copy_size); > } > > return ret; > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-09-28 12:15 ` Neil Armstrong @ 2018-09-28 17:08 ` Emil Karlson 2018-09-28 17:16 ` Emil Karlson 2018-10-03 11:01 ` Enric Balletbo i Serra 2018-09-28 20:27 ` [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Renner Berthing 1 sibling, 2 replies; 12+ messages in thread From: Emil Karlson @ 2018-09-28 17:08 UTC (permalink / raw) To: Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, lee.jones, bleung, olof, Enric Balletbo i Serra, linux-kernel Cc: Emil Karlson Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events be truncated on many chromebooks so that Left and Right keys on Column 12 were always 0. Use ret as memcpy len to fix this. drivers/platform/chrome/cros_ec_proto.c:509 get_next_event_xfer uses ret from cros_ec_cmd_xfer for memcpy for msg->data len drivers/platform/chrome/cros_ec_proto.c:445 cros_ec_cmd_xfer gets ret from send_command drivers/platform/chrome/cros_ec_proto.c:93 send_command gets ret from bus specific xfer_fn drivers/mfd/cros_ec_spi.c:598 cros_ec_cmd_xfer_spi copies len amount to ec_msg->data and returns len as ret drivers/mfd/cros_ec_i2c.c:267 cros_ec_cmd_xfer_i2c copies len amount to ec_msg->data and returns len as ret so msg->data length is always the same as ret. Fixes: 57e94c8b974d ("mfd: cros-ec: Increase maximum mkbp event size") Signed-off-by: Emil Karlson <jekarlson@gmail.com> --- drivers/platform/chrome/cros_ec_proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 398393ab5df8..b6fd4838f60f 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -520,7 +520,7 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, ret = cros_ec_cmd_xfer(ec_dev, msg); if (ret > 0) { ec_dev->event_size = ret - 1; - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); + memcpy(&ec_dev->event_data, msg->data, ret); } return ret; -- 2.19.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-09-28 17:08 ` [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer Emil Karlson @ 2018-09-28 17:16 ` Emil Karlson 2018-10-03 11:01 ` Enric Balletbo i Serra 1 sibling, 0 replies; 12+ messages in thread From: Emil Karlson @ 2018-09-28 17:16 UTC (permalink / raw) To: Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, Lee Jones, bleung, olof, Enric Balletbo i Serra, linux-kernel I failed in In-Reply-To: <22cc40a7-015b-6038-2093-8cd1ff0c807e@baylibre.com> somehow. I was urged to go for correct rather than conservative fix on IRC. I have tested this on my message version 0 Kevin and it fixes the original problem for me, but someone should perhaps test this on newer system that has message version 1. Background work documented in commit message. Best regards -Emil On Fri, Sep 28, 2018 at 8:08 PM Emil Karlson <jekarlson@gmail.com> wrote: > > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > be truncated on many chromebooks so that Left and Right keys on Column 12 were > always 0. Use ret as memcpy len to fix this. > > drivers/platform/chrome/cros_ec_proto.c:509 > get_next_event_xfer uses ret from cros_ec_cmd_xfer for memcpy for msg->data len > drivers/platform/chrome/cros_ec_proto.c:445 > cros_ec_cmd_xfer gets ret from send_command > drivers/platform/chrome/cros_ec_proto.c:93 > send_command gets ret from bus specific xfer_fn > drivers/mfd/cros_ec_spi.c:598 > cros_ec_cmd_xfer_spi copies len amount to ec_msg->data and returns len as ret > drivers/mfd/cros_ec_i2c.c:267 > cros_ec_cmd_xfer_i2c copies len amount to ec_msg->data and returns len as ret > > so msg->data length is always the same as ret. > > Fixes: 57e94c8b974d ("mfd: cros-ec: Increase maximum mkbp event size") > Signed-off-by: Emil Karlson <jekarlson@gmail.com> > --- > drivers/platform/chrome/cros_ec_proto.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > index 398393ab5df8..b6fd4838f60f 100644 > --- a/drivers/platform/chrome/cros_ec_proto.c > +++ b/drivers/platform/chrome/cros_ec_proto.c > @@ -520,7 +520,7 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, > ret = cros_ec_cmd_xfer(ec_dev, msg); > if (ret > 0) { > ec_dev->event_size = ret - 1; > - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); > + memcpy(&ec_dev->event_data, msg->data, ret); > } > > return ret; > -- > 2.19.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-09-28 17:08 ` [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer Emil Karlson 2018-09-28 17:16 ` Emil Karlson @ 2018-10-03 11:01 ` Enric Balletbo i Serra 2018-10-03 13:41 ` Neil Armstrong 2018-10-10 3:44 ` [PATCH] " Benson Leung 1 sibling, 2 replies; 12+ messages in thread From: Enric Balletbo i Serra @ 2018-10-03 11:01 UTC (permalink / raw) To: Emil Karlson, Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, lee.jones, bleung, olof, linux-kernel Hi Emil, Many thanks to catch this and fix. Some comments below. You missed to add the v2, please send the next patch with v3 prefix. On 28/9/18 19:08, Emil Karlson wrote: > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > be truncated on many chromebooks so that Left and Right keys on Column 12 were > always 0. Use ret as memcpy len to fix this. > That's fine > drivers/platform/chrome/cros_ec_proto.c:509 > get_next_event_xfer uses ret from cros_ec_cmd_xfer for memcpy for msg->data len > drivers/platform/chrome/cros_ec_proto.c:445 > cros_ec_cmd_xfer gets ret from send_command > drivers/platform/chrome/cros_ec_proto.c:93 > send_command gets ret from bus specific xfer_fn > drivers/mfd/cros_ec_spi.c:598 > cros_ec_cmd_xfer_spi copies len amount to ec_msg->data and returns len as ret > drivers/mfd/cros_ec_i2c.c:267 > cros_ec_cmd_xfer_i2c copies len amount to ec_msg->data and returns len as ret > > so msg->data length is always the same as ret. > Instead of describe the different calls involved and the returns. I'd explain why using ret fixes the issue. > Fixes: 57e94c8b974d ("mfd: cros-ec: Increase maximum mkbp event size") > Signed-off-by: Emil Karlson <jekarlson@gmail.com> > --- > drivers/platform/chrome/cros_ec_proto.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > index 398393ab5df8..b6fd4838f60f 100644 > --- a/drivers/platform/chrome/cros_ec_proto.c > +++ b/drivers/platform/chrome/cros_ec_proto.c > @@ -520,7 +520,7 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, > ret = cros_ec_cmd_xfer(ec_dev, msg); > if (ret > 0) { > ec_dev->event_size = ret - 1; > - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); > + memcpy(&ec_dev->event_data, msg->data, ret); > } > > return ret; > After thinking a bit more on this I think that how you fixed this is really clear. I was wondering if the downstream solution would be better but as is really late and will be good have this as urgent fix for the coming release I am happy with it. We can always send follow up patches to sync with the downstream version if is preferred. Neil, can you give us your Tested-by to have the make sure this doesn't break with protocol v1, I don't have such hardware. Benson, will be really good have this merged in this rc. Are you fine with this solution? BTW, you can add my in next version. Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-10-03 11:01 ` Enric Balletbo i Serra @ 2018-10-03 13:41 ` Neil Armstrong 2018-10-03 18:43 ` [PATCH v3] " Emil Karlson 2018-10-10 3:44 ` [PATCH] " Benson Leung 1 sibling, 1 reply; 12+ messages in thread From: Neil Armstrong @ 2018-10-03 13:41 UTC (permalink / raw) To: Enric Balletbo i Serra, Emil Karlson, Emil Renner Berthing, Stefan Adolfsson, lee.jones, bleung, olof, linux-kernel Hi Enric, On 03/10/2018 13:01, Enric Balletbo i Serra wrote: > Hi Emil, > > Many thanks to catch this and fix. Some comments below. > > You missed to add the v2, please send the next patch with v3 prefix. > > On 28/9/18 19:08, Emil Karlson wrote: >> Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events >> be truncated on many chromebooks so that Left and Right keys on Column 12 were >> always 0. Use ret as memcpy len to fix this. >> [...] > > Neil, can you give us your Tested-by to have the make sure this doesn't break > with protocol v1, I don't have such hardware. I'd like to, but I'm unable to get developer mode and legacy boot back on my TEEMO device... Neil > > Benson, will be really good have this merged in this rc. Are you fine with this > solution? > > BTW, you can add my in next version. > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-10-03 13:41 ` Neil Armstrong @ 2018-10-03 18:43 ` Emil Karlson 2018-10-10 5:06 ` Benson Leung 0 siblings, 1 reply; 12+ messages in thread From: Emil Karlson @ 2018-10-03 18:43 UTC (permalink / raw) To: Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, lee.jones, bleung, olof, Enric Balletbo i Serra, linux-kernel Cc: Emil Karlson Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events be truncated on many chromebooks so that Left and Right keys on Column 12 were always 0. Use ret as memcpy len to fix this. The old code was using ec_dev->event_size, which is the event payload/data size excluding event_type header, for the length of the memcpy operation. Use ret as memcpy length to avoid the off by one and copy the whole msg->data. Fixes: 57e94c8b974d ("mfd: cros-ec: Increase maximum mkbp event size") Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Tested-by: Emil Renner Berthing <kernel@esmil.dk> Signed-off-by: Emil Karlson <jekarlson@gmail.com> --- Notes: Changes in v3 - Improve commit message. Changes in v2 - Fix the off by one in memcpy parameter rather than restoring the old behaviour for v0 messages. drivers/platform/chrome/cros_ec_proto.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c index 398393ab5df8..b6fd4838f60f 100644 --- a/drivers/platform/chrome/cros_ec_proto.c +++ b/drivers/platform/chrome/cros_ec_proto.c @@ -520,7 +520,7 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, ret = cros_ec_cmd_xfer(ec_dev, msg); if (ret > 0) { ec_dev->event_size = ret - 1; - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); + memcpy(&ec_dev->event_data, msg->data, ret); } return ret; -- 2.19.0 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v3] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-10-03 18:43 ` [PATCH v3] " Emil Karlson @ 2018-10-10 5:06 ` Benson Leung 0 siblings, 0 replies; 12+ messages in thread From: Benson Leung @ 2018-10-10 5:06 UTC (permalink / raw) To: Emil Karlson Cc: kernel, Neil Armstrong, Stefan Adolfsson, Lee Jones, Olof Johansson, Enric Balletbo i Serra, Linux Kernel Hi Emil, On Wed, Oct 3, 2018 at 11:43 AM Emil Karlson <jekarlson@gmail.com> wrote: > > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > be truncated on many chromebooks so that Left and Right keys on Column 12 were > always 0. Use ret as memcpy len to fix this. > > The old code was using ec_dev->event_size, which is the event payload/data size > excluding event_type header, for the length of the memcpy operation. Use ret > as memcpy length to avoid the off by one and copy the whole msg->data. > > Fixes: 57e94c8b974d ("mfd: cros-ec: Increase maximum mkbp event size") > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Tested-by: Emil Renner Berthing <kernel@esmil.dk> > Signed-off-by: Emil Karlson <jekarlson@gmail.com> Applied. Sent to Greg for rc8, hopefully. -- Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer 2018-10-03 11:01 ` Enric Balletbo i Serra 2018-10-03 13:41 ` Neil Armstrong @ 2018-10-10 3:44 ` Benson Leung 1 sibling, 0 replies; 12+ messages in thread From: Benson Leung @ 2018-10-10 3:44 UTC (permalink / raw) To: Enric Balletbo i Serra Cc: jekarlson, kernel, Neil Armstrong, Stefan Adolfsson, Lee Jones, Olof Johansson, Linux Kernel Hi Enric, On Wed, Oct 3, 2018 at 4:01 AM Enric Balletbo i Serra <enric.balletbo@collabora.com> wrote: > > Hi Emil, > > Many thanks to catch this and fix. Some comments below. > > You missed to add the v2, please send the next patch with v3 prefix. > > On 28/9/18 19:08, Emil Karlson wrote: > > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > > be truncated on many chromebooks so that Left and Right keys on Column 12 were > > always 0. Use ret as memcpy len to fix this. > > > > That's fine > > > drivers/platform/chrome/cros_ec_proto.c:509 > > get_next_event_xfer uses ret from cros_ec_cmd_xfer for memcpy for msg->data len > > drivers/platform/chrome/cros_ec_proto.c:445 > > cros_ec_cmd_xfer gets ret from send_command > > drivers/platform/chrome/cros_ec_proto.c:93 > > send_command gets ret from bus specific xfer_fn > > drivers/mfd/cros_ec_spi.c:598 > > cros_ec_cmd_xfer_spi copies len amount to ec_msg->data and returns len as ret > > drivers/mfd/cros_ec_i2c.c:267 > > cros_ec_cmd_xfer_i2c copies len amount to ec_msg->data and returns len as ret > > > > so msg->data length is always the same as ret. > > > > Instead of describe the different calls involved and the returns. I'd explain > why using ret fixes the issue. > > > Fixes: 57e94c8b974d ("mfd: cros-ec: Increase maximum mkbp event size") > > Signed-off-by: Emil Karlson <jekarlson@gmail.com> > > --- > > drivers/platform/chrome/cros_ec_proto.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > > index 398393ab5df8..b6fd4838f60f 100644 > > --- a/drivers/platform/chrome/cros_ec_proto.c > > +++ b/drivers/platform/chrome/cros_ec_proto.c > > @@ -520,7 +520,7 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, > > ret = cros_ec_cmd_xfer(ec_dev, msg); > > if (ret > 0) { > > ec_dev->event_size = ret - 1; > > - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); > > + memcpy(&ec_dev->event_data, msg->data, ret); > > } > > > > return ret; > > > > After thinking a bit more on this I think that how you fixed this is really > clear. I was wondering if the downstream solution would be better but as is > really late and will be good have this as urgent fix for the coming release I am > happy with it. We can always send follow up patches to sync with the downstream > version if is preferred. > > Neil, can you give us your Tested-by to have the make sure this doesn't break > with protocol v1, I don't have such hardware. > > Benson, will be really good have this merged in this rc. Are you fine with this > solution? Yes, I'm fine with it. Let me get this ready and try to get it before we run out of rcs of 4.19. > > BTW, you can add my in next version. > > Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> -- Benson Leung Staff Software Engineer Chrome OS Kernel Google Inc. bleung@google.com Chromium OS Project bleung@chromium.org ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 2018-09-28 12:15 ` Neil Armstrong 2018-09-28 17:08 ` [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer Emil Karlson @ 2018-09-28 20:27 ` Emil Renner Berthing 1 sibling, 0 replies; 12+ messages in thread From: Emil Renner Berthing @ 2018-09-28 20:27 UTC (permalink / raw) To: narmstrong Cc: jekarlson, sadolfsson, Linux Kernel Mailing List, lee.jones, bleung, olof, Enric Balletbo i Serra On Fri, 28 Sep 2018 at 14:15, Neil Armstrong <narmstrong@baylibre.com> wrote: > On 27/09/2018 11:24, Emil Karlson wrote: > > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > > be truncated on many chromebooks so that Left and Right keys on Column 12 were > > always 0. This commit fixes the issue by restoring the old semantics when the > > protocol version is 0. > > OK for me, can someone confirms it fixes the issue ?? Both this patch and the updated version fixes the same issue for me on rk3399-gru-kevin. /Emil > Thanks, > Neil > > > --- > > drivers/platform/chrome/cros_ec_proto.c | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c > > index 398393ab5df8..457e4940dba4 100644 > > --- a/drivers/platform/chrome/cros_ec_proto.c > > +++ b/drivers/platform/chrome/cros_ec_proto.c > > @@ -519,8 +519,14 @@ static int get_next_event_xfer(struct cros_ec_device *ec_dev, > > > > ret = cros_ec_cmd_xfer(ec_dev, msg); > > if (ret > 0) { > > + unsigned int copy_size; > > + > > ec_dev->event_size = ret - 1; > > - memcpy(&ec_dev->event_data, msg->data, ec_dev->event_size); > > + if (!version) > > + copy_size = sizeof(struct ec_response_get_next_event); > > + else > > + copy_size = ec_dev->event_size; > > + memcpy(&ec_dev->event_data, msg->data, copy_size); > > } > > > > return ret; > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 2018-09-27 9:24 [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Karlson 2018-09-27 9:30 ` Emil Karlson 2018-09-28 12:15 ` Neil Armstrong @ 2018-10-09 10:13 ` Lee Jones 2 siblings, 0 replies; 12+ messages in thread From: Lee Jones @ 2018-10-09 10:13 UTC (permalink / raw) To: Emil Karlson Cc: Emil Renner Berthing, Neil Armstrong, Stefan Adolfsson, linux-kernel On Thu, 27 Sep 2018, Emil Karlson wrote: > Commit 57e94c8b974db2d83c60e1139c89a70806abbea0 caused cros-ec keyboard events > be truncated on many chromebooks so that Left and Right keys on Column 12 were > always 0. This commit fixes the issue by restoring the old semantics when the > protocol version is 0. > --- > drivers/platform/chrome/cros_ec_proto.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) Once you guys have all reviewed, agreed and tested the patches, can you please send a final version with all of the Acks pre-collected as a separate (not threaded/attached to this thread) submission please? -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-10-10 5:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-09-27 9:24 [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Karlson 2018-09-27 9:30 ` Emil Karlson 2018-09-28 12:15 ` Neil Armstrong 2018-09-28 17:08 ` [PATCH] mfd: cros-ec: copy the whole event in get_next_event_xfer Emil Karlson 2018-09-28 17:16 ` Emil Karlson 2018-10-03 11:01 ` Enric Balletbo i Serra 2018-10-03 13:41 ` Neil Armstrong 2018-10-03 18:43 ` [PATCH v3] " Emil Karlson 2018-10-10 5:06 ` Benson Leung 2018-10-10 3:44 ` [PATCH] " Benson Leung 2018-09-28 20:27 ` [PATCH] mfd: cros-ec: copy the whole event when msg->version is 0 Emil Renner Berthing 2018-10-09 10:13 ` Lee Jones
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).