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

Groups > linux.kernel > #1608717 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/7] mfd: cros-ec: Some fixes and improvements. Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
    [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev. Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
      Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from  cros_ec_dev. Jonathan Cameron <jic23@kernel.org> - 2017-03-25 16:30 +0100
        Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from  cros_ec_dev. Jonathan Cameron <jic23@kernel.org> - 2017-03-25 16:30 +0100
        Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from  cros_ec_dev. Lee Jones <lee.jones@linaro.org> - 2017-03-27 14:50 +0200
          Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from  cros_ec_dev. Jonathan Cameron <jic23@kernel.org> - 2017-04-02 12:50 +0200
    [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
      Re: [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice Lee Jones <lee.jones@linaro.org> - 2017-04-03 16:30 +0200
        Re: [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-04-03 17:10 +0200
          Re: [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice Lee Jones <lee.jones@linaro.org> - 2017-04-04 10:40 +0200
    [PATCH 1/7] mfd: cros-ec: Fix host command buffer size Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
      Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size Lee Jones <lee.jones@linaro.org> - 2017-04-03 16:30 +0200
        Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size Lee Jones <lee.jones@linaro.org> - 2017-04-03 16:40 +0200
          Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-04-03 17:20 +0200
            Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size Lee Jones <lee.jones@linaro.org> - 2017-04-04 10:40 +0200
      Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size Lee Jones <lee.jones@linaro.org> - 2017-04-04 10:50 +0200
    [PATCH 6/7] rtc: cros-ec: add cros-ec-rtc driver. Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
    [PATCH 5/7] mfd: cros_ec: Introduce RTC commands and events definitions. Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
    [PATCH 2/7] iio: cros_ec_sensors: Fix return value to get raw and calibbias data. Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
      Re: [PATCH 2/7] iio: cros_ec_sensors: Fix return value to get raw and  calibbias data. Jonathan Cameron <jic23@kernel.org> - 2017-03-25 16:10 +0100
    [PATCH 3/7] iio: cros_ec_sensors: Use devm to setup the triggered buffer. Enric Balletbo i Serra <enric.balletbo@collabora.com> - 2017-03-24 18:50 +0100
      Re: [PATCH 3/7] iio: cros_ec_sensors: Use devm to setup the triggered  buffer. Jonathan Cameron <jic23@kernel.org> - 2017-03-25 16:20 +0100

Page 1 of 2  [1] 2  Next page →

#1608717 — [PATCH 0/7] mfd: cros-ec: Some fixes and improvements.

Dear all,

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

 * 1/7 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/7 iio: cros_ec_sensors: Fix return value to get raw and calibbias data.

  This is another FIX, so would be interesting see it merged in this release
cycle too. Like the above patch can be picked independently of the other
patches and should go through IIO Jonathan's Cameron tree.

 * 3/7 iio: cros_ec_sensors: Use devm to setup the triggered buffer.

  This is an improvement that can wait to 4.12 merge window, again can be
picked independently of the other patches and can go through IIO Jonathan's
Cameron tree.

 * 4/7 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.

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

 These patches are the cros-ec RTC driver, 5 and 6 patches are already
acked by the subsystem maintainers involved and are equal to the last
version I send. Patch 7 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]

[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 (3):
  iio: cros_ec_sensors: Fix return value to get raw and calibbias data.
  iio: cros_ec_sensors: Use devm to setup the triggered buffer.
  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   |  38 +-
 .../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, 626 insertions(+), 215 deletions(-)
 create mode 100644 drivers/rtc/rtc-cros-ec.c

-- 
2.9.3

[toc] | [next] | [standalone]

#1608720 — [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

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>
---

  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.

[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

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

#1609174 — Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

On 24/03/17 17:44, 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>
Few trivial bits inline, but more comments than anything that needs
changing.

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

Lee, so who gets to do the immutable branch for this one?

> ---
> 
>   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.
> 
> [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;
> -
Huh, not sure why this hasn't been throwing warnings as ec_device is
never used for anything.  Ah well, good to clean it up ;)

>  	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;
> +
If really going for the nitpick. Blank line doesn't really add anything...
>  	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)
> +{
Fair enough on the parameter rename. Slightly better this way...
Absolute ideal would have been a separate rename patch then this one, but
this is fine...
> +	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];
>  };
>  
>  /**
> 

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

#1609175 — Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

On 25/03/17 15:22, Jonathan Cameron wrote:
> On 24/03/17 17:44, 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>
> Few trivial bits inline, but more comments than anything that needs
> changing.
> 
> Acked-by: Jonathan Cameron <jic23@kernel.org>
> 
> Lee, so who gets to do the immutable branch for this one?
Given rest of the series, I'm voting for you :)
> 
>> ---
>>
>>   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.
>>
>> [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;
>> -
> Huh, not sure why this hasn't been throwing warnings as ec_device is
> never used for anything.  Ah well, good to clean it up ;)
> 
>>  	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;
>> +
> If really going for the nitpick. Blank line doesn't really add anything...
>>  	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)
>> +{
> Fair enough on the parameter rename. Slightly better this way...
> Absolute ideal would have been a separate rename patch then this one, but
> this is fine...
>> +	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];
>>  };
>>  
>>  /**
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

#1609796 — Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

On Sat, 25 Mar 2017, Jonathan Cameron wrote:

> On 24/03/17 17:44, 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>
> Few trivial bits inline, but more comments than anything that needs
> changing.
> 
> Acked-by: Jonathan Cameron <jic23@kernel.org>
> 
> Lee, so who gets to do the immutable branch for this one?

I have no strong opinion.

> > ---
> > 
> >   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.
> > 
> > [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;
> > -
> Huh, not sure why this hasn't been throwing warnings as ec_device is
> never used for anything.  Ah well, good to clean it up ;)
> 
> >  	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;
> > +
> If really going for the nitpick. Blank line doesn't really add anything...
> >  	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)
> > +{
> Fair enough on the parameter rename. Slightly better this way...
> Absolute ideal would have been a separate rename patch then this one, but
> this is fine...
> > +	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

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

#1614698 — Re: [PATCH 4/7] mfd: cros_ec: Get rid of cros_ec_check_features from cros_ec_dev.

On 27/03/17 13:42, Lee Jones wrote:
> On Sat, 25 Mar 2017, Jonathan Cameron wrote:
> 
>> On 24/03/17 17:44, 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>
>> Few trivial bits inline, but more comments than anything that needs
>> changing.
>>
>> Acked-by: Jonathan Cameron <jic23@kernel.org>
>>
>> Lee, so who gets to do the immutable branch for this one?
> 
> I have no strong opinion.
> 
Given the substantial rtc part I'd be stretching to take this through IIO.
Probably one for you given the very nature of MFD will mean it's not a
surprise to anyone.

Thanks,

Jonathan
>>> ---
>>>
>>>   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.
>>>
>>> [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;
>>> -
>> Huh, not sure why this hasn't been throwing warnings as ec_device is
>> never used for anything.  Ah well, good to clean it up ;)
>>
>>>  	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;
>>> +
>> If really going for the nitpick. Blank line doesn't really add anything...
>>>  	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)
>>> +{
>> Fair enough on the parameter rename. Slightly better this way...
>> Absolute ideal would have been a separate rename patch then this one, but
>> this is fine...
>>> +	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];
>>>  };
>>>  
>>>  /**
>>>
>>
> 

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

#1608721 — [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice

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>
---
 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..1ff88b1 100644
--- a/drivers/mfd/cros_ec.c
+++ b/drivers/mfd/cros_ec.c
@@ -51,6 +51,11 @@ 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",
+	.id   = -1,
+};
+
 static irqreturn_t ec_irq_thread(int irq, void *data)
 {
 	struct cros_ec_device *ec_dev = data;
@@ -238,6 +243,15 @@ 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, 0, &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

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

#1615245 — Re: [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice

On Fri, 24 Mar 2017, Enric Balletbo i Serra wrote:

> 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>
> ---
>  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..1ff88b1 100644
> --- a/drivers/mfd/cros_ec.c
> +++ b/drivers/mfd/cros_ec.c
> @@ -51,6 +51,11 @@ 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",
> +	.id   = -1,

Why -1?

> +};
> +
>  static irqreturn_t ec_irq_thread(int irq, void *data)
>  {
>  	struct cros_ec_device *ec_dev = data;
> @@ -238,6 +243,15 @@ 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, 0, &ec_rtc_cell, 1, NULL, 0, NULL);

Why 0?

> +	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

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

#1615308 — Re: [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice

On 03/04/17 16:25, Lee Jones wrote:
> On Fri, 24 Mar 2017, Enric Balletbo i Serra wrote:
> 
>> 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>
>> ---
>>  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..1ff88b1 100644
>> --- a/drivers/mfd/cros_ec.c
>> +++ b/drivers/mfd/cros_ec.c
>> @@ -51,6 +51,11 @@ 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",
>> +	.id   = -1,
> 
> Why -1?
> 
>> +};
>> +
>>  static irqreturn_t ec_irq_thread(int irq, void *data)
>>  {
>>  	struct cros_ec_device *ec_dev = data;
>> @@ -238,6 +243,15 @@ 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, 0, &ec_rtc_cell, 1, NULL, 0, NULL);
> 
> Why 0?
> 

Right, just maintained the one in the original. I think I can use
PLATFORM_DEVID_AUTO here?

>> +	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.
> 

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

#1615787 — Re: [PATCH 7/7] mfd: cros_ec: add RTC as mfd subdevice

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

> 
> 
> On 03/04/17 16:25, Lee Jones wrote:
> > On Fri, 24 Mar 2017, Enric Balletbo i Serra wrote:
> > 
> >> 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>
> >> ---
> >>  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..1ff88b1 100644
> >> --- a/drivers/mfd/cros_ec.c
> >> +++ b/drivers/mfd/cros_ec.c
> >> @@ -51,6 +51,11 @@ 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",
> >> +	.id   = -1,
> > 
> > Why -1?
> > 
> >> +};
> >> +
> >>  static irqreturn_t ec_irq_thread(int irq, void *data)
> >>  {
> >>  	struct cros_ec_device *ec_dev = data;
> >> @@ -238,6 +243,15 @@ 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, 0, &ec_rtc_cell, 1, NULL, 0, NULL);
> > 
> > Why 0?
> > 
> 
> Right, just maintained the one in the original. I think I can use
> PLATFORM_DEVID_AUTO here?

Yes, and don't set the .id to -1 in the cell.

> >> +	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

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

#1608722 — [PATCH 1/7] mfd: cros-ec: Fix host command buffer size

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.

 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

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

#1615246 — Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size

On Fri, 24 Mar 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.
> 
>  include/linux/mfd/cros_ec.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied, thanks.

> 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

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

#1615266 — Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size

On Mon, 03 Apr 2017, Lee Jones wrote:

> On Fri, 24 Mar 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.
> > 
> >  include/linux/mfd/cros_ec.h | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> Applied, thanks.

Scrap that, I'll keep it with the set.

> > 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

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

#1615318 — Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size

On 03/04/17 16:30, Lee Jones wrote:
> On Mon, 03 Apr 2017, Lee Jones wrote:
> 
>> On Fri, 24 Mar 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.
>>>
>>>  include/linux/mfd/cros_ec.h | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> Applied, thanks.
> 
> Scrap that, I'll keep it with the set.
> 

Ok, I'll send a v2 asap with this patch and the few modifications requested on
patch 7/7. I'm wondering though if this specific patch could go with current
release cycle as (explained above) is a fix and current kernel is affected.

Thanks,
 Enric

>>> 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.
>>
> 

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

#1615785 — Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size

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

> 
> 
> On 03/04/17 16:30, Lee Jones wrote:
> > On Mon, 03 Apr 2017, Lee Jones wrote:
> > 
> >> On Fri, 24 Mar 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.
> >>>
> >>>  include/linux/mfd/cros_ec.h | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>
> >> Applied, thanks.
> > 
> > Scrap that, I'll keep it with the set.
> > 
> 
> Ok, I'll send a v2 asap with this patch and the few modifications requested on
> patch 7/7. I'm wondering though if this specific patch could go with current
> release cycle as (explained above) is a fix and current kernel is affected.

Yes, but please send it property next time.

I.e. On its own and with either a "Fixes:" tag or Cc stable.

If you don't know what I'm talking about look it up in Documentation.

> >>> 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

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

#1615793 — Re: [PATCH 1/7] mfd: cros-ec: Fix host command buffer size

On Fri, 24 Mar 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.
> 
>  include/linux/mfd/cros_ec.h | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Applied to -fixes.

> 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

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

#1608723 — [PATCH 6/7] rtc: cros-ec: add cros-ec-rtc driver.

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>
---
 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

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

#1608725 — [PATCH 5/7] mfd: cros_ec: Introduce RTC commands and events definitions.

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

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

#1608728 — [PATCH 2/7] iio: cros_ec_sensors: Fix return value to get raw and calibbias data.

The cros_ec_sensors_read function must return the type of value on all
cases. This was always true except for RAW and CALIBBIAS data which
returned an error or 0. This patch just fixes the mistake I introduced
when submitting the series.

Fixes: commit c14dca07a31d (iio: cros_ec_sensors: add ChromeOS EC
Contiguous Sensors driver)

Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
---

  This is another FIX, so would be interesting see it merged in this release
cycle too. Like the above patch can be picked independently of the other
patches and should go through IIO Jonathan's Cameron tree.

 drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 4 ++--
 1 file changed, 2 insertions(+), 2 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 d6c372b..c17596f 100644
--- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
+++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
@@ -61,7 +61,7 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
 		ret = st->core.read_ec_sensors_data(indio_dev, 1 << idx, &data);
 		if (ret < 0)
 			break;
-
+		ret = IIO_VAL_INT;
 		*val = data;
 		break;
 	case IIO_CHAN_INFO_CALIBBIAS:
@@ -76,7 +76,7 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
 		for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
 			st->core.calib[i] =
 				st->core.resp->sensor_offset.offset[i];
-
+		ret = IIO_VAL_INT;
 		*val = st->core.calib[idx];
 		break;
 	case IIO_CHAN_INFO_SCALE:
-- 
2.9.3

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

#1609172 — Re: [PATCH 2/7] iio: cros_ec_sensors: Fix return value to get raw and calibbias data.

On 24/03/17 17:44, Enric Balletbo i Serra wrote:
> The cros_ec_sensors_read function must return the type of value on all
> cases. This was always true except for RAW and CALIBBIAS data which
> returned an error or 0. This patch just fixes the mistake I introduced
> when submitting the series.
> 
> Fixes: commit c14dca07a31d (iio: cros_ec_sensors: add ChromeOS EC
> Contiguous Sensors driver)
> 
> Signed-off-by: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Applied to the fixes-togreg branch of iio.git.

Thanks,

Jonathan
> ---
> 
>   This is another FIX, so would be interesting see it merged in this release
> cycle too. Like the above patch can be picked independently of the other
> patches and should go through IIO Jonathan's Cameron tree.
> 
>  drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 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 d6c372b..c17596f 100644
> --- a/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> +++ b/drivers/iio/common/cros_ec_sensors/cros_ec_sensors.c
> @@ -61,7 +61,7 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
>  		ret = st->core.read_ec_sensors_data(indio_dev, 1 << idx, &data);
>  		if (ret < 0)
>  			break;
> -
> +		ret = IIO_VAL_INT;
>  		*val = data;
>  		break;
>  	case IIO_CHAN_INFO_CALIBBIAS:
> @@ -76,7 +76,7 @@ static int cros_ec_sensors_read(struct iio_dev *indio_dev,
>  		for (i = CROS_EC_SENSOR_X; i < CROS_EC_SENSOR_MAX_AXIS; i++)
>  			st->core.calib[i] =
>  				st->core.resp->sensor_offset.offset[i];
> -
> +		ret = IIO_VAL_INT;
>  		*val = st->core.calib[idx];
>  		break;
>  	case IIO_CHAN_INFO_SCALE:
> 

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web