| Message ID | 20200413195514.192868-1-gwendal@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drivers: counter: Add Cros EC Sync counter | expand |
Hi Gwendal, Thank you for your patch. On 13/4/20 21:55, 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> > --- > Changes in v2: > - fix license. > - remove extra unlock. > > drivers/counter/Kconfig | 11 + > drivers/counter/Makefile | 1 + > drivers/counter/cros_ec_sync.c | 332 ++++++++++++++++++ > .../cros_ec_sensors/cros_ec_sensors_core.c | 15 + > drivers/platform/chrome/cros_ec_sensorhub.c | 3 + From the chrome-platform side: Acked-by: Enric Balletbo i Serra <enric.balletbo@collabora.com> Hopefully the IIO and counter maintainers can give you more specific feedback. Thanks, Enric > .../linux/iio/common/cros_ec_sensors_core.h | 4 +- > 6 files changed, 365 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..4e644d651fde6 > --- /dev/null > +++ b/drivers/counter/cros_ec_sync.c > @@ -0,0 +1,332 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for synchronisation sensor behind CrOS EC. > + * > + * Copyright 2020 Google LLC. > + * > + * 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; > + } > + 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[]; > >
On Mon, Apr 13, 2020 at 12:55:14PM -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. Hi Gwendal, Sorry for the delay. I have some changes requested below. > 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 Although in theory a user could manually disable the actions for a Signal, this is a very roundabout way of actually disabling the Count. It's better to expose an "enable" attribute to allow the users to perform this functionality; for example: echo 0 > counterX/count0/enable echo 1 > counterX/count0/enable > > To read the current counter value: > via counter ABI: > cat counterX/count0/count > via iio ABI > cat iio:deviceY/in_count_raw I know we discussed this in the last review but it's still the same as before: IIO_COUNT interface is deprecated so new drivers won't be allowed to use it. You'll have to remove the IIO_COUNT code in this driver and replace it with Counter subsystem equivalents. > 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> > --- > Changes in v2: > - fix license. > - remove extra unlock. > > drivers/counter/Kconfig | 11 + > drivers/counter/Makefile | 1 + > drivers/counter/cros_ec_sync.c | 332 ++++++++++++++++++ > .../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, 365 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..4e644d651fde6 > --- /dev/null > +++ b/drivers/counter/cros_ec_sync.c > @@ -0,0 +1,332 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Driver for synchronisation sensor behind CrOS EC. > + * > + * Copyright 2020 Google LLC. > + * > + * 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; > + } > + 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; It's somewhat unclear at first glance which function correlates to index position 0 here. Create an enum structure and use the enum constants to set the index of the functions in cros_ec_sync_functions. Then you can set *function to the corresponding enum constant here -- that should make it clear which function is returned. Take a look at the drivers/counter/ftm-quaddec.c file for a good example of how it handles the function_get callback. > + 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; For the same reason as the function_get callback above, I'd like for *action to be explicitly set to a respective named constant. > + 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, It doesn't look like setting the action is actually a feature of this device. I'd recommend just dropping the action_set callback all together and instead implementing an "enable" attribute for your Count to handle that functionality. > +}; > + > +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'; As I mentioned earlier, this counter channel should be completely removed. You'll need to reimplement the cros_ec_sensors_limited_info attributes using the Counter subsystem. I'll explain this more below. > + > + /* 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 > + }, > + { }, > +}; Instead of an iio_chan_spec_ext_info structure, you can reimplement this as a counter_count_ext structure. It'll look something like this: const struct counter_count_ext cros_ec_sensors_limited_info[] = { { .name = "id", .read = cros_ec_sensors_id }, { .name = "location", .read = cros_ec_sensors_loc }, }; Note however that cros_ec_sensors_id and cros_ec_sensors_loc will need to match expected function format for counter_count_ext read callbacks; that means you'll need to implement proper versions of these two functions. William Breathitt Gray > +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 >
On Tue, Apr 14, 2020 at 1:48 PM William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > > On Mon, Apr 13, 2020 at 12:55:14PM -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. > > Hi Gwendal, > > Sorry for the delay. I have some changes requested below. > > > 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 > > Although in theory a user could manually disable the actions for a > Signal, this is a very roundabout way of actually disabling the Count. > It's better to expose an "enable" attribute to allow the users to > perform this functionality; for example: > > echo 0 > counterX/count0/enable > echo 1 > counterX/count0/enable > > > > > To read the current counter value: > > via counter ABI: > > cat counterX/count0/count > > via iio ABI > > cat iio:deviceY/in_count_raw > > I know we discussed this in the last review but it's still the same as > before: IIO_COUNT interface is deprecated so new drivers won't be > allowed to use it. You'll have to remove the IIO_COUNT code in this > driver and replace it with Counter subsystem equivalents. I understand the need of a clean separation between counter and IIO subsystems. I will wait for counter to offer a way to gather timestamp'ed counts. Do you have a plan/proposed ABI you can share? Thanks, Gwendal.
On Mon, Apr 20, 2020 at 11:54:16AM -0700, Gwendal Grignou wrote: > On Tue, Apr 14, 2020 at 1:48 PM William Breathitt Gray > <vilhelm.gray@gmail.com> wrote: > > > > On Mon, Apr 13, 2020 at 12:55:14PM -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. > > > > Hi Gwendal, > > > > Sorry for the delay. I have some changes requested below. > > > > > 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 > > > > Although in theory a user could manually disable the actions for a > > Signal, this is a very roundabout way of actually disabling the Count. > > It's better to expose an "enable" attribute to allow the users to > > perform this functionality; for example: > > > > echo 0 > counterX/count0/enable > > echo 1 > counterX/count0/enable > > > > > > > > To read the current counter value: > > > via counter ABI: > > > cat counterX/count0/count > > > via iio ABI > > > cat iio:deviceY/in_count_raw > > > > I know we discussed this in the last review but it's still the same as > > before: IIO_COUNT interface is deprecated so new drivers won't be > > allowed to use it. You'll have to remove the IIO_COUNT code in this > > driver and replace it with Counter subsystem equivalents. > I understand the need of a clean separation between counter and IIO subsystems. > I will wait for counter to offer a way to gather timestamp'ed counts. > Do you have a plan/proposed ABI you can share? > > Thanks, > > Gwendal. Hi Gwendal, I'm working on a reimplementation of the internals of the Counter subsystem: https://gitlab.com/vilhelmgray/iio/-/tree/counter_chardev I'm hoping to submit it to the mailing list later this week if I don't hit any delays; it'll include support as well for a character device interface for userspace application. Once those changes are merged into IIO, I'll submit a patch to add timestamp support -- hopefully within a week or two after. Right now I haven't yet chosen any specific format for timestamps, but I will likely match the format IIO subsystem currently has for its timestamp support so that migration is easier for these drivers. William Breathitt Gray
On Mon, 20 Apr 2020 15:08:10 -0400 William Breathitt Gray <vilhelm.gray@gmail.com> wrote: > On Mon, Apr 20, 2020 at 11:54:16AM -0700, Gwendal Grignou wrote: > > On Tue, Apr 14, 2020 at 1:48 PM William Breathitt Gray > > <vilhelm.gray@gmail.com> wrote: > > > > > > On Mon, Apr 13, 2020 at 12:55:14PM -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. > > > > > > Hi Gwendal, > > > > > > Sorry for the delay. I have some changes requested below. > > > > > > > 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 > > > > > > Although in theory a user could manually disable the actions for a > > > Signal, this is a very roundabout way of actually disabling the Count. > > > It's better to expose an "enable" attribute to allow the users to > > > perform this functionality; for example: > > > > > > echo 0 > counterX/count0/enable > > > echo 1 > counterX/count0/enable > > > > > > > > > > > To read the current counter value: > > > > via counter ABI: > > > > cat counterX/count0/count > > > > via iio ABI > > > > cat iio:deviceY/in_count_raw > > > > > > I know we discussed this in the last review but it's still the same as > > > before: IIO_COUNT interface is deprecated so new drivers won't be > > > allowed to use it. You'll have to remove the IIO_COUNT code in this > > > driver and replace it with Counter subsystem equivalents. > > I understand the need of a clean separation between counter and IIO subsystems. > > I will wait for counter to offer a way to gather timestamp'ed counts. > > Do you have a plan/proposed ABI you can share? > > > > Thanks, > > > > Gwendal. > > Hi Gwendal, > > I'm working on a reimplementation of the internals of the Counter > subsystem: https://gitlab.com/vilhelmgray/iio/-/tree/counter_chardev > > I'm hoping to submit it to the mailing list later this week if I don't > hit any delays; it'll include support as well for a character device > interface for userspace application. > > Once those changes are merged into IIO, I'll submit a patch to add > timestamp support -- hopefully within a week or two after. Right now I > haven't yet chosen any specific format for timestamps, but I will likely > match the format IIO subsystem currently has for its timestamp support > so that migration is easier for these drivers. > Don't copy our crazy clock choosing stuff. That's legacy rubbish for compatibility with a silly choice I made a long time ago. Pick a sensible clock type and stick to it. Jonathan > 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..4e644d651fde6 --- /dev/null +++ b/drivers/counter/cros_ec_sync.c @@ -0,0 +1,332 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Driver for synchronisation sensor behind CrOS EC. + * + * Copyright 2020 Google LLC. + * + * 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; + } + 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[];
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> --- Changes in v2: - fix license. - remove extra unlock. drivers/counter/Kconfig | 11 + drivers/counter/Makefile | 1 + drivers/counter/cros_ec_sync.c | 332 ++++++++++++++++++ .../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, 365 insertions(+), 1 deletion(-) create mode 100644 drivers/counter/cros_ec_sync.c