| Message ID | 20200826002945.394562-1-gwendal@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] platform: cros_ec: Reduce ligthbar get version command | expand |
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;
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; >
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; > >
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;
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(+)