* [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format
@ 2020-09-09 4:04 Stephen Boyd
2020-09-15 12:48 ` Enric Balletbo i Serra
0 siblings, 1 reply; 5+ messages in thread
From: Stephen Boyd @ 2020-09-09 4:04 UTC (permalink / raw)
To: Benson Leung, Enric Balletbo i Serra
Cc: linux-kernel, Gwendal Grignou, Prashant Malani, Guenter Roeck
Let's try to read more information out of more modern cros_ec devices by
using the v2 format first and then fall back to the v1 format. This
gives us more information about things such as DP mode of the typec pins
and the CC state, along with some more things.
Cc: Gwendal Grignou <gwendal@chromium.org>
Cc: Prashant Malani <pmalani@chromium.org>
Cc: Guenter Roeck <groeck@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
Should we move read_buf to the heap?
drivers/platform/chrome/cros_ec_debugfs.c | 40 +++++++++++++++++------
1 file changed, 30 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
index 272c89837d74..4f8c902c0de6 100644
--- a/drivers/platform/chrome/cros_ec_debugfs.c
+++ b/drivers/platform/chrome/cros_ec_debugfs.c
@@ -195,28 +195,31 @@ static ssize_t cros_ec_pdinfo_read(struct file *file,
size_t count,
loff_t *ppos)
{
- char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
+ char read_buf[EC_USB_PD_MAX_PORTS * 64], *p = read_buf;
struct cros_ec_debugfs *debug_info = file->private_data;
struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
struct {
struct cros_ec_command msg;
union {
- struct ec_response_usb_pd_control_v1 resp;
+ struct ec_response_usb_pd_control_v2 resp_v2;
+ struct ec_response_usb_pd_control_v1 resp_v1;
struct ec_params_usb_pd_control params;
};
} __packed ec_buf;
struct cros_ec_command *msg;
- struct ec_response_usb_pd_control_v1 *resp;
+ struct ec_response_usb_pd_control_v1 *resp_v1;
+ struct ec_response_usb_pd_control_v2 *resp_v2;
struct ec_params_usb_pd_control *params;
int i;
msg = &ec_buf.msg;
params = (struct ec_params_usb_pd_control *)msg->data;
- resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
+ resp_v1 = (struct ec_response_usb_pd_control_v1 *)msg->data;
+ resp_v2 = (struct ec_response_usb_pd_control_v2 *)msg->data;
msg->command = EC_CMD_USB_PD_CONTROL;
- msg->version = 1;
- msg->insize = sizeof(*resp);
+ msg->version = 2;
+ msg->insize = sizeof(*resp_v2);
msg->outsize = sizeof(*params);
/*
@@ -229,13 +232,30 @@ static ssize_t cros_ec_pdinfo_read(struct file *file,
params->mux = 0;
params->swap = 0;
- if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
+ if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0) {
+ if (i == 0 && msg->version == 2) {
+ /* Try again with version 1 */
+ msg->version = 1;
+ msg->insize = sizeof(*resp_v1);
+ i = 0;
+ continue;
+ }
+
break;
+ }
p += scnprintf(p, sizeof(read_buf) + read_buf - p,
- "p%d: %s en:%.2x role:%.2x pol:%.2x\n", i,
- resp->state, resp->enabled, resp->role,
- resp->polarity);
+ "p%d: %s en:%.2x role:%.2x pol:%.2x", i,
+ resp_v1->state, resp_v1->enabled, resp_v1->role,
+ resp_v1->polarity);
+ if (msg->version == 2) {
+ p += scnprintf(p, sizeof(read_buf) + read_buf - p,
+ " cc:%.2x dp:%.2x ctrl:%.2x cs:%.2x gen:%.2x",
+ resp_v2->cc_state, resp_v2->dp_mode,
+ resp_v2->control_flags, resp_v2->cable_speed,
+ resp_v2->cable_gen);
+ }
+ p += scnprintf(p, sizeof(read_buf) + read_buf - p, "\n");
}
return simple_read_from_buffer(user_buf, count, ppos,
--
Sent by a computer, using git, on the internet
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format
2020-09-09 4:04 [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format Stephen Boyd
@ 2020-09-15 12:48 ` Enric Balletbo i Serra
2020-09-15 15:34 ` Prashant Malani
0 siblings, 1 reply; 5+ messages in thread
From: Enric Balletbo i Serra @ 2020-09-15 12:48 UTC (permalink / raw)
To: Stephen Boyd, Benson Leung
Cc: linux-kernel, Gwendal Grignou, Prashant Malani, Guenter Roeck
Hi Stephen, Prashant,
On 9/9/20 6:04, Stephen Boyd wrote:
> Let's try to read more information out of more modern cros_ec devices by
> using the v2 format first and then fall back to the v1 format. This
> gives us more information about things such as DP mode of the typec pins
> and the CC state, along with some more things.
>
> Cc: Gwendal Grignou <gwendal@chromium.org>
> Cc: Prashant Malani <pmalani@chromium.org>
> Cc: Guenter Roeck <groeck@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> ---
>
I saw some discussion going on in gerrit (a pity the discussion didn't happen in
mainline) Did you end with a conclusion? Can I remove this patch from my backlog?
Thanks,
Enric
> Should we move read_buf to the heap?
>
> drivers/platform/chrome/cros_ec_debugfs.c | 40 +++++++++++++++++------
> 1 file changed, 30 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/chrome/cros_ec_debugfs.c b/drivers/platform/chrome/cros_ec_debugfs.c
> index 272c89837d74..4f8c902c0de6 100644
> --- a/drivers/platform/chrome/cros_ec_debugfs.c
> +++ b/drivers/platform/chrome/cros_ec_debugfs.c
> @@ -195,28 +195,31 @@ static ssize_t cros_ec_pdinfo_read(struct file *file,
> size_t count,
> loff_t *ppos)
> {
> - char read_buf[EC_USB_PD_MAX_PORTS * 40], *p = read_buf;
> + char read_buf[EC_USB_PD_MAX_PORTS * 64], *p = read_buf;
> struct cros_ec_debugfs *debug_info = file->private_data;
> struct cros_ec_device *ec_dev = debug_info->ec->ec_dev;
> struct {
> struct cros_ec_command msg;
> union {
> - struct ec_response_usb_pd_control_v1 resp;
> + struct ec_response_usb_pd_control_v2 resp_v2;
> + struct ec_response_usb_pd_control_v1 resp_v1;
> struct ec_params_usb_pd_control params;
> };
> } __packed ec_buf;
> struct cros_ec_command *msg;
> - struct ec_response_usb_pd_control_v1 *resp;
> + struct ec_response_usb_pd_control_v1 *resp_v1;
> + struct ec_response_usb_pd_control_v2 *resp_v2;
> struct ec_params_usb_pd_control *params;
> int i;
>
> msg = &ec_buf.msg;
> params = (struct ec_params_usb_pd_control *)msg->data;
> - resp = (struct ec_response_usb_pd_control_v1 *)msg->data;
> + resp_v1 = (struct ec_response_usb_pd_control_v1 *)msg->data;
> + resp_v2 = (struct ec_response_usb_pd_control_v2 *)msg->data;
>
> msg->command = EC_CMD_USB_PD_CONTROL;
> - msg->version = 1;
> - msg->insize = sizeof(*resp);
> + msg->version = 2;
> + msg->insize = sizeof(*resp_v2);
> msg->outsize = sizeof(*params);
>
> /*
> @@ -229,13 +232,30 @@ static ssize_t cros_ec_pdinfo_read(struct file *file,
> params->mux = 0;
> params->swap = 0;
>
> - if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0)
> + if (cros_ec_cmd_xfer_status(ec_dev, msg) < 0) {
> + if (i == 0 && msg->version == 2) {
> + /* Try again with version 1 */
> + msg->version = 1;
> + msg->insize = sizeof(*resp_v1);
> + i = 0;
> + continue;
> + }
> +
> break;
> + }
>
> p += scnprintf(p, sizeof(read_buf) + read_buf - p,
> - "p%d: %s en:%.2x role:%.2x pol:%.2x\n", i,
> - resp->state, resp->enabled, resp->role,
> - resp->polarity);
> + "p%d: %s en:%.2x role:%.2x pol:%.2x", i,
> + resp_v1->state, resp_v1->enabled, resp_v1->role,
> + resp_v1->polarity);
> + if (msg->version == 2) {
> + p += scnprintf(p, sizeof(read_buf) + read_buf - p,
> + " cc:%.2x dp:%.2x ctrl:%.2x cs:%.2x gen:%.2x",
> + resp_v2->cc_state, resp_v2->dp_mode,
> + resp_v2->control_flags, resp_v2->cable_speed,
> + resp_v2->cable_gen);
> + }
> + p += scnprintf(p, sizeof(read_buf) + read_buf - p, "\n");
> }
>
> return simple_read_from_buffer(user_buf, count, ppos,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format
2020-09-15 12:48 ` Enric Balletbo i Serra
@ 2020-09-15 15:34 ` Prashant Malani
2020-09-16 9:38 ` Enric Balletbo i Serra
0 siblings, 1 reply; 5+ messages in thread
From: Prashant Malani @ 2020-09-15 15:34 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Stephen Boyd, Benson Leung, Linux Kernel Mailing List,
Gwendal Grignou, Guenter Roeck
HI Enric,
On Tue, Sep 15, 2020 at 5:48 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Stephen, Prashant,
>
> On 9/9/20 6:04, Stephen Boyd wrote:
> > Let's try to read more information out of more modern cros_ec devices by
> > using the v2 format first and then fall back to the v1 format. This
> > gives us more information about things such as DP mode of the typec pins
> > and the CC state, along with some more things.
> >
> > Cc: Gwendal Grignou <gwendal@chromium.org>
> > Cc: Prashant Malani <pmalani@chromium.org>
> > Cc: Guenter Roeck <groeck@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> > ---
> >
>
> I saw some discussion going on in gerrit (a pity the discussion didn't happen in
> mainline) Did you end with a conclusion? Can I remove this patch from my backlog?
My apologies for not posting the comment here.
To summarize: the userspace EC utility ectool [1] can offer the
equivalent output, but with better formatting. So I believe the
decision is to use that instead of this patch.
I also posed a counter-question: can we remove this debugfs pdinfo
file entirely, since we can pull this information using ectool?
[1]: https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/util/ectool.c
Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format
2020-09-15 15:34 ` Prashant Malani
@ 2020-09-16 9:38 ` Enric Balletbo i Serra
2020-09-16 15:11 ` Prashant Malani
0 siblings, 1 reply; 5+ messages in thread
From: Enric Balletbo i Serra @ 2020-09-16 9:38 UTC (permalink / raw)
To: Prashant Malani
Cc: Stephen Boyd, Benson Leung, Linux Kernel Mailing List,
Gwendal Grignou, Guenter Roeck
Hi Prashant,
On 15/9/20 17:34, Prashant Malani wrote:
> HI Enric,
>
> On Tue, Sep 15, 2020 at 5:48 AM Enric Balletbo i Serra
> <enric.balletbo@collabora.com> wrote:
>>
>> Hi Stephen, Prashant,
>>
>> On 9/9/20 6:04, Stephen Boyd wrote:
>>> Let's try to read more information out of more modern cros_ec devices by
>>> using the v2 format first and then fall back to the v1 format. This
>>> gives us more information about things such as DP mode of the typec pins
>>> and the CC state, along with some more things.
>>>
>>> Cc: Gwendal Grignou <gwendal@chromium.org>
>>> Cc: Prashant Malani <pmalani@chromium.org>
>>> Cc: Guenter Roeck <groeck@chromium.org>
>>> Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>>> ---
>>>
>>
>> I saw some discussion going on in gerrit (a pity the discussion didn't happen in
>> mainline) Did you end with a conclusion? Can I remove this patch from my backlog?
>
> My apologies for not posting the comment here.
> To summarize: the userspace EC utility ectool [1] can offer the
> equivalent output, but with better formatting. So I believe the
> decision is to use that instead of this patch.
> I also posed a counter-question: can we remove this debugfs pdinfo
> file entirely, since we can pull this information using ectool?
>
If I am not mistaken pdinfo is used by your userspace, so, assuming we don't
break (or we have a plan to not break) things I'm fine with it. In general,
delegating things to userspace and get rid of kernel code is good for everyone.
Thanks,
Enric
> [1]: https://chromium.googlesource.com/chromiumos/platform/ec/+/refs/heads/master/util/ectool.c
>
> Best regards,
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format
2020-09-16 9:38 ` Enric Balletbo i Serra
@ 2020-09-16 15:11 ` Prashant Malani
0 siblings, 0 replies; 5+ messages in thread
From: Prashant Malani @ 2020-09-16 15:11 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Stephen Boyd, Benson Leung, Linux Kernel Mailing List,
Gwendal Grignou, Guenter Roeck
Hi Enric,
On Wed, Sep 16, 2020 at 2:39 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Prashant,
>
>
> If I am not mistaken pdinfo is used by your userspace, so, assuming we don't
> break (or we have a plan to not break) things I'm fine with it. In general,
> delegating things to userspace and get rid of kernel code is good for everyone.
Sounds good, thanks! I will look into this and if it isn't breaking
anything I'll send a patch which does away with pdinfo.
Best regards,
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-16 15:30 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 4:04 [PATCH] platform/chrome: cros_ec_debugfs: Support pd_info v2 format Stephen Boyd
2020-09-15 12:48 ` Enric Balletbo i Serra
2020-09-15 15:34 ` Prashant Malani
2020-09-16 9:38 ` Enric Balletbo i Serra
2020-09-16 15:11 ` Prashant Malani
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).