* [PATCH] platform/chrome: wilco_ec: Add debugfs test_event file
@ 2019-09-06 20:42 Daniel Campello
2019-09-06 22:20 ` Nick Crews
0 siblings, 1 reply; 3+ messages in thread
From: Daniel Campello @ 2019-09-06 20:42 UTC (permalink / raw)
To: LKML
Cc: Daniel Campello, Enric Balletbo i Serra, Benson Leung,
Alexandre Belloni, Duncan Laurie, Nick Crews
This change introduces a new debugfs file 'test_event' that when written
to causes the EC to generate a test event.
Signed-off-by: Daniel Campello <campello@chromium.org>
---
drivers/platform/chrome/wilco_ec/debugfs.c | 33 +++++++++++++++++-----
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
index 8d65a1e2f1a3..2103c3ed8385 100644
--- a/drivers/platform/chrome/wilco_ec/debugfs.c
+++ b/drivers/platform/chrome/wilco_ec/debugfs.c
@@ -160,29 +160,29 @@ static const struct file_operations fops_raw = {
#define CMD_KB_CHROME 0x88
#define SUB_CMD_H1_GPIO 0x0A
+#define SUB_CMD_TEST_EVENT 0x0B
-struct h1_gpio_status_request {
+struct ec_request {
u8 cmd; /* Always CMD_KB_CHROME */
u8 reserved;
u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */
} __packed;
-struct hi_gpio_status_response {
+struct ec_response {
u8 status; /* 0 if allowed */
u8 val; /* BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL */
} __packed;
-static int h1_gpio_get(void *arg, u64 *val)
+static int write_to_mailbox(struct wilco_ec_device *ec, u8 sub_cmd, u64 *val)
{
- struct wilco_ec_device *ec = arg;
- struct h1_gpio_status_request rq;
- struct hi_gpio_status_response rs;
+ struct ec_request rq;
+ struct ec_response rs;
struct wilco_ec_message msg;
int ret;
memset(&rq, 0, sizeof(rq));
rq.cmd = CMD_KB_CHROME;
- rq.sub_cmd = SUB_CMD_H1_GPIO;
+ rq.sub_cmd = sub_cmd;
memset(&msg, 0, sizeof(msg));
msg.type = WILCO_EC_MSG_LEGACY;
@@ -201,8 +201,25 @@ static int h1_gpio_get(void *arg, u64 *val)
return 0;
}
+static int h1_gpio_get(void *arg, u64 *val)
+{
+ return write_to_mailbox(arg, SUB_CMD_H1_GPIO, val);
+}
+
DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n");
+static int test_event_set(void *arg, u64 val)
+{
+ u64 ret;
+
+ return write_to_mailbox(arg, SUB_CMD_TEST_EVENT, &ret);
+}
+
+/* Format set to NULL since it is only used on read operations which are
+ * forbidden by file permissions.
+ */
+DEFINE_DEBUGFS_ATTRIBUTE(fops_test_event, NULL, test_event_set, NULL);
+
/**
* wilco_ec_debugfs_probe() - Create the debugfs node
* @pdev: The platform device, probably created in core.c
@@ -226,6 +243,8 @@ static int wilco_ec_debugfs_probe(struct platform_device *pdev)
debugfs_create_file("raw", 0644, debug_info->dir, NULL, &fops_raw);
debugfs_create_file("h1_gpio", 0444, debug_info->dir, ec,
&fops_h1_gpio);
+ debugfs_create_file("test_event", 0200, debug_info->dir, ec,
+ &fops_test_event);
return 0;
}
--
2.23.0.162.g0b9fbb3734-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] platform/chrome: wilco_ec: Add debugfs test_event file
2019-09-06 20:42 [PATCH] platform/chrome: wilco_ec: Add debugfs test_event file Daniel Campello
@ 2019-09-06 22:20 ` Nick Crews
2019-09-18 17:32 ` Daniel Campello
0 siblings, 1 reply; 3+ messages in thread
From: Nick Crews @ 2019-09-06 22:20 UTC (permalink / raw)
To: Daniel Campello
Cc: LKML, Enric Balletbo i Serra, Benson Leung, Alexandre Belloni,
Duncan Laurie, Trent Begin
Thanks for the patch Daniel! A few thoughts that I didn't
have on the review on Gerrit, sorry :) After those changes,
Reviewed-by: Nick Crews <ncrews@chromium.org>
On Fri, Sep 6, 2019 at 4:42 PM Daniel Campello <campello@chromium.org> wrote:
>
> This change introduces a new debugfs file 'test_event' that when written
> to causes the EC to generate a test event.
>
> Signed-off-by: Daniel Campello <campello@chromium.org>
> ---
>
> drivers/platform/chrome/wilco_ec/debugfs.c | 33 +++++++++++++++++-----
> 1 file changed, 26 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
> index 8d65a1e2f1a3..2103c3ed8385 100644
> --- a/drivers/platform/chrome/wilco_ec/debugfs.c
> +++ b/drivers/platform/chrome/wilco_ec/debugfs.c
> @@ -160,29 +160,29 @@ static const struct file_operations fops_raw = {
>
> #define CMD_KB_CHROME 0x88
> #define SUB_CMD_H1_GPIO 0x0A
> +#define SUB_CMD_TEST_EVENT 0x0B
>
> -struct h1_gpio_status_request {
> +struct ec_request {
> u8 cmd; /* Always CMD_KB_CHROME */
> u8 reserved;
> u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */
This comment should be removed.
> } __packed;
>
> -struct hi_gpio_status_response {
> +struct ec_response {
> u8 status; /* 0 if allowed */
> u8 val; /* BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL */
This comment should be moved to h1_gpio_get().
> } __packed;
>
> -static int h1_gpio_get(void *arg, u64 *val)
> +static int write_to_mailbox(struct wilco_ec_device *ec, u8 sub_cmd, u64 *val)
What about send_ec_cmd() or similar? Something that communicates that
we are sometimes telling the EC to do something, and sometimes reading something
back. Also, since we are adding in another layer in here, we can fix
the signature from
the one required by a debugfs attribute. Use "ret" instead of "val"
and make it a u8*.
> {
> - struct wilco_ec_device *ec = arg;
> - struct h1_gpio_status_request rq;
> - struct hi_gpio_status_response rs;
> + struct ec_request rq;
> + struct ec_response rs;
> struct wilco_ec_message msg;
> int ret;
>
> memset(&rq, 0, sizeof(rq));
> rq.cmd = CMD_KB_CHROME;
> - rq.sub_cmd = SUB_CMD_H1_GPIO;
> + rq.sub_cmd = sub_cmd;
>
> memset(&msg, 0, sizeof(msg));
> msg.type = WILCO_EC_MSG_LEGACY;
> @@ -201,8 +201,25 @@ static int h1_gpio_get(void *arg, u64 *val)
> return 0;
> }
>
> +static int h1_gpio_get(void *arg, u64 *val)
> +{
> + return write_to_mailbox(arg, SUB_CMD_H1_GPIO, val);
> +}
> +
> DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n");
>
A one line comment as to what test_event does?
> +static int test_event_set(void *arg, u64 val)
> +{
> + u64 ret;
> +
> + return write_to_mailbox(arg, SUB_CMD_TEST_EVENT, &ret);
> +}
> +
> +/* Format set to NULL since it is only used on read operations which are
> + * forbidden by file permissions.
> + */
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_test_event, NULL, test_event_set, NULL);
> +
> /**
> * wilco_ec_debugfs_probe() - Create the debugfs node
> * @pdev: The platform device, probably created in core.c
> @@ -226,6 +243,8 @@ static int wilco_ec_debugfs_probe(struct platform_device *pdev)
> debugfs_create_file("raw", 0644, debug_info->dir, NULL, &fops_raw);
> debugfs_create_file("h1_gpio", 0444, debug_info->dir, ec,
> &fops_h1_gpio);
> + debugfs_create_file("test_event", 0200, debug_info->dir, ec,
> + &fops_test_event);
>
> return 0;
> }
> --
> 2.23.0.162.g0b9fbb3734-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] platform/chrome: wilco_ec: Add debugfs test_event file
2019-09-06 22:20 ` Nick Crews
@ 2019-09-18 17:32 ` Daniel Campello
0 siblings, 0 replies; 3+ messages in thread
From: Daniel Campello @ 2019-09-18 17:32 UTC (permalink / raw)
To: Nick Crews
Cc: LKML, Enric Balletbo i Serra, Benson Leung, Alexandre Belloni,
Duncan Laurie, Trent Begin
On Fri, Sep 06, 2019 at 06:20:51PM -0400, Nick Crews wrote:
> Thanks for the patch Daniel! A few thoughts that I didn't
> have on the review on Gerrit, sorry :) After those changes,
>
No problem, I will take care of them.
> Reviewed-by: Nick Crews <ncrews@chromium.org>
>
> > u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */
>
> This comment should be removed.
>
I overlooked this. Done.
> This comment should be moved to h1_gpio_get().
>
Agree
> > -static int h1_gpio_get(void *arg, u64 *val)
> > +static int write_to_mailbox(struct wilco_ec_device *ec, u8 sub_cmd, u64 *val)
>
> What about send_ec_cmd() or similar? Something that communicates that
> we are sometimes telling the EC to do something, and sometimes reading something
> back. Also, since we are adding in another layer in here, we can fix
> the signature from
> the one required by a debugfs attribute. Use "ret" instead of "val"
> and make it a u8*.
>
send_ec_cmd() sounds good to me but ret is already used. Will change
to out_val as discussed offline.
> A one line comment as to what test_event does?
>
> > +static int test_event_set(void *arg, u64 val)
> > +{
> > + u64 ret;
> > +
> > + return write_to_mailbox(arg, SUB_CMD_TEST_EVENT, &ret);
> > +}
> > +
Will do. Thanks again.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-09-18 17:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-06 20:42 [PATCH] platform/chrome: wilco_ec: Add debugfs test_event file Daniel Campello
2019-09-06 22:20 ` Nick Crews
2019-09-18 17:32 ` Daniel Campello
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).