On Thu, May 21, 2020 at 2:21 AM Enric Balletbo i Serra
<enric.balle...@collabora.com> wrote:
>
> Hi Gwendal,
>
> Thank you for your patch.
>
> On 21/5/20 7:28, Gwendal Grignou wrote:
> > When EC does not support uptime command (EC_CMD_GET_UPTIME_INFO),
> > return -EPROTO to read of /sys/kernel/debug/cros_ec/uptime without
> > calling the EC after the first try.
> >
> > The EC console log will not contain EC_CMD_GET_UPTIME_INFO anymore.
> >
>
> Oh, what you mean with this? Uptime is only exposed via sysfs or I am missing
> something.
I was checking the ec console log (via
/sys/kernel/debug/cros_ec/console_log) to check the EC was not
receiving and erroring on the command (pixelbook)
>
> > Signed-off-by: Gwendal Grignou <gwen...@chromium.org>
> > ---
> > drivers/platform/chrome/cros_ec_debugfs.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_debugfs.c
> > b/drivers/platform/chrome/cros_ec_debugfs.c
> > index 6ae484989d1f5..70a29afb6d9e7 100644
> > --- a/drivers/platform/chrome/cros_ec_debugfs.c
> > +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> > @@ -49,6 +49,8 @@ struct cros_ec_debugfs {
> > struct delayed_work log_poll_work;
> > /* EC panicinfo */
> > struct debugfs_blob_wrapper panicinfo_blob;
> > + /* EC uptime */
> > + bool uptime_supported;
>
> Ideally, if uptime can be supported or not we should only expose uptime when
> is
> supported, so the sysfs entry should only be created when is supported,
> similar
> to what we do with the console_log. See cros_ec_create_console_log()
>
> If doing this doesn't break userspace, I'd prefer doing in that way.
Good point, sending a v2.
Gwendal.
>
> Thanks,
> Enric
>
>
> > };
> >
> > /*
> > @@ -256,12 +258,19 @@ static ssize_t cros_ec_uptime_read(struct file *file,
> > char __user *user_buf,
> > char read_buf[32];
> > int ret;
> >
> > + if (!debug_info->uptime_supported)
> > + return -EPROTO;
> > +
> > resp = (struct ec_response_uptime_info *)&msg.resp;
> >
> > msg.cmd.command = EC_CMD_GET_UPTIME_INFO;
> > msg.cmd.insize = sizeof(*resp);
> >
> > ret = cros_ec_cmd_xfer_status(ec_dev, &msg.cmd);
> > + if (ret == -EPROTO && msg.cmd.result == EC_RES_INVALID_COMMAND) {
> > + debug_info->uptime_supported = false;
> > + return ret;
> > + }
> > if (ret < 0)
> > return ret;
> >
> > @@ -434,6 +443,9 @@ static int cros_ec_debugfs_probe(struct platform_device
> > *pd)
> > debug_info->ec = ec;
> > debug_info->dir = debugfs_create_dir(name, NULL);
> >
> > + /* Give uptime a chance to run. */
> > + debug_info->uptime_supported = true;
> > +
> > ret = cros_ec_create_panicinfo(debug_info);
> > if (ret)
> > goto remove_debugfs;
> >
