[PATCH v3 0/4] platform/chrome: vboot context support

[PATCH v3 0/4] platform/chrome: vboot context support

[Emilio López]

Hi everyone,

This series adds support for reading and writing the verified boot context
nvram space on the EC using the cros_ec sysfs interface.

The first patch improves is_visible() functionality, making it work
for binary attributes as well as normal ones. This is needed so the
sysfs group can be hidden when the EC doesn't offer any space for
the context.

The second patch documents the property used in the binding documents.
This used to live in the next patch on the previous versions of this series.

The third patch is the actual code implementing the interface to read
and write the context data.

The fourth patch adds the DT properties on peach boards which, judging by
the vendor tree, use the EC to store the verified boot context.

The series was tested on a peach pi and was found to work OK there. As
always, all comments and further tests are welcome :)

Cheers!
Emilio

Emilio López (4):
  sysfs: Support is_visible() on binary attributes
  Documentation: bindings: mfd: cros ec: document vbc EC property
  platform/chrome: Support reading/writing the vboot context
  ARM: dts: Enable EC vboot context support on Peach boards

 Documentation/devicetree/bindings/mfd/cros-ec.txt |   4 +
 arch/arm/boot/dts/exynos5420-peach-pit.dts        |   1 +
 arch/arm/boot/dts/exynos5800-peach-pi.dts         |   1 +
 drivers/platform/chrome/Makefile                  |   3 +-
 drivers/platform/chrome/cros_ec_dev.c             |   1 +
 drivers/platform/chrome/cros_ec_vbc.c             | 137 ++++++++++++++++++++++
 fs/sysfs/group.c                                  |  17 ++-
 include/linux/mfd/cros_ec.h                       |   1 +
 include/linux/sysfs.h                             |  18 ++-
 9 files changed, 176 insertions(+), 7 deletions(-)
 create mode 100644 drivers/platform/chrome/cros_ec_vbc.c

-- 
2.1.4

[PATCH 1/4] sysfs: Support is_visible() on binary attributes

[Emilio López]

According to the sysfs header file:

    "The returned value will replace static permissions defined in
     struct attribute or struct bin_attribute."

but this isn't the case, as is_visible is only called on struct attribute
only. This patch introduces a new is_bin_visible() function to implement
the same functionality for binary attributes, and updates documentation
accordingly.

Note that to keep functionality and code similar to that of normal
attributes, the mode is now checked as well to ensure it contains only
read/write permissions or SYSFS_PREALLOC.

Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
---

Changes from v1:
 - Don't overload is_visible, introduce is_bin_visible instead as
   discussed on the list.

Changes from v2:
 - Note change in mode checking on the commit log
 - Add Guenter's reviewed-by

 fs/sysfs/group.c      | 17 +++++++++++++++--
 include/linux/sysfs.h | 18 ++++++++++++++----
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
index 39a0199..51b56e6 100644
--- a/fs/sysfs/group.c
+++ b/fs/sysfs/group.c
@@ -73,13 +73,26 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
 	}
 
 	if (grp->bin_attrs) {
-		for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
+		for (i = 0, bin_attr = grp->bin_attrs; *bin_attr; i++, bin_attr++) {
+			umode_t mode = (*bin_attr)->attr.mode;
+
 			if (update)
 				kernfs_remove_by_name(parent,
 						(*bin_attr)->attr.name);
+			if (grp->is_bin_visible) {
+				mode = grp->is_bin_visible(kobj, *bin_attr, i);
+				if (!mode)
+					continue;
+			}
+
+			WARN(mode & ~(SYSFS_PREALLOC | 0664),
+			     "Attribute %s: Invalid permissions 0%o\n",
+			     (*bin_attr)->attr.name, mode);
+
+			mode &= SYSFS_PREALLOC | 0664;
 			error = sysfs_add_file_mode_ns(parent,
 					&(*bin_attr)->attr, true,
-					(*bin_attr)->attr.mode, NULL);
+					mode, NULL);
 			if (error)
 				break;
 		}
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index 9f65758..2f66050 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -64,10 +64,18 @@ do {							\
  *		a new subdirectory with this name.
  * @is_visible:	Optional: Function to return permissions associated with an
  *		attribute of the group. Will be called repeatedly for each
- *		attribute in the group. Only read/write permissions as well as
- *		SYSFS_PREALLOC are accepted. Must return 0 if an attribute is
- *		not visible. The returned value will replace static permissions
- *		defined in struct attribute or struct bin_attribute.
+ *		non-binary attribute in the group. Only read/write
+ *		permissions as well as SYSFS_PREALLOC are accepted. Must
+ *		return 0 if an attribute is not visible. The returned value
+ *		will replace static permissions defined in struct attribute.
+ * @is_bin_visible:
+ *		Optional: Function to return permissions associated with a
+ *		binary attribute of the group. Will be called repeatedly
+ *		for each binary attribute in the group. Only read/write
+ *		permissions as well as SYSFS_PREALLOC are accepted. Must
+ *		return 0 if a binary attribute is not visible. The returned
+ *		value will replace static permissions defined in
+ *		struct bin_attribute.
  * @attrs:	Pointer to NULL terminated list of attributes.
  * @bin_attrs:	Pointer to NULL terminated list of binary attributes.
  *		Either attrs or bin_attrs or both must be provided.
@@ -76,6 +84,8 @@ struct attribute_group {
 	const char		*name;
 	umode_t			(*is_visible)(struct kobject *,
 					      struct attribute *, int);
+	umode_t			(*is_bin_visible)(struct kobject *,
+						  struct bin_attribute *, int);
 	struct attribute	**attrs;
 	struct bin_attribute	**bin_attrs;
 };
-- 
2.1.4

[PATCH v3 2/4] Documentation: bindings: mfd: cros ec: document vbc EC property

[Emilio López]

Some EC implementations include a small nvram space used to store
verified boot context data. This boolean property lets us indicate
whether this space is available or not on a specific EC implementation.

Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
---

Patch is new in v3, split from 3/4

 Documentation/devicetree/bindings/mfd/cros-ec.txt | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
index 1777916..136e0c2 100644
--- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
+++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
@@ -34,6 +34,10 @@ Required properties (LPC):
 - compatible: "google,cros-ec-lpc"
 - reg: List of (IO address, size) pairs defining the interface uses
 
+Optional properties (all):
+- google,has-vbc-nvram: Some implementations of the EC include a small
+  nvram space used to store verified boot context data. This boolean flag
+  is used to specify whether this nvram is present or not.
 
 Example for I2C:
 
-- 
2.1.4

[PATCH v3 3/4] platform/chrome: Support reading/writing the vboot context

[Emilio López]

Some EC implementations include a small nvram space used to store
verified boot context data. This patch offers a way to expose this
data to userspace.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
---

Changes from v1:
 - Use is_bin_visible instead of is_visible

Changes from v2:
 - Changes suggested by Javier (makefile, size checks, etc)
 - Add his reviewed-by

 drivers/platform/chrome/Makefile      |   3 +-
 drivers/platform/chrome/cros_ec_dev.c |   1 +
 drivers/platform/chrome/cros_ec_vbc.c | 137 ++++++++++++++++++++++++++++++++++
 include/linux/mfd/cros_ec.h           |   1 +
 4 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 drivers/platform/chrome/cros_ec_vbc.c

diff --git a/drivers/platform/chrome/Makefile b/drivers/platform/chrome/Makefile
index 4a11b01..bc498bd 100644
--- a/drivers/platform/chrome/Makefile
+++ b/drivers/platform/chrome/Makefile
@@ -1,7 +1,8 @@
 
 obj-$(CONFIG_CHROMEOS_LAPTOP)	+= chromeos_laptop.o
 obj-$(CONFIG_CHROMEOS_PSTORE)	+= chromeos_pstore.o
-cros_ec_devs-objs               := cros_ec_dev.o cros_ec_sysfs.o cros_ec_lightbar.o
+cros_ec_devs-objs		:= cros_ec_dev.o cros_ec_sysfs.o \
+				   cros_ec_lightbar.o cros_ec_vbc.o
 obj-$(CONFIG_CROS_EC_CHARDEV)   += cros_ec_devs.o
 obj-$(CONFIG_CROS_EC_LPC)       += cros_ec_lpc.o
 obj-$(CONFIG_CROS_EC_PROTO)	+= cros_ec_proto.o
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index e8fcdc2..d19263f 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -32,6 +32,7 @@ static int ec_major;
 static const struct attribute_group *cros_ec_groups[] = {
 	&cros_ec_attr_group,
 	&cros_ec_lightbar_attr_group,
+	&cros_ec_vbc_attr_group,
 	NULL,
 };
 
diff --git a/drivers/platform/chrome/cros_ec_vbc.c b/drivers/platform/chrome/cros_ec_vbc.c
new file mode 100644
index 0000000..564a0d0
--- /dev/null
+++ b/drivers/platform/chrome/cros_ec_vbc.c
@@ -0,0 +1,137 @@
+/*
+ * cros_ec_vbc - Expose the vboot context nvram to userspace
+ *
+ * Copyright (C) 2015 Collabora Ltd.
+ *
+ * based on vendor driver,
+ *
+ * Copyright (C) 2012 The Chromium OS Authors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/slab.h>
+
+static ssize_t vboot_context_read(struct file *filp, struct kobject *kobj,
+				  struct bin_attribute *att, char *buf,
+				  loff_t pos, size_t count)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+					      class_dev);
+	struct cros_ec_device *ecdev = ec->ec_dev;
+	struct ec_params_vbnvcontext *params;
+	struct cros_ec_command *msg;
+	int err;
+	const size_t para_sz = sizeof(params->op);
+	const size_t resp_sz = sizeof(struct ec_response_vbnvcontext);
+	const size_t payload = max(para_sz, resp_sz);
+
+	msg = kmalloc(sizeof(*msg) + payload, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	/* NB: we only kmalloc()ated enough space for the op field */
+	params = (struct ec_params_vbnvcontext *)msg->data;
+	params->op = EC_VBNV_CONTEXT_OP_READ;
+
+	msg->version = EC_VER_VBNV_CONTEXT;
+	msg->command = EC_CMD_VBNV_CONTEXT;
+	msg->outsize = para_sz;
+	msg->insize = resp_sz;
+
+	err = cros_ec_cmd_xfer(ecdev, msg);
+	if (err < 0) {
+		dev_err(dev, "Error sending read request: %d\n", err);
+		kfree(msg);
+		return err;
+	}
+
+	memcpy(buf, msg->data, resp_sz);
+
+	kfree(msg);
+	return resp_sz;
+}
+
+static ssize_t vboot_context_write(struct file *filp, struct kobject *kobj,
+				   struct bin_attribute *attr, char *buf,
+				   loff_t pos, size_t count)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+					      class_dev);
+	struct cros_ec_device *ecdev = ec->ec_dev;
+	struct ec_params_vbnvcontext *params;
+	struct cros_ec_command *msg;
+	int err;
+	const size_t para_sz = sizeof(*params);
+	const size_t data_sz = sizeof(params->block);
+
+	/* Only write full values */
+	if (count != data_sz)
+		return -EINVAL;
+
+	msg = kmalloc(sizeof(*msg) + para_sz, GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	params = (struct ec_params_vbnvcontext *)msg->data;
+	params->op = EC_VBNV_CONTEXT_OP_WRITE;
+	memcpy(params->block, buf, data_sz);
+
+	msg->version = EC_VER_VBNV_CONTEXT;
+	msg->command = EC_CMD_VBNV_CONTEXT;
+	msg->outsize = para_sz;
+	msg->insize = 0;
+
+	err = cros_ec_cmd_xfer(ecdev, msg);
+	if (err < 0) {
+		dev_err(dev, "Error sending write request: %d\n", err);
+		kfree(msg);
+		return err;
+	}
+
+	kfree(msg);
+	return data_sz;
+}
+
+static umode_t cros_ec_vbc_is_visible(struct kobject *kobj,
+				      struct bin_attribute *a, int n)
+{
+	struct device *dev = container_of(kobj, struct device, kobj);
+	struct cros_ec_dev *ec = container_of(dev, struct cros_ec_dev,
+					      class_dev);
+	struct device_node *np = ec->ec_dev->dev->of_node;
+
+	if (IS_ENABLED(CONFIG_OF) && np) {
+		if (of_property_read_bool(np, "google,has-vbc-nvram"))
+			return a->attr.mode;
+	}
+
+	return 0;
+}
+
+static BIN_ATTR_RW(vboot_context, 16);
+
+static struct bin_attribute *cros_ec_vbc_bin_attrs[] = {
+	&bin_attr_vboot_context,
+	NULL
+};
+
+struct attribute_group cros_ec_vbc_attr_group = {
+	.name = "vbc",
+	.bin_attrs = cros_ec_vbc_bin_attrs,
+	.is_bin_visible = cros_ec_vbc_is_visible,
+};
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index da72671..494682c 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -255,5 +255,6 @@ int cros_ec_query_all(struct cros_ec_device *ec_dev);
 /* sysfs stuff */
 extern struct attribute_group cros_ec_attr_group;
 extern struct attribute_group cros_ec_lightbar_attr_group;
+extern struct attribute_group cros_ec_vbc_attr_group;
 
 #endif /* __LINUX_MFD_CROS_EC_H */
-- 
2.1.4

[PATCH v3 4/4] ARM: dts: Enable EC vboot context support on Peach boards

[Emilio López]

The Peach boards use the EC to store the vboot context information,
so add the corresponding properties on the EC node to indicate so.

Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
Acked-by: Javier Martinez Canillas <javier@osg.samsung.com>
Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
---

Changes from v1:
 - none

Changes from v2:
 - collect tags from Krzysztof & Javier

 arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 +
 arch/arm/boot/dts/exynos5800-peach-pi.dts  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
index 8f4d76c..ac02fb4 100644
--- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
+++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
@@ -935,6 +935,7 @@
 		pinctrl-0 = <&ec_spi_cs &ec_irq>;
 		reg = <0>;
 		spi-max-frequency = <3125000>;
+		google,has-vbc-nvram;
 
 		controller-data {
 			samsung,spi-feedback-delay = <1>;
diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
index 7d5b386..b60dec0 100644
--- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
+++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
@@ -898,6 +898,7 @@
 		pinctrl-0 = <&ec_spi_cs &ec_irq>;
 		reg = <0>;
 		spi-max-frequency = <3125000>;
+		google,has-vbc-nvram;
 
 		controller-data {
 			samsung,spi-feedback-delay = <1>;
-- 
2.1.4

Re: [PATCH 1/4] sysfs: Support is_visible() on binary attributes

[Emilio López]

This is v3, even if the subject doesn't say so. This is what happens 
when you forget to use --reroll-count and try to fix it manually :)

Emilio

On 21/09/15 10:38, Emilio López wrote:
> According to the sysfs header file:
>
>      "The returned value will replace static permissions defined in
>       struct attribute or struct bin_attribute."
>
> but this isn't the case, as is_visible is only called on struct attribute
> only. This patch introduces a new is_bin_visible() function to implement
> the same functionality for binary attributes, and updates documentation
> accordingly.
>
> Note that to keep functionality and code similar to that of normal
> attributes, the mode is now checked as well to ensure it contains only
> read/write permissions or SYSFS_PREALLOC.
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
> ---
>
> Changes from v1:
>   - Don't overload is_visible, introduce is_bin_visible instead as
>     discussed on the list.
>
> Changes from v2:
>   - Note change in mode checking on the commit log
>   - Add Guenter's reviewed-by
>
>   fs/sysfs/group.c      | 17 +++++++++++++++--
>   include/linux/sysfs.h | 18 ++++++++++++++----
>   2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/fs/sysfs/group.c b/fs/sysfs/group.c
> index 39a0199..51b56e6 100644
> --- a/fs/sysfs/group.c
> +++ b/fs/sysfs/group.c
> @@ -73,13 +73,26 @@ static int create_files(struct kernfs_node *parent, struct kobject *kobj,
>   	}
>
>   	if (grp->bin_attrs) {
> -		for (bin_attr = grp->bin_attrs; *bin_attr; bin_attr++) {
> +		for (i = 0, bin_attr = grp->bin_attrs; *bin_attr; i++, bin_attr++) {
> +			umode_t mode = (*bin_attr)->attr.mode;
> +
>   			if (update)
>   				kernfs_remove_by_name(parent,
>   						(*bin_attr)->attr.name);
> +			if (grp->is_bin_visible) {
> +				mode = grp->is_bin_visible(kobj, *bin_attr, i);
> +				if (!mode)
> +					continue;
> +			}
> +
> +			WARN(mode & ~(SYSFS_PREALLOC | 0664),
> +			     "Attribute %s: Invalid permissions 0%o\n",
> +			     (*bin_attr)->attr.name, mode);
> +
> +			mode &= SYSFS_PREALLOC | 0664;
>   			error = sysfs_add_file_mode_ns(parent,
>   					&(*bin_attr)->attr, true,
> -					(*bin_attr)->attr.mode, NULL);
> +					mode, NULL);
>   			if (error)
>   				break;
>   		}
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index 9f65758..2f66050 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -64,10 +64,18 @@ do {							\
>    *		a new subdirectory with this name.
>    * @is_visible:	Optional: Function to return permissions associated with an
>    *		attribute of the group. Will be called repeatedly for each
> - *		attribute in the group. Only read/write permissions as well as
> - *		SYSFS_PREALLOC are accepted. Must return 0 if an attribute is
> - *		not visible. The returned value will replace static permissions
> - *		defined in struct attribute or struct bin_attribute.
> + *		non-binary attribute in the group. Only read/write
> + *		permissions as well as SYSFS_PREALLOC are accepted. Must
> + *		return 0 if an attribute is not visible. The returned value
> + *		will replace static permissions defined in struct attribute.
> + * @is_bin_visible:
> + *		Optional: Function to return permissions associated with a
> + *		binary attribute of the group. Will be called repeatedly
> + *		for each binary attribute in the group. Only read/write
> + *		permissions as well as SYSFS_PREALLOC are accepted. Must
> + *		return 0 if a binary attribute is not visible. The returned
> + *		value will replace static permissions defined in
> + *		struct bin_attribute.
>    * @attrs:	Pointer to NULL terminated list of attributes.
>    * @bin_attrs:	Pointer to NULL terminated list of binary attributes.
>    *		Either attrs or bin_attrs or both must be provided.
> @@ -76,6 +84,8 @@ struct attribute_group {
>   	const char		*name;
>   	umode_t			(*is_visible)(struct kobject *,
>   					      struct attribute *, int);
> +	umode_t			(*is_bin_visible)(struct kobject *,
> +						  struct bin_attribute *, int);
>   	struct attribute	**attrs;
>   	struct bin_attribute	**bin_attrs;
>   };
>

Re: [PATCH v3 2/4] Documentation: bindings: mfd: cros ec: document vbc EC property

[Javier Martinez Canillas]

Hello Emilio,

On Mon, Sep 21, 2015 at 6:38 AM, Emilio López
<emilio.lopez@collabora.co.uk> wrote:
> Some EC implementations include a small nvram space used to store
> verified boot context data. This boolean property lets us indicate
> whether this space is available or not on a specific EC implementation.
>
> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>

Looks good to me.

Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>

Best regards,
Javier

Re: [PATCH v3 2/4] Documentation: bindings: mfd: cros ec: document vbc EC property

[Lee Jones]

On Mon, 21 Sep 2015, Emilio López wrote:

> Some EC implementations include a small nvram space used to store
> verified boot context data. This boolean property lets us indicate
> whether this space is available or not on a specific EC implementation.
> 
> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
> ---
> 
> Patch is new in v3, split from 3/4
> 
>  Documentation/devicetree/bindings/mfd/cros-ec.txt | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> index 1777916..136e0c2 100644
> --- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
> +++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> @@ -34,6 +34,10 @@ Required properties (LPC):
>  - compatible: "google,cros-ec-lpc"
>  - reg: List of (IO address, size) pairs defining the interface uses
>  
> +Optional properties (all):
> +- google,has-vbc-nvram: Some implementations of the EC include a small
> +  nvram space used to store verified boot context data. This boolean flag
> +  is used to specify whether this nvram is present or not.

Is there no way to check for this at runtime?

>  Example for I2C:
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v3 2/4] Documentation: bindings: mfd: cros ec: document vbc EC property

[Emilio López]

Hi Lee,

On 22/09/15 21:34, Lee Jones wrote:
> On Mon, 21 Sep 2015, Emilio López wrote:
>
>> Some EC implementations include a small nvram space used to store
>> verified boot context data. This boolean property lets us indicate
>> whether this space is available or not on a specific EC implementation.
>>
>> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
>> ---
>>
>> Patch is new in v3, split from 3/4
>>
>>   Documentation/devicetree/bindings/mfd/cros-ec.txt | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
>> index 1777916..136e0c2 100644
>> --- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
>> +++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
>> @@ -34,6 +34,10 @@ Required properties (LPC):
>>   - compatible: "google,cros-ec-lpc"
>>   - reg: List of (IO address, size) pairs defining the interface uses
>>
>> +Optional properties (all):
>> +- google,has-vbc-nvram: Some implementations of the EC include a small
>> +  nvram space used to store verified boot context data. This boolean flag
>> +  is used to specify whether this nvram is present or not.
>
> Is there no way to check for this at runtime?

You can issue a read and see if it succeeds, but the problem is that 
some EC implementations return success even when they don't implement 
the functionality.

Cheers,

Emilio

Re: [PATCH v3 2/4] Documentation: bindings: mfd: cros ec: document vbc EC property

[Lee Jones]

On Mon, 21 Sep 2015, Emilio López wrote:

> Some EC implementations include a small nvram space used to store
> verified boot context data. This boolean property lets us indicate
> whether this space is available or not on a specific EC implementation.
> 
> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
> ---
> 
> Patch is new in v3, split from 3/4
> 
>  Documentation/devicetree/bindings/mfd/cros-ec.txt | 4 ++++
>  1 file changed, 4 insertions(+)

Applied, thanks.

> diff --git a/Documentation/devicetree/bindings/mfd/cros-ec.txt b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> index 1777916..136e0c2 100644
> --- a/Documentation/devicetree/bindings/mfd/cros-ec.txt
> +++ b/Documentation/devicetree/bindings/mfd/cros-ec.txt
> @@ -34,6 +34,10 @@ Required properties (LPC):
>  - compatible: "google,cros-ec-lpc"
>  - reg: List of (IO address, size) pairs defining the interface uses
>  
> +Optional properties (all):
> +- google,has-vbc-nvram: Some implementations of the EC include a small
> +  nvram space used to store verified boot context data. This boolean flag
> +  is used to specify whether this nvram is present or not.
>  
>  Example for I2C:
>  

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 1/4] sysfs: Support is_visible() on binary attributes

[Greg KH]

On Mon, Sep 21, 2015 at 10:38:20AM -0300, Emilio López wrote:
> According to the sysfs header file:
> 
>     "The returned value will replace static permissions defined in
>      struct attribute or struct bin_attribute."
> 
> but this isn't the case, as is_visible is only called on struct attribute
> only. This patch introduces a new is_bin_visible() function to implement
> the same functionality for binary attributes, and updates documentation
> accordingly.
> 
> Note that to keep functionality and code similar to that of normal
> attributes, the mode is now checked as well to ensure it contains only
> read/write permissions or SYSFS_PREALLOC.
> 
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>

As this should probably go through the "platform drivers" maintainer,
I'll just give you this:

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

So it can go through their tree and not require me to just take this
one.

thanks,

greg k-h

Re: [PATCH 1/4] sysfs: Support is_visible() on binary attributes

[Olof Johansson]

On Sun, Oct 04, 2015 at 07:33:34PM +0100, Greg KH wrote:
> On Mon, Sep 21, 2015 at 10:38:20AM -0300, Emilio L?pez wrote:
> > According to the sysfs header file:
> > 
> >     "The returned value will replace static permissions defined in
> >      struct attribute or struct bin_attribute."
> > 
> > but this isn't the case, as is_visible is only called on struct attribute
> > only. This patch introduces a new is_bin_visible() function to implement
> > the same functionality for binary attributes, and updates documentation
> > accordingly.
> > 
> > Note that to keep functionality and code similar to that of normal
> > attributes, the mode is now checked as well to ensure it contains only
> > read/write permissions or SYSFS_PREALLOC.
> > 
> > Reviewed-by: Guenter Roeck <linux@roeck-us.net>
> > Signed-off-by: Emilio L?pez <emilio.lopez@collabora.co.uk>
> 
> As this should probably go through the "platform drivers" maintainer,
> I'll just give you this:
> 
> Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> So it can go through their tree and not require me to just take this
> one.

Thanks, applied to the chrome-platform tree now.


-Olof

Re: [PATCH v3 3/4] platform/chrome: Support reading/writing the vboot context

[Olof Johansson]

On Mon, Sep 21, 2015 at 10:38:22AM -0300, Emilio L??pez wrote:
> Some EC implementations include a small nvram space used to store
> verified boot context data. This patch offers a way to expose this
> data to userspace.
> 
> Reviewed-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Signed-off-by: Emilio L??pez <emilio.lopez@collabora.co.uk>

Applied, thanks!


-Olof

Re: [PATCH v3 4/4] ARM: dts: Enable EC vboot context support on Peach boards

[Kukjin Kim]

On 09/21/15 22:38, Emilio López wrote:
> The Peach boards use the EC to store the vboot context information,
> so add the corresponding properties on the EC node to indicate so.
> 
> Reviewed-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
> Acked-by: Javier Martinez Canillas <javier@osg.samsung.com>
> Signed-off-by: Emilio López <emilio.lopez@collabora.co.uk>
> ---
> 
> Changes from v1:
>  - none
> 
> Changes from v2:
>  - collect tags from Krzysztof & Javier
> 
>  arch/arm/boot/dts/exynos5420-peach-pit.dts | 1 +
>  arch/arm/boot/dts/exynos5800-peach-pi.dts  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos5420-peach-pit.dts b/arch/arm/boot/dts/exynos5420-peach-pit.dts
> index 8f4d76c..ac02fb4 100644
> --- a/arch/arm/boot/dts/exynos5420-peach-pit.dts
> +++ b/arch/arm/boot/dts/exynos5420-peach-pit.dts
> @@ -935,6 +935,7 @@
>  		pinctrl-0 = <&ec_spi_cs &ec_irq>;
>  		reg = <0>;
>  		spi-max-frequency = <3125000>;
> +		google,has-vbc-nvram;
>  
>  		controller-data {
>  			samsung,spi-feedback-delay = <1>;
> diff --git a/arch/arm/boot/dts/exynos5800-peach-pi.dts b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> index 7d5b386..b60dec0 100644
> --- a/arch/arm/boot/dts/exynos5800-peach-pi.dts
> +++ b/arch/arm/boot/dts/exynos5800-peach-pi.dts
> @@ -898,6 +898,7 @@
>  		pinctrl-0 = <&ec_spi_cs &ec_irq>;
>  		reg = <0>;
>  		spi-max-frequency = <3125000>;
> +		google,has-vbc-nvram;
>  
>  		controller-data {
>  			samsung,spi-feedback-delay = <1>;

Applied, thanks.

- Kukjin