* [PATCH v2] platform: cros_ec: Reduce ligthbar get version command
@ 2020-08-26 0:29 Gwendal Grignou
2020-08-28 22:05 ` Gwendal Grignou
2020-08-29 15:54 ` Jonathan Cameron
0 siblings, 2 replies; 5+ messages in thread
From: Gwendal Grignou @ 2020-08-26 0:29 UTC (permalink / raw)
To: bleung, enric.balletbo; +Cc: linux-iio, Gwendal Grignou
By default, the lightbar commands are set to the
biggest lightbar command and response. That length is greater than 128
bytes and may not work on all machines.
But all EC are probed for lightbar by sending a get version request.
Set that request size precisely.
Before the command would be:
cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 194, insize: 128, result: 0
Afer:
cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 1, insize: 8, result: 0
Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
Changes since v1:
- Remove BUG and TEST fields.
drivers/platform/chrome/cros_ec_lightbar.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
index b59180bff5a3e..ef61298c30bdd 100644
--- a/drivers/platform/chrome/cros_ec_lightbar.c
+++ b/drivers/platform/chrome/cros_ec_lightbar.c
@@ -116,6 +116,8 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
param = (struct ec_params_lightbar *)msg->data;
param->cmd = LIGHTBAR_CMD_VERSION;
+ msg->outsize = sizeof(param->cmd);
+ msg->result = sizeof(resp->version);
ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
if (ret < 0) {
ret = 0;
--
2.28.0.297.g1956fa8f8d-goog
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command
2020-08-26 0:29 [PATCH v2] platform: cros_ec: Reduce ligthbar get version command Gwendal Grignou
@ 2020-08-28 22:05 ` Gwendal Grignou
2020-08-29 15:54 ` Jonathan Cameron
1 sibling, 0 replies; 5+ messages in thread
From: Gwendal Grignou @ 2020-08-28 22:05 UTC (permalink / raw)
To: Benson Leung, Enric Balletbo i Serra; +Cc: linux-kernel
[-iio list][+kernel list]
On Tue, Aug 25, 2020 at 5:29 PM Gwendal Grignou <gwendal@chromium.org> wrote:
>
> By default, the lightbar commands are set to the
> biggest lightbar command and response. That length is greater than 128
> bytes and may not work on all machines.
> But all EC are probed for lightbar by sending a get version request.
> Set that request size precisely.
>
> Before the command would be:
> cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 194, insize: 128, result: 0
> Afer:
> cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 1, insize: 8, result: 0
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> Changes since v1:
> - Remove BUG and TEST fields.
>
> drivers/platform/chrome/cros_ec_lightbar.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index b59180bff5a3e..ef61298c30bdd 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -116,6 +116,8 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
>
> param = (struct ec_params_lightbar *)msg->data;
> param->cmd = LIGHTBAR_CMD_VERSION;
> + msg->outsize = sizeof(param->cmd);
> + msg->result = sizeof(resp->version);
> ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> if (ret < 0) {
> ret = 0;
> --
> 2.28.0.297.g1956fa8f8d-goog
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command
2020-08-26 0:29 [PATCH v2] platform: cros_ec: Reduce ligthbar get version command Gwendal Grignou
2020-08-28 22:05 ` Gwendal Grignou
@ 2020-08-29 15:54 ` Jonathan Cameron
2020-08-30 7:00 ` Gwendal Grignou
1 sibling, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2020-08-29 15:54 UTC (permalink / raw)
To: Gwendal Grignou; +Cc: bleung, enric.balletbo, linux-iio
On Tue, 25 Aug 2020 17:29:45 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:
> By default, the lightbar commands are set to the
> biggest lightbar command and response. That length is greater than 128
> bytes and may not work on all machines.
> But all EC are probed for lightbar by sending a get version request.
> Set that request size precisely.
>
> Before the command would be:
> cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 194, insize: 128, result: 0
> Afer:
> cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 1, insize: 8, result: 0
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
Hi Gwendal,
Description seems to me to suggest this is a fix?
Are there known machines on which it doesn't work currently?
If so, please can I have a fixes tag. If it's just a precaution
against future problems then let me know and I can add it for the
next merge window.
Thanks,
Jonathan
> ---
> Changes since v1:
> - Remove BUG and TEST fields.
>
> drivers/platform/chrome/cros_ec_lightbar.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> index b59180bff5a3e..ef61298c30bdd 100644
> --- a/drivers/platform/chrome/cros_ec_lightbar.c
> +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> @@ -116,6 +116,8 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
>
> param = (struct ec_params_lightbar *)msg->data;
> param->cmd = LIGHTBAR_CMD_VERSION;
> + msg->outsize = sizeof(param->cmd);
> + msg->result = sizeof(resp->version);
> ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> if (ret < 0) {
> ret = 0;
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command
2020-08-29 15:54 ` Jonathan Cameron
@ 2020-08-30 7:00 ` Gwendal Grignou
2020-08-30 11:11 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Gwendal Grignou @ 2020-08-30 7:00 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Benson Leung, Enric Balletbo i Serra, linux-iio, linux-kernel
On Sat, Aug 29, 2020 at 8:54 AM Jonathan Cameron <jic23@kernel.org> wrote:
>
> On Tue, 25 Aug 2020 17:29:45 -0700
> Gwendal Grignou <gwendal@chromium.org> wrote:
>
> > By default, the lightbar commands are set to the
> > biggest lightbar command and response. That length is greater than 128
> > bytes and may not work on all machines.
> > But all EC are probed for lightbar by sending a get version request.
> > Set that request size precisely.
> >
> > Before the command would be:
> > cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 194, insize: 128, result: 0
> > Afer:
> > cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 1, insize: 8, result: 0
> >
> > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> Hi Gwendal,
>
> Description seems to me to suggest this is a fix?
> Are there known machines on which it doesn't work currently?
We have a prototype [without lightbar] where the command size was
limited to 128 bytes.
Given we issue a get_lightbar_version on all chromebooks, we had a
failure on this prototype. Devices with a lightbar must support a
command size greater or equal to 194 bytes.
Beside helping the prototype to boot, this patch slightly speeds up
the enumeration of devices managed by the EC.
>
> If so, please can I have a fixes tag. If it's just a precaution
> against future problems then let me know and I can add it for the
> next merge window.
Done in v3.
Note I made a mistake by sending the patch to linux-iio as it targeted
platform/chromeos.
>
> Thanks,
>
> Jonathan
>
> > ---
> > Changes since v1:
> > - Remove BUG and TEST fields.
> >
> > drivers/platform/chrome/cros_ec_lightbar.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> > index b59180bff5a3e..ef61298c30bdd 100644
> > --- a/drivers/platform/chrome/cros_ec_lightbar.c
> > +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> > @@ -116,6 +116,8 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
> >
> > param = (struct ec_params_lightbar *)msg->data;
> > param->cmd = LIGHTBAR_CMD_VERSION;
> > + msg->outsize = sizeof(param->cmd);
> > + msg->result = sizeof(resp->version);
> > ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> > if (ret < 0) {
> > ret = 0;
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command
2020-08-30 7:00 ` Gwendal Grignou
@ 2020-08-30 11:11 ` Jonathan Cameron
0 siblings, 0 replies; 5+ messages in thread
From: Jonathan Cameron @ 2020-08-30 11:11 UTC (permalink / raw)
To: Gwendal Grignou
Cc: Benson Leung, Enric Balletbo i Serra, linux-iio, linux-kernel
On Sun, 30 Aug 2020 00:00:02 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:
> On Sat, Aug 29, 2020 at 8:54 AM Jonathan Cameron <jic23@kernel.org> wrote:
> >
> > On Tue, 25 Aug 2020 17:29:45 -0700
> > Gwendal Grignou <gwendal@chromium.org> wrote:
> >
> > > By default, the lightbar commands are set to the
> > > biggest lightbar command and response. That length is greater than 128
> > > bytes and may not work on all machines.
> > > But all EC are probed for lightbar by sending a get version request.
> > > Set that request size precisely.
> > >
> > > Before the command would be:
> > > cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 194, insize: 128, result: 0
> > > Afer:
> > > cros_ec_cmd: version: 0, command: EC_CMD_LIGHTBAR_CMD, outsize: 1, insize: 8, result: 0
> > >
> > > Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> > Hi Gwendal,
> >
> > Description seems to me to suggest this is a fix?
> > Are there known machines on which it doesn't work currently?
> We have a prototype [without lightbar] where the command size was
> limited to 128 bytes.
> Given we issue a get_lightbar_version on all chromebooks, we had a
> failure on this prototype. Devices with a lightbar must support a
> command size greater or equal to 194 bytes.
> Beside helping the prototype to boot, this patch slightly speeds up
> the enumeration of devices managed by the EC.
> >
> > If so, please can I have a fixes tag. If it's just a precaution
> > against future problems then let me know and I can add it for the
> > next merge window.
> Done in v3.
> Note I made a mistake by sending the patch to linux-iio as it targeted
> platform/chromeos.
I hadn't even noticed that it wasn't in IIO. Oops :)
Jonathan
> >
> > Thanks,
> >
> > Jonathan
> >
> > > ---
> > > Changes since v1:
> > > - Remove BUG and TEST fields.
> > >
> > > drivers/platform/chrome/cros_ec_lightbar.c | 2 ++
> > > 1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/platform/chrome/cros_ec_lightbar.c b/drivers/platform/chrome/cros_ec_lightbar.c
> > > index b59180bff5a3e..ef61298c30bdd 100644
> > > --- a/drivers/platform/chrome/cros_ec_lightbar.c
> > > +++ b/drivers/platform/chrome/cros_ec_lightbar.c
> > > @@ -116,6 +116,8 @@ static int get_lightbar_version(struct cros_ec_dev *ec,
> > >
> > > param = (struct ec_params_lightbar *)msg->data;
> > > param->cmd = LIGHTBAR_CMD_VERSION;
> > > + msg->outsize = sizeof(param->cmd);
> > > + msg->result = sizeof(resp->version);
> > > ret = cros_ec_cmd_xfer_status(ec->ec_dev, msg);
> > > if (ret < 0) {
> > > ret = 0;
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-30 11:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-08-26 0:29 [PATCH v2] platform: cros_ec: Reduce ligthbar get version command Gwendal Grignou
2020-08-28 22:05 ` Gwendal Grignou
2020-08-29 15:54 ` Jonathan Cameron
2020-08-30 7:00 ` Gwendal Grignou
2020-08-30 11:11 ` Jonathan Cameron
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.