[PATCH v2] platform: cros_ec: Reduce ligthbar get version command

[PATCH v2] platform: cros_ec: Reduce ligthbar get version command

[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

Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command

[Jonathan Cameron]

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;

Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command

[Gwendal Grignou]

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;
>

Re: [PATCH v2] platform: cros_ec: Reduce ligthbar get version command

[Jonathan Cameron]

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;  
> >