[v8] platform: x86: Add ChromeOS ACPI device driver

Commit Message

Muhammad Usama Anjum April 15, 2022, 5:08 p.m. UTC

From: Enric Balletbo i Serra <[e-mail]>

The x86 Chromebooks have ChromeOS ACPI device. This driver attaches to
the ChromeOS ACPI device and exports the values reported by ACPI in a
sysfs directory. This data isn't present in ACPI tables when read
through ACPI tools, hence a driver is needed to do it. The driver gets
data from firmware using ACPI component of the kernel. The ACPI values
are presented in string form (numbers as decimal values) or binary
blobs, and can be accessed as the contents of the appropriate read only
files in the standard ACPI device's sysfs directory tree. This data is
consumed by the ChromeOS user space.

Cc: Rafael J. Wysocki <[e-mail]>
Cc: Dmitry Torokhov <[e-mail]>
Cc: Hans de Goede <[e-mail]>
Acked-by: Rafael J. Wysocki <[e-mail]>
Signed-off-by: Enric Balletbo i Serra <[e-mail]>
Signed-off-by: Muhammad Usama Anjum <[e-mail]>
---
Changes in v8:
- Change struct platform_driver name from chromeos_acpi_driver to
  chromeos_acpi_device_driver

Changes in v7:
- Rename acpi_chromeos Kconfig option to chromeos_acpi
- Change this acpi driver to platform driver
- Minor cosmetic changes

There were the following concerns on v4 which have been delt with in
v5/v6:
- Remove BINF.{0,1,4} from sysfs as they are reserved and not used
  anymore
- Reword the description of MECK
- Change function name from chromeos_acpi_alloc_name() to
  chromeos_acpi_gen_file_name()
- Remove local variable obj in chromeos_acpi_add_method()
- Replace usage of dev_info() to dev_dbg()
- Improve the description of the patch
- Add the firmware interface document which serves as primary
  documentation and garantees that this interface will not change
- GGL0001 is valid PNP ID of the Google. PNP ID can be used with the
  ACPI devices. Consensus was developed on it in discussion of v4.

Changes in v6:
- Correct authorship and path email's From
- Add changelog between v4 and v5 in detail
- Add copywrite year 2022
- Improve the description and add concerns from V4 which have been fixed

Changes in v5:
- Improve the description of the patch
- Document firmware interface
- Update sysfs interface documentation
- Remove binf{0,1,4} as they have been deprecated
- Update some cleanup logic in case of error
  - Remove freeing of chromeos_acpi.root explicitely in
    chromeos_acpi_device_remove() as it'll be automatically freed by
    chromeos_acpi_remove_groups()
  - If sysfs_create_groups() fails in chromeos_acpi_process_mlst(),
    cleanup all groups
- Cosmetic changes

Changes in v4:
https://lore.kernel.org/lkml/[e-mail]/t/
- Add COMPILE_TEST to increase build coverage.
- Add sysfs ABI documentation.
- Rebased on top of 5.7-rc1 and solve conflicts.
- Cc ACPI maintainers.

Changes in v3:
- Use attribute groups instead of adding files "by hand".
- Do not use "raw" kobject to create directories.
- Do not abuse of the platform_device interface. Remove it.

Changes in v2:
- Note that this version is a total rework, with those major changes:
- Use lists to track dinamically allocated attributes and groups.
- Use sysfs binary attributes to store the ACPI contents.
- Remove all the functionalities except the one that creates the sysfs files.
---
 .../ABI/testing/sysfs-driver-chromeos-acpi    | 126 +++++
 .../acpi/chromeos-acpi-device.rst             | 363 +++++++++++++
 Documentation/firmware-guide/acpi/index.rst   |   1 +
 drivers/platform/x86/Kconfig                  |  11 +
 drivers/platform/x86/Makefile                 |   3 +
 drivers/platform/x86/chromeos_acpi.c          | 513 ++++++++++++++++++
 6 files changed, 1017 insertions(+)
 create mode 100644 Documentation/ABI/testing/sysfs-driver-chromeos-acpi
 create mode 100644 Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
 create mode 100644 drivers/platform/x86/chromeos_acpi.c

Comments

Barnabás Pőcze April 15, 2022, 7:47 p.m. UTC | #1

Hi


2022. április 15., péntek 19:08 keltezéssel, Muhammad Usama Anjum <[e-mail]> írta

> From: Enric Balletbo i Serra <[e-mail]>
>
> The x86 Chromebooks have ChromeOS ACPI device. This driver attaches to
> the ChromeOS ACPI device and exports the values reported by ACPI in a
> sysfs directory. This data isn't present in ACPI tables when read
> through ACPI tools, hence a driver is needed to do it. The driver gets
> data from firmware using ACPI component of the kernel. The ACPI values
> are presented in string form (numbers as decimal values) or binary
> blobs, and can be accessed as the contents of the appropriate read only
> files in the standard ACPI device's sysfs directory tree. This data is
> consumed by the ChromeOS user space.
>
> Cc: Rafael J. Wysocki <[e-mail]>
> Cc: Dmitry Torokhov <[e-mail]>
> Cc: Hans de Goede <[e-mail]>
> Acked-by: Rafael J. Wysocki <[e-mail]>
> Signed-off-by: Enric Balletbo i Serra <[e-mail]>
> Signed-off-by: Muhammad Usama Anjum <[e-mail]>
> ---
> Changes in v8:
> - Change struct platform_driver name from chromeos_acpi_driver to
>   chromeos_acpi_device_driver
>
> Changes in v7:
> - Rename acpi_chromeos Kconfig option to chromeos_acpi
> - Change this acpi driver to platform driver
> - Minor cosmetic changes
>
> There were the following concerns on v4 which have been delt with in
> v5/v6:
> - Remove BINF.{0,1,4} from sysfs as they are reserved and not used
>   anymore
> - Reword the description of MECK
> - Change function name from chromeos_acpi_alloc_name() to
>   chromeos_acpi_gen_file_name()
> - Remove local variable obj in chromeos_acpi_add_method()
> - Replace usage of dev_info() to dev_dbg()
> - Improve the description of the patch
> - Add the firmware interface document which serves as primary
>   documentation and garantees that this interface will not change
> - GGL0001 is valid PNP ID of the Google. PNP ID can be used with the
>   ACPI devices. Consensus was developed on it in discussion of v4.
>
> Changes in v6:
> - Correct authorship and path email's From
> - Add changelog between v4 and v5 in detail
> - Add copywrite year 2022
> - Improve the description and add concerns from V4 which have been fixed
>
> Changes in v5:
> - Improve the description of the patch
> - Document firmware interface
> - Update sysfs interface documentation
> - Remove binf{0,1,4} as they have been deprecated
> - Update some cleanup logic in case of error
>   - Remove freeing of chromeos_acpi.root explicitely in
>     chromeos_acpi_device_remove() as it'll be automatically freed by
>     chromeos_acpi_remove_groups()
>   - If sysfs_create_groups() fails in chromeos_acpi_process_mlst(),
>     cleanup all groups
> - Cosmetic changes
>
> Changes in v4:
> https://lore.kernel.org/lkml/[e-mail]/t/
> - Add COMPILE_TEST to increase build coverage.
> - Add sysfs ABI documentation.
> - Rebased on top of 5.7-rc1 and solve conflicts.
> - Cc ACPI maintainers.
>
> Changes in v3:
> - Use attribute groups instead of adding files "by hand".
> - Do not use "raw" kobject to create directories.
> - Do not abuse of the platform_device interface. Remove it.
>
> Changes in v2:
> - Note that this version is a total rework, with those major changes:
> - Use lists to track dinamically allocated attributes and groups.
> - Use sysfs binary attributes to store the ACPI contents.
> - Remove all the functionalities except the one that creates the sysfs files.
> ---
>  .../ABI/testing/sysfs-driver-chromeos-acpi    | 126 +++++
>  .../acpi/chromeos-acpi-device.rst             | 363 +++++++++++++
>  Documentation/firmware-guide/acpi/index.rst   |   1 +
>  drivers/platform/x86/Kconfig                  |  11 +
>  drivers/platform/x86/Makefile                 |   3 +
>  drivers/platform/x86/chromeos_acpi.c          | 513 ++++++++++++++++++
>  6 files changed, 1017 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>  create mode 100644 Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>  create mode 100644 drivers/platform/x86/chromeos_acpi.c
>
> [...]
> diff --git a/drivers/platform/x86/chromeos_acpi.c b/drivers/platform/x86/chromeos_acpi.c
> new file mode 100644
> index 0000000000000..de86119a446b8
> --- /dev/null
> +++ b/drivers/platform/x86/chromeos_acpi.c
> @@ -0,0 +1,513 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ChromeOS specific ACPI extensions
> + *
> + * Copyright 2011 Google, Inc.
> + * Copyright 2020-2022 Google LLC
> + *
> + * This file is a rework and part of the code is ported from chromeos-3.18
> + * kernel and was originally written by Vadim Bendebury <[e-mail]>.
> + *
> + * This driver attaches to the ChromeOS ACPI device and then exports the
> + * values reported by the ACPI in a sysfs directory. All values are
> + * presented in the string form (numbers as decimal values) and can be
> + * accessed as the contents of the appropriate read only files in the
> + * sysfs directory tree.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +
> +/*
> + * ACPI method name for MLST; the response for this method is a package of
> + * strings listing the methods which should be reflected in sysfs.
> + */
> +#define MLST "MLST"
> +
> +/*
> + * The default list of methods the ChromeOS ACPI device is supposed to export,
> + * if the MLST method is not present or is poorly formed.  The MLST method
> + * itself is included, to aid in debugging.
> + */
> +static char *chromeos_acpi_default_methods[] = {
> +	"CHSW", "HWID", "BINF", "GPIO", "CHNV", "FWID", "FRID", MLST
> +};
> +
> +/*
> + * Representation of a single sysfs attribute. In addition to the standard
> + * bin_attribute structure has a list of these structures (to keep track for
> + * de-allocation when removing the driver) and a pointer to the actual
> + * attribute name and value, reported when accessing the appropriate sysfs
> + * file.
> + */
> +struct chromeos_acpi_attribute {
> +	struct bin_attribute bin_attr;
> +	struct list_head list;
> +	char *name;
> +	char *data;
> +};
> +
> +/*
> + * Representation of a sysfs attribute group (a sub directory in the device's
> + * sysfs directory). In addition to the standard structure has lists to allow
> + * to keep track of the allocated structures.
> + */
> +struct chromeos_acpi_attribute_group {
> +	struct attribute_group group;
> +	struct list_head attribs;
> +	struct list_head list;
> +	char *name;
> +};
> +
> +/*
> + * This is the main structure, we use it to store data and adds links pointing
> + * at lists of allocated attributes and attribute groups.
> + */
> +struct chromeos_acpi_dev {
> +	struct chromeos_acpi_attribute_group *root;
> +	const struct attribute_group **dev_groups;
> +	struct list_head groups;
> +	unsigned int num_groups;
> +	unsigned int num_attrs;
> +};
> +
> +static struct chromeos_acpi_dev chromeos_acpi;
> +
> +static ssize_t chromeos_acpi_read_bin_attribute(struct file *filp,
> +						struct kobject *kobj,
> +						struct bin_attribute *bin_attr,
> +						char *buffer, loff_t pos,
> +						size_t count)
> +{
> +	struct chromeos_acpi_attribute *info = bin_attr->private;
> +
> +	return memory_read_from_buffer(buffer, count, &pos, info->data,
> +				       info->bin_attr.size);
> +}
> +
> +static char *chromeos_acpi_gen_file_name(char *name, int count, int index)
> +{
> +	char *str;
> +
> +	if (count == 1)
> +		str = kstrdup(name, GFP_KERNEL);
> +	else
> +		str = kasprintf(GFP_KERNEL, "%s.%d", name, index);
> +
> +	return str;
> +}
> +
> +static int
> +chromeos_acpi_add_attr(struct chromeos_acpi_attribute_group *aag,
> +		       union acpi_object *element, char *name,
> +		       int count, int index)
> +{
> +	struct chromeos_acpi_attribute *info;
> +	char buffer[24]; /* enough to store a u64 and null character */
> +	int length;
> +	int ret;
> +
> +	/* Files BINF.{0,1,4} are historical and no longer used. */
> +	if (!strcmp(name, "BINF") && (index == 0 || index == 1 || index == 4))
> +		return 0;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->name = chromeos_acpi_gen_file_name(name, count, index);
> +	if (!info->name) {
> +		ret = -ENOMEM;
> +		goto free_attribute;
> +	}
> +	sysfs_bin_attr_init(&info->bin_attr);
> +	info->bin_attr.attr.name = info->name;
> +	info->bin_attr.attr.mode = 0444;
> +
> +	switch (element->type) {
> +	case ACPI_TYPE_BUFFER:
> +		length = element->buffer.length;
> +		info->data = kmemdup(element->buffer.pointer,
> +				     length, GFP_KERNEL);
> +		break;
> +	case ACPI_TYPE_INTEGER:
> +		length = snprintf(buffer, sizeof(buffer), "%d",
> +				  (int)element->integer.value);
> +		info->data = kmemdup(buffer, length, GFP_KERNEL);

You can use `kasprintf()` here, no?


> +		break;
> +	case ACPI_TYPE_STRING:
> +		length = element->string.length + 1;
> +		info->data = kstrdup(element->string.pointer, GFP_KERNEL);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto free_attr_name;
> +	}
> +
> +	if (!info->data) {
> +		ret = -ENOMEM;
> +		goto free_attr_name;
> +	}
> +
> +	info->bin_attr.size = length;
> +	info->bin_attr.read = chromeos_acpi_read_bin_attribute;
> +	info->bin_attr.private = info;
> +
> +	INIT_LIST_HEAD(&info->list);

(technically you only need to initialize the list head explicitly, so this is not
 strictly needed; there are other examples of such `INIT_LIST_HEAD()` calls
 in the code)


> +
> +	list_add(&info->list, &aag->attribs);
> +	return 0;
> +
> +free_attr_name:
> +	kfree(info->name);
> +free_attribute:
> +	kfree(info);
> +	return ret;
> +}
> +
> +static void
> +chromeos_acpi_remove_attribs(struct chromeos_acpi_attribute_group *aag)
> +{
> +	struct chromeos_acpi_attribute *attr, *tmp_attr;
> +
> +	list_for_each_entry_safe(attr, tmp_attr, &aag->attribs, list) {
> +		kfree(attr->name);
> +		kfree(attr->data);
> +		kfree(attr);
> +	}
> +}
> +
> +static int
> +chromeos_acpi_add_attribs_to_group(struct chromeos_acpi_attribute_group *aag,
> +				   unsigned int num_attrs)
> +{
> +	struct chromeos_acpi_attribute *attr;
> +	int count = 0;
> +
> +	aag->group.bin_attrs = kcalloc(num_attrs + 1,
> +				       sizeof(*aag->group.bin_attrs),
> +				       GFP_KERNEL);
> +	if (!aag->group.bin_attrs)
> +		return -ENOMEM;
> +
> +	list_for_each_entry(attr, &aag->attribs, list) {
> +		aag->group.bin_attrs[count] = &attr->bin_attr;
> +		count++;
> +	}
> +
> +	chromeos_acpi.num_groups++;
> +	list_add(&aag->list, &chromeos_acpi.groups);
> +
> +	return 0;
> +}
> +
> +/**
> + * chromeos_acpi_add_group() - Create a sysfs group including attributes
> + *			       representing a nested ACPI package
> + *
> + * @obj: Package contents as returned by ACPI
> + * @name: Name of the group
> + * @num_attrs: Number of attributes of this package
> + * @index: Index number of this particular group
> + *
> + * The created group is called @name in case there is a single instance, or
> + * @name.@index otherwise.
> + *
> + * All group and attribute storage allocations are included in the lists for
> + * tracking of allocated memory.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +static int chromeos_acpi_add_group(union acpi_object *obj, char *name,
> +				   int num_attrs, int index)
> +{
> +	struct chromeos_acpi_attribute_group *aag;
> +	union acpi_object *element;
> +	int i, count, ret;
> +
> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
> +	if (!aag)
> +		return -ENOMEM;
> +	aag->name = chromeos_acpi_gen_file_name(name, num_attrs, index);
> +	if (!aag->name) {
> +		ret = -ENOMEM;
> +		goto free_group;
> +	}
> +
> +	INIT_LIST_HEAD(&aag->attribs);
> +	INIT_LIST_HEAD(&aag->list);
> +
> +	count = obj->package.count;
> +	element = obj->package.elements;
> +	for (i = 0; i < count; i++, element++) {
> +		ret = chromeos_acpi_add_attr(aag, element, name, count, i);
> +		if (ret)
> +			goto free_group_attr;
> +	}
> +
> +	aag->group.name = aag->name;
> +
> +	ret = chromeos_acpi_add_attribs_to_group(aag, count);
> +	if (ret)
> +		goto free_group_attr;
> +
> +	return 0;
> +
> +free_group_attr:
> +	chromeos_acpi_remove_attribs(aag);
> +	kfree(aag->name);
> +free_group:
> +	kfree(aag);
> +	return ret;
> +}
> +
> +static void chromeos_acpi_remove_groups(void)
> +{
> +	struct chromeos_acpi_attribute_group *aag, *tmp_aag;
> +
> +	list_for_each_entry_safe(aag, tmp_aag, &chromeos_acpi.groups, list) {
> +		chromeos_acpi_remove_attribs(aag);
> +		kfree(aag->group.bin_attrs);
> +		kfree(aag->name);
> +		kfree(aag);
> +	}
> +}
> +
> +/**
> + * chromeos_acpi_handle_package() - Create sysfs group including attributes
> + *				    representing an ACPI package
> + *
> + * @pdev: Platform device
> + * @obj: Package contents as returned by ACPI
> + * @name: Name of the group
> + *
> + * Scalar objects included in the package get sysfs attributes created for
> + * them. Nested packages are passed to a function creating a sysfs group per
> + * package.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +static int chromeos_acpi_handle_package(struct platform_device *pdev,
> +					union acpi_object *obj, char *name)
> +{
> +	struct device *dev = &pdev->dev;
> +	int count = obj->package.count;
> +	union acpi_object *element;
> +	int i, ret;
> +
> +	element = obj->package.elements;
> +	for (i = 0; i < count; i++, element++) {
> +		if (element->type == ACPI_TYPE_BUFFER ||
> +		    element->type == ACPI_TYPE_STRING ||
> +		    element->type == ACPI_TYPE_INTEGER) {
> +			/* Create a single attribute in the root directory */
> +			ret = chromeos_acpi_add_attr(chromeos_acpi.root,
> +						     element, name,
> +						     count, i);
> +			if (ret) {
> +				dev_err(dev, "error adding attributes (%d)\n",
> +					ret);
> +				return ret;
> +			}
> +			chromeos_acpi.num_attrs++;
> +		} else if (element->type == ACPI_TYPE_PACKAGE) {
> +			/* Create a group of attributes */
> +			ret = chromeos_acpi_add_group(element, name, count, i);
> +			if (ret) {
> +				dev_err(dev, "error adding a group (%d)\n",
> +					ret);
> +				return ret;
> +			}
> +		} else {
> +			if (ret) {

`ret` can be potentially uninitialized here, no?


> +				dev_err(dev, "error on element type (%d)\n",
> +					ret);
> +				return -EINVAL;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * chromeos_acpi_add_method() - Evaluate an ACPI method and create sysfs
> + *				attributes
> + *
> + * @pdev: Platform device
> + * @name: Name of the method to evaluate
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(dev, "failed to retrieve %s (%d)\n", name, status);

(maybe `acpi_format_exception(status)` would be more meaningful than the numeric value)


> +		return status;

This return value is potentially propagated to become the return value of
the probe function. The problem is that it is not a negative errno that the probe
method should return but rather an ACPI status code.


> +	}
> +
> +	if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
> +		ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
> +
> +	kfree(output.pointer);
> +	return ret;
> +}
> +
> +/**
> + * chromeos_acpi_process_mlst() - Evaluate the MLST method and add methods
> + *				  listed in the response
> + *
> + * @pdev: Platform device
> + *
> + * Returns: 0 if successful, non-zero if error.
> + */
> +static int chromeos_acpi_process_mlst(struct platform_device *pdev)
> +{
> +	struct chromeos_acpi_attribute_group *aag;
> +	char name[ACPI_NAMESEG_SIZE + 1];
> +	union acpi_object *element, *obj;
> +	struct device *dev = &pdev->dev;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	acpi_status status;
> +	int ret = 0;
> +	int size;
> +	int i;
> +
> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
> +				      &output);
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
> +	obj = output.pointer;
> +	if (obj->type != ACPI_TYPE_PACKAGE) {
> +		ret = -EINVAL;
> +		goto free_acpi_buffer;
> +	}
> +
> +	element = obj->package.elements;
> +	for (i = 0; i < obj->package.count; i++, element++) {
> +		if (element->type == ACPI_TYPE_STRING) {
> +			size = min(element->string.length + 1,
> +				   (u32)ACPI_NAMESEG_SIZE + 1);

Is truncation a real possibility? Shouldn't it abort/etc. in that case?
And `min()` "returns" a u32 here but `size` is an `int`.


> +			strscpy(name, element->string.pointer, size);
> +			ret = chromeos_acpi_add_method(pdev, name);
> +			if (ret) {
> +				chromeos_acpi_remove_groups();
> +				break;

Is just a `break` is enough here to handle the error? If this is not fatal,
then why is a `dev_warn()` not sufficient? If this is fatal, why continue
with the rest of the function?


> +			}
> +		}
> +	}
> +
> +	/* Add root attributes to the main group */
> +	ret = chromeos_acpi_add_attribs_to_group(chromeos_acpi.root,
> +						 chromeos_acpi.num_attrs);
> +	if (ret)
> +		goto free_acpi_buffer;
> +
> +	chromeos_acpi.dev_groups = kcalloc(chromeos_acpi.num_groups + 1,
> +					   sizeof(struct attribute_group),
> +					   GFP_KERNEL);
> +
> +	i = 0;
> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
> +		chromeos_acpi.dev_groups[i] = &aag->group;
> +		i++;
> +	}
> +
> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
> +	if (ret) {
> +		kfree(chromeos_acpi.dev_groups);
> +
> +		/* Remove allocated chromeos acpi groups and attributes */
> +		chromeos_acpi_remove_groups();
> +	}
> +
> +free_acpi_buffer:
> +	kfree(output.pointer);
> +	return ret;
> +}
> +
> +static int chromeos_acpi_device_probe(struct platform_device *pdev)
> +{
> +	struct chromeos_acpi_attribute_group *aag;
> +	struct device *dev = &pdev->dev;
> +	int i, ret;
> +
> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
> +	if (!aag)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&aag->attribs);
> +	INIT_LIST_HEAD(&aag->list);
> +	INIT_LIST_HEAD(&chromeos_acpi.groups);
> +
> +	chromeos_acpi.root = aag;
> +
> +	/*
> +	 * Attempt to add methods by querying the device's MLST method
> +	 * for the list of methods.
> +	 */
> +	if (!chromeos_acpi_process_mlst(pdev))
> +		return 0;
> +
> +	dev_dbg(dev, "falling back to default list of methods\n");
> +
> +	for (i = 0; i < ARRAY_SIZE(chromeos_acpi_default_methods); i++) {
> +		ret = chromeos_acpi_add_method(pdev,
> +					       chromeos_acpi_default_methods[i]);
> +		if (ret) {
> +			dev_err(dev, "failed to add default methods (%d)\n",
> +				ret);
> +			goto free_group_root;
> +		}
> +	}
> +
> +	return 0;
> +
> +free_group_root:
> +	kfree(chromeos_acpi.root);
> +	return ret;
> +}
> +
> +static int chromeos_acpi_device_remove(struct platform_device *pdev)
> +{
> +	/* Remove sysfs groups */
> +	sysfs_remove_groups(&pdev->dev.kobj, chromeos_acpi.dev_groups);
> +	kfree(chromeos_acpi.dev_groups);
> +
> +	/* Remove allocated chromeos acpi groups and attributes */
> +	chromeos_acpi_remove_groups();
> +
> +	return 0;
> +}
> +
> +/* GGL is valid PNP ID of Google. PNP ID can be used with the ACPI devices. */
> +static const struct acpi_device_id chromeos_device_ids[] = {
> +	{ "GGL0001", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, chromeos_device_ids);
> +
> +static struct platform_driver chromeos_acpi_device_driver = {
> +	.probe = chromeos_acpi_device_probe,
> +	.remove = chromeos_acpi_device_remove,
> +	.driver = {
> +		.name   = "chromeos-acpi",
> +		.acpi_match_table = ACPI_PTR(chromeos_device_ids)
> +	}
> +};
> +
> +module_platform_driver(chromeos_acpi_device_driver);
> +
> +MODULE_AUTHOR("Enric Balletbo i Serra <[e-mail]>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ChromeOS specific ACPI extensions");
> --
> 2.30.2
>
>

Excuse me if I have missed previous discussions about it, but I am confused by
the design. Why is a global variable needed here? The global struct's members
are overwritten in the probe method in any case.

And checkpatch reports that no MAINTAINERS entry has been added for the new file.
(And it appears to be right if I have not missed anything.)


Regards,
Barnabás Pőcze

Muhammad Usama Anjum April 18, 2022, 5:31 p.m. UTC | #2

Hi,

Thanks for reviewing.

>> +	switch (element->type) {
>> +	case ACPI_TYPE_BUFFER:
>> +		length = element->buffer.length;
>> +		info->data = kmemdup(element->buffer.pointer,
>> +				     length, GFP_KERNEL);
>> +		break;
>> +	case ACPI_TYPE_INTEGER:
>> +		length = snprintf(buffer, sizeof(buffer), "%d",
>> +				  (int)element->integer.value);
>> +		info->data = kmemdup(buffer, length, GFP_KERNEL);
> 
> You can use `kasprintf()` here, no?
> 
Choosing kmemdup vs k*printf depends on what is being achieved. Usage of
kmemdup indicates that only the memory is being duplicated here. While
in case of k*printf, some transformation is done. Thus in normal memory
duplication cases like this, the usage of kmemdup makes code more
readable and seems preferable to me.

>> +static int chromeos_acpi_handle_package(struct platform_device *pdev,
>> +					union acpi_object *obj, char *name)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	int count = obj->package.count;
>> +	union acpi_object *element;
>> +	int i, ret;
>> +
>> +	element = obj->package.elements;
>> +	for (i = 0; i < count; i++, element++) {
>> +		if (element->type == ACPI_TYPE_BUFFER ||
>> +		    element->type == ACPI_TYPE_STRING ||
>> +		    element->type == ACPI_TYPE_INTEGER) {
>> +			/* Create a single attribute in the root directory */
>> +			ret = chromeos_acpi_add_attr(chromeos_acpi.root,
>> +						     element, name,
>> +						     count, i);
>> +			if (ret) {
>> +				dev_err(dev, "error adding attributes (%d)\n",
>> +					ret);
>> +				return ret;
>> +			}
>> +			chromeos_acpi.num_attrs++;
>> +		} else if (element->type == ACPI_TYPE_PACKAGE) {
>> +			/* Create a group of attributes */
>> +			ret = chromeos_acpi_add_group(element, name, count, i);
>> +			if (ret) {
>> +				dev_err(dev, "error adding a group (%d)\n",
>> +					ret);
>> +				return ret;
>> +			}
>> +		} else {
>> +			if (ret) {
> 
> `ret` can be potentially uninitialized here, no?
> 
> 
This driver is written in this way that the element->type must be from
these 4 types always. Having a second look, it seems a bit illogical to
check the value of ret if some other element->type happen to be present.
I'll remove this `if (ret)` condition entirely.

>> +				dev_err(dev, "error on element type (%d)\n",
>> +					ret);
>> +				return -EINVAL;
>> +			}
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * chromeos_acpi_add_method() - Evaluate an ACPI method and create sysfs
>> + *				attributes
>> + *
>> + * @pdev: Platform device
>> + * @name: Name of the method to evaluate
>> + *
>> + * Return: 0 on success, non-zero on failure
>> + */
>> +static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	acpi_status status;
>> +	int ret = 0;
>> +
>> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
>> +	if (ACPI_FAILURE(status)) {
>> +		dev_err(dev, "failed to retrieve %s (%d)\n", name, status);
> 
> (maybe `acpi_format_exception(status)` would be more meaningful than the numeric value)
>
Yeah, it'll be more better. I'll use this macro.

> 
>> +		return status;
> 
> This return value is potentially propagated to become the return value of
> the probe function. The problem is that it is not a negative errno that the probe
> method should return but rather an ACPI status code.
> 
Good catch. I'll return -EINVAL here instead.

> 
>> +	}
>> +
>> +	if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
>> +		ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
>> +
>> +	kfree(output.pointer);
>> +	return ret;
>> +}
>> +
>> +/**
>> + * chromeos_acpi_process_mlst() - Evaluate the MLST method and add methods
>> + *				  listed in the response
>> + *
>> + * @pdev: Platform device
>> + *
>> + * Returns: 0 if successful, non-zero if error.
>> + */
>> +static int chromeos_acpi_process_mlst(struct platform_device *pdev)
>> +{
>> +	struct chromeos_acpi_attribute_group *aag;
>> +	char name[ACPI_NAMESEG_SIZE + 1];
>> +	union acpi_object *element, *obj;
>> +	struct device *dev = &pdev->dev;
>> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	acpi_status status;
>> +	int ret = 0;
>> +	int size;
>> +	int i;
>> +
>> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
>> +				      &output);
>> +	if (ACPI_FAILURE(status))
>> +		return status;
>> +
>> +	obj = output.pointer;
>> +	if (obj->type != ACPI_TYPE_PACKAGE) {
>> +		ret = -EINVAL;
>> +		goto free_acpi_buffer;
>> +	}
>> +
>> +	element = obj->package.elements;
>> +	for (i = 0; i < obj->package.count; i++, element++) {
>> +		if (element->type == ACPI_TYPE_STRING) {
>> +			size = min(element->string.length + 1,
>> +				   (u32)ACPI_NAMESEG_SIZE + 1);
> 
> Is truncation a real possibility? Shouldn't it abort/etc. in that case?
> And `min()` "returns" a u32 here but `size` is an `int`.
> 
There is not likely, but can still happen. We want to not abort and
carry on with truncated string. I'll update the type of size to u32.

> 
>> +			strscpy(name, element->string.pointer, size);
>> +			ret = chromeos_acpi_add_method(pdev, name);
>> +			if (ret) {
>> +				chromeos_acpi_remove_groups();
>> +				break;
> 
> Is just a `break` is enough here to handle the error? If this is not fatal,
> then why is a `dev_warn()` not sufficient? If this is fatal, why continue
> with the rest of the function?
> 
I'll have a look.

> 
> Excuse me if I have missed previous discussions about it, but I am confused by
> the design. Why is a global variable needed here? The global struct's members
> are overwritten in the probe method in any case.
> 
The global variable is needed in probe and remove functions. Researching
this more, I think dev->driver_data can be used instead of global
variable. I'll test it. Thanks for mentioning it.

> And checkpatch reports that no MAINTAINERS entry has been added for the new file.
> (And it appears to be right if I have not missed anything.)
> 
I thought there will be default maintainer of this directory. But there
isn't. I'll send a separate email to discuss this.

> 
> Regards,
> Barnabás Pőcze

Muhammad Usama Anjum April 18, 2022, 5:57 p.m. UTC | #3

Hi Benson,

We are very near to getting this patch in. I can see that you are
maintaining Chrome Platform support. This driver is in
drivers/platform/x86/chromeos_acpi.c. Can you maintain this driver as
well? I'll add you to the maintainers.

Thank you,
Usama

On 4/15/22 10:08 PM, Muhammad Usama Anjum wrote:
> From: Enric Balletbo i Serra <[e-mail]>
> 
> The x86 Chromebooks have ChromeOS ACPI device. This driver attaches to
> the ChromeOS ACPI device and exports the values reported by ACPI in a
> sysfs directory. This data isn't present in ACPI tables when read
> through ACPI tools, hence a driver is needed to do it. The driver gets
> data from firmware using ACPI component of the kernel. The ACPI values
> are presented in string form (numbers as decimal values) or binary
> blobs, and can be accessed as the contents of the appropriate read only
> files in the standard ACPI device's sysfs directory tree. This data is
> consumed by the ChromeOS user space.
> 
> Cc: Rafael J. Wysocki <[e-mail]>
> Cc: Dmitry Torokhov <[e-mail]>
> Cc: Hans de Goede <[e-mail]>
> Acked-by: Rafael J. Wysocki <[e-mail]>
> Signed-off-by: Enric Balletbo i Serra <[e-mail]>
> Signed-off-by: Muhammad Usama Anjum <[e-mail]>
> ---
> Changes in v8:
> - Change struct platform_driver name from chromeos_acpi_driver to
>   chromeos_acpi_device_driver
> 
> Changes in v7:
> - Rename acpi_chromeos Kconfig option to chromeos_acpi
> - Change this acpi driver to platform driver
> - Minor cosmetic changes
> 
> There were the following concerns on v4 which have been delt with in
> v5/v6:
> - Remove BINF.{0,1,4} from sysfs as they are reserved and not used
>   anymore
> - Reword the description of MECK
> - Change function name from chromeos_acpi_alloc_name() to
>   chromeos_acpi_gen_file_name()
> - Remove local variable obj in chromeos_acpi_add_method()
> - Replace usage of dev_info() to dev_dbg()
> - Improve the description of the patch
> - Add the firmware interface document which serves as primary
>   documentation and garantees that this interface will not change
> - GGL0001 is valid PNP ID of the Google. PNP ID can be used with the
>   ACPI devices. Consensus was developed on it in discussion of v4.
> 
> Changes in v6:
> - Correct authorship and path email's From
> - Add changelog between v4 and v5 in detail
> - Add copywrite year 2022
> - Improve the description and add concerns from V4 which have been fixed
> 
> Changes in v5:
> - Improve the description of the patch
> - Document firmware interface
> - Update sysfs interface documentation
> - Remove binf{0,1,4} as they have been deprecated
> - Update some cleanup logic in case of error
>   - Remove freeing of chromeos_acpi.root explicitely in
>     chromeos_acpi_device_remove() as it'll be automatically freed by
>     chromeos_acpi_remove_groups()
>   - If sysfs_create_groups() fails in chromeos_acpi_process_mlst(),
>     cleanup all groups
> - Cosmetic changes
> 
> Changes in v4:
> https://lore.kernel.org/lkml/[e-mail]/t/
> - Add COMPILE_TEST to increase build coverage.
> - Add sysfs ABI documentation.
> - Rebased on top of 5.7-rc1 and solve conflicts.
> - Cc ACPI maintainers.
> 
> Changes in v3:
> - Use attribute groups instead of adding files "by hand".
> - Do not use "raw" kobject to create directories.
> - Do not abuse of the platform_device interface. Remove it.
> 
> Changes in v2:
> - Note that this version is a total rework, with those major changes:
> - Use lists to track dinamically allocated attributes and groups.
> - Use sysfs binary attributes to store the ACPI contents.
> - Remove all the functionalities except the one that creates the sysfs files.
> ---
>  .../ABI/testing/sysfs-driver-chromeos-acpi    | 126 +++++
>  .../acpi/chromeos-acpi-device.rst             | 363 +++++++++++++
>  Documentation/firmware-guide/acpi/index.rst   |   1 +
>  drivers/platform/x86/Kconfig                  |  11 +
>  drivers/platform/x86/Makefile                 |   3 +
>  drivers/platform/x86/chromeos_acpi.c          | 513 ++++++++++++++++++
>  6 files changed, 1017 insertions(+)
>  create mode 100644 Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>  create mode 100644 Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>  create mode 100644 drivers/platform/x86/chromeos_acpi.c
> 
> diff --git a/Documentation/ABI/testing/sysfs-driver-chromeos-acpi b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
> new file mode 100644
> index 0000000000000..b74fbca62f4c3
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
> @@ -0,0 +1,126 @@
> +What:		/sys/bus/platform/devices/GGL0001:00/BINF.2
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows information about the current boot of
> +		the active EC firmware.
> +		  * 0 - Read only (recovery) firmware.
> +		  * 1 - Rewritable firmware.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/BINF.3
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows information about the current boot of
> +		the active main	firmware type.
> +		  * 0 - Recovery.
> +		  * 1 - Normal.
> +		  * 2 - Developer.
> +		  * 3 - Netboot (factory installation only).
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/CHSW
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the switch position for the Chrome OS specific
> +		hardware switches.
> +		  * 0   - No changes.
> +		  * 2   - Recovery button was pressed when firmware booted.
> +		  * 4   - Recovery button was pressed when EC firmware booted.
> +		  * 32  - Developer switch was enabled when firmware booted.
> +		  * 512 - Firmware write protect was disabled when firmware
> +			  booted.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/FMAP
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the physical memory address of the start of
> +		the main processor firmware flashmap.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/FRID
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the firmware version for the read-only portion
> +		of the main processor firmware.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/FWID
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the firmware version for the rewritable portion
> +		of the main processor firmware.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.0
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the type of the GPIO signal for the Chrome OS
> +		specific GPIO assignments.
> +		  * 1   - Recovery button.
> +		  * 2   - Developer mode switch.
> +		  * 3   - Firmware write protect switch.
> +		  * 256 to 511 - Debug header GPIO 0 to GPIO 255.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.1
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the signal attributes of the GPIO signal.
> +		  * 0 - Signal is active low.
> +		  * 1 - Signal is active high.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.2
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the GPIO number on the specified GPIO
> +		controller.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.3
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the name of the GPIO controller.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/HWID
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the hardware ID for the Chromebook.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/MECK
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This binary file returns the SHA-1 or SHA-256 hash that is
> +		read out of the Management Engine extend registers during
> +		boot. The hash is exported vi ACPI so the OS can verify that
> +		the Management Engine firmware has not changed. If Management
> +		Engine is not present, or if the firmware was unable to read the
> +		extend registers, this buffer size can be zero.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/VBNV.0
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the offset in CMOS bank 0 of the verified boot
> +		non-volatile storage block, counting from the first writable
> +		CMOS byte (that is, 'offset = 0' is the byte following the 14
> +		bytes of clock data).
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/VBNV.1
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This file shows the size in bytes of the verified boot
> +		non-volatile storage block.
> +
> +What:		/sys/bus/platform/devices/GGL0001:00/VDAT
> +Date:		April 2022
> +KernelVersion:	5.19
> +Description:
> +		This binary file returns the verified boot data block shared
> +		between the firmware verification step and the kernel
> +		verification step.
> diff --git a/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
> new file mode 100644
> index 0000000000000..ef3cc980bad54
> --- /dev/null
> +++ b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
> @@ -0,0 +1,363 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +=====================
> +Chrome OS ACPI Device
> +=====================
> +
> +Hardware functionality specific to Chrome OS is exposed through a Chrome OS ACPI device.
> +The plug and play ID of a Chrome OS ACPI device is GGL0001. GGL is valid PNP ID of Google.
> +PNP ID can be used with the ACPI devices accourding to the guidelines. The following ACPI
> +objects are supported:
> +
> +.. flat-table:: Supported ACPI Objects
> +   :widths: 1 2
> +   :header-rows: 1
> +
> +   * - Object
> +     - Description
> +
> +   * - CHSW
> +     - Chrome OS switch positions
> +
> +   * - HWID
> +     - Chrome OS hardware ID
> +
> +   * - FWID
> +     - Chrome OS firmware version
> +
> +   * - FRID
> +     - Chrome OS read-only firmware version
> +
> +   * - BINF
> +     - Chrome OS boot information
> +
> +   * - GPIO
> +     - Chrome OS GPIO assignments
> +
> +   * - VBNV
> +     - Chrome OS NVRAM locations
> +
> +   * - VDTA
> +     - Chrome OS verified boot data
> +
> +   * - FMAP
> +     - Chrome OS flashmap base address
> +
> +   * - MLST
> +     - Chrome OS method list
> +
> +CHSW (Chrome OS switch positions)
> +=================================
> +This control method returns the switch positions for Chrome OS specific hardware switches.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +An integer containing the switch positions as bitfields:
> +
> +.. flat-table::
> +   :widths: 1 2
> +
> +   * - 0x00000002
> +     - Recovery button was pressed when x86 firmware booted.
> +
> +   * - 0x00000004
> +     - Recovery button was pressed when EC firmware booted. (required if EC EEPROM is
> +       rewritable; otherwise optional)
> +
> +   * - 0x00000020
> +     - Developer switch was enabled when x86 firmware booted.
> +
> +   * - 0x00000200
> +     - Firmware write protect was disabled when x86 firmware booted. (required if
> +       firmware write protect is controlled through x86 BIOS; otherwise optional)
> +
> +All other bits are reserved and should be set to 0.
> +
> +HWID (Chrome OS hardware ID)
> +============================
> +This control method returns the hardware ID for the Chromebook.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +A null-terminated ASCII string containing the hardware ID from the Model-Specific Data area of
> +EEPROM.
> +
> +Note that the hardware ID can be up to 256 characters long, including the terminating null.
> +
> +FWID (Chrome OS firmware version)
> +=================================
> +This control method returns the firmware version for the rewritable portion of the main
> +processor firmware.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +A null-terminated ASCII string containing the complete firmware version for the rewritable
> +portion of the main processor firmware.
> +
> +FRID (Chrome OS read-only firmware version)
> +===========================================
> +This control method returns the firmware version for the read-only portion of the main
> +processor firmware.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +A null-terminated ASCII string containing the complete firmware version for the read-only
> +(bootstrap + recovery ) portion of the main processor firmware.
> +
> +BINF (Chrome OS boot information)
> +=================================
> +This control method returns information about the current boot.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +
> +.. code-block::
> +
> +   Package {
> +           Reserved1
> +           Reserved2
> +           Active EC Firmware
> +           Active Main Firmware Type
> +           Reserved5
> +   }
> +
> +.. flat-table::
> +   :widths: 1 1 2
> +   :header-rows: 1
> +
> +   * - Field
> +     - Format
> +     - Description
> +
> +   * - Reserved1
> +     - DWORD
> +     - Set to 256 (0x100). This indicates this field is no longer used.
> +
> +   * - Reserved2
> +     - DWORD
> +     - Set to 256 (0x100). This indicates this field is no longer used.
> +
> +   * - Active EC firmware
> +     - DWORD
> +     - The EC firmware which was used during boot.
> +
> +       - 0 - Read-only (recovery) firmware
> +       - 1 - Rewritable firmware.
> +
> +       Set to 0 if EC firmware is always read-only.
> +
> +   * - Active Main Firmware Type
> +     - DWORD
> +     - The main firmware type which was used during boot.
> +
> +       - 0 - Recovery
> +       - 1 - Normal
> +       - 2 - Developer
> +       - 3 - netboot (factory installation only)
> +
> +       Other values are reserved.
> +
> +   * - Reserved5
> +     - DWORD
> +     - Set to 256 (0x100). This indicates this field is no longer used.
> +
> +GPIO (Chrome OS GPIO assignments)
> +=================================
> +This control method returns information about Chrome OS specific GPIO assignments for
> +Chrome OS hardware, so the kernel can directly control that hardware.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +.. code-block::
> +
> +        Package {
> +                Package {
> +                        // First GPIO assignment
> +                        Signal Type        //DWORD
> +                        Attributes         //DWORD
> +                        Controller Offset  //DWORD
> +                        Controller Name    //ASCIIZ
> +                },
> +                ...
> +                Package {
> +                        // Last GPIO assignment
> +                        Signal Type        //DWORD
> +                        Attributes         //DWORD
> +                        Controller Offset  //DWORD
> +                        Controller Name    //ASCIIZ
> +                }
> +        }
> +
> +Where ASCIIZ means a null-terminated ASCII string.
> +
> +.. flat-table::
> +   :widths: 1 1 2
> +   :header-rows: 1
> +
> +   * - Field
> +     - Format
> +     - Description
> +
> +   * - Signal Type
> +     - DWORD
> +     - Type of GPIO signal
> +
> +       - 0x00000001 - Recovery button
> +       - 0x00000002 - Developer mode switch
> +       - 0x00000003 - Firmware write protect switch
> +       - 0x00000100 - Debug header GPIO 0
> +       - ...
> +       - 0x000001FF - Debug header GPIO 255
> +
> +       Other values are reserved.
> +
> +   * - Attributes
> +     - DWORD
> +     - Signal attributes as bitfields:
> +
> +       - 0x00000001 - Signal is active-high (for button, a GPIO value
> +         of 1 means the button is pressed; for switches, a GPIO value
> +         of 1 means the switch is enabled). If this bit is 0, the signal
> +         is active low. Set to 0 for debug header GPIOs.
> +
> +   * - Controller Offset
> +     - DWORD
> +     - GPIO number on the specified controller.
> +
> +   * - Controller Name
> +     - ASCIIZ
> +     - Name of the controller for the GPIO.
> +       Currently supported names:
> +       "NM10" - Intel NM10 chip
> +
> +VBNV (Chrome OS NVRAM locations)
> +================================
> +This control method returns information about the NVRAM (CMOS) locations used to
> +communicate with the BIOS.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +.. code-block::
> +
> +        Package {
> +                NV Storage Block Offset  //DWORD
> +                NV Storage Block Size    //DWORD
> +        }
> +
> +.. flat-table::
> +   :widths: 1 1 2
> +   :header-rows: 1
> +
> +   * - Field
> +     - Format
> +     - Description
> +
> +   * - NV Storage Block Offset
> +     - DWORD
> +     - Offset in CMOS bank 0 of the verified boot non-volatile storage block, counting from
> +       the first writable CMOS byte (that is, offset=0 is the byte following the 14 bytes of
> +       clock data).
> +
> +   * - NV Storage Block Size
> +     - DWORD
> +     - Size in bytes of the verified boot non-volatile storage block.
> +
> +FMAP (Chrome OS flashmap address)
> +=================================
> +This control method returns the physical memory address of the start of the main processor
> +firmware flashmap.
> +
> +Arguments:
> +----------
> +None
> +
> +NoneResult code:
> +----------------
> +A DWORD containing the physical memory address of the start of the main processor firmware
> +flashmap.
> +
> +VDTA (Chrome OS verified boot data)
> +===================================
> +This control method returns the verified boot data block shared between the firmware
> +verification step and the kernel verification step.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +A buffer containing the verified boot data block.
> +
> +MECK (Management Engine Checksum)
> +=================================
> +This control method returns the SHA-1 or SHA-256 hash that is read out of the Management
> +Engine extend registers during boot. The hash is exported via ACPI so the OS can verify that
> +the ME firmware has not changed. If Management Engine is not present, or if the firmware was
> +unable to read the extend registers, this buffer can be zero.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +A buffer containing the ME hash.
> +
> +MLST (Chrome OS method list)
> +============================
> +This control method returns a list of the other control methods supported by the Chrome OS
> +hardware device.
> +
> +Arguments:
> +----------
> +None
> +
> +Result code:
> +------------
> +A package containing a list of null-terminated ASCII strings, one for each control method
> +supported by the Chrome OS hardware device, not including the MLST method itself.
> +For this version of the specification, the result is:
> +
> +.. code-block::
> +
> +        Package {
> +                "CHSW",
> +                "FWID",
> +                "HWID",
> +                "FRID",
> +                "BINF",
> +                "GPIO",
> +                "VBNV",
> +                "FMAP",
> +                "VDTA",
> +                "MECK"
> +        }
> diff --git a/Documentation/firmware-guide/acpi/index.rst b/Documentation/firmware-guide/acpi/index.rst
> index b053b0c3d6969..b6a42f4ffe032 100644
> --- a/Documentation/firmware-guide/acpi/index.rst
> +++ b/Documentation/firmware-guide/acpi/index.rst
> @@ -29,3 +29,4 @@ ACPI Support
>     non-d0-probe
>     extcon-intel-int3496
>     intel-pmc-mux
> +   chromeos-acpi-device
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 5d9dd70e4e0f5..53ddfe6f99b06 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -322,6 +322,17 @@ config ASUS_NB_WMI
>  	  If you have an ACPI-WMI compatible Asus Notebook, say Y or M
>  	  here.
>  
> +config CHROMEOS_ACPI
> +	tristate "ChromeOS specific ACPI extensions"
> +	depends on ACPI || COMPILE_TEST
> +	help
> +	  This driver provides the firmware interface for the services
> +	  exported through the ChromeOS interfaces when using ChromeOS
> +	  ACPI firmware.
> +
> +	  If you have an ACPI-compatible Chromebook, say Y or M
> +	  here.
> +
>  config ASUS_TF103C_DOCK
>  	tristate "Asus TF103C 2-in-1 keyboard dock"
>  	depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index fe4d4c8970efa..ae68f4fa9a6c4 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -44,6 +44,9 @@ obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
>  # Cisco/Meraki
>  obj-$(CONFIG_MERAKI_MX100)	+= meraki-mx100.o
>  
> +# Chrome
> +obj-$(CONFIG_CHROMEOS_ACPI)	+= chromeos_acpi.o
> +
>  # Dell
>  obj-$(CONFIG_X86_PLATFORM_DRIVERS_DELL)		+= dell/
>  
> diff --git a/drivers/platform/x86/chromeos_acpi.c b/drivers/platform/x86/chromeos_acpi.c
> new file mode 100644
> index 0000000000000..de86119a446b8
> --- /dev/null
> +++ b/drivers/platform/x86/chromeos_acpi.c
> @@ -0,0 +1,513 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * ChromeOS specific ACPI extensions
> + *
> + * Copyright 2011 Google, Inc.
> + * Copyright 2020-2022 Google LLC
> + *
> + * This file is a rework and part of the code is ported from chromeos-3.18
> + * kernel and was originally written by Vadim Bendebury <[e-mail]>.
> + *
> + * This driver attaches to the ChromeOS ACPI device and then exports the
> + * values reported by the ACPI in a sysfs directory. All values are
> + * presented in the string form (numbers as decimal values) and can be
> + * accessed as the contents of the appropriate read only files in the
> + * sysfs directory tree.
> + */
> +
> +#include <linux/acpi.h>
> +#include <linux/platform_device.h>
> +#include <linux/kernel.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +
> +/*
> + * ACPI method name for MLST; the response for this method is a package of
> + * strings listing the methods which should be reflected in sysfs.
> + */
> +#define MLST "MLST"
> +
> +/*
> + * The default list of methods the ChromeOS ACPI device is supposed to export,
> + * if the MLST method is not present or is poorly formed.  The MLST method
> + * itself is included, to aid in debugging.
> + */
> +static char *chromeos_acpi_default_methods[] = {
> +	"CHSW", "HWID", "BINF", "GPIO", "CHNV", "FWID", "FRID", MLST
> +};
> +
> +/*
> + * Representation of a single sysfs attribute. In addition to the standard
> + * bin_attribute structure has a list of these structures (to keep track for
> + * de-allocation when removing the driver) and a pointer to the actual
> + * attribute name and value, reported when accessing the appropriate sysfs
> + * file.
> + */
> +struct chromeos_acpi_attribute {
> +	struct bin_attribute bin_attr;
> +	struct list_head list;
> +	char *name;
> +	char *data;
> +};
> +
> +/*
> + * Representation of a sysfs attribute group (a sub directory in the device's
> + * sysfs directory). In addition to the standard structure has lists to allow
> + * to keep track of the allocated structures.
> + */
> +struct chromeos_acpi_attribute_group {
> +	struct attribute_group group;
> +	struct list_head attribs;
> +	struct list_head list;
> +	char *name;
> +};
> +
> +/*
> + * This is the main structure, we use it to store data and adds links pointing
> + * at lists of allocated attributes and attribute groups.
> + */
> +struct chromeos_acpi_dev {
> +	struct chromeos_acpi_attribute_group *root;
> +	const struct attribute_group **dev_groups;
> +	struct list_head groups;
> +	unsigned int num_groups;
> +	unsigned int num_attrs;
> +};
> +
> +static struct chromeos_acpi_dev chromeos_acpi;
> +
> +static ssize_t chromeos_acpi_read_bin_attribute(struct file *filp,
> +						struct kobject *kobj,
> +						struct bin_attribute *bin_attr,
> +						char *buffer, loff_t pos,
> +						size_t count)
> +{
> +	struct chromeos_acpi_attribute *info = bin_attr->private;
> +
> +	return memory_read_from_buffer(buffer, count, &pos, info->data,
> +				       info->bin_attr.size);
> +}
> +
> +static char *chromeos_acpi_gen_file_name(char *name, int count, int index)
> +{
> +	char *str;
> +
> +	if (count == 1)
> +		str = kstrdup(name, GFP_KERNEL);
> +	else
> +		str = kasprintf(GFP_KERNEL, "%s.%d", name, index);
> +
> +	return str;
> +}
> +
> +static int
> +chromeos_acpi_add_attr(struct chromeos_acpi_attribute_group *aag,
> +		       union acpi_object *element, char *name,
> +		       int count, int index)
> +{
> +	struct chromeos_acpi_attribute *info;
> +	char buffer[24]; /* enough to store a u64 and null character */
> +	int length;
> +	int ret;
> +
> +	/* Files BINF.{0,1,4} are historical and no longer used. */
> +	if (!strcmp(name, "BINF") && (index == 0 || index == 1 || index == 4))
> +		return 0;
> +
> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->name = chromeos_acpi_gen_file_name(name, count, index);
> +	if (!info->name) {
> +		ret = -ENOMEM;
> +		goto free_attribute;
> +	}
> +	sysfs_bin_attr_init(&info->bin_attr);
> +	info->bin_attr.attr.name = info->name;
> +	info->bin_attr.attr.mode = 0444;
> +
> +	switch (element->type) {
> +	case ACPI_TYPE_BUFFER:
> +		length = element->buffer.length;
> +		info->data = kmemdup(element->buffer.pointer,
> +				     length, GFP_KERNEL);
> +		break;
> +	case ACPI_TYPE_INTEGER:
> +		length = snprintf(buffer, sizeof(buffer), "%d",
> +				  (int)element->integer.value);
> +		info->data = kmemdup(buffer, length, GFP_KERNEL);
> +		break;
> +	case ACPI_TYPE_STRING:
> +		length = element->string.length + 1;
> +		info->data = kstrdup(element->string.pointer, GFP_KERNEL);
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		goto free_attr_name;
> +	}
> +
> +	if (!info->data) {
> +		ret = -ENOMEM;
> +		goto free_attr_name;
> +	}
> +
> +	info->bin_attr.size = length;
> +	info->bin_attr.read = chromeos_acpi_read_bin_attribute;
> +	info->bin_attr.private = info;
> +
> +	INIT_LIST_HEAD(&info->list);
> +
> +	list_add(&info->list, &aag->attribs);
> +	return 0;
> +
> +free_attr_name:
> +	kfree(info->name);
> +free_attribute:
> +	kfree(info);
> +	return ret;
> +}
> +
> +static void
> +chromeos_acpi_remove_attribs(struct chromeos_acpi_attribute_group *aag)
> +{
> +	struct chromeos_acpi_attribute *attr, *tmp_attr;
> +
> +	list_for_each_entry_safe(attr, tmp_attr, &aag->attribs, list) {
> +		kfree(attr->name);
> +		kfree(attr->data);
> +		kfree(attr);
> +	}
> +}
> +
> +static int
> +chromeos_acpi_add_attribs_to_group(struct chromeos_acpi_attribute_group *aag,
> +				   unsigned int num_attrs)
> +{
> +	struct chromeos_acpi_attribute *attr;
> +	int count = 0;
> +
> +	aag->group.bin_attrs = kcalloc(num_attrs + 1,
> +				       sizeof(*aag->group.bin_attrs),
> +				       GFP_KERNEL);
> +	if (!aag->group.bin_attrs)
> +		return -ENOMEM;
> +
> +	list_for_each_entry(attr, &aag->attribs, list) {
> +		aag->group.bin_attrs[count] = &attr->bin_attr;
> +		count++;
> +	}
> +
> +	chromeos_acpi.num_groups++;
> +	list_add(&aag->list, &chromeos_acpi.groups);
> +
> +	return 0;
> +}
> +
> +/**
> + * chromeos_acpi_add_group() - Create a sysfs group including attributes
> + *			       representing a nested ACPI package
> + *
> + * @obj: Package contents as returned by ACPI
> + * @name: Name of the group
> + * @num_attrs: Number of attributes of this package
> + * @index: Index number of this particular group
> + *
> + * The created group is called @name in case there is a single instance, or
> + * @name.@index otherwise.
> + *
> + * All group and attribute storage allocations are included in the lists for
> + * tracking of allocated memory.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +static int chromeos_acpi_add_group(union acpi_object *obj, char *name,
> +				   int num_attrs, int index)
> +{
> +	struct chromeos_acpi_attribute_group *aag;
> +	union acpi_object *element;
> +	int i, count, ret;
> +
> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
> +	if (!aag)
> +		return -ENOMEM;
> +	aag->name = chromeos_acpi_gen_file_name(name, num_attrs, index);
> +	if (!aag->name) {
> +		ret = -ENOMEM;
> +		goto free_group;
> +	}
> +
> +	INIT_LIST_HEAD(&aag->attribs);
> +	INIT_LIST_HEAD(&aag->list);
> +
> +	count = obj->package.count;
> +	element = obj->package.elements;
> +	for (i = 0; i < count; i++, element++) {
> +		ret = chromeos_acpi_add_attr(aag, element, name, count, i);
> +		if (ret)
> +			goto free_group_attr;
> +	}
> +
> +	aag->group.name = aag->name;
> +
> +	ret = chromeos_acpi_add_attribs_to_group(aag, count);
> +	if (ret)
> +		goto free_group_attr;
> +
> +	return 0;
> +
> +free_group_attr:
> +	chromeos_acpi_remove_attribs(aag);
> +	kfree(aag->name);
> +free_group:
> +	kfree(aag);
> +	return ret;
> +}
> +
> +static void chromeos_acpi_remove_groups(void)
> +{
> +	struct chromeos_acpi_attribute_group *aag, *tmp_aag;
> +
> +	list_for_each_entry_safe(aag, tmp_aag, &chromeos_acpi.groups, list) {
> +		chromeos_acpi_remove_attribs(aag);
> +		kfree(aag->group.bin_attrs);
> +		kfree(aag->name);
> +		kfree(aag);
> +	}
> +}
> +
> +/**
> + * chromeos_acpi_handle_package() - Create sysfs group including attributes
> + *				    representing an ACPI package
> + *
> + * @pdev: Platform device
> + * @obj: Package contents as returned by ACPI
> + * @name: Name of the group
> + *
> + * Scalar objects included in the package get sysfs attributes created for
> + * them. Nested packages are passed to a function creating a sysfs group per
> + * package.
> + *
> + * Return: 0 on success, negative errno on failure.
> + */
> +static int chromeos_acpi_handle_package(struct platform_device *pdev,
> +					union acpi_object *obj, char *name)
> +{
> +	struct device *dev = &pdev->dev;
> +	int count = obj->package.count;
> +	union acpi_object *element;
> +	int i, ret;
> +
> +	element = obj->package.elements;
> +	for (i = 0; i < count; i++, element++) {
> +		if (element->type == ACPI_TYPE_BUFFER ||
> +		    element->type == ACPI_TYPE_STRING ||
> +		    element->type == ACPI_TYPE_INTEGER) {
> +			/* Create a single attribute in the root directory */
> +			ret = chromeos_acpi_add_attr(chromeos_acpi.root,
> +						     element, name,
> +						     count, i);
> +			if (ret) {
> +				dev_err(dev, "error adding attributes (%d)\n",
> +					ret);
> +				return ret;
> +			}
> +			chromeos_acpi.num_attrs++;
> +		} else if (element->type == ACPI_TYPE_PACKAGE) {
> +			/* Create a group of attributes */
> +			ret = chromeos_acpi_add_group(element, name, count, i);
> +			if (ret) {
> +				dev_err(dev, "error adding a group (%d)\n",
> +					ret);
> +				return ret;
> +			}
> +		} else {
> +			if (ret) {
> +				dev_err(dev, "error on element type (%d)\n",
> +					ret);
> +				return -EINVAL;
> +			}
> +		}
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * chromeos_acpi_add_method() - Evaluate an ACPI method and create sysfs
> + *				attributes
> + *
> + * @pdev: Platform device
> + * @name: Name of the method to evaluate
> + *
> + * Return: 0 on success, non-zero on failure
> + */
> +static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	acpi_status status;
> +	int ret = 0;
> +
> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
> +	if (ACPI_FAILURE(status)) {
> +		dev_err(dev, "failed to retrieve %s (%d)\n", name, status);
> +		return status;
> +	}
> +
> +	if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
> +		ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
> +
> +	kfree(output.pointer);
> +	return ret;
> +}
> +
> +/**
> + * chromeos_acpi_process_mlst() - Evaluate the MLST method and add methods
> + *				  listed in the response
> + *
> + * @pdev: Platform device
> + *
> + * Returns: 0 if successful, non-zero if error.
> + */
> +static int chromeos_acpi_process_mlst(struct platform_device *pdev)
> +{
> +	struct chromeos_acpi_attribute_group *aag;
> +	char name[ACPI_NAMESEG_SIZE + 1];
> +	union acpi_object *element, *obj;
> +	struct device *dev = &pdev->dev;
> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +	acpi_status status;
> +	int ret = 0;
> +	int size;
> +	int i;
> +
> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
> +				      &output);
> +	if (ACPI_FAILURE(status))
> +		return status;
> +
> +	obj = output.pointer;
> +	if (obj->type != ACPI_TYPE_PACKAGE) {
> +		ret = -EINVAL;
> +		goto free_acpi_buffer;
> +	}
> +
> +	element = obj->package.elements;
> +	for (i = 0; i < obj->package.count; i++, element++) {
> +		if (element->type == ACPI_TYPE_STRING) {
> +			size = min(element->string.length + 1,
> +				   (u32)ACPI_NAMESEG_SIZE + 1);
> +			strscpy(name, element->string.pointer, size);
> +			ret = chromeos_acpi_add_method(pdev, name);
> +			if (ret) {
> +				chromeos_acpi_remove_groups();
> +				break;
> +			}
> +		}
> +	}
> +
> +	/* Add root attributes to the main group */
> +	ret = chromeos_acpi_add_attribs_to_group(chromeos_acpi.root,
> +						 chromeos_acpi.num_attrs);
> +	if (ret)
> +		goto free_acpi_buffer;
> +
> +	chromeos_acpi.dev_groups = kcalloc(chromeos_acpi.num_groups + 1,
> +					   sizeof(struct attribute_group),
> +					   GFP_KERNEL);
> +
> +	i = 0;
> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
> +		chromeos_acpi.dev_groups[i] = &aag->group;
> +		i++;
> +	}
> +
> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
> +	if (ret) {
> +		kfree(chromeos_acpi.dev_groups);
> +
> +		/* Remove allocated chromeos acpi groups and attributes */
> +		chromeos_acpi_remove_groups();
> +	}
> +
> +free_acpi_buffer:
> +	kfree(output.pointer);
> +	return ret;
> +}
> +
> +static int chromeos_acpi_device_probe(struct platform_device *pdev)
> +{
> +	struct chromeos_acpi_attribute_group *aag;
> +	struct device *dev = &pdev->dev;
> +	int i, ret;
> +
> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
> +	if (!aag)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&aag->attribs);
> +	INIT_LIST_HEAD(&aag->list);
> +	INIT_LIST_HEAD(&chromeos_acpi.groups);
> +
> +	chromeos_acpi.root = aag;
> +
> +	/*
> +	 * Attempt to add methods by querying the device's MLST method
> +	 * for the list of methods.
> +	 */
> +	if (!chromeos_acpi_process_mlst(pdev))
> +		return 0;
> +
> +	dev_dbg(dev, "falling back to default list of methods\n");
> +
> +	for (i = 0; i < ARRAY_SIZE(chromeos_acpi_default_methods); i++) {
> +		ret = chromeos_acpi_add_method(pdev,
> +					       chromeos_acpi_default_methods[i]);
> +		if (ret) {
> +			dev_err(dev, "failed to add default methods (%d)\n",
> +				ret);
> +			goto free_group_root;
> +		}
> +	}
> +
> +	return 0;
> +
> +free_group_root:
> +	kfree(chromeos_acpi.root);
> +	return ret;
> +}
> +
> +static int chromeos_acpi_device_remove(struct platform_device *pdev)
> +{
> +	/* Remove sysfs groups */
> +	sysfs_remove_groups(&pdev->dev.kobj, chromeos_acpi.dev_groups);
> +	kfree(chromeos_acpi.dev_groups);
> +
> +	/* Remove allocated chromeos acpi groups and attributes */
> +	chromeos_acpi_remove_groups();
> +
> +	return 0;
> +}
> +
> +/* GGL is valid PNP ID of Google. PNP ID can be used with the ACPI devices. */
> +static const struct acpi_device_id chromeos_device_ids[] = {
> +	{ "GGL0001", 0 },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(acpi, chromeos_device_ids);
> +
> +static struct platform_driver chromeos_acpi_device_driver = {
> +	.probe = chromeos_acpi_device_probe,
> +	.remove = chromeos_acpi_device_remove,
> +	.driver = {
> +		.name   = "chromeos-acpi",
> +		.acpi_match_table = ACPI_PTR(chromeos_device_ids)
> +	}
> +};
> +
> +module_platform_driver(chromeos_acpi_device_driver);
> +
> +MODULE_AUTHOR("Enric Balletbo i Serra <[e-mail]>");
> +MODULE_LICENSE("GPL");
> +MODULE_DESCRIPTION("ChromeOS specific ACPI extensions");

Andy Shevchenko April 18, 2022, 9 p.m. UTC | #4

On Fri, Apr 15, 2022 at 8:08 PM Muhammad Usama Anjum
<[e-mail]> wrote:
>
> From: Enric Balletbo i Serra <[e-mail]>
>
> The x86 Chromebooks have ChromeOS ACPI device. This driver attaches to

Either 'devices' or ' the ChromeOS'.

> the ChromeOS ACPI device and exports the values reported by ACPI in a
> sysfs directory. This data isn't present in ACPI tables when read
> through ACPI tools, hence a driver is needed to do it. The driver gets
> data from firmware using ACPI component of the kernel. The ACPI values

'the ACPI'

> are presented in string form (numbers as decimal values) or binary
> blobs, and can be accessed as the contents of the appropriate read only
> files in the standard ACPI device's sysfs directory tree. This data is
> consumed by the ChromeOS user space.

...

> Cc: Rafael J. Wysocki <[e-mail]>

No need to have in the commit message, esp. taking into account below tag.

> Cc: Dmitry Torokhov <[e-mail]>

> Cc: Hans de Goede <[e-mail]>
> Acked-by: Rafael J. Wysocki <[e-mail]>

Missed Co-developed-by?

> Signed-off-by: Enric Balletbo i Serra <[e-mail]>
> Signed-off-by: Muhammad Usama Anjum <[e-mail]>

...

> +What:          /sys/bus/platform/devices/GGL0001:00/VBNV.1

In all these examples theoretically the second part can be :01 or
anything depending on how many devices of the kind the platform has
(note, that may be the case when in the ACPI tables the first device,
for example, simply has status not 15, like in multi-platform DSDT
when it's done at boot-time).

...

> +Hardware functionality specific to Chrome OS is exposed through a Chrome OS ACPI device.
> +The plug and play ID of a Chrome OS ACPI device is GGL0001. GGL is valid PNP ID of Google.

a valid

> +PNP ID can be used with the ACPI devices accourding to the guidelines. The following ACPI

according

> +objects are supported:

...

> +   * - 0x00000200
> +     - Firmware write protect was disabled when x86 firmware booted. (required if

protection

> +       firmware write protect is controlled through x86 BIOS; otherwise optional)

protection

...

> +MECK (Management Engine Checksum)
> +=================================
> +This control method returns the SHA-1 or SHA-256 hash that is read out of the Management
> +Engine extend registers during boot. The hash is exported via ACPI so the OS can verify that

extended ?

> +the ME firmware has not changed. If Management Engine is not present, or if the firmware was
> +unable to read the extend registers, this buffer can be zero.

Ditto.

...

> +static char *chromeos_acpi_default_methods[] = {
> +       "CHSW", "HWID", "BINF", "GPIO", "CHNV", "FWID", "FRID", MLST

You can still leave comma at the end.

> +};

...

> +static char *chromeos_acpi_gen_file_name(char *name, int count, int index)
> +{
> +       char *str;

You can avoid it, by returning directly.

> +       if (count == 1)
> +               str = kstrdup(name, GFP_KERNEL);
> +       else
> +               str = kasprintf(GFP_KERNEL, "%s.%d", name, index);
> +
> +       return str;
> +}

...

> +       switch (element->type) {
> +       case ACPI_TYPE_BUFFER:
> +               length = element->buffer.length;
> +               info->data = kmemdup(element->buffer.pointer,
> +                                    length, GFP_KERNEL);
> +               break;
> +       case ACPI_TYPE_INTEGER:

> +               length = snprintf(buffer, sizeof(buffer), "%d",
> +                                 (int)element->integer.value);
> +               info->data = kmemdup(buffer, length, GFP_KERNEL);

kasprintf()

> +               break;
> +       case ACPI_TYPE_STRING:
> +               length = element->string.length + 1;
> +               info->data = kstrdup(element->string.pointer, GFP_KERNEL);
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               goto free_attr_name;
> +       }

> +       if (!info->data) {
> +               ret = -ENOMEM;
> +               goto free_attr_name;
> +       }

While saving a few lines of code this is more fragile to have it after
the switch, because it might be in the future that some of those types
won't fill info->data and be correct. But I have no strong opinion
here.

...

> +                       if (ret) {
> +                               dev_err(dev, "error adding attributes (%d)\n",
> +                                       ret);
> +                               return ret;
> +                       }

> +                       if (ret) {
> +                               dev_err(dev, "error adding a group (%d)\n",
> +                                       ret);
> +                               return ret;
> +                       }

> +                       if (ret) {
> +                               dev_err(dev, "error on element type (%d)\n",
> +                                       ret);
> +                               return -EINVAL;
> +                       }

I believe all three can be converted to return dev_error_probe(...);

...

> +static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
> +       acpi_status status;
> +       int ret = 0;
> +
> +       status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
> +       if (ACPI_FAILURE(status)) {
> +               dev_err(dev, "failed to retrieve %s (%d)\n", name, status);
> +               return status;

Are you sure it's valid error code returned here?

> +       }
> +
> +       if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
> +               ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
> +
> +       kfree(output.pointer);
> +       return ret;
> +}
> +

...

> +       status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
> +                                     &output);
> +       if (ACPI_FAILURE(status))
> +               return status;

Are you sure it's correct error code returned here?

> +       obj = output.pointer;
> +       if (obj->type != ACPI_TYPE_PACKAGE) {
> +               ret = -EINVAL;
> +               goto free_acpi_buffer;
> +       }

...

> +static int chromeos_acpi_device_probe(struct platform_device *pdev)
> +{
> +       struct chromeos_acpi_attribute_group *aag;
> +       struct device *dev = &pdev->dev;
> +       int i, ret;
> +
> +       aag = kzalloc(sizeof(*aag), GFP_KERNEL);

devm_kzalloc() ?

> +       if (!aag)
> +               return -ENOMEM;
> +
> +       INIT_LIST_HEAD(&aag->attribs);
> +       INIT_LIST_HEAD(&aag->list);
> +       INIT_LIST_HEAD(&chromeos_acpi.groups);
> +
> +       chromeos_acpi.root = aag;
> +
> +       /*
> +        * Attempt to add methods by querying the device's MLST method
> +        * for the list of methods.
> +        */
> +       if (!chromeos_acpi_process_mlst(pdev))
> +               return 0;
> +
> +       dev_dbg(dev, "falling back to default list of methods\n");
> +
> +       for (i = 0; i < ARRAY_SIZE(chromeos_acpi_default_methods); i++) {
> +               ret = chromeos_acpi_add_method(pdev,
> +                                              chromeos_acpi_default_methods[i]);
> +               if (ret) {
> +                       dev_err(dev, "failed to add default methods (%d)\n",
> +                               ret);
> +                       goto free_group_root;

return dev_err_probe(...); ?

> +               }
> +       }
> +
> +       return 0;
> +
> +free_group_root:
> +       kfree(chromeos_acpi.root);
> +       return ret;
> +}
> +
> +static int chromeos_acpi_device_remove(struct platform_device *pdev)
> +{
> +       /* Remove sysfs groups */
> +       sysfs_remove_groups(&pdev->dev.kobj, chromeos_acpi.dev_groups);
> +       kfree(chromeos_acpi.dev_groups);
> +
> +       /* Remove allocated chromeos acpi groups and attributes */
> +       chromeos_acpi_remove_groups();

I'm wondering why you have no such things in error path of the ->probe().

> +       return 0;
> +}

...

> +static struct platform_driver chromeos_acpi_device_driver = {
> +       .probe = chromeos_acpi_device_probe,
> +       .remove = chromeos_acpi_device_remove,
> +       .driver = {
> +               .name   = "chromeos-acpi",

> +               .acpi_match_table = ACPI_PTR(chromeos_device_ids)

The point of usage of ACPI_PTR is...?

> +       }
> +};

> +

No need to have this blank line.

> +module_platform_driver(chromeos_acpi_device_driver);

Muhammad Usama Anjum April 19, 2022, 9:24 a.m. UTC | #5

On 4/18/22 10:31 PM, Muhammad Usama Anjum wrote:
> Hi,
> 
> Thanks for reviewing.
> 
>>> +	switch (element->type) {
>>> +	case ACPI_TYPE_BUFFER:
>>> +		length = element->buffer.length;
>>> +		info->data = kmemdup(element->buffer.pointer,
>>> +				     length, GFP_KERNEL);
>>> +		break;
>>> +	case ACPI_TYPE_INTEGER:
>>> +		length = snprintf(buffer, sizeof(buffer), "%d",
>>> +				  (int)element->integer.value);
>>> +		info->data = kmemdup(buffer, length, GFP_KERNEL);
>>
>> You can use `kasprintf()` here, no?
>>
Yeah, I can use sasprintf() in place of snprintf() and kmemdup(). Thanks.

> Choosing kmemdup vs k*printf depends on what is being achieved. Usage of
> kmemdup indicates that only the memory is being duplicated here. While
> in case of k*printf, some transformation is done. Thus in normal memory
> duplication cases like this, the usage of kmemdup makes code more
> readable and seems preferable to me.
>

Muhammad Usama Anjum April 22, 2022, 2:47 p.m. UTC | #6

Hi Hans,

Can I add you as a maintainer of drivers/platform/x86/chromeos_acpi.c?
You already maintain some other drivers in this directory.

Thanks you,
Usama

On 4/18/22 10:57 PM, Muhammad Usama Anjum wrote:
> Hi Benson,
> 
> We are very near to getting this patch in. I can see that you are
> maintaining Chrome Platform support. This driver is in
> drivers/platform/x86/chromeos_acpi.c. Can you maintain this driver as
> well? I'll add you to the maintainers.
> 
> Thank you,
> Usama
> 
> On 4/15/22 10:08 PM, Muhammad Usama Anjum wrote:
>> From: Enric Balletbo i Serra <[e-mail]>
>>
>> The x86 Chromebooks have ChromeOS ACPI device. This driver attaches to
>> the ChromeOS ACPI device and exports the values reported by ACPI in a
>> sysfs directory. This data isn't present in ACPI tables when read
>> through ACPI tools, hence a driver is needed to do it. The driver gets
>> data from firmware using ACPI component of the kernel. The ACPI values
>> are presented in string form (numbers as decimal values) or binary
>> blobs, and can be accessed as the contents of the appropriate read only
>> files in the standard ACPI device's sysfs directory tree. This data is
>> consumed by the ChromeOS user space.
>>
>> Cc: Rafael J. Wysocki <[e-mail]>
>> Cc: Dmitry Torokhov <[e-mail]>
>> Cc: Hans de Goede <[e-mail]>
>> Acked-by: Rafael J. Wysocki <[e-mail]>
>> Signed-off-by: Enric Balletbo i Serra <[e-mail]>
>> Signed-off-by: Muhammad Usama Anjum <[e-mail]>
>> ---
>> Changes in v8:
>> - Change struct platform_driver name from chromeos_acpi_driver to
>>   chromeos_acpi_device_driver
>>
>> Changes in v7:
>> - Rename acpi_chromeos Kconfig option to chromeos_acpi
>> - Change this acpi driver to platform driver
>> - Minor cosmetic changes
>>
>> There were the following concerns on v4 which have been delt with in
>> v5/v6:
>> - Remove BINF.{0,1,4} from sysfs as they are reserved and not used
>>   anymore
>> - Reword the description of MECK
>> - Change function name from chromeos_acpi_alloc_name() to
>>   chromeos_acpi_gen_file_name()
>> - Remove local variable obj in chromeos_acpi_add_method()
>> - Replace usage of dev_info() to dev_dbg()
>> - Improve the description of the patch
>> - Add the firmware interface document which serves as primary
>>   documentation and garantees that this interface will not change
>> - GGL0001 is valid PNP ID of the Google. PNP ID can be used with the
>>   ACPI devices. Consensus was developed on it in discussion of v4.
>>
>> Changes in v6:
>> - Correct authorship and path email's From
>> - Add changelog between v4 and v5 in detail
>> - Add copywrite year 2022
>> - Improve the description and add concerns from V4 which have been fixed
>>
>> Changes in v5:
>> - Improve the description of the patch
>> - Document firmware interface
>> - Update sysfs interface documentation
>> - Remove binf{0,1,4} as they have been deprecated
>> - Update some cleanup logic in case of error
>>   - Remove freeing of chromeos_acpi.root explicitely in
>>     chromeos_acpi_device_remove() as it'll be automatically freed by
>>     chromeos_acpi_remove_groups()
>>   - If sysfs_create_groups() fails in chromeos_acpi_process_mlst(),
>>     cleanup all groups
>> - Cosmetic changes
>>
>> Changes in v4:
>> https://lore.kernel.org/lkml/[e-mail]/t/
>> - Add COMPILE_TEST to increase build coverage.
>> - Add sysfs ABI documentation.
>> - Rebased on top of 5.7-rc1 and solve conflicts.
>> - Cc ACPI maintainers.
>>
>> Changes in v3:
>> - Use attribute groups instead of adding files "by hand".
>> - Do not use "raw" kobject to create directories.
>> - Do not abuse of the platform_device interface. Remove it.
>>
>> Changes in v2:
>> - Note that this version is a total rework, with those major changes:
>> - Use lists to track dinamically allocated attributes and groups.
>> - Use sysfs binary attributes to store the ACPI contents.
>> - Remove all the functionalities except the one that creates the sysfs files.
>> ---
>>  .../ABI/testing/sysfs-driver-chromeos-acpi    | 126 +++++
>>  .../acpi/chromeos-acpi-device.rst             | 363 +++++++++++++
>>  Documentation/firmware-guide/acpi/index.rst   |   1 +
>>  drivers/platform/x86/Kconfig                  |  11 +
>>  drivers/platform/x86/Makefile                 |   3 +
>>  drivers/platform/x86/chromeos_acpi.c          | 513 ++++++++++++++++++
>>  6 files changed, 1017 insertions(+)
>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>>  create mode 100644 Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>>  create mode 100644 drivers/platform/x86/chromeos_acpi.c
>>
>> diff --git a/Documentation/ABI/testing/sysfs-driver-chromeos-acpi b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>> new file mode 100644
>> index 0000000000000..b74fbca62f4c3
>> --- /dev/null
>> +++ b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>> @@ -0,0 +1,126 @@
>> +What:		/sys/bus/platform/devices/GGL0001:00/BINF.2
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows information about the current boot of
>> +		the active EC firmware.
>> +		  * 0 - Read only (recovery) firmware.
>> +		  * 1 - Rewritable firmware.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/BINF.3
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows information about the current boot of
>> +		the active main	firmware type.
>> +		  * 0 - Recovery.
>> +		  * 1 - Normal.
>> +		  * 2 - Developer.
>> +		  * 3 - Netboot (factory installation only).
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/CHSW
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the switch position for the Chrome OS specific
>> +		hardware switches.
>> +		  * 0   - No changes.
>> +		  * 2   - Recovery button was pressed when firmware booted.
>> +		  * 4   - Recovery button was pressed when EC firmware booted.
>> +		  * 32  - Developer switch was enabled when firmware booted.
>> +		  * 512 - Firmware write protect was disabled when firmware
>> +			  booted.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/FMAP
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the physical memory address of the start of
>> +		the main processor firmware flashmap.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/FRID
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the firmware version for the read-only portion
>> +		of the main processor firmware.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/FWID
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the firmware version for the rewritable portion
>> +		of the main processor firmware.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.0
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the type of the GPIO signal for the Chrome OS
>> +		specific GPIO assignments.
>> +		  * 1   - Recovery button.
>> +		  * 2   - Developer mode switch.
>> +		  * 3   - Firmware write protect switch.
>> +		  * 256 to 511 - Debug header GPIO 0 to GPIO 255.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.1
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the signal attributes of the GPIO signal.
>> +		  * 0 - Signal is active low.
>> +		  * 1 - Signal is active high.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.2
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the GPIO number on the specified GPIO
>> +		controller.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.3
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the name of the GPIO controller.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/HWID
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the hardware ID for the Chromebook.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/MECK
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This binary file returns the SHA-1 or SHA-256 hash that is
>> +		read out of the Management Engine extend registers during
>> +		boot. The hash is exported vi ACPI so the OS can verify that
>> +		the Management Engine firmware has not changed. If Management
>> +		Engine is not present, or if the firmware was unable to read the
>> +		extend registers, this buffer size can be zero.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/VBNV.0
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the offset in CMOS bank 0 of the verified boot
>> +		non-volatile storage block, counting from the first writable
>> +		CMOS byte (that is, 'offset = 0' is the byte following the 14
>> +		bytes of clock data).
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/VBNV.1
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This file shows the size in bytes of the verified boot
>> +		non-volatile storage block.
>> +
>> +What:		/sys/bus/platform/devices/GGL0001:00/VDAT
>> +Date:		April 2022
>> +KernelVersion:	5.19
>> +Description:
>> +		This binary file returns the verified boot data block shared
>> +		between the firmware verification step and the kernel
>> +		verification step.
>> diff --git a/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>> new file mode 100644
>> index 0000000000000..ef3cc980bad54
>> --- /dev/null
>> +++ b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>> @@ -0,0 +1,363 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +=====================
>> +Chrome OS ACPI Device
>> +=====================
>> +
>> +Hardware functionality specific to Chrome OS is exposed through a Chrome OS ACPI device.
>> +The plug and play ID of a Chrome OS ACPI device is GGL0001. GGL is valid PNP ID of Google.
>> +PNP ID can be used with the ACPI devices accourding to the guidelines. The following ACPI
>> +objects are supported:
>> +
>> +.. flat-table:: Supported ACPI Objects
>> +   :widths: 1 2
>> +   :header-rows: 1
>> +
>> +   * - Object
>> +     - Description
>> +
>> +   * - CHSW
>> +     - Chrome OS switch positions
>> +
>> +   * - HWID
>> +     - Chrome OS hardware ID
>> +
>> +   * - FWID
>> +     - Chrome OS firmware version
>> +
>> +   * - FRID
>> +     - Chrome OS read-only firmware version
>> +
>> +   * - BINF
>> +     - Chrome OS boot information
>> +
>> +   * - GPIO
>> +     - Chrome OS GPIO assignments
>> +
>> +   * - VBNV
>> +     - Chrome OS NVRAM locations
>> +
>> +   * - VDTA
>> +     - Chrome OS verified boot data
>> +
>> +   * - FMAP
>> +     - Chrome OS flashmap base address
>> +
>> +   * - MLST
>> +     - Chrome OS method list
>> +
>> +CHSW (Chrome OS switch positions)
>> +=================================
>> +This control method returns the switch positions for Chrome OS specific hardware switches.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +An integer containing the switch positions as bitfields:
>> +
>> +.. flat-table::
>> +   :widths: 1 2
>> +
>> +   * - 0x00000002
>> +     - Recovery button was pressed when x86 firmware booted.
>> +
>> +   * - 0x00000004
>> +     - Recovery button was pressed when EC firmware booted. (required if EC EEPROM is
>> +       rewritable; otherwise optional)
>> +
>> +   * - 0x00000020
>> +     - Developer switch was enabled when x86 firmware booted.
>> +
>> +   * - 0x00000200
>> +     - Firmware write protect was disabled when x86 firmware booted. (required if
>> +       firmware write protect is controlled through x86 BIOS; otherwise optional)
>> +
>> +All other bits are reserved and should be set to 0.
>> +
>> +HWID (Chrome OS hardware ID)
>> +============================
>> +This control method returns the hardware ID for the Chromebook.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +A null-terminated ASCII string containing the hardware ID from the Model-Specific Data area of
>> +EEPROM.
>> +
>> +Note that the hardware ID can be up to 256 characters long, including the terminating null.
>> +
>> +FWID (Chrome OS firmware version)
>> +=================================
>> +This control method returns the firmware version for the rewritable portion of the main
>> +processor firmware.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +A null-terminated ASCII string containing the complete firmware version for the rewritable
>> +portion of the main processor firmware.
>> +
>> +FRID (Chrome OS read-only firmware version)
>> +===========================================
>> +This control method returns the firmware version for the read-only portion of the main
>> +processor firmware.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +A null-terminated ASCII string containing the complete firmware version for the read-only
>> +(bootstrap + recovery ) portion of the main processor firmware.
>> +
>> +BINF (Chrome OS boot information)
>> +=================================
>> +This control method returns information about the current boot.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +
>> +.. code-block::
>> +
>> +   Package {
>> +           Reserved1
>> +           Reserved2
>> +           Active EC Firmware
>> +           Active Main Firmware Type
>> +           Reserved5
>> +   }
>> +
>> +.. flat-table::
>> +   :widths: 1 1 2
>> +   :header-rows: 1
>> +
>> +   * - Field
>> +     - Format
>> +     - Description
>> +
>> +   * - Reserved1
>> +     - DWORD
>> +     - Set to 256 (0x100). This indicates this field is no longer used.
>> +
>> +   * - Reserved2
>> +     - DWORD
>> +     - Set to 256 (0x100). This indicates this field is no longer used.
>> +
>> +   * - Active EC firmware
>> +     - DWORD
>> +     - The EC firmware which was used during boot.
>> +
>> +       - 0 - Read-only (recovery) firmware
>> +       - 1 - Rewritable firmware.
>> +
>> +       Set to 0 if EC firmware is always read-only.
>> +
>> +   * - Active Main Firmware Type
>> +     - DWORD
>> +     - The main firmware type which was used during boot.
>> +
>> +       - 0 - Recovery
>> +       - 1 - Normal
>> +       - 2 - Developer
>> +       - 3 - netboot (factory installation only)
>> +
>> +       Other values are reserved.
>> +
>> +   * - Reserved5
>> +     - DWORD
>> +     - Set to 256 (0x100). This indicates this field is no longer used.
>> +
>> +GPIO (Chrome OS GPIO assignments)
>> +=================================
>> +This control method returns information about Chrome OS specific GPIO assignments for
>> +Chrome OS hardware, so the kernel can directly control that hardware.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +.. code-block::
>> +
>> +        Package {
>> +                Package {
>> +                        // First GPIO assignment
>> +                        Signal Type        //DWORD
>> +                        Attributes         //DWORD
>> +                        Controller Offset  //DWORD
>> +                        Controller Name    //ASCIIZ
>> +                },
>> +                ...
>> +                Package {
>> +                        // Last GPIO assignment
>> +                        Signal Type        //DWORD
>> +                        Attributes         //DWORD
>> +                        Controller Offset  //DWORD
>> +                        Controller Name    //ASCIIZ
>> +                }
>> +        }
>> +
>> +Where ASCIIZ means a null-terminated ASCII string.
>> +
>> +.. flat-table::
>> +   :widths: 1 1 2
>> +   :header-rows: 1
>> +
>> +   * - Field
>> +     - Format
>> +     - Description
>> +
>> +   * - Signal Type
>> +     - DWORD
>> +     - Type of GPIO signal
>> +
>> +       - 0x00000001 - Recovery button
>> +       - 0x00000002 - Developer mode switch
>> +       - 0x00000003 - Firmware write protect switch
>> +       - 0x00000100 - Debug header GPIO 0
>> +       - ...
>> +       - 0x000001FF - Debug header GPIO 255
>> +
>> +       Other values are reserved.
>> +
>> +   * - Attributes
>> +     - DWORD
>> +     - Signal attributes as bitfields:
>> +
>> +       - 0x00000001 - Signal is active-high (for button, a GPIO value
>> +         of 1 means the button is pressed; for switches, a GPIO value
>> +         of 1 means the switch is enabled). If this bit is 0, the signal
>> +         is active low. Set to 0 for debug header GPIOs.
>> +
>> +   * - Controller Offset
>> +     - DWORD
>> +     - GPIO number on the specified controller.
>> +
>> +   * - Controller Name
>> +     - ASCIIZ
>> +     - Name of the controller for the GPIO.
>> +       Currently supported names:
>> +       "NM10" - Intel NM10 chip
>> +
>> +VBNV (Chrome OS NVRAM locations)
>> +================================
>> +This control method returns information about the NVRAM (CMOS) locations used to
>> +communicate with the BIOS.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +.. code-block::
>> +
>> +        Package {
>> +                NV Storage Block Offset  //DWORD
>> +                NV Storage Block Size    //DWORD
>> +        }
>> +
>> +.. flat-table::
>> +   :widths: 1 1 2
>> +   :header-rows: 1
>> +
>> +   * - Field
>> +     - Format
>> +     - Description
>> +
>> +   * - NV Storage Block Offset
>> +     - DWORD
>> +     - Offset in CMOS bank 0 of the verified boot non-volatile storage block, counting from
>> +       the first writable CMOS byte (that is, offset=0 is the byte following the 14 bytes of
>> +       clock data).
>> +
>> +   * - NV Storage Block Size
>> +     - DWORD
>> +     - Size in bytes of the verified boot non-volatile storage block.
>> +
>> +FMAP (Chrome OS flashmap address)
>> +=================================
>> +This control method returns the physical memory address of the start of the main processor
>> +firmware flashmap.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +NoneResult code:
>> +----------------
>> +A DWORD containing the physical memory address of the start of the main processor firmware
>> +flashmap.
>> +
>> +VDTA (Chrome OS verified boot data)
>> +===================================
>> +This control method returns the verified boot data block shared between the firmware
>> +verification step and the kernel verification step.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +A buffer containing the verified boot data block.
>> +
>> +MECK (Management Engine Checksum)
>> +=================================
>> +This control method returns the SHA-1 or SHA-256 hash that is read out of the Management
>> +Engine extend registers during boot. The hash is exported via ACPI so the OS can verify that
>> +the ME firmware has not changed. If Management Engine is not present, or if the firmware was
>> +unable to read the extend registers, this buffer can be zero.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +A buffer containing the ME hash.
>> +
>> +MLST (Chrome OS method list)
>> +============================
>> +This control method returns a list of the other control methods supported by the Chrome OS
>> +hardware device.
>> +
>> +Arguments:
>> +----------
>> +None
>> +
>> +Result code:
>> +------------
>> +A package containing a list of null-terminated ASCII strings, one for each control method
>> +supported by the Chrome OS hardware device, not including the MLST method itself.
>> +For this version of the specification, the result is:
>> +
>> +.. code-block::
>> +
>> +        Package {
>> +                "CHSW",
>> +                "FWID",
>> +                "HWID",
>> +                "FRID",
>> +                "BINF",
>> +                "GPIO",
>> +                "VBNV",
>> +                "FMAP",
>> +                "VDTA",
>> +                "MECK"
>> +        }
>> diff --git a/Documentation/firmware-guide/acpi/index.rst b/Documentation/firmware-guide/acpi/index.rst
>> index b053b0c3d6969..b6a42f4ffe032 100644
>> --- a/Documentation/firmware-guide/acpi/index.rst
>> +++ b/Documentation/firmware-guide/acpi/index.rst
>> @@ -29,3 +29,4 @@ ACPI Support
>>     non-d0-probe
>>     extcon-intel-int3496
>>     intel-pmc-mux
>> +   chromeos-acpi-device
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index 5d9dd70e4e0f5..53ddfe6f99b06 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -322,6 +322,17 @@ config ASUS_NB_WMI
>>  	  If you have an ACPI-WMI compatible Asus Notebook, say Y or M
>>  	  here.
>>  
>> +config CHROMEOS_ACPI
>> +	tristate "ChromeOS specific ACPI extensions"
>> +	depends on ACPI || COMPILE_TEST
>> +	help
>> +	  This driver provides the firmware interface for the services
>> +	  exported through the ChromeOS interfaces when using ChromeOS
>> +	  ACPI firmware.
>> +
>> +	  If you have an ACPI-compatible Chromebook, say Y or M
>> +	  here.
>> +
>>  config ASUS_TF103C_DOCK
>>  	tristate "Asus TF103C 2-in-1 keyboard dock"
>>  	depends on ACPI
>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>> index fe4d4c8970efa..ae68f4fa9a6c4 100644
>> --- a/drivers/platform/x86/Makefile
>> +++ b/drivers/platform/x86/Makefile
>> @@ -44,6 +44,9 @@ obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
>>  # Cisco/Meraki
>>  obj-$(CONFIG_MERAKI_MX100)	+= meraki-mx100.o
>>  
>> +# Chrome
>> +obj-$(CONFIG_CHROMEOS_ACPI)	+= chromeos_acpi.o
>> +
>>  # Dell
>>  obj-$(CONFIG_X86_PLATFORM_DRIVERS_DELL)		+= dell/
>>  
>> diff --git a/drivers/platform/x86/chromeos_acpi.c b/drivers/platform/x86/chromeos_acpi.c
>> new file mode 100644
>> index 0000000000000..de86119a446b8
>> --- /dev/null
>> +++ b/drivers/platform/x86/chromeos_acpi.c
>> @@ -0,0 +1,513 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * ChromeOS specific ACPI extensions
>> + *
>> + * Copyright 2011 Google, Inc.
>> + * Copyright 2020-2022 Google LLC
>> + *
>> + * This file is a rework and part of the code is ported from chromeos-3.18
>> + * kernel and was originally written by Vadim Bendebury <[e-mail]>.
>> + *
>> + * This driver attaches to the ChromeOS ACPI device and then exports the
>> + * values reported by the ACPI in a sysfs directory. All values are
>> + * presented in the string form (numbers as decimal values) and can be
>> + * accessed as the contents of the appropriate read only files in the
>> + * sysfs directory tree.
>> + */
>> +
>> +#include <linux/acpi.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/kernel.h>
>> +#include <linux/list.h>
>> +#include <linux/module.h>
>> +
>> +/*
>> + * ACPI method name for MLST; the response for this method is a package of
>> + * strings listing the methods which should be reflected in sysfs.
>> + */
>> +#define MLST "MLST"
>> +
>> +/*
>> + * The default list of methods the ChromeOS ACPI device is supposed to export,
>> + * if the MLST method is not present or is poorly formed.  The MLST method
>> + * itself is included, to aid in debugging.
>> + */
>> +static char *chromeos_acpi_default_methods[] = {
>> +	"CHSW", "HWID", "BINF", "GPIO", "CHNV", "FWID", "FRID", MLST
>> +};
>> +
>> +/*
>> + * Representation of a single sysfs attribute. In addition to the standard
>> + * bin_attribute structure has a list of these structures (to keep track for
>> + * de-allocation when removing the driver) and a pointer to the actual
>> + * attribute name and value, reported when accessing the appropriate sysfs
>> + * file.
>> + */
>> +struct chromeos_acpi_attribute {
>> +	struct bin_attribute bin_attr;
>> +	struct list_head list;
>> +	char *name;
>> +	char *data;
>> +};
>> +
>> +/*
>> + * Representation of a sysfs attribute group (a sub directory in the device's
>> + * sysfs directory). In addition to the standard structure has lists to allow
>> + * to keep track of the allocated structures.
>> + */
>> +struct chromeos_acpi_attribute_group {
>> +	struct attribute_group group;
>> +	struct list_head attribs;
>> +	struct list_head list;
>> +	char *name;
>> +};
>> +
>> +/*
>> + * This is the main structure, we use it to store data and adds links pointing
>> + * at lists of allocated attributes and attribute groups.
>> + */
>> +struct chromeos_acpi_dev {
>> +	struct chromeos_acpi_attribute_group *root;
>> +	const struct attribute_group **dev_groups;
>> +	struct list_head groups;
>> +	unsigned int num_groups;
>> +	unsigned int num_attrs;
>> +};
>> +
>> +static struct chromeos_acpi_dev chromeos_acpi;
>> +
>> +static ssize_t chromeos_acpi_read_bin_attribute(struct file *filp,
>> +						struct kobject *kobj,
>> +						struct bin_attribute *bin_attr,
>> +						char *buffer, loff_t pos,
>> +						size_t count)
>> +{
>> +	struct chromeos_acpi_attribute *info = bin_attr->private;
>> +
>> +	return memory_read_from_buffer(buffer, count, &pos, info->data,
>> +				       info->bin_attr.size);
>> +}
>> +
>> +static char *chromeos_acpi_gen_file_name(char *name, int count, int index)
>> +{
>> +	char *str;
>> +
>> +	if (count == 1)
>> +		str = kstrdup(name, GFP_KERNEL);
>> +	else
>> +		str = kasprintf(GFP_KERNEL, "%s.%d", name, index);
>> +
>> +	return str;
>> +}
>> +
>> +static int
>> +chromeos_acpi_add_attr(struct chromeos_acpi_attribute_group *aag,
>> +		       union acpi_object *element, char *name,
>> +		       int count, int index)
>> +{
>> +	struct chromeos_acpi_attribute *info;
>> +	char buffer[24]; /* enough to store a u64 and null character */
>> +	int length;
>> +	int ret;
>> +
>> +	/* Files BINF.{0,1,4} are historical and no longer used. */
>> +	if (!strcmp(name, "BINF") && (index == 0 || index == 1 || index == 4))
>> +		return 0;
>> +
>> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +	if (!info)
>> +		return -ENOMEM;
>> +
>> +	info->name = chromeos_acpi_gen_file_name(name, count, index);
>> +	if (!info->name) {
>> +		ret = -ENOMEM;
>> +		goto free_attribute;
>> +	}
>> +	sysfs_bin_attr_init(&info->bin_attr);
>> +	info->bin_attr.attr.name = info->name;
>> +	info->bin_attr.attr.mode = 0444;
>> +
>> +	switch (element->type) {
>> +	case ACPI_TYPE_BUFFER:
>> +		length = element->buffer.length;
>> +		info->data = kmemdup(element->buffer.pointer,
>> +				     length, GFP_KERNEL);
>> +		break;
>> +	case ACPI_TYPE_INTEGER:
>> +		length = snprintf(buffer, sizeof(buffer), "%d",
>> +				  (int)element->integer.value);
>> +		info->data = kmemdup(buffer, length, GFP_KERNEL);
>> +		break;
>> +	case ACPI_TYPE_STRING:
>> +		length = element->string.length + 1;
>> +		info->data = kstrdup(element->string.pointer, GFP_KERNEL);
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		goto free_attr_name;
>> +	}
>> +
>> +	if (!info->data) {
>> +		ret = -ENOMEM;
>> +		goto free_attr_name;
>> +	}
>> +
>> +	info->bin_attr.size = length;
>> +	info->bin_attr.read = chromeos_acpi_read_bin_attribute;
>> +	info->bin_attr.private = info;
>> +
>> +	INIT_LIST_HEAD(&info->list);
>> +
>> +	list_add(&info->list, &aag->attribs);
>> +	return 0;
>> +
>> +free_attr_name:
>> +	kfree(info->name);
>> +free_attribute:
>> +	kfree(info);
>> +	return ret;
>> +}
>> +
>> +static void
>> +chromeos_acpi_remove_attribs(struct chromeos_acpi_attribute_group *aag)
>> +{
>> +	struct chromeos_acpi_attribute *attr, *tmp_attr;
>> +
>> +	list_for_each_entry_safe(attr, tmp_attr, &aag->attribs, list) {
>> +		kfree(attr->name);
>> +		kfree(attr->data);
>> +		kfree(attr);
>> +	}
>> +}
>> +
>> +static int
>> +chromeos_acpi_add_attribs_to_group(struct chromeos_acpi_attribute_group *aag,
>> +				   unsigned int num_attrs)
>> +{
>> +	struct chromeos_acpi_attribute *attr;
>> +	int count = 0;
>> +
>> +	aag->group.bin_attrs = kcalloc(num_attrs + 1,
>> +				       sizeof(*aag->group.bin_attrs),
>> +				       GFP_KERNEL);
>> +	if (!aag->group.bin_attrs)
>> +		return -ENOMEM;
>> +
>> +	list_for_each_entry(attr, &aag->attribs, list) {
>> +		aag->group.bin_attrs[count] = &attr->bin_attr;
>> +		count++;
>> +	}
>> +
>> +	chromeos_acpi.num_groups++;
>> +	list_add(&aag->list, &chromeos_acpi.groups);
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * chromeos_acpi_add_group() - Create a sysfs group including attributes
>> + *			       representing a nested ACPI package
>> + *
>> + * @obj: Package contents as returned by ACPI
>> + * @name: Name of the group
>> + * @num_attrs: Number of attributes of this package
>> + * @index: Index number of this particular group
>> + *
>> + * The created group is called @name in case there is a single instance, or
>> + * @name.@index otherwise.
>> + *
>> + * All group and attribute storage allocations are included in the lists for
>> + * tracking of allocated memory.
>> + *
>> + * Return: 0 on success, negative errno on failure.
>> + */
>> +static int chromeos_acpi_add_group(union acpi_object *obj, char *name,
>> +				   int num_attrs, int index)
>> +{
>> +	struct chromeos_acpi_attribute_group *aag;
>> +	union acpi_object *element;
>> +	int i, count, ret;
>> +
>> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
>> +	if (!aag)
>> +		return -ENOMEM;
>> +	aag->name = chromeos_acpi_gen_file_name(name, num_attrs, index);
>> +	if (!aag->name) {
>> +		ret = -ENOMEM;
>> +		goto free_group;
>> +	}
>> +
>> +	INIT_LIST_HEAD(&aag->attribs);
>> +	INIT_LIST_HEAD(&aag->list);
>> +
>> +	count = obj->package.count;
>> +	element = obj->package.elements;
>> +	for (i = 0; i < count; i++, element++) {
>> +		ret = chromeos_acpi_add_attr(aag, element, name, count, i);
>> +		if (ret)
>> +			goto free_group_attr;
>> +	}
>> +
>> +	aag->group.name = aag->name;
>> +
>> +	ret = chromeos_acpi_add_attribs_to_group(aag, count);
>> +	if (ret)
>> +		goto free_group_attr;
>> +
>> +	return 0;
>> +
>> +free_group_attr:
>> +	chromeos_acpi_remove_attribs(aag);
>> +	kfree(aag->name);
>> +free_group:
>> +	kfree(aag);
>> +	return ret;
>> +}
>> +
>> +static void chromeos_acpi_remove_groups(void)
>> +{
>> +	struct chromeos_acpi_attribute_group *aag, *tmp_aag;
>> +
>> +	list_for_each_entry_safe(aag, tmp_aag, &chromeos_acpi.groups, list) {
>> +		chromeos_acpi_remove_attribs(aag);
>> +		kfree(aag->group.bin_attrs);
>> +		kfree(aag->name);
>> +		kfree(aag);
>> +	}
>> +}
>> +
>> +/**
>> + * chromeos_acpi_handle_package() - Create sysfs group including attributes
>> + *				    representing an ACPI package
>> + *
>> + * @pdev: Platform device
>> + * @obj: Package contents as returned by ACPI
>> + * @name: Name of the group
>> + *
>> + * Scalar objects included in the package get sysfs attributes created for
>> + * them. Nested packages are passed to a function creating a sysfs group per
>> + * package.
>> + *
>> + * Return: 0 on success, negative errno on failure.
>> + */
>> +static int chromeos_acpi_handle_package(struct platform_device *pdev,
>> +					union acpi_object *obj, char *name)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	int count = obj->package.count;
>> +	union acpi_object *element;
>> +	int i, ret;
>> +
>> +	element = obj->package.elements;
>> +	for (i = 0; i < count; i++, element++) {
>> +		if (element->type == ACPI_TYPE_BUFFER ||
>> +		    element->type == ACPI_TYPE_STRING ||
>> +		    element->type == ACPI_TYPE_INTEGER) {
>> +			/* Create a single attribute in the root directory */
>> +			ret = chromeos_acpi_add_attr(chromeos_acpi.root,
>> +						     element, name,
>> +						     count, i);
>> +			if (ret) {
>> +				dev_err(dev, "error adding attributes (%d)\n",
>> +					ret);
>> +				return ret;
>> +			}
>> +			chromeos_acpi.num_attrs++;
>> +		} else if (element->type == ACPI_TYPE_PACKAGE) {
>> +			/* Create a group of attributes */
>> +			ret = chromeos_acpi_add_group(element, name, count, i);
>> +			if (ret) {
>> +				dev_err(dev, "error adding a group (%d)\n",
>> +					ret);
>> +				return ret;
>> +			}
>> +		} else {
>> +			if (ret) {
>> +				dev_err(dev, "error on element type (%d)\n",
>> +					ret);
>> +				return -EINVAL;
>> +			}
>> +		}
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/**
>> + * chromeos_acpi_add_method() - Evaluate an ACPI method and create sysfs
>> + *				attributes
>> + *
>> + * @pdev: Platform device
>> + * @name: Name of the method to evaluate
>> + *
>> + * Return: 0 on success, non-zero on failure
>> + */
>> +static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	acpi_status status;
>> +	int ret = 0;
>> +
>> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
>> +	if (ACPI_FAILURE(status)) {
>> +		dev_err(dev, "failed to retrieve %s (%d)\n", name, status);
>> +		return status;
>> +	}
>> +
>> +	if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
>> +		ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
>> +
>> +	kfree(output.pointer);
>> +	return ret;
>> +}
>> +
>> +/**
>> + * chromeos_acpi_process_mlst() - Evaluate the MLST method and add methods
>> + *				  listed in the response
>> + *
>> + * @pdev: Platform device
>> + *
>> + * Returns: 0 if successful, non-zero if error.
>> + */
>> +static int chromeos_acpi_process_mlst(struct platform_device *pdev)
>> +{
>> +	struct chromeos_acpi_attribute_group *aag;
>> +	char name[ACPI_NAMESEG_SIZE + 1];
>> +	union acpi_object *element, *obj;
>> +	struct device *dev = &pdev->dev;
>> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>> +	acpi_status status;
>> +	int ret = 0;
>> +	int size;
>> +	int i;
>> +
>> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
>> +				      &output);
>> +	if (ACPI_FAILURE(status))
>> +		return status;
>> +
>> +	obj = output.pointer;
>> +	if (obj->type != ACPI_TYPE_PACKAGE) {
>> +		ret = -EINVAL;
>> +		goto free_acpi_buffer;
>> +	}
>> +
>> +	element = obj->package.elements;
>> +	for (i = 0; i < obj->package.count; i++, element++) {
>> +		if (element->type == ACPI_TYPE_STRING) {
>> +			size = min(element->string.length + 1,
>> +				   (u32)ACPI_NAMESEG_SIZE + 1);
>> +			strscpy(name, element->string.pointer, size);
>> +			ret = chromeos_acpi_add_method(pdev, name);
>> +			if (ret) {
>> +				chromeos_acpi_remove_groups();
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>> +	/* Add root attributes to the main group */
>> +	ret = chromeos_acpi_add_attribs_to_group(chromeos_acpi.root,
>> +						 chromeos_acpi.num_attrs);
>> +	if (ret)
>> +		goto free_acpi_buffer;
>> +
>> +	chromeos_acpi.dev_groups = kcalloc(chromeos_acpi.num_groups + 1,
>> +					   sizeof(struct attribute_group),
>> +					   GFP_KERNEL);
>> +
>> +	i = 0;
>> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
>> +		chromeos_acpi.dev_groups[i] = &aag->group;
>> +		i++;
>> +	}
>> +
>> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
>> +	if (ret) {
>> +		kfree(chromeos_acpi.dev_groups);
>> +
>> +		/* Remove allocated chromeos acpi groups and attributes */
>> +		chromeos_acpi_remove_groups();
>> +	}
>> +
>> +free_acpi_buffer:
>> +	kfree(output.pointer);
>> +	return ret;
>> +}
>> +
>> +static int chromeos_acpi_device_probe(struct platform_device *pdev)
>> +{
>> +	struct chromeos_acpi_attribute_group *aag;
>> +	struct device *dev = &pdev->dev;
>> +	int i, ret;
>> +
>> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
>> +	if (!aag)
>> +		return -ENOMEM;
>> +
>> +	INIT_LIST_HEAD(&aag->attribs);
>> +	INIT_LIST_HEAD(&aag->list);
>> +	INIT_LIST_HEAD(&chromeos_acpi.groups);
>> +
>> +	chromeos_acpi.root = aag;
>> +
>> +	/*
>> +	 * Attempt to add methods by querying the device's MLST method
>> +	 * for the list of methods.
>> +	 */
>> +	if (!chromeos_acpi_process_mlst(pdev))
>> +		return 0;
>> +
>> +	dev_dbg(dev, "falling back to default list of methods\n");
>> +
>> +	for (i = 0; i < ARRAY_SIZE(chromeos_acpi_default_methods); i++) {
>> +		ret = chromeos_acpi_add_method(pdev,
>> +					       chromeos_acpi_default_methods[i]);
>> +		if (ret) {
>> +			dev_err(dev, "failed to add default methods (%d)\n",
>> +				ret);
>> +			goto free_group_root;
>> +		}
>> +	}
>> +
>> +	return 0;
>> +
>> +free_group_root:
>> +	kfree(chromeos_acpi.root);
>> +	return ret;
>> +}
>> +
>> +static int chromeos_acpi_device_remove(struct platform_device *pdev)
>> +{
>> +	/* Remove sysfs groups */
>> +	sysfs_remove_groups(&pdev->dev.kobj, chromeos_acpi.dev_groups);
>> +	kfree(chromeos_acpi.dev_groups);
>> +
>> +	/* Remove allocated chromeos acpi groups and attributes */
>> +	chromeos_acpi_remove_groups();
>> +
>> +	return 0;
>> +}
>> +
>> +/* GGL is valid PNP ID of Google. PNP ID can be used with the ACPI devices. */
>> +static const struct acpi_device_id chromeos_device_ids[] = {
>> +	{ "GGL0001", 0 },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(acpi, chromeos_device_ids);
>> +
>> +static struct platform_driver chromeos_acpi_device_driver = {
>> +	.probe = chromeos_acpi_device_probe,
>> +	.remove = chromeos_acpi_device_remove,
>> +	.driver = {
>> +		.name   = "chromeos-acpi",
>> +		.acpi_match_table = ACPI_PTR(chromeos_device_ids)
>> +	}
>> +};
>> +
>> +module_platform_driver(chromeos_acpi_device_driver);
>> +
>> +MODULE_AUTHOR("Enric Balletbo i Serra <[e-mail]>");
>> +MODULE_LICENSE("GPL");
>> +MODULE_DESCRIPTION("ChromeOS specific ACPI extensions");
>

Greg Kroah-Hartman April 24, 2022, 8:43 a.m. UTC | #7

On Fri, Apr 15, 2022 at 10:08:15PM +0500, Muhammad Usama Anjum wrote:
> +	i = 0;
> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
> +		chromeos_acpi.dev_groups[i] = &aag->group;
> +		i++;
> +	}
> +
> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);

You have raced with userspace and lost here :(

Use the default groups pointer in the platform driver for this, and use
the is_visible() callback to know to show, or not show, the attribute
instead of building up dynamic lists of attributes at runtime.  That
will save you lots of crazy logic and housekeeping _AND_ userspace tools
will work properly as well.

thanks,

greg k-h

Hans de Goede April 27, 2022, 2:22 p.m. UTC | #8

Hi Muhammad,

On 4/22/22 16:47, Muhammad Usama Anjum wrote:
> Hi Hans,
> 
> Can I add you as a maintainer of drivers/platform/x86/chromeos_acpi.c?
> You already maintain some other drivers in this directory.

If you want to rely on me as default/fallback maintainer
for the new chromeos_acpi driver, then simply don't add
a MAINTAINERS entry for this driver.

I'm the default maintainer for everything under
drivers/platform/x86/ .

With that said:


> On 4/18/22 10:57 PM, Muhammad Usama Anjum wrote:
>> Hi Benson,
>>
>> We are very near to getting this patch in. I can see that you are
>> maintaining Chrome Platform support. This driver is in
>> drivers/platform/x86/chromeos_acpi.c. Can you maintain this driver as
>> well? I'll add you to the maintainers.

now that you mention it, moving this to drivers/platform/chrome/
actually seems like the most sensible thing to do given that
this is chrome specific platform code. So please move it
there for the next version.

Talking about the next version, please address all review
remarks from Barnabás for the next version.

Regards,

Hans



>> On 4/15/22 10:08 PM, Muhammad Usama Anjum wrote:
>>> From: Enric Balletbo i Serra <[e-mail]>
>>>
>>> The x86 Chromebooks have ChromeOS ACPI device. This driver attaches to
>>> the ChromeOS ACPI device and exports the values reported by ACPI in a
>>> sysfs directory. This data isn't present in ACPI tables when read
>>> through ACPI tools, hence a driver is needed to do it. The driver gets
>>> data from firmware using ACPI component of the kernel. The ACPI values
>>> are presented in string form (numbers as decimal values) or binary
>>> blobs, and can be accessed as the contents of the appropriate read only
>>> files in the standard ACPI device's sysfs directory tree. This data is
>>> consumed by the ChromeOS user space.
>>>
>>> Cc: Rafael J. Wysocki <[e-mail]>
>>> Cc: Dmitry Torokhov <[e-mail]>
>>> Cc: Hans de Goede <[e-mail]>
>>> Acked-by: Rafael J. Wysocki <[e-mail]>
>>> Signed-off-by: Enric Balletbo i Serra <[e-mail]>
>>> Signed-off-by: Muhammad Usama Anjum <[e-mail]>
>>> ---
>>> Changes in v8:
>>> - Change struct platform_driver name from chromeos_acpi_driver to
>>>   chromeos_acpi_device_driver
>>>
>>> Changes in v7:
>>> - Rename acpi_chromeos Kconfig option to chromeos_acpi
>>> - Change this acpi driver to platform driver
>>> - Minor cosmetic changes
>>>
>>> There were the following concerns on v4 which have been delt with in
>>> v5/v6:
>>> - Remove BINF.{0,1,4} from sysfs as they are reserved and not used
>>>   anymore
>>> - Reword the description of MECK
>>> - Change function name from chromeos_acpi_alloc_name() to
>>>   chromeos_acpi_gen_file_name()
>>> - Remove local variable obj in chromeos_acpi_add_method()
>>> - Replace usage of dev_info() to dev_dbg()
>>> - Improve the description of the patch
>>> - Add the firmware interface document which serves as primary
>>>   documentation and garantees that this interface will not change
>>> - GGL0001 is valid PNP ID of the Google. PNP ID can be used with the
>>>   ACPI devices. Consensus was developed on it in discussion of v4.
>>>
>>> Changes in v6:
>>> - Correct authorship and path email's From
>>> - Add changelog between v4 and v5 in detail
>>> - Add copywrite year 2022
>>> - Improve the description and add concerns from V4 which have been fixed
>>>
>>> Changes in v5:
>>> - Improve the description of the patch
>>> - Document firmware interface
>>> - Update sysfs interface documentation
>>> - Remove binf{0,1,4} as they have been deprecated
>>> - Update some cleanup logic in case of error
>>>   - Remove freeing of chromeos_acpi.root explicitely in
>>>     chromeos_acpi_device_remove() as it'll be automatically freed by
>>>     chromeos_acpi_remove_groups()
>>>   - If sysfs_create_groups() fails in chromeos_acpi_process_mlst(),
>>>     cleanup all groups
>>> - Cosmetic changes
>>>
>>> Changes in v4:
>>> https://lore.kernel.org/lkml/[e-mail]/t/
>>> - Add COMPILE_TEST to increase build coverage.
>>> - Add sysfs ABI documentation.
>>> - Rebased on top of 5.7-rc1 and solve conflicts.
>>> - Cc ACPI maintainers.
>>>
>>> Changes in v3:
>>> - Use attribute groups instead of adding files "by hand".
>>> - Do not use "raw" kobject to create directories.
>>> - Do not abuse of the platform_device interface. Remove it.
>>>
>>> Changes in v2:
>>> - Note that this version is a total rework, with those major changes:
>>> - Use lists to track dinamically allocated attributes and groups.
>>> - Use sysfs binary attributes to store the ACPI contents.
>>> - Remove all the functionalities except the one that creates the sysfs files.
>>> ---
>>>  .../ABI/testing/sysfs-driver-chromeos-acpi    | 126 +++++
>>>  .../acpi/chromeos-acpi-device.rst             | 363 +++++++++++++
>>>  Documentation/firmware-guide/acpi/index.rst   |   1 +
>>>  drivers/platform/x86/Kconfig                  |  11 +
>>>  drivers/platform/x86/Makefile                 |   3 +
>>>  drivers/platform/x86/chromeos_acpi.c          | 513 ++++++++++++++++++
>>>  6 files changed, 1017 insertions(+)
>>>  create mode 100644 Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>>>  create mode 100644 Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>>>  create mode 100644 drivers/platform/x86/chromeos_acpi.c
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-driver-chromeos-acpi b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>>> new file mode 100644
>>> index 0000000000000..b74fbca62f4c3
>>> --- /dev/null
>>> +++ b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
>>> @@ -0,0 +1,126 @@
>>> +What:		/sys/bus/platform/devices/GGL0001:00/BINF.2
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows information about the current boot of
>>> +		the active EC firmware.
>>> +		  * 0 - Read only (recovery) firmware.
>>> +		  * 1 - Rewritable firmware.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/BINF.3
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows information about the current boot of
>>> +		the active main	firmware type.
>>> +		  * 0 - Recovery.
>>> +		  * 1 - Normal.
>>> +		  * 2 - Developer.
>>> +		  * 3 - Netboot (factory installation only).
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/CHSW
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the switch position for the Chrome OS specific
>>> +		hardware switches.
>>> +		  * 0   - No changes.
>>> +		  * 2   - Recovery button was pressed when firmware booted.
>>> +		  * 4   - Recovery button was pressed when EC firmware booted.
>>> +		  * 32  - Developer switch was enabled when firmware booted.
>>> +		  * 512 - Firmware write protect was disabled when firmware
>>> +			  booted.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/FMAP
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the physical memory address of the start of
>>> +		the main processor firmware flashmap.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/FRID
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the firmware version for the read-only portion
>>> +		of the main processor firmware.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/FWID
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the firmware version for the rewritable portion
>>> +		of the main processor firmware.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.0
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the type of the GPIO signal for the Chrome OS
>>> +		specific GPIO assignments.
>>> +		  * 1   - Recovery button.
>>> +		  * 2   - Developer mode switch.
>>> +		  * 3   - Firmware write protect switch.
>>> +		  * 256 to 511 - Debug header GPIO 0 to GPIO 255.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.1
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the signal attributes of the GPIO signal.
>>> +		  * 0 - Signal is active low.
>>> +		  * 1 - Signal is active high.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.2
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the GPIO number on the specified GPIO
>>> +		controller.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.3
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the name of the GPIO controller.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/HWID
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the hardware ID for the Chromebook.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/MECK
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This binary file returns the SHA-1 or SHA-256 hash that is
>>> +		read out of the Management Engine extend registers during
>>> +		boot. The hash is exported vi ACPI so the OS can verify that
>>> +		the Management Engine firmware has not changed. If Management
>>> +		Engine is not present, or if the firmware was unable to read the
>>> +		extend registers, this buffer size can be zero.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/VBNV.0
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the offset in CMOS bank 0 of the verified boot
>>> +		non-volatile storage block, counting from the first writable
>>> +		CMOS byte (that is, 'offset = 0' is the byte following the 14
>>> +		bytes of clock data).
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/VBNV.1
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This file shows the size in bytes of the verified boot
>>> +		non-volatile storage block.
>>> +
>>> +What:		/sys/bus/platform/devices/GGL0001:00/VDAT
>>> +Date:		April 2022
>>> +KernelVersion:	5.19
>>> +Description:
>>> +		This binary file returns the verified boot data block shared
>>> +		between the firmware verification step and the kernel
>>> +		verification step.
>>> diff --git a/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>>> new file mode 100644
>>> index 0000000000000..ef3cc980bad54
>>> --- /dev/null
>>> +++ b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
>>> @@ -0,0 +1,363 @@
>>> +.. SPDX-License-Identifier: GPL-2.0
>>> +
>>> +=====================
>>> +Chrome OS ACPI Device
>>> +=====================
>>> +
>>> +Hardware functionality specific to Chrome OS is exposed through a Chrome OS ACPI device.
>>> +The plug and play ID of a Chrome OS ACPI device is GGL0001. GGL is valid PNP ID of Google.
>>> +PNP ID can be used with the ACPI devices accourding to the guidelines. The following ACPI
>>> +objects are supported:
>>> +
>>> +.. flat-table:: Supported ACPI Objects
>>> +   :widths: 1 2
>>> +   :header-rows: 1
>>> +
>>> +   * - Object
>>> +     - Description
>>> +
>>> +   * - CHSW
>>> +     - Chrome OS switch positions
>>> +
>>> +   * - HWID
>>> +     - Chrome OS hardware ID
>>> +
>>> +   * - FWID
>>> +     - Chrome OS firmware version
>>> +
>>> +   * - FRID
>>> +     - Chrome OS read-only firmware version
>>> +
>>> +   * - BINF
>>> +     - Chrome OS boot information
>>> +
>>> +   * - GPIO
>>> +     - Chrome OS GPIO assignments
>>> +
>>> +   * - VBNV
>>> +     - Chrome OS NVRAM locations
>>> +
>>> +   * - VDTA
>>> +     - Chrome OS verified boot data
>>> +
>>> +   * - FMAP
>>> +     - Chrome OS flashmap base address
>>> +
>>> +   * - MLST
>>> +     - Chrome OS method list
>>> +
>>> +CHSW (Chrome OS switch positions)
>>> +=================================
>>> +This control method returns the switch positions for Chrome OS specific hardware switches.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +An integer containing the switch positions as bitfields:
>>> +
>>> +.. flat-table::
>>> +   :widths: 1 2
>>> +
>>> +   * - 0x00000002
>>> +     - Recovery button was pressed when x86 firmware booted.
>>> +
>>> +   * - 0x00000004
>>> +     - Recovery button was pressed when EC firmware booted. (required if EC EEPROM is
>>> +       rewritable; otherwise optional)
>>> +
>>> +   * - 0x00000020
>>> +     - Developer switch was enabled when x86 firmware booted.
>>> +
>>> +   * - 0x00000200
>>> +     - Firmware write protect was disabled when x86 firmware booted. (required if
>>> +       firmware write protect is controlled through x86 BIOS; otherwise optional)
>>> +
>>> +All other bits are reserved and should be set to 0.
>>> +
>>> +HWID (Chrome OS hardware ID)
>>> +============================
>>> +This control method returns the hardware ID for the Chromebook.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +A null-terminated ASCII string containing the hardware ID from the Model-Specific Data area of
>>> +EEPROM.
>>> +
>>> +Note that the hardware ID can be up to 256 characters long, including the terminating null.
>>> +
>>> +FWID (Chrome OS firmware version)
>>> +=================================
>>> +This control method returns the firmware version for the rewritable portion of the main
>>> +processor firmware.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +A null-terminated ASCII string containing the complete firmware version for the rewritable
>>> +portion of the main processor firmware.
>>> +
>>> +FRID (Chrome OS read-only firmware version)
>>> +===========================================
>>> +This control method returns the firmware version for the read-only portion of the main
>>> +processor firmware.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +A null-terminated ASCII string containing the complete firmware version for the read-only
>>> +(bootstrap + recovery ) portion of the main processor firmware.
>>> +
>>> +BINF (Chrome OS boot information)
>>> +=================================
>>> +This control method returns information about the current boot.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +
>>> +.. code-block::
>>> +
>>> +   Package {
>>> +           Reserved1
>>> +           Reserved2
>>> +           Active EC Firmware
>>> +           Active Main Firmware Type
>>> +           Reserved5
>>> +   }
>>> +
>>> +.. flat-table::
>>> +   :widths: 1 1 2
>>> +   :header-rows: 1
>>> +
>>> +   * - Field
>>> +     - Format
>>> +     - Description
>>> +
>>> +   * - Reserved1
>>> +     - DWORD
>>> +     - Set to 256 (0x100). This indicates this field is no longer used.
>>> +
>>> +   * - Reserved2
>>> +     - DWORD
>>> +     - Set to 256 (0x100). This indicates this field is no longer used.
>>> +
>>> +   * - Active EC firmware
>>> +     - DWORD
>>> +     - The EC firmware which was used during boot.
>>> +
>>> +       - 0 - Read-only (recovery) firmware
>>> +       - 1 - Rewritable firmware.
>>> +
>>> +       Set to 0 if EC firmware is always read-only.
>>> +
>>> +   * - Active Main Firmware Type
>>> +     - DWORD
>>> +     - The main firmware type which was used during boot.
>>> +
>>> +       - 0 - Recovery
>>> +       - 1 - Normal
>>> +       - 2 - Developer
>>> +       - 3 - netboot (factory installation only)
>>> +
>>> +       Other values are reserved.
>>> +
>>> +   * - Reserved5
>>> +     - DWORD
>>> +     - Set to 256 (0x100). This indicates this field is no longer used.
>>> +
>>> +GPIO (Chrome OS GPIO assignments)
>>> +=================================
>>> +This control method returns information about Chrome OS specific GPIO assignments for
>>> +Chrome OS hardware, so the kernel can directly control that hardware.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +.. code-block::
>>> +
>>> +        Package {
>>> +                Package {
>>> +                        // First GPIO assignment
>>> +                        Signal Type        //DWORD
>>> +                        Attributes         //DWORD
>>> +                        Controller Offset  //DWORD
>>> +                        Controller Name    //ASCIIZ
>>> +                },
>>> +                ...
>>> +                Package {
>>> +                        // Last GPIO assignment
>>> +                        Signal Type        //DWORD
>>> +                        Attributes         //DWORD
>>> +                        Controller Offset  //DWORD
>>> +                        Controller Name    //ASCIIZ
>>> +                }
>>> +        }
>>> +
>>> +Where ASCIIZ means a null-terminated ASCII string.
>>> +
>>> +.. flat-table::
>>> +   :widths: 1 1 2
>>> +   :header-rows: 1
>>> +
>>> +   * - Field
>>> +     - Format
>>> +     - Description
>>> +
>>> +   * - Signal Type
>>> +     - DWORD
>>> +     - Type of GPIO signal
>>> +
>>> +       - 0x00000001 - Recovery button
>>> +       - 0x00000002 - Developer mode switch
>>> +       - 0x00000003 - Firmware write protect switch
>>> +       - 0x00000100 - Debug header GPIO 0
>>> +       - ...
>>> +       - 0x000001FF - Debug header GPIO 255
>>> +
>>> +       Other values are reserved.
>>> +
>>> +   * - Attributes
>>> +     - DWORD
>>> +     - Signal attributes as bitfields:
>>> +
>>> +       - 0x00000001 - Signal is active-high (for button, a GPIO value
>>> +         of 1 means the button is pressed; for switches, a GPIO value
>>> +         of 1 means the switch is enabled). If this bit is 0, the signal
>>> +         is active low. Set to 0 for debug header GPIOs.
>>> +
>>> +   * - Controller Offset
>>> +     - DWORD
>>> +     - GPIO number on the specified controller.
>>> +
>>> +   * - Controller Name
>>> +     - ASCIIZ
>>> +     - Name of the controller for the GPIO.
>>> +       Currently supported names:
>>> +       "NM10" - Intel NM10 chip
>>> +
>>> +VBNV (Chrome OS NVRAM locations)
>>> +================================
>>> +This control method returns information about the NVRAM (CMOS) locations used to
>>> +communicate with the BIOS.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +.. code-block::
>>> +
>>> +        Package {
>>> +                NV Storage Block Offset  //DWORD
>>> +                NV Storage Block Size    //DWORD
>>> +        }
>>> +
>>> +.. flat-table::
>>> +   :widths: 1 1 2
>>> +   :header-rows: 1
>>> +
>>> +   * - Field
>>> +     - Format
>>> +     - Description
>>> +
>>> +   * - NV Storage Block Offset
>>> +     - DWORD
>>> +     - Offset in CMOS bank 0 of the verified boot non-volatile storage block, counting from
>>> +       the first writable CMOS byte (that is, offset=0 is the byte following the 14 bytes of
>>> +       clock data).
>>> +
>>> +   * - NV Storage Block Size
>>> +     - DWORD
>>> +     - Size in bytes of the verified boot non-volatile storage block.
>>> +
>>> +FMAP (Chrome OS flashmap address)
>>> +=================================
>>> +This control method returns the physical memory address of the start of the main processor
>>> +firmware flashmap.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +NoneResult code:
>>> +----------------
>>> +A DWORD containing the physical memory address of the start of the main processor firmware
>>> +flashmap.
>>> +
>>> +VDTA (Chrome OS verified boot data)
>>> +===================================
>>> +This control method returns the verified boot data block shared between the firmware
>>> +verification step and the kernel verification step.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +A buffer containing the verified boot data block.
>>> +
>>> +MECK (Management Engine Checksum)
>>> +=================================
>>> +This control method returns the SHA-1 or SHA-256 hash that is read out of the Management
>>> +Engine extend registers during boot. The hash is exported via ACPI so the OS can verify that
>>> +the ME firmware has not changed. If Management Engine is not present, or if the firmware was
>>> +unable to read the extend registers, this buffer can be zero.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +A buffer containing the ME hash.
>>> +
>>> +MLST (Chrome OS method list)
>>> +============================
>>> +This control method returns a list of the other control methods supported by the Chrome OS
>>> +hardware device.
>>> +
>>> +Arguments:
>>> +----------
>>> +None
>>> +
>>> +Result code:
>>> +------------
>>> +A package containing a list of null-terminated ASCII strings, one for each control method
>>> +supported by the Chrome OS hardware device, not including the MLST method itself.
>>> +For this version of the specification, the result is:
>>> +
>>> +.. code-block::
>>> +
>>> +        Package {
>>> +                "CHSW",
>>> +                "FWID",
>>> +                "HWID",
>>> +                "FRID",
>>> +                "BINF",
>>> +                "GPIO",
>>> +                "VBNV",
>>> +                "FMAP",
>>> +                "VDTA",
>>> +                "MECK"
>>> +        }
>>> diff --git a/Documentation/firmware-guide/acpi/index.rst b/Documentation/firmware-guide/acpi/index.rst
>>> index b053b0c3d6969..b6a42f4ffe032 100644
>>> --- a/Documentation/firmware-guide/acpi/index.rst
>>> +++ b/Documentation/firmware-guide/acpi/index.rst
>>> @@ -29,3 +29,4 @@ ACPI Support
>>>     non-d0-probe
>>>     extcon-intel-int3496
>>>     intel-pmc-mux
>>> +   chromeos-acpi-device
>>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>>> index 5d9dd70e4e0f5..53ddfe6f99b06 100644
>>> --- a/drivers/platform/x86/Kconfig
>>> +++ b/drivers/platform/x86/Kconfig
>>> @@ -322,6 +322,17 @@ config ASUS_NB_WMI
>>>  	  If you have an ACPI-WMI compatible Asus Notebook, say Y or M
>>>  	  here.
>>>  
>>> +config CHROMEOS_ACPI
>>> +	tristate "ChromeOS specific ACPI extensions"
>>> +	depends on ACPI || COMPILE_TEST
>>> +	help
>>> +	  This driver provides the firmware interface for the services
>>> +	  exported through the ChromeOS interfaces when using ChromeOS
>>> +	  ACPI firmware.
>>> +
>>> +	  If you have an ACPI-compatible Chromebook, say Y or M
>>> +	  here.
>>> +
>>>  config ASUS_TF103C_DOCK
>>>  	tristate "Asus TF103C 2-in-1 keyboard dock"
>>>  	depends on ACPI
>>> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
>>> index fe4d4c8970efa..ae68f4fa9a6c4 100644
>>> --- a/drivers/platform/x86/Makefile
>>> +++ b/drivers/platform/x86/Makefile
>>> @@ -44,6 +44,9 @@ obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
>>>  # Cisco/Meraki
>>>  obj-$(CONFIG_MERAKI_MX100)	+= meraki-mx100.o
>>>  
>>> +# Chrome
>>> +obj-$(CONFIG_CHROMEOS_ACPI)	+= chromeos_acpi.o
>>> +
>>>  # Dell
>>>  obj-$(CONFIG_X86_PLATFORM_DRIVERS_DELL)		+= dell/
>>>  
>>> diff --git a/drivers/platform/x86/chromeos_acpi.c b/drivers/platform/x86/chromeos_acpi.c
>>> new file mode 100644
>>> index 0000000000000..de86119a446b8
>>> --- /dev/null
>>> +++ b/drivers/platform/x86/chromeos_acpi.c
>>> @@ -0,0 +1,513 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only
>>> +/*
>>> + * ChromeOS specific ACPI extensions
>>> + *
>>> + * Copyright 2011 Google, Inc.
>>> + * Copyright 2020-2022 Google LLC
>>> + *
>>> + * This file is a rework and part of the code is ported from chromeos-3.18
>>> + * kernel and was originally written by Vadim Bendebury <[e-mail]>.
>>> + *
>>> + * This driver attaches to the ChromeOS ACPI device and then exports the
>>> + * values reported by the ACPI in a sysfs directory. All values are
>>> + * presented in the string form (numbers as decimal values) and can be
>>> + * accessed as the contents of the appropriate read only files in the
>>> + * sysfs directory tree.
>>> + */
>>> +
>>> +#include <linux/acpi.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/kernel.h>
>>> +#include <linux/list.h>
>>> +#include <linux/module.h>
>>> +
>>> +/*
>>> + * ACPI method name for MLST; the response for this method is a package of
>>> + * strings listing the methods which should be reflected in sysfs.
>>> + */
>>> +#define MLST "MLST"
>>> +
>>> +/*
>>> + * The default list of methods the ChromeOS ACPI device is supposed to export,
>>> + * if the MLST method is not present or is poorly formed.  The MLST method
>>> + * itself is included, to aid in debugging.
>>> + */
>>> +static char *chromeos_acpi_default_methods[] = {
>>> +	"CHSW", "HWID", "BINF", "GPIO", "CHNV", "FWID", "FRID", MLST
>>> +};
>>> +
>>> +/*
>>> + * Representation of a single sysfs attribute. In addition to the standard
>>> + * bin_attribute structure has a list of these structures (to keep track for
>>> + * de-allocation when removing the driver) and a pointer to the actual
>>> + * attribute name and value, reported when accessing the appropriate sysfs
>>> + * file.
>>> + */
>>> +struct chromeos_acpi_attribute {
>>> +	struct bin_attribute bin_attr;
>>> +	struct list_head list;
>>> +	char *name;
>>> +	char *data;
>>> +};
>>> +
>>> +/*
>>> + * Representation of a sysfs attribute group (a sub directory in the device's
>>> + * sysfs directory). In addition to the standard structure has lists to allow
>>> + * to keep track of the allocated structures.
>>> + */
>>> +struct chromeos_acpi_attribute_group {
>>> +	struct attribute_group group;
>>> +	struct list_head attribs;
>>> +	struct list_head list;
>>> +	char *name;
>>> +};
>>> +
>>> +/*
>>> + * This is the main structure, we use it to store data and adds links pointing
>>> + * at lists of allocated attributes and attribute groups.
>>> + */
>>> +struct chromeos_acpi_dev {
>>> +	struct chromeos_acpi_attribute_group *root;
>>> +	const struct attribute_group **dev_groups;
>>> +	struct list_head groups;
>>> +	unsigned int num_groups;
>>> +	unsigned int num_attrs;
>>> +};
>>> +
>>> +static struct chromeos_acpi_dev chromeos_acpi;
>>> +
>>> +static ssize_t chromeos_acpi_read_bin_attribute(struct file *filp,
>>> +						struct kobject *kobj,
>>> +						struct bin_attribute *bin_attr,
>>> +						char *buffer, loff_t pos,
>>> +						size_t count)
>>> +{
>>> +	struct chromeos_acpi_attribute *info = bin_attr->private;
>>> +
>>> +	return memory_read_from_buffer(buffer, count, &pos, info->data,
>>> +				       info->bin_attr.size);
>>> +}
>>> +
>>> +static char *chromeos_acpi_gen_file_name(char *name, int count, int index)
>>> +{
>>> +	char *str;
>>> +
>>> +	if (count == 1)
>>> +		str = kstrdup(name, GFP_KERNEL);
>>> +	else
>>> +		str = kasprintf(GFP_KERNEL, "%s.%d", name, index);
>>> +
>>> +	return str;
>>> +}
>>> +
>>> +static int
>>> +chromeos_acpi_add_attr(struct chromeos_acpi_attribute_group *aag,
>>> +		       union acpi_object *element, char *name,
>>> +		       int count, int index)
>>> +{
>>> +	struct chromeos_acpi_attribute *info;
>>> +	char buffer[24]; /* enough to store a u64 and null character */
>>> +	int length;
>>> +	int ret;
>>> +
>>> +	/* Files BINF.{0,1,4} are historical and no longer used. */
>>> +	if (!strcmp(name, "BINF") && (index == 0 || index == 1 || index == 4))
>>> +		return 0;
>>> +
>>> +	info = kzalloc(sizeof(*info), GFP_KERNEL);
>>> +	if (!info)
>>> +		return -ENOMEM;
>>> +
>>> +	info->name = chromeos_acpi_gen_file_name(name, count, index);
>>> +	if (!info->name) {
>>> +		ret = -ENOMEM;
>>> +		goto free_attribute;
>>> +	}
>>> +	sysfs_bin_attr_init(&info->bin_attr);
>>> +	info->bin_attr.attr.name = info->name;
>>> +	info->bin_attr.attr.mode = 0444;
>>> +
>>> +	switch (element->type) {
>>> +	case ACPI_TYPE_BUFFER:
>>> +		length = element->buffer.length;
>>> +		info->data = kmemdup(element->buffer.pointer,
>>> +				     length, GFP_KERNEL);
>>> +		break;
>>> +	case ACPI_TYPE_INTEGER:
>>> +		length = snprintf(buffer, sizeof(buffer), "%d",
>>> +				  (int)element->integer.value);
>>> +		info->data = kmemdup(buffer, length, GFP_KERNEL);
>>> +		break;
>>> +	case ACPI_TYPE_STRING:
>>> +		length = element->string.length + 1;
>>> +		info->data = kstrdup(element->string.pointer, GFP_KERNEL);
>>> +		break;
>>> +	default:
>>> +		ret = -EINVAL;
>>> +		goto free_attr_name;
>>> +	}
>>> +
>>> +	if (!info->data) {
>>> +		ret = -ENOMEM;
>>> +		goto free_attr_name;
>>> +	}
>>> +
>>> +	info->bin_attr.size = length;
>>> +	info->bin_attr.read = chromeos_acpi_read_bin_attribute;
>>> +	info->bin_attr.private = info;
>>> +
>>> +	INIT_LIST_HEAD(&info->list);
>>> +
>>> +	list_add(&info->list, &aag->attribs);
>>> +	return 0;
>>> +
>>> +free_attr_name:
>>> +	kfree(info->name);
>>> +free_attribute:
>>> +	kfree(info);
>>> +	return ret;
>>> +}
>>> +
>>> +static void
>>> +chromeos_acpi_remove_attribs(struct chromeos_acpi_attribute_group *aag)
>>> +{
>>> +	struct chromeos_acpi_attribute *attr, *tmp_attr;
>>> +
>>> +	list_for_each_entry_safe(attr, tmp_attr, &aag->attribs, list) {
>>> +		kfree(attr->name);
>>> +		kfree(attr->data);
>>> +		kfree(attr);
>>> +	}
>>> +}
>>> +
>>> +static int
>>> +chromeos_acpi_add_attribs_to_group(struct chromeos_acpi_attribute_group *aag,
>>> +				   unsigned int num_attrs)
>>> +{
>>> +	struct chromeos_acpi_attribute *attr;
>>> +	int count = 0;
>>> +
>>> +	aag->group.bin_attrs = kcalloc(num_attrs + 1,
>>> +				       sizeof(*aag->group.bin_attrs),
>>> +				       GFP_KERNEL);
>>> +	if (!aag->group.bin_attrs)
>>> +		return -ENOMEM;
>>> +
>>> +	list_for_each_entry(attr, &aag->attribs, list) {
>>> +		aag->group.bin_attrs[count] = &attr->bin_attr;
>>> +		count++;
>>> +	}
>>> +
>>> +	chromeos_acpi.num_groups++;
>>> +	list_add(&aag->list, &chromeos_acpi.groups);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * chromeos_acpi_add_group() - Create a sysfs group including attributes
>>> + *			       representing a nested ACPI package
>>> + *
>>> + * @obj: Package contents as returned by ACPI
>>> + * @name: Name of the group
>>> + * @num_attrs: Number of attributes of this package
>>> + * @index: Index number of this particular group
>>> + *
>>> + * The created group is called @name in case there is a single instance, or
>>> + * @name.@index otherwise.
>>> + *
>>> + * All group and attribute storage allocations are included in the lists for
>>> + * tracking of allocated memory.
>>> + *
>>> + * Return: 0 on success, negative errno on failure.
>>> + */
>>> +static int chromeos_acpi_add_group(union acpi_object *obj, char *name,
>>> +				   int num_attrs, int index)
>>> +{
>>> +	struct chromeos_acpi_attribute_group *aag;
>>> +	union acpi_object *element;
>>> +	int i, count, ret;
>>> +
>>> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
>>> +	if (!aag)
>>> +		return -ENOMEM;
>>> +	aag->name = chromeos_acpi_gen_file_name(name, num_attrs, index);
>>> +	if (!aag->name) {
>>> +		ret = -ENOMEM;
>>> +		goto free_group;
>>> +	}
>>> +
>>> +	INIT_LIST_HEAD(&aag->attribs);
>>> +	INIT_LIST_HEAD(&aag->list);
>>> +
>>> +	count = obj->package.count;
>>> +	element = obj->package.elements;
>>> +	for (i = 0; i < count; i++, element++) {
>>> +		ret = chromeos_acpi_add_attr(aag, element, name, count, i);
>>> +		if (ret)
>>> +			goto free_group_attr;
>>> +	}
>>> +
>>> +	aag->group.name = aag->name;
>>> +
>>> +	ret = chromeos_acpi_add_attribs_to_group(aag, count);
>>> +	if (ret)
>>> +		goto free_group_attr;
>>> +
>>> +	return 0;
>>> +
>>> +free_group_attr:
>>> +	chromeos_acpi_remove_attribs(aag);
>>> +	kfree(aag->name);
>>> +free_group:
>>> +	kfree(aag);
>>> +	return ret;
>>> +}
>>> +
>>> +static void chromeos_acpi_remove_groups(void)
>>> +{
>>> +	struct chromeos_acpi_attribute_group *aag, *tmp_aag;
>>> +
>>> +	list_for_each_entry_safe(aag, tmp_aag, &chromeos_acpi.groups, list) {
>>> +		chromeos_acpi_remove_attribs(aag);
>>> +		kfree(aag->group.bin_attrs);
>>> +		kfree(aag->name);
>>> +		kfree(aag);
>>> +	}
>>> +}
>>> +
>>> +/**
>>> + * chromeos_acpi_handle_package() - Create sysfs group including attributes
>>> + *				    representing an ACPI package
>>> + *
>>> + * @pdev: Platform device
>>> + * @obj: Package contents as returned by ACPI
>>> + * @name: Name of the group
>>> + *
>>> + * Scalar objects included in the package get sysfs attributes created for
>>> + * them. Nested packages are passed to a function creating a sysfs group per
>>> + * package.
>>> + *
>>> + * Return: 0 on success, negative errno on failure.
>>> + */
>>> +static int chromeos_acpi_handle_package(struct platform_device *pdev,
>>> +					union acpi_object *obj, char *name)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	int count = obj->package.count;
>>> +	union acpi_object *element;
>>> +	int i, ret;
>>> +
>>> +	element = obj->package.elements;
>>> +	for (i = 0; i < count; i++, element++) {
>>> +		if (element->type == ACPI_TYPE_BUFFER ||
>>> +		    element->type == ACPI_TYPE_STRING ||
>>> +		    element->type == ACPI_TYPE_INTEGER) {
>>> +			/* Create a single attribute in the root directory */
>>> +			ret = chromeos_acpi_add_attr(chromeos_acpi.root,
>>> +						     element, name,
>>> +						     count, i);
>>> +			if (ret) {
>>> +				dev_err(dev, "error adding attributes (%d)\n",
>>> +					ret);
>>> +				return ret;
>>> +			}
>>> +			chromeos_acpi.num_attrs++;
>>> +		} else if (element->type == ACPI_TYPE_PACKAGE) {
>>> +			/* Create a group of attributes */
>>> +			ret = chromeos_acpi_add_group(element, name, count, i);
>>> +			if (ret) {
>>> +				dev_err(dev, "error adding a group (%d)\n",
>>> +					ret);
>>> +				return ret;
>>> +			}
>>> +		} else {
>>> +			if (ret) {
>>> +				dev_err(dev, "error on element type (%d)\n",
>>> +					ret);
>>> +				return -EINVAL;
>>> +			}
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * chromeos_acpi_add_method() - Evaluate an ACPI method and create sysfs
>>> + *				attributes
>>> + *
>>> + * @pdev: Platform device
>>> + * @name: Name of the method to evaluate
>>> + *
>>> + * Return: 0 on success, non-zero on failure
>>> + */
>>> +static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
>>> +{
>>> +	struct device *dev = &pdev->dev;
>>> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +	acpi_status status;
>>> +	int ret = 0;
>>> +
>>> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
>>> +	if (ACPI_FAILURE(status)) {
>>> +		dev_err(dev, "failed to retrieve %s (%d)\n", name, status);
>>> +		return status;
>>> +	}
>>> +
>>> +	if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
>>> +		ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
>>> +
>>> +	kfree(output.pointer);
>>> +	return ret;
>>> +}
>>> +
>>> +/**
>>> + * chromeos_acpi_process_mlst() - Evaluate the MLST method and add methods
>>> + *				  listed in the response
>>> + *
>>> + * @pdev: Platform device
>>> + *
>>> + * Returns: 0 if successful, non-zero if error.
>>> + */
>>> +static int chromeos_acpi_process_mlst(struct platform_device *pdev)
>>> +{
>>> +	struct chromeos_acpi_attribute_group *aag;
>>> +	char name[ACPI_NAMESEG_SIZE + 1];
>>> +	union acpi_object *element, *obj;
>>> +	struct device *dev = &pdev->dev;
>>> +	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
>>> +	acpi_status status;
>>> +	int ret = 0;
>>> +	int size;
>>> +	int i;
>>> +
>>> +	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
>>> +				      &output);
>>> +	if (ACPI_FAILURE(status))
>>> +		return status;
>>> +
>>> +	obj = output.pointer;
>>> +	if (obj->type != ACPI_TYPE_PACKAGE) {
>>> +		ret = -EINVAL;
>>> +		goto free_acpi_buffer;
>>> +	}
>>> +
>>> +	element = obj->package.elements;
>>> +	for (i = 0; i < obj->package.count; i++, element++) {
>>> +		if (element->type == ACPI_TYPE_STRING) {
>>> +			size = min(element->string.length + 1,
>>> +				   (u32)ACPI_NAMESEG_SIZE + 1);
>>> +			strscpy(name, element->string.pointer, size);
>>> +			ret = chromeos_acpi_add_method(pdev, name);
>>> +			if (ret) {
>>> +				chromeos_acpi_remove_groups();
>>> +				break;
>>> +			}
>>> +		}
>>> +	}
>>> +
>>> +	/* Add root attributes to the main group */
>>> +	ret = chromeos_acpi_add_attribs_to_group(chromeos_acpi.root,
>>> +						 chromeos_acpi.num_attrs);
>>> +	if (ret)
>>> +		goto free_acpi_buffer;
>>> +
>>> +	chromeos_acpi.dev_groups = kcalloc(chromeos_acpi.num_groups + 1,
>>> +					   sizeof(struct attribute_group),
>>> +					   GFP_KERNEL);
>>> +
>>> +	i = 0;
>>> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
>>> +		chromeos_acpi.dev_groups[i] = &aag->group;
>>> +		i++;
>>> +	}
>>> +
>>> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
>>> +	if (ret) {
>>> +		kfree(chromeos_acpi.dev_groups);
>>> +
>>> +		/* Remove allocated chromeos acpi groups and attributes */
>>> +		chromeos_acpi_remove_groups();
>>> +	}
>>> +
>>> +free_acpi_buffer:
>>> +	kfree(output.pointer);
>>> +	return ret;
>>> +}
>>> +
>>> +static int chromeos_acpi_device_probe(struct platform_device *pdev)
>>> +{
>>> +	struct chromeos_acpi_attribute_group *aag;
>>> +	struct device *dev = &pdev->dev;
>>> +	int i, ret;
>>> +
>>> +	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
>>> +	if (!aag)
>>> +		return -ENOMEM;
>>> +
>>> +	INIT_LIST_HEAD(&aag->attribs);
>>> +	INIT_LIST_HEAD(&aag->list);
>>> +	INIT_LIST_HEAD(&chromeos_acpi.groups);
>>> +
>>> +	chromeos_acpi.root = aag;
>>> +
>>> +	/*
>>> +	 * Attempt to add methods by querying the device's MLST method
>>> +	 * for the list of methods.
>>> +	 */
>>> +	if (!chromeos_acpi_process_mlst(pdev))
>>> +		return 0;
>>> +
>>> +	dev_dbg(dev, "falling back to default list of methods\n");
>>> +
>>> +	for (i = 0; i < ARRAY_SIZE(chromeos_acpi_default_methods); i++) {
>>> +		ret = chromeos_acpi_add_method(pdev,
>>> +					       chromeos_acpi_default_methods[i]);
>>> +		if (ret) {
>>> +			dev_err(dev, "failed to add default methods (%d)\n",
>>> +				ret);
>>> +			goto free_group_root;
>>> +		}
>>> +	}
>>> +
>>> +	return 0;
>>> +
>>> +free_group_root:
>>> +	kfree(chromeos_acpi.root);
>>> +	return ret;
>>> +}
>>> +
>>> +static int chromeos_acpi_device_remove(struct platform_device *pdev)
>>> +{
>>> +	/* Remove sysfs groups */
>>> +	sysfs_remove_groups(&pdev->dev.kobj, chromeos_acpi.dev_groups);
>>> +	kfree(chromeos_acpi.dev_groups);
>>> +
>>> +	/* Remove allocated chromeos acpi groups and attributes */
>>> +	chromeos_acpi_remove_groups();
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/* GGL is valid PNP ID of Google. PNP ID can be used with the ACPI devices. */
>>> +static const struct acpi_device_id chromeos_device_ids[] = {
>>> +	{ "GGL0001", 0 },
>>> +	{ }
>>> +};
>>> +MODULE_DEVICE_TABLE(acpi, chromeos_device_ids);
>>> +
>>> +static struct platform_driver chromeos_acpi_device_driver = {
>>> +	.probe = chromeos_acpi_device_probe,
>>> +	.remove = chromeos_acpi_device_remove,
>>> +	.driver = {
>>> +		.name   = "chromeos-acpi",
>>> +		.acpi_match_table = ACPI_PTR(chromeos_device_ids)
>>> +	}
>>> +};
>>> +
>>> +module_platform_driver(chromeos_acpi_device_driver);
>>> +
>>> +MODULE_AUTHOR("Enric Balletbo i Serra <[e-mail]>");
>>> +MODULE_LICENSE("GPL");
>>> +MODULE_DESCRIPTION("ChromeOS specific ACPI extensions");
>>
>

Muhammad Usama Anjum April 28, 2022, 8:06 a.m. UTC | #9

On 4/24/22 1:43 PM, Greg Kroah-Hartman wrote:
> On Fri, Apr 15, 2022 at 10:08:15PM +0500, Muhammad Usama Anjum wrote:
>> +	i = 0;
>> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
>> +		chromeos_acpi.dev_groups[i] = &aag->group;
>> +		i++;
>> +	}
>> +
>> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
> 
> You have raced with userspace and lost here :(
> 
> Use the default groups pointer in the platform driver for this, and use
> the is_visible() callback to know to show, or not show, the attribute
> instead of building up dynamic lists of attributes at runtime.  That
> will save you lots of crazy logic and housekeeping _AND_ userspace tools
> will work properly as well.
> 
Yeah, using dev_group pointer in platform driver makes it simple. We
need to define the attributes at compile time. At run time, we can only
make decision to show or not to show the attribute. This simplifies
logic for the static attributes which will always be there. But there
are some set of attributes which may be 1 to N depending upon the
platform such as GPIO.0, GPIO.2 .... and GPIO.N. I'm working on
overcoming this. Once this is done, the driver is going to be very simple.

> thanks,
> 
> greg k-h

Muhammad Usama Anjum April 28, 2022, 8:12 a.m. UTC | #10

Hi,

On 4/27/22 7:22 PM, Hans de Goede wrote:
> On 4/22/22 16:47, Muhammad Usama Anjum wrote:
>> Hi Hans,
>>
>> Can I add you as a maintainer of drivers/platform/x86/chromeos_acpi.c?
>> You already maintain some other drivers in this directory.
> If you want to rely on me as default/fallback maintainer
> for the new chromeos_acpi driver, then simply don't add
> a MAINTAINERS entry for this driver.
> 
> I'm the default maintainer for everything under
> drivers/platform/x86/ .
> 
> With that said:
> 
> 
>> On 4/18/22 10:57 PM, Muhammad Usama Anjum wrote:
>>> Hi Benson,
>>>
>>> We are very near to getting this patch in. I can see that you are
>>> maintaining Chrome Platform support. This driver is in
>>> drivers/platform/x86/chromeos_acpi.c. Can you maintain this driver as
>>> well? I'll add you to the maintainers.
> now that you mention it, moving this to drivers/platform/chrome/
> actually seems like the most sensible thing to do given that
> this is chrome specific platform code. So please move it
> there for the next version.
> 
This driver was added in platform/x86 as it is only x86 specific.
Definitely it makes more sense to move it to platform/chrome. I'll move it.

> Talking about the next version, please address all review
> remarks from Barnabás for the next version.
> 
After Greg's comment to define attributes at compile time, the driver
will change completely and way simpler.

> Regards,
> 
> Hans
> 
> 
>

Muhammad Usama Anjum April 28, 2022, 12:24 p.m. UTC | #11

On 4/24/22 1:43 PM, Greg Kroah-Hartman wrote:
> On Fri, Apr 15, 2022 at 10:08:15PM +0500, Muhammad Usama Anjum wrote:
>> +	i = 0;
>> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
>> +		chromeos_acpi.dev_groups[i] = &aag->group;
>> +		i++;
>> +	}
>> +
>> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
> 
> You have raced with userspace and lost here :(
> 
Sorry, What does it mean exactly?

> Use the default groups pointer in the platform driver for this, and use
> the is_visible() callback to know to show, or not show, the attribute
> instead of building up dynamic lists of attributes at runtime.  That
> will save you lots of crazy logic and housekeeping _AND_ userspace tools
> will work properly as well.
> 

Driver has the 2 kinds of attributes:

A) Attributes which are always there. For example, CHSW and HWIDs etc.
They can be easily shows via dev_groups pointer in platform driver.

B) Attribute groups which vary between 0 to N. N is platform dependent
and can be determined at runtime. For example, GPIO attribute group
which have 4 sub attributes in it:

Group GPIO.0 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
Group GPIO.1 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
...
Group GPIO.N --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3

My Chromebook has 2 GPIO attribute groups while I've found logs of a
Chromebook which has 7 GPIO groups.

Why these groups cannot be defined at compile time (Shortcomings):

1) We don't know the total GPIO groups.
Possible solution: Determine GPIO groups' number at run time and define
attributes at run time.

2) We cannot determine from attribute name that this group will be
visible or not as is_visible doesn't provide information about its group
name.
umode_t (*is_visible)(struct kobject *, struct attribute *, int);

3) In attribute.show functions, we only know about the attribute's name
and not the group's name. We cannot evaluate and show the attribute.
ssize_t (*show)(struct device *dev, struct device_attribute *attr, char
*buf);
Possible solution for 2) and 3):
Embed the group name into attribute name like:
attributes GPIO.0_GPIO.0, GPIO.0_GPIO.1, GPIO.0_GPIO.2 and GPIO.0_GPIO.3
attributes GPIO.1_GPIO.0, GPIO.1_GPIO.1, GPIO.2_GPIO.2 and GPIO.3_GPIO.3
But this is completely new ABI which we don't desire.

After looking at dependence on runtime values, can we keep the existing
version of the driver instead of trying to workout some other hybrid
solution?

> thanks,
> 
> greg k-h

Greg Kroah-Hartman April 28, 2022, 12:40 p.m. UTC | #12

On Thu, Apr 28, 2022 at 05:24:04PM +0500, Muhammad Usama Anjum wrote:
> On 4/24/22 1:43 PM, Greg Kroah-Hartman wrote:
> > On Fri, Apr 15, 2022 at 10:08:15PM +0500, Muhammad Usama Anjum wrote:
> >> +	i = 0;
> >> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
> >> +		chromeos_acpi.dev_groups[i] = &aag->group;
> >> +		i++;
> >> +	}
> >> +
> >> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
> > 
> > You have raced with userspace and lost here :(
> > 
> Sorry, What does it mean exactly?

Long old post that describes the issue in detail is here:
	http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/

> > Use the default groups pointer in the platform driver for this, and use
> > the is_visible() callback to know to show, or not show, the attribute
> > instead of building up dynamic lists of attributes at runtime.  That
> > will save you lots of crazy logic and housekeeping _AND_ userspace tools
> > will work properly as well.
> > 
> 
> Driver has the 2 kinds of attributes:
> 
> A) Attributes which are always there. For example, CHSW and HWIDs etc.
> They can be easily shows via dev_groups pointer in platform driver.

Great.

> B) Attribute groups which vary between 0 to N. N is platform dependent
> and can be determined at runtime. For example, GPIO attribute group
> which have 4 sub attributes in it:
> 
> Group GPIO.0 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
> Group GPIO.1 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
> ...
> Group GPIO.N --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
> 
> My Chromebook has 2 GPIO attribute groups while I've found logs of a
> Chromebook which has 7 GPIO groups.
> 
> Why these groups cannot be defined at compile time (Shortcomings):
> 
> 1) We don't know the total GPIO groups.
> Possible solution: Determine GPIO groups' number at run time and define
> attributes at run time.

What is the max number of groups you can ever have?  10?  100?  1000?
Pick a high number, define them all (macros make this easy), and then
only enable the ones that you need at runtime.

> 2) We cannot determine from attribute name that this group will be
> visible or not as is_visible doesn't provide information about its group
> name.
> umode_t (*is_visible)(struct kobject *, struct attribute *, int);

Look at the attribute pointer.  That's all you care about.  Compare it
to a real pointer and away you go!

> 3) In attribute.show functions, we only know about the attribute's name
> and not the group's name. We cannot evaluate and show the attribute.
> ssize_t (*show)(struct device *dev, struct device_attribute *attr, char
> *buf);
> Possible solution for 2) and 3):
> Embed the group name into attribute name like:
> attributes GPIO.0_GPIO.0, GPIO.0_GPIO.1, GPIO.0_GPIO.2 and GPIO.0_GPIO.3
> attributes GPIO.1_GPIO.0, GPIO.1_GPIO.1, GPIO.2_GPIO.2 and GPIO.3_GPIO.3
> But this is completely new ABI which we don't desire.

This whole thing is a new abi :)

> After looking at dependence on runtime values, can we keep the existing
> version of the driver instead of trying to workout some other hybrid
> solution?

Again, you are racing with userspace and loosing.  If you do not mind
userspace not noticing the attributes, fine, leave it as-is, but odds
are you don't want that.

thanks,

greg k-h

Greg Kroah-Hartman April 28, 2022, 12:41 p.m. UTC | #13

On Thu, Apr 28, 2022 at 02:40:08PM +0200, Greg Kroah-Hartman wrote:
> On Thu, Apr 28, 2022 at 05:24:04PM +0500, Muhammad Usama Anjum wrote:
> > On 4/24/22 1:43 PM, Greg Kroah-Hartman wrote:
> > > On Fri, Apr 15, 2022 at 10:08:15PM +0500, Muhammad Usama Anjum wrote:
> > >> +	i = 0;
> > >> +	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
> > >> +		chromeos_acpi.dev_groups[i] = &aag->group;
> > >> +		i++;
> > >> +	}
> > >> +
> > >> +	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
> > > 
> > > You have raced with userspace and lost here :(
> > > 
> > Sorry, What does it mean exactly?
> 
> Long old post that describes the issue in detail is here:
> 	http://www.kroah.com/log/blog/2013/06/26/how-to-create-a-sysfs-file-correctly/
> 
> > > Use the default groups pointer in the platform driver for this, and use
> > > the is_visible() callback to know to show, or not show, the attribute
> > > instead of building up dynamic lists of attributes at runtime.  That
> > > will save you lots of crazy logic and housekeeping _AND_ userspace tools
> > > will work properly as well.
> > > 
> > 
> > Driver has the 2 kinds of attributes:
> > 
> > A) Attributes which are always there. For example, CHSW and HWIDs etc.
> > They can be easily shows via dev_groups pointer in platform driver.
> 
> Great.
> 
> > B) Attribute groups which vary between 0 to N. N is platform dependent
> > and can be determined at runtime. For example, GPIO attribute group
> > which have 4 sub attributes in it:
> > 
> > Group GPIO.0 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
> > Group GPIO.1 --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
> > ...
> > Group GPIO.N --> attributes GPIO.0, GPIO.1, GPIO.2 and GPIO.3
> > 
> > My Chromebook has 2 GPIO attribute groups while I've found logs of a
> > Chromebook which has 7 GPIO groups.
> > 
> > Why these groups cannot be defined at compile time (Shortcomings):
> > 
> > 1) We don't know the total GPIO groups.
> > Possible solution: Determine GPIO groups' number at run time and define
> > attributes at run time.
> 
> What is the max number of groups you can ever have?  10?  100?  1000?
> Pick a high number, define them all (macros make this easy), and then
> only enable the ones that you need at runtime.
> 
> > 2) We cannot determine from attribute name that this group will be
> > visible or not as is_visible doesn't provide information about its group
> > name.
> > umode_t (*is_visible)(struct kobject *, struct attribute *, int);
> 
> Look at the attribute pointer.  That's all you care about.  Compare it
> to a real pointer and away you go!

Also remember, each group has a is_visible function, so you know what
group this is implicitly.

thanks,

greg k-h

Patch

diff --git a/Documentation/ABI/testing/sysfs-driver-chromeos-acpi b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
new file mode 100644
index 0000000000000..b74fbca62f4c3
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-driver-chromeos-acpi
@@ -0,0 +1,126 @@ 
+What:		/sys/bus/platform/devices/GGL0001:00/BINF.2
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows information about the current boot of
+		the active EC firmware.
+		  * 0 - Read only (recovery) firmware.
+		  * 1 - Rewritable firmware.
+
+What:		/sys/bus/platform/devices/GGL0001:00/BINF.3
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows information about the current boot of
+		the active main	firmware type.
+		  * 0 - Recovery.
+		  * 1 - Normal.
+		  * 2 - Developer.
+		  * 3 - Netboot (factory installation only).
+
+What:		/sys/bus/platform/devices/GGL0001:00/CHSW
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the switch position for the Chrome OS specific
+		hardware switches.
+		  * 0   - No changes.
+		  * 2   - Recovery button was pressed when firmware booted.
+		  * 4   - Recovery button was pressed when EC firmware booted.
+		  * 32  - Developer switch was enabled when firmware booted.
+		  * 512 - Firmware write protect was disabled when firmware
+			  booted.
+
+What:		/sys/bus/platform/devices/GGL0001:00/FMAP
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the physical memory address of the start of
+		the main processor firmware flashmap.
+
+What:		/sys/bus/platform/devices/GGL0001:00/FRID
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the firmware version for the read-only portion
+		of the main processor firmware.
+
+What:		/sys/bus/platform/devices/GGL0001:00/FWID
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the firmware version for the rewritable portion
+		of the main processor firmware.
+
+What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.0
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the type of the GPIO signal for the Chrome OS
+		specific GPIO assignments.
+		  * 1   - Recovery button.
+		  * 2   - Developer mode switch.
+		  * 3   - Firmware write protect switch.
+		  * 256 to 511 - Debug header GPIO 0 to GPIO 255.
+
+What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.1
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the signal attributes of the GPIO signal.
+		  * 0 - Signal is active low.
+		  * 1 - Signal is active high.
+
+What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.2
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the GPIO number on the specified GPIO
+		controller.
+
+What:		/sys/bus/platform/devices/GGL0001:00/GPIO.X/GPIO.3
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the name of the GPIO controller.
+
+What:		/sys/bus/platform/devices/GGL0001:00/HWID
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the hardware ID for the Chromebook.
+
+What:		/sys/bus/platform/devices/GGL0001:00/MECK
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This binary file returns the SHA-1 or SHA-256 hash that is
+		read out of the Management Engine extend registers during
+		boot. The hash is exported vi ACPI so the OS can verify that
+		the Management Engine firmware has not changed. If Management
+		Engine is not present, or if the firmware was unable to read the
+		extend registers, this buffer size can be zero.
+
+What:		/sys/bus/platform/devices/GGL0001:00/VBNV.0
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the offset in CMOS bank 0 of the verified boot
+		non-volatile storage block, counting from the first writable
+		CMOS byte (that is, 'offset = 0' is the byte following the 14
+		bytes of clock data).
+
+What:		/sys/bus/platform/devices/GGL0001:00/VBNV.1
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This file shows the size in bytes of the verified boot
+		non-volatile storage block.
+
+What:		/sys/bus/platform/devices/GGL0001:00/VDAT
+Date:		April 2022
+KernelVersion:	5.19
+Description:
+		This binary file returns the verified boot data block shared
+		between the firmware verification step and the kernel
+		verification step.
diff --git a/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
new file mode 100644
index 0000000000000..ef3cc980bad54
--- /dev/null
+++ b/Documentation/firmware-guide/acpi/chromeos-acpi-device.rst
@@ -0,0 +1,363 @@ 
+.. SPDX-License-Identifier: GPL-2.0
+
+=====================
+Chrome OS ACPI Device
+=====================
+
+Hardware functionality specific to Chrome OS is exposed through a Chrome OS ACPI device.
+The plug and play ID of a Chrome OS ACPI device is GGL0001. GGL is valid PNP ID of Google.
+PNP ID can be used with the ACPI devices accourding to the guidelines. The following ACPI
+objects are supported:
+
+.. flat-table:: Supported ACPI Objects
+   :widths: 1 2
+   :header-rows: 1
+
+   * - Object
+     - Description
+
+   * - CHSW
+     - Chrome OS switch positions
+
+   * - HWID
+     - Chrome OS hardware ID
+
+   * - FWID
+     - Chrome OS firmware version
+
+   * - FRID
+     - Chrome OS read-only firmware version
+
+   * - BINF
+     - Chrome OS boot information
+
+   * - GPIO
+     - Chrome OS GPIO assignments
+
+   * - VBNV
+     - Chrome OS NVRAM locations
+
+   * - VDTA
+     - Chrome OS verified boot data
+
+   * - FMAP
+     - Chrome OS flashmap base address
+
+   * - MLST
+     - Chrome OS method list
+
+CHSW (Chrome OS switch positions)
+=================================
+This control method returns the switch positions for Chrome OS specific hardware switches.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+An integer containing the switch positions as bitfields:
+
+.. flat-table::
+   :widths: 1 2
+
+   * - 0x00000002
+     - Recovery button was pressed when x86 firmware booted.
+
+   * - 0x00000004
+     - Recovery button was pressed when EC firmware booted. (required if EC EEPROM is
+       rewritable; otherwise optional)
+
+   * - 0x00000020
+     - Developer switch was enabled when x86 firmware booted.
+
+   * - 0x00000200
+     - Firmware write protect was disabled when x86 firmware booted. (required if
+       firmware write protect is controlled through x86 BIOS; otherwise optional)
+
+All other bits are reserved and should be set to 0.
+
+HWID (Chrome OS hardware ID)
+============================
+This control method returns the hardware ID for the Chromebook.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+A null-terminated ASCII string containing the hardware ID from the Model-Specific Data area of
+EEPROM.
+
+Note that the hardware ID can be up to 256 characters long, including the terminating null.
+
+FWID (Chrome OS firmware version)
+=================================
+This control method returns the firmware version for the rewritable portion of the main
+processor firmware.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+A null-terminated ASCII string containing the complete firmware version for the rewritable
+portion of the main processor firmware.
+
+FRID (Chrome OS read-only firmware version)
+===========================================
+This control method returns the firmware version for the read-only portion of the main
+processor firmware.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+A null-terminated ASCII string containing the complete firmware version for the read-only
+(bootstrap + recovery ) portion of the main processor firmware.
+
+BINF (Chrome OS boot information)
+=================================
+This control method returns information about the current boot.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+
+.. code-block::
+
+   Package {
+           Reserved1
+           Reserved2
+           Active EC Firmware
+           Active Main Firmware Type
+           Reserved5
+   }
+
+.. flat-table::
+   :widths: 1 1 2
+   :header-rows: 1
+
+   * - Field
+     - Format
+     - Description
+
+   * - Reserved1
+     - DWORD
+     - Set to 256 (0x100). This indicates this field is no longer used.
+
+   * - Reserved2
+     - DWORD
+     - Set to 256 (0x100). This indicates this field is no longer used.
+
+   * - Active EC firmware
+     - DWORD
+     - The EC firmware which was used during boot.
+
+       - 0 - Read-only (recovery) firmware
+       - 1 - Rewritable firmware.
+
+       Set to 0 if EC firmware is always read-only.
+
+   * - Active Main Firmware Type
+     - DWORD
+     - The main firmware type which was used during boot.
+
+       - 0 - Recovery
+       - 1 - Normal
+       - 2 - Developer
+       - 3 - netboot (factory installation only)
+
+       Other values are reserved.
+
+   * - Reserved5
+     - DWORD
+     - Set to 256 (0x100). This indicates this field is no longer used.
+
+GPIO (Chrome OS GPIO assignments)
+=================================
+This control method returns information about Chrome OS specific GPIO assignments for
+Chrome OS hardware, so the kernel can directly control that hardware.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+.. code-block::
+
+        Package {
+                Package {
+                        // First GPIO assignment
+                        Signal Type        //DWORD
+                        Attributes         //DWORD
+                        Controller Offset  //DWORD
+                        Controller Name    //ASCIIZ
+                },
+                ...
+                Package {
+                        // Last GPIO assignment
+                        Signal Type        //DWORD
+                        Attributes         //DWORD
+                        Controller Offset  //DWORD
+                        Controller Name    //ASCIIZ
+                }
+        }
+
+Where ASCIIZ means a null-terminated ASCII string.
+
+.. flat-table::
+   :widths: 1 1 2
+   :header-rows: 1
+
+   * - Field
+     - Format
+     - Description
+
+   * - Signal Type
+     - DWORD
+     - Type of GPIO signal
+
+       - 0x00000001 - Recovery button
+       - 0x00000002 - Developer mode switch
+       - 0x00000003 - Firmware write protect switch
+       - 0x00000100 - Debug header GPIO 0
+       - ...
+       - 0x000001FF - Debug header GPIO 255
+
+       Other values are reserved.
+
+   * - Attributes
+     - DWORD
+     - Signal attributes as bitfields:
+
+       - 0x00000001 - Signal is active-high (for button, a GPIO value
+         of 1 means the button is pressed; for switches, a GPIO value
+         of 1 means the switch is enabled). If this bit is 0, the signal
+         is active low. Set to 0 for debug header GPIOs.
+
+   * - Controller Offset
+     - DWORD
+     - GPIO number on the specified controller.
+
+   * - Controller Name
+     - ASCIIZ
+     - Name of the controller for the GPIO.
+       Currently supported names:
+       "NM10" - Intel NM10 chip
+
+VBNV (Chrome OS NVRAM locations)
+================================
+This control method returns information about the NVRAM (CMOS) locations used to
+communicate with the BIOS.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+.. code-block::
+
+        Package {
+                NV Storage Block Offset  //DWORD
+                NV Storage Block Size    //DWORD
+        }
+
+.. flat-table::
+   :widths: 1 1 2
+   :header-rows: 1
+
+   * - Field
+     - Format
+     - Description
+
+   * - NV Storage Block Offset
+     - DWORD
+     - Offset in CMOS bank 0 of the verified boot non-volatile storage block, counting from
+       the first writable CMOS byte (that is, offset=0 is the byte following the 14 bytes of
+       clock data).
+
+   * - NV Storage Block Size
+     - DWORD
+     - Size in bytes of the verified boot non-volatile storage block.
+
+FMAP (Chrome OS flashmap address)
+=================================
+This control method returns the physical memory address of the start of the main processor
+firmware flashmap.
+
+Arguments:
+----------
+None
+
+NoneResult code:
+----------------
+A DWORD containing the physical memory address of the start of the main processor firmware
+flashmap.
+
+VDTA (Chrome OS verified boot data)
+===================================
+This control method returns the verified boot data block shared between the firmware
+verification step and the kernel verification step.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+A buffer containing the verified boot data block.
+
+MECK (Management Engine Checksum)
+=================================
+This control method returns the SHA-1 or SHA-256 hash that is read out of the Management
+Engine extend registers during boot. The hash is exported via ACPI so the OS can verify that
+the ME firmware has not changed. If Management Engine is not present, or if the firmware was
+unable to read the extend registers, this buffer can be zero.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+A buffer containing the ME hash.
+
+MLST (Chrome OS method list)
+============================
+This control method returns a list of the other control methods supported by the Chrome OS
+hardware device.
+
+Arguments:
+----------
+None
+
+Result code:
+------------
+A package containing a list of null-terminated ASCII strings, one for each control method
+supported by the Chrome OS hardware device, not including the MLST method itself.
+For this version of the specification, the result is:
+
+.. code-block::
+
+        Package {
+                "CHSW",
+                "FWID",
+                "HWID",
+                "FRID",
+                "BINF",
+                "GPIO",
+                "VBNV",
+                "FMAP",
+                "VDTA",
+                "MECK"
+        }
diff --git a/Documentation/firmware-guide/acpi/index.rst b/Documentation/firmware-guide/acpi/index.rst
index b053b0c3d6969..b6a42f4ffe032 100644
--- a/Documentation/firmware-guide/acpi/index.rst
+++ b/Documentation/firmware-guide/acpi/index.rst
@@ -29,3 +29,4 @@  ACPI Support
    non-d0-probe
    extcon-intel-int3496
    intel-pmc-mux
+   chromeos-acpi-device
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 5d9dd70e4e0f5..53ddfe6f99b06 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -322,6 +322,17 @@  config ASUS_NB_WMI
 	  If you have an ACPI-WMI compatible Asus Notebook, say Y or M
 	  here.
 
+config CHROMEOS_ACPI
+	tristate "ChromeOS specific ACPI extensions"
+	depends on ACPI || COMPILE_TEST
+	help
+	  This driver provides the firmware interface for the services
+	  exported through the ChromeOS interfaces when using ChromeOS
+	  ACPI firmware.
+
+	  If you have an ACPI-compatible Chromebook, say Y or M
+	  here.
+
 config ASUS_TF103C_DOCK
 	tristate "Asus TF103C 2-in-1 keyboard dock"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index fe4d4c8970efa..ae68f4fa9a6c4 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -44,6 +44,9 @@  obj-$(CONFIG_EEEPC_WMI)		+= eeepc-wmi.o
 # Cisco/Meraki
 obj-$(CONFIG_MERAKI_MX100)	+= meraki-mx100.o
 
+# Chrome
+obj-$(CONFIG_CHROMEOS_ACPI)	+= chromeos_acpi.o
+
 # Dell
 obj-$(CONFIG_X86_PLATFORM_DRIVERS_DELL)		+= dell/
 
diff --git a/drivers/platform/x86/chromeos_acpi.c b/drivers/platform/x86/chromeos_acpi.c
new file mode 100644
index 0000000000000..de86119a446b8
--- /dev/null
+++ b/drivers/platform/x86/chromeos_acpi.c
@@ -0,0 +1,513 @@ 
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ChromeOS specific ACPI extensions
+ *
+ * Copyright 2011 Google, Inc.
+ * Copyright 2020-2022 Google LLC
+ *
+ * This file is a rework and part of the code is ported from chromeos-3.18
+ * kernel and was originally written by Vadim Bendebury <[e-mail]>.
+ *
+ * This driver attaches to the ChromeOS ACPI device and then exports the
+ * values reported by the ACPI in a sysfs directory. All values are
+ * presented in the string form (numbers as decimal values) and can be
+ * accessed as the contents of the appropriate read only files in the
+ * sysfs directory tree.
+ */
+
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/module.h>
+
+/*
+ * ACPI method name for MLST; the response for this method is a package of
+ * strings listing the methods which should be reflected in sysfs.
+ */
+#define MLST "MLST"
+
+/*
+ * The default list of methods the ChromeOS ACPI device is supposed to export,
+ * if the MLST method is not present or is poorly formed.  The MLST method
+ * itself is included, to aid in debugging.
+ */
+static char *chromeos_acpi_default_methods[] = {
+	"CHSW", "HWID", "BINF", "GPIO", "CHNV", "FWID", "FRID", MLST
+};
+
+/*
+ * Representation of a single sysfs attribute. In addition to the standard
+ * bin_attribute structure has a list of these structures (to keep track for
+ * de-allocation when removing the driver) and a pointer to the actual
+ * attribute name and value, reported when accessing the appropriate sysfs
+ * file.
+ */
+struct chromeos_acpi_attribute {
+	struct bin_attribute bin_attr;
+	struct list_head list;
+	char *name;
+	char *data;
+};
+
+/*
+ * Representation of a sysfs attribute group (a sub directory in the device's
+ * sysfs directory). In addition to the standard structure has lists to allow
+ * to keep track of the allocated structures.
+ */
+struct chromeos_acpi_attribute_group {
+	struct attribute_group group;
+	struct list_head attribs;
+	struct list_head list;
+	char *name;
+};
+
+/*
+ * This is the main structure, we use it to store data and adds links pointing
+ * at lists of allocated attributes and attribute groups.
+ */
+struct chromeos_acpi_dev {
+	struct chromeos_acpi_attribute_group *root;
+	const struct attribute_group **dev_groups;
+	struct list_head groups;
+	unsigned int num_groups;
+	unsigned int num_attrs;
+};
+
+static struct chromeos_acpi_dev chromeos_acpi;
+
+static ssize_t chromeos_acpi_read_bin_attribute(struct file *filp,
+						struct kobject *kobj,
+						struct bin_attribute *bin_attr,
+						char *buffer, loff_t pos,
+						size_t count)
+{
+	struct chromeos_acpi_attribute *info = bin_attr->private;
+
+	return memory_read_from_buffer(buffer, count, &pos, info->data,
+				       info->bin_attr.size);
+}
+
+static char *chromeos_acpi_gen_file_name(char *name, int count, int index)
+{
+	char *str;
+
+	if (count == 1)
+		str = kstrdup(name, GFP_KERNEL);
+	else
+		str = kasprintf(GFP_KERNEL, "%s.%d", name, index);
+
+	return str;
+}
+
+static int
+chromeos_acpi_add_attr(struct chromeos_acpi_attribute_group *aag,
+		       union acpi_object *element, char *name,
+		       int count, int index)
+{
+	struct chromeos_acpi_attribute *info;
+	char buffer[24]; /* enough to store a u64 and null character */
+	int length;
+	int ret;
+
+	/* Files BINF.{0,1,4} are historical and no longer used. */
+	if (!strcmp(name, "BINF") && (index == 0 || index == 1 || index == 4))
+		return 0;
+
+	info = kzalloc(sizeof(*info), GFP_KERNEL);
+	if (!info)
+		return -ENOMEM;
+
+	info->name = chromeos_acpi_gen_file_name(name, count, index);
+	if (!info->name) {
+		ret = -ENOMEM;
+		goto free_attribute;
+	}
+	sysfs_bin_attr_init(&info->bin_attr);
+	info->bin_attr.attr.name = info->name;
+	info->bin_attr.attr.mode = 0444;
+
+	switch (element->type) {
+	case ACPI_TYPE_BUFFER:
+		length = element->buffer.length;
+		info->data = kmemdup(element->buffer.pointer,
+				     length, GFP_KERNEL);
+		break;
+	case ACPI_TYPE_INTEGER:
+		length = snprintf(buffer, sizeof(buffer), "%d",
+				  (int)element->integer.value);
+		info->data = kmemdup(buffer, length, GFP_KERNEL);
+		break;
+	case ACPI_TYPE_STRING:
+		length = element->string.length + 1;
+		info->data = kstrdup(element->string.pointer, GFP_KERNEL);
+		break;
+	default:
+		ret = -EINVAL;
+		goto free_attr_name;
+	}
+
+	if (!info->data) {
+		ret = -ENOMEM;
+		goto free_attr_name;
+	}
+
+	info->bin_attr.size = length;
+	info->bin_attr.read = chromeos_acpi_read_bin_attribute;
+	info->bin_attr.private = info;
+
+	INIT_LIST_HEAD(&info->list);
+
+	list_add(&info->list, &aag->attribs);
+	return 0;
+
+free_attr_name:
+	kfree(info->name);
+free_attribute:
+	kfree(info);
+	return ret;
+}
+
+static void
+chromeos_acpi_remove_attribs(struct chromeos_acpi_attribute_group *aag)
+{
+	struct chromeos_acpi_attribute *attr, *tmp_attr;
+
+	list_for_each_entry_safe(attr, tmp_attr, &aag->attribs, list) {
+		kfree(attr->name);
+		kfree(attr->data);
+		kfree(attr);
+	}
+}
+
+static int
+chromeos_acpi_add_attribs_to_group(struct chromeos_acpi_attribute_group *aag,
+				   unsigned int num_attrs)
+{
+	struct chromeos_acpi_attribute *attr;
+	int count = 0;
+
+	aag->group.bin_attrs = kcalloc(num_attrs + 1,
+				       sizeof(*aag->group.bin_attrs),
+				       GFP_KERNEL);
+	if (!aag->group.bin_attrs)
+		return -ENOMEM;
+
+	list_for_each_entry(attr, &aag->attribs, list) {
+		aag->group.bin_attrs[count] = &attr->bin_attr;
+		count++;
+	}
+
+	chromeos_acpi.num_groups++;
+	list_add(&aag->list, &chromeos_acpi.groups);
+
+	return 0;
+}
+
+/**
+ * chromeos_acpi_add_group() - Create a sysfs group including attributes
+ *			       representing a nested ACPI package
+ *
+ * @obj: Package contents as returned by ACPI
+ * @name: Name of the group
+ * @num_attrs: Number of attributes of this package
+ * @index: Index number of this particular group
+ *
+ * The created group is called @name in case there is a single instance, or
+ * @name.@index otherwise.
+ *
+ * All group and attribute storage allocations are included in the lists for
+ * tracking of allocated memory.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+static int chromeos_acpi_add_group(union acpi_object *obj, char *name,
+				   int num_attrs, int index)
+{
+	struct chromeos_acpi_attribute_group *aag;
+	union acpi_object *element;
+	int i, count, ret;
+
+	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
+	if (!aag)
+		return -ENOMEM;
+	aag->name = chromeos_acpi_gen_file_name(name, num_attrs, index);
+	if (!aag->name) {
+		ret = -ENOMEM;
+		goto free_group;
+	}
+
+	INIT_LIST_HEAD(&aag->attribs);
+	INIT_LIST_HEAD(&aag->list);
+
+	count = obj->package.count;
+	element = obj->package.elements;
+	for (i = 0; i < count; i++, element++) {
+		ret = chromeos_acpi_add_attr(aag, element, name, count, i);
+		if (ret)
+			goto free_group_attr;
+	}
+
+	aag->group.name = aag->name;
+
+	ret = chromeos_acpi_add_attribs_to_group(aag, count);
+	if (ret)
+		goto free_group_attr;
+
+	return 0;
+
+free_group_attr:
+	chromeos_acpi_remove_attribs(aag);
+	kfree(aag->name);
+free_group:
+	kfree(aag);
+	return ret;
+}
+
+static void chromeos_acpi_remove_groups(void)
+{
+	struct chromeos_acpi_attribute_group *aag, *tmp_aag;
+
+	list_for_each_entry_safe(aag, tmp_aag, &chromeos_acpi.groups, list) {
+		chromeos_acpi_remove_attribs(aag);
+		kfree(aag->group.bin_attrs);
+		kfree(aag->name);
+		kfree(aag);
+	}
+}
+
+/**
+ * chromeos_acpi_handle_package() - Create sysfs group including attributes
+ *				    representing an ACPI package
+ *
+ * @pdev: Platform device
+ * @obj: Package contents as returned by ACPI
+ * @name: Name of the group
+ *
+ * Scalar objects included in the package get sysfs attributes created for
+ * them. Nested packages are passed to a function creating a sysfs group per
+ * package.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+static int chromeos_acpi_handle_package(struct platform_device *pdev,
+					union acpi_object *obj, char *name)
+{
+	struct device *dev = &pdev->dev;
+	int count = obj->package.count;
+	union acpi_object *element;
+	int i, ret;
+
+	element = obj->package.elements;
+	for (i = 0; i < count; i++, element++) {
+		if (element->type == ACPI_TYPE_BUFFER ||
+		    element->type == ACPI_TYPE_STRING ||
+		    element->type == ACPI_TYPE_INTEGER) {
+			/* Create a single attribute in the root directory */
+			ret = chromeos_acpi_add_attr(chromeos_acpi.root,
+						     element, name,
+						     count, i);
+			if (ret) {
+				dev_err(dev, "error adding attributes (%d)\n",
+					ret);
+				return ret;
+			}
+			chromeos_acpi.num_attrs++;
+		} else if (element->type == ACPI_TYPE_PACKAGE) {
+			/* Create a group of attributes */
+			ret = chromeos_acpi_add_group(element, name, count, i);
+			if (ret) {
+				dev_err(dev, "error adding a group (%d)\n",
+					ret);
+				return ret;
+			}
+		} else {
+			if (ret) {
+				dev_err(dev, "error on element type (%d)\n",
+					ret);
+				return -EINVAL;
+			}
+		}
+	}
+
+	return 0;
+}
+
+/**
+ * chromeos_acpi_add_method() - Evaluate an ACPI method and create sysfs
+ *				attributes
+ *
+ * @pdev: Platform device
+ * @name: Name of the method to evaluate
+ *
+ * Return: 0 on success, non-zero on failure
+ */
+static int chromeos_acpi_add_method(struct platform_device *pdev, char *name)
+{
+	struct device *dev = &pdev->dev;
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_status status;
+	int ret = 0;
+
+	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, name, NULL, &output);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "failed to retrieve %s (%d)\n", name, status);
+		return status;
+	}
+
+	if (((union acpi_object *)output.pointer)->type == ACPI_TYPE_PACKAGE)
+		ret = chromeos_acpi_handle_package(pdev, output.pointer, name);
+
+	kfree(output.pointer);
+	return ret;
+}
+
+/**
+ * chromeos_acpi_process_mlst() - Evaluate the MLST method and add methods
+ *				  listed in the response
+ *
+ * @pdev: Platform device
+ *
+ * Returns: 0 if successful, non-zero if error.
+ */
+static int chromeos_acpi_process_mlst(struct platform_device *pdev)
+{
+	struct chromeos_acpi_attribute_group *aag;
+	char name[ACPI_NAMESEG_SIZE + 1];
+	union acpi_object *element, *obj;
+	struct device *dev = &pdev->dev;
+	struct acpi_buffer output = { ACPI_ALLOCATE_BUFFER, NULL };
+	acpi_status status;
+	int ret = 0;
+	int size;
+	int i;
+
+	status = acpi_evaluate_object(ACPI_COMPANION(&pdev->dev)->handle, MLST, NULL,
+				      &output);
+	if (ACPI_FAILURE(status))
+		return status;
+
+	obj = output.pointer;
+	if (obj->type != ACPI_TYPE_PACKAGE) {
+		ret = -EINVAL;
+		goto free_acpi_buffer;
+	}
+
+	element = obj->package.elements;
+	for (i = 0; i < obj->package.count; i++, element++) {
+		if (element->type == ACPI_TYPE_STRING) {
+			size = min(element->string.length + 1,
+				   (u32)ACPI_NAMESEG_SIZE + 1);
+			strscpy(name, element->string.pointer, size);
+			ret = chromeos_acpi_add_method(pdev, name);
+			if (ret) {
+				chromeos_acpi_remove_groups();
+				break;
+			}
+		}
+	}
+
+	/* Add root attributes to the main group */
+	ret = chromeos_acpi_add_attribs_to_group(chromeos_acpi.root,
+						 chromeos_acpi.num_attrs);
+	if (ret)
+		goto free_acpi_buffer;
+
+	chromeos_acpi.dev_groups = kcalloc(chromeos_acpi.num_groups + 1,
+					   sizeof(struct attribute_group),
+					   GFP_KERNEL);
+
+	i = 0;
+	list_for_each_entry(aag, &chromeos_acpi.groups, list) {
+		chromeos_acpi.dev_groups[i] = &aag->group;
+		i++;
+	}
+
+	ret = sysfs_create_groups(&dev->kobj, chromeos_acpi.dev_groups);
+	if (ret) {
+		kfree(chromeos_acpi.dev_groups);
+
+		/* Remove allocated chromeos acpi groups and attributes */
+		chromeos_acpi_remove_groups();
+	}
+
+free_acpi_buffer:
+	kfree(output.pointer);
+	return ret;
+}
+
+static int chromeos_acpi_device_probe(struct platform_device *pdev)
+{
+	struct chromeos_acpi_attribute_group *aag;
+	struct device *dev = &pdev->dev;
+	int i, ret;
+
+	aag = kzalloc(sizeof(*aag), GFP_KERNEL);
+	if (!aag)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&aag->attribs);
+	INIT_LIST_HEAD(&aag->list);
+	INIT_LIST_HEAD(&chromeos_acpi.groups);
+
+	chromeos_acpi.root = aag;
+
+	/*
+	 * Attempt to add methods by querying the device's MLST method
+	 * for the list of methods.
+	 */
+	if (!chromeos_acpi_process_mlst(pdev))
+		return 0;
+
+	dev_dbg(dev, "falling back to default list of methods\n");
+
+	for (i = 0; i < ARRAY_SIZE(chromeos_acpi_default_methods); i++) {
+		ret = chromeos_acpi_add_method(pdev,
+					       chromeos_acpi_default_methods[i]);
+		if (ret) {
+			dev_err(dev, "failed to add default methods (%d)\n",
+				ret);
+			goto free_group_root;
+		}
+	}
+
+	return 0;
+
+free_group_root:
+	kfree(chromeos_acpi.root);
+	return ret;
+}
+
+static int chromeos_acpi_device_remove(struct platform_device *pdev)
+{
+	/* Remove sysfs groups */
+	sysfs_remove_groups(&pdev->dev.kobj, chromeos_acpi.dev_groups);
+	kfree(chromeos_acpi.dev_groups);
+
+	/* Remove allocated chromeos acpi groups and attributes */
+	chromeos_acpi_remove_groups();
+
+	return 0;
+}
+
+/* GGL is valid PNP ID of Google. PNP ID can be used with the ACPI devices. */
+static const struct acpi_device_id chromeos_device_ids[] = {
+	{ "GGL0001", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, chromeos_device_ids);
+
+static struct platform_driver chromeos_acpi_device_driver = {
+	.probe = chromeos_acpi_device_probe,
+	.remove = chromeos_acpi_device_remove,
+	.driver = {
+		.name   = "chromeos-acpi",
+		.acpi_match_table = ACPI_PTR(chromeos_device_ids)
+	}
+};
+
+module_platform_driver(chromeos_acpi_device_driver);
+
+MODULE_AUTHOR("Enric Balletbo i Serra <[e-mail]>");
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("ChromeOS specific ACPI extensions");