Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1619137 > unrolled thread

[PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory Moritz Fischer <moritz.fischer@ettus.com> - 2017-04-08 00:10 +0200
    [PATCH 3/3] hwmon: cros-ec-hwmon: Add Chromium-EC HWMON driver Moritz Fischer <moritz.fischer@ettus.com> - 2017-04-08 00:10 +0200
    [PATCH 2/3] dt-bindings: hwmon: Add bindings for Google Chromium EC HWMON Moritz Fischer <moritz.fischer@ettus.com> - 2017-04-08 00:10 +0200
    Re: [PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory Guenter Roeck <linux@roeck-us.net> - 2017-04-10 01:10 +0200
      Re: [PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory Moritz Fischer <mdf@kernel.org> - 2017-04-10 02:50 +0200

#1619137 — [PATCH 1/3] mfd: cros-ec: Add functions to read mapped memory

From: Moritz Fischer <mdf@kernel.org>

The ChromeOS EC has mapped memory regions where things like temperature
sensors and fan speed are stored. Provide access to those from the
cros-ec mfd device.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 drivers/platform/chrome/cros_ec_proto.c | 55 +++++++++++++++++++++++++++++++++
 include/linux/mfd/cros_ec.h             | 39 +++++++++++++++++++++++
 2 files changed, 94 insertions(+)

diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index ed5dee7..28063de 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -494,3 +494,58 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
 		return get_keyboard_state_event(ec_dev);
 }
 EXPORT_SYMBOL(cros_ec_get_next_event);
+
+static int __cros_ec_read_mapped_mem(struct cros_ec_device *ec, uint8_t offset,
+				     void *buf, size_t size)
+{
+	int ret;
+	struct ec_params_read_memmap *params;
+	struct cros_ec_command *msg;
+
+	msg = kzalloc(sizeof(*msg) + max(sizeof(*params), size), GFP_KERNEL);
+	if (!msg)
+		return -ENOMEM;
+
+	msg->version = 0;
+	msg->command = EC_CMD_READ_MEMMAP;
+	msg->insize = size;
+	msg->outsize = sizeof(*params);
+
+	params = (struct ec_params_read_memmap *)msg->data;
+	params->offset = offset;
+	params->size = size;
+
+	ret = cros_ec_cmd_xfer(ec, msg);
+	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+		dev_warn(ec->dev, "cannot read mapped reg: %d/%d\n",
+			 ret, msg->result);
+		goto out_free;
+	}
+
+	memcpy(buf, msg->data, size);
+
+out_free:
+	kfree(msg);
+	return ret;
+}
+
+int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
+			      uint32_t *data)
+{
+	return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
+}
+EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem32);
+
+int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
+			      uint16_t *data)
+{
+	return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
+}
+EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem16);
+
+int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
+			     uint8_t *data)
+{
+	return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
+}
+EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem8);
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index b3d04de..c2de878 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -190,6 +190,45 @@ struct cros_ec_dev {
 };
 
 /**
+ * cros_ec_read_mapped_mem8 - Read mapped memory in the ChromeOS EC
+ *
+ * This can be called by drivers to access the mapped memory in the EC
+ *
+ * @ec_dev: Device to read from
+ * @offset: Offset to read
+ * @data: Return data
+ * @return: 0 if Ok, -ve on error
+ */
+int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
+			     uint8_t *data);
+
+/**
+ * cros_ec_read_mapped_mem16 - Read mapped memory in the ChromeOS EC
+ *
+ * This can be called by drivers to access the mapped memory in the EC
+ *
+ * @ec_dev: Device to read from
+ * @offset: Offset to read
+ * @data: Return data
+ * @return: 0 if Ok, -ve on error
+ */
+int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
+			      uint16_t *data);
+
+/**
+ * cros_ec_read_mapped_mem32 - Read mapped memory in the ChromeOS EC
+ *
+ * This can be called by drivers to access the mapped memory in the EC
+ *
+ * @ec_dev: Device to read from
+ * @offset: Offset to read
+ * @data: Return data
+ * @return: 0 if Ok, -ve on error
+ */
+int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
+			      uint32_t *data);
+
+/**
  * cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
  *
  * This can be called by drivers to handle a suspend event.
-- 
2.7.4

[toc] | [next] | [standalone]

#1619140 — [PATCH 3/3] hwmon: cros-ec-hwmon: Add Chromium-EC HWMON driver

From: Moritz Fischer <mdf@kernel.org>

This adds a hwmon driver for the Chromium EC's fans
and temperature sensors.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---

This one still needs some work, but I figured some early feedback might not hurt.
Specifically I was wondering if using the devm_hwmon_register_with_info() is
preferable to the devm_hwmon_register_with_groups().

The EC has a bunch of additional features such as setting thermal limits etc,
which I'd still like to add but I figured I'll get some feedback on what I got so far.

Thanks,

Moritz

---
 drivers/hwmon/Kconfig         |   8 ++
 drivers/hwmon/Makefile        |   1 +
 drivers/hwmon/cros-ec-hwmon.c | 244 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 253 insertions(+)
 create mode 100644 drivers/hwmon/cros-ec-hwmon.c

diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 0649d53f3..3b9155f 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1254,6 +1254,14 @@ config SENSORS_PCF8591
 	  These devices are hard to detect and rarely found on mainstream
 	  hardware.  If unsure, say N.
 
+config SENSORS_CROS_EC
+	tristate "ChromeOS EC hwmon"
+	depends on MFD_CROS_EC
+	help
+	  If you say yes here you get hwmon support that will expose the
+	  ChromeOS internal sensors for fanspeed and temperature to the
+	  Linux hwmon subsystem.
+
 source drivers/hwmon/pmbus/Kconfig
 
 config SENSORS_PWM_FAN
diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
index 5509edf..e59b5da 100644
--- a/drivers/hwmon/Makefile
+++ b/drivers/hwmon/Makefile
@@ -134,6 +134,7 @@ obj-$(CONFIG_SENSORS_PC87360)	+= pc87360.o
 obj-$(CONFIG_SENSORS_PC87427)	+= pc87427.o
 obj-$(CONFIG_SENSORS_PCF8591)	+= pcf8591.o
 obj-$(CONFIG_SENSORS_POWR1220)  += powr1220.o
+obj-$(CONFIG_SENSORS_CROS_EC)   += cros-ec-hwmon.o
 obj-$(CONFIG_SENSORS_PWM_FAN)	+= pwm-fan.o
 obj-$(CONFIG_SENSORS_S3C)	+= s3c-hwmon.o
 obj-$(CONFIG_SENSORS_SCH56XX_COMMON)+= sch56xx-common.o
diff --git a/drivers/hwmon/cros-ec-hwmon.c b/drivers/hwmon/cros-ec-hwmon.c
new file mode 100644
index 0000000..29d8b06
--- /dev/null
+++ b/drivers/hwmon/cros-ec-hwmon.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright (c) 2017, National Instruments Corp.
+ *
+ * Chromium EC Fan speed and temperature sensor driver
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/device.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/sysfs.h>
+#include <linux/spi/spi.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/of_platform.h>
+#include <linux/hwmon.h>
+#include <linux/hwmon-sysfs.h>
+#include <linux/bitops.h>
+#include <linux/mfd/cros_ec.h>
+
+struct cros_ec_hwmon_priv {
+	struct cros_ec_device *ec;
+	struct device *hwmon_dev;
+
+	struct attribute **attrs;
+
+	struct attribute_group attr_group;
+	const struct attribute_group *groups[2];
+};
+
+#define KELVIN_TO_MILLICELSIUS(x) (((x) - 273) * 1000)
+
+static int __cros_ec_hwmon_probe_fans(struct cros_ec_hwmon_priv *priv)
+{
+	int err, idx;
+	uint16_t data;
+
+	for (idx = 0; idx < EC_FAN_SPEED_ENTRIES; idx++) {
+		err = cros_ec_read_mapped_mem16(priv->ec,
+					       EC_MEMMAP_FAN + 2 * idx,
+					       &data);
+		if (err)
+			return err;
+
+		if (data == EC_FAN_SPEED_NOT_PRESENT)
+			break;
+	}
+
+	return idx;
+}
+
+static int __cros_ec_hwmon_probe_temps(struct cros_ec_hwmon_priv *priv)
+{
+	uint8_t data;
+	int err, idx;
+
+	err = cros_ec_read_mapped_mem8(priv->ec, EC_MEMMAP_THERMAL_VERSION,
+				       &data);
+
+	/* if we have a read error, or EC_MEMMAP_THERMAL_VERSION is not set,
+	 * most likely we don't have temperature sensors ...
+	 */
+	if (err || !data)
+		return 0;
+
+	for (idx = 0; idx < EC_TEMP_SENSOR_ENTRIES; idx++) {
+		err = cros_ec_read_mapped_mem8(priv->ec,
+					       EC_MEMMAP_TEMP_SENSOR + idx,
+					       &data);
+		if (err)
+			return idx;
+
+		/* this assumes that they're all good up to idx */
+		switch (data) {
+		case EC_TEMP_SENSOR_NOT_PRESENT:
+		case EC_TEMP_SENSOR_ERROR:
+		case EC_TEMP_SENSOR_NOT_POWERED:
+		case EC_TEMP_SENSOR_NOT_CALIBRATED:
+			return idx;
+		default:
+			continue;
+		};
+	}
+
+	return idx;
+}
+
+static ssize_t cros_ec_hwmon_read_fan_rpm(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	uint16_t data;
+	int err;
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
+
+	err = cros_ec_read_mapped_mem16(priv->ec,
+					EC_MEMMAP_FAN + 2 * sattr->index,
+					&data);
+	if (err)
+		return err;
+
+	return sprintf(buf, "%d\n", data);
+}
+
+static ssize_t cros_ec_hwmon_read_temp(struct device *dev,
+				  struct device_attribute *attr,
+				  char *buf)
+{
+	uint8_t data;
+	int err, tmp;
+
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	struct cros_ec_hwmon_priv *priv = dev_get_drvdata(dev);
+
+	err = cros_ec_read_mapped_mem8(priv->ec,
+				       EC_MEMMAP_TEMP_SENSOR + 1 * sattr->index,
+				       &data);
+	if (err)
+		return err;
+
+	switch (data) {
+	case EC_TEMP_SENSOR_NOT_PRESENT:
+	case EC_TEMP_SENSOR_ERROR:
+	case EC_TEMP_SENSOR_NOT_POWERED:
+	case EC_TEMP_SENSOR_NOT_CALIBRATED:
+		dev_info(priv->ec->dev, "Failure: result=%d\n", data);
+		return -EIO;
+	}
+
+	/* make sure we don't overflow when adding offset*/
+	tmp = data + EC_TEMP_SENSOR_OFFSET;
+
+	return sprintf(buf, "%d\n", KELVIN_TO_MILLICELSIUS(tmp));
+}
+
+static int cros_ec_hwmon_probe(struct platform_device *pdev)
+{
+	struct cros_ec_device *ec = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_hwmon_priv *ec_hwmon;
+	struct sensor_device_attribute *attr;
+	int num_fans, num_temps, i;
+
+	ec_hwmon = devm_kzalloc(&pdev->dev, sizeof(*ec_hwmon), GFP_KERNEL);
+	if (!ec_hwmon)
+		return -ENOMEM;
+	ec_hwmon->ec = ec;
+
+	num_fans = __cros_ec_hwmon_probe_fans(ec_hwmon);
+	if (num_fans < 0)
+		return num_fans;
+
+	num_temps = __cros_ec_hwmon_probe_temps(ec_hwmon);
+	if (num_fans < 0)
+		return num_temps;
+
+	ec_hwmon->attrs = devm_kzalloc(&pdev->dev,
+				       sizeof(*ec_hwmon->attrs) *
+				       (num_fans + num_temps + 1),
+				       GFP_KERNEL);
+	if (!ec_hwmon->attrs)
+		return -ENOMEM;
+
+	for (i = 0; i < num_fans; i++) {
+		attr = devm_kzalloc(&pdev->dev, sizeof(*attr), GFP_KERNEL);
+		if (!attr)
+			return -ENOMEM;
+		sysfs_attr_init(&attr->dev_attr.attr);
+		attr->dev_attr.attr.name = devm_kasprintf(&pdev->dev,
+							  GFP_KERNEL,
+							  "fan%d_input",
+							  i);
+		if (!attr->dev_attr.attr.name)
+			return -ENOMEM;
+
+		attr->dev_attr.show = cros_ec_hwmon_read_fan_rpm;
+		attr->dev_attr.attr.mode = S_IRUGO;
+		attr->index = i;
+		ec_hwmon->attrs[i] = &attr->dev_attr.attr;
+
+	}
+
+	for (i = 0; i < num_temps; i++) {
+		attr = devm_kzalloc(&pdev->dev, sizeof(*attr), GFP_KERNEL);
+		if (!attr)
+			return -ENOMEM;
+		sysfs_attr_init(&attr->dev_attr.attr);
+		attr->dev_attr.attr.name = devm_kasprintf(&pdev->dev,
+							  GFP_KERNEL,
+							  "temp%d_input",
+							  i);
+		if (!attr->dev_attr.attr.name)
+			return -ENOMEM;
+
+		attr->dev_attr.show = cros_ec_hwmon_read_temp;
+		attr->dev_attr.attr.mode = S_IRUGO;
+		attr->index = i;
+		ec_hwmon->attrs[i + num_fans] = &attr->dev_attr.attr;
+
+	}
+
+	ec_hwmon->attr_group.attrs = ec_hwmon->attrs;
+	ec_hwmon->groups[0] = &ec_hwmon->attr_group;
+
+	ec_hwmon->hwmon_dev = devm_hwmon_device_register_with_groups(&pdev->dev,
+		    "ec_hwmon", ec_hwmon, ec_hwmon->groups);
+
+	if (IS_ERR(ec_hwmon->hwmon_dev))
+		return PTR_ERR(ec_hwmon->hwmon_dev);
+
+	platform_set_drvdata(pdev, ec_hwmon);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id cros_ec_hwmon_of_match[] = {
+	{ .compatible = "google,cros-ec-hwmon" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, cros_ec_hwmon_of_match);
+#endif
+
+static struct platform_driver cros_ec_hwmon_driver = {
+	.probe = cros_ec_hwmon_probe,
+	.driver = {
+		.name = "cros-ec-hwmon",
+		.of_match_table = of_match_ptr(cros_ec_hwmon_of_match),
+	},
+};
+module_platform_driver(cros_ec_hwmon_driver);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ChromeOS EC Hardware Monitor driver");
+MODULE_ALIAS("platform:cros-ec-hwmon");
+MODULE_AUTHOR("Moritz Fischer <mdf@kernel.org>");
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1619142 — [PATCH 2/3] dt-bindings: hwmon: Add bindings for Google Chromium EC HWMON

From: Moritz Fischer <mdf@kernel.org>

Add bindings for the Chromium EC HWMON. The Chromium EC HWMON
allows monitoring of temperature sensors and fans attached to the
EC.

Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
 .../devicetree/bindings/hwmon/cros-ec-hwmon.txt    | 25 ++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt

diff --git a/Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt b/Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt
new file mode 100644
index 0000000..4c94869
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/cros-ec-hwmon.txt
@@ -0,0 +1,25 @@
+Chromium Embedded Controller EC temperature and fan control
+-----------------------------------------------------------
+
+Google's Chromium EC HWMON is a hwmon implemented byimplemented by the Chromium EC
+firmware attached to the Embedded Controller (EC) and controlled via a host-command
+interface.
+
+An EC HWMON node should be only found as a sub-node of the EC node (see
+Documentation/devicetree/bindings/mfd/cros-ec.txt).
+
+Required properties:
+- compatible: Must contain "google,cros-ec-hwmon"
+
+Example:
+	embedded-controller@1e {
+		reg = <0x1e>;
+		compatible = "google,cros-ec-i2c";
+		interrupts = <0 IRQ_TYPE_LEVEL_LOW>;
+		interrupt-parent = <&gpio0>;
+
+		hwmon {
+			compatible = "google,cros-ec-hwmon";
+		};
+};
+
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1619570

On 04/07/2017 03:00 PM, Moritz Fischer wrote:
> From: Moritz Fischer <mdf@kernel.org>
>
> The ChromeOS EC has mapped memory regions where things like temperature
> sensors and fan speed are stored. Provide access to those from the
> cros-ec mfd device.
>
> Signed-off-by: Moritz Fischer <mdf@kernel.org>

I'll have to consult with others at Google if this is a good idea.
Benson, can you comment ?

> ---
>  drivers/platform/chrome/cros_ec_proto.c | 55 +++++++++++++++++++++++++++++++++
>  include/linux/mfd/cros_ec.h             | 39 +++++++++++++++++++++++
>  2 files changed, 94 insertions(+)
>
> diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> index ed5dee7..28063de 100644
> --- a/drivers/platform/chrome/cros_ec_proto.c
> +++ b/drivers/platform/chrome/cros_ec_proto.c
> @@ -494,3 +494,58 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
>  		return get_keyboard_state_event(ec_dev);
>  }
>  EXPORT_SYMBOL(cros_ec_get_next_event);
> +
> +static int __cros_ec_read_mapped_mem(struct cros_ec_device *ec, uint8_t offset,
> +				     void *buf, size_t size)
> +{
> +	int ret;
> +	struct ec_params_read_memmap *params;
> +	struct cros_ec_command *msg;
> +
> +	msg = kzalloc(sizeof(*msg) + max(sizeof(*params), size), GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +

I don't think using kzalloc here makes much sense. It is well known
that size is <= 4, so using a local buffer should not be a problem.

> +	msg->version = 0;
> +	msg->command = EC_CMD_READ_MEMMAP;
> +	msg->insize = size;
> +	msg->outsize = sizeof(*params);
> +
> +	params = (struct ec_params_read_memmap *)msg->data;
> +	params->offset = offset;
> +	params->size = size;
> +
> +	ret = cros_ec_cmd_xfer(ec, msg);
> +	if (ret < 0 || msg->result != EC_RES_SUCCESS) {

cros_ec_cmd_xfer_status() was introduced to be able to avoid the second check.

> +		dev_warn(ec->dev, "cannot read mapped reg: %d/%d\n",
> +			 ret, msg->result);
> +		goto out_free;
> +	}
> +
> +	memcpy(buf, msg->data, size);
> +
> +out_free:
> +	kfree(msg);
> +	return ret;
> +}
> +
> +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> +			      uint32_t *data)
> +{
> +	return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem32);
> +
> +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> +			      uint16_t *data)
> +{
> +	return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem16);
> +

Either case, this assumes that EC endianness matches host endianness. I don't
think we can just assume that this is the case.

> +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> +			     uint8_t *data)
> +{
> +	return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> +}
> +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem8);
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index b3d04de..c2de878 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -190,6 +190,45 @@ struct cros_ec_dev {
>  };
>
>  /**
> + * cros_ec_read_mapped_mem8 - Read mapped memory in the ChromeOS EC
> + *
> + * This can be called by drivers to access the mapped memory in the EC
> + *
> + * @ec_dev: Device to read from
> + * @offset: Offset to read
> + * @data: Return data
> + * @return: 0 if Ok, -ve on error
> + */
> +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> +			     uint8_t *data);
> +
> +/**
> + * cros_ec_read_mapped_mem16 - Read mapped memory in the ChromeOS EC
> + *
> + * This can be called by drivers to access the mapped memory in the EC
> + *
> + * @ec_dev: Device to read from
> + * @offset: Offset to read
> + * @data: Return data
> + * @return: 0 if Ok, -ve on error
> + */
> +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> +			      uint16_t *data);
> +
> +/**
> + * cros_ec_read_mapped_mem32 - Read mapped memory in the ChromeOS EC
> + *
> + * This can be called by drivers to access the mapped memory in the EC
> + *
> + * @ec_dev: Device to read from
> + * @offset: Offset to read
> + * @data: Return data
> + * @return: 0 if Ok, -ve on error
> + */
> +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> +			      uint32_t *data);
> +
> +/**
>   * cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
>   *
>   * This can be called by drivers to handle a suspend event.
>

[toc] | [prev] | [next] | [standalone]

#1619581

On Sun, Apr 09, 2017 at 04:02:04PM -0700, Guenter Roeck wrote:
> On 04/07/2017 03:00 PM, Moritz Fischer wrote:
> > From: Moritz Fischer <mdf@kernel.org>
> >
> > The ChromeOS EC has mapped memory regions where things like temperature
> > sensors and fan speed are stored. Provide access to those from the
> > cros-ec mfd device.
> >
> > Signed-off-by: Moritz Fischer <mdf@kernel.org>
>
> I'll have to consult with others at Google if this is a good idea.
> Benson, can you comment ?

Well to my knowledge the only other way to get to it is the 'ectool'
from userland
via ioctl calls. The other option would be IIO ...
>
> > ---
> >  drivers/platform/chrome/cros_ec_proto.c | 55 +++++++++++++++++++++++++++++++++
> >  include/linux/mfd/cros_ec.h             | 39 +++++++++++++++++++++++
> >  2 files changed, 94 insertions(+)
> >
> > diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
> > index ed5dee7..28063de 100644
> > --- a/drivers/platform/chrome/cros_ec_proto.c
> > +++ b/drivers/platform/chrome/cros_ec_proto.c
> > @@ -494,3 +494,58 @@ int cros_ec_get_next_event(struct cros_ec_device *ec_dev)
> >   return get_keyboard_state_event(ec_dev);
> >  }
> >  EXPORT_SYMBOL(cros_ec_get_next_event);
> > +
> > +static int __cros_ec_read_mapped_mem(struct cros_ec_device *ec, uint8_t offset,
> > +     void *buf, size_t size)
> > +{
> > + int ret;
> > + struct ec_params_read_memmap *params;
> > + struct cros_ec_command *msg;
> > +
> > + msg = kzalloc(sizeof(*msg) + max(sizeof(*params), size), GFP_KERNEL);
> > + if (!msg)
> > + return -ENOMEM;
> > +
>
> I don't think using kzalloc here makes much sense. It is well known
> that size is <= 4, so using a local buffer should not be a problem.

Good point, that was basically copy & paste from other cros-ec code ;-)
I'll fix this.

>
> > + msg->version = 0;
> > + msg->command = EC_CMD_READ_MEMMAP;
> > + msg->insize = size;
> > + msg->outsize = sizeof(*params);
> > +
> > + params = (struct ec_params_read_memmap *)msg->data;
> > + params->offset = offset;
> > + params->size = size;
> > +
> > + ret = cros_ec_cmd_xfer(ec, msg);
> > + if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>
> cros_ec_cmd_xfer_status() was introduced to be able to avoid the second check.
>

Alright, cool. Will fix this.

> > + dev_warn(ec->dev, "cannot read mapped reg: %d/%d\n",
> > + ret, msg->result);
> > + goto out_free;
> > + }
> > +
> > + memcpy(buf, msg->data, size);
> > +
> > +out_free:
> > + kfree(msg);
> > + return ret;
> > +}
> > +
> > +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> > +      uint32_t *data)
> > +{
> > + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem32);
> > +
> > +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> > +      uint16_t *data)
> > +{
> > + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem16);
> > +
>
> Either case, this assumes that EC endianness matches host endianness. I don't
> think we can just assume that this is the case.

Huh, yeah. Will need to figure out how to detect the EC endianness in
that case.

>
> > +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> > +     uint8_t *data)
> > +{
> > + return __cros_ec_read_mapped_mem(ec, offset, data, sizeof(*data));
> > +}
> > +EXPORT_SYMBOL_GPL(cros_ec_read_mapped_mem8);
> > diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> > index b3d04de..c2de878 100644
> > --- a/include/linux/mfd/cros_ec.h
> > +++ b/include/linux/mfd/cros_ec.h
> > @@ -190,6 +190,45 @@ struct cros_ec_dev {
> >  };
> >
> >  /**
> > + * cros_ec_read_mapped_mem8 - Read mapped memory in the ChromeOS EC
> > + *
> > + * This can be called by drivers to access the mapped memory in the EC
> > + *
> > + * @ec_dev: Device to read from
> > + * @offset: Offset to read
> > + * @data: Return data
> > + * @return: 0 if Ok, -ve on error
> > + */
> > +int cros_ec_read_mapped_mem8(struct cros_ec_device *ec, const uint8_t offset,
> > +     uint8_t *data);
> > +
> > +/**
> > + * cros_ec_read_mapped_mem16 - Read mapped memory in the ChromeOS EC
> > + *
> > + * This can be called by drivers to access the mapped memory in the EC
> > + *
> > + * @ec_dev: Device to read from
> > + * @offset: Offset to read
> > + * @data: Return data
> > + * @return: 0 if Ok, -ve on error
> > + */
> > +int cros_ec_read_mapped_mem16(struct cros_ec_device *ec, const uint8_t offset,
> > +      uint16_t *data);
> > +
> > +/**
> > + * cros_ec_read_mapped_mem32 - Read mapped memory in the ChromeOS EC
> > + *
> > + * This can be called by drivers to access the mapped memory in the EC
> > + *
> > + * @ec_dev: Device to read from
> > + * @offset: Offset to read
> > + * @data: Return data
> > + * @return: 0 if Ok, -ve on error
> > + */
> > +int cros_ec_read_mapped_mem32(struct cros_ec_device *ec, const uint8_t offset,
> > +      uint32_t *data);
> > +
> > +/**
> >   * cros_ec_suspend - Handle a suspend operation for the ChromeOS EC device
> >   *
> >   * This can be called by drivers to handle a suspend event.
> >
>

Thanks for the feedback,

Moritz

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web