[PATCH v2 0/5] mfd: cros-ec: Some fixes and improvements.

[PATCH v2 0/5] mfd: cros-ec: Some fixes and improvements.

[Enric Balletbo i Serra]

Dear all,

This is another patch series to fix and improve some cros-ec mfd related
things.

 * 1/5 mfd: cros-ec: Fix host command buffer size

  This patch is a FIX, and I think that would be interesting see it merged
in this release cycle. This should go through the MFD tree and can be picked
independently of the other patches. Lee Jones I think this is for you.

 * 2/5 mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

  As pointed by Lee Jones in this thread [1] we should not use the MFD API
outside of MFD. For this reason the cros-ec-rtc did not get accepted yet.
The reality is that we are calling mfd_add_devices from cros-ec-dev driver
already, so this patch get rid off the MFD calls inside the chardev driver
and moves to cros-ec MFD. Also I think the chardev device should simply
implement the ioctl calls to access to it from userspace.

 The above patch involves MFD, IIO and platform chrome subsystems.

 * 3/5 mfd: cros_ec: Introduce RTC commands and events definitions
 * 4/5 rtc: cros-ec: add cros-ec-rtc driver
 * 5/5 mfd: cros_ec: add RTC as mfd subdevice

 These patches are the cros-ec RTC driver, 3 and 4 patches are already
acked by the subsystem maintainers involved and are equal to the last
version I send. Patch 5 registers the rtc cell inside the cros-ec MFD
intead of cros-ec-dev chardev driver.

 Note that these 3 patches depends on [2] to build. I recommend apply
these series on top of [3]

Changes since v1:
 - Removed patch 'iio: cros_ec_sensors: Fix return value to get raw and
   calibbias data' from series as was already picked.
 - Removed patch 'iio: cros_ec_sensors: Fix return value to get raw and
   calibbias data' from series as was already picked.
 - Patch 2/5: Acked-by: Jonathan Cameron <jic23@kernel.org>

[1] https://www.spinics.net/lists/kernel/msg2465099.html
[2] https://lkml.org/lkml/2017/3/17/319
[3] https://lkml.org/lkml/2017/3/17/321

Best regards,

Enric Balletbo i Serra (1):
  mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

Stephen Barber (3):
  mfd: cros_ec: Introduce RTC commands and events definitions.
  rtc: cros-ec: add cros-ec-rtc driver.
  mfd: cros_ec: add RTC as mfd subdevice

Vic Yang (1):
  mfd: cros-ec: Fix host command buffer size

 .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |   8 -
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |   8 +-
 drivers/iio/light/cros_ec_light_prox.c             |   8 -
 drivers/iio/pressure/cros_ec_baro.c                |   8 -
 drivers/mfd/cros_ec.c                              | 178 +++++++++
 drivers/platform/chrome/cros_ec_dev.c              | 161 --------
 drivers/rtc/Kconfig                                |  10 +
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-cros-ec.c                          | 412 +++++++++++++++++++++
 include/linux/mfd/cros_ec.h                        |   9 +-
 include/linux/mfd/cros_ec_commands.h               |   8 +
 11 files changed, 621 insertions(+), 190 deletions(-)
 create mode 100644 drivers/rtc/rtc-cros-ec.c

-- 
2.9.3

[PATCH v2 1/5] mfd: cros-ec: Fix host command buffer size

[Enric Balletbo i Serra]

From: Vic Yang <victoryang@google.com>

For SPI, we can get up to 32 additional bytes for response preamble.
The current overhead (2 bytes) may cause problems when we try to receive
a big response. Update it to 32 bytes.

Without this fix we could see a kernel BUG when we receive a big response
from the Chrome EC when is connected via SPI.

Signed-off-by: Vic Yang <victoryang@google.com>
Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

  This patch is a FIX, and I think that would be interesting see it merged
in this release cycle. This should go through the MFD tree and can be picked
independently of the other patches. Lee Jones I think this is for you.

Changes since v1:
 - None

 include/linux/mfd/cros_ec.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index b3e812f..3b16c90 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -35,10 +35,11 @@
  * Max bus-specific overhead incurred by request/responses.
  * I2C requires 1 additional byte for requests.
  * I2C requires 2 additional bytes for responses.
+ * SPI requires up to 32 additional bytes for responses.
  * */
 #define EC_PROTO_VERSION_UNKNOWN	0
 #define EC_MAX_REQUEST_OVERHEAD		1
-#define EC_MAX_RESPONSE_OVERHEAD	2
+#define EC_MAX_RESPONSE_OVERHEAD	32
 
 /*
  * Command interface between EC and AP, for LPC, I2C and SPI interfaces.
-- 
2.9.3

Re: [PATCH v2 1/5] mfd: cros-ec: Fix host command buffer size

[Lee Jones]

On Mon, 03 Apr 2017, Enric Balletbo i Serra wrote:

> From: Vic Yang <victoryang@google.com>
> 
> For SPI, we can get up to 32 additional bytes for response preamble.
> The current overhead (2 bytes) may cause problems when we try to receive
> a big response. Update it to 32 bytes.
> 
> Without this fix we could see a kernel BUG when we receive a big response
> from the Chrome EC when is connected via SPI.
> 
> Signed-off-by: Vic Yang <victoryang@google.com>
> Tested-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
>   This patch is a FIX, and I think that would be interesting see it merged
> in this release cycle. This should go through the MFD tree and can be picked
> independently of the other patches. Lee Jones I think this is for you.
> 
> Changes since v1:
>  - None
> 
>  include/linux/mfd/cros_ec.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

This patch is already in Mainline.

> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index b3e812f..3b16c90 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -35,10 +35,11 @@
>   * Max bus-specific overhead incurred by request/responses.
>   * I2C requires 1 additional byte for requests.
>   * I2C requires 2 additional bytes for responses.
> + * SPI requires up to 32 additional bytes for responses.
>   * */
>  #define EC_PROTO_VERSION_UNKNOWN	0
>  #define EC_MAX_REQUEST_OVERHEAD		1
> -#define EC_MAX_RESPONSE_OVERHEAD	2
> +#define EC_MAX_RESPONSE_OVERHEAD	32
>  
>  /*
>   * Command interface between EC and AP, for LPC, I2C and SPI interfaces.

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

[PATCH v2 2/5] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

[Enric Balletbo i Serra]

The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
Controller to user-space and should not be used to add MFD devices by
calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
driver and removes the MFD bits from the character device driver. Also
makes independent the IIO driver from the character device as also has no
sense.

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Jonathan Cameron <jic23@kernel.org>
---

  As pointed by Lee Jones in this thread [1] we should not use the MFD API
outside of MFD. For this reason the cros-ec-rtc did not get accepted yet.
The reality is that we are calling mfd_add_devices from cros-ec-dev driver
already, so this patch get rid off the MFD calls inside the chardev driver
and moves to cros-ec MFD. Also I think the chardev device should simply
implement the ioctl calls to access to it from userspace.

Changes since v1:
 - Acked-by: Jonathan Cameron <jic23@kernel.org>

[1]  https://www.spinics.net/lists/kernel/msg2465099.html

 .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |   8 -
 .../common/cros_ec_sensors/cros_ec_sensors_core.c  |   8 +-
 drivers/iio/light/cros_ec_light_prox.c             |   8 -
 drivers/iio/pressure/cros_ec_baro.c                |   8 -
 drivers/mfd/cros_ec.c                              | 160 ++++++++++++++++++++
 drivers/platform/chrome/cros_ec_dev.c              | 161 ---------------------
 include/linux/mfd/cros_ec.h                        |   6 +-
 7 files changed, 170 insertions(+), 189 deletions(-)

diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
index 38e8783..9b53a01 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -191,19 +191,11 @@ static const struct iio_info ec_sensors_info = {
 static int cros_ec_sensors_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
-	struct cros_ec_device *ec_device;
 	struct iio_dev *indio_dev;
 	struct cros_ec_sensors_state *state;
 	struct iio_chan_spec *channel;
 	int ret, i;
 
-	if (!ec_dev || !ec_dev->ec_dev) {
-		dev_warn(&pdev->dev, "No CROS EC device found.\n");
-		return -EINVAL;
-	}
-	ec_device = ec_dev->ec_dev;
-
 	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
 	if (!indio_dev)
 		return -ENOMEM;
diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
index 416cae5..0cdb64a 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
@@ -41,12 +41,13 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 {
 	struct device *dev = &pdev->dev;
 	struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
-	struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_device *ec_dev = dev_get_drvdata(pdev->dev.parent);
 	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
 
 	platform_set_drvdata(pdev, indio_dev);
 
-	state->ec = ec->ec_dev;
+	state->ec = ec_dev;
+
 	state->msg = devm_kzalloc(&pdev->dev,
 				max((u16)sizeof(struct ec_params_motion_sense),
 				state->ec->max_response), GFP_KERNEL);
@@ -59,7 +60,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
 
 	/* Set up the host command structure. */
 	state->msg->version = 2;
-	state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
+	state->msg->command = EC_CMD_MOTION_SENSE_CMD +
+				sensor_platform->cmd_offset;
 	state->msg->outsize = sizeof(struct ec_params_motion_sense);
 
 	indio_dev->dev.parent = &pdev->dev;
diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
index 7217223..2133ddc 100644
--- a/drivers/iio/light/cros_ec_light_prox.c
+++ b/drivers/iio/light/cros_ec_light_prox.c
@@ -181,19 +181,11 @@ static const struct iio_info cros_ec_light_prox_info = {
 static int cros_ec_light_prox_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
-	struct cros_ec_device *ec_device;
 	struct iio_dev *indio_dev;
 	struct cros_ec_light_prox_state *state;
 	struct iio_chan_spec *channel;
 	int ret;
 
-	if (!ec_dev || !ec_dev->ec_dev) {
-		dev_warn(dev, "No CROS EC device found.\n");
-		return -EINVAL;
-	}
-	ec_device = ec_dev->ec_dev;
-
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
 	if (!indio_dev)
 		return -ENOMEM;
diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
index 48b2a30..dbea18b 100644
--- a/drivers/iio/pressure/cros_ec_baro.c
+++ b/drivers/iio/pressure/cros_ec_baro.c
@@ -126,19 +126,11 @@ static const struct iio_info cros_ec_baro_info = {
 static int cros_ec_baro_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
-	struct cros_ec_device *ec_device;
 	struct iio_dev *indio_dev;
 	struct cros_ec_baro_state *state;
 	struct iio_chan_spec *channel;
 	int ret;
 
-	if (!ec_dev || !ec_dev->ec_dev) {
-		dev_warn(dev, "No CROS EC device found.\n");
-		return -EINVAL;
-	}
-	ec_device = ec_dev->ec_dev;
-
 	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
 	if (!indio_dev)
 		return -ENOMEM;
diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index d4a407e..bbc17ab 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -84,6 +84,160 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
 	return cros_ec_cmd_xfer(ec_dev, &buf.msg);
 }
 
+static int cros_ec_check_features(struct cros_ec_device *ec_dev, int feature)
+{
+	struct cros_ec_command *msg;
+	int ret;
+
+	if (ec_dev->features[0] == -1U && ec_dev->features[1] == -1U) {
+		/* features bitmap not read yet */
+
+		msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->features),
+			      GFP_KERNEL);
+		if (!msg)
+			return -ENOMEM;
+
+		msg->version = 0;
+		msg->command = EC_CMD_GET_FEATURES + ec_p.cmd_offset;
+		msg->insize = sizeof(ec_dev->features);
+		msg->outsize = 0;
+
+		ret = cros_ec_cmd_xfer(ec_dev, msg);
+		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+			dev_warn(ec_dev->dev, "cannot get EC features: %d/%d\n",
+				 ret, msg->result);
+			memset(ec_dev->features, 0, sizeof(ec_dev->features));
+		}
+
+		memcpy(ec_dev->features, msg->data, sizeof(ec_dev->features));
+
+		dev_dbg(ec_dev->dev, "EC features %08x %08x\n",
+			ec_dev->features[0], ec_dev->features[1]);
+
+		kfree(msg);
+	}
+
+	return ec_dev->features[feature / 32] & EC_FEATURE_MASK_0(feature);
+}
+
+static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
+{
+	/*
+	 * Issue a command to get the number of sensor reported.
+	 * Build an array of sensors driver and register them all.
+	 */
+	int ret, i, id, sensor_num;
+	struct mfd_cell *sensor_cells;
+	struct cros_ec_sensor_platform *sensor_platforms;
+	int sensor_type[MOTIONSENSE_TYPE_MAX];
+	struct ec_params_motion_sense *params;
+	struct ec_response_motion_sense *resp;
+	struct cros_ec_command *msg;
+
+	msg = kzalloc(sizeof(struct cros_ec_command) +
+		      max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
+	if (msg == NULL)
+		return;
+
+	msg->version = 2;
+	msg->command = EC_CMD_MOTION_SENSE_CMD + ec_p.cmd_offset;
+	msg->outsize = sizeof(*params);
+	msg->insize = sizeof(*resp);
+
+	params = (struct ec_params_motion_sense *)msg->data;
+	params->cmd = MOTIONSENSE_CMD_DUMP;
+
+	ret = cros_ec_cmd_xfer(ec_dev, msg);
+	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+		dev_warn(ec_dev->dev, "cannot get EC sensor information: %d/%d\n",
+			 ret, msg->result);
+		goto error;
+	}
+
+	resp = (struct ec_response_motion_sense *)msg->data;
+	sensor_num = resp->dump.sensor_count;
+	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
+	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
+			       GFP_KERNEL);
+	if (sensor_cells == NULL)
+		goto error;
+
+	sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
+		  (sensor_num + 1), GFP_KERNEL);
+	if (sensor_platforms == NULL)
+		goto error_platforms;
+
+	memset(sensor_type, 0, sizeof(sensor_type));
+	id = 0;
+	for (i = 0; i < sensor_num; i++) {
+		params->cmd = MOTIONSENSE_CMD_INFO;
+		params->info.sensor_num = i;
+		ret = cros_ec_cmd_xfer(ec_dev, msg);
+		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
+			dev_warn(ec_dev->dev, "no info for EC sensor %d : %d/%d\n",
+				 i, ret, msg->result);
+			continue;
+		}
+		switch (resp->info.type) {
+		case MOTIONSENSE_TYPE_ACCEL:
+			sensor_cells[id].name = "cros-ec-accel";
+			break;
+		case MOTIONSENSE_TYPE_BARO:
+			sensor_cells[id].name = "cros-ec-baro";
+			break;
+		case MOTIONSENSE_TYPE_GYRO:
+			sensor_cells[id].name = "cros-ec-gyro";
+			break;
+		case MOTIONSENSE_TYPE_MAG:
+			sensor_cells[id].name = "cros-ec-mag";
+			break;
+		case MOTIONSENSE_TYPE_PROX:
+			sensor_cells[id].name = "cros-ec-prox";
+			break;
+		case MOTIONSENSE_TYPE_LIGHT:
+			sensor_cells[id].name = "cros-ec-light";
+			break;
+		case MOTIONSENSE_TYPE_ACTIVITY:
+			sensor_cells[id].name = "cros-ec-activity";
+			break;
+		default:
+			dev_warn(ec_dev->dev, "unknown type %d\n",
+				 resp->info.type);
+			continue;
+		}
+		sensor_platforms[id].sensor_num = i;
+		sensor_platforms[id].cmd_offset = ec_p.cmd_offset;
+		sensor_cells[id].id = sensor_type[resp->info.type];
+		sensor_cells[id].platform_data = &sensor_platforms[id];
+		sensor_cells[id].pdata_size =
+			sizeof(struct cros_ec_sensor_platform);
+
+		sensor_type[resp->info.type]++;
+		id++;
+	}
+	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
+		sensor_platforms[id].sensor_num = sensor_num;
+
+		sensor_cells[id].name = "cros-ec-angle";
+		sensor_cells[id].id = 0;
+		sensor_cells[id].platform_data = &sensor_platforms[id];
+		sensor_cells[id].pdata_size =
+			sizeof(struct cros_ec_sensor_platform);
+		id++;
+	}
+
+	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, sensor_cells,
+			      id, NULL, 0, NULL);
+	if (ret)
+		dev_err(ec_dev->dev, "failed to add EC sensors\n");
+
+	kfree(sensor_platforms);
+error_platforms:
+	kfree(sensor_cells);
+error:
+	kfree(msg);
+}
+
 int cros_ec_register(struct cros_ec_device *ec_dev)
 {
 	struct device *dev = ec_dev->dev;
@@ -94,6 +248,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	ec_dev->max_request = sizeof(struct ec_params_hello);
 	ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
 	ec_dev->max_passthru = 0;
+	ec_dev->features[0] = -1U; /* Not cached yet */
+	ec_dev->features[1] = -1U; /* Not cached yet */
 
 	ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
 	if (!ec_dev->din)
@@ -127,6 +283,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 		goto fail_mfd;
 	}
 
+	/* Check whether this EC is a sensor hub. */
+	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
+		cros_ec_sensors_register(ec_dev);
+
 	if (ec_dev->max_passthru) {
 		/*
 		 * Register a PD device as well on top of this device.
diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
index b9bf086..4947650 100644
--- a/drivers/platform/chrome/cros_ec_dev.c
+++ b/drivers/platform/chrome/cros_ec_dev.c
@@ -90,41 +90,6 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
 	return ret;
 }
 
-static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
-{
-	struct cros_ec_command *msg;
-	int ret;
-
-	if (ec->features[0] == -1U && ec->features[1] == -1U) {
-		/* features bitmap not read yet */
-
-		msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
-		if (!msg)
-			return -ENOMEM;
-
-		msg->version = 0;
-		msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
-		msg->insize = sizeof(ec->features);
-		msg->outsize = 0;
-
-		ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
-			dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
-				 ret, msg->result);
-			memset(ec->features, 0, sizeof(ec->features));
-		}
-
-		memcpy(ec->features, msg->data, sizeof(ec->features));
-
-		dev_dbg(ec->dev, "EC features %08x %08x\n",
-			ec->features[0], ec->features[1]);
-
-		kfree(msg);
-	}
-
-	return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
-}
-
 /* Device file ops */
 static int ec_device_open(struct inode *inode, struct file *filp)
 {
@@ -268,126 +233,6 @@ static void __remove(struct device *dev)
 	kfree(ec);
 }
 
-static void cros_ec_sensors_register(struct cros_ec_dev *ec)
-{
-	/*
-	 * Issue a command to get the number of sensor reported.
-	 * Build an array of sensors driver and register them all.
-	 */
-	int ret, i, id, sensor_num;
-	struct mfd_cell *sensor_cells;
-	struct cros_ec_sensor_platform *sensor_platforms;
-	int sensor_type[MOTIONSENSE_TYPE_MAX];
-	struct ec_params_motion_sense *params;
-	struct ec_response_motion_sense *resp;
-	struct cros_ec_command *msg;
-
-	msg = kzalloc(sizeof(struct cros_ec_command) +
-		      max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
-	if (msg == NULL)
-		return;
-
-	msg->version = 2;
-	msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
-	msg->outsize = sizeof(*params);
-	msg->insize = sizeof(*resp);
-
-	params = (struct ec_params_motion_sense *)msg->data;
-	params->cmd = MOTIONSENSE_CMD_DUMP;
-
-	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
-		dev_warn(ec->dev, "cannot get EC sensor information: %d/%d\n",
-			 ret, msg->result);
-		goto error;
-	}
-
-	resp = (struct ec_response_motion_sense *)msg->data;
-	sensor_num = resp->dump.sensor_count;
-	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
-	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
-			       GFP_KERNEL);
-	if (sensor_cells == NULL)
-		goto error;
-
-	sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
-		  (sensor_num + 1), GFP_KERNEL);
-	if (sensor_platforms == NULL)
-		goto error_platforms;
-
-	memset(sensor_type, 0, sizeof(sensor_type));
-	id = 0;
-	for (i = 0; i < sensor_num; i++) {
-		params->cmd = MOTIONSENSE_CMD_INFO;
-		params->info.sensor_num = i;
-		ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
-		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
-			dev_warn(ec->dev, "no info for EC sensor %d : %d/%d\n",
-				 i, ret, msg->result);
-			continue;
-		}
-		switch (resp->info.type) {
-		case MOTIONSENSE_TYPE_ACCEL:
-			sensor_cells[id].name = "cros-ec-accel";
-			break;
-		case MOTIONSENSE_TYPE_BARO:
-			sensor_cells[id].name = "cros-ec-baro";
-			break;
-		case MOTIONSENSE_TYPE_GYRO:
-			sensor_cells[id].name = "cros-ec-gyro";
-			break;
-		case MOTIONSENSE_TYPE_MAG:
-			sensor_cells[id].name = "cros-ec-mag";
-			break;
-		case MOTIONSENSE_TYPE_PROX:
-			sensor_cells[id].name = "cros-ec-prox";
-			break;
-		case MOTIONSENSE_TYPE_LIGHT:
-			sensor_cells[id].name = "cros-ec-light";
-			break;
-		case MOTIONSENSE_TYPE_ACTIVITY:
-			sensor_cells[id].name = "cros-ec-activity";
-			break;
-		default:
-			dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
-			continue;
-		}
-		sensor_platforms[id].sensor_num = i;
-		sensor_cells[id].id = sensor_type[resp->info.type];
-		sensor_cells[id].platform_data = &sensor_platforms[id];
-		sensor_cells[id].pdata_size =
-			sizeof(struct cros_ec_sensor_platform);
-
-		sensor_type[resp->info.type]++;
-		id++;
-	}
-	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
-		sensor_platforms[id].sensor_num = sensor_num;
-
-		sensor_cells[id].name = "cros-ec-angle";
-		sensor_cells[id].id = 0;
-		sensor_cells[id].platform_data = &sensor_platforms[id];
-		sensor_cells[id].pdata_size =
-			sizeof(struct cros_ec_sensor_platform);
-		id++;
-	}
-	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
-		sensor_cells[id].name = "cros-ec-ring";
-		id++;
-	}
-
-	ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
-			      NULL, 0, NULL);
-	if (ret)
-		dev_err(ec->dev, "failed to add EC sensors\n");
-
-	kfree(sensor_platforms);
-error_platforms:
-	kfree(sensor_cells);
-error:
-	kfree(msg);
-}
-
 static int ec_device_probe(struct platform_device *pdev)
 {
 	int retval = -ENOMEM;
@@ -402,8 +247,6 @@ static int ec_device_probe(struct platform_device *pdev)
 	ec->ec_dev = dev_get_drvdata(dev->parent);
 	ec->dev = dev;
 	ec->cmd_offset = ec_platform->cmd_offset;
-	ec->features[0] = -1U; /* Not cached yet */
-	ec->features[1] = -1U; /* Not cached yet */
 	device_initialize(&ec->class_dev);
 	cdev_init(&ec->cdev, &fops);
 
@@ -432,10 +275,6 @@ static int ec_device_probe(struct platform_device *pdev)
 	if (cros_ec_debugfs_init(ec))
 		dev_warn(dev, "failed to create debugfs directory\n");
 
-	/* check whether this EC is a sensor hub. */
-	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
-		cros_ec_sensors_register(ec);
-
 	/* Take control of the lightbar from the EC. */
 	lb_manual_suspend_ctrl(ec, 1);
 
diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
index 3b16c90..8f87999 100644
--- a/include/linux/mfd/cros_ec.h
+++ b/include/linux/mfd/cros_ec.h
@@ -115,6 +115,7 @@ struct cros_ec_command {
  * @event_notifier: interrupt event notifier for transport devices.
  * @event_data: raw payload transferred with the MKBP event.
  * @event_size: size in bytes of the event data.
+ * @features: stores the EC features.
  */
 struct cros_ec_device {
 
@@ -149,15 +150,19 @@ struct cros_ec_device {
 
 	struct ec_response_get_next_event event_data;
 	int event_size;
+	u32 features[2];
 };
 
 /**
  * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
  *
  * @sensor_num: Id of the sensor, as reported by the EC.
+ * @cmd_offset: offset to apply for each command. Set when
+ * registering a devicde behind another one.
  */
 struct cros_ec_sensor_platform {
 	u8 sensor_num;
+	u16 cmd_offset;
 };
 
 /* struct cros_ec_platform - ChromeOS EC platform information
@@ -191,7 +196,6 @@ struct cros_ec_dev {
 	struct device *dev;
 	struct cros_ec_debugfs *debug_info;
 	u16 cmd_offset;
-	u32 features[2];
 };
 
 /**
-- 
2.9.3

Re: [PATCH v2 2/5] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

[Lee Jones]

On Mon, 03 Apr 2017, Enric Balletbo i Serra wrote:

> The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
> Controller to user-space and should not be used to add MFD devices by
> calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
> driver and removes the MFD bits from the character device driver. Also
> makes independent the IIO driver from the character device as also has no
> sense.
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Acked-by: Jonathan Cameron <jic23@kernel.org>
> ---
> 
>   As pointed by Lee Jones in this thread [1] we should not use the MFD API
> outside of MFD. For this reason the cros-ec-rtc did not get accepted yet.
> The reality is that we are calling mfd_add_devices from cros-ec-dev driver
> already, so this patch get rid off the MFD calls inside the chardev driver
> and moves to cros-ec MFD. Also I think the chardev device should simply
> implement the ioctl calls to access to it from userspace.
> 
> Changes since v1:
>  - Acked-by: Jonathan Cameron <jic23@kernel.org>
> 
> [1]  https://www.spinics.net/lists/kernel/msg2465099.html
> 
>  .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |   8 -
>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  |   8 +-
>  drivers/iio/light/cros_ec_light_prox.c             |   8 -
>  drivers/iio/pressure/cros_ec_baro.c                |   8 -
>  drivers/mfd/cros_ec.c                              | 160 ++++++++++++++++++++
>  drivers/platform/chrome/cros_ec_dev.c              | 161 ---------------------

Did you create this patch with "-M"?

This also requires a drivers/platform Ack.

>  include/linux/mfd/cros_ec.h                        |   6 +-
>  7 files changed, 170 insertions(+), 189 deletions(-)
> 
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> index 38e8783..9b53a01 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -191,19 +191,11 @@ static const struct iio_info ec_sensors_info = {
>  static int cros_ec_sensors_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> -	struct cros_ec_device *ec_device;
>  	struct iio_dev *indio_dev;
>  	struct cros_ec_sensors_state *state;
>  	struct iio_chan_spec *channel;
>  	int ret, i;
>  
> -	if (!ec_dev || !ec_dev->ec_dev) {
> -		dev_warn(&pdev->dev, "No CROS EC device found.\n");
> -		return -EINVAL;
> -	}
> -	ec_device = ec_dev->ec_dev;
> -
>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
>  	if (!indio_dev)
>  		return -ENOMEM;
> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> index 416cae5..0cdb64a 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
> @@ -41,12 +41,13 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>  {
>  	struct device *dev = &pdev->dev;
>  	struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
> -	struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
> +	struct cros_ec_device *ec_dev = dev_get_drvdata(pdev->dev.parent);
>  	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
>  
>  	platform_set_drvdata(pdev, indio_dev);
>  
> -	state->ec = ec->ec_dev;
> +	state->ec = ec_dev;
> +
>  	state->msg = devm_kzalloc(&pdev->dev,
>  				max((u16)sizeof(struct ec_params_motion_sense),
>  				state->ec->max_response), GFP_KERNEL);
> @@ -59,7 +60,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>  
>  	/* Set up the host command structure. */
>  	state->msg->version = 2;
> -	state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> +	state->msg->command = EC_CMD_MOTION_SENSE_CMD +
> +				sensor_platform->cmd_offset;
>  	state->msg->outsize = sizeof(struct ec_params_motion_sense);
>  
>  	indio_dev->dev.parent = &pdev->dev;
> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
> index 7217223..2133ddc 100644
> --- a/drivers/iio/light/cros_ec_light_prox.c
> +++ b/drivers/iio/light/cros_ec_light_prox.c
> @@ -181,19 +181,11 @@ static const struct iio_info cros_ec_light_prox_info = {
>  static int cros_ec_light_prox_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> -	struct cros_ec_device *ec_device;
>  	struct iio_dev *indio_dev;
>  	struct cros_ec_light_prox_state *state;
>  	struct iio_chan_spec *channel;
>  	int ret;
>  
> -	if (!ec_dev || !ec_dev->ec_dev) {
> -		dev_warn(dev, "No CROS EC device found.\n");
> -		return -EINVAL;
> -	}
> -	ec_device = ec_dev->ec_dev;
> -
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>  	if (!indio_dev)
>  		return -ENOMEM;
> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
> index 48b2a30..dbea18b 100644
> --- a/drivers/iio/pressure/cros_ec_baro.c
> +++ b/drivers/iio/pressure/cros_ec_baro.c
> @@ -126,19 +126,11 @@ static const struct iio_info cros_ec_baro_info = {
>  static int cros_ec_baro_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
> -	struct cros_ec_device *ec_device;
>  	struct iio_dev *indio_dev;
>  	struct cros_ec_baro_state *state;
>  	struct iio_chan_spec *channel;
>  	int ret;
>  
> -	if (!ec_dev || !ec_dev->ec_dev) {
> -		dev_warn(dev, "No CROS EC device found.\n");
> -		return -EINVAL;
> -	}
> -	ec_device = ec_dev->ec_dev;
> -
>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>  	if (!indio_dev)
>  		return -ENOMEM;
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index d4a407e..bbc17ab 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -84,6 +84,160 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
>  	return cros_ec_cmd_xfer(ec_dev, &buf.msg);
>  }
>  
> +static int cros_ec_check_features(struct cros_ec_device *ec_dev, int feature)
> +{
> +	struct cros_ec_command *msg;
> +	int ret;
> +
> +	if (ec_dev->features[0] == -1U && ec_dev->features[1] == -1U) {
> +		/* features bitmap not read yet */
> +
> +		msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->features),
> +			      GFP_KERNEL);
> +		if (!msg)
> +			return -ENOMEM;
> +
> +		msg->version = 0;
> +		msg->command = EC_CMD_GET_FEATURES + ec_p.cmd_offset;
> +		msg->insize = sizeof(ec_dev->features);
> +		msg->outsize = 0;
> +
> +		ret = cros_ec_cmd_xfer(ec_dev, msg);
> +		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> +			dev_warn(ec_dev->dev, "cannot get EC features: %d/%d\n",
> +				 ret, msg->result);
> +			memset(ec_dev->features, 0, sizeof(ec_dev->features));
> +		}
> +
> +		memcpy(ec_dev->features, msg->data, sizeof(ec_dev->features));
> +
> +		dev_dbg(ec_dev->dev, "EC features %08x %08x\n",
> +			ec_dev->features[0], ec_dev->features[1]);
> +
> +		kfree(msg);
> +	}
> +
> +	return ec_dev->features[feature / 32] & EC_FEATURE_MASK_0(feature);
> +}
> +
> +static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
> +{
> +	/*
> +	 * Issue a command to get the number of sensor reported.
> +	 * Build an array of sensors driver and register them all.
> +	 */
> +	int ret, i, id, sensor_num;
> +	struct mfd_cell *sensor_cells;
> +	struct cros_ec_sensor_platform *sensor_platforms;
> +	int sensor_type[MOTIONSENSE_TYPE_MAX];
> +	struct ec_params_motion_sense *params;
> +	struct ec_response_motion_sense *resp;
> +	struct cros_ec_command *msg;
> +
> +	msg = kzalloc(sizeof(struct cros_ec_command) +
> +		      max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
> +	if (msg == NULL)
> +		return;
> +
> +	msg->version = 2;
> +	msg->command = EC_CMD_MOTION_SENSE_CMD + ec_p.cmd_offset;
> +	msg->outsize = sizeof(*params);
> +	msg->insize = sizeof(*resp);
> +
> +	params = (struct ec_params_motion_sense *)msg->data;
> +	params->cmd = MOTIONSENSE_CMD_DUMP;
> +
> +	ret = cros_ec_cmd_xfer(ec_dev, msg);
> +	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> +		dev_warn(ec_dev->dev, "cannot get EC sensor information: %d/%d\n",
> +			 ret, msg->result);
> +		goto error;
> +	}
> +
> +	resp = (struct ec_response_motion_sense *)msg->data;
> +	sensor_num = resp->dump.sensor_count;
> +	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
> +	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
> +			       GFP_KERNEL);
> +	if (sensor_cells == NULL)
> +		goto error;
> +
> +	sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
> +		  (sensor_num + 1), GFP_KERNEL);
> +	if (sensor_platforms == NULL)
> +		goto error_platforms;
> +
> +	memset(sensor_type, 0, sizeof(sensor_type));
> +	id = 0;
> +	for (i = 0; i < sensor_num; i++) {
> +		params->cmd = MOTIONSENSE_CMD_INFO;
> +		params->info.sensor_num = i;
> +		ret = cros_ec_cmd_xfer(ec_dev, msg);
> +		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> +			dev_warn(ec_dev->dev, "no info for EC sensor %d : %d/%d\n",
> +				 i, ret, msg->result);
> +			continue;
> +		}
> +		switch (resp->info.type) {
> +		case MOTIONSENSE_TYPE_ACCEL:
> +			sensor_cells[id].name = "cros-ec-accel";
> +			break;
> +		case MOTIONSENSE_TYPE_BARO:
> +			sensor_cells[id].name = "cros-ec-baro";
> +			break;
> +		case MOTIONSENSE_TYPE_GYRO:
> +			sensor_cells[id].name = "cros-ec-gyro";
> +			break;
> +		case MOTIONSENSE_TYPE_MAG:
> +			sensor_cells[id].name = "cros-ec-mag";
> +			break;
> +		case MOTIONSENSE_TYPE_PROX:
> +			sensor_cells[id].name = "cros-ec-prox";
> +			break;
> +		case MOTIONSENSE_TYPE_LIGHT:
> +			sensor_cells[id].name = "cros-ec-light";
> +			break;
> +		case MOTIONSENSE_TYPE_ACTIVITY:
> +			sensor_cells[id].name = "cros-ec-activity";
> +			break;
> +		default:
> +			dev_warn(ec_dev->dev, "unknown type %d\n",
> +				 resp->info.type);
> +			continue;
> +		}
> +		sensor_platforms[id].sensor_num = i;
> +		sensor_platforms[id].cmd_offset = ec_p.cmd_offset;
> +		sensor_cells[id].id = sensor_type[resp->info.type];
> +		sensor_cells[id].platform_data = &sensor_platforms[id];
> +		sensor_cells[id].pdata_size =
> +			sizeof(struct cros_ec_sensor_platform);
> +
> +		sensor_type[resp->info.type]++;
> +		id++;
> +	}
> +	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
> +		sensor_platforms[id].sensor_num = sensor_num;
> +
> +		sensor_cells[id].name = "cros-ec-angle";
> +		sensor_cells[id].id = 0;
> +		sensor_cells[id].platform_data = &sensor_platforms[id];
> +		sensor_cells[id].pdata_size =
> +			sizeof(struct cros_ec_sensor_platform);
> +		id++;
> +	}
> +
> +	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, sensor_cells,
> +			      id, NULL, 0, NULL);
> +	if (ret)
> +		dev_err(ec_dev->dev, "failed to add EC sensors\n");
> +
> +	kfree(sensor_platforms);
> +error_platforms:
> +	kfree(sensor_cells);
> +error:
> +	kfree(msg);
> +}
> +
>  int cros_ec_register(struct cros_ec_device *ec_dev)
>  {
>  	struct device *dev = ec_dev->dev;
> @@ -94,6 +248,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  	ec_dev->max_request = sizeof(struct ec_params_hello);
>  	ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
>  	ec_dev->max_passthru = 0;
> +	ec_dev->features[0] = -1U; /* Not cached yet */
> +	ec_dev->features[1] = -1U; /* Not cached yet */
>  
>  	ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>  	if (!ec_dev->din)
> @@ -127,6 +283,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  		goto fail_mfd;
>  	}
>  
> +	/* Check whether this EC is a sensor hub. */
> +	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
> +		cros_ec_sensors_register(ec_dev);
> +
>  	if (ec_dev->max_passthru) {
>  		/*
>  		 * Register a PD device as well on top of this device.
> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
> index b9bf086..4947650 100644
> --- a/drivers/platform/chrome/cros_ec_dev.c
> +++ b/drivers/platform/chrome/cros_ec_dev.c
> @@ -90,41 +90,6 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
>  	return ret;
>  }
>  
> -static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
> -{
> -	struct cros_ec_command *msg;
> -	int ret;
> -
> -	if (ec->features[0] == -1U && ec->features[1] == -1U) {
> -		/* features bitmap not read yet */
> -
> -		msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
> -		if (!msg)
> -			return -ENOMEM;
> -
> -		msg->version = 0;
> -		msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
> -		msg->insize = sizeof(ec->features);
> -		msg->outsize = 0;
> -
> -		ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> -		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> -			dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
> -				 ret, msg->result);
> -			memset(ec->features, 0, sizeof(ec->features));
> -		}
> -
> -		memcpy(ec->features, msg->data, sizeof(ec->features));
> -
> -		dev_dbg(ec->dev, "EC features %08x %08x\n",
> -			ec->features[0], ec->features[1]);
> -
> -		kfree(msg);
> -	}
> -
> -	return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
> -}
> -
>  /* Device file ops */
>  static int ec_device_open(struct inode *inode, struct file *filp)
>  {
> @@ -268,126 +233,6 @@ static void __remove(struct device *dev)
>  	kfree(ec);
>  }
>  
> -static void cros_ec_sensors_register(struct cros_ec_dev *ec)
> -{
> -	/*
> -	 * Issue a command to get the number of sensor reported.
> -	 * Build an array of sensors driver and register them all.
> -	 */
> -	int ret, i, id, sensor_num;
> -	struct mfd_cell *sensor_cells;
> -	struct cros_ec_sensor_platform *sensor_platforms;
> -	int sensor_type[MOTIONSENSE_TYPE_MAX];
> -	struct ec_params_motion_sense *params;
> -	struct ec_response_motion_sense *resp;
> -	struct cros_ec_command *msg;
> -
> -	msg = kzalloc(sizeof(struct cros_ec_command) +
> -		      max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
> -	if (msg == NULL)
> -		return;
> -
> -	msg->version = 2;
> -	msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
> -	msg->outsize = sizeof(*params);
> -	msg->insize = sizeof(*resp);
> -
> -	params = (struct ec_params_motion_sense *)msg->data;
> -	params->cmd = MOTIONSENSE_CMD_DUMP;
> -
> -	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> -	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> -		dev_warn(ec->dev, "cannot get EC sensor information: %d/%d\n",
> -			 ret, msg->result);
> -		goto error;
> -	}
> -
> -	resp = (struct ec_response_motion_sense *)msg->data;
> -	sensor_num = resp->dump.sensor_count;
> -	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
> -	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
> -			       GFP_KERNEL);
> -	if (sensor_cells == NULL)
> -		goto error;
> -
> -	sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
> -		  (sensor_num + 1), GFP_KERNEL);
> -	if (sensor_platforms == NULL)
> -		goto error_platforms;
> -
> -	memset(sensor_type, 0, sizeof(sensor_type));
> -	id = 0;
> -	for (i = 0; i < sensor_num; i++) {
> -		params->cmd = MOTIONSENSE_CMD_INFO;
> -		params->info.sensor_num = i;
> -		ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
> -		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
> -			dev_warn(ec->dev, "no info for EC sensor %d : %d/%d\n",
> -				 i, ret, msg->result);
> -			continue;
> -		}
> -		switch (resp->info.type) {
> -		case MOTIONSENSE_TYPE_ACCEL:
> -			sensor_cells[id].name = "cros-ec-accel";
> -			break;
> -		case MOTIONSENSE_TYPE_BARO:
> -			sensor_cells[id].name = "cros-ec-baro";
> -			break;
> -		case MOTIONSENSE_TYPE_GYRO:
> -			sensor_cells[id].name = "cros-ec-gyro";
> -			break;
> -		case MOTIONSENSE_TYPE_MAG:
> -			sensor_cells[id].name = "cros-ec-mag";
> -			break;
> -		case MOTIONSENSE_TYPE_PROX:
> -			sensor_cells[id].name = "cros-ec-prox";
> -			break;
> -		case MOTIONSENSE_TYPE_LIGHT:
> -			sensor_cells[id].name = "cros-ec-light";
> -			break;
> -		case MOTIONSENSE_TYPE_ACTIVITY:
> -			sensor_cells[id].name = "cros-ec-activity";
> -			break;
> -		default:
> -			dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
> -			continue;
> -		}
> -		sensor_platforms[id].sensor_num = i;
> -		sensor_cells[id].id = sensor_type[resp->info.type];
> -		sensor_cells[id].platform_data = &sensor_platforms[id];
> -		sensor_cells[id].pdata_size =
> -			sizeof(struct cros_ec_sensor_platform);
> -
> -		sensor_type[resp->info.type]++;
> -		id++;
> -	}
> -	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
> -		sensor_platforms[id].sensor_num = sensor_num;
> -
> -		sensor_cells[id].name = "cros-ec-angle";
> -		sensor_cells[id].id = 0;
> -		sensor_cells[id].platform_data = &sensor_platforms[id];
> -		sensor_cells[id].pdata_size =
> -			sizeof(struct cros_ec_sensor_platform);
> -		id++;
> -	}
> -	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
> -		sensor_cells[id].name = "cros-ec-ring";
> -		id++;
> -	}
> -
> -	ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
> -			      NULL, 0, NULL);
> -	if (ret)
> -		dev_err(ec->dev, "failed to add EC sensors\n");
> -
> -	kfree(sensor_platforms);
> -error_platforms:
> -	kfree(sensor_cells);
> -error:
> -	kfree(msg);
> -}
> -
>  static int ec_device_probe(struct platform_device *pdev)
>  {
>  	int retval = -ENOMEM;
> @@ -402,8 +247,6 @@ static int ec_device_probe(struct platform_device *pdev)
>  	ec->ec_dev = dev_get_drvdata(dev->parent);
>  	ec->dev = dev;
>  	ec->cmd_offset = ec_platform->cmd_offset;
> -	ec->features[0] = -1U; /* Not cached yet */
> -	ec->features[1] = -1U; /* Not cached yet */
>  	device_initialize(&ec->class_dev);
>  	cdev_init(&ec->cdev, &fops);
>  
> @@ -432,10 +275,6 @@ static int ec_device_probe(struct platform_device *pdev)
>  	if (cros_ec_debugfs_init(ec))
>  		dev_warn(dev, "failed to create debugfs directory\n");
>  
> -	/* check whether this EC is a sensor hub. */
> -	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
> -		cros_ec_sensors_register(ec);
> -
>  	/* Take control of the lightbar from the EC. */
>  	lb_manual_suspend_ctrl(ec, 1);
>  
> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
> index 3b16c90..8f87999 100644
> --- a/include/linux/mfd/cros_ec.h
> +++ b/include/linux/mfd/cros_ec.h
> @@ -115,6 +115,7 @@ struct cros_ec_command {
>   * @event_notifier: interrupt event notifier for transport devices.
>   * @event_data: raw payload transferred with the MKBP event.
>   * @event_size: size in bytes of the event data.
> + * @features: stores the EC features.
>   */
>  struct cros_ec_device {
>  
> @@ -149,15 +150,19 @@ struct cros_ec_device {
>  
>  	struct ec_response_get_next_event event_data;
>  	int event_size;
> +	u32 features[2];
>  };
>  
>  /**
>   * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
>   *
>   * @sensor_num: Id of the sensor, as reported by the EC.
> + * @cmd_offset: offset to apply for each command. Set when
> + * registering a devicde behind another one.
>   */
>  struct cros_ec_sensor_platform {
>  	u8 sensor_num;
> +	u16 cmd_offset;
>  };
>  
>  /* struct cros_ec_platform - ChromeOS EC platform information
> @@ -191,7 +196,6 @@ struct cros_ec_dev {
>  	struct device *dev;
>  	struct cros_ec_debugfs *debug_info;
>  	u16 cmd_offset;
> -	u32 features[2];
>  };
>  
>  /**

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

Re: [PATCH v2 2/5] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

[Enric Balletbo i Serra]

On 11/04/17 11:19, Lee Jones wrote:
> On Mon, 03 Apr 2017, Enric Balletbo i Serra wrote:
> 
>> The cros_ec_dev driver should be used only to expose the Chrome OS Embedded
>> Controller to user-space and should not be used to add MFD devices by
>> calling mfd_add_devices. This patch moves this logic to the MFD cros_ec
>> driver and removes the MFD bits from the character device driver. Also
>> makes independent the IIO driver from the character device as also has no
>> sense.
>>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> Acked-by: Jonathan Cameron <jic23@kernel.org>
>> ---
>>
>>   As pointed by Lee Jones in this thread [1] we should not use the MFD API
>> outside of MFD. For this reason the cros-ec-rtc did not get accepted yet.
>> The reality is that we are calling mfd_add_devices from cros-ec-dev driver
>> already, so this patch get rid off the MFD calls inside the chardev driver
>> and moves to cros-ec MFD. Also I think the chardev device should simply
>> implement the ioctl calls to access to it from userspace.
>>
>> Changes since v1:
>>  - Acked-by: Jonathan Cameron <jic23@kernel.org>
>>
>> [1]  https://www.spinics.net/lists/kernel/msg2465099.html
>>
>>  .../iio/common/cros_ec_sensors/cros_ec_sensors.c   |   8 -
>>  .../common/cros_ec_sensors/cros_ec_sensors_core.c  |   8 +-
>>  drivers/iio/light/cros_ec_light_prox.c             |   8 -
>>  drivers/iio/pressure/cros_ec_baro.c                |   8 -
>>  drivers/mfd/cros_ec.c                              | 160 ++++++++++++++++++++
>>  drivers/platform/chrome/cros_ec_dev.c              | 161 ---------------------
> 
> Did you create this patch with "-M"?
> 

Oops, no, will do in next version

> This also requires a drivers/platform Ack.
> 

Olof, Benson, could one of you take a look at this and ack if you find this it
is ok, please?

Thanks,
 Enric


>>  include/linux/mfd/cros_ec.h                        |   6 +-
>>  7 files changed, 170 insertions(+), 189 deletions(-)
>>
>> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
>> index 38e8783..9b53a01 100644
>> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
>> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
>> @@ -191,19 +191,11 @@ static const struct iio_info ec_sensors_info = {
>>  static int cros_ec_sensors_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> -	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>> -	struct cros_ec_device *ec_device;
>>  	struct iio_dev *indio_dev;
>>  	struct cros_ec_sensors_state *state;
>>  	struct iio_chan_spec *channel;
>>  	int ret, i;
>>  
>> -	if (!ec_dev || !ec_dev->ec_dev) {
>> -		dev_warn(&pdev->dev, "No CROS EC device found.\n");
>> -		return -EINVAL;
>> -	}
>> -	ec_device = ec_dev->ec_dev;
>> -
>>  	indio_dev = devm_iio_device_alloc(&pdev->dev, sizeof(*state));
>>  	if (!indio_dev)
>>  		return -ENOMEM;
>> diff --git a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> index 416cae5..0cdb64a 100644
>> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors_core.c
>> @@ -41,12 +41,13 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>>  {
>>  	struct device *dev = &pdev->dev;
>>  	struct cros_ec_sensors_core_state *state = iio_priv(indio_dev);
>> -	struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
>> +	struct cros_ec_device *ec_dev = dev_get_drvdata(pdev->dev.parent);
>>  	struct cros_ec_sensor_platform *sensor_platform = dev_get_platdata(dev);
>>  
>>  	platform_set_drvdata(pdev, indio_dev);
>>  
>> -	state->ec = ec->ec_dev;
>> +	state->ec = ec_dev;
>> +
>>  	state->msg = devm_kzalloc(&pdev->dev,
>>  				max((u16)sizeof(struct ec_params_motion_sense),
>>  				state->ec->max_response), GFP_KERNEL);
>> @@ -59,7 +60,8 @@ int cros_ec_sensors_core_init(struct platform_device *pdev,
>>  
>>  	/* Set up the host command structure. */
>>  	state->msg->version = 2;
>> -	state->msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
>> +	state->msg->command = EC_CMD_MOTION_SENSE_CMD +
>> +				sensor_platform->cmd_offset;
>>  	state->msg->outsize = sizeof(struct ec_params_motion_sense);
>>  
>>  	indio_dev->dev.parent = &pdev->dev;
>> diff --git a/drivers/iio/light/cros_ec_light_prox.c b/drivers/iio/light/cros_ec_light_prox.c
>> index 7217223..2133ddc 100644
>> --- a/drivers/iio/light/cros_ec_light_prox.c
>> +++ b/drivers/iio/light/cros_ec_light_prox.c
>> @@ -181,19 +181,11 @@ static const struct iio_info cros_ec_light_prox_info = {
>>  static int cros_ec_light_prox_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> -	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>> -	struct cros_ec_device *ec_device;
>>  	struct iio_dev *indio_dev;
>>  	struct cros_ec_light_prox_state *state;
>>  	struct iio_chan_spec *channel;
>>  	int ret;
>>  
>> -	if (!ec_dev || !ec_dev->ec_dev) {
>> -		dev_warn(dev, "No CROS EC device found.\n");
>> -		return -EINVAL;
>> -	}
>> -	ec_device = ec_dev->ec_dev;
>> -
>>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>>  	if (!indio_dev)
>>  		return -ENOMEM;
>> diff --git a/drivers/iio/pressure/cros_ec_baro.c b/drivers/iio/pressure/cros_ec_baro.c
>> index 48b2a30..dbea18b 100644
>> --- a/drivers/iio/pressure/cros_ec_baro.c
>> +++ b/drivers/iio/pressure/cros_ec_baro.c
>> @@ -126,19 +126,11 @@ static const struct iio_info cros_ec_baro_info = {
>>  static int cros_ec_baro_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> -	struct cros_ec_dev *ec_dev = dev_get_drvdata(dev->parent);
>> -	struct cros_ec_device *ec_device;
>>  	struct iio_dev *indio_dev;
>>  	struct cros_ec_baro_state *state;
>>  	struct iio_chan_spec *channel;
>>  	int ret;
>>  
>> -	if (!ec_dev || !ec_dev->ec_dev) {
>> -		dev_warn(dev, "No CROS EC device found.\n");
>> -		return -EINVAL;
>> -	}
>> -	ec_device = ec_dev->ec_dev;
>> -
>>  	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>>  	if (!indio_dev)
>>  		return -ENOMEM;
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index d4a407e..bbc17ab 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -84,6 +84,160 @@ static int cros_ec_sleep_event(struct cros_ec_device *ec_dev, u8 sleep_event)
>>  	return cros_ec_cmd_xfer(ec_dev, &buf.msg);
>>  }
>>  
>> +static int cros_ec_check_features(struct cros_ec_device *ec_dev, int feature)
>> +{
>> +	struct cros_ec_command *msg;
>> +	int ret;
>> +
>> +	if (ec_dev->features[0] == -1U && ec_dev->features[1] == -1U) {
>> +		/* features bitmap not read yet */
>> +
>> +		msg = kmalloc(sizeof(*msg) + sizeof(ec_dev->features),
>> +			      GFP_KERNEL);
>> +		if (!msg)
>> +			return -ENOMEM;
>> +
>> +		msg->version = 0;
>> +		msg->command = EC_CMD_GET_FEATURES + ec_p.cmd_offset;
>> +		msg->insize = sizeof(ec_dev->features);
>> +		msg->outsize = 0;
>> +
>> +		ret = cros_ec_cmd_xfer(ec_dev, msg);
>> +		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> +			dev_warn(ec_dev->dev, "cannot get EC features: %d/%d\n",
>> +				 ret, msg->result);
>> +			memset(ec_dev->features, 0, sizeof(ec_dev->features));
>> +		}
>> +
>> +		memcpy(ec_dev->features, msg->data, sizeof(ec_dev->features));
>> +
>> +		dev_dbg(ec_dev->dev, "EC features %08x %08x\n",
>> +			ec_dev->features[0], ec_dev->features[1]);
>> +
>> +		kfree(msg);
>> +	}
>> +
>> +	return ec_dev->features[feature / 32] & EC_FEATURE_MASK_0(feature);
>> +}
>> +
>> +static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
>> +{
>> +	/*
>> +	 * Issue a command to get the number of sensor reported.
>> +	 * Build an array of sensors driver and register them all.
>> +	 */
>> +	int ret, i, id, sensor_num;
>> +	struct mfd_cell *sensor_cells;
>> +	struct cros_ec_sensor_platform *sensor_platforms;
>> +	int sensor_type[MOTIONSENSE_TYPE_MAX];
>> +	struct ec_params_motion_sense *params;
>> +	struct ec_response_motion_sense *resp;
>> +	struct cros_ec_command *msg;
>> +
>> +	msg = kzalloc(sizeof(struct cros_ec_command) +
>> +		      max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
>> +	if (msg == NULL)
>> +		return;
>> +
>> +	msg->version = 2;
>> +	msg->command = EC_CMD_MOTION_SENSE_CMD + ec_p.cmd_offset;
>> +	msg->outsize = sizeof(*params);
>> +	msg->insize = sizeof(*resp);
>> +
>> +	params = (struct ec_params_motion_sense *)msg->data;
>> +	params->cmd = MOTIONSENSE_CMD_DUMP;
>> +
>> +	ret = cros_ec_cmd_xfer(ec_dev, msg);
>> +	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> +		dev_warn(ec_dev->dev, "cannot get EC sensor information: %d/%d\n",
>> +			 ret, msg->result);
>> +		goto error;
>> +	}
>> +
>> +	resp = (struct ec_response_motion_sense *)msg->data;
>> +	sensor_num = resp->dump.sensor_count;
>> +	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
>> +	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
>> +			       GFP_KERNEL);
>> +	if (sensor_cells == NULL)
>> +		goto error;
>> +
>> +	sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
>> +		  (sensor_num + 1), GFP_KERNEL);
>> +	if (sensor_platforms == NULL)
>> +		goto error_platforms;
>> +
>> +	memset(sensor_type, 0, sizeof(sensor_type));
>> +	id = 0;
>> +	for (i = 0; i < sensor_num; i++) {
>> +		params->cmd = MOTIONSENSE_CMD_INFO;
>> +		params->info.sensor_num = i;
>> +		ret = cros_ec_cmd_xfer(ec_dev, msg);
>> +		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> +			dev_warn(ec_dev->dev, "no info for EC sensor %d : %d/%d\n",
>> +				 i, ret, msg->result);
>> +			continue;
>> +		}
>> +		switch (resp->info.type) {
>> +		case MOTIONSENSE_TYPE_ACCEL:
>> +			sensor_cells[id].name = "cros-ec-accel";
>> +			break;
>> +		case MOTIONSENSE_TYPE_BARO:
>> +			sensor_cells[id].name = "cros-ec-baro";
>> +			break;
>> +		case MOTIONSENSE_TYPE_GYRO:
>> +			sensor_cells[id].name = "cros-ec-gyro";
>> +			break;
>> +		case MOTIONSENSE_TYPE_MAG:
>> +			sensor_cells[id].name = "cros-ec-mag";
>> +			break;
>> +		case MOTIONSENSE_TYPE_PROX:
>> +			sensor_cells[id].name = "cros-ec-prox";
>> +			break;
>> +		case MOTIONSENSE_TYPE_LIGHT:
>> +			sensor_cells[id].name = "cros-ec-light";
>> +			break;
>> +		case MOTIONSENSE_TYPE_ACTIVITY:
>> +			sensor_cells[id].name = "cros-ec-activity";
>> +			break;
>> +		default:
>> +			dev_warn(ec_dev->dev, "unknown type %d\n",
>> +				 resp->info.type);
>> +			continue;
>> +		}
>> +		sensor_platforms[id].sensor_num = i;
>> +		sensor_platforms[id].cmd_offset = ec_p.cmd_offset;
>> +		sensor_cells[id].id = sensor_type[resp->info.type];
>> +		sensor_cells[id].platform_data = &sensor_platforms[id];
>> +		sensor_cells[id].pdata_size =
>> +			sizeof(struct cros_ec_sensor_platform);
>> +
>> +		sensor_type[resp->info.type]++;
>> +		id++;
>> +	}
>> +	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
>> +		sensor_platforms[id].sensor_num = sensor_num;
>> +
>> +		sensor_cells[id].name = "cros-ec-angle";
>> +		sensor_cells[id].id = 0;
>> +		sensor_cells[id].platform_data = &sensor_platforms[id];
>> +		sensor_cells[id].pdata_size =
>> +			sizeof(struct cros_ec_sensor_platform);
>> +		id++;
>> +	}
>> +
>> +	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, sensor_cells,
>> +			      id, NULL, 0, NULL);
>> +	if (ret)
>> +		dev_err(ec_dev->dev, "failed to add EC sensors\n");
>> +
>> +	kfree(sensor_platforms);
>> +error_platforms:
>> +	kfree(sensor_cells);
>> +error:
>> +	kfree(msg);
>> +}
>> +
>>  int cros_ec_register(struct cros_ec_device *ec_dev)
>>  {
>>  	struct device *dev = ec_dev->dev;
>> @@ -94,6 +248,8 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>>  	ec_dev->max_request = sizeof(struct ec_params_hello);
>>  	ec_dev->max_response = sizeof(struct ec_response_get_protocol_info);
>>  	ec_dev->max_passthru = 0;
>> +	ec_dev->features[0] = -1U; /* Not cached yet */
>> +	ec_dev->features[1] = -1U; /* Not cached yet */
>>  
>>  	ec_dev->din = devm_kzalloc(dev, ec_dev->din_size, GFP_KERNEL);
>>  	if (!ec_dev->din)
>> @@ -127,6 +283,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>>  		goto fail_mfd;
>>  	}
>>  
>> +	/* Check whether this EC is a sensor hub. */
>> +	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
>> +		cros_ec_sensors_register(ec_dev);
>> +
>>  	if (ec_dev->max_passthru) {
>>  		/*
>>  		 * Register a PD device as well on top of this device.
>> diff --git a/drivers/platform/chrome/cros_ec_dev.c b/drivers/platform/chrome/cros_ec_dev.c
>> index b9bf086..4947650 100644
>> --- a/drivers/platform/chrome/cros_ec_dev.c
>> +++ b/drivers/platform/chrome/cros_ec_dev.c
>> @@ -90,41 +90,6 @@ static int ec_get_version(struct cros_ec_dev *ec, char *str, int maxlen)
>>  	return ret;
>>  }
>>  
>> -static int cros_ec_check_features(struct cros_ec_dev *ec, int feature)
>> -{
>> -	struct cros_ec_command *msg;
>> -	int ret;
>> -
>> -	if (ec->features[0] == -1U && ec->features[1] == -1U) {
>> -		/* features bitmap not read yet */
>> -
>> -		msg = kmalloc(sizeof(*msg) + sizeof(ec->features), GFP_KERNEL);
>> -		if (!msg)
>> -			return -ENOMEM;
>> -
>> -		msg->version = 0;
>> -		msg->command = EC_CMD_GET_FEATURES + ec->cmd_offset;
>> -		msg->insize = sizeof(ec->features);
>> -		msg->outsize = 0;
>> -
>> -		ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
>> -		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> -			dev_warn(ec->dev, "cannot get EC features: %d/%d\n",
>> -				 ret, msg->result);
>> -			memset(ec->features, 0, sizeof(ec->features));
>> -		}
>> -
>> -		memcpy(ec->features, msg->data, sizeof(ec->features));
>> -
>> -		dev_dbg(ec->dev, "EC features %08x %08x\n",
>> -			ec->features[0], ec->features[1]);
>> -
>> -		kfree(msg);
>> -	}
>> -
>> -	return ec->features[feature / 32] & EC_FEATURE_MASK_0(feature);
>> -}
>> -
>>  /* Device file ops */
>>  static int ec_device_open(struct inode *inode, struct file *filp)
>>  {
>> @@ -268,126 +233,6 @@ static void __remove(struct device *dev)
>>  	kfree(ec);
>>  }
>>  
>> -static void cros_ec_sensors_register(struct cros_ec_dev *ec)
>> -{
>> -	/*
>> -	 * Issue a command to get the number of sensor reported.
>> -	 * Build an array of sensors driver and register them all.
>> -	 */
>> -	int ret, i, id, sensor_num;
>> -	struct mfd_cell *sensor_cells;
>> -	struct cros_ec_sensor_platform *sensor_platforms;
>> -	int sensor_type[MOTIONSENSE_TYPE_MAX];
>> -	struct ec_params_motion_sense *params;
>> -	struct ec_response_motion_sense *resp;
>> -	struct cros_ec_command *msg;
>> -
>> -	msg = kzalloc(sizeof(struct cros_ec_command) +
>> -		      max(sizeof(*params), sizeof(*resp)), GFP_KERNEL);
>> -	if (msg == NULL)
>> -		return;
>> -
>> -	msg->version = 2;
>> -	msg->command = EC_CMD_MOTION_SENSE_CMD + ec->cmd_offset;
>> -	msg->outsize = sizeof(*params);
>> -	msg->insize = sizeof(*resp);
>> -
>> -	params = (struct ec_params_motion_sense *)msg->data;
>> -	params->cmd = MOTIONSENSE_CMD_DUMP;
>> -
>> -	ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
>> -	if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> -		dev_warn(ec->dev, "cannot get EC sensor information: %d/%d\n",
>> -			 ret, msg->result);
>> -		goto error;
>> -	}
>> -
>> -	resp = (struct ec_response_motion_sense *)msg->data;
>> -	sensor_num = resp->dump.sensor_count;
>> -	/* Allocate 2 extra sensors in case lid angle or FIFO are needed */
>> -	sensor_cells = kzalloc(sizeof(struct mfd_cell) * (sensor_num + 2),
>> -			       GFP_KERNEL);
>> -	if (sensor_cells == NULL)
>> -		goto error;
>> -
>> -	sensor_platforms = kzalloc(sizeof(struct cros_ec_sensor_platform) *
>> -		  (sensor_num + 1), GFP_KERNEL);
>> -	if (sensor_platforms == NULL)
>> -		goto error_platforms;
>> -
>> -	memset(sensor_type, 0, sizeof(sensor_type));
>> -	id = 0;
>> -	for (i = 0; i < sensor_num; i++) {
>> -		params->cmd = MOTIONSENSE_CMD_INFO;
>> -		params->info.sensor_num = i;
>> -		ret = cros_ec_cmd_xfer(ec->ec_dev, msg);
>> -		if (ret < 0 || msg->result != EC_RES_SUCCESS) {
>> -			dev_warn(ec->dev, "no info for EC sensor %d : %d/%d\n",
>> -				 i, ret, msg->result);
>> -			continue;
>> -		}
>> -		switch (resp->info.type) {
>> -		case MOTIONSENSE_TYPE_ACCEL:
>> -			sensor_cells[id].name = "cros-ec-accel";
>> -			break;
>> -		case MOTIONSENSE_TYPE_BARO:
>> -			sensor_cells[id].name = "cros-ec-baro";
>> -			break;
>> -		case MOTIONSENSE_TYPE_GYRO:
>> -			sensor_cells[id].name = "cros-ec-gyro";
>> -			break;
>> -		case MOTIONSENSE_TYPE_MAG:
>> -			sensor_cells[id].name = "cros-ec-mag";
>> -			break;
>> -		case MOTIONSENSE_TYPE_PROX:
>> -			sensor_cells[id].name = "cros-ec-prox";
>> -			break;
>> -		case MOTIONSENSE_TYPE_LIGHT:
>> -			sensor_cells[id].name = "cros-ec-light";
>> -			break;
>> -		case MOTIONSENSE_TYPE_ACTIVITY:
>> -			sensor_cells[id].name = "cros-ec-activity";
>> -			break;
>> -		default:
>> -			dev_warn(ec->dev, "unknown type %d\n", resp->info.type);
>> -			continue;
>> -		}
>> -		sensor_platforms[id].sensor_num = i;
>> -		sensor_cells[id].id = sensor_type[resp->info.type];
>> -		sensor_cells[id].platform_data = &sensor_platforms[id];
>> -		sensor_cells[id].pdata_size =
>> -			sizeof(struct cros_ec_sensor_platform);
>> -
>> -		sensor_type[resp->info.type]++;
>> -		id++;
>> -	}
>> -	if (sensor_type[MOTIONSENSE_TYPE_ACCEL] >= 2) {
>> -		sensor_platforms[id].sensor_num = sensor_num;
>> -
>> -		sensor_cells[id].name = "cros-ec-angle";
>> -		sensor_cells[id].id = 0;
>> -		sensor_cells[id].platform_data = &sensor_platforms[id];
>> -		sensor_cells[id].pdata_size =
>> -			sizeof(struct cros_ec_sensor_platform);
>> -		id++;
>> -	}
>> -	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE_FIFO)) {
>> -		sensor_cells[id].name = "cros-ec-ring";
>> -		id++;
>> -	}
>> -
>> -	ret = mfd_add_devices(ec->dev, 0, sensor_cells, id,
>> -			      NULL, 0, NULL);
>> -	if (ret)
>> -		dev_err(ec->dev, "failed to add EC sensors\n");
>> -
>> -	kfree(sensor_platforms);
>> -error_platforms:
>> -	kfree(sensor_cells);
>> -error:
>> -	kfree(msg);
>> -}
>> -
>>  static int ec_device_probe(struct platform_device *pdev)
>>  {
>>  	int retval = -ENOMEM;
>> @@ -402,8 +247,6 @@ static int ec_device_probe(struct platform_device *pdev)
>>  	ec->ec_dev = dev_get_drvdata(dev->parent);
>>  	ec->dev = dev;
>>  	ec->cmd_offset = ec_platform->cmd_offset;
>> -	ec->features[0] = -1U; /* Not cached yet */
>> -	ec->features[1] = -1U; /* Not cached yet */
>>  	device_initialize(&ec->class_dev);
>>  	cdev_init(&ec->cdev, &fops);
>>  
>> @@ -432,10 +275,6 @@ static int ec_device_probe(struct platform_device *pdev)
>>  	if (cros_ec_debugfs_init(ec))
>>  		dev_warn(dev, "failed to create debugfs directory\n");
>>  
>> -	/* check whether this EC is a sensor hub. */
>> -	if (cros_ec_check_features(ec, EC_FEATURE_MOTION_SENSE))
>> -		cros_ec_sensors_register(ec);
>> -
>>  	/* Take control of the lightbar from the EC. */
>>  	lb_manual_suspend_ctrl(ec, 1);
>>  
>> diff --git a/include/linux/mfd/cros_ec.h b/include/linux/mfd/cros_ec.h
>> index 3b16c90..8f87999 100644
>> --- a/include/linux/mfd/cros_ec.h
>> +++ b/include/linux/mfd/cros_ec.h
>> @@ -115,6 +115,7 @@ struct cros_ec_command {
>>   * @event_notifier: interrupt event notifier for transport devices.
>>   * @event_data: raw payload transferred with the MKBP event.
>>   * @event_size: size in bytes of the event data.
>> + * @features: stores the EC features.
>>   */
>>  struct cros_ec_device {
>>  
>> @@ -149,15 +150,19 @@ struct cros_ec_device {
>>  
>>  	struct ec_response_get_next_event event_data;
>>  	int event_size;
>> +	u32 features[2];
>>  };
>>  
>>  /**
>>   * struct cros_ec_sensor_platform - ChromeOS EC sensor platform information
>>   *
>>   * @sensor_num: Id of the sensor, as reported by the EC.
>> + * @cmd_offset: offset to apply for each command. Set when
>> + * registering a devicde behind another one.
>>   */
>>  struct cros_ec_sensor_platform {
>>  	u8 sensor_num;
>> +	u16 cmd_offset;
>>  };
>>  
>>  /* struct cros_ec_platform - ChromeOS EC platform information
>> @@ -191,7 +196,6 @@ struct cros_ec_dev {
>>  	struct device *dev;
>>  	struct cros_ec_debugfs *debug_info;
>>  	u16 cmd_offset;
>> -	u32 features[2];
>>  };
>>  
>>  /**
> 

[PATCH v2 3/5] mfd: cros_ec: Introduce RTC commands and events definitions.

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

The EC can function as a simple RT, this patch adds the RTC related
definitions needed by the rtc-cros-ec driver.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/mfd/cros_ec_commands.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/linux/mfd/cros_ec_commands.h b/include/linux/mfd/cros_ec_commands.h
index 84e1abb..852ce34 100644
--- a/include/linux/mfd/cros_ec_commands.h
+++ b/include/linux/mfd/cros_ec_commands.h
@@ -286,6 +286,9 @@ enum host_event_code {
 	/* Hang detect logic detected a hang and warm rebooted the AP */
 	EC_HOST_EVENT_HANG_REBOOT = 21,
 
+	/* EC RTC event occurred */
+	EC_HOST_EVENT_RTC = 26,
+
 	/*
 	 * The high bit of the event mask is not used as a host event code.  If
 	 * it reads back as set, then the entire event mask should be
@@ -794,6 +797,8 @@ enum ec_feature_code {
 	EC_FEATURE_USB_MUX = 23,
 	/* Motion Sensor code has an internal software FIFO */
 	EC_FEATURE_MOTION_SENSE_FIFO = 24,
+	/* EC has RTC feature that can be controlled by host commands */
+	EC_FEATURE_RTC = 27,
 };
 
 #define EC_FEATURE_MASK_0(event_code) (1UL << (event_code % 32))
@@ -1704,6 +1709,9 @@ struct ec_response_rtc {
 #define EC_CMD_RTC_SET_VALUE 0x46
 #define EC_CMD_RTC_SET_ALARM 0x47
 
+/* Pass as param to SET_ALARM to clear the current alarm */
+#define EC_RTC_ALARM_CLEAR 0
+
 /*****************************************************************************/
 /* Port80 log access */
 
-- 
2.9.3

[PATCH v2 4/5] rtc: cros-ec: add cros-ec-rtc driver.

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

On platforms with a Chrome OS EC, the EC can function as a simple RTC.
Add a basic driver with this functionality.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Acked-by: Alexandre Belloni <alexandre.belloni@free-electrons.com>
---

Changes since v1:
 - None

 drivers/rtc/Kconfig       |  10 ++
 drivers/rtc/Makefile      |   1 +
 drivers/rtc/rtc-cros-ec.c | 412 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 423 insertions(+)
 create mode 100644 drivers/rtc/rtc-cros-ec.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 050bec7..b2939b4 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1204,6 +1204,16 @@ config RTC_DRV_ZYNQMP
 	  If you say yes here you get support for the RTC controller found on
 	  Xilinx Zynq Ultrascale+ MPSoC.
 
+config RTC_DRV_CROS_EC
+	tristate "Chrome OS EC RTC driver"
+	depends on MFD_CROS_EC
+	help
+	  If you say yes here you will get support for the
+	  Chrome OS Embedded Controller's RTC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rtc-cros-ec.
+
 comment "on-CPU RTC drivers"
 
 config RTC_DRV_ASM9260
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 13857d2..8162983 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_RTC_DRV_BQ4802)	+= rtc-bq4802.o
 obj-$(CONFIG_RTC_DRV_CMOS)	+= rtc-cmos.o
 obj-$(CONFIG_RTC_DRV_COH901331)	+= rtc-coh901331.o
 obj-$(CONFIG_RTC_DRV_CPCAP)	+= rtc-cpcap.o
+obj-$(CONFIG_RTC_DRV_CROS_EC)	+= rtc-cros-ec.o
 obj-$(CONFIG_RTC_DRV_DA9052)	+= rtc-da9052.o
 obj-$(CONFIG_RTC_DRV_DA9055)	+= rtc-da9055.o
 obj-$(CONFIG_RTC_DRV_DA9063)	+= rtc-da9063.o
diff --git a/drivers/rtc/rtc-cros-ec.c b/drivers/rtc/rtc-cros-ec.c
new file mode 100644
index 0000000..a5c2512
--- /dev/null
+++ b/drivers/rtc/rtc-cros-ec.c
@@ -0,0 +1,412 @@
+/*
+ * RTC driver for Chrome OS Embedded Controller
+ *
+ * Copyright (c) 2017, Google, Inc
+ *
+ * Author: Stephen Barber <smbarber@chromium.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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/kernel.h>
+#include <linux/mfd/cros_ec.h>
+#include <linux/mfd/cros_ec_commands.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/slab.h>
+
+#define DRV_NAME	"cros-ec-rtc"
+
+/**
+ * struct cros_ec_rtc - Driver data for EC RTC
+ *
+ * @cros_ec: Pointer to EC device
+ * @rtc: Pointer to RTC device
+ * @notifier: Notifier info for responding to EC events
+ * @saved_alarm: Alarm to restore when interrupts are reenabled
+ */
+struct cros_ec_rtc {
+	struct cros_ec_device *cros_ec;
+	struct rtc_device *rtc;
+	struct notifier_block notifier;
+	u32 saved_alarm;
+};
+
+static int cros_ec_rtc_get(struct cros_ec_device *cros_ec, u32 command,
+			   u32 *response)
+{
+	int ret;
+	struct {
+		struct cros_ec_command msg;
+		struct ec_response_rtc data;
+	} __packed msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg.command = command;
+	msg.msg.insize = sizeof(msg.data);
+
+	ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+	if (ret < 0) {
+		dev_err(cros_ec->dev,
+			"error getting %s from EC: %d\n",
+			command == EC_CMD_RTC_GET_VALUE ? "time" : "alarm",
+			ret);
+		return ret;
+	}
+
+	*response = msg.data.time;
+
+	return 0;
+}
+
+static int cros_ec_rtc_set(struct cros_ec_device *cros_ec, u32 command,
+			   u32 param)
+{
+	int ret = 0;
+	struct {
+		struct cros_ec_command msg;
+		struct ec_response_rtc data;
+	} __packed msg;
+
+	memset(&msg, 0, sizeof(msg));
+	msg.msg.command = command;
+	msg.msg.outsize = sizeof(msg.data);
+	msg.data.time = param;
+
+	ret = cros_ec_cmd_xfer_status(cros_ec, &msg.msg);
+	if (ret < 0) {
+		dev_err(cros_ec->dev, "error setting %s on EC: %d\n",
+			command == EC_CMD_RTC_SET_VALUE ? "time" : "alarm",
+			ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Read the current time from the EC. */
+static int cros_ec_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 time;
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &time);
+	if (ret) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	rtc_time64_to_tm(time, tm);
+
+	return 0;
+}
+
+/* Set the current EC time. */
+static int cros_ec_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	time64_t time;
+
+	time = rtc_tm_to_time64(tm);
+	if (time < 0 || time > U32_MAX)
+		return -EINVAL;
+
+	ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_VALUE, (u32)time);
+	if (ret < 0) {
+		dev_err(dev, "error setting time: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+/* Read alarm time from RTC. */
+static int cros_ec_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 current_time, alarm_offset;
+
+	/*
+	 * The EC host command for getting the alarm is relative (i.e. 5
+	 * seconds from now) whereas rtc_wkalrm is absolute. Get the current
+	 * RTC time first so we can calculate the relative time.
+	 */
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM, &alarm_offset);
+	if (ret < 0) {
+		dev_err(dev, "error getting alarm: %d\n", ret);
+		return ret;
+	}
+
+	rtc_time64_to_tm(current_time + alarm_offset, &alrm->time);
+
+	return 0;
+}
+
+/* Set the EC's RTC alarm. */
+static int cros_ec_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	time64_t alarm_time;
+	u32 current_time, alarm_offset;
+
+	/*
+	 * The EC host command for setting the alarm is relative
+	 * (i.e. 5 seconds from now) whereas rtc_wkalrm is absolute.
+	 * Get the current RTC time first so we can calculate the
+	 * relative time.
+	 */
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	alarm_time = rtc_tm_to_time64(&alrm->time);
+
+	if (alarm_time < 0 || alarm_time > U32_MAX)
+		return -EINVAL;
+
+	if (!alrm->enabled) {
+		/*
+		 * If the alarm is being disabled, send an alarm
+		 * clear command.
+		 */
+		alarm_offset = EC_RTC_ALARM_CLEAR;
+		cros_ec_rtc->saved_alarm = (u32)alarm_time;
+	} else {
+		/* Don't set an alarm in the past. */
+		if ((u32)alarm_time < current_time)
+			alarm_offset = EC_RTC_ALARM_CLEAR;
+		else
+			alarm_offset = (u32)alarm_time - current_time;
+	}
+
+	ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM, alarm_offset);
+	if (ret < 0) {
+		dev_err(dev, "error setting alarm: %d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_alarm_irq_enable(struct device *dev,
+					unsigned int enabled)
+{
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(dev);
+	struct cros_ec_device *cros_ec = cros_ec_rtc->cros_ec;
+	int ret;
+	u32 current_time, alarm_offset, alarm_value;
+
+	ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_VALUE, &current_time);
+	if (ret < 0) {
+		dev_err(dev, "error getting time: %d\n", ret);
+		return ret;
+	}
+
+	if (enabled) {
+		/* Restore saved alarm if it's still in the future. */
+		if (cros_ec_rtc->saved_alarm < current_time)
+			alarm_offset = EC_RTC_ALARM_CLEAR;
+		else
+			alarm_offset = cros_ec_rtc->saved_alarm - current_time;
+
+		ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+				      alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error restoring alarm: %d\n", ret);
+			return ret;
+		}
+	} else {
+		/* Disable alarm, saving the old alarm value. */
+		ret = cros_ec_rtc_get(cros_ec, EC_CMD_RTC_GET_ALARM,
+				      &alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error saving alarm: %d\n", ret);
+			return ret;
+		}
+
+		alarm_value = current_time + alarm_offset;
+
+		/*
+		 * If the current EC alarm is already past, we don't want
+		 * to set an alarm when we go through the alarm irq enable
+		 * path.
+		 */
+		if (alarm_value < current_time)
+			cros_ec_rtc->saved_alarm = EC_RTC_ALARM_CLEAR;
+		else
+			cros_ec_rtc->saved_alarm = alarm_value;
+
+		alarm_offset = EC_RTC_ALARM_CLEAR;
+		ret = cros_ec_rtc_set(cros_ec, EC_CMD_RTC_SET_ALARM,
+				      alarm_offset);
+		if (ret < 0) {
+			dev_err(dev, "error disabling alarm: %d\n", ret);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_event(struct notifier_block *nb,
+			     unsigned long queued_during_suspend,
+			     void *_notify)
+{
+	struct cros_ec_rtc *cros_ec_rtc;
+	struct rtc_device *rtc;
+	struct cros_ec_device *cros_ec;
+	u32 host_event;
+
+	cros_ec_rtc = container_of(nb, struct cros_ec_rtc, notifier);
+	rtc = cros_ec_rtc->rtc;
+	cros_ec = cros_ec_rtc->cros_ec;
+
+	host_event = cros_ec_get_host_event(cros_ec);
+	if (host_event & EC_HOST_EVENT_MASK(EC_HOST_EVENT_RTC)) {
+		rtc_update_irq(rtc, 1, RTC_IRQF | RTC_AF);
+		return NOTIFY_OK;
+	} else {
+		return NOTIFY_DONE;
+	}
+}
+
+static const struct rtc_class_ops cros_ec_rtc_ops = {
+	.read_time = cros_ec_rtc_read_time,
+	.set_time = cros_ec_rtc_set_time,
+	.read_alarm = cros_ec_rtc_read_alarm,
+	.set_alarm = cros_ec_rtc_set_alarm,
+	.alarm_irq_enable = cros_ec_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_PM_SLEEP
+static int cros_ec_rtc_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+	return 0;
+}
+
+static int cros_ec_rtc_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct cros_ec_rtc *cros_ec_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(cros_ec_rtc->cros_ec->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(cros_ec_rtc_pm_ops, cros_ec_rtc_suspend,
+			 cros_ec_rtc_resume);
+
+static int cros_ec_rtc_probe(struct platform_device *pdev)
+{
+	struct cros_ec_device *cros_ec_dev = dev_get_drvdata(pdev->dev.parent);
+	struct cros_ec_rtc *cros_ec_rtc;
+	struct rtc_time tm;
+	int ret;
+
+	cros_ec_rtc = devm_kzalloc(&pdev->dev, sizeof(*cros_ec_rtc),
+				   GFP_KERNEL);
+	if (!cros_ec_rtc)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, cros_ec_rtc);
+	cros_ec_rtc->cros_ec = cros_ec_dev;
+
+	/* Get initial time */
+	ret = cros_ec_rtc_read_time(&pdev->dev, &tm);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to read RTC time\n");
+		return ret;
+	}
+
+	ret = device_init_wakeup(&pdev->dev, 1);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to initialize wakeup\n");
+		return ret;
+	}
+
+	cros_ec_rtc->rtc = devm_rtc_device_register(&pdev->dev, DRV_NAME,
+						    &cros_ec_rtc_ops,
+						    THIS_MODULE);
+	if (IS_ERR(cros_ec_rtc->rtc)) {
+		ret = PTR_ERR(cros_ec_rtc->rtc);
+		dev_err(&pdev->dev, "failed to register rtc device\n");
+		return ret;
+	}
+
+	/* Get RTC events from the EC. */
+	cros_ec_rtc->notifier.notifier_call = cros_ec_rtc_event;
+	ret = blocking_notifier_chain_register(&cros_ec_dev->event_notifier,
+					       &cros_ec_rtc->notifier);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static int cros_ec_rtc_remove(struct platform_device *pdev)
+{
+	struct cros_ec_rtc *cros_ec_rtc = platform_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = blocking_notifier_chain_unregister(
+				&cros_ec_rtc->cros_ec->event_notifier,
+				&cros_ec_rtc->notifier);
+	if (ret) {
+		dev_err(dev, "failed to unregister notifier\n");
+		return ret;
+	}
+
+	return 0;
+}
+
+static struct platform_driver cros_ec_rtc_driver = {
+	.probe = cros_ec_rtc_probe,
+	.remove = cros_ec_rtc_remove,
+	.driver = {
+		.name = DRV_NAME,
+		.pm = &cros_ec_rtc_pm_ops,
+	},
+};
+
+module_platform_driver(cros_ec_rtc_driver);
+
+MODULE_DESCRIPTION("RTC driver for Chrome OS ECs");
+MODULE_AUTHOR("Stephen Barber <smbarber@chromium.org>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
-- 
2.9.3

[PATCH v2 5/5] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

From: Stephen Barber <smbarber@chromium.org>

If the EC supports RTC host commands, expose an RTC device.

Signed-off-by: Stephen Barber <smbarber@chromium.org>
Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

Changes since v1:
 - Use PLATFORM_DEVID_AUTO to add the subdevice.

 drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
index bbc17ab..76874be 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
 	.pdata_size = sizeof(pd_p),
 };
 
+static const struct mfd_cell ec_rtc_cell = {
+	.name = "cros-ec-rtc",
+};
+
 static irqreturn_t ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
@@ -238,6 +242,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
 	kfree(msg);
 }
 
+static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
+{
+	int ret;
+
+	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
+			      1, NULL, 0, NULL);
+	if (ret)
+		dev_err(ec_dev->dev, "failed to add EC RTC\n");
+}
+
 int cros_ec_register(struct cros_ec_device *ec_dev)
 {
 	struct device *dev = ec_dev->dev;
@@ -287,6 +301,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
 	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
 		cros_ec_sensors_register(ec_dev);
 
+	/* Check whether this EC has RTC support */
+	if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
+		cros_ec_rtc_register(ec_dev);
+
 	if (ec_dev->max_passthru) {
 		/*
 		 * Register a PD device as well on top of this device.
-- 
2.9.3

Re: [PATCH v2 5/5] mfd: cros_ec: add RTC as mfd subdevice

[Lee Jones]

On Mon, 03 Apr 2017, Enric Balletbo i Serra wrote:

> From: Stephen Barber <smbarber@chromium.org>
> 
> If the EC supports RTC host commands, expose an RTC device.

This could be made nicer by checking RTC compatibility in the RTC
driver.  So register it regardless, then check if the device is
supported from within.  If it's not supported simply return -ENODEV
from probe.

> Signed-off-by: Stephen Barber <smbarber@chromium.org>
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> ---
> 
> Changes since v1:
>  - Use PLATFORM_DEVID_AUTO to add the subdevice.
> 
>  drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
> index bbc17ab..76874be 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
>  	.pdata_size = sizeof(pd_p),
>  };
>  
> +static const struct mfd_cell ec_rtc_cell = {
> +	.name = "cros-ec-rtc",
> +};
> +
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> @@ -238,6 +242,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
>  	kfree(msg);
>  }
>  
> +static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
> +{
> +	int ret;
> +
> +	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
> +			      1, NULL, 0, NULL);
> +	if (ret)
> +		dev_err(ec_dev->dev, "failed to add EC RTC\n");
> +}
> +
>  int cros_ec_register(struct cros_ec_device *ec_dev)
>  {
>  	struct device *dev = ec_dev->dev;
> @@ -287,6 +301,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>  	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
>  		cros_ec_sensors_register(ec_dev);
>  
> +	/* Check whether this EC has RTC support */
> +	if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
> +		cros_ec_rtc_register(ec_dev);
> +
>  	if (ec_dev->max_passthru) {
>  		/*
>  		 * Register a PD device as well on top of this device.

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

Re: [PATCH v2 5/5] mfd: cros_ec: add RTC as mfd subdevice

[Enric Balletbo i Serra]

Lee,

On 11/04/17 11:48, Lee Jones wrote:
> On Mon, 03 Apr 2017, Enric Balletbo i Serra wrote:
> 
>> From: Stephen Barber <smbarber@chromium.org>
>>
>> If the EC supports RTC host commands, expose an RTC device.
> 
> This could be made nicer by checking RTC compatibility in the RTC
> driver.  So register it regardless, then check if the device is
> supported from within.  If it's not supported simply return -ENODEV
> from probe.
> 

Currently rtc-cros-ec and the other subdevices doesn't know how to check the
cros-ec MFD features, to do this I need to export cros_ec_check_features to a
public header and the rtc and other subdrivers should call the check function.

Seems to me more reasonable that the MFD checks the possible cells that can have
and only register the ones that really has (the cros-ec MFD can have different
cells depending on the device). Call every probe for all possible cells seems to
me a unnecessary step. The MFD is able to know which cells has, so IMHO then the
subdrivers are more independent of the MFD.

It's just an opinion so I'll let you the final decision.

Cheers,
 Enric

>> Signed-off-by: Stephen Barber <smbarber@chromium.org>
>> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
>> ---
>>
>> Changes since v1:
>>  - Use PLATFORM_DEVID_AUTO to add the subdevice.
>>
>>  drivers/mfd/cros_ec.c | 18 ++++++++++++++++++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/drivers/mfd/cros_ec.c b/drivers/mfd/cros_ec.c
>> index bbc17ab..76874be 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -51,6 +51,10 @@ static const struct mfd_cell ec_pd_cell = {
>>  	.pdata_size = sizeof(pd_p),
>>  };
>>  
>> +static const struct mfd_cell ec_rtc_cell = {
>> +	.name = "cros-ec-rtc",
>> +};
>> +
>>  static irqreturn_t ec_irq_thread(int irq, void *data)
>>  {
>>  	struct cros_ec_device *ec_dev = data;
>> @@ -238,6 +242,16 @@ static void cros_ec_sensors_register(struct cros_ec_device *ec_dev)
>>  	kfree(msg);
>>  }
>>  
>> +static void cros_ec_rtc_register(struct cros_ec_device *ec_dev)
>> +{
>> +	int ret;
>> +
>> +	ret = mfd_add_devices(ec_dev->dev, PLATFORM_DEVID_AUTO, &ec_rtc_cell,
>> +			      1, NULL, 0, NULL);
>> +	if (ret)
>> +		dev_err(ec_dev->dev, "failed to add EC RTC\n");
>> +}
>> +
>>  int cros_ec_register(struct cros_ec_device *ec_dev)
>>  {
>>  	struct device *dev = ec_dev->dev;
>> @@ -287,6 +301,10 @@ int cros_ec_register(struct cros_ec_device *ec_dev)
>>  	if (cros_ec_check_features(ec_dev, EC_FEATURE_MOTION_SENSE))
>>  		cros_ec_sensors_register(ec_dev);
>>  
>> +	/* Check whether this EC has RTC support */
>> +	if (cros_ec_check_features(ec_dev, EC_FEATURE_RTC))
>> +		cros_ec_rtc_register(ec_dev);
>> +
>>  	if (ec_dev->max_passthru) {
>>  		/*
>>  		 * Register a PD device as well on top of this device.
>