* [PATCH] platform: cros_ec_debugfs: control uptime information request
@ 2020-05-21 5:28 Gwendal Grignou
2020-05-21 9:21 ` Enric Balletbo i Serra
0 siblings, 1 reply; 3+ messages in thread
From: Gwendal Grignou @ 2020-05-21 5:28 UTC (permalink / raw)
To: groeck, bleung, enric.balletbo, twawrzynczak
Cc: linux-kernel, Gwendal Grignou
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.
Signed-off-by: Gwendal Grignou <gwendal@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;
};
/*
@@ -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;
--
2.26.2.761.g0e0b3e54be-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] platform: cros_ec_debugfs: control uptime information request
2020-05-21 5:28 [PATCH] platform: cros_ec_debugfs: control uptime information request Gwendal Grignou
@ 2020-05-21 9:21 ` Enric Balletbo i Serra
2020-05-26 18:49 ` Gwendal Grignou
0 siblings, 1 reply; 3+ messages in thread
From: Enric Balletbo i Serra @ 2020-05-21 9:21 UTC (permalink / raw)
To: Gwendal Grignou, groeck, bleung, twawrzynczak; +Cc: linux-kernel
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.
> Signed-off-by: Gwendal Grignou <gwendal@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.
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;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] platform: cros_ec_debugfs: control uptime information request
2020-05-21 9:21 ` Enric Balletbo i Serra
@ 2020-05-26 18:49 ` Gwendal Grignou
0 siblings, 0 replies; 3+ messages in thread
From: Gwendal Grignou @ 2020-05-26 18:49 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Guenter Roeck, Benson Leung, Tim Wawrzynczak, linux-kernel
On Thu, May 21, 2020 at 2:21 AM Enric Balletbo i Serra
<enric.balletbo@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 <gwendal@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;
> >
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-26 18:50 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-21 5:28 [PATCH] platform: cros_ec_debugfs: control uptime information request Gwendal Grignou
2020-05-21 9:21 ` Enric Balletbo i Serra
2020-05-26 18:49 ` Gwendal Grignou
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).