linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drivers: counter: Add Cros EC Sync counter
@ 2020-04-08  5:36 Gwendal Grignou
  2020-04-08  6:15 ` Gwendal Grignou
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Gwendal Grignou @ 2020-04-08  5:36 UTC (permalink / raw)
  To: bleung, vilhelm.gray, enric.balletbo, ic23, groeck
  Cc: linux-kernel, linux-iio, Gwendal Grignou

When the camera vsync pin is connected to the embedded controller (EC) of
a chromebook, the EC reports a sensor with a counter that increases
at each GPIO rising edge.

The sensor is presented using the counter subsystem.
In addition, it is also presented via the IIO subsystem with a timestamp,
allowing synchronisation with sensors connected to the same EC, for
image stabilisation or augmented reality applications.

To enable the counter:
via counter ABI:
echo "rising edge" > counterX/count0/signal_action
via iio ABI
echo 1 > iio:deviceY/en

To disable the counter:
via counter ABI:
echo "none" > counterX/count0/signal_action
via iio ABI
echo 0 > iio:deviceY/en

To read the current counter value:
via counter ABI:
cat counterX/count0/count
via iio ABI
cat iio:deviceY/in_count_raw
We can also read the value through the IIO buffer:
echo 1 > iio:deviceY/scan_elements/in_timestamp_en
echo 1 > iio:deviceY/scan_elements/in_count_en
echo 1 > iio:deviceY/buffer/enable

and each time to counter increase, the following binary blob
will be appended to dev/iio:deviceY:
000f 0000 0000 0000 dc66 816c 0071 0000
 \   <-- padding -> <-- timestamp ---->
  count

Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
---
An initial version of the driver was presented few month ago at
https://patchwork.kernel.org/patch/11037009/

 drivers/counter/Kconfig                       |  11 +
 drivers/counter/Makefile                      |   1 +
 drivers/counter/cros_ec_sync.c                | 342 ++++++++++++++++++
 .../cros_ec_sensors/cros_ec_sensors_core.c    |  15 +
 drivers/platform/chrome/cros_ec_sensorhub.c   |   3 +
 .../linux/iio/common/cros_ec_sensors_core.h   |   4 +-
 6 files changed, 375 insertions(+), 1 deletion(-)
 create mode 100644 drivers/counter/cros_ec_sync.c

diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
index c80fa76bb5311..18fde918ff40b 100644
--- a/drivers/counter/Kconfig
+++ b/drivers/counter/Kconfig
@@ -29,6 +29,17 @@ config 104_QUAD_8
 	  The base port addresses for the devices may be configured via the base
 	  array module parameter.
 
+config CROS_EC_SYNC
+	tristate "ChromeOS EC Counter driver"
+	depends on IIO_CROS_EC_SENSORS_CORE
+	help
+	  Module to handle synchronisation sensor presented by the ChromeOS EC
+	  Sensor hub.
+	  Synchronisation sensor sends event to the host when the camera
+	  take a picture. It allows synchronisation with other MEMS sensor,
+	  like gyroscope for image statbilization or augmented reality
+	  application (AR).
+
 config STM32_TIMER_CNT
 	tristate "STM32 Timer encoder counter driver"
 	depends on MFD_STM32_TIMERS || COMPILE_TEST
diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
index 55142d1f4c436..98378fca50ad6 100644
--- a/drivers/counter/Makefile
+++ b/drivers/counter/Makefile
@@ -6,6 +6,7 @@
 obj-$(CONFIG_COUNTER) += counter.o
 
 obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
+obj-$(CONFIG_CROS_EC_SYNC)	+= cros_ec_sync.o
 obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o
 obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o
 obj-$(CONFIG_TI_EQEP)		+= ti-eqep.o
diff --git a/drivers/counter/cros_ec_sync.c b/drivers/counter/cros_ec_sync.c
new file mode 100644
index 0000000000000..94b3ababab4bd
--- /dev/null
+++ b/drivers/counter/cros_ec_sync.c
@@ -0,0 +1,342 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for synchronisation sensor behind CrOS EC.
+ *
+ * Copyright 2020 Google, Inc
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * This driver uses the cros-ec interface to communicate with the Chrome OS
+ * EC about counter sensors. Counters are presented through
+ * iio sysfs.
+ */
+
+#include <linux/counter.h>
+#include <linux/device.h>
+#include <linux/iio/buffer.h>
+#include <linux/iio/common/cros_ec_sensors_core.h>
+#include <linux/iio/iio.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_data/cros_ec_commands.h>
+#include <linux/platform_data/cros_ec_proto.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/*
+ * One channel for counter, the other for timestamp.
+ */
+#define MAX_CHANNELS (1 + 1)
+
+/**
+ * struct cros_ec_sync_state - device structure
+ *
+ * @core: common structure for all cros_ec sensor.
+ *        Must be at the beggining.
+ * @channels: presented iio channels(2).
+ * @counter: counter data structure.
+ */
+struct cros_ec_sync_state {
+	struct cros_ec_sensors_core_state core;
+	struct iio_chan_spec channels[MAX_CHANNELS];
+	struct counter_device counter;
+};
+
+/**
+ * cros_ec_sync_get_enable() - Check if the counter is enabled.
+ *
+ * @st:     core cros_ec sensor
+ * @val:    status: 0: disabled, 1 enabled.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+static int cros_ec_sync_get_enable(struct cros_ec_sensors_core_state *st,
+				   int *val)
+{
+	int ret;
+
+	mutex_lock(&st->cmd_lock);
+	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
+	st->param.sensor_odr.data = EC_MOTION_SENSE_NO_VALUE;
+
+	ret = cros_ec_motion_send_host_cmd(st, 0);
+	mutex_unlock(&st->cmd_lock);
+	if (ret)
+		return ret;
+
+	*val = !!st->resp->sensor_odr.ret;
+	return 0;
+}
+
+/**
+ * cros_ec_sync_set_enable() - Allow the counter to count.
+ *
+ * When enable, the counter will increase for each VSYNC rising edge
+ * and will produce an event in the iio buffer, if enabled.
+ *
+ * @st:     core cros_ec sensor
+ * @val:    0: disable, 1 enable.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+static int cros_ec_sync_set_enable(struct cros_ec_sensors_core_state *st,
+				   int val)
+{
+	int ret;
+
+	mutex_lock(&st->cmd_lock);
+	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
+	st->param.sensor_odr.data = val;
+	st->param.sensor_odr.roundup = 1;
+
+	ret = cros_ec_motion_send_host_cmd(st, 0);
+	mutex_unlock(&st->cmd_lock);
+	return ret;
+}
+
+static int cros_ec_sync_iio_read(struct iio_dev *indio_dev,
+				 struct iio_chan_spec const *chan,
+				 int *val, int *val2, long mask)
+{
+	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
+	u16 data;
+	int ret;
+	int idx = chan->scan_index;
+
+	switch (mask) {
+	case IIO_CHAN_INFO_RAW:
+		mutex_lock(&st->cmd_lock);
+		ret = cros_ec_sensors_read_cmd(indio_dev, 1 << idx, &data);
+		mutex_unlock(&st->cmd_lock);
+		if (ret < 0)
+			break;
+		ret = IIO_VAL_INT;
+		*val = data;
+		break;
+	case IIO_CHAN_INFO_ENABLE:
+		ret = cros_ec_sync_get_enable(st, val);
+		if (ret < 0)
+			break;
+		ret = IIO_VAL_INT;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	mutex_unlock(&st->cmd_lock);
+	return ret;
+}
+
+static int cros_ec_sync_iio_write(struct iio_dev *indio_dev,
+				  struct iio_chan_spec const *chan,
+				  int val, int val2, long mask)
+{
+	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
+
+	switch (mask) {
+	case IIO_CHAN_INFO_ENABLE:
+		if (val < 0 || val > 1)
+			return -EINVAL;
+
+		return cros_ec_sync_set_enable(st, val);
+
+	default:
+		return -EINVAL;
+	}
+}
+
+static const struct iio_info cros_ec_sync_info = {
+	.read_raw = &cros_ec_sync_iio_read,
+	.write_raw = &cros_ec_sync_iio_write,
+};
+
+/* The counter can only increase, so only one function present. */
+static enum counter_count_function cros_ec_sync_functions[] = {
+	COUNTER_COUNT_FUNCTION_INCREASE,
+};
+
+/* 2 synapse actions allowed: count for each rising edge, or not. */
+static enum counter_synapse_action cros_ec_sync_synapse_actions[] = {
+	COUNTER_SYNAPSE_ACTION_NONE,
+	COUNTER_SYNAPSE_ACTION_RISING_EDGE,
+};
+
+static int cros_ec_sync_read_count(struct counter_device *counter,
+				   struct counter_count *count,
+				   unsigned long *val)
+{
+	struct cros_ec_sensors_core_state *st = counter->priv;
+	u16 raw;
+	int ret;
+
+	mutex_lock(&st->cmd_lock);
+	ret = cros_ec_sensors_read_cmd(iio_priv_to_dev(st), 1, &raw);
+	mutex_unlock(&st->cmd_lock);
+	if (ret)
+		return ret;
+
+	*val = raw;
+	return 0;
+}
+
+static int cros_ec_sync_function_get(struct counter_device *counter,
+				     struct counter_count *count,
+				     size_t *function)
+{
+	*function = 0;
+	return 0;
+}
+
+static int cros_ec_sync_action_get(struct counter_device *counter,
+				   struct counter_count *count,
+				   struct counter_synapse *synapse,
+				   size_t *action)
+{
+	struct cros_ec_sensors_core_state *st = counter->priv;
+	int ret;
+	int raw;
+
+	ret = cros_ec_sync_get_enable(st, &raw);
+	if (ret)
+		return ret;
+
+	*action = !!raw;
+	return 0;
+}
+
+static int cros_ec_sync_action_set(struct counter_device *counter,
+				   struct counter_count *count,
+				   struct counter_synapse *synapse,
+				   size_t action)
+{
+	struct cros_ec_sensors_core_state *st = counter->priv;
+
+	return cros_ec_sync_set_enable(st, action);
+}
+
+static const struct counter_ops cros_ec_sync_ops = {
+	.count_read = cros_ec_sync_read_count,
+	.function_get = cros_ec_sync_function_get,
+	.action_get = cros_ec_sync_action_get,
+	.action_set = cros_ec_sync_action_set,
+};
+
+static struct counter_signal cros_ec_sync_signals[] = {
+	{
+		.id = 0,
+		.name = "vsync"
+	}
+};
+
+static struct counter_synapse cros_ec_sync_synapses[] = {
+	{
+		.actions_list = cros_ec_sync_synapse_actions,
+		.num_actions = ARRAY_SIZE(cros_ec_sync_synapse_actions),
+		.signal = cros_ec_sync_signals
+	}
+};
+
+static struct counter_count cros_ec_sync_counts[] = {
+	{
+		.id = 0,
+		.name = "vsync",
+		.functions_list = cros_ec_sync_functions,
+		.num_functions = ARRAY_SIZE(cros_ec_sync_functions),
+		.synapses = cros_ec_sync_synapses,
+		.num_synapses = ARRAY_SIZE(cros_ec_sync_synapses),
+	}
+};
+
+static int cros_ec_sync_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct iio_dev *indio_dev;
+	struct cros_ec_sync_state *state;
+	struct iio_chan_spec *channel;
+	int ret;
+
+	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
+	if (!indio_dev)
+		return -ENOMEM;
+
+	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
+					cros_ec_sensors_capture,
+					cros_ec_sensors_push_data);
+	if (ret)
+		return ret;
+
+	indio_dev->info = &cros_ec_sync_info;
+	state = iio_priv(indio_dev);
+
+	/* Initialize IIO device */
+	channel = state->channels;
+
+	/* Counter channel */
+	channel->type = IIO_COUNT;
+	channel->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
+	channel->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE);
+	channel->scan_type.realbits = CROS_EC_SENSOR_BITS;
+	channel->scan_type.storagebits = CROS_EC_SENSOR_BITS;
+	channel->scan_type.shift = 0;
+	channel->scan_index = 0;
+	channel->ext_info = cros_ec_sensors_limited_info;
+	channel->scan_type.sign = 'u';
+
+	/* Timestamp channel */
+	channel++;
+	channel->type = IIO_TIMESTAMP;
+	channel->channel = -1;
+	channel->scan_index = 1;
+	channel->scan_type.sign = 's';
+	channel->scan_type.realbits = 64;
+	channel->scan_type.storagebits = 64;
+
+	indio_dev->channels = state->channels;
+	indio_dev->num_channels = MAX_CHANNELS;
+
+	state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
+
+	/* Initialize Counter device */
+	state->counter.name = dev_name(&pdev->dev);
+	state->counter.parent = &pdev->dev;
+	state->counter.ops = &cros_ec_sync_ops;
+	state->counter.counts = cros_ec_sync_counts;
+	state->counter.num_counts = ARRAY_SIZE(cros_ec_sync_counts);
+	state->counter.signals = cros_ec_sync_signals;
+	state->counter.num_signals = ARRAY_SIZE(cros_ec_sync_signals);
+	state->counter.priv = state;
+
+	ret = devm_iio_device_register(dev, indio_dev);
+	if (ret)
+		return ret;
+
+	return devm_counter_register(dev, &state->counter);
+}
+
+static const struct platform_device_id cros_ec_sync_ids[] = {
+	{
+		.name = "cros-ec-sync",
+	},
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, cros_ec_sync_ids);
+
+static struct platform_driver cros_ec_sync_platform_driver = {
+	.driver = {
+		.name	= "cros-ec-sync",
+	},
+	.probe		= cros_ec_sync_probe,
+	.id_table	= cros_ec_sync_ids,
+};
+module_platform_driver(cros_ec_sync_platform_driver);
+
+MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
+MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
+MODULE_LICENSE("GPL v2");
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 c831915ca7e56..3a15094616710 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
@@ -31,6 +31,7 @@
 static char *cros_ec_loc[] = {
 	[MOTIONSENSE_LOC_BASE] = "base",
 	[MOTIONSENSE_LOC_LID] = "lid",
+	[MOTIONSENSE_LOC_CAMERA] = "camera",
 	[MOTIONSENSE_LOC_MAX] = "unknown",
 };
 
@@ -467,6 +468,20 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
 };
 EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
 
+const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
+	{
+		.name = "id",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = cros_ec_sensors_id
+	},
+	{
+		.name = "location",
+		.shared = IIO_SHARED_BY_ALL,
+		.read = cros_ec_sensors_loc
+	},
+	{ },
+};
+EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
 /**
  * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
  * @st:		pointer to state information for device
diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
index b7f2c00db5e1e..e4ae0868d1e06 100644
--- a/drivers/platform/chrome/cros_ec_sensorhub.c
+++ b/drivers/platform/chrome/cros_ec_sensorhub.c
@@ -106,6 +106,9 @@ static int cros_ec_sensorhub_register(struct device *dev,
 		case MOTIONSENSE_TYPE_ACTIVITY:
 			name = "cros-ec-activity";
 			break;
+		case MOTIONSENSE_TYPE_SYNC:
+			name = "cros-ec-sync";
+			break;
 		default:
 			dev_warn(dev, "unknown type %d\n",
 				 sensorhub->resp->info.type);
diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
index 7bc961defa87e..e416b28cf24c7 100644
--- a/include/linux/iio/common/cros_ec_sensors_core.h
+++ b/include/linux/iio/common/cros_ec_sensors_core.h
@@ -114,7 +114,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
 			       struct iio_chan_spec const *chan,
 			       int val, int val2, long mask);
 
-/* List of extended channel specification for all sensors */
+/* List of extended channel specification for all sensors. */
+extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
+/* Add calibration to set above. */
 extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
 extern const struct attribute *cros_ec_sensor_fifo_attributes[];
 
-- 
2.26.0.110.g2183baf09c-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] drivers: counter: Add Cros EC Sync counter
  2020-04-08  5:36 [PATCH] drivers: counter: Add Cros EC Sync counter Gwendal Grignou
@ 2020-04-08  6:15 ` Gwendal Grignou
  2020-04-11 15:00 ` William Breathitt Gray
  2020-04-12 11:06 ` Jonathan Cameron
  2 siblings, 0 replies; 6+ messages in thread
From: Gwendal Grignou @ 2020-04-08  6:15 UTC (permalink / raw)
  To: Benson Leung, vilhelm.gray, Enric Balletbo i Serra,
	Guenter Roeck, Jonathan Cameron
  Cc: linux-kernel, linux-iio

[+Jonathan]

On Tue, Apr 7, 2020 at 10:37 PM Gwendal Grignou <gwendal@chromium.org> wrote:
>
> When the camera vsync pin is connected to the embedded controller (EC) of
> a chromebook, the EC reports a sensor with a counter that increases
> at each GPIO rising edge.
>
> The sensor is presented using the counter subsystem.
> In addition, it is also presented via the IIO subsystem with a timestamp,
> allowing synchronisation with sensors connected to the same EC, for
> image stabilisation or augmented reality applications.
>
> To enable the counter:
> via counter ABI:
> echo "rising edge" > counterX/count0/signal_action
> via iio ABI
> echo 1 > iio:deviceY/en
>
> To disable the counter:
> via counter ABI:
> echo "none" > counterX/count0/signal_action
> via iio ABI
> echo 0 > iio:deviceY/en
>
> To read the current counter value:
> via counter ABI:
> cat counterX/count0/count
> via iio ABI
> cat iio:deviceY/in_count_raw
> We can also read the value through the IIO buffer:
> echo 1 > iio:deviceY/scan_elements/in_timestamp_en
> echo 1 > iio:deviceY/scan_elements/in_count_en
> echo 1 > iio:deviceY/buffer/enable
>
> and each time to counter increase, the following binary blob
> will be appended to dev/iio:deviceY:
> 000f 0000 0000 0000 dc66 816c 0071 0000
>  \   <-- padding -> <-- timestamp ---->
>   count
>
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> An initial version of the driver was presented few month ago at
> https://patchwork.kernel.org/patch/11037009/
>
>  drivers/counter/Kconfig                       |  11 +
>  drivers/counter/Makefile                      |   1 +
>  drivers/counter/cros_ec_sync.c                | 342 ++++++++++++++++++
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |  15 +
>  drivers/platform/chrome/cros_ec_sensorhub.c   |   3 +
>  .../linux/iio/common/cros_ec_sensors_core.h   |   4 +-
>  6 files changed, 375 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/counter/cros_ec_sync.c
>
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index c80fa76bb5311..18fde918ff40b 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -29,6 +29,17 @@ config 104_QUAD_8
>           The base port addresses for the devices may be configured via the base
>           array module parameter.
>
> +config CROS_EC_SYNC
> +       tristate "ChromeOS EC Counter driver"
> +       depends on IIO_CROS_EC_SENSORS_CORE
> +       help
> +         Module to handle synchronisation sensor presented by the ChromeOS EC
> +         Sensor hub.
> +         Synchronisation sensor sends event to the host when the camera
> +         take a picture. It allows synchronisation with other MEMS sensor,
> +         like gyroscope for image statbilization or augmented reality
> +         application (AR).
> +
>  config STM32_TIMER_CNT
>         tristate "STM32 Timer encoder counter driver"
>         depends on MFD_STM32_TIMERS || COMPILE_TEST
> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> index 55142d1f4c436..98378fca50ad6 100644
> --- a/drivers/counter/Makefile
> +++ b/drivers/counter/Makefile
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_COUNTER) += counter.o
>
>  obj-$(CONFIG_104_QUAD_8)       += 104-quad-8.o
> +obj-$(CONFIG_CROS_EC_SYNC)     += cros_ec_sync.o
>  obj-$(CONFIG_STM32_TIMER_CNT)  += stm32-timer-cnt.o
>  obj-$(CONFIG_STM32_LPTIMER_CNT)        += stm32-lptimer-cnt.o
>  obj-$(CONFIG_TI_EQEP)          += ti-eqep.o
> diff --git a/drivers/counter/cros_ec_sync.c b/drivers/counter/cros_ec_sync.c
> new file mode 100644
> index 0000000000000..94b3ababab4bd
> --- /dev/null
> +++ b/drivers/counter/cros_ec_sync.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for synchronisation sensor behind CrOS EC.
> + *
> + * Copyright 2020 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about counter sensors. Counters are presented through
> + * iio sysfs.
> + */
> +
> +#include <linux/counter.h>
> +#include <linux/device.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/common/cros_ec_sensors_core.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/*
> + * One channel for counter, the other for timestamp.
> + */
> +#define MAX_CHANNELS (1 + 1)
> +
> +/**
> + * struct cros_ec_sync_state - device structure
> + *
> + * @core: common structure for all cros_ec sensor.
> + *        Must be at the beggining.
> + * @channels: presented iio channels(2).
> + * @counter: counter data structure.
> + */
> +struct cros_ec_sync_state {
> +       struct cros_ec_sensors_core_state core;
> +       struct iio_chan_spec channels[MAX_CHANNELS];
> +       struct counter_device counter;
> +};
> +
> +/**
> + * cros_ec_sync_get_enable() - Check if the counter is enabled.
> + *
> + * @st:     core cros_ec sensor
> + * @val:    status: 0: disabled, 1 enabled.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int cros_ec_sync_get_enable(struct cros_ec_sensors_core_state *st,
> +                                  int *val)
> +{
> +       int ret;
> +
> +       mutex_lock(&st->cmd_lock);
> +       st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> +       st->param.sensor_odr.data = EC_MOTION_SENSE_NO_VALUE;
> +
> +       ret = cros_ec_motion_send_host_cmd(st, 0);
> +       mutex_unlock(&st->cmd_lock);
> +       if (ret)
> +               return ret;
> +
> +       *val = !!st->resp->sensor_odr.ret;
> +       return 0;
> +}
> +
> +/**
> + * cros_ec_sync_set_enable() - Allow the counter to count.
> + *
> + * When enable, the counter will increase for each VSYNC rising edge
> + * and will produce an event in the iio buffer, if enabled.
> + *
> + * @st:     core cros_ec sensor
> + * @val:    0: disable, 1 enable.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int cros_ec_sync_set_enable(struct cros_ec_sensors_core_state *st,
> +                                  int val)
> +{
> +       int ret;
> +
> +       mutex_lock(&st->cmd_lock);
> +       st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> +       st->param.sensor_odr.data = val;
> +       st->param.sensor_odr.roundup = 1;
> +
> +       ret = cros_ec_motion_send_host_cmd(st, 0);
> +       mutex_unlock(&st->cmd_lock);
> +       return ret;
> +}
> +
> +static int cros_ec_sync_iio_read(struct iio_dev *indio_dev,
> +                                struct iio_chan_spec const *chan,
> +                                int *val, int *val2, long mask)
> +{
> +       struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +       u16 data;
> +       int ret;
> +       int idx = chan->scan_index;
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_RAW:
> +               mutex_lock(&st->cmd_lock);
> +               ret = cros_ec_sensors_read_cmd(indio_dev, 1 << idx, &data);
> +               mutex_unlock(&st->cmd_lock);
> +               if (ret < 0)
> +                       break;
> +               ret = IIO_VAL_INT;
> +               *val = data;
> +               break;
> +       case IIO_CHAN_INFO_ENABLE:
> +               ret = cros_ec_sync_get_enable(st, val);
> +               if (ret < 0)
> +                       break;
> +               ret = IIO_VAL_INT;
> +               break;
> +       default:
> +               ret = -EINVAL;
> +               break;
> +       }
> +       mutex_unlock(&st->cmd_lock);
> +       return ret;
> +}
> +
> +static int cros_ec_sync_iio_write(struct iio_dev *indio_dev,
> +                                 struct iio_chan_spec const *chan,
> +                                 int val, int val2, long mask)
> +{
> +       struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +
> +       switch (mask) {
> +       case IIO_CHAN_INFO_ENABLE:
> +               if (val < 0 || val > 1)
> +                       return -EINVAL;
> +
> +               return cros_ec_sync_set_enable(st, val);
> +
> +       default:
> +               return -EINVAL;
> +       }
> +}
> +
> +static const struct iio_info cros_ec_sync_info = {
> +       .read_raw = &cros_ec_sync_iio_read,
> +       .write_raw = &cros_ec_sync_iio_write,
> +};
> +
> +/* The counter can only increase, so only one function present. */
> +static enum counter_count_function cros_ec_sync_functions[] = {
> +       COUNTER_COUNT_FUNCTION_INCREASE,
> +};
> +
> +/* 2 synapse actions allowed: count for each rising edge, or not. */
> +static enum counter_synapse_action cros_ec_sync_synapse_actions[] = {
> +       COUNTER_SYNAPSE_ACTION_NONE,
> +       COUNTER_SYNAPSE_ACTION_RISING_EDGE,
> +};
> +
> +static int cros_ec_sync_read_count(struct counter_device *counter,
> +                                  struct counter_count *count,
> +                                  unsigned long *val)
> +{
> +       struct cros_ec_sensors_core_state *st = counter->priv;
> +       u16 raw;
> +       int ret;
> +
> +       mutex_lock(&st->cmd_lock);
> +       ret = cros_ec_sensors_read_cmd(iio_priv_to_dev(st), 1, &raw);
> +       mutex_unlock(&st->cmd_lock);
> +       if (ret)
> +               return ret;
> +
> +       *val = raw;
> +       return 0;
> +}
> +
> +static int cros_ec_sync_function_get(struct counter_device *counter,
> +                                    struct counter_count *count,
> +                                    size_t *function)
> +{
> +       *function = 0;
> +       return 0;
> +}
> +
> +static int cros_ec_sync_action_get(struct counter_device *counter,
> +                                  struct counter_count *count,
> +                                  struct counter_synapse *synapse,
> +                                  size_t *action)
> +{
> +       struct cros_ec_sensors_core_state *st = counter->priv;
> +       int ret;
> +       int raw;
> +
> +       ret = cros_ec_sync_get_enable(st, &raw);
> +       if (ret)
> +               return ret;
> +
> +       *action = !!raw;
> +       return 0;
> +}
> +
> +static int cros_ec_sync_action_set(struct counter_device *counter,
> +                                  struct counter_count *count,
> +                                  struct counter_synapse *synapse,
> +                                  size_t action)
> +{
> +       struct cros_ec_sensors_core_state *st = counter->priv;
> +
> +       return cros_ec_sync_set_enable(st, action);
> +}
> +
> +static const struct counter_ops cros_ec_sync_ops = {
> +       .count_read = cros_ec_sync_read_count,
> +       .function_get = cros_ec_sync_function_get,
> +       .action_get = cros_ec_sync_action_get,
> +       .action_set = cros_ec_sync_action_set,
> +};
> +
> +static struct counter_signal cros_ec_sync_signals[] = {
> +       {
> +               .id = 0,
> +               .name = "vsync"
> +       }
> +};
> +
> +static struct counter_synapse cros_ec_sync_synapses[] = {
> +       {
> +               .actions_list = cros_ec_sync_synapse_actions,
> +               .num_actions = ARRAY_SIZE(cros_ec_sync_synapse_actions),
> +               .signal = cros_ec_sync_signals
> +       }
> +};
> +
> +static struct counter_count cros_ec_sync_counts[] = {
> +       {
> +               .id = 0,
> +               .name = "vsync",
> +               .functions_list = cros_ec_sync_functions,
> +               .num_functions = ARRAY_SIZE(cros_ec_sync_functions),
> +               .synapses = cros_ec_sync_synapses,
> +               .num_synapses = ARRAY_SIZE(cros_ec_sync_synapses),
> +       }
> +};
> +
> +static int cros_ec_sync_probe(struct platform_device *pdev)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct iio_dev *indio_dev;
> +       struct cros_ec_sync_state *state;
> +       struct iio_chan_spec *channel;
> +       int ret;
> +
> +       indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> +       if (!indio_dev)
> +               return -ENOMEM;
> +
> +       ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> +                                       cros_ec_sensors_capture,
> +                                       cros_ec_sensors_push_data);
> +       if (ret)
> +               return ret;
> +
> +       indio_dev->info = &cros_ec_sync_info;
> +       state = iio_priv(indio_dev);
> +
> +       /* Initialize IIO device */
> +       channel = state->channels;
> +
> +       /* Counter channel */
> +       channel->type = IIO_COUNT;
> +       channel->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> +       channel->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE);
> +       channel->scan_type.realbits = CROS_EC_SENSOR_BITS;
> +       channel->scan_type.storagebits = CROS_EC_SENSOR_BITS;
> +       channel->scan_type.shift = 0;
> +       channel->scan_index = 0;
> +       channel->ext_info = cros_ec_sensors_limited_info;
> +       channel->scan_type.sign = 'u';
> +
> +       /* Timestamp channel */
> +       channel++;
> +       channel->type = IIO_TIMESTAMP;
> +       channel->channel = -1;
> +       channel->scan_index = 1;
> +       channel->scan_type.sign = 's';
> +       channel->scan_type.realbits = 64;
> +       channel->scan_type.storagebits = 64;
> +
> +       indio_dev->channels = state->channels;
> +       indio_dev->num_channels = MAX_CHANNELS;
> +
> +       state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
> +
> +       /* Initialize Counter device */
> +       state->counter.name = dev_name(&pdev->dev);
> +       state->counter.parent = &pdev->dev;
> +       state->counter.ops = &cros_ec_sync_ops;
> +       state->counter.counts = cros_ec_sync_counts;
> +       state->counter.num_counts = ARRAY_SIZE(cros_ec_sync_counts);
> +       state->counter.signals = cros_ec_sync_signals;
> +       state->counter.num_signals = ARRAY_SIZE(cros_ec_sync_signals);
> +       state->counter.priv = state;
> +
> +       ret = devm_iio_device_register(dev, indio_dev);
> +       if (ret)
> +               return ret;
> +
> +       return devm_counter_register(dev, &state->counter);
> +}
> +
> +static const struct platform_device_id cros_ec_sync_ids[] = {
> +       {
> +               .name = "cros-ec-sync",
> +       },
> +       { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, cros_ec_sync_ids);
> +
> +static struct platform_driver cros_ec_sync_platform_driver = {
> +       .driver = {
> +               .name   = "cros-ec-sync",
> +       },
> +       .probe          = cros_ec_sync_probe,
> +       .id_table       = cros_ec_sync_ids,
> +};
> +module_platform_driver(cros_ec_sync_platform_driver);
> +
> +MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
> +MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
> +MODULE_LICENSE("GPL v2");
> 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 c831915ca7e56..3a15094616710 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
> @@ -31,6 +31,7 @@
>  static char *cros_ec_loc[] = {
>         [MOTIONSENSE_LOC_BASE] = "base",
>         [MOTIONSENSE_LOC_LID] = "lid",
> +       [MOTIONSENSE_LOC_CAMERA] = "camera",
>         [MOTIONSENSE_LOC_MAX] = "unknown",
>  };
>
> @@ -467,6 +468,20 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
>  };
>  EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
>
> +const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
> +       {
> +               .name = "id",
> +               .shared = IIO_SHARED_BY_ALL,
> +               .read = cros_ec_sensors_id
> +       },
> +       {
> +               .name = "location",
> +               .shared = IIO_SHARED_BY_ALL,
> +               .read = cros_ec_sensors_loc
> +       },
> +       { },
> +};
> +EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
>  /**
>   * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
>   * @st:                pointer to state information for device
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
> index b7f2c00db5e1e..e4ae0868d1e06 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> @@ -106,6 +106,9 @@ static int cros_ec_sensorhub_register(struct device *dev,
>                 case MOTIONSENSE_TYPE_ACTIVITY:
>                         name = "cros-ec-activity";
>                         break;
> +               case MOTIONSENSE_TYPE_SYNC:
> +                       name = "cros-ec-sync";
> +                       break;
>                 default:
>                         dev_warn(dev, "unknown type %d\n",
>                                  sensorhub->resp->info.type);
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index 7bc961defa87e..e416b28cf24c7 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -114,7 +114,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
>                                struct iio_chan_spec const *chan,
>                                int val, int val2, long mask);
>
> -/* List of extended channel specification for all sensors */
> +/* List of extended channel specification for all sensors. */
> +extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
> +/* Add calibration to set above. */
>  extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
>  extern const struct attribute *cros_ec_sensor_fifo_attributes[];
>
> --
> 2.26.0.110.g2183baf09c-goog
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drivers: counter: Add Cros EC Sync counter
  2020-04-08  5:36 [PATCH] drivers: counter: Add Cros EC Sync counter Gwendal Grignou
  2020-04-08  6:15 ` Gwendal Grignou
@ 2020-04-11 15:00 ` William Breathitt Gray
  2020-04-13 15:17   ` Enric Balletbo i Serra
  2020-04-12 11:06 ` Jonathan Cameron
  2 siblings, 1 reply; 6+ messages in thread
From: William Breathitt Gray @ 2020-04-11 15:00 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: bleung, enric.balletbo, jic23, groeck, linux-kernel, linux-iio

[-- Attachment #1: Type: text/plain, Size: 16727 bytes --]

On Tue, Apr 07, 2020 at 10:36:55PM -0700, Gwendal Grignou wrote:
> When the camera vsync pin is connected to the embedded controller (EC) of
> a chromebook, the EC reports a sensor with a counter that increases
> at each GPIO rising edge.
> 
> The sensor is presented using the counter subsystem.
> In addition, it is also presented via the IIO subsystem with a timestamp,
> allowing synchronisation with sensors connected to the same EC, for
> image stabilisation or augmented reality applications.
> 
> To enable the counter:
> via counter ABI:
> echo "rising edge" > counterX/count0/signal_action
> via iio ABI
> echo 1 > iio:deviceY/en
> 
> To disable the counter:
> via counter ABI:
> echo "none" > counterX/count0/signal_action
> via iio ABI
> echo 0 > iio:deviceY/en
> 
> To read the current counter value:
> via counter ABI:
> cat counterX/count0/count
> via iio ABI
> cat iio:deviceY/in_count_raw
> We can also read the value through the IIO buffer:
> echo 1 > iio:deviceY/scan_elements/in_timestamp_en
> echo 1 > iio:deviceY/scan_elements/in_count_en
> echo 1 > iio:deviceY/buffer/enable
> 
> and each time to counter increase, the following binary blob
> will be appended to dev/iio:deviceY:
> 000f 0000 0000 0000 dc66 816c 0071 0000
>  \   <-- padding -> <-- timestamp ---->
>   count
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> An initial version of the driver was presented few month ago at
> https://patchwork.kernel.org/patch/11037009/
> 
>  drivers/counter/Kconfig                       |  11 +
>  drivers/counter/Makefile                      |   1 +
>  drivers/counter/cros_ec_sync.c                | 342 ++++++++++++++++++
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |  15 +
>  drivers/platform/chrome/cros_ec_sensorhub.c   |   3 +
>  .../linux/iio/common/cros_ec_sensors_core.h   |   4 +-
>  6 files changed, 375 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/counter/cros_ec_sync.c

Hi Gwendal,

I'm getting some conflicts when I try to merge this patch, so it looks
like we're on different bases. Would you be able to rebase on the
current testing branch of the IIO tree and resubmit this patch?

Thanks,

William Breathitt Gray

> 
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index c80fa76bb5311..18fde918ff40b 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -29,6 +29,17 @@ config 104_QUAD_8
>  	  The base port addresses for the devices may be configured via the base
>  	  array module parameter.
>  
> +config CROS_EC_SYNC
> +	tristate "ChromeOS EC Counter driver"
> +	depends on IIO_CROS_EC_SENSORS_CORE
> +	help
> +	  Module to handle synchronisation sensor presented by the ChromeOS EC
> +	  Sensor hub.
> +	  Synchronisation sensor sends event to the host when the camera
> +	  take a picture. It allows synchronisation with other MEMS sensor,
> +	  like gyroscope for image statbilization or augmented reality
> +	  application (AR).
> +
>  config STM32_TIMER_CNT
>  	tristate "STM32 Timer encoder counter driver"
>  	depends on MFD_STM32_TIMERS || COMPILE_TEST
> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> index 55142d1f4c436..98378fca50ad6 100644
> --- a/drivers/counter/Makefile
> +++ b/drivers/counter/Makefile
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_COUNTER) += counter.o
>  
>  obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
> +obj-$(CONFIG_CROS_EC_SYNC)	+= cros_ec_sync.o
>  obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o
>  obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o
>  obj-$(CONFIG_TI_EQEP)		+= ti-eqep.o
> diff --git a/drivers/counter/cros_ec_sync.c b/drivers/counter/cros_ec_sync.c
> new file mode 100644
> index 0000000000000..94b3ababab4bd
> --- /dev/null
> +++ b/drivers/counter/cros_ec_sync.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for synchronisation sensor behind CrOS EC.
> + *
> + * Copyright 2020 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about counter sensors. Counters are presented through
> + * iio sysfs.
> + */
> +
> +#include <linux/counter.h>
> +#include <linux/device.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/common/cros_ec_sensors_core.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/*
> + * One channel for counter, the other for timestamp.
> + */
> +#define MAX_CHANNELS (1 + 1)
> +
> +/**
> + * struct cros_ec_sync_state - device structure
> + *
> + * @core: common structure for all cros_ec sensor.
> + *        Must be at the beggining.
> + * @channels: presented iio channels(2).
> + * @counter: counter data structure.
> + */
> +struct cros_ec_sync_state {
> +	struct cros_ec_sensors_core_state core;
> +	struct iio_chan_spec channels[MAX_CHANNELS];
> +	struct counter_device counter;
> +};
> +
> +/**
> + * cros_ec_sync_get_enable() - Check if the counter is enabled.
> + *
> + * @st:     core cros_ec sensor
> + * @val:    status: 0: disabled, 1 enabled.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int cros_ec_sync_get_enable(struct cros_ec_sensors_core_state *st,
> +				   int *val)
> +{
> +	int ret;
> +
> +	mutex_lock(&st->cmd_lock);
> +	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> +	st->param.sensor_odr.data = EC_MOTION_SENSE_NO_VALUE;
> +
> +	ret = cros_ec_motion_send_host_cmd(st, 0);
> +	mutex_unlock(&st->cmd_lock);
> +	if (ret)
> +		return ret;
> +
> +	*val = !!st->resp->sensor_odr.ret;
> +	return 0;
> +}
> +
> +/**
> + * cros_ec_sync_set_enable() - Allow the counter to count.
> + *
> + * When enable, the counter will increase for each VSYNC rising edge
> + * and will produce an event in the iio buffer, if enabled.
> + *
> + * @st:     core cros_ec sensor
> + * @val:    0: disable, 1 enable.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int cros_ec_sync_set_enable(struct cros_ec_sensors_core_state *st,
> +				   int val)
> +{
> +	int ret;
> +
> +	mutex_lock(&st->cmd_lock);
> +	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> +	st->param.sensor_odr.data = val;
> +	st->param.sensor_odr.roundup = 1;
> +
> +	ret = cros_ec_motion_send_host_cmd(st, 0);
> +	mutex_unlock(&st->cmd_lock);
> +	return ret;
> +}
> +
> +static int cros_ec_sync_iio_read(struct iio_dev *indio_dev,
> +				 struct iio_chan_spec const *chan,
> +				 int *val, int *val2, long mask)
> +{
> +	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +	u16 data;
> +	int ret;
> +	int idx = chan->scan_index;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->cmd_lock);
> +		ret = cros_ec_sensors_read_cmd(indio_dev, 1 << idx, &data);
> +		mutex_unlock(&st->cmd_lock);
> +		if (ret < 0)
> +			break;
> +		ret = IIO_VAL_INT;
> +		*val = data;
> +		break;
> +	case IIO_CHAN_INFO_ENABLE:
> +		ret = cros_ec_sync_get_enable(st, val);
> +		if (ret < 0)
> +			break;
> +		ret = IIO_VAL_INT;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	mutex_unlock(&st->cmd_lock);
> +	return ret;
> +}
> +
> +static int cros_ec_sync_iio_write(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec const *chan,
> +				  int val, int val2, long mask)
> +{
> +	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_ENABLE:
> +		if (val < 0 || val > 1)
> +			return -EINVAL;
> +
> +		return cros_ec_sync_set_enable(st, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info cros_ec_sync_info = {
> +	.read_raw = &cros_ec_sync_iio_read,
> +	.write_raw = &cros_ec_sync_iio_write,
> +};
> +
> +/* The counter can only increase, so only one function present. */
> +static enum counter_count_function cros_ec_sync_functions[] = {
> +	COUNTER_COUNT_FUNCTION_INCREASE,
> +};
> +
> +/* 2 synapse actions allowed: count for each rising edge, or not. */
> +static enum counter_synapse_action cros_ec_sync_synapse_actions[] = {
> +	COUNTER_SYNAPSE_ACTION_NONE,
> +	COUNTER_SYNAPSE_ACTION_RISING_EDGE,
> +};
> +
> +static int cros_ec_sync_read_count(struct counter_device *counter,
> +				   struct counter_count *count,
> +				   unsigned long *val)
> +{
> +	struct cros_ec_sensors_core_state *st = counter->priv;
> +	u16 raw;
> +	int ret;
> +
> +	mutex_lock(&st->cmd_lock);
> +	ret = cros_ec_sensors_read_cmd(iio_priv_to_dev(st), 1, &raw);
> +	mutex_unlock(&st->cmd_lock);
> +	if (ret)
> +		return ret;
> +
> +	*val = raw;
> +	return 0;
> +}
> +
> +static int cros_ec_sync_function_get(struct counter_device *counter,
> +				     struct counter_count *count,
> +				     size_t *function)
> +{
> +	*function = 0;
> +	return 0;
> +}
> +
> +static int cros_ec_sync_action_get(struct counter_device *counter,
> +				   struct counter_count *count,
> +				   struct counter_synapse *synapse,
> +				   size_t *action)
> +{
> +	struct cros_ec_sensors_core_state *st = counter->priv;
> +	int ret;
> +	int raw;
> +
> +	ret = cros_ec_sync_get_enable(st, &raw);
> +	if (ret)
> +		return ret;
> +
> +	*action = !!raw;
> +	return 0;
> +}
> +
> +static int cros_ec_sync_action_set(struct counter_device *counter,
> +				   struct counter_count *count,
> +				   struct counter_synapse *synapse,
> +				   size_t action)
> +{
> +	struct cros_ec_sensors_core_state *st = counter->priv;
> +
> +	return cros_ec_sync_set_enable(st, action);
> +}
> +
> +static const struct counter_ops cros_ec_sync_ops = {
> +	.count_read = cros_ec_sync_read_count,
> +	.function_get = cros_ec_sync_function_get,
> +	.action_get = cros_ec_sync_action_get,
> +	.action_set = cros_ec_sync_action_set,
> +};
> +
> +static struct counter_signal cros_ec_sync_signals[] = {
> +	{
> +		.id = 0,
> +		.name = "vsync"
> +	}
> +};
> +
> +static struct counter_synapse cros_ec_sync_synapses[] = {
> +	{
> +		.actions_list = cros_ec_sync_synapse_actions,
> +		.num_actions = ARRAY_SIZE(cros_ec_sync_synapse_actions),
> +		.signal = cros_ec_sync_signals
> +	}
> +};
> +
> +static struct counter_count cros_ec_sync_counts[] = {
> +	{
> +		.id = 0,
> +		.name = "vsync",
> +		.functions_list = cros_ec_sync_functions,
> +		.num_functions = ARRAY_SIZE(cros_ec_sync_functions),
> +		.synapses = cros_ec_sync_synapses,
> +		.num_synapses = ARRAY_SIZE(cros_ec_sync_synapses),
> +	}
> +};
> +
> +static int cros_ec_sync_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct cros_ec_sync_state *state;
> +	struct iio_chan_spec *channel;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> +					cros_ec_sensors_capture,
> +					cros_ec_sensors_push_data);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->info = &cros_ec_sync_info;
> +	state = iio_priv(indio_dev);
> +
> +	/* Initialize IIO device */
> +	channel = state->channels;
> +
> +	/* Counter channel */
> +	channel->type = IIO_COUNT;
> +	channel->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> +	channel->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE);
> +	channel->scan_type.realbits = CROS_EC_SENSOR_BITS;
> +	channel->scan_type.storagebits = CROS_EC_SENSOR_BITS;
> +	channel->scan_type.shift = 0;
> +	channel->scan_index = 0;
> +	channel->ext_info = cros_ec_sensors_limited_info;
> +	channel->scan_type.sign = 'u';
> +
> +	/* Timestamp channel */
> +	channel++;
> +	channel->type = IIO_TIMESTAMP;
> +	channel->channel = -1;
> +	channel->scan_index = 1;
> +	channel->scan_type.sign = 's';
> +	channel->scan_type.realbits = 64;
> +	channel->scan_type.storagebits = 64;
> +
> +	indio_dev->channels = state->channels;
> +	indio_dev->num_channels = MAX_CHANNELS;
> +
> +	state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
> +
> +	/* Initialize Counter device */
> +	state->counter.name = dev_name(&pdev->dev);
> +	state->counter.parent = &pdev->dev;
> +	state->counter.ops = &cros_ec_sync_ops;
> +	state->counter.counts = cros_ec_sync_counts;
> +	state->counter.num_counts = ARRAY_SIZE(cros_ec_sync_counts);
> +	state->counter.signals = cros_ec_sync_signals;
> +	state->counter.num_signals = ARRAY_SIZE(cros_ec_sync_signals);
> +	state->counter.priv = state;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return devm_counter_register(dev, &state->counter);
> +}
> +
> +static const struct platform_device_id cros_ec_sync_ids[] = {
> +	{
> +		.name = "cros-ec-sync",
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, cros_ec_sync_ids);
> +
> +static struct platform_driver cros_ec_sync_platform_driver = {
> +	.driver = {
> +		.name	= "cros-ec-sync",
> +	},
> +	.probe		= cros_ec_sync_probe,
> +	.id_table	= cros_ec_sync_ids,
> +};
> +module_platform_driver(cros_ec_sync_platform_driver);
> +
> +MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
> +MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
> +MODULE_LICENSE("GPL v2");
> 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 c831915ca7e56..3a15094616710 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
> @@ -31,6 +31,7 @@
>  static char *cros_ec_loc[] = {
>  	[MOTIONSENSE_LOC_BASE] = "base",
>  	[MOTIONSENSE_LOC_LID] = "lid",
> +	[MOTIONSENSE_LOC_CAMERA] = "camera",
>  	[MOTIONSENSE_LOC_MAX] = "unknown",
>  };
>  
> @@ -467,6 +468,20 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
>  };
>  EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
>  
> +const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
> +	{
> +		.name = "id",
> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = cros_ec_sensors_id
> +	},
> +	{
> +		.name = "location",
> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = cros_ec_sensors_loc
> +	},
> +	{ },
> +};
> +EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
>  /**
>   * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
>   * @st:		pointer to state information for device
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
> index b7f2c00db5e1e..e4ae0868d1e06 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> @@ -106,6 +106,9 @@ static int cros_ec_sensorhub_register(struct device *dev,
>  		case MOTIONSENSE_TYPE_ACTIVITY:
>  			name = "cros-ec-activity";
>  			break;
> +		case MOTIONSENSE_TYPE_SYNC:
> +			name = "cros-ec-sync";
> +			break;
>  		default:
>  			dev_warn(dev, "unknown type %d\n",
>  				 sensorhub->resp->info.type);
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index 7bc961defa87e..e416b28cf24c7 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -114,7 +114,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
>  			       struct iio_chan_spec const *chan,
>  			       int val, int val2, long mask);
>  
> -/* List of extended channel specification for all sensors */
> +/* List of extended channel specification for all sensors. */
> +extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
> +/* Add calibration to set above. */
>  extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
>  extern const struct attribute *cros_ec_sensor_fifo_attributes[];
>  
> -- 
> 2.26.0.110.g2183baf09c-goog
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drivers: counter: Add Cros EC Sync counter
  2020-04-08  5:36 [PATCH] drivers: counter: Add Cros EC Sync counter Gwendal Grignou
  2020-04-08  6:15 ` Gwendal Grignou
  2020-04-11 15:00 ` William Breathitt Gray
@ 2020-04-12 11:06 ` Jonathan Cameron
  2 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2020-04-12 11:06 UTC (permalink / raw)
  To: Gwendal Grignou
  Cc: bleung, vilhelm.gray, enric.balletbo, ic23, groeck, linux-kernel,
	linux-iio

On Tue,  7 Apr 2020 22:36:55 -0700
Gwendal Grignou <gwendal@chromium.org> wrote:

> When the camera vsync pin is connected to the embedded controller (EC) of
> a chromebook, the EC reports a sensor with a counter that increases
> at each GPIO rising edge.
> 
> The sensor is presented using the counter subsystem.
> In addition, it is also presented via the IIO subsystem with a timestamp,
> allowing synchronisation with sensors connected to the same EC, for
> image stabilisation or augmented reality applications.
> 
> To enable the counter:
> via counter ABI:
> echo "rising edge" > counterX/count0/signal_action
> via iio ABI
> echo 1 > iio:deviceY/en
> 
> To disable the counter:
> via counter ABI:
> echo "none" > counterX/count0/signal_action
> via iio ABI
> echo 0 > iio:deviceY/en
> 
> To read the current counter value:
> via counter ABI:
> cat counterX/count0/count
> via iio ABI
> cat iio:deviceY/in_count_raw
> We can also read the value through the IIO buffer:
> echo 1 > iio:deviceY/scan_elements/in_timestamp_en
> echo 1 > iio:deviceY/scan_elements/in_count_en
> echo 1 > iio:deviceY/buffer/enable
> 
> and each time to counter increase, the following binary blob
> will be appended to dev/iio:deviceY:
> 000f 0000 0000 0000 dc66 816c 0071 0000
>  \   <-- padding -> <-- timestamp ---->
>   count
> 
> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> ---
> An initial version of the driver was presented few month ago at
> https://patchwork.kernel.org/patch/11037009/
> 
>  drivers/counter/Kconfig                       |  11 +
>  drivers/counter/Makefile                      |   1 +
>  drivers/counter/cros_ec_sync.c                | 342 ++++++++++++++++++
>  .../cros_ec_sensors/cros_ec_sensors_core.c    |  15 +
>  drivers/platform/chrome/cros_ec_sensorhub.c   |   3 +
>  .../linux/iio/common/cros_ec_sensors_core.h   |   4 +-
>  6 files changed, 375 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/counter/cros_ec_sync.c
> 
> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> index c80fa76bb5311..18fde918ff40b 100644
> --- a/drivers/counter/Kconfig
> +++ b/drivers/counter/Kconfig
> @@ -29,6 +29,17 @@ config 104_QUAD_8
>  	  The base port addresses for the devices may be configured via the base
>  	  array module parameter.
>  
> +config CROS_EC_SYNC
> +	tristate "ChromeOS EC Counter driver"
> +	depends on IIO_CROS_EC_SENSORS_CORE
> +	help
> +	  Module to handle synchronisation sensor presented by the ChromeOS EC
> +	  Sensor hub.
> +	  Synchronisation sensor sends event to the host when the camera
> +	  take a picture. It allows synchronisation with other MEMS sensor,
> +	  like gyroscope for image statbilization or augmented reality
> +	  application (AR).
> +
>  config STM32_TIMER_CNT
>  	tristate "STM32 Timer encoder counter driver"
>  	depends on MFD_STM32_TIMERS || COMPILE_TEST
> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> index 55142d1f4c436..98378fca50ad6 100644
> --- a/drivers/counter/Makefile
> +++ b/drivers/counter/Makefile
> @@ -6,6 +6,7 @@
>  obj-$(CONFIG_COUNTER) += counter.o
>  
>  obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
> +obj-$(CONFIG_CROS_EC_SYNC)	+= cros_ec_sync.o
>  obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o
>  obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o
>  obj-$(CONFIG_TI_EQEP)		+= ti-eqep.o
> diff --git a/drivers/counter/cros_ec_sync.c b/drivers/counter/cros_ec_sync.c
> new file mode 100644
> index 0000000000000..94b3ababab4bd
> --- /dev/null
> +++ b/drivers/counter/cros_ec_sync.c
> @@ -0,0 +1,342 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Driver for synchronisation sensor behind CrOS EC.
> + *
> + * Copyright 2020 Google, Inc
> + *
> + * This software is licensed under the terms of the GNU General Public
> + * License version 2, as published by the Free Software Foundation, and
> + * may be copied, distributed, and modified under those terms.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> + * EC about counter sensors. Counters are presented through
> + * iio sysfs.
> + */
> +
> +#include <linux/counter.h>
> +#include <linux/device.h>
> +#include <linux/iio/buffer.h>
> +#include <linux/iio/common/cros_ec_sensors_core.h>
> +#include <linux/iio/iio.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/platform_data/cros_ec_commands.h>
> +#include <linux/platform_data/cros_ec_proto.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +/*
> + * One channel for counter, the other for timestamp.
> + */
> +#define MAX_CHANNELS (1 + 1)
> +
> +/**
> + * struct cros_ec_sync_state - device structure
> + *
> + * @core: common structure for all cros_ec sensor.
> + *        Must be at the beggining.
> + * @channels: presented iio channels(2).
> + * @counter: counter data structure.
> + */
> +struct cros_ec_sync_state {
> +	struct cros_ec_sensors_core_state core;
> +	struct iio_chan_spec channels[MAX_CHANNELS];
> +	struct counter_device counter;
> +};
> +
> +/**
> + * cros_ec_sync_get_enable() - Check if the counter is enabled.
> + *
> + * @st:     core cros_ec sensor
> + * @val:    status: 0: disabled, 1 enabled.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int cros_ec_sync_get_enable(struct cros_ec_sensors_core_state *st,
> +				   int *val)
> +{
> +	int ret;
> +
> +	mutex_lock(&st->cmd_lock);
> +	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> +	st->param.sensor_odr.data = EC_MOTION_SENSE_NO_VALUE;
> +
> +	ret = cros_ec_motion_send_host_cmd(st, 0);
> +	mutex_unlock(&st->cmd_lock);
> +	if (ret)
> +		return ret;
> +
> +	*val = !!st->resp->sensor_odr.ret;
> +	return 0;
> +}
> +
> +/**
> + * cros_ec_sync_set_enable() - Allow the counter to count.
> + *
> + * When enable, the counter will increase for each VSYNC rising edge
> + * and will produce an event in the iio buffer, if enabled.
> + *
> + * @st:     core cros_ec sensor
> + * @val:    0: disable, 1 enable.
> + *
> + * Return: 0 on success, -errno on failure.
> + */
> +static int cros_ec_sync_set_enable(struct cros_ec_sensors_core_state *st,
> +				   int val)
> +{
> +	int ret;
> +
> +	mutex_lock(&st->cmd_lock);
> +	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> +	st->param.sensor_odr.data = val;
> +	st->param.sensor_odr.roundup = 1;
> +
> +	ret = cros_ec_motion_send_host_cmd(st, 0);
> +	mutex_unlock(&st->cmd_lock);
> +	return ret;
> +}
> +
> +static int cros_ec_sync_iio_read(struct iio_dev *indio_dev,
> +				 struct iio_chan_spec const *chan,
> +				 int *val, int *val2, long mask)
> +{
> +	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +	u16 data;
> +	int ret;
> +	int idx = chan->scan_index;
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_RAW:
> +		mutex_lock(&st->cmd_lock);
> +		ret = cros_ec_sensors_read_cmd(indio_dev, 1 << idx, &data);
> +		mutex_unlock(&st->cmd_lock);
> +		if (ret < 0)
> +			break;
> +		ret = IIO_VAL_INT;
> +		*val = data;
> +		break;
> +	case IIO_CHAN_INFO_ENABLE:
> +		ret = cros_ec_sync_get_enable(st, val);
> +		if (ret < 0)
> +			break;
> +		ret = IIO_VAL_INT;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	mutex_unlock(&st->cmd_lock);
> +	return ret;
> +}
> +
> +static int cros_ec_sync_iio_write(struct iio_dev *indio_dev,
> +				  struct iio_chan_spec const *chan,
> +				  int val, int val2, long mask)
> +{
> +	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> +
> +	switch (mask) {
> +	case IIO_CHAN_INFO_ENABLE:
> +		if (val < 0 || val > 1)
> +			return -EINVAL;
> +
> +		return cros_ec_sync_set_enable(st, val);
> +
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static const struct iio_info cros_ec_sync_info = {
> +	.read_raw = &cros_ec_sync_iio_read,
> +	.write_raw = &cros_ec_sync_iio_write,
> +};
> +
> +/* The counter can only increase, so only one function present. */
> +static enum counter_count_function cros_ec_sync_functions[] = {
> +	COUNTER_COUNT_FUNCTION_INCREASE,
> +};
> +
> +/* 2 synapse actions allowed: count for each rising edge, or not. */
> +static enum counter_synapse_action cros_ec_sync_synapse_actions[] = {
> +	COUNTER_SYNAPSE_ACTION_NONE,
> +	COUNTER_SYNAPSE_ACTION_RISING_EDGE,
> +};
> +
> +static int cros_ec_sync_read_count(struct counter_device *counter,
> +				   struct counter_count *count,
> +				   unsigned long *val)
> +{
> +	struct cros_ec_sensors_core_state *st = counter->priv;
> +	u16 raw;
> +	int ret;
> +
> +	mutex_lock(&st->cmd_lock);
> +	ret = cros_ec_sensors_read_cmd(iio_priv_to_dev(st), 1, &raw);
> +	mutex_unlock(&st->cmd_lock);
> +	if (ret)
> +		return ret;
> +
> +	*val = raw;
> +	return 0;
> +}
> +
> +static int cros_ec_sync_function_get(struct counter_device *counter,
> +				     struct counter_count *count,
> +				     size_t *function)
> +{
> +	*function = 0;
> +	return 0;
> +}
> +
> +static int cros_ec_sync_action_get(struct counter_device *counter,
> +				   struct counter_count *count,
> +				   struct counter_synapse *synapse,
> +				   size_t *action)
> +{
> +	struct cros_ec_sensors_core_state *st = counter->priv;
> +	int ret;
> +	int raw;
> +
> +	ret = cros_ec_sync_get_enable(st, &raw);
> +	if (ret)
> +		return ret;
> +
> +	*action = !!raw;
> +	return 0;
> +}
> +
> +static int cros_ec_sync_action_set(struct counter_device *counter,
> +				   struct counter_count *count,
> +				   struct counter_synapse *synapse,
> +				   size_t action)
> +{
> +	struct cros_ec_sensors_core_state *st = counter->priv;
> +
> +	return cros_ec_sync_set_enable(st, action);
> +}
> +
> +static const struct counter_ops cros_ec_sync_ops = {
> +	.count_read = cros_ec_sync_read_count,
> +	.function_get = cros_ec_sync_function_get,
> +	.action_get = cros_ec_sync_action_get,
> +	.action_set = cros_ec_sync_action_set,
> +};
> +
> +static struct counter_signal cros_ec_sync_signals[] = {
> +	{
> +		.id = 0,
> +		.name = "vsync"
> +	}
> +};
> +
> +static struct counter_synapse cros_ec_sync_synapses[] = {
> +	{
> +		.actions_list = cros_ec_sync_synapse_actions,
> +		.num_actions = ARRAY_SIZE(cros_ec_sync_synapse_actions),
> +		.signal = cros_ec_sync_signals
> +	}
> +};
> +
> +static struct counter_count cros_ec_sync_counts[] = {
> +	{
> +		.id = 0,
> +		.name = "vsync",
> +		.functions_list = cros_ec_sync_functions,
> +		.num_functions = ARRAY_SIZE(cros_ec_sync_functions),
> +		.synapses = cros_ec_sync_synapses,
> +		.num_synapses = ARRAY_SIZE(cros_ec_sync_synapses),
> +	}
> +};
> +
> +static int cros_ec_sync_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct iio_dev *indio_dev;
> +	struct cros_ec_sync_state *state;
> +	struct iio_chan_spec *channel;
> +	int ret;
> +
> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> +	if (!indio_dev)
> +		return -ENOMEM;
> +
> +	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> +					cros_ec_sensors_capture,
> +					cros_ec_sensors_push_data);
> +	if (ret)
> +		return ret;
> +
> +	indio_dev->info = &cros_ec_sync_info;
> +	state = iio_priv(indio_dev);
> +
> +	/* Initialize IIO device */
> +	channel = state->channels;
> +
> +	/* Counter channel */
> +	channel->type = IIO_COUNT;
> +	channel->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> +	channel->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE);
> +	channel->scan_type.realbits = CROS_EC_SENSOR_BITS;
> +	channel->scan_type.storagebits = CROS_EC_SENSOR_BITS;
> +	channel->scan_type.shift = 0;
> +	channel->scan_index = 0;
> +	channel->ext_info = cros_ec_sensors_limited_info;
> +	channel->scan_type.sign = 'u';
> +
> +	/* Timestamp channel */
> +	channel++;
> +	channel->type = IIO_TIMESTAMP;
> +	channel->channel = -1;
> +	channel->scan_index = 1;
> +	channel->scan_type.sign = 's';
> +	channel->scan_type.realbits = 64;
> +	channel->scan_type.storagebits = 64;
> +
> +	indio_dev->channels = state->channels;
> +	indio_dev->num_channels = MAX_CHANNELS;
> +
> +	state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
> +
> +	/* Initialize Counter device */
> +	state->counter.name = dev_name(&pdev->dev);
> +	state->counter.parent = &pdev->dev;
> +	state->counter.ops = &cros_ec_sync_ops;
> +	state->counter.counts = cros_ec_sync_counts;
> +	state->counter.num_counts = ARRAY_SIZE(cros_ec_sync_counts);
> +	state->counter.signals = cros_ec_sync_signals;
> +	state->counter.num_signals = ARRAY_SIZE(cros_ec_sync_signals);
> +	state->counter.priv = state;
> +
> +	ret = devm_iio_device_register(dev, indio_dev);
> +	if (ret)
> +		return ret;
> +
> +	return devm_counter_register(dev, &state->counter);
> +}
> +
> +static const struct platform_device_id cros_ec_sync_ids[] = {
> +	{
> +		.name = "cros-ec-sync",
> +	},
> +	{ /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(platform, cros_ec_sync_ids);
> +
> +static struct platform_driver cros_ec_sync_platform_driver = {
> +	.driver = {
> +		.name	= "cros-ec-sync",
> +	},
> +	.probe		= cros_ec_sync_probe,
> +	.id_table	= cros_ec_sync_ids,
> +};
> +module_platform_driver(cros_ec_sync_platform_driver);
> +
> +MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
> +MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
> +MODULE_LICENSE("GPL v2");
> 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 c831915ca7e56..3a15094616710 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
> @@ -31,6 +31,7 @@
>  static char *cros_ec_loc[] = {
>  	[MOTIONSENSE_LOC_BASE] = "base",
>  	[MOTIONSENSE_LOC_LID] = "lid",
> +	[MOTIONSENSE_LOC_CAMERA] = "camera",
>  	[MOTIONSENSE_LOC_MAX] = "unknown",
>  };
>  
> @@ -467,6 +468,20 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
>  };
>  EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
>  
> +const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
> +	{
> +		.name = "id",
> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = cros_ec_sensors_id
> +	},
> +	{
> +		.name = "location",
> +		.shared = IIO_SHARED_BY_ALL,
> +		.read = cros_ec_sensors_loc
> +	},
> +	{ },
> +};
> +EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
>  /**
>   * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
>   * @st:		pointer to state information for device
> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
> index b7f2c00db5e1e..e4ae0868d1e06 100644
> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> @@ -106,6 +106,9 @@ static int cros_ec_sensorhub_register(struct device *dev,
>  		case MOTIONSENSE_TYPE_ACTIVITY:
>  			name = "cros-ec-activity";
>  			break;
> +		case MOTIONSENSE_TYPE_SYNC:
> +			name = "cros-ec-sync";
> +			break;
>  		default:
>  			dev_warn(dev, "unknown type %d\n",
>  				 sensorhub->resp->info.type);
> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> index 7bc961defa87e..e416b28cf24c7 100644
> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> @@ -114,7 +114,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
>  			       struct iio_chan_spec const *chan,
>  			       int val, int val2, long mask);
>  
> -/* List of extended channel specification for all sensors */
> +/* List of extended channel specification for all sensors. */
> +extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
> +/* Add calibration to set above. */
>  extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
>  extern const struct attribute *cros_ec_sensor_fifo_attributes[];
>  


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drivers: counter: Add Cros EC Sync counter
  2020-04-11 15:00 ` William Breathitt Gray
@ 2020-04-13 15:17   ` Enric Balletbo i Serra
  2020-04-13 19:26     ` Gwendal Grignou
  0 siblings, 1 reply; 6+ messages in thread
From: Enric Balletbo i Serra @ 2020-04-13 15:17 UTC (permalink / raw)
  To: William Breathitt Gray, Gwendal Grignou
  Cc: bleung, jic23, groeck, linux-kernel, linux-iio

Hi Gwendal, William,

Thank you for the patch. Some few comments below.

On 11/4/20 17:00, William Breathitt Gray wrote:
> On Tue, Apr 07, 2020 at 10:36:55PM -0700, Gwendal Grignou wrote:
>> When the camera vsync pin is connected to the embedded controller (EC) of
>> a chromebook, the EC reports a sensor with a counter that increases
>> at each GPIO rising edge.
>>
>> The sensor is presented using the counter subsystem.
>> In addition, it is also presented via the IIO subsystem with a timestamp,
>> allowing synchronisation with sensors connected to the same EC, for
>> image stabilisation or augmented reality applications.
>>
>> To enable the counter:
>> via counter ABI:
>> echo "rising edge" > counterX/count0/signal_action
>> via iio ABI
>> echo 1 > iio:deviceY/en
>>
>> To disable the counter:
>> via counter ABI:
>> echo "none" > counterX/count0/signal_action
>> via iio ABI
>> echo 0 > iio:deviceY/en
>>
>> To read the current counter value:
>> via counter ABI:
>> cat counterX/count0/count
>> via iio ABI
>> cat iio:deviceY/in_count_raw
>> We can also read the value through the IIO buffer:
>> echo 1 > iio:deviceY/scan_elements/in_timestamp_en
>> echo 1 > iio:deviceY/scan_elements/in_count_en
>> echo 1 > iio:deviceY/buffer/enable
>>
>> and each time to counter increase, the following binary blob
>> will be appended to dev/iio:deviceY:
>> 000f 0000 0000 0000 dc66 816c 0071 0000
>>  \   <-- padding -> <-- timestamp ---->
>>   count
>>
>> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
>> ---
>> An initial version of the driver was presented few month ago at
>> https://patchwork.kernel.org/patch/11037009/
>>
>>  drivers/counter/Kconfig                       |  11 +
>>  drivers/counter/Makefile                      |   1 +
>>  drivers/counter/cros_ec_sync.c                | 342 ++++++++++++++++++
>>  .../cros_ec_sensors/cros_ec_sensors_core.c    |  15 +
>>  drivers/platform/chrome/cros_ec_sensorhub.c   |   3 +
>>  .../linux/iio/common/cros_ec_sensors_core.h   |   4 +-
>>  6 files changed, 375 insertions(+), 1 deletion(-)
>>  create mode 100644 drivers/counter/cros_ec_sync.c
> 
> Hi Gwendal,
> 
> I'm getting some conflicts when I try to merge this patch, so it looks
> like we're on different bases. Would you be able to rebase on the
> current testing branch of the IIO tree and resubmit this patch?
> 

I think this patch is based on latest patches from the chrome-platform tree,
those landed last week and are merged in current 5.7-rc1. I applies cleanly on
top of current 5.7-rc1 released yesterday.


> Thanks,
> 
> William Breathitt Gray
> 
>>
>> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
>> index c80fa76bb5311..18fde918ff40b 100644
>> --- a/drivers/counter/Kconfig
>> +++ b/drivers/counter/Kconfig
>> @@ -29,6 +29,17 @@ config 104_QUAD_8
>>  	  The base port addresses for the devices may be configured via the base
>>  	  array module parameter.
>>  
>> +config CROS_EC_SYNC
>> +	tristate "ChromeOS EC Counter driver"
>> +	depends on IIO_CROS_EC_SENSORS_CORE
>> +	help
>> +	  Module to handle synchronisation sensor presented by the ChromeOS EC
>> +	  Sensor hub.
>> +	  Synchronisation sensor sends event to the host when the camera
>> +	  take a picture. It allows synchronisation with other MEMS sensor,
>> +	  like gyroscope for image statbilization or augmented reality
>> +	  application (AR).
>> +
>>  config STM32_TIMER_CNT
>>  	tristate "STM32 Timer encoder counter driver"
>>  	depends on MFD_STM32_TIMERS || COMPILE_TEST
>> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
>> index 55142d1f4c436..98378fca50ad6 100644
>> --- a/drivers/counter/Makefile
>> +++ b/drivers/counter/Makefile
>> @@ -6,6 +6,7 @@
>>  obj-$(CONFIG_COUNTER) += counter.o
>>  
>>  obj-$(CONFIG_104_QUAD_8)	+= 104-quad-8.o
>> +obj-$(CONFIG_CROS_EC_SYNC)	+= cros_ec_sync.o
>>  obj-$(CONFIG_STM32_TIMER_CNT)	+= stm32-timer-cnt.o
>>  obj-$(CONFIG_STM32_LPTIMER_CNT)	+= stm32-lptimer-cnt.o
>>  obj-$(CONFIG_TI_EQEP)		+= ti-eqep.o
>> diff --git a/drivers/counter/cros_ec_sync.c b/drivers/counter/cros_ec_sync.c
>> new file mode 100644
>> index 0000000000000..94b3ababab4bd
>> --- /dev/null
>> +++ b/drivers/counter/cros_ec_sync.c
>> @@ -0,0 +1,342 @@
>> +// SPDX-License-Identifier: GPL-2.0

Following other ChromeOS drivers this should be GPL-2.0-only

>> +/*
>> + * Driver for synchronisation sensor behind CrOS EC.
>> + *
>> + * Copyright 2020 Google, Inc

Copyright 2020 Google LLC.

>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *

Remove the license boiler plate it is redundand and already specified in the
SPDX license tag.

>> + * This driver uses the cros-ec interface to communicate with the Chrome OS
>> + * EC about counter sensors. Counters are presented through
>> + * iio sysfs.
>> + */
>> +
>> +#include <linux/counter.h>
>> +#include <linux/device.h>
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/common/cros_ec_sensors_core.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_data/cros_ec_commands.h>
>> +#include <linux/platform_data/cros_ec_proto.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +
>> +/*
>> + * One channel for counter, the other for timestamp.
>> + */
>> +#define MAX_CHANNELS (1 + 1)
>> +
>> +/**
>> + * struct cros_ec_sync_state - device structure
>> + *
>> + * @core: common structure for all cros_ec sensor.
>> + *        Must be at the beggining.
>> + * @channels: presented iio channels(2).
>> + * @counter: counter data structure.
>> + */
>> +struct cros_ec_sync_state {
>> +	struct cros_ec_sensors_core_state core;
>> +	struct iio_chan_spec channels[MAX_CHANNELS];
>> +	struct counter_device counter;
>> +};
>> +
>> +/**
>> + * cros_ec_sync_get_enable() - Check if the counter is enabled.
>> + *
>> + * @st:     core cros_ec sensor
>> + * @val:    status: 0: disabled, 1 enabled.
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +static int cros_ec_sync_get_enable(struct cros_ec_sensors_core_state *st,
>> +				   int *val)
>> +{
>> +	int ret;
>> +
>> +	mutex_lock(&st->cmd_lock);
>> +	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
>> +	st->param.sensor_odr.data = EC_MOTION_SENSE_NO_VALUE;
>> +
>> +	ret = cros_ec_motion_send_host_cmd(st, 0);
>> +	mutex_unlock(&st->cmd_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	*val = !!st->resp->sensor_odr.ret;
>> +	return 0;
>> +}
>> +
>> +/**
>> + * cros_ec_sync_set_enable() - Allow the counter to count.
>> + *
>> + * When enable, the counter will increase for each VSYNC rising edge
>> + * and will produce an event in the iio buffer, if enabled.
>> + *
>> + * @st:     core cros_ec sensor
>> + * @val:    0: disable, 1 enable.
>> + *
>> + * Return: 0 on success, -errno on failure.
>> + */
>> +static int cros_ec_sync_set_enable(struct cros_ec_sensors_core_state *st,
>> +				   int val)
>> +{
>> +	int ret;
>> +
>> +	mutex_lock(&st->cmd_lock);
>> +	st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
>> +	st->param.sensor_odr.data = val;
>> +	st->param.sensor_odr.roundup = 1;
>> +
>> +	ret = cros_ec_motion_send_host_cmd(st, 0);
>> +	mutex_unlock(&st->cmd_lock);
>> +	return ret;
>> +}
>> +
>> +static int cros_ec_sync_iio_read(struct iio_dev *indio_dev,
>> +				 struct iio_chan_spec const *chan,
>> +				 int *val, int *val2, long mask)
>> +{
>> +	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>> +	u16 data;
>> +	int ret;
>> +	int idx = chan->scan_index;
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_RAW:
>> +		mutex_lock(&st->cmd_lock);

I think you need to review the mutex locking logic.

>> +		ret = cros_ec_sensors_read_cmd(indio_dev, 1 << idx, &data);
>> +		mutex_unlock(&st->cmd_lock);
>> +		if (ret < 0)
>> +			break;
>> +		ret = IIO_VAL_INT;
>> +		*val = data;
>> +		break;
>> +	case IIO_CHAN_INFO_ENABLE:
>> +		ret = cros_ec_sync_get_enable(st, val);
>> +		if (ret < 0)
>> +			break;
>> +		ret = IIO_VAL_INT;
>> +		break;
>> +	default:
>> +		ret = -EINVAL;
>> +		break;
>> +	}
>> +	mutex_unlock(&st->cmd_lock);

Where did you lock the mutex?

>> +	return ret;
>> +}
>> +
>> +static int cros_ec_sync_iio_write(struct iio_dev *indio_dev,
>> +				  struct iio_chan_spec const *chan,
>> +				  int val, int val2, long mask)
>> +{
>> +	struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
>> +
>> +	switch (mask) {
>> +	case IIO_CHAN_INFO_ENABLE:
>> +		if (val < 0 || val > 1)
>> +			return -EINVAL;
>> +
>> +		return cros_ec_sync_set_enable(st, val);
>> +
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +static const struct iio_info cros_ec_sync_info = {
>> +	.read_raw = &cros_ec_sync_iio_read,
>> +	.write_raw = &cros_ec_sync_iio_write,
>> +};
>> +
>> +/* The counter can only increase, so only one function present. */
>> +static enum counter_count_function cros_ec_sync_functions[] = {
>> +	COUNTER_COUNT_FUNCTION_INCREASE,
>> +};
>> +
>> +/* 2 synapse actions allowed: count for each rising edge, or not. */
>> +static enum counter_synapse_action cros_ec_sync_synapse_actions[] = {
>> +	COUNTER_SYNAPSE_ACTION_NONE,
>> +	COUNTER_SYNAPSE_ACTION_RISING_EDGE,
>> +};
>> +
>> +static int cros_ec_sync_read_count(struct counter_device *counter,
>> +				   struct counter_count *count,
>> +				   unsigned long *val)
>> +{
>> +	struct cros_ec_sensors_core_state *st = counter->priv;
>> +	u16 raw;
>> +	int ret;
>> +
>> +	mutex_lock(&st->cmd_lock);
>> +	ret = cros_ec_sensors_read_cmd(iio_priv_to_dev(st), 1, &raw);
>> +	mutex_unlock(&st->cmd_lock);
>> +	if (ret)
>> +		return ret;
>> +
>> +	*val = raw;
>> +	return 0;
>> +}
>> +
>> +static int cros_ec_sync_function_get(struct counter_device *counter,
>> +				     struct counter_count *count,
>> +				     size_t *function)
>> +{
>> +	*function = 0;
>> +	return 0;
>> +}
>> +
>> +static int cros_ec_sync_action_get(struct counter_device *counter,
>> +				   struct counter_count *count,
>> +				   struct counter_synapse *synapse,
>> +				   size_t *action)
>> +{
>> +	struct cros_ec_sensors_core_state *st = counter->priv;
>> +	int ret;
>> +	int raw;
>> +
>> +	ret = cros_ec_sync_get_enable(st, &raw);
>> +	if (ret)
>> +		return ret;
>> +
>> +	*action = !!raw;
>> +	return 0;
>> +}
>> +
>> +static int cros_ec_sync_action_set(struct counter_device *counter,
>> +				   struct counter_count *count,
>> +				   struct counter_synapse *synapse,
>> +				   size_t action)
>> +{
>> +	struct cros_ec_sensors_core_state *st = counter->priv;
>> +
>> +	return cros_ec_sync_set_enable(st, action);
>> +}
>> +
>> +static const struct counter_ops cros_ec_sync_ops = {
>> +	.count_read = cros_ec_sync_read_count,
>> +	.function_get = cros_ec_sync_function_get,
>> +	.action_get = cros_ec_sync_action_get,
>> +	.action_set = cros_ec_sync_action_set,
>> +};
>> +
>> +static struct counter_signal cros_ec_sync_signals[] = {
>> +	{
>> +		.id = 0,
>> +		.name = "vsync"
>> +	}
>> +};
>> +
>> +static struct counter_synapse cros_ec_sync_synapses[] = {
>> +	{
>> +		.actions_list = cros_ec_sync_synapse_actions,
>> +		.num_actions = ARRAY_SIZE(cros_ec_sync_synapse_actions),
>> +		.signal = cros_ec_sync_signals
>> +	}
>> +};
>> +
>> +static struct counter_count cros_ec_sync_counts[] = {
>> +	{
>> +		.id = 0,
>> +		.name = "vsync",
>> +		.functions_list = cros_ec_sync_functions,
>> +		.num_functions = ARRAY_SIZE(cros_ec_sync_functions),
>> +		.synapses = cros_ec_sync_synapses,
>> +		.num_synapses = ARRAY_SIZE(cros_ec_sync_synapses),
>> +	}
>> +};
>> +
>> +static int cros_ec_sync_probe(struct platform_device *pdev)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct iio_dev *indio_dev;
>> +	struct cros_ec_sync_state *state;
>> +	struct iio_chan_spec *channel;
>> +	int ret;
>> +
>> +	indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
>> +	if (!indio_dev)
>> +		return -ENOMEM;
>> +
>> +	ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
>> +					cros_ec_sensors_capture,
>> +					cros_ec_sensors_push_data);
>> +	if (ret)
>> +		return ret;
>> +
>> +	indio_dev->info = &cros_ec_sync_info;
>> +	state = iio_priv(indio_dev);
>> +
>> +	/* Initialize IIO device */
>> +	channel = state->channels;
>> +
>> +	/* Counter channel */
>> +	channel->type = IIO_COUNT;
>> +	channel->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> +	channel->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE);
>> +	channel->scan_type.realbits = CROS_EC_SENSOR_BITS;
>> +	channel->scan_type.storagebits = CROS_EC_SENSOR_BITS;
>> +	channel->scan_type.shift = 0;
>> +	channel->scan_index = 0;
>> +	channel->ext_info = cros_ec_sensors_limited_info;
>> +	channel->scan_type.sign = 'u';
>> +
>> +	/* Timestamp channel */
>> +	channel++;
>> +	channel->type = IIO_TIMESTAMP;
>> +	channel->channel = -1;
>> +	channel->scan_index = 1;
>> +	channel->scan_type.sign = 's';
>> +	channel->scan_type.realbits = 64;
>> +	channel->scan_type.storagebits = 64;
>> +
>> +	indio_dev->channels = state->channels;
>> +	indio_dev->num_channels = MAX_CHANNELS;
>> +
>> +	state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
>> +
>> +	/* Initialize Counter device */
>> +	state->counter.name = dev_name(&pdev->dev);
>> +	state->counter.parent = &pdev->dev;
>> +	state->counter.ops = &cros_ec_sync_ops;
>> +	state->counter.counts = cros_ec_sync_counts;
>> +	state->counter.num_counts = ARRAY_SIZE(cros_ec_sync_counts);
>> +	state->counter.signals = cros_ec_sync_signals;
>> +	state->counter.num_signals = ARRAY_SIZE(cros_ec_sync_signals);
>> +	state->counter.priv = state;
>> +
>> +	ret = devm_iio_device_register(dev, indio_dev);
>> +	if (ret)
>> +		return ret;
>> +
>> +	return devm_counter_register(dev, &state->counter);
>> +}
>> +
>> +static const struct platform_device_id cros_ec_sync_ids[] = {
>> +	{
>> +		.name = "cros-ec-sync",
>> +	},
>> +	{ /* sentinel */ }
>> +};
>> +MODULE_DEVICE_TABLE(platform, cros_ec_sync_ids);
>> +
>> +static struct platform_driver cros_ec_sync_platform_driver = {
>> +	.driver = {
>> +		.name	= "cros-ec-sync",
>> +	},
>> +	.probe		= cros_ec_sync_probe,
>> +	.id_table	= cros_ec_sync_ids,
>> +};
>> +module_platform_driver(cros_ec_sync_platform_driver);
>> +
>> +MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
>> +MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
>> +MODULE_LICENSE("GPL v2");
>> 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 c831915ca7e56..3a15094616710 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
>> @@ -31,6 +31,7 @@
>>  static char *cros_ec_loc[] = {
>>  	[MOTIONSENSE_LOC_BASE] = "base",
>>  	[MOTIONSENSE_LOC_LID] = "lid",
>> +	[MOTIONSENSE_LOC_CAMERA] = "camera",
>>  	[MOTIONSENSE_LOC_MAX] = "unknown",
>>  };
>>  
>> @@ -467,6 +468,20 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
>>  };
>>  EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
>>  
>> +const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
>> +	{
>> +		.name = "id",
>> +		.shared = IIO_SHARED_BY_ALL,
>> +		.read = cros_ec_sensors_id
>> +	},
>> +	{
>> +		.name = "location",
>> +		.shared = IIO_SHARED_BY_ALL,
>> +		.read = cros_ec_sensors_loc
>> +	},
>> +	{ },
>> +};
>> +EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
>>  /**
>>   * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
>>   * @st:		pointer to state information for device
>> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
>> index b7f2c00db5e1e..e4ae0868d1e06 100644
>> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
>> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
>> @@ -106,6 +106,9 @@ static int cros_ec_sensorhub_register(struct device *dev,
>>  		case MOTIONSENSE_TYPE_ACTIVITY:
>>  			name = "cros-ec-activity";
>>  			break;
>> +		case MOTIONSENSE_TYPE_SYNC:
>> +			name = "cros-ec-sync";
>> +			break;
>>  		default:
>>  			dev_warn(dev, "unknown type %d\n",
>>  				 sensorhub->resp->info.type);
>> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
>> index 7bc961defa87e..e416b28cf24c7 100644
>> --- a/include/linux/iio/common/cros_ec_sensors_core.h
>> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
>> @@ -114,7 +114,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
>>  			       struct iio_chan_spec const *chan,
>>  			       int val, int val2, long mask);
>>  
>> -/* List of extended channel specification for all sensors */
>> +/* List of extended channel specification for all sensors. */
>> +extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
>> +/* Add calibration to set above. */

This change is unrelated to this patch.

>>  extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
>>  extern const struct attribute *cros_ec_sensor_fifo_attributes[];
>>  
>> -- 
>> 2.26.0.110.g2183baf09c-goog
>>

Thanks,
 Enric

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] drivers: counter: Add Cros EC Sync counter
  2020-04-13 15:17   ` Enric Balletbo i Serra
@ 2020-04-13 19:26     ` Gwendal Grignou
  0 siblings, 0 replies; 6+ messages in thread
From: Gwendal Grignou @ 2020-04-13 19:26 UTC (permalink / raw)
  To: Enric Balletbo i Serra
  Cc: William Breathitt Gray, Benson Leung, Jonathan Cameron,
	Guenter Roeck, linux-kernel, linux-iio

On Mon, Apr 13, 2020 at 8:17 AM Enric Balletbo i Serra
<enric.balletbo@collabora.com> wrote:
>
> Hi Gwendal, William,
>
> Thank you for the patch. Some few comments below.
>
> On 11/4/20 17:00, William Breathitt Gray wrote:
> > On Tue, Apr 07, 2020 at 10:36:55PM -0700, Gwendal Grignou wrote:
> >> When the camera vsync pin is connected to the embedded controller (EC) of
> >> a chromebook, the EC reports a sensor with a counter that increases
> >> at each GPIO rising edge.
> >>
> >> The sensor is presented using the counter subsystem.
> >> In addition, it is also presented via the IIO subsystem with a timestamp,
> >> allowing synchronisation with sensors connected to the same EC, for
> >> image stabilisation or augmented reality applications.
> >>
> >> To enable the counter:
> >> via counter ABI:
> >> echo "rising edge" > counterX/count0/signal_action
> >> via iio ABI
> >> echo 1 > iio:deviceY/en
> >>
> >> To disable the counter:
> >> via counter ABI:
> >> echo "none" > counterX/count0/signal_action
> >> via iio ABI
> >> echo 0 > iio:deviceY/en
> >>
> >> To read the current counter value:
> >> via counter ABI:
> >> cat counterX/count0/count
> >> via iio ABI
> >> cat iio:deviceY/in_count_raw
> >> We can also read the value through the IIO buffer:
> >> echo 1 > iio:deviceY/scan_elements/in_timestamp_en
> >> echo 1 > iio:deviceY/scan_elements/in_count_en
> >> echo 1 > iio:deviceY/buffer/enable
> >>
> >> and each time to counter increase, the following binary blob
> >> will be appended to dev/iio:deviceY:
> >> 000f 0000 0000 0000 dc66 816c 0071 0000
> >>  \   <-- padding -> <-- timestamp ---->
> >>   count
> >>
> >> Signed-off-by: Gwendal Grignou <gwendal@chromium.org>
> >> ---
> >> An initial version of the driver was presented few month ago at
> >> https://patchwork.kernel.org/patch/11037009/
> >>
> >>  drivers/counter/Kconfig                       |  11 +
> >>  drivers/counter/Makefile                      |   1 +
> >>  drivers/counter/cros_ec_sync.c                | 342 ++++++++++++++++++
> >>  .../cros_ec_sensors/cros_ec_sensors_core.c    |  15 +
> >>  drivers/platform/chrome/cros_ec_sensorhub.c   |   3 +
> >>  .../linux/iio/common/cros_ec_sensors_core.h   |   4 +-
> >>  6 files changed, 375 insertions(+), 1 deletion(-)
> >>  create mode 100644 drivers/counter/cros_ec_sync.c
> >
> > Hi Gwendal,
> >
> > I'm getting some conflicts when I try to merge this patch, so it looks
> > like we're on different bases. Would you be able to rebase on the
> > current testing branch of the IIO tree and resubmit this patch?
> >
>
> I think this patch is based on latest patches from the chrome-platform tree,
> those landed last week and are merged in current 5.7-rc1. I applies cleanly on
> top of current 5.7-rc1 released yesterday.
>
>
> > Thanks,
> >
> > William Breathitt Gray
> >
> >>
> >> diff --git a/drivers/counter/Kconfig b/drivers/counter/Kconfig
> >> index c80fa76bb5311..18fde918ff40b 100644
> >> --- a/drivers/counter/Kconfig
> >> +++ b/drivers/counter/Kconfig
> >> @@ -29,6 +29,17 @@ config 104_QUAD_8
> >>        The base port addresses for the devices may be configured via the base
> >>        array module parameter.
> >>
> >> +config CROS_EC_SYNC
> >> +    tristate "ChromeOS EC Counter driver"
> >> +    depends on IIO_CROS_EC_SENSORS_CORE
> >> +    help
> >> +      Module to handle synchronisation sensor presented by the ChromeOS EC
> >> +      Sensor hub.
> >> +      Synchronisation sensor sends event to the host when the camera
> >> +      take a picture. It allows synchronisation with other MEMS sensor,
> >> +      like gyroscope for image statbilization or augmented reality
> >> +      application (AR).
> >> +
> >>  config STM32_TIMER_CNT
> >>      tristate "STM32 Timer encoder counter driver"
> >>      depends on MFD_STM32_TIMERS || COMPILE_TEST
> >> diff --git a/drivers/counter/Makefile b/drivers/counter/Makefile
> >> index 55142d1f4c436..98378fca50ad6 100644
> >> --- a/drivers/counter/Makefile
> >> +++ b/drivers/counter/Makefile
> >> @@ -6,6 +6,7 @@
> >>  obj-$(CONFIG_COUNTER) += counter.o
> >>
> >>  obj-$(CONFIG_104_QUAD_8)    += 104-quad-8.o
> >> +obj-$(CONFIG_CROS_EC_SYNC)  += cros_ec_sync.o
> >>  obj-$(CONFIG_STM32_TIMER_CNT)       += stm32-timer-cnt.o
> >>  obj-$(CONFIG_STM32_LPTIMER_CNT)     += stm32-lptimer-cnt.o
> >>  obj-$(CONFIG_TI_EQEP)               += ti-eqep.o
> >> diff --git a/drivers/counter/cros_ec_sync.c b/drivers/counter/cros_ec_sync.c
> >> new file mode 100644
> >> index 0000000000000..94b3ababab4bd
> >> --- /dev/null
> >> +++ b/drivers/counter/cros_ec_sync.c
> >> @@ -0,0 +1,342 @@
> >> +// SPDX-License-Identifier: GPL-2.0
>
> Following other ChromeOS drivers this should be GPL-2.0-only
>
> >> +/*
> >> + * Driver for synchronisation sensor behind CrOS EC.
> >> + *
> >> + * Copyright 2020 Google, Inc
>
> Copyright 2020 Google LLC.
Fixed in v2.
>
> >> + *
> >> + * This software is licensed under the terms of the GNU General Public
> >> + * License version 2, as published by the Free Software Foundation, and
> >> + * may be copied, distributed, and modified under those terms.
> >> + *
> >> + * This program is distributed in the hope that it will be useful,
> >> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> >> + * GNU General Public License for more details.
> >> + *
>
> Remove the license boiler plate it is redundand and already specified in the
> SPDX license tag.
Fixed in v2.
>
> >> + * This driver uses the cros-ec interface to communicate with the Chrome OS
> >> + * EC about counter sensors. Counters are presented through
> >> + * iio sysfs.
> >> + */
> >> +
> >> +#include <linux/counter.h>
> >> +#include <linux/device.h>
> >> +#include <linux/iio/buffer.h>
> >> +#include <linux/iio/common/cros_ec_sensors_core.h>
> >> +#include <linux/iio/iio.h>
> >> +#include <linux/kernel.h>
> >> +#include <linux/module.h>
> >> +#include <linux/platform_data/cros_ec_commands.h>
> >> +#include <linux/platform_data/cros_ec_proto.h>
> >> +#include <linux/platform_device.h>
> >> +#include <linux/slab.h>
> >> +
> >> +/*
> >> + * One channel for counter, the other for timestamp.
> >> + */
> >> +#define MAX_CHANNELS (1 + 1)
> >> +
> >> +/**
> >> + * struct cros_ec_sync_state - device structure
> >> + *
> >> + * @core: common structure for all cros_ec sensor.
> >> + *        Must be at the beggining.
> >> + * @channels: presented iio channels(2).
> >> + * @counter: counter data structure.
> >> + */
> >> +struct cros_ec_sync_state {
> >> +    struct cros_ec_sensors_core_state core;
> >> +    struct iio_chan_spec channels[MAX_CHANNELS];
> >> +    struct counter_device counter;
> >> +};
> >> +
> >> +/**
> >> + * cros_ec_sync_get_enable() - Check if the counter is enabled.
> >> + *
> >> + * @st:     core cros_ec sensor
> >> + * @val:    status: 0: disabled, 1 enabled.
> >> + *
> >> + * Return: 0 on success, -errno on failure.
> >> + */
> >> +static int cros_ec_sync_get_enable(struct cros_ec_sensors_core_state *st,
> >> +                               int *val)
> >> +{
> >> +    int ret;
> >> +
> >> +    mutex_lock(&st->cmd_lock);
> >> +    st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> >> +    st->param.sensor_odr.data = EC_MOTION_SENSE_NO_VALUE;
> >> +
> >> +    ret = cros_ec_motion_send_host_cmd(st, 0);
> >> +    mutex_unlock(&st->cmd_lock);
> >> +    if (ret)
> >> +            return ret;
> >> +
> >> +    *val = !!st->resp->sensor_odr.ret;
> >> +    return 0;
> >> +}
> >> +
> >> +/**
> >> + * cros_ec_sync_set_enable() - Allow the counter to count.
> >> + *
> >> + * When enable, the counter will increase for each VSYNC rising edge
> >> + * and will produce an event in the iio buffer, if enabled.
> >> + *
> >> + * @st:     core cros_ec sensor
> >> + * @val:    0: disable, 1 enable.
> >> + *
> >> + * Return: 0 on success, -errno on failure.
> >> + */
> >> +static int cros_ec_sync_set_enable(struct cros_ec_sensors_core_state *st,
> >> +                               int val)
> >> +{
> >> +    int ret;
> >> +
> >> +    mutex_lock(&st->cmd_lock);
> >> +    st->param.cmd = MOTIONSENSE_CMD_SENSOR_ODR;
> >> +    st->param.sensor_odr.data = val;
> >> +    st->param.sensor_odr.roundup = 1;
> >> +
> >> +    ret = cros_ec_motion_send_host_cmd(st, 0);
> >> +    mutex_unlock(&st->cmd_lock);
> >> +    return ret;
> >> +}
> >> +
> >> +static int cros_ec_sync_iio_read(struct iio_dev *indio_dev,
> >> +                             struct iio_chan_spec const *chan,
> >> +                             int *val, int *val2, long mask)
> >> +{
> >> +    struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> >> +    u16 data;
> >> +    int ret;
> >> +    int idx = chan->scan_index;
> >> +
> >> +    switch (mask) {
> >> +    case IIO_CHAN_INFO_RAW:
> >> +            mutex_lock(&st->cmd_lock);
>
> I think you need to review the mutex locking logic.
>
> >> +            ret = cros_ec_sensors_read_cmd(indio_dev, 1 << idx, &data);
> >> +            mutex_unlock(&st->cmd_lock);
> >> +            if (ret < 0)
> >> +                    break;
> >> +            ret = IIO_VAL_INT;
> >> +            *val = data;
> >> +            break;
> >> +    case IIO_CHAN_INFO_ENABLE:
> >> +            ret = cros_ec_sync_get_enable(st, val);
> >> +            if (ret < 0)
> >> +                    break;
> >> +            ret = IIO_VAL_INT;
> >> +            break;
> >> +    default:
> >> +            ret = -EINVAL;
> >> +            break;
> >> +    }
> >> +    mutex_unlock(&st->cmd_lock);
>
> Where did you lock the mutex?
Fixed in v2. That line is an error.
>
> >> +    return ret;
> >> +}
> >> +
> >> +static int cros_ec_sync_iio_write(struct iio_dev *indio_dev,
> >> +                              struct iio_chan_spec const *chan,
> >> +                              int val, int val2, long mask)
> >> +{
> >> +    struct cros_ec_sensors_core_state *st = iio_priv(indio_dev);
> >> +
> >> +    switch (mask) {
> >> +    case IIO_CHAN_INFO_ENABLE:
> >> +            if (val < 0 || val > 1)
> >> +                    return -EINVAL;
> >> +
> >> +            return cros_ec_sync_set_enable(st, val);
> >> +
> >> +    default:
> >> +            return -EINVAL;
> >> +    }
> >> +}
> >> +
> >> +static const struct iio_info cros_ec_sync_info = {
> >> +    .read_raw = &cros_ec_sync_iio_read,
> >> +    .write_raw = &cros_ec_sync_iio_write,
> >> +};
> >> +
> >> +/* The counter can only increase, so only one function present. */
> >> +static enum counter_count_function cros_ec_sync_functions[] = {
> >> +    COUNTER_COUNT_FUNCTION_INCREASE,
> >> +};
> >> +
> >> +/* 2 synapse actions allowed: count for each rising edge, or not. */
> >> +static enum counter_synapse_action cros_ec_sync_synapse_actions[] = {
> >> +    COUNTER_SYNAPSE_ACTION_NONE,
> >> +    COUNTER_SYNAPSE_ACTION_RISING_EDGE,
> >> +};
> >> +
> >> +static int cros_ec_sync_read_count(struct counter_device *counter,
> >> +                               struct counter_count *count,
> >> +                               unsigned long *val)
> >> +{
> >> +    struct cros_ec_sensors_core_state *st = counter->priv;
> >> +    u16 raw;
> >> +    int ret;
> >> +
> >> +    mutex_lock(&st->cmd_lock);
> >> +    ret = cros_ec_sensors_read_cmd(iio_priv_to_dev(st), 1, &raw);
> >> +    mutex_unlock(&st->cmd_lock);
> >> +    if (ret)
> >> +            return ret;
> >> +
> >> +    *val = raw;
> >> +    return 0;
> >> +}
> >> +
> >> +static int cros_ec_sync_function_get(struct counter_device *counter,
> >> +                                 struct counter_count *count,
> >> +                                 size_t *function)
> >> +{
> >> +    *function = 0;
> >> +    return 0;
> >> +}
> >> +
> >> +static int cros_ec_sync_action_get(struct counter_device *counter,
> >> +                               struct counter_count *count,
> >> +                               struct counter_synapse *synapse,
> >> +                               size_t *action)
> >> +{
> >> +    struct cros_ec_sensors_core_state *st = counter->priv;
> >> +    int ret;
> >> +    int raw;
> >> +
> >> +    ret = cros_ec_sync_get_enable(st, &raw);
> >> +    if (ret)
> >> +            return ret;
> >> +
> >> +    *action = !!raw;
> >> +    return 0;
> >> +}
> >> +
> >> +static int cros_ec_sync_action_set(struct counter_device *counter,
> >> +                               struct counter_count *count,
> >> +                               struct counter_synapse *synapse,
> >> +                               size_t action)
> >> +{
> >> +    struct cros_ec_sensors_core_state *st = counter->priv;
> >> +
> >> +    return cros_ec_sync_set_enable(st, action);
> >> +}
> >> +
> >> +static const struct counter_ops cros_ec_sync_ops = {
> >> +    .count_read = cros_ec_sync_read_count,
> >> +    .function_get = cros_ec_sync_function_get,
> >> +    .action_get = cros_ec_sync_action_get,
> >> +    .action_set = cros_ec_sync_action_set,
> >> +};
> >> +
> >> +static struct counter_signal cros_ec_sync_signals[] = {
> >> +    {
> >> +            .id = 0,
> >> +            .name = "vsync"
> >> +    }
> >> +};
> >> +
> >> +static struct counter_synapse cros_ec_sync_synapses[] = {
> >> +    {
> >> +            .actions_list = cros_ec_sync_synapse_actions,
> >> +            .num_actions = ARRAY_SIZE(cros_ec_sync_synapse_actions),
> >> +            .signal = cros_ec_sync_signals
> >> +    }
> >> +};
> >> +
> >> +static struct counter_count cros_ec_sync_counts[] = {
> >> +    {
> >> +            .id = 0,
> >> +            .name = "vsync",
> >> +            .functions_list = cros_ec_sync_functions,
> >> +            .num_functions = ARRAY_SIZE(cros_ec_sync_functions),
> >> +            .synapses = cros_ec_sync_synapses,
> >> +            .num_synapses = ARRAY_SIZE(cros_ec_sync_synapses),
> >> +    }
> >> +};
> >> +
> >> +static int cros_ec_sync_probe(struct platform_device *pdev)
> >> +{
> >> +    struct device *dev = &pdev->dev;
> >> +    struct iio_dev *indio_dev;
> >> +    struct cros_ec_sync_state *state;
> >> +    struct iio_chan_spec *channel;
> >> +    int ret;
> >> +
> >> +    indio_dev = devm_iio_device_alloc(dev, sizeof(*state));
> >> +    if (!indio_dev)
> >> +            return -ENOMEM;
> >> +
> >> +    ret = cros_ec_sensors_core_init(pdev, indio_dev, true,
> >> +                                    cros_ec_sensors_capture,
> >> +                                    cros_ec_sensors_push_data);
> >> +    if (ret)
> >> +            return ret;
> >> +
> >> +    indio_dev->info = &cros_ec_sync_info;
> >> +    state = iio_priv(indio_dev);
> >> +
> >> +    /* Initialize IIO device */
> >> +    channel = state->channels;
> >> +
> >> +    /* Counter channel */
> >> +    channel->type = IIO_COUNT;
> >> +    channel->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> >> +    channel->info_mask_shared_by_all = BIT(IIO_CHAN_INFO_ENABLE);
> >> +    channel->scan_type.realbits = CROS_EC_SENSOR_BITS;
> >> +    channel->scan_type.storagebits = CROS_EC_SENSOR_BITS;
> >> +    channel->scan_type.shift = 0;
> >> +    channel->scan_index = 0;
> >> +    channel->ext_info = cros_ec_sensors_limited_info;
> >> +    channel->scan_type.sign = 'u';
> >> +
> >> +    /* Timestamp channel */
> >> +    channel++;
> >> +    channel->type = IIO_TIMESTAMP;
> >> +    channel->channel = -1;
> >> +    channel->scan_index = 1;
> >> +    channel->scan_type.sign = 's';
> >> +    channel->scan_type.realbits = 64;
> >> +    channel->scan_type.storagebits = 64;
> >> +
> >> +    indio_dev->channels = state->channels;
> >> +    indio_dev->num_channels = MAX_CHANNELS;
> >> +
> >> +    state->core.read_ec_sensors_data = cros_ec_sensors_read_cmd;
> >> +
> >> +    /* Initialize Counter device */
> >> +    state->counter.name = dev_name(&pdev->dev);
> >> +    state->counter.parent = &pdev->dev;
> >> +    state->counter.ops = &cros_ec_sync_ops;
> >> +    state->counter.counts = cros_ec_sync_counts;
> >> +    state->counter.num_counts = ARRAY_SIZE(cros_ec_sync_counts);
> >> +    state->counter.signals = cros_ec_sync_signals;
> >> +    state->counter.num_signals = ARRAY_SIZE(cros_ec_sync_signals);
> >> +    state->counter.priv = state;
> >> +
> >> +    ret = devm_iio_device_register(dev, indio_dev);
> >> +    if (ret)
> >> +            return ret;
> >> +
> >> +    return devm_counter_register(dev, &state->counter);
> >> +}
> >> +
> >> +static const struct platform_device_id cros_ec_sync_ids[] = {
> >> +    {
> >> +            .name = "cros-ec-sync",
> >> +    },
> >> +    { /* sentinel */ }
> >> +};
> >> +MODULE_DEVICE_TABLE(platform, cros_ec_sync_ids);
> >> +
> >> +static struct platform_driver cros_ec_sync_platform_driver = {
> >> +    .driver = {
> >> +            .name   = "cros-ec-sync",
> >> +    },
> >> +    .probe          = cros_ec_sync_probe,
> >> +    .id_table       = cros_ec_sync_ids,
> >> +};
> >> +module_platform_driver(cros_ec_sync_platform_driver);
> >> +
> >> +MODULE_AUTHOR("Gwendal Grignou <gwendal@chromium.org>");
> >> +MODULE_DESCRIPTION("ChromeOS EC synchronisation sensor driver");
> >> +MODULE_LICENSE("GPL v2");
> >> 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 c831915ca7e56..3a15094616710 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
> >> @@ -31,6 +31,7 @@
> >>  static char *cros_ec_loc[] = {
> >>      [MOTIONSENSE_LOC_BASE] = "base",
> >>      [MOTIONSENSE_LOC_LID] = "lid",
> >> +    [MOTIONSENSE_LOC_CAMERA] = "camera",
> >>      [MOTIONSENSE_LOC_MAX] = "unknown",
> >>  };
> >>
> >> @@ -467,6 +468,20 @@ const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[] = {
> >>  };
> >>  EXPORT_SYMBOL_GPL(cros_ec_sensors_ext_info);
> >>
> >> +const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[] = {
> >> +    {
> >> +            .name = "id",
> >> +            .shared = IIO_SHARED_BY_ALL,
> >> +            .read = cros_ec_sensors_id
> >> +    },
> >> +    {
> >> +            .name = "location",
> >> +            .shared = IIO_SHARED_BY_ALL,
> >> +            .read = cros_ec_sensors_loc
> >> +    },
> >> +    { },
> >> +};
> >> +EXPORT_SYMBOL_GPL(cros_ec_sensors_limited_info);
> >>  /**
> >>   * cros_ec_sensors_idx_to_reg - convert index into offset in shared memory
> >>   * @st:             pointer to state information for device
> >> diff --git a/drivers/platform/chrome/cros_ec_sensorhub.c b/drivers/platform/chrome/cros_ec_sensorhub.c
> >> index b7f2c00db5e1e..e4ae0868d1e06 100644
> >> --- a/drivers/platform/chrome/cros_ec_sensorhub.c
> >> +++ b/drivers/platform/chrome/cros_ec_sensorhub.c
> >> @@ -106,6 +106,9 @@ static int cros_ec_sensorhub_register(struct device *dev,
> >>              case MOTIONSENSE_TYPE_ACTIVITY:
> >>                      name = "cros-ec-activity";
> >>                      break;
> >> +            case MOTIONSENSE_TYPE_SYNC:
> >> +                    name = "cros-ec-sync";
> >> +                    break;
> >>              default:
> >>                      dev_warn(dev, "unknown type %d\n",
> >>                               sensorhub->resp->info.type);
> >> diff --git a/include/linux/iio/common/cros_ec_sensors_core.h b/include/linux/iio/common/cros_ec_sensors_core.h
> >> index 7bc961defa87e..e416b28cf24c7 100644
> >> --- a/include/linux/iio/common/cros_ec_sensors_core.h
> >> +++ b/include/linux/iio/common/cros_ec_sensors_core.h
> >> @@ -114,7 +114,9 @@ int cros_ec_sensors_core_write(struct cros_ec_sensors_core_state *st,
> >>                             struct iio_chan_spec const *chan,
> >>                             int val, int val2, long mask);
> >>
> >> -/* List of extended channel specification for all sensors */
> >> +/* List of extended channel specification for all sensors. */
> >> +extern const struct iio_chan_spec_ext_info cros_ec_sensors_limited_info[];
> >> +/* Add calibration to set above. */
>
> This change is unrelated to this patch.
It is needed because the sync sensor does not need calibration. It is
used when setting the ext_info of the sync channel.


>
> >>  extern const struct iio_chan_spec_ext_info cros_ec_sensors_ext_info[];
> >>  extern const struct attribute *cros_ec_sensor_fifo_attributes[];
> >>
> >> --
> >> 2.26.0.110.g2183baf09c-goog
> >>
>
> Thanks,
>  Enric

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-24 15:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-08  5:36 [PATCH] drivers: counter: Add Cros EC Sync counter Gwendal Grignou
2020-04-08  6:15 ` Gwendal Grignou
2020-04-11 15:00 ` William Breathitt Gray
2020-04-13 15:17   ` Enric Balletbo i Serra
2020-04-13 19:26     ` Gwendal Grignou
2020-04-12 11:06 ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).