As part of Chrome OS's FAFT (Fully Automated Firmware Testing)
tests, we need to ensure that the H1 chip is properly setting
some GPIO lines. The h1_gpio attribute exposes the state
of the lines:
- ENTRY_TO_FACT_MODE in BIT(0)
- SPI_CHROME_SEL in BIT(1)
There are two reasons that I am exposing this in debugfs,
and not as a GPIO:
1. This is only useful for testing, so end users shouldn't ever
care about this. In fact, if it passes the tests, then the value of
h1_gpio will always be 2, so it would be really uninteresting for users.
2. This GPIO is not connected to, controlled by, or really even related
to the AP. The GPIO runs between the EC and the H1 security chip.
Changes in v3:
- Fix documentation to correspond with formatting change in v2.
Changes in v2:
- Zero out the unused fields in the request.
- Format result as "%02x\n" instead of as a decimal.
Signed-off-by: Nick Crews <[email protected]>
---
drivers/platform/chrome/wilco_ec/debugfs.c | 64 +++++++++++++++++++++-
1 file changed, 63 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
index 17c4c9068aaf..1b93243c8954 100644
--- a/drivers/platform/chrome/wilco_ec/debugfs.c
+++ b/drivers/platform/chrome/wilco_ec/debugfs.c
@@ -4,7 +4,7 @@
*
* Copyright 2019 Google LLC
*
- * There is only one attribute used for debugging, called raw.
+ * The raw attribute is very useful for EC debugging.
* You can write a hexadecimal sentence to raw, and that series of bytes
* will be sent to the EC. Then, you can read the bytes of response
* by reading from raw.
@@ -30,6 +30,16 @@
* Note that the first 32 bytes of the received MBOX[] will be printed,
* even if some of the data is junk. It is up to you to know how many of
* the first bytes of data are the actual response.
+ *
+ * There is also another debugfs attribute, called h1_gpio.
+ * As part of Chrome OS's FAFT (Fully Automated Firmware Testing)
+ * tests, we need to ensure that the H1 chip is properly setting
+ * some GPIO lines. The h1_gpio attribute exposes the state
+ * of the lines:
+ * - ENTRY_TO_FACT_MODE in BIT(0)
+ * - SPI_CHROME_SEL in BIT(1)
+
+ * Output will formatted with "%02x\n".
*/
#include <linux/ctype.h>
@@ -189,6 +199,56 @@ static const struct file_operations fops_raw = {
.llseek = no_llseek,
};
+#define CMD_KB_CHROME 0x88
+#define SUB_CMD_H1_GPIO 0x0A
+
+struct h1_gpio_status_request {
+ u8 cmd; /* Always CMD_KB_CHROME */
+ u8 reserved;
+ u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */
+} __packed;
+
+struct hi_gpio_status_response {
+ u8 status; /* 0 if allowed */
+ u8 val; /* BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL */
+} __packed;
+
+static ssize_t h1_gpio_read(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct h1_gpio_status_request rq;
+ struct hi_gpio_status_response rs;
+ struct wilco_ec_message msg;
+ char buf[4];
+ int ret;
+
+ memset(&rq, 0, sizeof(rq));
+ rq.cmd = CMD_KB_CHROME;
+ rq.sub_cmd = SUB_CMD_H1_GPIO;
+
+ memset(&msg, 0, sizeof(msg));
+ msg.type = WILCO_EC_MSG_LEGACY;
+ msg.request_data = &rq;
+ msg.request_size = sizeof(rq);
+ msg.response_data = &rs;
+ msg.response_size = sizeof(rs);
+ ret = wilco_ec_mailbox(debug_info->ec, &msg);
+ if (ret < 0)
+ return ret;
+ if (rs.status)
+ return -EIO;
+
+ sprintf(buf, "%02x\n", rs.val);
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, sizeof(buf));
+}
+
+static const struct file_operations fops_h1_gpio = {
+ .owner = THIS_MODULE,
+ .read = h1_gpio_read,
+ .llseek = no_llseek,
+};
+
/**
* wilco_ec_debugfs_probe() - Create the debugfs node
* @pdev: The platform device, probably created in core.c
@@ -210,6 +270,8 @@ static int wilco_ec_debugfs_probe(struct platform_device *pdev)
if (!debug_info->dir)
return 0;
debugfs_create_file("raw", 0644, debug_info->dir, NULL, &fops_raw);
+ debugfs_create_file("h1_gpio", 0444, debug_info->dir, NULL,
+ &fops_h1_gpio);
return 0;
}
--
2.20.1
Hi Nick,
Some comments below ...
Missatge de Nick Crews <[email protected]> del dia dj., 11 d’abr.
2019 a les 0:09:
>
> As part of Chrome OS's FAFT (Fully Automated Firmware Testing)
> tests, we need to ensure that the H1 chip is properly setting
> some GPIO lines. The h1_gpio attribute exposes the state
> of the lines:
> - ENTRY_TO_FACT_MODE in BIT(0)
> - SPI_CHROME_SEL in BIT(1)
>
> There are two reasons that I am exposing this in debugfs,
> and not as a GPIO:
> 1. This is only useful for testing, so end users shouldn't ever
> care about this. In fact, if it passes the tests, then the value of
> h1_gpio will always be 2, so it would be really uninteresting for users.
> 2. This GPIO is not connected to, controlled by, or really even related
> to the AP. The GPIO runs between the EC and the H1 security chip.
>
> Changes in v3:
> - Fix documentation to correspond with formatting change in v2.
> Changes in v2:
> - Zero out the unused fields in the request.
> - Format result as "%02x\n" instead of as a decimal.
I don't really mind, but wouldn't be more clear prefix with 0x (0x%02x)?
>
> Signed-off-by: Nick Crews <[email protected]>
> ---
> drivers/platform/chrome/wilco_ec/debugfs.c | 64 +++++++++++++++++++++-
ABI documentation missing.
> 1 file changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
> index 17c4c9068aaf..1b93243c8954 100644
> --- a/drivers/platform/chrome/wilco_ec/debugfs.c
> +++ b/drivers/platform/chrome/wilco_ec/debugfs.c
> @@ -4,7 +4,7 @@
> *
> * Copyright 2019 Google LLC
> *
> - * There is only one attribute used for debugging, called raw.
> + * The raw attribute is very useful for EC debugging.
> * You can write a hexadecimal sentence to raw, and that series of bytes
> * will be sent to the EC. Then, you can read the bytes of response
> * by reading from raw.
> @@ -30,6 +30,16 @@
> * Note that the first 32 bytes of the received MBOX[] will be printed,
> * even if some of the data is junk. It is up to you to know how many of
> * the first bytes of data are the actual response.
> + *
> + * There is also another debugfs attribute, called h1_gpio.
> + * As part of Chrome OS's FAFT (Fully Automated Firmware Testing)
> + * tests, we need to ensure that the H1 chip is properly setting
> + * some GPIO lines. The h1_gpio attribute exposes the state
> + * of the lines:
> + * - ENTRY_TO_FACT_MODE in BIT(0)
> + * - SPI_CHROME_SEL in BIT(1)
> +
> + * Output will formatted with "%02x\n".
> */
>
> #include <linux/ctype.h>
> @@ -189,6 +199,56 @@ static const struct file_operations fops_raw = {
> .llseek = no_llseek,
> };
>
> +#define CMD_KB_CHROME 0x88
> +#define SUB_CMD_H1_GPIO 0x0A
> +
> +struct h1_gpio_status_request {
> + u8 cmd; /* Always CMD_KB_CHROME */
> + u8 reserved;
> + u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */
> +} __packed;
> +
> +struct hi_gpio_status_response {
> + u8 status; /* 0 if allowed */
> + u8 val; /* BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL */
> +} __packed;
> +
> +static ssize_t h1_gpio_read(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct h1_gpio_status_request rq;
> + struct hi_gpio_status_response rs;
> + struct wilco_ec_message msg;
> + char buf[4];
> + int ret;
> +
> + memset(&rq, 0, sizeof(rq));
> + rq.cmd = CMD_KB_CHROME;
> + rq.sub_cmd = SUB_CMD_H1_GPIO;
> +
> + memset(&msg, 0, sizeof(msg));
> + msg.type = WILCO_EC_MSG_LEGACY;
> + msg.request_data = &rq;
> + msg.request_size = sizeof(rq);
> + msg.response_data = &rs;
> + msg.response_size = sizeof(rs);
> + ret = wilco_ec_mailbox(debug_info->ec, &msg);
> + if (ret < 0)
> + return ret;
> + if (rs.status)
> + return -EIO;
> +
> + sprintf(buf, "%02x\n", rs.val);
> +
> + return simple_read_from_buffer(user_buf, count, ppos, buf, sizeof(buf));
> +}
> +
> +static const struct file_operations fops_h1_gpio = {
> + .owner = THIS_MODULE,
> + .read = h1_gpio_read,
> + .llseek = no_llseek,
> +};
> +
I think that I'd use the DEFINE_DEBUGFS_ATTRIBUTE here.
> /**
> * wilco_ec_debugfs_probe() - Create the debugfs node
> * @pdev: The platform device, probably created in core.c
> @@ -210,6 +270,8 @@ static int wilco_ec_debugfs_probe(struct platform_device *pdev)
> if (!debug_info->dir)
> return 0;
> debugfs_create_file("raw", 0644, debug_info->dir, NULL, &fops_raw);
> + debugfs_create_file("h1_gpio", 0444, debug_info->dir, NULL,
> + &fops_h1_gpio);
>
> return 0;
> }
> --
> 2.20.1
>
Thanks,
Enric
Thanks for the comments Enric! I'll resend in a day or two.
On Thu, Apr 11, 2019 at 3:43 PM Enric Balletbo Serra
<[email protected]> wrote:
>
> Hi Nick,
>
> Some comments below ...
>
> Missatge de Nick Crews <[email protected]> del dia dj., 11 d’abr.
> 2019 a les 0:09:
> >
> > As part of Chrome OS's FAFT (Fully Automated Firmware Testing)
> > tests, we need to ensure that the H1 chip is properly setting
> > some GPIO lines. The h1_gpio attribute exposes the state
> > of the lines:
> > - ENTRY_TO_FACT_MODE in BIT(0)
> > - SPI_CHROME_SEL in BIT(1)
> >
> > There are two reasons that I am exposing this in debugfs,
> > and not as a GPIO:
> > 1. This is only useful for testing, so end users shouldn't ever
> > care about this. In fact, if it passes the tests, then the value of
> > h1_gpio will always be 2, so it would be really uninteresting for users.
> > 2. This GPIO is not connected to, controlled by, or really even related
> > to the AP. The GPIO runs between the EC and the H1 security chip.
> >
> > Changes in v3:
> > - Fix documentation to correspond with formatting change in v2.
> > Changes in v2:
> > - Zero out the unused fields in the request.
> > - Format result as "%02x\n" instead of as a decimal.
>
> I don't really mind, but wouldn't be more clear prefix with 0x (0x%02x)?
I figured this would be easier for a program to parse, but you're right it
would be more clear. I can change it.
>
> >
> > Signed-off-by: Nick Crews <[email protected]>
> > ---
> > drivers/platform/chrome/wilco_ec/debugfs.c | 64 +++++++++++++++++++++-
>
> ABI documentation missing.
Whoops, will do. The Documentation is out of date with the raw
attribute as well,
so I'll fix that too.
>
> > 1 file changed, 63 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
> > index 17c4c9068aaf..1b93243c8954 100644
> > --- a/drivers/platform/chrome/wilco_ec/debugfs.c
> > +++ b/drivers/platform/chrome/wilco_ec/debugfs.c
> > @@ -4,7 +4,7 @@
> > *
> > * Copyright 2019 Google LLC
> > *
> > - * There is only one attribute used for debugging, called raw.
> > + * The raw attribute is very useful for EC debugging.
> > * You can write a hexadecimal sentence to raw, and that series of bytes
> > * will be sent to the EC. Then, you can read the bytes of response
> > * by reading from raw.
> > @@ -30,6 +30,16 @@
> > * Note that the first 32 bytes of the received MBOX[] will be printed,
> > * even if some of the data is junk. It is up to you to know how many of
> > * the first bytes of data are the actual response.
> > + *
> > + * There is also another debugfs attribute, called h1_gpio.
> > + * As part of Chrome OS's FAFT (Fully Automated Firmware Testing)
> > + * tests, we need to ensure that the H1 chip is properly setting
> > + * some GPIO lines. The h1_gpio attribute exposes the state
> > + * of the lines:
> > + * - ENTRY_TO_FACT_MODE in BIT(0)
> > + * - SPI_CHROME_SEL in BIT(1)
> > +
> > + * Output will formatted with "%02x\n".
> > */
> >
> > #include <linux/ctype.h>
> > @@ -189,6 +199,56 @@ static const struct file_operations fops_raw = {
> > .llseek = no_llseek,
> > };
> >
> > +#define CMD_KB_CHROME 0x88
> > +#define SUB_CMD_H1_GPIO 0x0A
> > +
> > +struct h1_gpio_status_request {
> > + u8 cmd; /* Always CMD_KB_CHROME */
> > + u8 reserved;
> > + u8 sub_cmd; /* Always SUB_CMD_H1_GPIO */
> > +} __packed;
> > +
> > +struct hi_gpio_status_response {
> > + u8 status; /* 0 if allowed */
> > + u8 val; /* BIT(0)=ENTRY_TO_FACT_MODE, BIT(1)=SPI_CHROME_SEL */
> > +} __packed;
> > +
> > +static ssize_t h1_gpio_read(struct file *file, char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct h1_gpio_status_request rq;
> > + struct hi_gpio_status_response rs;
> > + struct wilco_ec_message msg;
> > + char buf[4];
> > + int ret;
> > +
> > + memset(&rq, 0, sizeof(rq));
> > + rq.cmd = CMD_KB_CHROME;
> > + rq.sub_cmd = SUB_CMD_H1_GPIO;
> > +
> > + memset(&msg, 0, sizeof(msg));
> > + msg.type = WILCO_EC_MSG_LEGACY;
> > + msg.request_data = &rq;
> > + msg.request_size = sizeof(rq);
> > + msg.response_data = &rs;
> > + msg.response_size = sizeof(rs);
> > + ret = wilco_ec_mailbox(debug_info->ec, &msg);
> > + if (ret < 0)
> > + return ret;
> > + if (rs.status)
> > + return -EIO;
> > +
> > + sprintf(buf, "%02x\n", rs.val);
> > +
> > + return simple_read_from_buffer(user_buf, count, ppos, buf, sizeof(buf));
> > +}
> > +
> > +static const struct file_operations fops_h1_gpio = {
> > + .owner = THIS_MODULE,
> > + .read = h1_gpio_read,
> > + .llseek = no_llseek,
> > +};
> > +
>
> I think that I'd use the DEFINE_DEBUGFS_ATTRIBUTE here.
Will do.
>
> > /**
> > * wilco_ec_debugfs_probe() - Create the debugfs node
> > * @pdev: The platform device, probably created in core.c
> > @@ -210,6 +270,8 @@ static int wilco_ec_debugfs_probe(struct platform_device *pdev)
> > if (!debug_info->dir)
> > return 0;
> > debugfs_create_file("raw", 0644, debug_info->dir, NULL, &fops_raw);
> > + debugfs_create_file("h1_gpio", 0444, debug_info->dir, NULL,
> > + &fops_h1_gpio);
> >
> > return 0;
> > }
> > --
> > 2.20.1
> >
>
> Thanks,
> Enric