[PATCH v5] platform/chrome: wilco_ec: Add debugfs test_event file

[PATCH v5] platform/chrome: wilco_ec: Add debugfs test_event file

[Daniel Campello]

This change introduces a new debugfs file 'test_event' that when written
to causes the EC to generate a test event.
This adds a second sub cmd for the test event, and pulls out send_ec_cmd
to be a common helper between h1_gpio_get and test_event_set.

Signed-off-by: Daniel Campello <campello@chromium.org>
---
Changes for v2:
  - Cleaned up and added comments.
  - Renamed and updated function signature from write_to_mailbox to
    send_ec_cmd.
Changes for v3:
  - Switched NULL format string to empty format string
  - Renamed val parameter on send_ec_cmd to out_val
Changes for v4:
  - Provided a format string to avoid -Wformat-zero-length warning
Changes for v5:
  - Updated commit message to include more implementation details
  - Restored removed empty line between functions

 drivers/platform/chrome/wilco_ec/debugfs.c | 47 +++++++++++++++++-----
 1 file changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
index 8d65a1e2f1a3..df5a5f6c3ec6 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 */
+	u8 sub_cmd;
 } __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 */
+	u8 val;
 } __packed;

-static int h1_gpio_get(void *arg, u64 *val)
+static int send_ec_cmd(struct wilco_ec_device *ec, u8 sub_cmd, u8 *out_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;
@@ -196,13 +196,38 @@ static int h1_gpio_get(void *arg, u64 *val)
 	if (rs.status)
 		return -EIO;

-	*val = rs.val;
+	*out_val = rs.val;

 	return 0;
 }

+/**
+ * h1_gpio_get() - Gets h1 gpio status.
+ * @arg: The wilco EC device.
+ * @val: BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL
+ */
+static int h1_gpio_get(void *arg, u64 *val)
+{
+	return send_ec_cmd(arg, SUB_CMD_H1_GPIO, (u8 *)val);
+}
+
 DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n");

+/**
+ * test_event_set() - Sends command to EC to cause an EC test event.
+ * @arg: The wilco EC device.
+ * @val: unused.
+ */
+static int test_event_set(void *arg, u64 val)
+{
+	u8 ret;
+
+	return send_ec_cmd(arg, SUB_CMD_TEST_EVENT, &ret);
+}
+
+/* Format is unused since it is only required for get method which is NULL */
+DEFINE_DEBUGFS_ATTRIBUTE(fops_test_event, NULL, test_event_set, "%llu\n");
+
 /**
  * wilco_ec_debugfs_probe() - Create the debugfs node
  * @pdev: The platform device, probably created in core.c
@@ -226,6 +251,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.351.gc4317032e6-goog

Re: [PATCH v5] platform/chrome: wilco_ec: Add debugfs test_event file

[Benson Leung]

[-- Attachment #1: Type: text/plain, Size: 4393 bytes --]

Hi Daniel,

On Tue, Sep 24, 2019 at 02:37:16PM -0600, Daniel Campello wrote:
> This change introduces a new debugfs file 'test_event' that when written
> to causes the EC to generate a test event.
> This adds a second sub cmd for the test event, and pulls out send_ec_cmd
> to be a common helper between h1_gpio_get and test_event_set.
> 
> Signed-off-by: Daniel Campello <campello@chromium.org>

LGTM.
Reviewed-by: Benson Leung <bleung@chromium.org>

Merge window is open right now for v5.4, so we can't quite merge this change
for chrome-platform-5.5 yet.

Enric, can you queue this for the next branch if you have no objections?

Thanks,
Benson

> ---
> Changes for v2:
>   - Cleaned up and added comments.
>   - Renamed and updated function signature from write_to_mailbox to
>     send_ec_cmd.
> Changes for v3:
>   - Switched NULL format string to empty format string
>   - Renamed val parameter on send_ec_cmd to out_val
> Changes for v4:
>   - Provided a format string to avoid -Wformat-zero-length warning
> Changes for v5:
>   - Updated commit message to include more implementation details
>   - Restored removed empty line between functions
> 
>  drivers/platform/chrome/wilco_ec/debugfs.c | 47 +++++++++++++++++-----
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
> index 8d65a1e2f1a3..df5a5f6c3ec6 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 */
> +	u8 sub_cmd;
>  } __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 */
> +	u8 val;
>  } __packed;
> 
> -static int h1_gpio_get(void *arg, u64 *val)
> +static int send_ec_cmd(struct wilco_ec_device *ec, u8 sub_cmd, u8 *out_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;
> @@ -196,13 +196,38 @@ static int h1_gpio_get(void *arg, u64 *val)
>  	if (rs.status)
>  		return -EIO;
> 
> -	*val = rs.val;
> +	*out_val = rs.val;
> 
>  	return 0;
>  }
> 
> +/**
> + * h1_gpio_get() - Gets h1 gpio status.
> + * @arg: The wilco EC device.
> + * @val: BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL
> + */
> +static int h1_gpio_get(void *arg, u64 *val)
> +{
> +	return send_ec_cmd(arg, SUB_CMD_H1_GPIO, (u8 *)val);
> +}
> +
>  DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n");
> 
> +/**
> + * test_event_set() - Sends command to EC to cause an EC test event.
> + * @arg: The wilco EC device.
> + * @val: unused.
> + */
> +static int test_event_set(void *arg, u64 val)
> +{
> +	u8 ret;
> +
> +	return send_ec_cmd(arg, SUB_CMD_TEST_EVENT, &ret);
> +}
> +
> +/* Format is unused since it is only required for get method which is NULL */
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_test_event, NULL, test_event_set, "%llu\n");
> +
>  /**
>   * wilco_ec_debugfs_probe() - Create the debugfs node
>   * @pdev: The platform device, probably created in core.c
> @@ -226,6 +251,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.351.gc4317032e6-goog
> 

-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v5] platform/chrome: wilco_ec: Add debugfs test_event file

[Nick Crews]

Thanks Daniel, looks even better.

On Tue, Sep 24, 2019 at 2:37 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.
> This adds a second sub cmd for the test event, and pulls out send_ec_cmd
> to be a common helper between h1_gpio_get and test_event_set.
>
> Signed-off-by: Daniel Campello <campello@chromium.org>

Reviewed-by: Nick Crews <ncrews@chromium.org>

> ---
> Changes for v2:
>   - Cleaned up and added comments.
>   - Renamed and updated function signature from write_to_mailbox to
>     send_ec_cmd.
> Changes for v3:
>   - Switched NULL format string to empty format string
>   - Renamed val parameter on send_ec_cmd to out_val
> Changes for v4:
>   - Provided a format string to avoid -Wformat-zero-length warning
> Changes for v5:
>   - Updated commit message to include more implementation details
>   - Restored removed empty line between functions
>
>  drivers/platform/chrome/wilco_ec/debugfs.c | 47 +++++++++++++++++-----
>  1 file changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
> index 8d65a1e2f1a3..df5a5f6c3ec6 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 */
> +       u8 sub_cmd;
>  } __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 */
> +       u8 val;
>  } __packed;
>
> -static int h1_gpio_get(void *arg, u64 *val)
> +static int send_ec_cmd(struct wilco_ec_device *ec, u8 sub_cmd, u8 *out_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;
> @@ -196,13 +196,38 @@ static int h1_gpio_get(void *arg, u64 *val)
>         if (rs.status)
>                 return -EIO;
>
> -       *val = rs.val;
> +       *out_val = rs.val;
>
>         return 0;
>  }
>
> +/**
> + * h1_gpio_get() - Gets h1 gpio status.
> + * @arg: The wilco EC device.
> + * @val: BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL
> + */
> +static int h1_gpio_get(void *arg, u64 *val)
> +{
> +       return send_ec_cmd(arg, SUB_CMD_H1_GPIO, (u8 *)val);
> +}
> +
>  DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n");
>
> +/**
> + * test_event_set() - Sends command to EC to cause an EC test event.
> + * @arg: The wilco EC device.
> + * @val: unused.
> + */
> +static int test_event_set(void *arg, u64 val)
> +{
> +       u8 ret;
> +
> +       return send_ec_cmd(arg, SUB_CMD_TEST_EVENT, &ret);
> +}
> +
> +/* Format is unused since it is only required for get method which is NULL */
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_test_event, NULL, test_event_set, "%llu\n");
> +
>  /**
>   * wilco_ec_debugfs_probe() - Create the debugfs node
>   * @pdev: The platform device, probably created in core.c
> @@ -226,6 +251,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.351.gc4317032e6-goog
>

Re: [PATCH v5] platform/chrome: wilco_ec: Add debugfs test_event file

[Enric Balletbo i Serra]

Hi,

On 24/9/19 23:36, Benson Leung wrote:
> Hi Daniel,
> 
> On Tue, Sep 24, 2019 at 02:37:16PM -0600, Daniel Campello wrote:
>> This change introduces a new debugfs file 'test_event' that when written
>> to causes the EC to generate a test event.
>> This adds a second sub cmd for the test event, and pulls out send_ec_cmd
>> to be a common helper between h1_gpio_get and test_event_set.
>>
>> Signed-off-by: Daniel Campello <campello@chromium.org>
> 
> LGTM.
> Reviewed-by: Benson Leung <bleung@chromium.org>
> 
> Merge window is open right now for v5.4, so we can't quite merge this change
> for chrome-platform-5.5 yet.
> 
> Enric, can you queue this for the next branch if you have no objections?
> 

Queued for the autobuilders to play with. If all goes well will appear in
chrome-platform-5.5 and linux-next in 24/48h

Thanks,
 Enric

> Thanks,
> Benson
> 
>> ---
>> Changes for v2:
>>   - Cleaned up and added comments.
>>   - Renamed and updated function signature from write_to_mailbox to
>>     send_ec_cmd.
>> Changes for v3:
>>   - Switched NULL format string to empty format string
>>   - Renamed val parameter on send_ec_cmd to out_val
>> Changes for v4:
>>   - Provided a format string to avoid -Wformat-zero-length warning
>> Changes for v5:
>>   - Updated commit message to include more implementation details
>>   - Restored removed empty line between functions
>>
>>  drivers/platform/chrome/wilco_ec/debugfs.c | 47 +++++++++++++++++-----
>>  1 file changed, 37 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
>> index 8d65a1e2f1a3..df5a5f6c3ec6 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 */
>> +	u8 sub_cmd;
>>  } __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 */
>> +	u8 val;
>>  } __packed;
>>
>> -static int h1_gpio_get(void *arg, u64 *val)
>> +static int send_ec_cmd(struct wilco_ec_device *ec, u8 sub_cmd, u8 *out_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;
>> @@ -196,13 +196,38 @@ static int h1_gpio_get(void *arg, u64 *val)
>>  	if (rs.status)
>>  		return -EIO;
>>
>> -	*val = rs.val;
>> +	*out_val = rs.val;
>>
>>  	return 0;
>>  }
>>
>> +/**
>> + * h1_gpio_get() - Gets h1 gpio status.
>> + * @arg: The wilco EC device.
>> + * @val: BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL
>> + */
>> +static int h1_gpio_get(void *arg, u64 *val)
>> +{
>> +	return send_ec_cmd(arg, SUB_CMD_H1_GPIO, (u8 *)val);
>> +}
>> +
>>  DEFINE_DEBUGFS_ATTRIBUTE(fops_h1_gpio, h1_gpio_get, NULL, "0x%02llx\n");
>>
>> +/**
>> + * test_event_set() - Sends command to EC to cause an EC test event.
>> + * @arg: The wilco EC device.
>> + * @val: unused.
>> + */
>> +static int test_event_set(void *arg, u64 val)
>> +{
>> +	u8 ret;
>> +
>> +	return send_ec_cmd(arg, SUB_CMD_TEST_EVENT, &ret);
>> +}
>> +
>> +/* Format is unused since it is only required for get method which is NULL */
>> +DEFINE_DEBUGFS_ATTRIBUTE(fops_test_event, NULL, test_event_set, "%llu\n");
>> +
>>  /**
>>   * wilco_ec_debugfs_probe() - Create the debugfs node
>>   * @pdev: The platform device, probably created in core.c
>> @@ -226,6 +251,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.351.gc4317032e6-goog
>>
>